Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add modified OIDC authenticator #841

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Conversation

DiamondJoseph
Copy link
Contributor

Adds an OIDC Authenticator which negotiates with the ID provider rather than providing a specific algorithm

Checklist

  • Add a Changelog entry
  • Add the ticket number which this PR closes to the comment section

tiled/authenticators.py Outdated Show resolved Hide resolved
tiled/authenticators.py Outdated Show resolved Hide resolved
@DiamondJoseph DiamondJoseph force-pushed the oidc-from-well-known branch 2 times, most recently from 52695e2 to eab3d8d Compare January 10, 2025 16:13
Copy link
Member

@danielballan danielballan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great, thank you! Let me test it out locally before we merge.

@danielballan
Copy link
Member

I push some commits to get this closer to a working state:

  1. Revert the removal of confirmation_message. This is a tiled concept, not an OIDC concept. It enables the server to send a (templated) message to the client after login is complete. This is the correct place to place text required by, for example, DOE security policy or the ORICD Terms of Service.
  2. Expose client_id and authorization_endpoint, which are used by the machinery in tiled/server/authentication.py.
  3. Update example_configs/simple_oidc, which runs a local OIDC provider in a container for dev/demo/test.

At present, (3) does not quite work. Maybe you could take a look from here, @DiamondJoseph? See example_configs/simple_oidc/README.md to reproduce.

  File "/home/dallan/Repos/bnl/tiled/tiled/authenticators.py", line 201, in authenticate
    verified_body = jwt.decode(
                    ^^^^^^^^^^^
  File "/home/dallan/Repos/bnl/dd/.pixi/envs/default/lib/python3.12/site-packages/jose/jwt.py", line 142, in decode
    payload = jws.verify(token, key, algorithms, verify=verify_signature)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/dallan/Repos/bnl/dd/.pixi/envs/default/lib/python3.12/site-packages/jose/jws.py", line 73, in verify
    _verify_signature(signing_input, header, signature, key, algorithms)
  File "/home/dallan/Repos/bnl/dd/.pixi/envs/default/lib/python3.12/site-packages/jose/jws.py", line 261, in _verify_signature
    if not _sig_matches_keys(keys, signing_input, signature, alg):
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/dallan/Repos/bnl/dd/.pixi/envs/default/lib/python3.12/site-packages/jose/jws.py", line 208, in _sig_matches_keys
    key = jwk.construct(key, alg)
          ^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/dallan/Repos/bnl/dd/.pixi/envs/default/lib/python3.12/site-packages/jose/jwk.py", line 79, in construct
    return key_class(key_data, algorithm)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/dallan/Repos/bnl/dd/.pixi/envs/default/lib/python3.12/site-packages/jose/backends/cryptography_backend.py", line 272, in __init__
    raise JWKError("Unable to parse an RSA_JWK from key: %s" % key)
jose.exceptions.JWKError: Unable to parse an RSA_JWK from key: <Response [200 OK]>

@danielballan
Copy link
Member

Actually, I realized that the fix is straightforward. Pushed a couple more commits, and now it works for me:

In [14]: c = from_uri('http://localhost:8000')

You have 15 minutes to visit this URL

http://localhost:9000/auth?client_id=example_client_id&response_type=code&scope=openid&redirect_uri=http%3A%2F%2Flocalhost%3A8000%2Fapi%2Fv1%2Fauth%2Fprovider%2Fsimple_oidc%2Fdevice_code

and enter the code:

558C-F054


Waiting....
You have logged in with Simple OIDC as example.

# In web browser, I logged in with creds:
#
# [email protected]
# password
#
# and entered the code above when prompted...

In [15]: c
Out[15]: <Container {'A', 'B', 'C', 'D'}>

In [16]: c.context
Out[16]: <Context authenticated as 'example'>

Copy link
Contributor

@padraic-shafer padraic-shafer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this looks really nice.

FWIW, I tried running this with the supplied local OIDC provider. I was able to authenticate, but received a 500 error when it could not find a "principal" in the db. I'm not sure if I missed something in the config file (in which case we might need to update the docs).

# Run the OIDC provider
docker run --rm -p 9000:9000 -v $(pwd):/config -e CONFIG_FILE=/config/oidc_provider_config.json -e USERS_FILE=/config/users.json docker.io/qlik/simple-oidc-provider:0.2.4
# Run tiled server
OIDC_BASE_URL=http://localhost:9000 OIDC_CLIENT_ID=example_client_id OIDC_CLIENT_SECRET=example_client_secret tiled serve config example_configs/simple_oidc/config.yml
# Excerpt from server error after authentication
...
  File ".../tiled/server/authentication.py", line 652, in route
    session = await create_session(
              ^^^^^^^^^^^^^^^^^^^^^
  File ".../tiled/server/authentication.py", line 424, in create_session
    (new_identity,) = principal.identities
                      ^^^^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'identities'

docs/source/explanations/security.md Outdated Show resolved Hide resolved
docs/source/explanations/security.md Show resolved Hide resolved
@danielballan
Copy link
Member

danielballan commented Jan 17, 2025

I haven't seen this before and can't picture how it could happen:

  File ".../tiled/server/authentication.py", line 424, in create_session
    (new_identity,) = principal.identities
                      ^^^^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'identities'

Maybe you could reproduce it on Zoom and we could dig in a bit.

@padraic-shafer
Copy link
Contributor

padraic-shafer commented Jan 17, 2025

Maybe you could reproduce it on Zoom and we could dig in a bit.

I could not reproduce this -- neither in the same environment nor in a fresh installation. Let's chalk it up to user error.

Subsequent attempts all succeeded.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants