feat: Add a flag to enforce PKCE only for public clients

compose/compose_pkce.go

@@ -32,6 +32,7 @@ func OAuth2PKCEFactory(config *Config, storage interface{}, strategy interface{}
 		AuthorizeCodeStrategy:      strategy.(oauth2.AuthorizeCodeStrategy),
 		Storage:                    storage.(pkce.PKCERequestStorage),
 		Force:                      config.EnforcePKCE,
+		ForceForPublicClients:      config.EnforcePKCEForPublicClients,
 		EnablePlainChallengeMethod: config.EnablePKCEPlainChallengeMethod,
 	}
 }

compose/config.go

@@ -65,6 +65,9 @@ type Config struct {
 	// EnforcePKCE, if set to true, requires clients to perform authorize code flows with PKCE. Defaults to false.
 	EnforcePKCE bool
 
+	// EnforcePKCEForPublicClients requires only public clients to use PKCE with the authorize code flow. Defaults to false.
+	EnforcePKCEForPublicClients bool
+
 	// EnablePKCEPlainChallengeMethod sets whether or not to allow the plain challenge method (S256 should be used whenever possible, plain is really discouraged). Defaults to false.
 	EnablePKCEPlainChallengeMethod bool
 

handler/pkce/handler.go

@@ -37,6 +37,9 @@ type Handler struct {
 	// If set to true, clients must use PKCE.
 	Force bool
 
+	// If set to true, public clients must use PKCE.
+	ForceForPublicClients bool
+
 	// Whether or not to allow the plain challenge method (S256 should be used whenever possible, plain is really discouraged).
 	EnablePlainChallengeMethod bool
 
@@ -54,8 +57,9 @@ func (c *Handler) HandleAuthorizeEndpointRequest(ctx context.Context, ar fosite.
 
 	challenge := ar.GetRequestForm().Get("code_challenge")
 	method := ar.GetRequestForm().Get("code_challenge_method")
+	client := ar.GetClient()
 
-	if err := c.validate(challenge, method); err != nil {
+	if err := c.validate(challenge, method, client); err != nil {
 		return err
 	}
 
@@ -75,21 +79,24 @@ func (c *Handler) HandleAuthorizeEndpointRequest(ctx context.Context, ar fosite.
 	return nil
 }
 
-func (c *Handler) validate(challenge, method string) error {
-	if c.Force && challenge == "" {
+func (c *Handler) validate(challenge, method string, client fosite.Client) error {
+	if challenge == "" {
 		// If the server requires Proof Key for Code Exchange (PKCE) by OAuth
 		// clients and the client does not send the "code_challenge" in
 		// the request, the authorization endpoint MUST return the authorization
 		// error response with the "error" value set to "invalid_request".  The
 		// "error_description" or the response of "error_uri" SHOULD explain the
 		// nature of error, e.g., code challenge required.
-
-		return errors.WithStack(fosite.ErrInvalidRequest.
-			WithHint("Clients must include a code_challenge when performing the authorize code flow, but it is missing.").
-			WithDebug("The server is configured in a way that enforces PKCE for clients."))
-	}
-
-	if !c.Force && challenge == "" {
+		if c.Force {
+			return errors.WithStack(fosite.ErrInvalidRequest.
+				WithHint("Clients must include a code_challenge when performing the authorize code flow, but it is missing.").
+				WithDebug("The server is configured in a way that enforces PKCE for clients."))
+		}
+		if c.ForceForPublicClients && client.IsPublic() {
+			return errors.WithStack(fosite.ErrInvalidRequest.
+				WithHint("This client must include a code_challenge when performing the authorize code flow, but it is missing.").
+				WithDebug("The server is configured in a way that enforces PKCE for this client."))
+		}
 		return nil
 	}
 
@@ -145,7 +152,8 @@ func (c *Handler) HandleTokenEndpointRequest(ctx context.Context, request fosite
 
 	challenge := authorizeRequest.GetRequestForm().Get("code_challenge")
 	method := authorizeRequest.GetRequestForm().Get("code_challenge_method")
-	if err := c.validate(challenge, method); err != nil {
+	client := authorizeRequest.GetClient()
+	if err := c.validate(challenge, method, client); err != nil {
 		return err
 	}
 

handler/pkce/handler_test.go

@@ -282,10 +282,12 @@ func TestPKCEHandleTokenEndpointRequest(t *testing.T) {
 	for k, tc := range []struct {
 		d           string
 		force       bool
+		forcePublic bool
 		enablePlain bool
 		challenge   string
 		method      string
 		expectErr   bool
+		client      *fosite.DefaultClient
 	}{
 		{
 			d: "should pass because pkce is not enforced",
@@ -302,6 +304,13 @@ func TestPKCEHandleTokenEndpointRequest(t *testing.T) {
 			expectErr:   true,
 			method:      "S256",
 		},
+		{
+			d:           "should fail because forcePublic is enabled, the client is public, and no challenge was given",
+			forcePublic: true,
+			client:      &fosite.DefaultClient{Public: true},
+			expectErr:   true,
+			method:      "S256",
+		},
 		{
 			d:         "should fail because although force is enabled and a challenge was given, plain is disabled",
 			force:     true,
@@ -328,17 +337,25 @@ func TestPKCEHandleTokenEndpointRequest(t *testing.T) {
 			method:    "S256",
 			challenge: "challenge",
 		},
+		{
+			d:           "should pass because forcePublic is enabled with challenge given and method is S256",
+			forcePublic: true,
+			client:      &fosite.DefaultClient{Public: true},
+			method:      "S256",
+			challenge:   "challenge",
+		},
 	} {
 		t.Run(fmt.Sprintf("case=%d/description=%s", k, tc.d), func(t *testing.T) {
 			h := &Handler{
 				Force:                      tc.force,
+				ForceForPublicClients:      tc.forcePublic,
 				EnablePlainChallengeMethod: tc.enablePlain,
 			}
 
 			if tc.expectErr {
-				assert.Error(t, h.validate(tc.challenge, tc.method))
+				assert.Error(t, h.validate(tc.challenge, tc.method, tc.client))
 			} else {
-				assert.NoError(t, h.validate(tc.challenge, tc.method))
+				assert.NoError(t, h.validate(tc.challenge, tc.method, tc.client))
 			}
 		})
 	}