Skip to content

Commit

Permalink
fix: returning valid token_type in introspection response ory/hydra#1762
Browse files Browse the repository at this point in the history
  • Loading branch information
ajanthan authored and ajanthan committed Oct 2, 2020
1 parent db5ef81 commit 4aefc65
Show file tree
Hide file tree
Showing 12 changed files with 133 additions and 63 deletions.
20 changes: 10 additions & 10 deletions handler/oauth2/introspector.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,32 +36,32 @@ type CoreValidator struct {
DisableRefreshTokenValidation bool
}

func (c *CoreValidator) IntrospectToken(ctx context.Context, token string, tokenType fosite.TokenType, accessRequest fosite.AccessRequester, scopes []string) (fosite.TokenType, error) {
func (c *CoreValidator) IntrospectToken(ctx context.Context, token string, tokenUse fosite.TokenUse, accessRequest fosite.AccessRequester, scopes []string) (fosite.TokenUse, fosite.AccessTokenType, error) {
if c.DisableRefreshTokenValidation {
if err := c.introspectAccessToken(ctx, token, accessRequest, scopes); err != nil {
return "", err
return "", "", err
}
return fosite.AccessToken, nil
return fosite.AccessToken, fosite.BearerAccessToken, nil
}

var err error
switch tokenType {
switch tokenUse {
case fosite.RefreshToken:
if err = c.introspectRefreshToken(ctx, token, accessRequest, scopes); err == nil {
return fosite.RefreshToken, nil
return fosite.RefreshToken, "", nil
} else if err = c.introspectAccessToken(ctx, token, accessRequest, scopes); err == nil {
return fosite.AccessToken, nil
return fosite.AccessToken, fosite.BearerAccessToken, nil
}
return "", err
return "", "", err
}

if err = c.introspectAccessToken(ctx, token, accessRequest, scopes); err == nil {
return fosite.AccessToken, nil
return fosite.AccessToken, fosite.BearerAccessToken, nil
} else if err := c.introspectRefreshToken(ctx, token, accessRequest, scopes); err == nil {
return fosite.RefreshToken, nil
return fosite.RefreshToken, "", nil
}

return "", err
return "", "", err
}

func matchScopes(ss fosite.ScopeStrategy, granted, scopes []string) error {
Expand Down
8 changes: 4 additions & 4 deletions handler/oauth2/introspector_jwt.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,10 @@ type StatelessJWTValidator struct {
ScopeStrategy fosite.ScopeStrategy
}

func (v *StatelessJWTValidator) IntrospectToken(ctx context.Context, token string, tokenType fosite.TokenType, accessRequest fosite.AccessRequester, scopes []string) (fosite.TokenType, error) {
func (v *StatelessJWTValidator) IntrospectToken(ctx context.Context, token string, tokenUse fosite.TokenUse, accessRequest fosite.AccessRequester, scopes []string) (fosite.TokenUse, fosite.AccessTokenType, error) {
or, err := v.JWTAccessTokenStrategy.ValidateJWT(ctx, fosite.AccessToken, token)
if err != nil {
return "", err
return "", "", err
}

for _, scope := range scopes {
Expand All @@ -51,10 +51,10 @@ func (v *StatelessJWTValidator) IntrospectToken(ctx context.Context, token strin
}

if !v.ScopeStrategy(or.GetGrantedScopes(), scope) {
return "", errors.WithStack(fosite.ErrInvalidScope)
return "", "", errors.WithStack(fosite.ErrInvalidScope)
}
}

accessRequest.Merge(or)
return "", nil
return fosite.AccessToken, fosite.BearerAccessToken, nil
}
4 changes: 2 additions & 2 deletions handler/oauth2/introspector_jwt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ func TestIntrospectJWT(t *testing.T) {
}

areq := fosite.NewAccessRequest(nil)
_, err := v.IntrospectToken(nil, c.token(), fosite.AccessToken, areq, c.scopes)
_, _, err := v.IntrospectToken(nil, c.token(), fosite.AccessToken, areq, c.scopes)

if c.expectErr != nil {
require.EqualError(t, err, c.expectErr.Error())
Expand Down Expand Up @@ -146,7 +146,7 @@ func BenchmarkIntrospectJWT(b *testing.B) {
areq := fosite.NewAccessRequest(nil)

for n := 0; n < b.N; n++ {
_, err = v.IntrospectToken(nil, token, fosite.AccessToken, areq, []string{})
_, _, err = v.IntrospectToken(nil, token, fosite.AccessToken, areq, []string{})
}

assert.NoError(b, err)
Expand Down
8 changes: 4 additions & 4 deletions handler/oauth2/introspector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func TestIntrospectToken(t *testing.T) {
description string
setup func()
expectErr error
expectTT fosite.TokenType
expectTU fosite.TokenUse
}{
{
description: "should fail because no bearer token set",
Expand Down Expand Up @@ -99,18 +99,18 @@ func TestIntrospectToken(t *testing.T) {
setup: func() {
chgen.EXPECT().ValidateAccessToken(nil, areq, "1234").Return(nil)
},
expectTT: fosite.AccessToken,
expectTU: fosite.AccessToken,
},
} {
t.Run(fmt.Sprintf("case=%d", k), func(t *testing.T) {
c.setup()
tt, err := v.IntrospectToken(nil, fosite.AccessTokenFromRequest(httpreq), fosite.AccessToken, areq, []string{})
tu, _, err := v.IntrospectToken(nil, fosite.AccessTokenFromRequest(httpreq), fosite.AccessToken, areq, []string{})

if c.expectErr != nil {
require.EqualError(t, err, c.expectErr.Error())
} else {
require.NoError(t, err)
assert.Equal(t, c.expectTT, tt)
assert.Equal(t, c.expectTU, tu)
}
})
}
Expand Down
2 changes: 1 addition & 1 deletion integration/helper_endpoints_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func tokenIntrospectionHandler(t *testing.T, oauth2 fosite.OAuth2Provider, sessi
func tokenInfoHandler(t *testing.T, oauth2 fosite.OAuth2Provider, session fosite.Session) func(rw http.ResponseWriter, req *http.Request) {
return func(rw http.ResponseWriter, req *http.Request) {
ctx := fosite.NewContext()
_, resp, err := oauth2.IntrospectToken(ctx, fosite.AccessTokenFromRequest(req), fosite.AccessToken, session)
_, _, resp, err := oauth2.IntrospectToken(ctx, fosite.AccessTokenFromRequest(req), fosite.AccessToken, session)
if err != nil {
t.Logf("Info request failed because: %+v", err)
var e *fosite.RFC6749Error
Expand Down
8 changes: 4 additions & 4 deletions internal/introspector.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 10 additions & 8 deletions introspect.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import (
)

type TokenIntrospector interface {
IntrospectToken(ctx context.Context, token string, tokenType TokenType, accessRequest AccessRequester, scopes []string) (TokenType, error)
IntrospectToken(ctx context.Context, token string, tokenUse TokenUse, accessRequest AccessRequester, scopes []string) (TokenUse, AccessTokenType, error)
}

func AccessTokenFromRequest(req *http.Request) string {
Expand All @@ -52,27 +52,29 @@ func AccessTokenFromRequest(req *http.Request) string {
return split[1]
}

func (f *Fosite) IntrospectToken(ctx context.Context, token string, tokenType TokenType, session Session, scopes ...string) (TokenType, AccessRequester, error) {
func (f *Fosite) IntrospectToken(ctx context.Context, token string, tokenUse TokenUse, session Session, scopes ...string) (TokenUse, AccessTokenType, AccessRequester, error) {
var found = false
var foundTokenType TokenType = ""
var foundTokenUse TokenUse = ""
var foundAccessTokenType AccessTokenType = ""

ar := NewAccessRequest(session)
for _, validator := range f.TokenIntrospectionHandlers {
tt, err := validator.IntrospectToken(ctx, token, tokenType, ar, scopes)
tu, att, err := validator.IntrospectToken(ctx, token, tokenUse, ar, scopes)
if err == nil {
found = true
foundTokenType = tt
foundTokenUse = tu
foundAccessTokenType = att
} else if errors.Is(err, ErrUnknownRequest) {
// do nothing
} else {
rfcerr := ErrorToRFC6749Error(err)
return "", nil, errors.WithStack(rfcerr)
return "", "", nil, errors.WithStack(rfcerr)
}
}

if !found {
return "", nil, errors.WithStack(ErrRequestUnauthorized.WithHint("Unable to find a suitable validation strategy for the token, thus it is invalid."))
return "", "", nil, errors.WithStack(ErrRequestUnauthorized.WithHint("Unable to find a suitable validation strategy for the token, thus it is invalid."))
}

return foundTokenType, ar, nil
return foundTokenUse, foundAccessTokenType, ar, nil
}
12 changes: 6 additions & 6 deletions introspect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,24 +88,24 @@ func TestIntrospect(t *testing.T) {
scopes: []string{"foo"},
setup: func() {
f.TokenIntrospectionHandlers = TokenIntrospectionHandlers{validator}
validator.EXPECT().IntrospectToken(nil, "some-token", gomock.Any(), gomock.Any(), gomock.Any()).Return(TokenType(""), ErrUnknownRequest)
validator.EXPECT().IntrospectToken(nil, "some-token", gomock.Any(), gomock.Any(), gomock.Any()).Return(TokenUse(""), AccessTokenType(""), ErrUnknownRequest)
},
expectErr: ErrRequestUnauthorized,
},
{
description: "should fail",
scopes: []string{"foo"},
setup: func() {
validator.EXPECT().IntrospectToken(nil, "some-token", gomock.Any(), gomock.Any(), gomock.Any()).Return(TokenType(""), ErrInvalidClient)
validator.EXPECT().IntrospectToken(nil, "some-token", gomock.Any(), gomock.Any(), gomock.Any()).Return(TokenUse(""), AccessTokenType(""), ErrInvalidClient)
},
expectErr: ErrInvalidClient,
},
{
description: "should pass",
setup: func() {
validator.EXPECT().IntrospectToken(nil, "some-token", gomock.Any(), gomock.Any(), gomock.Any()).Do(func(ctx context.Context, _ string, _ TokenType, accessRequest AccessRequester, _ []string) {
validator.EXPECT().IntrospectToken(nil, "some-token", gomock.Any(), gomock.Any(), gomock.Any()).Do(func(ctx context.Context, _ string, _ TokenUse, accessRequest AccessRequester, _ []string) {
accessRequest.(*AccessRequest).GrantedScope = []string{"bar"}
}).Return(TokenType(""), nil)
}).Return(TokenUse(""), AccessTokenType(""), nil)
},
},
{
Expand All @@ -114,13 +114,13 @@ func TestIntrospect(t *testing.T) {
setup: func() {
validator.EXPECT().IntrospectToken(nil, "some-token", gomock.Any(), gomock.Any(), gomock.Any()).Do(func(ctx context.Context, _ string, _ TokenType, accessRequest AccessRequester, _ []string) {
accessRequest.(*AccessRequest).GrantedScope = []string{"bar"}
}).Return(TokenType(""), nil)
}).Return(TokenUse(""), AccessTokenType(""), nil)
},
},
} {
t.Run(fmt.Sprintf("case=%d", k), func(t *testing.T) {
c.setup()
_, _, err := f.IntrospectToken(nil, AccessTokenFromRequest(req), AccessToken, nil, c.scopes...)
_, _, _, err := f.IntrospectToken(nil, AccessTokenFromRequest(req), AccessToken, nil, c.scopes...)
if c.expectErr != nil {
assert.EqualError(t, err, c.expectErr.Error())
} else {
Expand Down
24 changes: 15 additions & 9 deletions introspection_request_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,17 +119,17 @@ func (f *Fosite) NewIntrospectionRequest(ctx context.Context, r *http.Request, s
}

token := r.PostForm.Get("token")
tokenType := r.PostForm.Get("token_type_hint")
tokenTypeHint := r.PostForm.Get("token_type_hint")
scope := r.PostForm.Get("scope")
if clientToken := AccessTokenFromRequest(r); clientToken != "" {
if token == clientToken {
return &IntrospectionResponse{Active: false}, errors.WithStack(ErrRequestUnauthorized.WithHint("Bearer and introspection token are identical."))
}

if tt, _, err := f.IntrospectToken(ctx, clientToken, AccessToken, session.Clone()); err != nil {
if tu, _, _, err := f.IntrospectToken(ctx, clientToken, AccessToken, session.Clone()); err != nil {
return &IntrospectionResponse{Active: false}, errors.WithStack(ErrRequestUnauthorized.WithHint("HTTP Authorization header missing, malformed, or credentials used are invalid."))
} else if tt != "" && tt != AccessToken {
return &IntrospectionResponse{Active: false}, errors.WithStack(ErrRequestUnauthorized.WithHintf("HTTP Authorization header did not provide a token of type \"access_token\", got type \"%s\".", tt))
} else if tu != "" && tu != AccessToken {
return &IntrospectionResponse{Active: false}, errors.WithStack(ErrRequestUnauthorized.WithHintf("HTTP Authorization header did not provide a token of type \"access_token\", got type \"%s\".", tu))
}
} else {
id, secret, ok := r.BasicAuth()
Expand Down Expand Up @@ -158,22 +158,24 @@ func (f *Fosite) NewIntrospectionRequest(ctx context.Context, r *http.Request, s
}
}

tt, ar, err := f.IntrospectToken(ctx, token, TokenType(tokenType), session, RemoveEmpty(strings.Split(scope, " "))...)
tu, att, ar, err := f.IntrospectToken(ctx, token, TokenUse(tokenTypeHint), session, RemoveEmpty(strings.Split(scope, " "))...)
if err != nil {
return &IntrospectionResponse{Active: false}, errors.WithStack(ErrInactiveToken.WithHint("An introspection strategy indicated that the token is inactive.").WithCause(err).WithDebug(err.Error()))
}

return &IntrospectionResponse{
Active: true,
AccessRequester: ar,
TokenType: tt,
TokenUse: tu,
AccessTokenType: att,
}, nil
}

type IntrospectionResponse struct {
Active bool `json:"active"`
AccessRequester AccessRequester `json:"extra"`
TokenType TokenType `json:"token_type,omitempty"`
TokenUse TokenUse `json:"token_use,omitempty"`
AccessTokenType AccessTokenType `json:"token_type,omitempty"`
}

func (r *IntrospectionResponse) IsActive() bool {
Expand All @@ -184,6 +186,10 @@ func (r *IntrospectionResponse) GetAccessRequester() AccessRequester {
return r.AccessRequester
}

func (r *IntrospectionResponse) GetTokenType() TokenType {
return r.TokenType
func (r *IntrospectionResponse) GetTokenUse() TokenUse {
return r.TokenUse
}

func (r *IntrospectionResponse) GetAccessTokenType() AccessTokenType {
return r.AccessTokenType
}
Loading

0 comments on commit 4aefc65

Please sign in to comment.