allow code grant without secret if code verifier is defined

Author: jarlah

manage/manager.go

@@ -218,7 +218,7 @@ func (m *Manager) GenerateAuthToken(ctx context.Context, rt oauth2.ResponseType,
 	}
 	return ti, nil
 }
- 
+
 // get authorization code data
 func (m *Manager) getAuthorizationCode(ctx context.Context, code string) (oauth2.TokenInfo, error) {
 	ti, err := m.tokenStore.GetByCode(ctx, code)
@@ -288,7 +288,11 @@ func (m *Manager) GenerateAccessToken(ctx context.Context, gt oauth2.GrantType,
 			return nil, errors.ErrInvalidClient
 		}
 	} else if len(cli.GetSecret()) > 0 && tgr.ClientSecret != cli.GetSecret() {
-		return nil, errors.ErrInvalidClient
+		// auth code flow doesnt require client_secret if used with PKCE and state parameter
+		// this is especially useful for mobile apps, that cant hold the secret
+		if !(gt == oauth2.AuthorizationCode && tgr.ClientSecret == "" && tgr.CodeVerifier != "") {
+			return nil, errors.ErrInvalidClient
+		}
 	}
 	if tgr.RedirectURI != "" {
 		if err := m.validateURI(cli.GetDomain(), tgr.RedirectURI); err != nil {

server/server_test.go

@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"net/http"
 	"net/http/httptest"
-	"net/url"
 	"testing"
 
 	"github.com/gavv/httpexpect"
@@ -26,9 +25,10 @@ var (
 	clientSecret = "11111111"
 
 	plainChallenge = "ThisIsAFourtyThreeCharactersLongStringThing"
-	s256Challenge  = "s256test"
-	// echo s256test | sha256 | base64 | tr '/+' '_-'
-	s256ChallengeHash = "W6YWc_4yHwYN-cGDgGmOMHF3l7KDy7VcRjf7q2FVF-o="
+	s256Challenge  = "s256tests256tests256tests256tests256tests256test"
+	// sha2562 := sha256.Sum256([]byte(s256Challenge))
+	// fmt.Printf(base64.URLEncoding.EncodeToString(sha2562[:]))
+	s256ChallengeHash = "To2Xqv01cm16bC9Sf7KRRS8CO2SFss_HSMQOr3sdCDE="
 )
 
 func init() {
@@ -107,7 +107,7 @@ func TestAuthorizeCode(t *testing.T) {
 		WithQuery("client_id", clientID).
 		WithQuery("scope", "all").
 		WithQuery("state", "123").
-		WithQuery("redirect_uri", url.QueryEscape(csrv.URL+"/oauth2")).
+		WithQuery("redirect_uri", csrv.URL+"/oauth2").
 		Expect().Status(http.StatusOK)
 }
 
@@ -134,7 +134,7 @@ func TestAuthorizeCodeWithChallengePlain(t *testing.T) {
 				WithFormField("grant_type", "authorization_code").
 				WithFormField("client_id", clientID).
 				WithFormField("code", code).
-				WithBasicAuth("code_verifier", "testchallenge").
+				WithFormField("code_verifier", plainChallenge).
 				Expect().
 				Status(http.StatusOK).
 				JSON().Object()
@@ -152,13 +152,14 @@ func TestAuthorizeCodeWithChallengePlain(t *testing.T) {
 		userID = "000000"
 		return
 	})
+	srv.SetClientInfoHandler(server.ClientFormHandler)
 
 	e.GET("/authorize").
 		WithQuery("response_type", "code").
 		WithQuery("client_id", clientID).
 		WithQuery("scope", "all").
 		WithQuery("state", "123").
-		WithQuery("redirect_uri", url.QueryEscape(csrv.URL+"/oauth2")).
+		WithQuery("redirect_uri", csrv.URL+"/oauth2").
 		WithQuery("code_challenge", plainChallenge).
 		Expect().Status(http.StatusOK)
 }
@@ -186,7 +187,7 @@ func TestAuthorizeCodeWithChallengeS256(t *testing.T) {
 				WithFormField("grant_type", "authorization_code").
 				WithFormField("client_id", clientID).
 				WithFormField("code", code).
-				WithBasicAuth("code_verifier", s256Challenge).
+				WithFormField("code_verifier", s256Challenge).
 				Expect().
 				Status(http.StatusOK).
 				JSON().Object()
@@ -204,13 +205,14 @@ func TestAuthorizeCodeWithChallengeS256(t *testing.T) {
 		userID = "000000"
 		return
 	})
+	srv.SetClientInfoHandler(server.ClientFormHandler)
 
 	e.GET("/authorize").
 		WithQuery("response_type", "code").
 		WithQuery("client_id", clientID).
 		WithQuery("scope", "all").
 		WithQuery("state", "123").
-		WithQuery("redirect_uri", url.QueryEscape(csrv.URL+"/oauth2")).
+		WithQuery("redirect_uri", csrv.URL+"/oauth2").
 		WithQuery("code_challenge", s256ChallengeHash).
 		WithQuery("code_challenge_method", "S256").
 		Expect().Status(http.StatusOK)
@@ -238,7 +240,7 @@ func TestImplicit(t *testing.T) {
 		WithQuery("client_id", clientID).
 		WithQuery("scope", "all").
 		WithQuery("state", "123").
-		WithQuery("redirect_uri", url.QueryEscape(csrv.URL+"/oauth2")).
+		WithQuery("redirect_uri", csrv.URL+"/oauth2").
 		Expect().Status(http.StatusOK)
 }
 
@@ -384,7 +386,7 @@ func TestRefreshing(t *testing.T) {
 		WithQuery("client_id", clientID).
 		WithQuery("scope", "all").
 		WithQuery("state", "123").
-		WithQuery("redirect_uri", url.QueryEscape(csrv.URL+"/oauth2")).
+		WithQuery("redirect_uri", csrv.URL+"/oauth2").
 		Expect().Status(http.StatusOK)
 }