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

feat(rp): return oidc.Tokens on token refresh #423

Merged
merged 1 commit into from
Aug 18, 2023
Merged

Conversation

muhlemmer
Copy link
Collaborator

BREAKING CHANGE:

  • rename RefreshAccessToken to RefreshToken
  • RefreshToken returns *oidc.Tokens instead of *oauth2.Token

This change allows the return of the id_token in an explicit manner,
as part of the oidc.Tokens struct.
The return type is now consistent with the CodeExchange function.

When an id_token is returned, it is verified.
In case no id_token was received,
RefreshTokens will not return an error.

As per specifictation:
https://openid.net/specs/openid-connect-core-1_0.html#RefreshTokenResponse

Upon successful validation of the Refresh Token,
the response body is the Token Response of Section 3.1.3.3
except that it might not contain an id_token.

Closes #364

BREAKING CHANGE:
- rename RefreshAccessToken to RefreshToken
- RefreshToken returns *oidc.Tokens instead of *oauth2.Token

This change allows the return of the id_token in an explicit manner,
as part of the oidc.Tokens struct.
The return type is now consistent with the CodeExchange function.

When an id_token is returned, it is verified.
In case no id_token was received,
RefreshTokens will not return an error.

As per specifictation:
https://openid.net/specs/openid-connect-core-1_0.html#RefreshTokenResponse

Upon successful validation of the Refresh Token,
the response body is the Token Response of Section 3.1.3.3
except that it might not contain an id_token.

Closes #364
@muhlemmer muhlemmer added this to the v3 milestone Aug 16, 2023
@muhlemmer muhlemmer linked an issue Aug 16, 2023 that may be closed by this pull request
@codecov
Copy link

codecov bot commented Aug 16, 2023

Codecov Report

Merging #423 (5247748) into next (e8262cb) will increase coverage by 0.38%.
The diff coverage is 96.15%.

@@            Coverage Diff             @@
##             next     #423      +/-   ##
==========================================
+ Coverage   52.96%   53.34%   +0.38%     
==========================================
  Files          74       74              
  Lines        5623     5635      +12     
==========================================
+ Hits         2978     3006      +28     
+ Misses       2389     2376      -13     
+ Partials      256      253       -3     
Files Changed Coverage Δ
pkg/client/rp/relying_party.go 56.37% <96.15%> (+3.88%) ⬆️

... and 1 file with indirect coverage changes

@muhlemmer
Copy link
Collaborator Author

@muir I would like your opinion on this PR. Is this what you had in mind? I'm adding it to next for v3 as a breaking change, as VerifyTokens requires a context, which was also added as a breaking change in this branch.

Copy link
Contributor

@muir muir left a comment

Choose a reason for hiding this comment

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

This looks fine and is a cleaner solution that we've got now.

@muhlemmer muhlemmer enabled auto-merge (squash) August 17, 2023 19:24
@muhlemmer muhlemmer merged commit 6708ef4 into next Aug 18, 2023
8 checks passed
@muhlemmer muhlemmer deleted the feat-resfresh-idtoken branch August 18, 2023 12:36
@github-actions
Copy link

🎉 This PR is included in version 3.0.0-next.9 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@github-actions
Copy link

🎉 This PR is included in version 3.0.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

refresh request discards id_token
3 participants