Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Returning Valid token_type in Introspection Response #486

Merged
merged 11 commits into from
Oct 6, 2020
4 changes: 2 additions & 2 deletions handler/oauth2/introspector.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ 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, error) {
if c.DisableRefreshTokenValidation {
if err := c.introspectAccessToken(ctx, token, accessRequest, scopes); err != nil {
return "", err
Expand All @@ -45,7 +45,7 @@ func (c *CoreValidator) IntrospectToken(ctx context.Context, token string, token
}

var err error
switch tokenType {
switch tokenUse {
case fosite.RefreshToken:
if err = c.introspectRefreshToken(ctx, token, accessRequest, scopes); err == nil {
return fosite.RefreshToken, nil
Expand Down
4 changes: 2 additions & 2 deletions handler/oauth2/introspector_jwt.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ 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, error) {
or, err := v.JWTAccessTokenStrategy.ValidateJWT(ctx, fosite.AccessToken, token)
if err != nil {
return "", err
Expand All @@ -56,5 +56,5 @@ func (v *StatelessJWTValidator) IntrospectToken(ctx context.Context, token strin
}

accessRequest.Merge(or)
return "", nil
return fosite.AccessToken, nil
}
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
1 change: 0 additions & 1 deletion internal/introspector.go

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

12 changes: 6 additions & 6 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, error)
}

func AccessTokenFromRequest(req *http.Request) string {
Expand All @@ -52,16 +52,16 @@ 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, AccessRequester, error) {
var found = false
var foundTokenType TokenType = ""
var foundTokenUse TokenUse = ""

ar := NewAccessRequest(session)
for _, validator := range f.TokenIntrospectionHandlers {
tt, err := validator.IntrospectToken(ctx, token, tokenType, ar, scopes)
tu, err := validator.IntrospectToken(ctx, token, tokenUse, ar, scopes)
if err == nil {
found = true
foundTokenType = tt
foundTokenUse = tu
} else if errors.Is(err, ErrUnknownRequest) {
// do nothing
} else {
Expand All @@ -74,5 +74,5 @@ func (f *Fosite) IntrospectToken(ctx context.Context, token string, tokenType To
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, ar, nil
}
10 changes: 5 additions & 5 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(""), 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(""), 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(""), nil)
},
},
{
Expand All @@ -114,7 +114,7 @@ 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(""), nil)
},
},
} {
Expand Down
29 changes: 20 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,29 @@ 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, 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()))
}
accessTokenType := ""

if tu == AccessToken {
accessTokenType = BearerAccessToken
}

return &IntrospectionResponse{
Active: true,
AccessRequester: ar,
TokenType: tt,
TokenUse: tu,
AccessTokenType: accessTokenType,
}, 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 string `json:"token_type,omitempty"`
}

