'OIDC_RSA_PRIVATE_KEY' is mandatory #873
Assignees
Labels
Comments
This was referenced Oct 5, 2020
Closed
|
I'm a bit interested in this, as we are currently using django-oidc-provider, but it has gone stale. It would be nice to migrate over to django-oauth-toolkit, but we would then need OIDC-support in django-oauth-toolkit. Perhaps @thinkwelltwd can contribute with some insight into the decisions on having it in settings? |
|
@FluidSense We'v have had to revert #859 until it doesn't break existing non-OIDC usage. Probably just a few configuration tweaks needed here and there. It also needs to be documented and clearly some more test cases are needed. The plan is to make a small 1.3.3 release without the OIDC feature and then hopefully get the contributors to finish up the PR such that it is mergeable into a new major release. It sill needs a fair amount of work to make that happen. Help making this happen would be appreciated! |
|
I see, that sounds like a good decision seeing as there has been no recent releases, but some fixes. Perhaps a place to start to look into things would be the reverted PR then, to see if I have something to contribute there, and if I can understand enough of it to document on behalf of the original authors |
|
Reverted in #877 |
tevansuk
added a commit
to tevansuk/django-oauth-toolkit
that referenced
this issue
Jan 25, 2021
Make OIDC support optional by not requiring OIDC_RSA_PRIVATE_KEY to be set in the settings, and using the standard oauthlib.oauth2.Server class when an OIDC private key is not configured. Add a test fixture wrapping oauth2_settings. This allows individual tests / test suites to override oauth2 settings and have them reset at the end of the test. This avoids configuration leaking from one test to another, and allows us to test multiple different configurations in one test run. When using the oauth2_settings fixture, allow configuration for the test case to be loaded from a pytest marker called oauth2_settings. Split out OIDC specific tests requiring specific OIDC configuration into separate TestCase. Adjust the OAuthLibMixin to fallback to using the server, validator and core classes specified in oauth2_settings when not hardcoded in to the class. These classes can still be specified as hard-coded attributes in sub-classes, but it's no longer required if you just want what is configured in oauth2_settings, so remove all attributes that are just pointing at the configuration anyway. Add a setting ALWAYS_RELOAD_OAUTHLIB_CORE, which causes OAuthLibMixin to reload the OAuthLibCore object on each request. This is only intended to be used during testing, to allow the views to recognise changes in configuration. Show missing coverage lines in the coverage report. Fixes: jazzband#873
n2ygk
added a commit
that referenced
this issue
Mar 17, 2021
* Openid Connect Core support - Round 3 * Add OpenID connect hybrid grant type * Add OpenID connect algorithm type to Application model * Add OpenID connect id token model * Add nonce Authorization as required by OpenID connect Implicit Flow * Add body to create_authorization_response to pass nonce and future OpenID parameters to oauthlib.common.Request * Add OpenID connect ID token creation and validation methods and scopes * Add OpenID connect response types * Add OpenID connect authorization code flow test * Add OpenID connect implicit flow tests * Add validate_user_match method to OAuth2Validator * Add RSA_PRIVATE_KEY setting with blank value * Update tox * Add get_jwt_bearer_token to OAuth2Validator * Add validate_jwt_bearer_token to OAuth2Validator * Change OAuth2Validator.validate_id_token default return value to False to avoid validation security breach * Change to use .encode to avoid py2.7 tox test error * Add OpenID connect hybrid flow tests * Change to use .encode to avoid py2.7 tox test error * Add RSA_PRIVATE_KEY to the list of settings that cannot be empt * Add support for oidc connect discovery * Use double quotes for strings * Rename migrations to avoid name and order conflict * Remove commando to install OAuthLib from master and removed jwcrypto duplication * Remove python 2 compatible code * Change errors access_denied/unauthorized_client/consent_required/login_required to be 400 as changed in oauthlib/pull/623 * Change iss claim value to come from settings * Change to use openid connect code server class * Change test to include missing state * Add id_token relation to AbstractAccessToken * Add claims property to AbstractIDToken * Change OAuth2Validator._create_access_token to save id_token to access_token * Add userinfo endpoint * Update migrations and remove oauthlib duplication * Remove old generated migrations * Add new migrations * Fix tests * Add nonce to hybrid tests * Add missing new attributes to test migration * Rebase fixing conflicts and tests * Remove auto generate message * Fix flake8 issues * Fix test doc deps * Add project settings to be ignored in coverage * Tweak migrations to support non-overidden models * OIDC_USERINFO_ENDPOINT is not mandatory * refresh_token grant should be support for OpenID hybrid * Fix the user info view, and remove hard dependency on DRF * Use proper URL generation for OIDC endpoints * Support rich ID tokens and userinfo claims Extend the validator and override get_additional_claims based on your own user model. * Bug fix for at_hash generation See https://openid.net/specs/openid-connect-core-1_0.html#id_token-tokenExample to prove algorithm * OIDC_ISS_ENDPOINT is an optional setting * Support OIDC urls from issuer url if provided * Test for generated OIDC urls * Flake * Rebase on master and migrate url function to re_path * Handle invalid token format exceptions as invalid tokens * Merge migrations and sort imports isort for flake8 lint check Co-authored-by: Dave Burkholder <[email protected]> Co-authored-by: Wiliam Souza <[email protected]> Co-authored-by: Allisson Azevedo <[email protected]> Co-authored-by: fvlima <[email protected]> Co-authored-by: Shaun Stanworth <[email protected]> * Make IDToken admin class swappable * Make OIDC support optional Make OIDC support optional by not requiring OIDC_RSA_PRIVATE_KEY to be set in the settings, and using the standard oauthlib.oauth2.Server class when an OIDC private key is not configured. Add a test fixture wrapping oauth2_settings. This allows individual tests / test suites to override oauth2 settings and have them reset at the end of the test. This avoids configuration leaking from one test to another, and allows us to test multiple different configurations in one test run. When using the oauth2_settings fixture, allow configuration for the test case to be loaded from a pytest marker called oauth2_settings. Split out OIDC specific tests requiring specific OIDC configuration into separate TestCase. Adjust the OAuthLibMixin to fallback to using the server, validator and core classes specified in oauth2_settings when not hardcoded in to the class. These classes can still be specified as hard-coded attributes in sub-classes, but it's no longer required if you just want what is configured in oauth2_settings, so remove all attributes that are just pointing at the configuration anyway. Add a setting ALWAYS_RELOAD_OAUTHLIB_CORE, which causes OAuthLibMixin to reload the OAuthLibCore object on each request. This is only intended to be used during testing, to allow the views to recognise changes in configuration. Show missing coverage lines in the coverage report. Fixes: #873 * Add tests for OIDC userinfo view * Add test for creating the OIDC issuer url for JWTs * Add ID token generation and validation tests * Add tests for OAuth2ProviderSettings * Add tests for IDToken model methods * Remove unnecessary __future__ declarations * Enable OIDC only views only when using OIDC Add a mixin for OIDC only views that checks that OIDC is correctly configured before allowing the request to continue. If OIDC is not enabled, raise an ImproperlyConfigured exception if the site is in DEBUG mode, otherwise log a warning and return a 404 response. * Remove mistakenly committed comment * Add OIDC documentation * Add OIDC support change to CHANGELOG.md * Support nonce and claims in OIDC auth * Add nonce and claims fields to Grant model. * Complete support for nonce in the OIDC auth code flow, and work around an oauthlib bug in the OIDC hybrid flow that caused a nonce presented in the authentication endpoint to be omitted from the ID token. * Switch to using `RequestValidator.finalize_id_token` instead of `RequestValidator.get_id_token`. This allows us to use the oauthlib stock implementation of `get_id_token`, which creates `at_hash` and `c_hash` when appropriate. * Support `claims` in the authentication endpoint to specify what claims are desired in the ID token, as per section 5.5. Interpreting the claims parameter is up to implementers. * Add documentation on using scopes and claims as authentication endpoint parameters to influence what claims to add to the tokens. * Add tests for `nonce` and `claims` handling. * Fix py3.5 tests getting scopes in unexpected order Not entirely sure how my previous changes caused that, but convert them to sets and compare the sets of scopes. * Simplify and optimise get_authorization_code_scopes `code` is sufficient for retrieving an auth code's scopes, and we don't need to do two DB queries where one will suffice. * Serve JWKs using well-known URL JWKs are commonly served from `.well-known/jwks.json`. This isn't a standard, but it is in common usage so it makes sense to conform. * Allow POST for OIDC UserInfo endpoint * Refactor how OIDC issuer url is generated Split out how to generate the OIDC issuer (when not specified in settings) out to a util function. Add tests Dont use reverse_lazy when the next thing we do with the url is to format it in to a string - no time for it to be lazy! * Support HS256 for OIDC * Add full support for HS256 signed OIDC keys. * Add a new default signing algorithm, NO_ALGORITHM. * Add docs on why you shouldn't choose HS256, and how to do it anyway. * Add docs on how to enable an app for OIDC. * Remove OIDC_ID_TOKEN_SIGNING_ALG_VALUES_SUPPORTED and generate it depending on whether you have added an RSA key. * Add validation for your application signing algorithm so its not possible to accidentally setup something insecure. * Add `Application.jwk_key` property to load the correct key for an application. * Peek at a supplied id_token to determine the application it belongs to so that we can load the correct key to verify the signature. * Add an OIDC_ENABLED setting, as we can now enable OIDC without adding OIDC_RSA_PRIVATE_KEY. * Update and add new tests. * fix: json.loads on python 3.5 requires a str * Remove changes to create_authorization_response When using OIDC, the additional parameters nonce and claims are correctly extracted by oauthlib, we don't need to manually parse them out and pass them as a phony body parameter. Similarly, the django request is passed down to the final wrapper layer that calls create_authorization_response in oauthlib, we can defer calling `request.get_raw_uri()` until that point and drop passing `uri` down two function calls. This reverts create_authorization_response back to basically how it was pre-OIDC changes. * Reduce changes compared to master There were a few other places where stylistic changes have been made that are not a part of the changes. Revert them to make the patch less different compared to master. * Use simpler import name for oauthlib OIDC server * Fix another reference to oauthlib.openid.Server * Add indirect six dependency from jwcrpyto jwcrypto has a direct dependency on six, but does not list it yet in a release. Previously, cryptography also depended on six, so this was unnoticed. * Django master now supports only python 3.8+ * Store jti instead of the ID token contents Add a jti parameter to each ID token generated. After verifying a received ID token JWT, extract the jti claim to verify that the ID token exists in our database and is not expired. We don't require the contents of the JWT to verify that an ID token hasn't expired or been revoked, just the jti claim, so don't bother to store or index the ID token contents. Because we only would look at this when presented with an ID token JWT, all the claim contents are available in this JWT. Add missing scope check when verifying an ID token, add tests to verify this. Add functions _load_id_token and _load_access_token to OAuth2Validator, analagous to _save_id_token and _create_access_token. These can be overridden in sub-classes to customise loading behaviour, if these models have been swapped. * Code review changes * Don't swallow ValueError in ClientProtectedResourceMixin * Use OAuthLibCore._get_escaped_full_path to encode URI before passing to oauthlib. * You cannot create an ID Token using client credentials flow, so remove dead code handling this eventuality from OAuth2Validator._save_id_token * Remove TODO comment about saving IDTokens, it isn't necessary. * Generate the correct issuer URL from oauthlib OAuthlib will use OAuth2Validator.get_oidc_issuer_endpoint to generate the issuer endpoint; we create a phony django request to use django's mechanisms for generating the fully qualified URL. However, when SSL is enableld, not enough information is passed through to determine that the protocol is HTTPS. Adjust OAuthLibCore.extract_headers() to inject a custom header when the django request is secure. Use this extra header when generating the issuer URL to determine whether to set the protocol to "https", and use a custom django.http.HttpRequest subclass to allow us to set this. Adjust OAuthLibCore.create_authorization_response() to pass through headers to oauthlib to allow this header to be received. * Update OIDC docs * Krl/oidc round three fixes (#1) Add kid to id_token header, conditional on the algorithm used * Fix linting from PR * Handle invalid tokens in userinfo endpoint * Update a comment Co-authored-by: Dave Burkholder <[email protected]> Co-authored-by: Wiliam Souza <[email protected]> Co-authored-by: Allisson Azevedo <[email protected]> Co-authored-by: fvlima <[email protected]> Co-authored-by: Shaun Stanworth <[email protected]> Co-authored-by: Alan Crosswell <[email protected]> Co-authored-by: Kristian Rune Larsen <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug
Existing code using the latest master commit (pr #859) throws an error when not using OIDC features.
To Reproduce
Install DOT master commit 295c065
Expected behavior
Code not using OIDC should continue to work.
Version
master 295c065
Additional context
The text was updated successfully, but these errors were encountered: