Unable to request narrower scopes in OAuth2 Refresh Token Exchange #696
Description
Preflight checklist
- I could not find a solution in the existing issues, docs, nor discussions.
- I agree to follow this project's Code of Conduct.
- I have read and am following this repository's Contribution Guidelines.
- This issue affects my Ory Cloud project.
- I have joined the Ory Community Slack.
- I am signed up to the Ory Security Patch Newsletter.
Describe the bug
Hello!
I recently discovered that the Refresh Token Grant Handler does not currently honor the (optional) user requested scope parameter to request a subset of the originally granted scopes.
For reference, here is the code in question:
for _, scope := range originalRequest.GetGrantedScopes() {
if !c.Config.GetScopeStrategy(ctx)(request.GetClient().GetScopes(), scope) {
return errorsx.WithStack(fosite.ErrInvalidScope.WithHintf("The OAuth 2.0 Client is not allowed to request scope '%s'.", scope))
}
request.GrantScope(scope)
}
The issues that I see with the current implementation are:
- The current implementation does not read the
scopeform param. - The implementation currently compares the originally granted scopes against the current client scopes, but this endpoint should be completely agnostic of the client scopes. I think it should only depend on the originally granted scopes (and optionally the
scopeform param). The result is that the refresh token exchange will fail if the client scopes are themselves narrowed after a refresh token is created.
This is the implementation that I would propose, given my understanding of https://www.rfc-editor.org/rfc/rfc6749#section-6
diff --git a/handler/oauth2/flow_refresh.go b/handler/oauth2/flow_refresh.go
index 0e3be4d..642e774 100644
--- a/handler/oauth2/flow_refresh.go
+++ b/handler/oauth2/flow_refresh.go
@@ -96,11 +96,15 @@ func (c *RefreshTokenGrantHandler) HandleTokenEndpointRequest(ctx context.Contex
}
request.SetSession(originalRequest.GetSession().Clone())
- request.SetRequestedScopes(originalRequest.GetRequestedScopes())
+ if request.GetRequestForm().Has("scope") {
+ request.SetRequestedScopes(strings.Split(request.GetRequestForm().Get("scope"), " "))
+ } else {
+ request.SetRequestedScopes(originalRequest.GetGrantedScopes())
+ }
request.SetRequestedAudience(originalRequest.GetRequestedAudience())
- for _, scope := range originalRequest.GetGrantedScopes() {
- if !c.Config.GetScopeStrategy(ctx)(request.GetClient().GetScopes(), scope) {
+ for _, scope := range request.GetRequestedScopes() {
+ if !c.Config.GetScopeStrategy(ctx)(originalRequest.GetGrantedScopes(), scope) {
return errorsx.WithStack(fosite.ErrInvalidScope.WithHintf("The OAuth 2.0 Client is not allowed to request scope '%s'.", scope))
}
request.GrantScope(scope)
Some pertinent notes from my reading of https://www.rfc-editor.org/rfc/rfc6749#section-6:
scope
OPTIONAL. The scope of the access request as described by
[Section 3.3](https://www.rfc-editor.org/rfc/rfc6749#section-3.3). 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.
- This optional
scopeparameter should allow the client to request a narrower set of scopes than were originally granted by the resource owner. - The refresh token grant handler should be completely agnostic of the client scopes. It should only depend on the originally granted scopes, and optionally the
scopeform param. - I would expect this narrower set of scopes to apply only to this specific access token, and not to future refresh token requests. (This is referenced later in the RFC with
"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.")
Reproducing the bug
The current unit tests reproduce this issue. For example:
=== RUN TestRefreshFlow_HandleTokenEndpointRequest/strategy=hmac/case=should_pass
flow_refresh_test.go:194:
Error Trace: flow_refresh_test.go:194
flow_refresh_test.go:356
Error: Not equal:
expected: fosite.Arguments{"foo", "bar", "offline"}
actual : fosite.Arguments{"foo", "offline"}
Diff:
--- Expected
+++ Actual
@@ -1,4 +1,3 @@
-(fosite.Arguments) (len=3) {
+(fosite.Arguments) (len=2) {
(string) (len=3) "foo",
- (string) (len=3) "bar",
(string) (len=7) "offline"
Test: TestRefreshFlow_HandleTokenEndpointRequest/strategy=hmac/case=should_pass
--- FAIL: TestRefreshFlow_HandleTokenEndpointRequest/strategy=hmac/case=should_pass (0.00s)
Expected :fosite.Arguments{"foo", "bar", "offline"}
Actual :fosite.Arguments{"foo", "offline"}
Relevant log output
No response
Relevant configuration
No response
Version
master
On which operating system are you observing this issue?
No response
In which environment are you deploying?
No response
Additional Context
No response