func (r *IntrospectionResponse) IsActive() bool {
Expand All @@ -184,6 +191,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() string {
return r.AccessTokenType
}
63 changes: 57 additions & 6 deletions introspection_request_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,57 @@ import (
"github.com/ory/fosite/storage"
)

func TestIntrospectionResponseTokenUse(t *testing.T) {
ctrl := gomock.NewController(t)
validator := internal.NewMockTokenIntrospector(ctrl)
defer ctrl.Finish()

f := compose.ComposeAllEnabled(new(compose.Config), storage.NewExampleStore(), []byte{}, nil).(*Fosite)
httpreq := &http.Request{
Method: "POST",
Header: http.Header{
"Authorization": []string{"bearer some-token"},
},
PostForm: url.Values{
"token": []string{"introspect-token"},
},
}
for k, c := range []struct {
description string
setup func()
expectedTU TokenUse
expectedATT string
}{
{
description: "introspecting access token",
setup: func() {
f.TokenIntrospectionHandlers = TokenIntrospectionHandlers{validator}
validator.EXPECT().IntrospectToken(context.TODO(), "some-token", gomock.Any(), gomock.Any(), gomock.Any()).Return(TokenUse(""), nil)
validator.EXPECT().IntrospectToken(context.TODO(), "introspect-token", gomock.Any(), gomock.Any(), gomock.Any()).Return(AccessToken, nil)
},
expectedATT: BearerAccessToken,
expectedTU: AccessToken,
},
{
description: "introspecting refresh token",
setup: func() {
f.TokenIntrospectionHandlers = TokenIntrospectionHandlers{validator}
validator.EXPECT().IntrospectToken(context.TODO(), "some-token", gomock.Any(), gomock.Any(), gomock.Any()).Return(TokenUse(""), nil)
validator.EXPECT().IntrospectToken(context.TODO(), "introspect-token", gomock.Any(), gomock.Any(), gomock.Any()).Return(RefreshToken, nil)
},
expectedATT: "",
expectedTU: RefreshToken,
},
} {
t.Run(fmt.Sprintf("case=%d", k), func(t *testing.T) {
c.setup()
res, err := f.NewIntrospectionRequest(context.TODO(), httpreq, &DefaultSession{})
require.NoError(t, err)
assert.Equal(t, c.expectedATT, res.GetAccessTokenType())
assert.Equal(t, c.expectedTU, res.GetTokenUse())
})
}
}
func TestIntrospectionResponse(t *testing.T) {
r := &fosite.IntrospectionResponse{
AccessRequester: fosite.NewAccessRequest(nil),
Expand Down Expand Up @@ -88,8 +139,8 @@ func TestNewIntrospectionRequest(t *testing.T) {
"token": []string{"introspect-token"},
},
}
validator.EXPECT().IntrospectToken(context.TODO(), "some-token", gomock.Any(), gomock.Any(), gomock.Any()).Return(TokenType(""), nil)
validator.EXPECT().IntrospectToken(context.TODO(), "introspect-token", gomock.Any(), gomock.Any(), gomock.Any()).Return(TokenType(""), newErr)
validator.EXPECT().IntrospectToken(context.TODO(), "some-token", gomock.Any(), gomock.Any(), gomock.Any()).Return(TokenUse(""), nil)
validator.EXPECT().IntrospectToken(context.TODO(), "introspect-token", gomock.Any(), gomock.Any(), gomock.Any()).Return(TokenUse(""), newErr)
},
isActive: false,
expectErr: ErrInactiveToken,
Expand All @@ -107,8 +158,8 @@ func TestNewIntrospectionRequest(t *testing.T) {
"token": []string{"introspect-token"},
},
}
validator.EXPECT().IntrospectToken(context.TODO(), "some-token", gomock.Any(), gomock.Any(), gomock.Any()).Return(TokenType(""), nil)
validator.EXPECT().IntrospectToken(context.TODO(), "introspect-token", gomock.Any(), gomock.Any(), gomock.Any()).Return(TokenType(""), nil)
validator.EXPECT().IntrospectToken(context.TODO(), "some-token", gomock.Any(), gomock.Any(), gomock.Any()).Return(TokenUse(""), nil)
validator.EXPECT().IntrospectToken(context.TODO(), "introspect-token", gomock.Any(), gomock.Any(), gomock.Any()).Return(TokenUse(""), nil)
},
isActive: true,
},
Expand All @@ -126,7 +177,7 @@ func TestNewIntrospectionRequest(t *testing.T) {
"token": []string{"introspect-token"},
},
}
validator.EXPECT().IntrospectToken(context.TODO(), "introspect-token", gomock.Any(), gomock.Any(), gomock.Any()).Return(TokenType(""), nil)
validator.EXPECT().IntrospectToken(context.TODO(), "introspect-token", gomock.Any(), gomock.Any(), gomock.Any()).Return(TokenUse(""), nil)
},
isActive: true,
},
Expand All @@ -144,7 +195,7 @@ func TestNewIntrospectionRequest(t *testing.T) {
"token": []string{"introspect-token"},
},
}
validator.EXPECT().IntrospectToken(context.TODO(), "introspect-token", gomock.Any(), gomock.Any(), gomock.Any()).Return(TokenType(""), nil)
validator.EXPECT().IntrospectToken(context.TODO(), "introspect-token", gomock.Any(), gomock.Any(), gomock.Any()).Return(TokenUse(""), nil)
},
isActive: true,
},
Expand Down
12 changes: 6 additions & 6 deletions introspection_response_writer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,9 @@ func TestWriteIntrospectionResponseBody(t *testing.T) {
description: "should success for not expired access token",
setup: func() {
ires.Active = true
ires.TokenType = AccessToken
ires.TokenUse = AccessToken
sess := &DefaultSession{}
sess.SetExpiresAt(ires.TokenType, time.Now().Add(time.Hour*2))
sess.SetExpiresAt(ires.TokenUse, time.Now().Add(time.Hour*2))
ires.AccessRequester = NewAccessRequest(sess)
},
active: true,
Expand All @@ -94,9 +94,9 @@ func TestWriteIntrospectionResponseBody(t *testing.T) {
description: "should success for expired access token",
setup: func() {
ires.Active = false
ires.TokenType = AccessToken
ires.TokenUse = AccessToken
sess := &DefaultSession{}
sess.SetExpiresAt(ires.TokenType, time.Now().Add(-time.Hour*2))
sess.SetExpiresAt(ires.TokenUse, time.Now().Add(-time.Hour*2))
ires.AccessRequester = NewAccessRequest(sess)
},
active: false,
Expand All @@ -106,9 +106,9 @@ func TestWriteIntrospectionResponseBody(t *testing.T) {
description: "should success for ExpiresAt not set access token",
setup: func() {
ires.Active = true
ires.TokenType = AccessToken
ires.TokenUse = AccessToken
sess := &DefaultSession{}
sess.SetExpiresAt(ires.TokenType, time.Time{})
sess.SetExpiresAt(ires.TokenUse, time.Time{})
ires.AccessRequester = NewAccessRequest(sess)
},
active: true,
Expand Down
15 changes: 12 additions & 3 deletions oauth2.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,18 @@ import (
"time"
)

type TokenUse = TokenType

//type TokenUse string
aeneasr marked this conversation as resolved.
Show resolved Hide resolved
type TokenType string

const (
AccessToken TokenType = "access_token"
RefreshToken TokenType = "refresh_token"
AuthorizeCode TokenType = "authorize_code"
IDToken TokenType = "id_token"

BearerAccessToken string = "bearer"
)

// OAuth2Provider is an interface that enables you to write OAuth2 handlers with only a few lines of code.
Expand Down Expand Up @@ -143,7 +148,7 @@ type OAuth2Provider interface {

// IntrospectToken returns token metadata, if the token is valid. Tokens generated by the authorization endpoint,
// such as the authorization code, can not be introspected.
IntrospectToken(ctx context.Context, token string, tokenType TokenType, session Session, scope ...string) (TokenType, AccessRequester, error)
IntrospectToken(ctx context.Context, token string, tokenUse TokenUse, session Session, scope ...string) (TokenUse, AccessRequester, error)

// NewIntrospectionRequest initiates token introspection as defined in
// https://tools.ietf.org/search/rfc7662#section-2.1
Expand All @@ -168,9 +173,13 @@ type IntrospectionResponder interface {
// AccessRequester returns nil when IsActive() is false and the original access request object otherwise.
GetAccessRequester() AccessRequester

// GetTokenType optionally returns the type of the token that was introspected. The could be "access_token", "refresh_token",
// GetTokenUse optionally returns the type of the token that was introspected. This could be "access_token", "refresh_token",
// or if the type can not be determined an empty string.
GetTokenType() TokenType
GetTokenUse() TokenUse

//GetAccessTokenType optionally returns the type of the access token that was introspected. This could be "bearer", "mac",
// or empty string if the type of the token is refresh token.
GetAccessTokenType() string
}

// Requester is an abstract interface for handling requests in Fosite.
Expand Down