From 6ad92642f0f01ff4d3662f3680a825db22594366 Mon Sep 17 00:00:00 2001
From: Amnay <amnay.m@gmail.com>
Date: Sun, 25 Apr 2021 20:01:10 +0200
Subject: [PATCH] feat: allow omitting scope in authorization redirect uri
 (#588)

---
 handler/oauth2/flow_authorize_code_auth.go    |  8 +++-
 .../oauth2/flow_authorize_code_auth_test.go   | 46 ++++++++++++++++++-
 2 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/handler/oauth2/flow_authorize_code_auth.go b/handler/oauth2/flow_authorize_code_auth.go
index b63fa3989..62c4ba51f 100644
--- a/handler/oauth2/flow_authorize_code_auth.go
+++ b/handler/oauth2/flow_authorize_code_auth.go
@@ -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 {
@@ -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
diff --git a/handler/oauth2/flow_authorize_code_auth_test.go b/handler/oauth2/flow_authorize_code_auth_test.go
index 5c9c20620..261d68c0a 100644
--- a/handler/oauth2/flow_authorize_code_auth_test.go
+++ b/handler/oauth2/flow_authorize_code_auth_test.go
@@ -45,19 +45,21 @@ 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(),
@@ -65,6 +67,7 @@ func TestAuthorizeCode_HandleAuthorizeEndpointRequest(t *testing.T) {
 					description: "should pass because not responsible for handling an empty response type",
 				},
 				{
+					handler: handler,
 					areq: &fosite.AuthorizeRequest{
 						ResponseTypes: fosite.Arguments{"foo"},
 						Request:       *fosite.NewRequest(),
@@ -72,6 +75,7 @@ func TestAuthorizeCode_HandleAuthorizeEndpointRequest(t *testing.T) {
 					description: "should pass because not responsible for handling an invalid response type",
 				},
 				{
+					handler: handler,
 					areq: &fosite.AuthorizeRequest{
 						ResponseTypes: fosite.Arguments{"code"},
 						Request: fosite.Request{
@@ -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{
@@ -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{
@@ -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 {