Skip to content

Commit b16e3fc

Browse files
arekkasarekkas
authored
0.3.1 (#98)
* all: better error handling - closes #100 * oauth2/implicit: bad HTML encoding of the scope parameter - closes #95 * oauth2: state parameter is missing when response_type=id_token - closes #96 * oauth2: id token hashes are not base64 url encoded - closes #97 * openid: hybrid flow using `token+code+id_token` returns multiple tokens of the same type - closes #99
1 parent cb328ca commit b16e3fc

12 files changed

+150
-28
lines changed

‎errors.go

Lines changed: 86 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,16 @@ var (
2323
ErrInsufficientEntropy = errors.Errorf("The request used a security parameter (e.g., anti-replay, anti-csrf) with insufficient entropy (minimum of %d characters)", MinParameterEntropy)
2424
ErrMisconfiguration = errors.New("The request failed because of an internal error that is probably caused by misconfiguration")
2525
ErrNotFound = errors.New("Could not find the requested resource(s)")
26+
ErrInvalidTokenFormat = errors.New("Invalid token format")
27+
ErrTokenSignatureMismatch = errors.New("Token signature mismatch")
28+
ErrTokenExpired = errors.New("Token expired")
29+
ErrScopeNotGranted = errors.New("The token was not granted the requested scope")
30+
ErrTokenClaim = errors.New("The token failed validation due to a claim mismatch")
2631
)
2732

2833
const (
34+
errRequestUnauthorized = "request_unauthorized"
35+
errRequestForbidden = "request_forbidden"
2936
errInvalidRequestName = "invalid_request"
3037
errUnauthorizedClientName = "unauthorized_client"
3138
errAccessDeniedName = "acccess_denied"
@@ -36,10 +43,16 @@ const (
3643
errUnsupportedGrantTypeName = "unsupported_grant_type"
3744
errInvalidGrantName = "invalid_grant"
3845
errInvalidClientName = "invalid_client"
39-
errInvalidError = "invalid_error"
46+
UnknownErrorName = "unknown_error"
47+
errNotFound = "not_found"
4048
errInvalidState = "invalid_state"
4149
errMisconfiguration = "misconfiguration"
4250
errInsufficientEntropy = "insufficient_entropy"
51+
errInvalidTokenFormat = "invalid_token"
52+
errTokenSignatureMismatch = "token_signature_mismatch"
53+
errTokenExpired = "token_expired"
54+
errScopeNotGranted = "scope_not_granted"
55+
errTokenClaim = "token_claim"
4356
)
4457

4558
type RFC6749Error struct {
@@ -52,6 +65,69 @@ type RFC6749Error struct {
5265

5366
func ErrorToRFC6749Error(err error) *RFC6749Error {
5467
switch errors.Cause(err) {
68+
case ErrTokenClaim: {
69+
return &RFC6749Error{
70+
Name: errTokenClaim,
71+
Description: ErrTokenClaim.Error(),
72+
Debug: err.Error(),
73+
Hint: "One or more token claims failed validation.",
74+
StatusCode: http.StatusUnauthorized,
75+
}
76+
}
77+
case ErrScopeNotGranted: {
78+
return &RFC6749Error{
79+
Name: errScopeNotGranted,
80+
Description: ErrScopeNotGranted.Error(),
81+
Debug: err.Error(),
82+
Hint: "The resource owner did not grant the requested scope.",
83+
StatusCode: http.StatusForbidden,
84+
}
85+
}
86+
case ErrTokenExpired: {
87+
return &RFC6749Error{
88+
Name: errTokenExpired,
89+
Description: ErrTokenExpired.Error(),
90+
Debug: err.Error(),
91+
Hint: "The token expired.",
92+
StatusCode: http.StatusUnauthorized,
93+
}
94+
}
95+
case ErrInvalidTokenFormat: {
96+
return &RFC6749Error{
97+
Name: errInvalidTokenFormat,
98+
Description: ErrInvalidTokenFormat.Error(),
99+
Debug: err.Error(),
100+
Hint: "Check that you provided a valid token in the right format.",
101+
StatusCode: http.StatusBadRequest,
102+
}
103+
}
104+
case ErrTokenSignatureMismatch: {
105+
return &RFC6749Error{
106+
Name: errTokenSignatureMismatch,
107+
Description: ErrTokenSignatureMismatch.Error(),
108+
Debug: err.Error(),
109+
Hint: "Check that you provided a valid token in the right format.",
110+
StatusCode: http.StatusBadRequest,
111+
}
112+
}
113+
case ErrRequestUnauthorized: {
114+
return &RFC6749Error{
115+
Name: errRequestUnauthorized,
116+
Description: ErrRequestUnauthorized.Error(),
117+
Debug: err.Error(),
118+
Hint: "Check that you provided valid credentials in the right format.",
119+
StatusCode: http.StatusUnauthorized,
120+
}
121+
}
122+
case ErrRequestForbidden: {
123+
return &RFC6749Error{
124+
Name: errRequestForbidden,
125+
Description: ErrRequestForbidden.Error(),
126+
Debug: err.Error(),
127+
Hint: "You are not allowed to perform this action.",
128+
StatusCode: http.StatusForbidden,
129+
}
130+
}
55131
case ErrInvalidRequest:
56132
return &RFC6749Error{
57133
Name: errInvalidRequestName,
@@ -123,7 +199,7 @@ func ErrorToRFC6749Error(err error) *RFC6749Error {
123199
Name: errInvalidClientName,
124200
Description: ErrInvalidClient.Error(),
125201
Debug: err.Error(),
126-
StatusCode: http.StatusBadRequest,
202+
StatusCode: http.StatusUnauthorized,
127203
}
128204
case ErrInvalidState:
129205
return &RFC6749Error{
@@ -146,9 +222,16 @@ func ErrorToRFC6749Error(err error) *RFC6749Error {
146222
Debug: err.Error(),
147223
StatusCode: http.StatusInternalServerError,
148224
}
225+
case ErrNotFound:
226+
return &RFC6749Error{
227+
Name: errNotFound,
228+
Description: ErrNotFound.Error(),
229+
Debug: err.Error(),
230+
StatusCode: http.StatusNotFound,
231+
}
149232
default:
150233
return &RFC6749Error{
151-
Name: errInvalidError,
234+
Name: UnknownErrorName,
152235
Description: "The error is unrecognizable.",
153236
Debug: err.Error(),
154237
StatusCode: http.StatusInternalServerError,

‎errors_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ import (
99
)
1010

1111
func TestErrorToRFC6749(t *testing.T) {
12-
assert.Equal(t, errInvalidError, ErrorToRFC6749Error(errors.New("")).Name)
13-
assert.Equal(t, errInvalidError, ErrorToRFC6749Error(native.New("")).Name)
12+
assert.Equal(t, UnknownErrorName, ErrorToRFC6749Error(errors.New("")).Name)
13+
assert.Equal(t, UnknownErrorName, ErrorToRFC6749Error(native.New("")).Name)
1414

1515
assert.Equal(t, errInvalidRequestName, ErrorToRFC6749Error(errors.Wrap(ErrInvalidRequest, "")).Name)
1616
assert.Equal(t, errUnauthorizedClientName, ErrorToRFC6749Error(errors.Wrap(ErrUnauthorizedClient, "")).Name)

‎handler/oauth2/flow_authorize_implicit.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ func (c *AuthorizeImplicitGrantTypeHandler) IssueImplicitAccessToken(ctx context
6767
resp.AddFragment("expires_in", strconv.Itoa(int(c.AccessTokenLifespan/time.Second)))
6868
resp.AddFragment("token_type", "bearer")
6969
resp.AddFragment("state", ar.GetState())
70-
resp.AddFragment("scope", strings.Join(ar.GetGrantedScopes(), "+"))
70+
resp.AddFragment("scope", strings.Join(ar.GetGrantedScopes(), " "))
7171
ar.SetResponseTypeHandled("token")
7272

7373
return nil

‎handler/oauth2/strategy_hmacsha.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
enigma "github.com/ory-am/fosite/token/hmac"
99
"github.com/pkg/errors"
1010
"golang.org/x/net/context"
11+
"fmt"
1112
)
1213

1314
type HMACSHAStrategy struct {
@@ -32,9 +33,9 @@ func (h HMACSHAStrategy) GenerateAccessToken(_ context.Context, _ fosite.Request
3233

3334
func (h HMACSHAStrategy) ValidateAccessToken(_ context.Context, r fosite.Requester, token string) (err error) {
3435
if session, ok := r.GetSession().(HMACSessionContainer); !ok {
35-
return errors.Errorf("Session must be of type HMACSessionContainer, got: %s", reflect.TypeOf(r.GetSession()))
36+
return errors.Wrap(fosite.ErrMisconfiguration, fmt.Sprintf("Session must be of type HMACSessionContainer, got: %s", reflect.TypeOf(r.GetSession())))
3637
} else if session.AccessTokenExpiresAt(r.GetRequestedAt().Add(h.AccessTokenLifespan)).Before(time.Now()) {
37-
return errors.Errorf("Access token expired at %s", session.AccessTokenExpiresAt(r.GetRequestedAt().Add(h.AccessTokenLifespan)))
38+
return errors.Wrap(fosite.ErrTokenExpired, fmt.Sprintf("Access token expired at %s", session.AccessTokenExpiresAt(r.GetRequestedAt().Add(h.AccessTokenLifespan))))
3839
}
3940
return h.Enigma.Validate(token)
4041
}
@@ -53,9 +54,9 @@ func (h HMACSHAStrategy) GenerateAuthorizeCode(_ context.Context, _ fosite.Reque
5354

5455
func (h HMACSHAStrategy) ValidateAuthorizeCode(_ context.Context, r fosite.Requester, token string) (err error) {
5556
if session, ok := r.GetSession().(HMACSessionContainer); !ok {
56-
return errors.Errorf("Session must be of type HMACSessionContainer, got: %s", reflect.TypeOf(r.GetSession()))
57+
return errors.Wrap(fosite.ErrMisconfiguration, fmt.Sprintf("Session must be of type HMACSessionContainer, got: %s", reflect.TypeOf(r.GetSession())))
5758
} else if session.AuthorizeCodeExpiresAt(r.GetRequestedAt().Add(h.AuthorizeCodeLifespan)).Before(time.Now()) {
58-
return errors.Errorf("Authorize code expired at %s", session.AuthorizeCodeExpiresAt(r.GetRequestedAt().Add(h.AccessTokenLifespan)))
59+
return errors.Wrap(fosite.ErrTokenExpired, fmt.Sprintf("Authorize code expired at %s", session.AuthorizeCodeExpiresAt(r.GetRequestedAt().Add(h.AccessTokenLifespan))))
5960
}
6061

6162
return h.Enigma.Validate(token)

‎handler/oauth2/strategy_jwt.go

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55

66
"github.com/ory-am/fosite"
77
"github.com/ory-am/fosite/token/jwt"
8+
jwtx "github.com/dgrijalva/jwt-go"
89
"github.com/pkg/errors"
910
"golang.org/x/net/context"
1011
)
@@ -67,7 +68,31 @@ func (h *RS256JWTStrategy) validate(token string) error {
6768

6869
// validate the token
6970
if err = t.Claims.Valid(); err != nil {
70-
return errors.New("Token claims did not validate")
71+
if e, ok := err.(*jwtx.ValidationError); ok {
72+
switch e.Errors {
73+
case jwtx.ValidationErrorMalformed:
74+
return errors.Wrap(fosite.ErrInvalidTokenFormat, err.Error())
75+
case jwtx.ValidationErrorUnverifiable:
76+
return errors.Wrap(fosite.ErrTokenSignatureMismatch, err.Error())
77+
case jwtx.ValidationErrorSignatureInvalid:
78+
return errors.Wrap(fosite.ErrTokenSignatureMismatch, err.Error())
79+
case jwtx.ValidationErrorAudience:
80+
return errors.Wrap(fosite.ErrTokenClaim, err.Error())
81+
case jwtx.ValidationErrorExpired:
82+
return errors.Wrap(fosite.ErrTokenExpired, err.Error())
83+
case jwtx.ValidationErrorIssuedAt:
84+
return errors.Wrap(fosite.ErrTokenClaim, err.Error())
85+
case jwtx.ValidationErrorIssuer :
86+
return errors.Wrap(fosite.ErrTokenClaim, err.Error())
87+
case jwtx.ValidationErrorNotValidYet:
88+
return errors.Wrap(fosite.ErrTokenClaim, err.Error())
89+
case jwtx.ValidationErrorId:
90+
return errors.Wrap(fosite.ErrTokenClaim, err.Error())
91+
case jwtx.ValidationErrorClaimsInvalid :
92+
return errors.Wrap(fosite.ErrTokenClaim, err.Error())
93+
}
94+
return errors.Wrap(fosite.ErrRequestUnauthorized, err.Error())
95+
}
7196
}
7297

7398
return nil

‎handler/oauth2/validator.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ func (c *CoreValidator) validateAccessToken(ctx context.Context, token string, a
3131
if err != nil {
3232
return errors.Wrap(fosite.ErrRequestUnauthorized, err.Error())
3333
} else if err := c.CoreStrategy.ValidateAccessToken(ctx, or, token); err != nil {
34-
return errors.Wrap(fosite.ErrRequestUnauthorized, err.Error())
34+
return err
3535
}
3636

3737
for _, scope := range scopes {
@@ -49,7 +49,7 @@ func (c *CoreValidator) validateRefreshToken(ctx context.Context, token string,
4949
if or, err := c.CoreStorage.GetAccessTokenSession(ctx, sig, accessRequest.GetSession()); err != nil {
5050
return errors.Wrap(fosite.ErrRequestUnauthorized, err.Error())
5151
} else if err := c.CoreStrategy.ValidateAccessToken(ctx, or, token); err != nil {
52-
return errors.Wrap(fosite.ErrRequestUnauthorized, err.Error())
52+
return err
5353
} else {
5454
accessRequest.Merge(or)
5555
}
@@ -60,9 +60,9 @@ func (c *CoreValidator) validateRefreshToken(ctx context.Context, token string,
6060
func (c *CoreValidator) validateAuthorizeCode(ctx context.Context, token string, accessRequest fosite.AccessRequester) error {
6161
sig := c.CoreStrategy.AccessTokenSignature(token)
6262
if or, err := c.CoreStorage.GetAccessTokenSession(ctx, sig, accessRequest.GetSession()); err != nil {
63-
return errors.Wrap(fosite.ErrRequestUnauthorized, err.Error())
63+
return errors.Wrap(err, fosite.ErrRequestUnauthorized.Error())
6464
} else if err := c.CoreStrategy.ValidateAccessToken(ctx, or, token); err != nil {
65-
return errors.Wrap(fosite.ErrRequestUnauthorized, err.Error())
65+
return err
6666
} else {
6767
accessRequest.Merge(or)
6868
}

‎handler/oauth2/validator_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,9 @@ func TestValidateToken(t *testing.T) {
5151
description: "should fail because validation fails",
5252
setup: func() {
5353
store.EXPECT().GetAccessTokenSession(nil, "asdf", nil).AnyTimes().Return(areq, nil)
54-
chgen.EXPECT().ValidateAccessToken(nil, areq, "1234").Return(errors.New(""))
54+
chgen.EXPECT().ValidateAccessToken(nil, areq, "1234").Return(errors.Wrap(fosite.ErrTokenExpired, ""))
5555
},
56-
expectErr: fosite.ErrRequestUnauthorized,
56+
expectErr: fosite.ErrTokenExpired,
5757
},
5858
{
5959
description: "should pass",

‎handler/openid/flow_hybrid.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"github.com/ory-am/fosite/token/jwt"
1111
"github.com/pkg/errors"
1212
"golang.org/x/net/context"
13+
"encoding/base64"
1314
)
1415

1516
type OpenIDConnectHybridHandler struct {
@@ -62,14 +63,13 @@ func (c *OpenIDConnectHybridHandler) HandleAuthorizeEndpointRequest(ctx context.
6263
}
6364

6465
resp.AddFragment("code", code)
65-
resp.AddFragment("state", ar.GetState())
6666
ar.SetResponseTypeHandled("code")
6767

6868
hash, err := c.Enigma.Hash([]byte(resp.GetFragment().Get("code")))
6969
if err != nil {
7070
return err
7171
}
72-
claims.CodeHash = hash[:c.Enigma.GetSigningMethodLength()/2]
72+
claims.CodeHash = []byte(base64.URLEncoding.EncodeToString([]byte(hash[:c.Enigma.GetSigningMethodLength()/2])))
7373
}
7474

7575
if ar.GetResponseTypes().Has("token") {
@@ -84,7 +84,7 @@ func (c *OpenIDConnectHybridHandler) HandleAuthorizeEndpointRequest(ctx context.
8484
if err != nil {
8585
return err
8686
}
87-
claims.AccessTokenHash = hash[:c.Enigma.GetSigningMethodLength()/2]
87+
claims.AccessTokenHash = []byte(base64.URLEncoding.EncodeToString([]byte(hash[:c.Enigma.GetSigningMethodLength()/2])))
8888
}
8989

9090
if !ar.GetGrantedScopes().Has("openid") {
@@ -93,13 +93,15 @@ func (c *OpenIDConnectHybridHandler) HandleAuthorizeEndpointRequest(ctx context.
9393

9494
if err := c.IDTokenHandleHelper.IssueImplicitIDToken(ctx, req, ar, resp); err != nil {
9595
return errors.Wrap(err, err.Error())
96-
} else if err := c.IDTokenHandleHelper.IssueImplicitIDToken(ctx, req, ar, resp); err != nil {
97-
return errors.Wrap(err, err.Error())
9896
}
9997

10098
// there is no need to check for https, because implicit flow does not require https
10199
// https://tools.ietf.org/html/rfc6819#section-4.4.2
102100

101+
if resp.GetFragment().Get("state") == "" {
102+
resp.AddFragment("state", ar.GetState())
103+
}
104+
103105
ar.SetResponseTypeHandled("id_token")
104106
return nil
105107
}

‎handler/openid/flow_implicit.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"github.com/ory-am/fosite/token/jwt"
1111
"github.com/pkg/errors"
1212
"golang.org/x/net/context"
13+
"encoding/base64"
1314
)
1415

1516
type OpenIDConnectImplicitHandler struct {
@@ -23,6 +24,9 @@ type OpenIDConnectImplicitHandler struct {
2324
func (c *OpenIDConnectImplicitHandler) HandleAuthorizeEndpointRequest(ctx context.Context, req *http.Request, ar fosite.AuthorizeRequester, resp fosite.AuthorizeResponder) error {
2425
if !(ar.GetGrantedScopes().Has("openid") && (ar.GetResponseTypes().Has("token", "id_token") || ar.GetResponseTypes().Exact("id_token"))) {
2526
return nil
27+
} else if ar.GetResponseTypes().Has("code") {
28+
// hybrid flow
29+
return nil
2630
}
2731

2832
if !ar.GetClient().GetGrantTypes().Has("implicit") {
@@ -59,7 +63,9 @@ func (c *OpenIDConnectImplicitHandler) HandleAuthorizeEndpointRequest(ctx contex
5963
return err
6064
}
6165

62-
claims.AccessTokenHash = hash[:c.RS256JWTStrategy.GetSigningMethodLength()/2]
66+
claims.AccessTokenHash = []byte(base64.URLEncoding.EncodeToString([]byte(hash[:c.RS256JWTStrategy.GetSigningMethodLength()/2])))
67+
} else {
68+
resp.AddFragment("state", ar.GetState())
6369
}
6470

6571
if err := c.IssueImplicitIDToken(ctx, req, ar, resp); err != nil {

‎handler/openid/flow_implicit_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ func TestImplicit_HandleAuthorizeEndpointRequest(t *testing.T) {
4848
description: "should not do anything because request requirements are not met",
4949
setup: func() {
5050
areq.ResponseTypes = fosite.Arguments{"id_token"}
51+
areq.State = "foostate"
5152
},
5253
},
5354
{
@@ -121,6 +122,7 @@ func TestImplicit_HandleAuthorizeEndpointRequest(t *testing.T) {
121122
},
122123
check: func() {
123124
assert.NotEmpty(t, aresp.GetFragment().Get("id_token"))
125+
assert.NotEmpty(t, aresp.GetFragment().Get("state"))
124126
assert.Empty(t, aresp.GetFragment().Get("access_token"))
125127
},
126128
},
@@ -131,6 +133,7 @@ func TestImplicit_HandleAuthorizeEndpointRequest(t *testing.T) {
131133
},
132134
check: func() {
133135
assert.NotEmpty(t, aresp.GetFragment().Get("id_token"))
136+
assert.NotEmpty(t, aresp.GetFragment().Get("state"))
134137
assert.NotEmpty(t, aresp.GetFragment().Get("access_token"))
135138
},
136139
},
@@ -142,6 +145,7 @@ func TestImplicit_HandleAuthorizeEndpointRequest(t *testing.T) {
142145
},
143146
check: func() {
144147
assert.NotEmpty(t, aresp.GetFragment().Get("id_token"))
148+
assert.NotEmpty(t, aresp.GetFragment().Get("state"))
145149
assert.NotEmpty(t, aresp.GetFragment().Get("access_token"))
146150
},
147151
},

0 commit comments

Comments
 (0)