OIDC: Preserve the refresh token if no new refresh token is returned #28023
Conversation
This file contains 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
This comment has been minimized.
This comment has been minimized.
|
I think I'll have to disable this CodeFlowTest, though I'll double check it is not related to this PR |
pedroigor
approved these changes
Sep 19, 2022
so what should we do with this PR? |
|
@pedroigor Thanks for the approval, |
sberyozkin
force-pushed
the
oidc_keep_refresh_token_cookie
branch
from
d4fb1c5
to
cc48515
Compare
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.
Fixes #27753.
As described in #27753, some providers do not recycle the tokens after the refresh grant completes, and in such cases, refreshing the sessions is not no longer possible because
quarkus-oidcdoes not keep the original refresh token which was used to complete the first (and currently last in such cases) refresh grant request.The right approach is to keep the original refresh token if no new refresh token has been returned in the refresh grant response. If this RT becomes expired then the provider will reject it anyway - but in this case the refresh token is cleared.
The actual fix is straightforward - just keep the current RT if the successful RT grant response has no new RT.
But I've spent nearly 2 days on creating a test - first in
integration-tests/oidc-wiremockwhere it worked but not always due to some wiremock state issues and eventually I did it inintegration-tests/oidc-tenancywhere we have a testOidcResourceprovider endpoint.The test emulates this flow as precisely as possible: 1) tokens are obtained using the code flow first, then, 2) after an id token expiration, the first refresh grant request is performed but it returns no new RT so the original one is kept, and finally, 3) when a new id token also expires, another RT grant it performed with OIDC server failing it and it causes a redirect for the user to reauthenticate