Repository navigation
Don't cache tokens rejected by the auth code nonce or max_age check - #979
Open
rumaisaaaaa263 wants to merge 1 commit into
Open
rumaisaaaaa263 wants to merge 1 commit into
rumaisaaaaa263 wants to merge 1 commit into
Conversation
obtain_token_by_auth_code_flow() and obtain_token_by_authorization_code() checked the ID token nonce and auth_time only after the token response had already gone through on_obtaining_tokens. Tokens from a rejected response therefore stayed in the token cache, and acquire_token_silent() would hand them out. Hold the event until the checks pass.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The rejection tests do not check the ID-token cache, so they could miss a regression that stores only that credential.
1 open issue
What changed in this PR
This PR prevents rejected authorization-code tokens from entering MSAL’s cache by delaying token callbacks until nonce and max_age validation succeeds.
Changes:
- Defer token callbacks in both current and legacy authorization-code flows.
- Add regression tests for rejected responses and verify valid tokens remain available through silent acquisition.
| File | Description |
|---|---|
tests/test_application.py |
Adds cache-rejection tests and a successful caching test. |
msal/oauth2cli/oidc.py |
Holds token events until ID-token checks pass. |
Review effort: Balanced
| self.assertEqual([], app.get_accounts()) | ||
| for credential_type in ( | ||
| msal.TokenCache.CredentialType.ACCESS_TOKEN, | ||
| msal.TokenCache.CredentialType.REFRESH_TOKEN): |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Tokens rejected by the nonce or max_age check are still cached
oidc.Client.obtain_token_by_auth_code_flow()checks the ID token nonce andauth_timeafter the redemption call returns, but by thenoauth2.Client._obtain_token()has already handed the response toon_obtaining_tokens, whichClientApplicationwires totoken_cache.add(). So whenacquire_token_by_auth_code_flow()oracquire_token_interactive()raises the nonce or max_ageRuntimeError,get_accounts()already lists the account andacquire_token_silent()returns the rejected access token from cache. That undoes the nonce protection against an injected authorization code and themax_agefreshness requirement, and an app that persists its cache keeps those tokens after the failed sign-in.acquire_token_by_authorization_code(nonce=...)has the same ordering.The token event is now held until the checks pass and then passed to the real callback, so a valid response is cached the same way as before.
TestAuthCodeTokensRejectedByIdTokenChecksAreNotCacheddrives the public API with a mismatched nonce, a staleauth_timeundermax_age=60, and the legacynonce=path; those three tests fail ondevand pass with this change, and a fourth checks that a response passing both checks is still cached and served byacquire_token_silent(). The unit test command from.Pipelines/template-pipeline-stages.ymlpasses on Python 3.9 (459 passed, 20 skipped).