fix: always rotate refresh tokens for public clients

[mikeroda]

When refreshing a token, always rotate for public clients, thus not requiring rotation to be enabled for all clients and preventing the possible error condition for public clients.

[cf-gitbot]

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

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

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

[strehle]

@mikeroda this PR has conflicts from the begin, so you need to resolve conflicts / rebase your branch

[strehle]

@mikeroda will check the PR and we take it on the list , means if we do not see a problem it should come into dev , end of next week

[strehle]

Sonar, ok: https://sonarcloud.io/summary/new_code?id=cloudfoundry-identity-parent&pullRequest=2846

[strehle]

Ok from my side @swalchemist can you check from broadcom side.
Only touches your clients if they use public clients...

[peterhaochen47]

Hi we have not looked at this PR yet.

[strehle]

Hi we have not looked at this PR yet.

Hi we have not looked at this PR yet.

Ok can you please look and vote if it needs to be reverted

[peterhaochen47]

We do have a public client. This change seems like a breaking change?

1. Previously, whether the refresh token is rotated is controlled by the uaa.jwt.refresh.rotate flag and defaulted to false to maintained backward compatibility. Correct?

This current change makes an exception to the flag and always rotate public clients's refresh token, and potentially breaking the backward compatibility brought by the flag.

2. Separately, this might also be confusing to the users when they set the uaa.jwt.refresh.rotate flag to false but still see their refresh tokens being rotated?

thus not requiring rotation to be enabled for all clients and preventing the possible error condition for public clients

Could you explain this? If you wish to rotate the refresh tokens, why not just set uaa.jwt.refresh.rotate: true to enable that for all clients? What "possible error condition" would that create for public clients?

[mikeroda]

Ultimately we want to get to always rotating refresh tokens for public clients. The problem with the flag is it affects all clients. This doesn't seem like an adverse change because if you didn't set the flag and attempted to refresh a token with a public client, you would get "Refresh without client authentication not allowed" error. Now you would get a successful response and newly rotated refresh token. So I don't see how this breaks anything.

[peterhaochen47]

if you didn't set the flag and attempted to refresh a token with a public client, you would get "Refresh without client authentication not allowed" error

I see. Currently, UAA doesn't allow token refresh for public clients when uaa.jwt.refresh.rotate: false? (is this documented somewhere?). Then yes, this is a non-breaking change.