From f5a0e9696750e3f1d67bd919a6588b175e7cc2bb Mon Sep 17 00:00:00 2001 From: hackerman <3372410+aeneasr@users.noreply.github.com> Date: Thu, 16 Apr 2020 12:16:53 +0200 Subject: [PATCH] fix: return invalid_grant instead of invalid_request in refresh flow (#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 --- handler/oauth2/flow_authorize_code_token.go | 4 ++-- handler/oauth2/flow_authorize_code_token_test.go | 4 ++-- handler/oauth2/flow_refresh.go | 4 ++-- handler/oauth2/flow_refresh_test.go | 6 +++--- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/handler/oauth2/flow_authorize_code_token.go b/handler/oauth2/flow_authorize_code_token.go index a15967313..c1ea9a640 100644 --- a/handler/oauth2/flow_authorize_code_token.go +++ b/handler/oauth2/flow_authorize_code_token.go @@ -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 @@ -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: diff --git a/handler/oauth2/flow_authorize_code_token_test.go b/handler/oauth2/flow_authorize_code_token_test.go index 0d62419e0..e6193b762 100644 --- a/handler/oauth2/flow_authorize_code_token_test.go +++ b/handler/oauth2/flow_authorize_code_token_test.go @@ -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{ @@ -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{ diff --git a/handler/oauth2/flow_refresh.go b/handler/oauth2/flow_refresh.go index 33a296755..1b79c34ab 100644 --- a/handler/oauth2/flow_refresh.go +++ b/handler/oauth2/flow_refresh.go @@ -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 { @@ -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()) diff --git a/handler/oauth2/flow_refresh_test.go b/handler/oauth2/flow_refresh_test.go index 6084d94d8..e9141e3f9 100644 --- a/handler/oauth2/flow_refresh_test.go +++ b/handler/oauth2/flow_refresh_test.go @@ -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", @@ -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", @@ -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",