[All] Add userdata_from_id_token as alternative to userdata_url
#725
Conversation
for more information, see https://pre-commit.ci
|
Thank you! I think this is very sensible. Perhaps config would be clearer if we used an explicit special value, like e.g. |
|
I think userdata_from_id_token = true is a good choice here
I think having a dedicated toggle |
|
Is pyjwr an optional dep for azuread ? Then we can clean it up there. If that optinal dependency is empty, let's then leave it empty and comment its no longer in use. Also i think we should add a lower bound to pyjwts major version, so we depend on pyjwt>=2 because i recall migrating to v2 from v1 was needed elsewhere |
it is, we can promote it to an unconditional dependency with the same bounds. |
remove newly redundant extras
|
Do you think we could add a test for this to help with future maintenance? |
Note, the current mock setup delivers different handlers (for ingesting sample users) depending on whether id tokens are intended to be supplied. The fixtures (for preparing a mock client and preconfigured authenticator) thus need specialisation depending on whether the id token is being used. However, the current mock also requires that tests individually manage the packaging of the id token when loading into the mock client's handler; the handler syntax is different between the two cases. This means that to test that JWT id tokens and JSON userinfo endpoints provide equivalent behaviour requires changes in both the test (in the preamble) and in fixtures (compared to unmodified tests). This has led to some code duplication, which would be a target for future refactoring.
for more information, see https://pre-commit.ci
rather than if attribute is defined
needed for audience verification
minrk
changed the title
|
@benjimin thanks for the test! |
|
Thanks for the assist and support! |
consideRatio
changed the title
userdata_from_id_token as alternative to userdata_url
This PR adds support for extracting claims from the id token, instead of by exchanging the access token with the userinfo endpoint. Addresses #487.
This is needed because some claims may only be available via the id token and not via the userinfo endpoint, for example, AWS Cognito only provides the group membership claim via the id token (i.e., needed to complete support for #706).
For background, OAuth2 was a standard for obtaining an access token ("authorisation not authentication") and OIDC extended this in two ways: by standardising a userinfo endpoint and by accompanying the access token with an id token. (Note this is only the core of OIDC; complete OIDC support also entails discovery mechanisms, etc.) To date, oauthenticator has always discarded the id token that it receives, and relied on the userinfo endpoint for performing authentication.
This PR requires the feature to be explicitly enabled by the admin, without introducing any additional config attribs. It uses a library which is already used elsewhere by JupyterHub.