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.

Sign up for GitHub

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

Jump to bottom

Change default of refresh token format #2406

Merged
merged 4 commits into from
Jul 27, 2023
Merged

Change default of refresh token format #2406

merged 4 commits into from
Jul 27, 2023

Conversation

strehle
Copy link
Member

@strehle strehle commented Jul 17, 2023

Default should be opaque, at least the default.
History about

cloudfoundry/uaa-release#93

Some tests rely on JWT for refresh, therefore added for these tests again jwt.token.refresh.format=jwt

@strehle
Change default of refresh token format
2083c52 
Default should be opaque, at least the default.
History about cloudfoundry/uaa-release#93
Some tests rely on JWT for refresh, therefore added for these tests again
jwt.token.refresh.format=jwt
@cf-gitbot
Copy link

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/185625077

The labels on this github issue will be updated when the story is started.

@strehle strehle added the expected behavior The isuse or PR is related to a RFC or other standard label Jul 17, 2023
@Tallicia
Copy link
Contributor

Need discuss deprecation timeline and notificaiton.

@strehle strehle requested review from a team July 19, 2023 15:57
@hsinn0
Copy link
Contributor

hsinn0 commented Jul 25, 2023

Need discuss deprecation timeline and notificaiton.

Can someone help me to understand this statement? Was there some offline discussion? What deprecation are we talking about?

@hsinn0 hsinn0 requested review from hsinn0 and removed request for hsinn0 July 25, 2023 21:32
hsinn0
hsinn0 reviewed Jul 25, 2023
View reviewed changes
Copy link
Contributor

@hsinn0 hsinn0 left a comment

Choose a reason for hiding this comment

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

It is my understanding this change affects the UAA API doc, config.tokenPolicy.refreshTokenFormat at https://docs.cloudfoundry.org/api/uaa/version/76.16.0/index.html#creating-an-identity-zone for example. Do we have plan to also update the doc?

@hsinn0 hsinn0 added the in_review The PR is currently in review label Jul 25, 2023
@strehle strehle requested a review from hsinn0 July 26, 2023 09:26
@strehle
Copy link
Member Author

strehle commented Jul 26, 2023 •
edited
Loading

It is my understanding this change affects the UAA API doc, config.tokenPolicy.refreshTokenFormat at https://docs.cloudfoundry.org/api/uaa/version/76.16.0/index.html#creating-an-identity-zone for example. Do we have plan to also update the doc?

Thanks , missed that.
I changed more defaults - internally and adopted the tests, which rely on JWT in refresh token, so that there a JWT comes, but the rest of the tests should not have problems with the opaque token

So this PR simply changes the default but if some one wants JWT for refresh tokens, then this can be configured, now with an extra step

@strehle strehle merged commit 11b086c into develop Jul 27, 2023
18 checks passed
@strehle strehle deleted the fix/refreshTokenDefault branch July 27, 2023 07:11
@cf-gitbot cf-gitbot added delivered accepted Accepted the issue and removed delivered labels Oct 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hsinn0 hsinn0 hsinn0 approved these changes

Assignees
No one assigned
Labels
accepted Accepted the issue expected behavior The isuse or PR is related to a RFC or other standard in_review The PR is currently in review
Projects
Foundational Infrastructure Working Group
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@strehle @cf-gitbot @Tallicia @hsinn0