Skip to content

Commit

Permalink
Add RefreshTokenScopes Config (#371)
Browse files Browse the repository at this point in the history 
When set to true, this will return refresh tokens even if the user did
not ask for the offline or offline_access Oauth Scope.
  • Loading branch information
@dmcinnes @aeneasr
dmcinnes authored and aeneasr committed Sep 16, 2019
1 parent cc34bfb commit bcc7859
Show file tree
Hide file tree
Showing 10 changed files with 202 additions and 36 deletions.
2 changes: 1 addition & 1 deletion README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ var config = &compose.Config{
}
```

**Note:** To issue refresh tokens with any of the grants, you need to include the `offline` scope in the OAuth2 request.
**Note:** To issue refresh tokens with any of the grants, you need to include the `offline` scope in the OAuth2 request. This can be modified by the `RefreshTokenScopes` compose configuration. When set to an empty array, _all_ grants will issue refresh tokens.

#### `fosite.WildcardScopeStrategy`

Expand Down
3 changes: 3 additions & 0 deletions compose/compose_oauth2.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ func OAuth2AuthorizeExplicitFactory(config *Config, storage interface{}, strateg
AudienceMatchingStrategy: config.GetAudienceStrategy(),
TokenRevocationStorage: storage.(oauth2.TokenRevocationStorage),
IsRedirectURISecure: config.GetRedirectSecureChecker(),
RefreshTokenScopes: config.GetRefreshTokenScopes(),
}
}

Expand Down Expand Up @@ -68,6 +69,7 @@ func OAuth2RefreshTokenGrantFactory(config *Config, storage interface{}, strateg
RefreshTokenLifespan: config.GetRefreshTokenLifespan(),
ScopeStrategy: config.GetScopeStrategy(),
AudienceMatchingStrategy: config.GetAudienceStrategy(),
RefreshTokenScopes: config.GetRefreshTokenScopes(),
}
}

Expand Down Expand Up @@ -97,6 +99,7 @@ func OAuth2ResourceOwnerPasswordCredentialsFactory(config *Config, storage inter
RefreshTokenStrategy: strategy.(oauth2.RefreshTokenStrategy),
ScopeStrategy: config.GetScopeStrategy(),
AudienceMatchingStrategy: config.GetAudienceStrategy(),
RefreshTokenScopes: config.GetRefreshTokenScopes(),
}
}

Expand Down
11 changes: 11 additions & 0 deletions compose/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ type Config struct {

// RedirectSecureChecker is a function that returns true if the provided URL can be securely used as a redirect URL.
RedirectSecureChecker func(*url.URL) bool

// RefreshTokenScopes defines which OAuth scopes will be given refresh tokens during the authorization code grant exchange. This defaults to "offline" and "offline_access". When set to an empty array, all exchanges will be given refresh tokens.
RefreshTokenScopes []string
}

// GetScopeStrategy returns the scope strategy to be used. Defaults to glob scope strategy.
Expand Down Expand Up @@ -168,3 +171,11 @@ func (c *Config) GetRedirectSecureChecker() func(*url.URL) bool {
}
return c.RedirectSecureChecker
}

// GetRefreshTokenScopes returns which scopes will provide refresh tokens.
func (c *Config) GetRefreshTokenScopes() []string {
if c.RefreshTokenScopes == nil {
return []string{"offline", "offline_access"}
}
return c.RefreshTokenScopes
}
2 changes: 2 additions & 0 deletions handler/oauth2/flow_authorize_code_auth.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ type AuthorizeExplicitGrantHandler struct {
TokenRevocationStorage TokenRevocationStorage

IsRedirectURISecure func(*url.URL) bool

RefreshTokenScopes []string
}

func (c *AuthorizeExplicitGrantHandler) secureChecker() func(*url.URL) bool {
Expand Down
2 changes: 1 addition & 1 deletion handler/oauth2/flow_authorize_code_token.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ func (c *AuthorizeExplicitGrantHandler) PopulateTokenEndpointResponse(ctx contex
}

var refresh, refreshSignature string
if authorizeRequest.GetGrantedScopes().HasOneOf("offline", "offline_access") {
if len(c.RefreshTokenScopes) == 0 || authorizeRequest.GetGrantedScopes().HasOneOf(c.RefreshTokenScopes...) {
refresh, refreshSignature, err = c.RefreshTokenStrategy.GenerateRefreshToken(ctx, requester)
if err != nil {
return errors.WithStack(fosite.ErrServerError.WithDebug(err.Error()))
Expand Down
84 changes: 73 additions & 11 deletions handler/oauth2/flow_authorize_code_token_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,16 +48,8 @@ func TestAuthorizeCode_PopulateTokenEndpointResponse(t *testing.T) {
t.Run("strategy="+k, func(t *testing.T) {
store := storage.NewMemoryStore()

h := AuthorizeExplicitGrantHandler{
CoreStorage: store,
AuthorizeCodeStrategy: strategy,
AccessTokenStrategy: strategy,
RefreshTokenStrategy: strategy,
ScopeStrategy: fosite.HierarchicScopeStrategy,
AudienceMatchingStrategy: fosite.DefaultAudienceMatchingStrategy,
AccessTokenLifespan: time.Minute,
//TokenRevocationStorage: store,
}
var h AuthorizeExplicitGrantHandler

for _, c := range []struct {
areq *fosite.AccessRequest
description string
Expand Down Expand Up @@ -130,7 +122,7 @@ func TestAuthorizeCode_PopulateTokenEndpointResponse(t *testing.T) {

require.NoError(t, store.CreateAuthorizeCodeSession(nil, sig, areq))
},
description: "should pass",
description: "should pass with offline scope and refresh token",
check: func(t *testing.T, aresp *fosite.AccessResponse) {
assert.NotEmpty(t, aresp.AccessToken)
assert.Equal(t, "bearer", aresp.TokenType)
Expand All @@ -139,8 +131,78 @@ func TestAuthorizeCode_PopulateTokenEndpointResponse(t *testing.T) {
assert.Equal(t, "foo offline", aresp.GetExtra("scope"))
},
},
{
areq: &fosite.AccessRequest{
GrantTypes: fosite.Arguments{"authorization_code"},
Request: fosite.Request{
Form: url.Values{},
Client: &fosite.DefaultClient{
GrantTypes: fosite.Arguments{"authorization_code"},
},
GrantedScope: fosite.Arguments{"foo"},
Session: &fosite.DefaultSession{},
RequestedAt: time.Now().UTC(),
},
},
setup: func(t *testing.T, areq *fosite.AccessRequest) {
h.RefreshTokenScopes = []string{}
code, sig, err := strategy.GenerateAuthorizeCode(nil, nil)
require.NoError(t, err)
areq.Form.Add("code", code)

require.NoError(t, store.CreateAuthorizeCodeSession(nil, sig, areq))
},
description: "should pass with refresh token always provided",
check: func(t *testing.T, aresp *fosite.AccessResponse) {
assert.NotEmpty(t, aresp.AccessToken)
assert.Equal(t, "bearer", aresp.TokenType)
assert.NotEmpty(t, aresp.GetExtra("refresh_token"))
assert.NotEmpty(t, aresp.GetExtra("expires_in"))
assert.Equal(t, "foo", aresp.GetExtra("scope"))
},
},
{
areq: &fosite.AccessRequest{
GrantTypes: fosite.Arguments{"authorization_code"},
Request: fosite.Request{
Form: url.Values{},
Client: &fosite.DefaultClient{
GrantTypes: fosite.Arguments{"authorization_code"},
},
GrantedScope: fosite.Arguments{"foo"},
Session: &fosite.DefaultSession{},
RequestedAt: time.Now().UTC(),
},
},
setup: func(t *testing.T, areq *fosite.AccessRequest) {
code, sig, err := strategy.GenerateAuthorizeCode(nil, nil)
require.NoError(t, err)
areq.Form.Add("code", code)

require.NoError(t, store.CreateAuthorizeCodeSession(nil, sig, areq))
},
description: "should not have refresh token",
check: func(t *testing.T, aresp *fosite.AccessResponse) {
assert.NotEmpty(t, aresp.AccessToken)
assert.Equal(t, "bearer", aresp.TokenType)
assert.Empty(t, aresp.GetExtra("refresh_token"))
assert.NotEmpty(t, aresp.GetExtra("expires_in"))
assert.Equal(t, "foo", aresp.GetExtra("scope"))
},
},
} {
t.Run("case="+c.description, func(t *testing.T) {
h = AuthorizeExplicitGrantHandler{
CoreStorage: store,
AuthorizeCodeStrategy: strategy,
AccessTokenStrategy: strategy,
RefreshTokenStrategy: strategy,
ScopeStrategy: fosite.HierarchicScopeStrategy,
AudienceMatchingStrategy: fosite.DefaultAudienceMatchingStrategy,
AccessTokenLifespan: time.Minute,
RefreshTokenScopes: []string{"offline"},
}

if c.setup != nil {
c.setup(t, c.areq)
}
Expand Down
9 changes: 7 additions & 2 deletions handler/oauth2/flow_refresh.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ package oauth2

import (
"context"
"fmt"
"strings"
"time"

"github.com/ory/fosite/storage"
Expand All @@ -45,6 +47,7 @@ type RefreshTokenGrantHandler struct {

ScopeStrategy fosite.ScopeStrategy
AudienceMatchingStrategy fosite.AudienceMatchingStrategy
RefreshTokenScopes []string
}

// HandleTokenEndpointRequest implements https://tools.ietf.org/html/rfc6749#section-6
Expand Down Expand Up @@ -72,8 +75,10 @@ func (c *RefreshTokenGrantHandler) HandleTokenEndpointRequest(ctx context.Contex
return errors.WithStack(fosite.ErrInvalidRequest.WithDebug(err.Error()))
}

if !originalRequest.GetGrantedScopes().HasOneOf("offline", "offline_access") {
return errors.WithStack(fosite.ErrScopeNotGranted.WithHint("The OAuth 2.0 Client was not granted scope \"offline\" or \"offline_access\" and may thus not perform the \"refresh_token\" authorization grant."))
if !(len(c.RefreshTokenScopes) == 0 || originalRequest.GetGrantedScopes().HasOneOf(c.RefreshTokenScopes...)) {
scopeNames := strings.Join(c.RefreshTokenScopes, " or ")
hint := fmt.Sprintf("The OAuth 2.0 Client was not granted scope %s and may thus not perform the \"refresh_token\" authorization grant.", scopeNames)
return errors.WithStack(fosite.ErrScopeNotGranted.WithHint(hint))

}

Expand Down
80 changes: 72 additions & 8 deletions handler/oauth2/flow_refresh_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,7 @@ func TestRefreshFlow_HandleTokenEndpointRequest(t *testing.T) {
t.Run("strategy="+k, func(t *testing.T) {

store := storage.NewMemoryStore()
h := RefreshTokenGrantHandler{
TokenRevocationStorage: store,
RefreshTokenStrategy: strategy,
AccessTokenLifespan: time.Hour,
RefreshTokenLifespan: time.Hour,
ScopeStrategy: fosite.HierarchicScopeStrategy,
AudienceMatchingStrategy: fosite.DefaultAudienceMatchingStrategy,
}
var h RefreshTokenGrantHandler

for _, c := range []struct {
description string
Expand Down Expand Up @@ -175,8 +168,79 @@ func TestRefreshFlow_HandleTokenEndpointRequest(t *testing.T) {
assert.Equal(t, time.Now().Add(time.Hour).UTC().Round(time.Second), areq.GetSession().GetExpiresAt(fosite.RefreshToken))
},
},
{
description: "should fail without offline scope",
setup: func() {
areq.GrantTypes = fosite.Arguments{"refresh_token"}
areq.Client = &fosite.DefaultClient{
ID: "foo",
GrantTypes: fosite.Arguments{"refresh_token"},
Scopes: []string{"foo", "bar"},
}

token, sig, err := strategy.GenerateRefreshToken(nil, nil)
require.NoError(t, err)

areq.Form.Add("refresh_token", token)
err = store.CreateRefreshTokenSession(nil, sig, &fosite.Request{
Client: areq.Client,
GrantedScope: fosite.Arguments{"foo"},
RequestedScope: fosite.Arguments{"foo", "bar"},
Session: sess,
Form: url.Values{"foo": []string{"bar"}},
RequestedAt: time.Now().UTC().Add(-time.Hour).Round(time.Hour),
})
require.NoError(t, err)
},
expectErr: fosite.ErrScopeNotGranted,
},
{
description: "should pass without offline scope when configured to allow refresh tokens",
setup: func() {
h.RefreshTokenScopes = []string{}
areq.GrantTypes = fosite.Arguments{"refresh_token"}
areq.Client = &fosite.DefaultClient{
ID: "foo",
GrantTypes: fosite.Arguments{"refresh_token"},
Scopes: []string{"foo", "bar"},
}

token, sig, err := strategy.GenerateRefreshToken(nil, nil)
require.NoError(t, err)

areq.Form.Add("refresh_token", token)
err = store.CreateRefreshTokenSession(nil, sig, &fosite.Request{
Client: areq.Client,
GrantedScope: fosite.Arguments{"foo"},
RequestedScope: fosite.Arguments{"foo", "bar"},
Session: sess,
Form: url.Values{"foo": []string{"bar"}},
RequestedAt: time.Now().UTC().Add(-time.Hour).Round(time.Hour),
})
require.NoError(t, err)
},
expect: func(t *testing.T) {
assert.NotEqual(t, sess, areq.Session)
assert.NotEqual(t, time.Now().UTC().Add(-time.Hour).Round(time.Hour), areq.RequestedAt)
assert.Equal(t, fosite.Arguments{"foo"}, areq.GrantedScope)
assert.Equal(t, fosite.Arguments{"foo", "bar"}, areq.RequestedScope)
assert.NotEqual(t, url.Values{"foo": []string{"bar"}}, areq.Form)
assert.Equal(t, time.Now().Add(time.Hour).UTC().Round(time.Second), areq.GetSession().GetExpiresAt(fosite.AccessToken))
assert.Equal(t, time.Now().Add(time.Hour).UTC().Round(time.Second), areq.GetSession().GetExpiresAt(fosite.RefreshToken))
},
},
} {
t.Run("case="+c.description, func(t *testing.T) {
h = RefreshTokenGrantHandler{
TokenRevocationStorage: store,
RefreshTokenStrategy: strategy,
AccessTokenLifespan: time.Hour,
RefreshTokenLifespan: time.Hour,
ScopeStrategy: fosite.HierarchicScopeStrategy,
AudienceMatchingStrategy: fosite.DefaultAudienceMatchingStrategy,
RefreshTokenScopes: []string{"offline"},
}

areq = fosite.NewAccessRequest(&fosite.DefaultSession{})
areq.Form = url.Values{}
c.setup()
Expand Down
3 changes: 2 additions & 1 deletion handler/oauth2/flow_resource_owner.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ type ResourceOwnerPasswordCredentialsGrantHandler struct {
RefreshTokenStrategy RefreshTokenStrategy
ScopeStrategy fosite.ScopeStrategy
AudienceMatchingStrategy fosite.AudienceMatchingStrategy
RefreshTokenScopes []string

*HandleHelper
}
Expand Down Expand Up @@ -92,7 +93,7 @@ func (c *ResourceOwnerPasswordCredentialsGrantHandler) PopulateTokenEndpointResp
}

var refresh, refreshSignature string
if requester.GetGrantedScopes().HasOneOf("offline", "offline_access") {
if len(c.RefreshTokenScopes) == 0 || requester.GetGrantedScopes().HasOneOf(c.RefreshTokenScopes...) {
var err error
refresh, refreshSignature, err = c.RefreshTokenStrategy.GenerateRefreshToken(ctx, requester)
if err != nil {
Expand Down
42 changes: 30 additions & 12 deletions handler/oauth2/flow_resource_owner_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,22 +135,14 @@ func TestResourceOwnerFlow_PopulateTokenEndpointResponse(t *testing.T) {
store := internal.NewMockResourceOwnerPasswordCredentialsGrantStorage(ctrl)
chgen := internal.NewMockAccessTokenStrategy(ctrl)
rtstr := internal.NewMockRefreshTokenStrategy(ctrl)
aresp := fosite.NewAccessResponse()
mockAT := "accesstoken.foo.bar"
mockRT := "refreshtoken.bar.foo"
defer ctrl.Finish()

areq := fosite.NewAccessRequest(nil)
var areq *fosite.AccessRequest
var aresp *fosite.AccessResponse
var h ResourceOwnerPasswordCredentialsGrantHandler

h := ResourceOwnerPasswordCredentialsGrantHandler{
ResourceOwnerPasswordCredentialsGrantStorage: store,
HandleHelper: &HandleHelper{
AccessTokenStorage: store,
AccessTokenStrategy: chgen,
AccessTokenLifespan: time.Hour,
},
RefreshTokenStrategy: rtstr,
}
for k, c := range []struct {
description string
setup func()
Expand All @@ -167,7 +159,6 @@ func TestResourceOwnerFlow_PopulateTokenEndpointResponse(t *testing.T) {
{
description: "should pass",
setup: func() {
areq.Session = &fosite.DefaultSession{}
areq.GrantTypes = fosite.Arguments{"password"}
chgen.EXPECT().GenerateAccessToken(nil, areq).Return(mockAT, "bar", nil)
store.EXPECT().CreateAccessTokenSession(nil, "bar", gomock.Eq(areq.Sanitize([]string{}))).Return(nil)
Expand All @@ -190,8 +181,35 @@ func TestResourceOwnerFlow_PopulateTokenEndpointResponse(t *testing.T) {
assert.NotNil(t, aresp.GetExtra("refresh_token"), "expected refresh token")
},
},
{
description: "should pass - refresh token without offline scope",
setup: func() {
h.RefreshTokenScopes = []string{}
areq.GrantTypes = fosite.Arguments{"password"}
rtstr.EXPECT().GenerateRefreshToken(nil, areq).Return(mockRT, "bar", nil)
store.EXPECT().CreateRefreshTokenSession(nil, "bar", gomock.Eq(areq.Sanitize([]string{}))).Return(nil)
chgen.EXPECT().GenerateAccessToken(nil, areq).Return(mockAT, "bar", nil)
store.EXPECT().CreateAccessTokenSession(nil, "bar", gomock.Eq(areq.Sanitize([]string{}))).Return(nil)
},
expect: func() {
assert.NotNil(t, aresp.GetExtra("refresh_token"), "expected refresh token")
},
},
} {
t.Run(fmt.Sprintf("case=%d", k), func(t *testing.T) {
areq = fosite.NewAccessRequest(nil)
aresp = fosite.NewAccessResponse()
areq.Session = &fosite.DefaultSession{}
h = ResourceOwnerPasswordCredentialsGrantHandler{
ResourceOwnerPasswordCredentialsGrantStorage: store,
HandleHelper: &HandleHelper{
AccessTokenStorage: store,
AccessTokenStrategy: chgen,
AccessTokenLifespan: time.Hour,
},
RefreshTokenStrategy: rtstr,
RefreshTokenScopes: []string{"offline"},
}
c.setup()
err := h.PopulateTokenEndpointResponse(nil, areq, aresp)
if c.expectErr != nil {
Expand Down

0 comments on commit bcc7859

Please sign in to comment.