Fix: Use proper scopes in refresh token handler
Fixes #696
The token refresh handler currently has two issues related to scopes:
- It does not read or honor the optional
scopeform parameter - It defaults to the current
clientscopes, rather than the originallygrantedscopes. One issue related to this is that if the client scopes are narrowed, then the existing refresh tokens may fail to refresh, even if the user originally consented to those scopes.
This PR fixes the OAuth2 token refresh handler to:
- Read and use the optional 'scope' form parameter, if present.
- Otherwise default to requesting the originally granted scopes.
The token refresh handler endpoint should be completely agnostic of:
- The originally requested scopes
- The client scopes (both current and past client scopes)
Related Issue or Design Document
#696
Checklist
- [x] I have read the contributing guidelines and signed the CLA.
- [x] I have referenced an issue containing the design document if my change introduces a new feature.
- [x] I have read the security policy.
- [x] I confirm that this pull request does not address a security vulnerability. If this pull request addresses a security vulnerability, I confirm that I got green light (please contact [email protected]) from the maintainers to push the changes.
- [x] I have added tests that prove my fix is effective or that my feature works.
- [ ] I have added necessary documentation within the code base (if appropriate).
Further comments
Perhaps we can put aside the "client scope" discussion for a moment, and simply agree that the Fosite library DOES have a bug whereby the scope form parameter is completely ignored. The refresh token handler does not read the scope form parameter in the current implementation, so it is currently impossible to request a more narrowly scoped access token.
Perhaps we can put aside the "client scope" discussion for a moment, and simply agree that the Fosite library DOES have a bug whereby the
scopeform parameter is completely ignored. The refresh token handler does not read thescopeform parameter in the current implementation, so it is currently impossible to request a more narrowly scoped access token.
Yeah I agree with focusing on the one aspect, it's more efficient. Especially with focusing on something I think is less ambiguous/open to interpretation/not explicitly stated in the spec, is a good idea.
@aeneasr please let me know if you have time to discuss https://github.com/ory/fosite/pull/698#discussion_r961835824
Given the discussion and reasoning above I will close this PR. Any ideas for future work outlined in this PR we‘ll happily accept though :)