fix: return invalid_grant instead of invalid_request according to rfc6749#section-5.2 #426
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
damienbr
changed the title
damienbr
changed the title
…oken is not found or does not belong to client.
…ation code flow when the user is not the owner of the authorization code or if the redirect uri doesn't match from the authorization request.
damienbr
force-pushed
the
issue-418-invalid-grant
branch
from
33a819b
to
0714d51
Compare
|
I opened GitHub to search the fosite source for this bug, and your very timely PR was the first thing I saw 🎉 |
aeneasr
reviewed
Apr 16, 2020
| @@ -65,7 +65,7 @@ func (c *RefreshTokenGrantHandler) HandleTokenEndpointRequest(ctx context.Contex | |||
| signature := c.RefreshTokenStrategy.RefreshTokenSignature(refresh) | |||
| originalRequest, err := c.TokenRevocationStorage.GetRefreshTokenSession(ctx, signature, request.GetSession()) | |||
| if errors.Cause(err) == fosite.ErrNotFound { | |||
| return errors.WithStack(fosite.ErrInvalidRequest.WithDebug(err.Error())) | |||
| return errors.WithStack(fosite.ErrInvalidGrant.WithHint("The refresh token has not been found.")) | |||
There was a problem hiding this comment.
This information must not publicly be disclosed to make token scanning attacks harder:
Suggested change
| return errors.WithStack(fosite.ErrInvalidGrant.WithHint("The refresh token has not been found.")) | |
| return errors.WithStack(fosite.ErrInvalidGrant.WithDebug("The refresh token has not been found: %s", err)) |
|
I cannot commit my suggested changes because you haven't enabled "allow edits by maintainers". Could you please apply the change, then this is ready to merge :) |
|
Yup, I saw your changes. 👍 |
|
I used another workflow to apply them :) |
aeneasr
added a commit
to ory/hydra
that referenced
this pull request
Apr 16, 2020
Resolves a regression issue which sends an invalid error response when a refresh token is being re-used, is not found, or the wrong client is accessing it. See https://community.ory.sh/t/refresh-token-endpoint-returns-invalid-request-error-expecting-invalid-grant/1637/2 See ory/fosite#426 See ory/fosite#418
5 tasks
aeneasr
added a commit
to ory/hydra
that referenced
this pull request
Apr 16, 2020
Resolves a regression issue which sends an invalid error response when a refresh token is being re-used, is not found, or the wrong client is accessing it. This patch also bumps jose-related tooling which introduces better security measure against certain types of x509 attacks. See https://community.ory.sh/t/refresh-token-endpoint-returns-invalid-request-error-expecting-invalid-grant/1637/2 See ory/fosite#426 See ory/fosite#418
Closed
6 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related issue
#418
Proposed changes
Refresh token flow has been changed in order to return invalid_grant instead of invalid_request when there is a client mismatch or when the refresh token has not been found.
Authorization code flow has been changed in order to return invalid_grant instead of invalid_request when there is a client mismatch or the redirect uri doesn't match the one from the authorization request (or is empty).
Checklist
vulnerability. If this pull request addresses a security vulnerability, I
confirm that I got green light (please contact
security@ory.sh) from the maintainers to push
the changes.