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

OIDC refresh token lost if not resent by provider #10924

Closed
sdaschner opened this issue Jul 23, 2020 · 3 comments · Fixed by #10931
Closed

OIDC refresh token lost if not resent by provider #10924

sdaschner opened this issue Jul 23, 2020 · 3 comments · Fixed by #10931
Labels
area/oidc kind/bug Something isn't working
Milestone

Comments

@sdaschner
Copy link
Contributor

Describe the bug
When using Google OpenID Connect as Quarkus OIDC provider, the application receives both an access and refresh token, when access_type=offline is sent to Google.

It work that Quarkus uses this refresh token under the hood to refresh the expired access token, however, Google sends the refresh token only for the very first time (and never again), when a token is requested.

CodeAuthenticationMechanism then causes the refresh token to be lost in the q_session cookie, since it overrides the value with the result from the refresh, and there the refresh token is non-existent.

Expected behavior
To save and reuse the refresh token, since it might not be resent -- this is the case for Google, not sure how other OIDC providers do this.

Potential fix

I've fixed this issue in my project by manually building quarkus-oidc with the following patch

Happy to provide a PR, however, I'm not quite sure whether this is just a dirty hack or solves it in all solutions; in general, it would be good if the OIDC extension wouldn't loose refresh tokens that are only issues once, like for Google.

@sdaschner sdaschner added the kind/bug Something isn't working label Jul 23, 2020
@sdaschner sdaschner changed the title OIDC refresh token lost with Google Login OIDC refresh token lost if not resent by provider Jul 23, 2020
@sberyozkin
Copy link
Member

sberyozkin commented Jul 23, 2020

@sdaschner Hi, thanks for spotting it, definitely, please provide a PR, if the refresh token grant succeeds but IDP chooses not to recycle RT then indeed we need to keep the current one.
Can you open a PR today please ? I'm on PTO from next week so I'd like to fix it by the end of the week. thanks
CC @pedroigor

@sdaschner
Copy link
Contributor Author

Sure, happy to do so (just couldn't comment on any other use case, have only tested the one I have with Google OIDC)

#10931

@sberyozkin
Copy link
Member

sberyozkin commented Jul 23, 2020

@sdaschner Thanks; we test against Keycloak which prefers to recycle RTs (which is recommended since RTs are long lived and powerful so the impact associated with them being misused is high), but I think in this case it is nearly an NPE fix, so a simple PR you have provided is OK

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/oidc kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants