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

Add scope parameter to the refreshToken request. #225

Merged
merged 4 commits into from
Nov 20, 2021

Conversation

KieranFJ
Copy link
Contributor

@KieranFJ KieranFJ commented Jul 7, 2020

Some providers require an optional scope when requesting a refresh of a token. If not provided they will return an invalid_request error.
Scope for the refresh token should be the same scope as the access token, so using the same method should be sufficient.

List of common tasks a pull request require complete

  • Changelog entry is added or the pull request don't alter library's functionality

KieranFJ added 3 commits July 7, 2020 11:04
Some providers require a scope when requesting a token refresh.

Uses the same process as requestResourceOwnerToken() due to the RFC requiring the scope to be the same for an access_token request and for a refresh token request
@mcouillard
Copy link
Contributor

We required the same fix while using Windows Active Directory. The addition of scope worked perfectly.

@JuliusPC
Copy link
Contributor

JuliusPC commented Dec 4, 2020

Some providers require an optional scope when requesting a refresh of a token. If not provided they will return an invalid_request error.

RFC 6749 claims the scope parameter to be optional when refreshing a token. This seems to be a bug in the provider’s OAuth implementation and should reported to them.

Sending scopes to the token endpoint when refreshing token opens the possibility to request an access token with narrower scope than the original authorization grant allows:

The requested scope MUST NOT include any scope not originally granted by the resource owner, and if omitted is treated as equal to the scope originally granted by the resource owner.
[…]
The authorization server MAY issue a new refresh token, in which case the client MUST discard the old refresh token and replace it with the new refresh token. The authorization server MAY revoke the old refresh token after issuing a new refresh token to the client. If a new refresh token is issued, the refresh token scope MUST be identical to that of the refresh token included by the client in the request.

@mcouillard
Copy link
Contributor

While I agree, getting Microsoft to change their default ADFS behavior will be tricky ;)

@JuliusPC
Copy link
Contributor

JuliusPC commented Dec 5, 2020

Anyway, this is a nice-to-have feature (independent of allowing to work around Microsoft’s weird implementation).
@jumbojett What prevents you from merging this?

@JuliusPC
Copy link
Contributor

I forked this library and added this functionality. I plan to make other changes to the OpenID Connect Library.

I made it configurable with sending it by default, so if a provider has problems with scopes in such request, you are able to disable it.

The only problem I see with this is: If the refresh token was obtained by the authorization code grant with set openid scope, this scope is missing in the refresh request. Just adding the openid scope in every authorization and refresh request does not solve the problem, since we don’t know in which flow the refresh token was requested and the Resource Owner Password Credentials Grant may result in a refresh token without the openid scope set. This may be a rare edge case...

@azmeuk azmeuk merged commit 0711823 into jumbojett:master Nov 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants