Skip to content

Commit

Permalink
feat: allow omitting scope in authorization redirect uri (#588)
Browse files Browse the repository at this point in the history
  • Loading branch information
amnay-mo authored Apr 25, 2021
1 parent 1950b7d commit 6ad9264
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 3 deletions.
8 changes: 7 additions & 1 deletion handler/oauth2/flow_authorize_code_auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ type AuthorizeExplicitGrantHandler struct {
IsRedirectURISecure func(*url.URL) bool

RefreshTokenScopes []string

// OmitRedirectScopeParam must be set to true if the scope query param is to be omitted
// in the authorization's redirect URI
OmitRedirectScopeParam bool
}

func (c *AuthorizeExplicitGrantHandler) secureChecker() func(*url.URL) bool {
Expand Down Expand Up @@ -115,7 +119,9 @@ func (c *AuthorizeExplicitGrantHandler) IssueAuthorizeCode(ctx context.Context,

resp.AddParameter("code", code)
resp.AddParameter("state", ar.GetState())
resp.AddParameter("scope", strings.Join(ar.GetGrantedScopes(), " "))
if !c.OmitRedirectScopeParam {
resp.AddParameter("scope", strings.Join(ar.GetGrantedScopes(), " "))
}

ar.SetResponseTypeHandled("code")
return nil
Expand Down
46 changes: 44 additions & 2 deletions handler/oauth2/flow_authorize_code_auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,33 +45,37 @@ func TestAuthorizeCode_HandleAuthorizeEndpointRequest(t *testing.T) {
} {
t.Run("strategy="+k, func(t *testing.T) {
store := storage.NewMemoryStore()
h := AuthorizeExplicitGrantHandler{
handler := AuthorizeExplicitGrantHandler{
CoreStorage: store,
AuthorizeCodeStrategy: strategy,
ScopeStrategy: fosite.HierarchicScopeStrategy,
AudienceMatchingStrategy: fosite.DefaultAudienceMatchingStrategy,
}
for _, c := range []struct {
handler AuthorizeExplicitGrantHandler
areq *fosite.AuthorizeRequest
description string
expectErr error
expect func(t *testing.T, areq *fosite.AuthorizeRequest, aresp *fosite.AuthorizeResponse)
}{
{
handler: handler,
areq: &fosite.AuthorizeRequest{
ResponseTypes: fosite.Arguments{""},
Request: *fosite.NewRequest(),
},
description: "should pass because not responsible for handling an empty response type",
},
{
handler: handler,
areq: &fosite.AuthorizeRequest{
ResponseTypes: fosite.Arguments{"foo"},
Request: *fosite.NewRequest(),
},
description: "should pass because not responsible for handling an invalid response type",
},
{
handler: handler,
areq: &fosite.AuthorizeRequest{
ResponseTypes: fosite.Arguments{"code"},
Request: fosite.Request{
Expand All @@ -86,6 +90,7 @@ func TestAuthorizeCode_HandleAuthorizeEndpointRequest(t *testing.T) {
expectErr: fosite.ErrInvalidRequest,
},
{
handler: handler,
areq: &fosite.AuthorizeRequest{
ResponseTypes: fosite.Arguments{"code"},
Request: fosite.Request{
Expand All @@ -102,6 +107,7 @@ func TestAuthorizeCode_HandleAuthorizeEndpointRequest(t *testing.T) {
expectErr: fosite.ErrInvalidRequest,
},
{
handler: handler,
areq: &fosite.AuthorizeRequest{
ResponseTypes: fosite.Arguments{"code"},
Request: fosite.Request{
Expand Down Expand Up @@ -130,10 +136,46 @@ func TestAuthorizeCode_HandleAuthorizeEndpointRequest(t *testing.T) {
assert.Equal(t, fosite.ResponseModeQuery, areq.GetResponseMode())
},
},
{
handler: AuthorizeExplicitGrantHandler{
CoreStorage: store,
AuthorizeCodeStrategy: strategy,
ScopeStrategy: fosite.HierarchicScopeStrategy,
AudienceMatchingStrategy: fosite.DefaultAudienceMatchingStrategy,
OmitRedirectScopeParam: true,
},
areq: &fosite.AuthorizeRequest{
ResponseTypes: fosite.Arguments{"code"},
Request: fosite.Request{
Client: &fosite.DefaultClient{
ResponseTypes: fosite.Arguments{"code"},
RedirectURIs: []string{"https://asdf.de/cb"},
Audience: []string{"https://www.ory.sh/api"},
},
RequestedAudience: []string{"https://www.ory.sh/api"},
GrantedScope: fosite.Arguments{"a", "b"},
Session: &fosite.DefaultSession{
ExpiresAt: map[fosite.TokenType]time.Time{fosite.AccessToken: time.Now().UTC().Add(time.Hour)},
},
RequestedAt: time.Now().UTC(),
},
State: "superstate",
RedirectURI: parseUrl("https://asdf.de/cb"),
},
description: "should pass but no scope in redirect uri",
expect: func(t *testing.T, areq *fosite.AuthorizeRequest, aresp *fosite.AuthorizeResponse) {
code := aresp.GetParameters().Get("code")
assert.NotEmpty(t, code)

assert.Empty(t, aresp.GetParameters().Get("scope"))
assert.Equal(t, areq.State, aresp.GetParameters().Get("state"))
assert.Equal(t, fosite.ResponseModeQuery, areq.GetResponseMode())
},
},
} {
t.Run("case="+c.description, func(t *testing.T) {
aresp := fosite.NewAuthorizeResponse()
err := h.HandleAuthorizeEndpointRequest(nil, c.areq, aresp)
err := c.handler.HandleAuthorizeEndpointRequest(nil, c.areq, aresp)
if c.expectErr != nil {
require.EqualError(t, err, c.expectErr.Error())
} else {
Expand Down

0 comments on commit 6ad9264

Please sign in to comment.