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

Enhance OIDC Client to support the token revocation #26868

Merged
merged 1 commit into from
Sep 12, 2022

Conversation

sberyozkin
Copy link
Member

@sberyozkin sberyozkin commented Jul 21, 2022

Fixes #26867

Keeping this PR as a draft until I figure out how to test it.

I just had to refactor OidcClientImpl a little bit to reuse the code which is used to post a request.

quarkus-oidc may also support it in the next phase (ex, to revoke the failed bearer tokens, on local logouts, etc).

CC @FroMage, FYI, OidcClient can also be configured to talk to Apple OIDC

@sberyozkin sberyozkin force-pushed the oidc_client_revoke_token branch 3 times, most recently from de3599a to 2d49748 Compare July 22, 2022 16:22
@sberyozkin sberyozkin requested a review from pedroigor July 22, 2022 16:23
@sberyozkin sberyozkin marked this pull request as ready for review July 22, 2022 16:23
@sberyozkin
Copy link
Member Author

sberyozkin commented Jul 22, 2022

@pedroigor Have a look please when you get a chance. The main initial use case is for users of Keycloak (and other providers which support the revocation) be able to proactively revoke a given access token without going the whole refresh process, and for some providers it can be the only option if no RT is available.
But this is only the first phase. As far as OIDC client is concerned, we might consider adding another method to revoke both access and refresh tokens which would also require passing a token hint.
I'd also be interested in expanding it to quarkus-oidc (revoke the invalid bearer tokens proactively, or the tokens after the local logout and for some other cases).
I've been thinking for a while what to return, Boolean seems reasonable as 200 is expected from Keycloak if the token has been revoked or already been invalidated, while 503 seems mostly a theoretical case, and I don't really expect OidcClient then keeping retrying it, for example, for another 3 mins, etc, if really really necessary then the revocation can be retried at the application level with some configured reasonable retry period. It is just that kind of operation which does not seem worth retrying a lot, and should be tried as a single, best effort attempt to help OIDC servers to remove the tokens.

Copy link
Contributor

@pedroigor pedroigor left a comment

Choose a reason for hiding this comment

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

@sberyozkin Yeah, it should be useful for those looking for revoking the AT. You did almost everything to enable RT too :)

@sberyozkin
Copy link
Member Author

Thanks @pedroigor, let me resolve the conflict and rebuild

@sberyozkin sberyozkin force-pushed the oidc_client_revoke_token branch from 2d49748 to a630176 Compare September 12, 2022 10:21
@quarkus-bot

This comment has been minimized.

@sberyozkin sberyozkin force-pushed the oidc_client_revoke_token branch from a630176 to 6923576 Compare September 12, 2022 12:54
@sberyozkin
Copy link
Member Author

During the rebase I lost the code moved in the original PR from OidcClientRecorder to OidcClientImpl (related to the client secret post auth) which is what was causing a test failure

@quarkus-bot

This comment has been minimized.

@sberyozkin
Copy link
Member Author

Hmm...

@sberyozkin sberyozkin force-pushed the oidc_client_revoke_token branch from 6923576 to a1c3921 Compare September 12, 2022 16:23
@sberyozkin
Copy link
Member Author

This test (oidc-client-wiremock) is passing all the time for me, it feels like KC 19.0.1 might not be removing some state in time between the tests, as I also some some unexpected test failures in one of the other PRs, for the moment it is not easy to pinpoint...

@sberyozkin sberyozkin merged commit 4694171 into quarkusio:main Sep 12, 2022
@sberyozkin sberyozkin deleted the oidc_client_revoke_token branch September 12, 2022 19:23
@quarkus-bot quarkus-bot bot added this to the 2.13 - main milestone Sep 12, 2022
@quarkus-bot quarkus-bot bot added the kind/enhancement New feature or request label Sep 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OidcClient should support the token revocation
2 participants