Skip to content

Commit

Permalink
fix: return invalid_grant instead of invalid_request in refresh flow (#…
Browse files Browse the repository at this point in the history
…427)

Return invalid_grant instead of invalid_request when in authorization 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.

Co-authored-by: Damien Bravin <damienbr@users.noreply.github.com>
  • Loading branch information
aeneasr and damienbr authored Apr 16, 2020
1 parent 0399871 commit f5a0e96
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 9 deletions.
4 changes: 2 additions & 2 deletions handler/oauth2/flow_authorize_code_token.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func (c *AuthorizeExplicitGrantHandler) HandleTokenEndpointRequest(ctx context.C
// confidential client, or if the client is public, ensure that the
// code was issued to "client_id" in the request,
if authorizeRequest.GetClient().GetID() != request.GetClient().GetID() {
return errors.WithStack(fosite.ErrInvalidRequest.WithHint("The OAuth 2.0 Client ID from this request does not match the one from the authorize request."))
return errors.WithStack(fosite.ErrInvalidGrant.WithHint("The OAuth 2.0 Client ID from this request does not match the one from the authorize request."))
}

// ensure that the "redirect_uri" parameter is present if the
Expand All @@ -99,7 +99,7 @@ func (c *AuthorizeExplicitGrantHandler) HandleTokenEndpointRequest(ctx context.C
// their values are identical.
forcedRedirectURI := authorizeRequest.GetRequestForm().Get("redirect_uri")
if forcedRedirectURI != "" && forcedRedirectURI != request.GetRequestForm().Get("redirect_uri") {
return errors.WithStack(fosite.ErrInvalidRequest.WithHint("The \"redirect_uri\" from this request does not match the one from the authorize request."))
return errors.WithStack(fosite.ErrInvalidGrant.WithHint("The \"redirect_uri\" from this request does not match the one from the authorize request."))
}

// Checking of POST client_id skipped, because:
Expand Down
4 changes: 2 additions & 2 deletions handler/oauth2/flow_authorize_code_token_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ func TestAuthorizeCode_HandleTokenEndpointRequest(t *testing.T) {

require.NoError(t, store.CreateAuthorizeCodeSession(nil, signature, authreq))
},
expectErr: fosite.ErrInvalidRequest,
expectErr: fosite.ErrInvalidGrant,
},
{
areq: &fosite.AccessRequest{
Expand All @@ -346,7 +346,7 @@ func TestAuthorizeCode_HandleTokenEndpointRequest(t *testing.T) {

require.NoError(t, store.CreateAuthorizeCodeSession(nil, signature, authreq))
},
expectErr: fosite.ErrInvalidRequest,
expectErr: fosite.ErrInvalidGrant,
},
{
areq: &fosite.AccessRequest{
Expand Down
4 changes: 2 additions & 2 deletions handler/oauth2/flow_refresh.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.WithDebug("The refresh token has not been found: %s", err))
} else if err != nil {
return errors.WithStack(fosite.ErrServerError.WithDebug(err.Error()))
} else if err := c.RefreshTokenStrategy.ValidateRefreshToken(ctx, originalRequest, refresh); err != nil {
Expand All @@ -83,7 +83,7 @@ func (c *RefreshTokenGrantHandler) HandleTokenEndpointRequest(ctx context.Contex

// The authorization server MUST ... and ensure that the refresh token was issued to the authenticated client
if originalRequest.GetClient().GetID() != request.GetClient().GetID() {
return errors.WithStack(fosite.ErrInvalidRequest.WithHint("The OAuth 2.0 Client ID from this request does not match the ID during the initial token issuance."))
return errors.WithStack(fosite.ErrInvalidGrant.WithHint("The OAuth 2.0 Client ID from this request does not match the ID during the initial token issuance."))
}

request.SetSession(originalRequest.GetSession().Clone())
Expand Down
6 changes: 3 additions & 3 deletions handler/oauth2/flow_refresh_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func TestRefreshFlow_HandleTokenEndpointRequest(t *testing.T) {

areq.Form.Add("refresh_token", "some.refreshtokensig")
},
expectErr: fosite.ErrInvalidRequest,
expectErr: fosite.ErrInvalidGrant,
},
{
description: "should fail because token is valid but does not exist",
Expand All @@ -85,7 +85,7 @@ func TestRefreshFlow_HandleTokenEndpointRequest(t *testing.T) {
require.NoError(t, err)
areq.Form.Add("refresh_token", token)
},
expectErr: fosite.ErrInvalidRequest,
expectErr: fosite.ErrInvalidGrant,
},
{
description: "should fail because client mismatches",
Expand All @@ -107,7 +107,7 @@ func TestRefreshFlow_HandleTokenEndpointRequest(t *testing.T) {
})
require.NoError(t, err)
},
expectErr: fosite.ErrInvalidRequest,
expectErr: fosite.ErrInvalidGrant,
},
{
description: "should fail because offline scope has been granted but client no longer allowed to request it",
Expand Down

0 comments on commit f5a0e96

Please sign in to comment.