Skip to content

Commit

Permalink
Improves error debug messages across the project
Browse files Browse the repository at this point in the history
  • Loading branch information
arekkas authored and arekkas committed Dec 9, 2017
1 parent 392c191 commit 7ec8d19
Show file tree
Hide file tree
Showing 25 changed files with 157 additions and 129 deletions.
20 changes: 10 additions & 10 deletions access_request_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,9 @@ func (f *Fosite) NewAccessRequest(ctx context.Context, r *http.Request, session
accessRequest := NewAccessRequest(session)

if r.Method != "POST" {
return accessRequest, errors.Wrap(ErrInvalidRequest, "HTTP method is not POST")
return accessRequest, errors.WithStack(ErrInvalidRequest.WithDebug("HTTP method is not POST"))
} else if err := r.ParseForm(); err != nil {
return accessRequest, errors.Wrap(ErrInvalidRequest, err.Error())
return accessRequest, errors.WithStack(ErrInvalidRequest.WithDebug(err.Error()))
}

accessRequest.Form = r.PostForm
Expand All @@ -67,7 +67,7 @@ func (f *Fosite) NewAccessRequest(ctx context.Context, r *http.Request, session
accessRequest.SetRequestedScopes(removeEmpty(strings.Split(r.PostForm.Get("scope"), " ")))
accessRequest.GrantTypes = removeEmpty(strings.Split(r.PostForm.Get("grant_type"), " "))
if len(accessRequest.GrantTypes) < 1 {
return accessRequest, errors.Wrap(ErrInvalidRequest, "No grant type given")
return accessRequest, errors.WithStack(ErrInvalidRequest.WithDebug("No grant type given"))
}

// Decode client_id and client_secret which should be in "application/x-www-form-urlencoded" format.
Expand All @@ -78,13 +78,13 @@ func (f *Fosite) NewAccessRequest(ctx context.Context, r *http.Request, session

client, err := f.Store.GetClient(ctx, clientID)
if err != nil {
return accessRequest, errors.Wrap(ErrInvalidClient, err.Error())
return accessRequest, errors.WithStack(ErrInvalidClient.WithDebug(err.Error()))
}

if !client.IsPublic() {
// Enforce client authentication
if err := f.Hasher.Compare(client.GetHashedSecret(), []byte(clientSecret)); err != nil {
return accessRequest, errors.Wrap(ErrInvalidClient, err.Error())
return accessRequest, errors.WithStack(ErrInvalidClient.WithDebug(err.Error()))
}
}
accessRequest.Client = client
Expand All @@ -110,9 +110,9 @@ func clientCredentialsFromRequest(r *http.Request) (clientID, clientSecret strin
if id, secret, ok := r.BasicAuth(); !ok {
return clientCredentialsFromRequestBody(r)
} else if clientID, err = url.QueryUnescape(id); err != nil {
return "", "", errors.Wrap(ErrInvalidRequest, `The client id in the HTTP authorization header could not be decoded from "application/x-www-form-urlencoded"`)
return "", "", errors.WithStack(ErrInvalidRequest.WithDebug(`The client id in the HTTP authorization header could not be decoded from "application/x-www-form-urlencoded"`))
} else if clientSecret, err = url.QueryUnescape(secret); err != nil {
return "", "", errors.Wrap(ErrInvalidRequest, `The client secret in the HTTP authorization header could not be decoded from "application/x-www-form-urlencoded"`)
return "", "", errors.WithStack(ErrInvalidRequest.WithDebug(`The client secret in the HTTP authorization header could not be decoded from "application/x-www-form-urlencoded"`))
}

return clientID, clientSecret, nil
Expand All @@ -123,13 +123,13 @@ func clientCredentialsFromRequestBody(r *http.Request) (clientID, clientSecret s
clientSecret = r.PostForm.Get("client_secret")

if clientID == "" {
return "", "", errors.Wrap(ErrInvalidRequest, "Client credentials missing or malformed in both HTTP Authorization header and HTTP POST body")
return "", "", errors.WithStack(ErrInvalidRequest.WithDebug("Client credentials missing or malformed in both HTTP Authorization header and HTTP POST body"))
}

if clientID, err = url.QueryUnescape(clientID); err != nil {
return "", "", errors.Wrap(ErrInvalidRequest, `The client id in the HTTP authorization header could not be decoded from "application/x-www-form-urlencoded"`)
return "", "", errors.WithStack(ErrInvalidRequest.WithDebug(`The client id in the HTTP authorization header could not be decoded from "application/x-www-form-urlencoded"`))
} else if clientSecret, err = url.QueryUnescape(clientSecret); err != nil {
return "", "", errors.Wrap(ErrInvalidRequest, `The client secret in the HTTP authorization header could not be decoded from "application/x-www-form-urlencoded"`)
return "", "", errors.WithStack(ErrInvalidRequest.WithDebug(`The client secret in the HTTP authorization header could not be decoded from "application/x-www-form-urlencoded"`))
}

return clientID, clientSecret, nil
Expand Down
2 changes: 1 addition & 1 deletion access_response_writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func (f *Fosite) NewAccessResponse(ctx context.Context, requester AccessRequeste
}

if response.GetAccessToken() == "" || response.GetTokenType() == "" {
return nil, errors.Wrap(ErrServerError, "Access token or token type not set")
return nil, errors.WithStack(ErrServerError.WithDebug("Access token or token type not set"))
}

return response, nil
Expand Down
4 changes: 2 additions & 2 deletions authorize_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func GetRedirectURIFromRequestValues(values url.Values) (string, error) {
// The endpoint URI MAY include an "application/x-www-form-urlencoded" formatted (per Appendix B) query component
redirectURI, err := url.QueryUnescape(values.Get("redirect_uri"))
if err != nil {
return "", errors.Wrap(ErrInvalidRequest, "redirect_uri parameter malformed or missing")
return "", errors.WithStack(ErrInvalidRequest.WithDebug("redirect_uri parameter malformed or missing"))
}
return redirectURI, nil
}
Expand Down Expand Up @@ -85,7 +85,7 @@ func MatchRedirectURIWithClientRedirectURIs(rawurl string, client Client) (*url.
}
}

return nil, errors.Wrap(ErrInvalidRequest, "redirect_uri parameter does not match with registered client redirect urls")
return nil, errors.WithStack(ErrInvalidRequest.WithDebug("redirect_uri parameter does not match with registered client redirect urls"))
}

// IsValidRedirectURI validates a redirect_uri as specified in:
Expand Down
12 changes: 7 additions & 5 deletions authorize_request_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import (

"context"

"fmt"

"github.com/pkg/errors"
)

Expand All @@ -31,7 +33,7 @@ func (c *Fosite) NewAuthorizeRequest(ctx context.Context, r *http.Request) (Auth
}

if err := r.ParseForm(); err != nil {
return request, errors.Wrap(ErrInvalidRequest, err.Error())
return request, errors.WithStack(ErrInvalidRequest.WithDebug(err.Error()))
}

request.Form = r.Form
Expand All @@ -44,15 +46,15 @@ func (c *Fosite) NewAuthorizeRequest(ctx context.Context, r *http.Request) (Auth
// Fetch redirect URI from request
rawRedirURI, err := GetRedirectURIFromRequestValues(r.Form)
if err != nil {
return request, errors.Wrap(ErrInvalidRequest, err.Error())
return request, errors.WithStack(ErrInvalidRequest.WithDebug(err.Error()))
}

// Validate redirect uri
redirectURI, err := MatchRedirectURIWithClientRedirectURIs(rawRedirURI, client)
if err != nil {
return request, errors.Wrap(ErrInvalidRequest, err.Error())
return request, errors.WithStack(ErrInvalidRequest.WithDebug(err.Error()))
} else if !IsValidRedirectURI(redirectURI) {
return request, errors.Wrap(ErrInvalidRequest, "not a valid redirect uri")
return request, errors.WithStack(ErrInvalidRequest.WithDebug("not a valid redirect uri"))
}
request.RedirectURI = redirectURI

Expand All @@ -72,7 +74,7 @@ func (c *Fosite) NewAuthorizeRequest(ctx context.Context, r *http.Request) (Auth
state := r.Form.Get("state")
if len(state) < MinParameterEntropy {
// We're assuming that using less then 8 characters for the state can not be considered "unguessable"
return request, errors.Wrapf(ErrInvalidState, "state length must at least be %d characters long", MinParameterEntropy)
return request, errors.WithStack(ErrInvalidState.WithDebug(fmt.Sprintf("State length must at least be %d characters long", MinParameterEntropy)))
}
request.State = state

Expand Down
11 changes: 11 additions & 0 deletions errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ import (
)

var (
ErrUnknownRequest = &RFC6749Error{
Name: errUnknownErrorName,
Description: "The handler is not responsible for this request",
Code: http.StatusInternalServerError,
}
ErrRequestForbidden = &RFC6749Error{
Name: errRequestForbidden,
Description: "The request is not allowed",
Expand Down Expand Up @@ -229,3 +234,9 @@ func (e *RFC6749Error) Details() []map[string]interface{} {
func (e *RFC6749Error) StatusCode() int {
return e.Code
}

func (e *RFC6749Error) WithDebug(debug string) *RFC6749Error {
err := *e
e.Debug = debug
return &err
}
14 changes: 14 additions & 0 deletions errors_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package fosite

import (
"testing"

"github.com/stretchr/testify/assert"
)

func TestAddDebug(t *testing.T) {
err := ErrRevokationClientMismatch.WithDebug("debug")
assert.NotEqual(t, err, ErrRevokationClientMismatch)
assert.Empty(t, ErrRevokationClientMismatch.Debug)
assert.NotEmpty(t, err.Debug)
}
4 changes: 0 additions & 4 deletions handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,8 @@ package fosite

import (
"context"

"github.com/pkg/errors"
)

var ErrUnknownRequest = errors.New("The handler is not responsible for this request.")

type AuthorizeEndpointHandler interface {
// HandleAuthorizeRequest handles an authorize endpoint request. To extend the handler's capabilities, the http request
// is passed along, if further information retrieval is required. If the handler feels that he is not responsible for
Expand Down
8 changes: 4 additions & 4 deletions handler/oauth2/flow_authorize_code_auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,13 @@ func (c *AuthorizeExplicitGrantHandler) HandleAuthorizeEndpointRequest(ctx conte
}

if !fosite.IsRedirectURISecure(ar.GetRedirectURI()) {
return errors.Wrap(fosite.ErrInvalidRequest, "Redirect URL is using an insecure protocol, http is only allowed for hosts with suffix `localhost`, for example: http://myapp.localhost/")
return errors.WithStack(fosite.ErrInvalidRequest.WithDebug("Redirect URL is using an insecure protocol, http is only allowed for hosts with suffix `localhost`, for example: http://myapp.localhost/"))
}

client := ar.GetClient()
for _, scope := range ar.GetRequestedScopes() {
if !c.ScopeStrategy(client.GetScopes(), scope) {
return errors.Wrap(fosite.ErrInvalidScope, fmt.Sprintf("The client is not allowed to request scope %s", scope))
return errors.WithStack(fosite.ErrInvalidScope.WithDebug(fmt.Sprintf("The client is not allowed to request scope %s", scope)))
}
}

Expand All @@ -71,12 +71,12 @@ func (c *AuthorizeExplicitGrantHandler) HandleAuthorizeEndpointRequest(ctx conte
func (c *AuthorizeExplicitGrantHandler) IssueAuthorizeCode(ctx context.Context, ar fosite.AuthorizeRequester, resp fosite.AuthorizeResponder) error {
code, signature, err := c.AuthorizeCodeStrategy.GenerateAuthorizeCode(ctx, ar)
if err != nil {
return errors.Wrap(fosite.ErrServerError, err.Error())
return errors.WithStack(fosite.ErrServerError.WithDebug(err.Error()))
}

ar.GetSession().SetExpiresAt(fosite.AuthorizeCode, time.Now().Add(c.AuthCodeLifespan))
if err := c.CoreStorage.CreateAuthorizeCodeSession(ctx, signature, ar); err != nil {
return errors.Wrap(fosite.ErrServerError, err.Error())
return errors.WithStack(fosite.ErrServerError.WithDebug(err.Error()))
}

resp.AddQuery("code", code)
Expand Down
24 changes: 12 additions & 12 deletions handler/oauth2/flow_authorize_code_token.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func (c *AuthorizeExplicitGrantHandler) HandleTokenEndpointRequest(ctx context.C
}

if !request.GetClient().GetGrantTypes().Has("authorization_code") {
return errors.Wrap(errors.WithStack(fosite.ErrInvalidGrant), "The client is not allowed to use grant type authorization_code")
return errors.WithStack(fosite.ErrInvalidGrant.WithDebug("The client is not allowed to use grant type authorization_code"))
}

code := request.GetRequestForm().Get("code")
Expand All @@ -46,13 +46,13 @@ func (c *AuthorizeExplicitGrantHandler) HandleTokenEndpointRequest(ctx context.C

return errors.WithStack(fosite.ErrInactiveCode)
} else if err != nil {
return errors.Wrap(fosite.ErrServerError, err.Error())
return errors.WithStack(fosite.ErrServerError.WithDebug(err.Error()))
}

// The authorization server MUST verify that the authorization code is valid
// This needs to happen after store retrieval for the session to be hydrated properly
if err := c.AuthorizeCodeStrategy.ValidateAuthorizeCode(ctx, request, code); err != nil {
return errors.Wrap(fosite.ErrInactiveCode, err.Error())
return errors.WithStack(fosite.ErrInactiveCode.WithDebug(err.Error()))
}

// Override scopes
Expand All @@ -62,7 +62,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.Wrap(errors.WithStack(fosite.ErrInactiveCode), "Client ID mismatch")
return errors.WithStack(fosite.ErrInactiveCode.WithDebug("Client ID mismatch"))
}

// ensure that the "redirect_uri" parameter is present if the
Expand All @@ -71,7 +71,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.Wrap(errors.WithStack(fosite.ErrInvalidRequest), "Redirect URI mismatch")
return errors.WithStack(fosite.ErrInvalidRequest.WithDebug("Redirect URI mismatch"))
}

// Checking of POST client_id skipped, because:
Expand All @@ -96,10 +96,10 @@ func (c *AuthorizeExplicitGrantHandler) PopulateTokenEndpointResponse(ctx contex
signature := c.AuthorizeCodeStrategy.AuthorizeCodeSignature(code)
authorizeRequest, err := c.CoreStorage.GetAuthorizeCodeSession(ctx, signature, requester.GetSession())
if err != nil {
return errors.Wrap(errors.WithStack(fosite.ErrServerError), err.Error())
return errors.WithStack(fosite.ErrServerError.WithDebug(err.Error()))
} else if err := c.AuthorizeCodeStrategy.ValidateAuthorizeCode(ctx, requester, code); err != nil {
// This needs to happen after store retrieval for the session to be hydrated properly
return errors.Wrap(errors.WithStack(fosite.ErrInvalidRequest), err.Error())
return errors.WithStack(fosite.ErrInvalidRequest.WithDebug(err.Error()))
}

for _, scope := range authorizeRequest.GetGrantedScopes() {
Expand All @@ -108,24 +108,24 @@ func (c *AuthorizeExplicitGrantHandler) PopulateTokenEndpointResponse(ctx contex

access, accessSignature, err := c.AccessTokenStrategy.GenerateAccessToken(ctx, requester)
if err != nil {
return errors.Wrap(errors.WithStack(fosite.ErrServerError), err.Error())
return errors.WithStack(fosite.ErrServerError.WithDebug(err.Error()))
}

var refresh, refreshSignature string
if authorizeRequest.GetGrantedScopes().Has("offline") {
refresh, refreshSignature, err = c.RefreshTokenStrategy.GenerateRefreshToken(ctx, requester)
if err != nil {
return errors.Wrap(errors.WithStack(fosite.ErrServerError), err.Error())
return errors.WithStack(fosite.ErrServerError.WithDebug(err.Error()))
}
}

if err := c.CoreStorage.DeleteAuthorizeCodeSession(ctx, signature); err != nil {
return errors.Wrap(errors.WithStack(fosite.ErrServerError), err.Error())
return errors.WithStack(fosite.ErrServerError.WithDebug(err.Error()))
} else if err := c.CoreStorage.CreateAccessTokenSession(ctx, accessSignature, requester); err != nil {
return errors.Wrap(errors.WithStack(fosite.ErrServerError), err.Error())
return errors.WithStack(fosite.ErrServerError.WithDebug(err.Error()))
} else if refreshSignature != "" {
if err := c.CoreStorage.CreateRefreshTokenSession(ctx, refreshSignature, requester); err != nil {
return errors.Wrap(errors.WithStack(fosite.ErrServerError), err.Error())
return errors.WithStack(fosite.ErrServerError.WithDebug(err.Error()))
}
}

Expand Down
10 changes: 5 additions & 5 deletions handler/oauth2/flow_authorize_implicit.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,17 +48,17 @@ func (c *AuthorizeImplicitGrantTypeHandler) HandleAuthorizeEndpointRequest(ctx c
}

if !ar.GetClient().GetResponseTypes().Has("token") {
return errors.Wrap(fosite.ErrInvalidGrant, "The client is not allowed to use response type token")
return errors.WithStack(fosite.ErrInvalidGrant.WithDebug("The client is not allowed to use response type token"))
}

if !ar.GetClient().GetGrantTypes().Has("implicit") {
return errors.Wrap(fosite.ErrInvalidGrant, "The client is not allowed to use grant type implicit")
return errors.WithStack(fosite.ErrInvalidGrant.WithDebug("The client is not allowed to use grant type implicit"))
}

client := ar.GetClient()
for _, scope := range ar.GetRequestedScopes() {
if !c.ScopeStrategy(client.GetScopes(), scope) {
return errors.Wrap(fosite.ErrInvalidScope, fmt.Sprintf("The client is not allowed to request scope %s", scope))
return errors.WithStack(fosite.ErrInvalidScope.WithDebug(fmt.Sprintf("The client is not allowed to request scope %s", scope)))
}
}

Expand All @@ -74,11 +74,11 @@ func (c *AuthorizeImplicitGrantTypeHandler) IssueImplicitAccessToken(ctx context
// Generate the code
token, signature, err := c.AccessTokenStrategy.GenerateAccessToken(ctx, ar)
if err != nil {
return errors.Wrap(fosite.ErrServerError, err.Error())
return errors.WithStack(fosite.ErrServerError.WithDebug(err.Error()))
}

if err := c.AccessTokenStorage.CreateAccessTokenSession(ctx, signature, ar); err != nil {
return errors.Wrap(fosite.ErrServerError, err.Error())
return errors.WithStack(fosite.ErrServerError.WithDebug(err.Error()))
}

resp.AddFragment("access_token", token)
Expand Down
6 changes: 3 additions & 3 deletions handler/oauth2/flow_client_credentials.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,15 @@ func (c *ClientCredentialsGrantHandler) HandleTokenEndpointRequest(_ context.Con
client := request.GetClient()
for _, scope := range request.GetRequestedScopes() {
if !c.ScopeStrategy(client.GetScopes(), scope) {
return errors.Wrap(fosite.ErrInvalidScope, fmt.Sprintf("The client is not allowed to request scope %s", scope))
return errors.WithStack(fosite.ErrInvalidScope.WithDebug(fmt.Sprintf("The client is not allowed to request scope %s", scope)))
}
}

// The client MUST authenticate with the authorization server as described in Section 3.2.1.
// This requirement is already fulfilled because fosite requries all token requests to be authenticated as described
// in https://tools.ietf.org/html/rfc6749#section-3.2.1
if client.IsPublic() {
return errors.Wrap(fosite.ErrInvalidGrant, "The client is public and thus not allowed to use grant type client_credentials")
return errors.WithStack(fosite.ErrInvalidGrant.WithDebug("The client is public and thus not allowed to use grant type client_credentials"))
}
// if the client is not public, he has already been authenticated by the access request handler.

Expand All @@ -63,7 +63,7 @@ func (c *ClientCredentialsGrantHandler) PopulateTokenEndpointResponse(ctx contex
}

if !request.GetClient().GetGrantTypes().Has("client_credentials") {
return errors.Wrap(fosite.ErrInvalidGrant, "The client is not allowed to use grant type client_credentials")
return errors.WithStack(fosite.ErrInvalidGrant.WithDebug("The client is not allowed to use grant type client_credentials"))
}

return c.IssueAccessToken(ctx, request, response)
Expand Down
Loading

0 comments on commit 7ec8d19

Please sign in to comment.