diff --git a/.travis.yml b/.travis.yml index 1adc12955..47e045fb2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -14,5 +14,9 @@ install: - glide install script: - - gotestcover -coverprofile="cover.out" -race -covermode="count" $(glide novendor) - - goveralls -coverprofile="cover.out" + - touch ./coverage.tmp + - | + echo 'mode: atomic' > coverage.txt + - | + go list ./... | grep -v /vendor | grep -v /internal | xargs -n1 -I{} sh -c 'go test -race -covermode=atomic -coverprofile=coverage.tmp -coverpkg $(go list ./... | grep -v /vendor | grep -v /internal | tr "\n" ",") {} && tail -n +2 coverage.tmp >> coverage.txt || exit 255' && rm coverage.tmp + - goveralls -coverprofile="coverage.txt" diff --git a/compose/compose.go b/compose/compose.go index a07be6720..dd9de6084 100644 --- a/compose/compose.go +++ b/compose/compose.go @@ -83,6 +83,7 @@ func ComposeAllEnabled(config *Config, storage interface{}, secret []byte, key * OpenIDConnectExplicitFactory, OpenIDConnectImplicitFactory, OpenIDConnectHybridFactory, + OpenIDConnectRefreshFactory, OAuth2TokenIntrospectionFactory, ) diff --git a/compose/compose_openid.go b/compose/compose_openid.go index 370b3778e..54a327d2e 100644 --- a/compose/compose_openid.go +++ b/compose/compose_openid.go @@ -5,8 +5,9 @@ import ( "github.com/ory/fosite/handler/openid" ) -// OpenIDConnectExplicitFactory creates an OpenID Connect explicit ("authorize code flow") grant handler. You must add this handler -// *after* you have added an OAuth2 authorize code handler! +// OpenIDConnectExplicitFactory creates an OpenID Connect explicit ("authorize code flow") grant handler. +// +// **Important note:** You must add this handler *after* you have added an OAuth2 authorize code handler! func OpenIDConnectExplicitFactory(config *Config, storage interface{}, strategy interface{}) interface{} { return &openid.OpenIDConnectExplicitHandler{ OpenIDConnectRequestStorage: storage.(openid.OpenIDConnectRequestStorage), @@ -16,8 +17,20 @@ func OpenIDConnectExplicitFactory(config *Config, storage interface{}, strategy } } -// OpenIDConnectImplicitFactory creates an OpenID Connect implicit ("implicit flow") grant handler. You must add this handler -// *after* you have added an OAuth2 authorize implicit handler! +// OpenIDConnectRefreshFactory creates a handler for refreshing openid connect tokens. +// +// **Important note:** You must add this handler *after* you have added an OAuth2 authorize code handler! +func OpenIDConnectRefreshFactory(config *Config, storage interface{}, strategy interface{}) interface{} { + return &openid.OpenIDConnectRefreshHandler{ + IDTokenHandleHelper: &openid.IDTokenHandleHelper{ + IDTokenStrategy: strategy.(openid.OpenIDConnectTokenStrategy), + }, + } +} + +// OpenIDConnectImplicitFactory creates an OpenID Connect implicit ("implicit flow") grant handler. +// +// **Important note:** You must add this handler *after* you have added an OAuth2 authorize code handler! func OpenIDConnectImplicitFactory(config *Config, storage interface{}, strategy interface{}) interface{} { return &openid.OpenIDConnectImplicitHandler{ AuthorizeImplicitGrantTypeHandler: &oauth2.AuthorizeImplicitGrantTypeHandler{ @@ -32,8 +45,9 @@ func OpenIDConnectImplicitFactory(config *Config, storage interface{}, strategy } } -// OpenIDConnectHybridFactory creates an OpenID Connect hybrid grant handler. You must add this handler -// *after* you have added an OAuth2 authorize code and implicit authorize handler! +// OpenIDConnectHybridFactory creates an OpenID Connect hybrid grant handler. +// +// **Important note:** You must add this handler *after* you have added an OAuth2 authorize code handler! func OpenIDConnectHybridFactory(config *Config, storage interface{}, strategy interface{}) interface{} { return &openid.OpenIDConnectHybridHandler{ AuthorizeExplicitGrantHandler: &oauth2.AuthorizeExplicitGrantHandler{ diff --git a/handler/openid/flow_refresh_token.go b/handler/openid/flow_refresh_token.go new file mode 100644 index 000000000..7ad745f84 --- /dev/null +++ b/handler/openid/flow_refresh_token.go @@ -0,0 +1,44 @@ +package openid + +import ( + "context" + + "github.com/ory/fosite" + "github.com/pkg/errors" + "time" +) + +type OpenIDConnectRefreshHandler struct { + *IDTokenHandleHelper +} + +func (c *OpenIDConnectRefreshHandler) HandleTokenEndpointRequest(ctx context.Context, request fosite.AccessRequester) error { + if !request.GetGrantTypes().Exact("refresh_token") { + return errors.WithStack(fosite.ErrUnknownRequest) + } + + if !request.GetGrantedScopes().Has("openid") { + return errors.WithStack(fosite.ErrUnknownRequest) + } + + sess, ok := request.GetSession().(Session) + if !ok { + return errors.New("Failed to generate id token because session must be of type fosite/handler/openid.Session") + } + + // We need to reset the expires at value + sess.IDTokenClaims().ExpiresAt = time.Time{} + return nil +} + +func (c *OpenIDConnectRefreshHandler) PopulateTokenEndpointResponse(ctx context.Context, requester fosite.AccessRequester, responder fosite.AccessResponder) error { + if !requester.GetGrantTypes().Exact("refresh_token") { + return errors.WithStack(fosite.ErrUnknownRequest) + } + + if !requester.GetGrantedScopes().Has("openid") { + return errors.WithStack(fosite.ErrUnknownRequest) + } + + return c.IssueExplicitIDToken(ctx, requester, responder) +} diff --git a/handler/openid/strategy_jwt.go b/handler/openid/strategy_jwt.go index ac3032690..7a53b272a 100644 --- a/handler/openid/strategy_jwt.go +++ b/handler/openid/strategy_jwt.go @@ -8,6 +8,7 @@ import ( "github.com/ory/fosite/token/jwt" "github.com/pkg/errors" "github.com/mohae/deepcopy" + "github.com/pborman/uuid" ) const defaultExpiryTime = time.Hour @@ -104,16 +105,16 @@ func (h DefaultStrategy) GenerateIDToken(_ context.Context, requester fosite.Req sess, ok := requester.GetSession().(Session) if !ok { - return "", errors.New("Session must be of type strategy.Session") + return "", errors.New("Failed to generate id token because session must be of type fosite/handler/openid.Session") } claims := sess.IDTokenClaims() if requester.GetRequestForm().Get("max_age") != "" && (claims.AuthTime.IsZero() || claims.AuthTime.After(time.Now())) { - return "", errors.New("Authentication time claim is required when max_age is set and can not be in the future") + return "", errors.New("Failed to generate id token because authentication time claim is required when max_age is set and can not be in the future") } if claims.Subject == "" { - return "", errors.New("Subject claim can not be empty") + return "", errors.New("Failed to generate id token because subject is an empty string") } if claims.ExpiresAt.IsZero() { @@ -121,7 +122,7 @@ func (h DefaultStrategy) GenerateIDToken(_ context.Context, requester fosite.Req } if claims.ExpiresAt.Before(time.Now()) { - return "", errors.New("Expiry claim can not be in the past") + return "", errors.New("Failed to generate id token because expiry claim can not be in the past") } if claims.AuthTime.IsZero() { @@ -135,7 +136,8 @@ func (h DefaultStrategy) GenerateIDToken(_ context.Context, requester fosite.Req nonce := requester.GetRequestForm().Get("nonce") // OPTIONAL. String value used to associate a Client session with an ID Token, and to mitigate replay attacks. if len(nonce) == 0 { - // skip this check, no nonce provided + // skip this check, no nonce provided, let's use a random one. + nonce = uuid.New() } else if len(nonce) < fosite.MinParameterEntropy { // We're assuming that using less then 8 characters for the state can not be considered "unguessable" return "", errors.WithStack(fosite.ErrInsufficientEntropy) diff --git a/integration/placeholder.go b/integration/placeholder.go new file mode 100644 index 000000000..76ab1b728 --- /dev/null +++ b/integration/placeholder.go @@ -0,0 +1 @@ +package integration diff --git a/integration/refresh_token_grant_test.go b/integration/refresh_token_grant_test.go index 3e08eb2ce..5b5f80cef 100644 --- a/integration/refresh_token_grant_test.go +++ b/integration/refresh_token_grant_test.go @@ -2,86 +2,98 @@ package integration_test import ( "testing" - "net/http" "time" - "github.com/ory/fosite" "github.com/ory/fosite/compose" - hst "github.com/ory/fosite/handler/oauth2" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/oauth2" + "github.com/ory/fosite/internal" + "github.com/ory/fosite/handler/openid" + "github.com/ory/fosite/token/jwt" ) func TestRefreshTokenFlow(t *testing.T) { - for _, strategy := range []hst.AccessTokenStrategy{ - hmacStrategy, - } { - runRefreshTokenGrantTest(t, strategy) + session := &defaultSession{ + DefaultSession: &openid.DefaultSession{ + Claims: &jwt.IDTokenClaims{ + Subject: "peter", + }, + Headers: &jwt.Headers{}, + }, } -} - -func runRefreshTokenGrantTest(t *testing.T, strategy interface{}) { - f := compose.Compose( - new(compose.Config), - fositeStore, - strategy, - nil, - compose.OAuth2AuthorizeExplicitFactory, - compose.OAuth2RefreshTokenGrantFactory, - compose.OAuth2TokenIntrospectionFactory, - ) - ts := mockServer(t, f, &fosite.DefaultSession{}) + f := compose.ComposeAllEnabled(new(compose.Config), fositeStore, []byte("some-secret-thats-random"), internal.MustRSAKey()) + ts := mockServer(t, f, session) defer ts.Close() oauthClient := newOAuth2Client(ts) state := "1234567890" fositeStore.Clients["my-client"].RedirectURIs[0] = ts.URL + "/callback" - for k, c := range []struct { + for _, c := range []struct { description string setup func() pass bool + check func(original, refreshed *oauth2.Token) }{ { - description: "should fail because scope missing", - setup: func() {}, + description: "should fail because refresh scope missing", + setup: func() { + oauthClient.Scopes = []string{"fosite"} + }, pass: false, }, { - description: "should pass", + description: "should pass but not yield id token", + setup: func() { + oauthClient.Scopes = []string{"offline"} + }, + pass: true, + check: func(original, refreshed *oauth2.Token) { + assert.NotEqual(t, original.RefreshToken, refreshed.RefreshToken) + assert.NotEqual(t, original.AccessToken, refreshed.AccessToken) + assert.Nil(t, refreshed.Extra("id_token")) + }, + }, + { + description: "should pass and yield id token", setup: func() { - oauthClient.Scopes = []string{"fosite", "offline"} + oauthClient.Scopes = []string{"fosite", "offline", "openid"} }, pass: true, + check: func(original, refreshed *oauth2.Token) { + assert.NotEqual(t, original.RefreshToken, refreshed.RefreshToken) + assert.NotEqual(t, original.AccessToken, refreshed.AccessToken) + assert.NotNil(t, refreshed.Extra("id_token")) + }, }, } { - c.setup() + t.Run("case="+c.description, func(t *testing.T) { + c.setup() - resp, err := http.Get(oauthClient.AuthCodeURL(state)) - require.Nil(t, err) - require.Equal(t, http.StatusOK, resp.StatusCode, "(%d) %s", k, c.description) + resp, err := http.Get(oauthClient.AuthCodeURL(state)) + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.StatusCode) + + if resp.StatusCode != http.StatusOK { + return + } - if resp.StatusCode == http.StatusOK { token, err := oauthClient.Exchange(oauth2.NoContext, resp.Request.URL.Query().Get("code")) - require.Nil(t, err, "(%d) %s", k, c.description) - require.NotEmpty(t, token.AccessToken, "(%d) %s", k, c.description) + require.NoError(t, err) + require.NotEmpty(t, token.AccessToken) t.Logf("Token %s\n", token) token.Expiry = token.Expiry.Add(-time.Hour * 24) - t.Logf("Token %s\n", token) tokenSource := oauthClient.TokenSource(oauth2.NoContext, token) refreshed, err := tokenSource.Token() if c.pass { - require.Nil(t, err, "(%d) %s: %s", k, c.description, err) - assert.NotEqual(t, token.RefreshToken, refreshed.RefreshToken, "(%d) %s", k, c.description) - assert.NotEqual(t, token.AccessToken, refreshed.AccessToken, "(%d) %s", k, c.description) + require.NoError(t, err) + c.check(token, refreshed) } else { - require.NotNil(t, err, "(%d) %s: %s", k, c.description, err) - + require.NotNil(t, err) } - } - t.Logf("Passed test case (%d) %s", k, c.description) + }) } }