From 57273ae9d149d7ad10eaaafedcb92f2d7fa00423 Mon Sep 17 00:00:00 2001
From: Gourab Singha <gaurabsingha16@gmail.com>
Date: Fri, 26 Jun 2026 01:07:31 +0530
Subject: [PATCH] feat: make PKCE optional for confidential clients in OAuth
 server

Fixes #2585
---
 internal/api/oauthserver/authorize.go      | 20 +++++----
 internal/api/oauthserver/authorize_test.go | 49 ++++++++++++++++++++++
 2 files changed, 62 insertions(+), 7 deletions(-)

diff --git a/internal/api/oauthserver/authorize.go b/internal/api/oauthserver/authorize.go
index 2addacfda..9dad6f2f8 100644
--- a/internal/api/oauthserver/authorize.go
+++ b/internal/api/oauthserver/authorize.go
@@ -129,7 +129,7 @@ func (s *Server) OAuthServerAuthorize(w http.ResponseWriter, r *http.Request) er
 
 	// From this point on, we have valid client + redirect_uri + all params, so we can redirect errors
 	// validate all other parameters - now we can redirect errors
-	if err := s.validateRemainingAuthorizeParams(params); err != nil {
+	if err := s.validateRemainingAuthorizeParams(params, client); err != nil {
 		errorRedirectURL := s.buildErrorRedirectURL(params.RedirectURI, oAuth2ErrorInvalidRequest, err.Error(), params.State)
 		http.Redirect(w, r, errorRedirectURL, http.StatusFound)
 		return nil
@@ -444,7 +444,7 @@ func (s *Server) validateBasicAuthorizeParams(params *AuthorizeParams) (*Authori
 }
 
 // validateRemainingAuthorizeParams validates all other parameters (can redirect errors since we have valid client + redirect_uri)
-func (s *Server) validateRemainingAuthorizeParams(params *AuthorizeParams) error {
+func (s *Server) validateRemainingAuthorizeParams(params *AuthorizeParams, client *models.OAuthServerClient) error {
 	if params.ResponseType == "" {
 		params.ResponseType = models.OAuthServerResponseTypeCode.String()
 	}
@@ -468,18 +468,24 @@ func (s *Server) validateRemainingAuthorizeParams(params *AuthorizeParams) error
 	}
 
 	// PKCE validation
-	if err := s.validatePKCEParams(params.CodeChallengeMethod, params.CodeChallenge); err != nil {
+	if err := s.validatePKCEParams(params.CodeChallengeMethod, params.CodeChallenge, client); err != nil {
 		return err
 	}
 
 	return nil
 }
 
-func (s *Server) validatePKCEParams(codeChallengeMethod, codeChallenge string) error {
-	// PKCE is mandatory for the authorization code flow OAuth2.1
-	// Both code_challenge and code_challenge_method must be provided together
+func (s *Server) validatePKCEParams(codeChallengeMethod, codeChallenge string, client *models.OAuthServerClient) error {
+	// PKCE is mandatory for public clients in the authorization code flow
+	// For confidential clients, it is optional.
 	if codeChallenge == "" || codeChallengeMethod == "" {
-		return errors.New("PKCE flow requires both code_challenge and code_challenge_method")
+		if client.IsPublic() {
+			return errors.New("PKCE flow requires both code_challenge and code_challenge_method for public clients")
+		}
+		if codeChallenge != "" || codeChallengeMethod != "" {
+			return errors.New("PKCE flow requires both code_challenge and code_challenge_method")
+		}
+		return nil
 	}
 
 	// Validate code challenge method (case-insensitive)
diff --git a/internal/api/oauthserver/authorize_test.go b/internal/api/oauthserver/authorize_test.go
index c7ec7f458..3d02c2e73 100644
--- a/internal/api/oauthserver/authorize_test.go
+++ b/internal/api/oauthserver/authorize_test.go
@@ -666,3 +666,52 @@ func (ts *OAuthAuthorizeTestSuite) TestConsent_InvalidActionRejected() {
 	err := ts.Server.OAuthServerConsent(w, req)
 	ts.assertHTTPError(err, http.StatusBadRequest, apierrors.ErrorCodeValidationFailed)
 }
+
+func (ts *OAuthAuthorizeTestSuite) TestAuthorize_ConfidentialClientWithoutPKCE() {
+	client := ts.createClient() // confidential by default
+	q := url.Values{
+		"client_id":     []string{client.ID.String()},
+		"redirect_uri":  []string{"https://example.com/callback"},
+		"scope":         []string{"openid"},
+		"response_type": []string{"code"},
+	}
+	req := httptest.NewRequest(http.MethodGet, "/oauth/authorize?"+q.Encode(), nil)
+	w := httptest.NewRecorder()
+	require.NoError(ts.T(), ts.Server.OAuthServerAuthorize(w, req))
+	assert.Equal(ts.T(), http.StatusFound, w.Code)
+
+	loc, err := url.Parse(w.Header().Get("Location"))
+	require.NoError(ts.T(), err)
+	authID := loc.Query().Get("authorization_id")
+	assert.NotEmpty(ts.T(), authID)
+}
+
+func (ts *OAuthAuthorizeTestSuite) TestAuthorize_PublicClientWithoutPKCE() {
+	// Create a public client
+	params := &OAuthServerClientRegisterParams{
+		ClientName:              "Test Public Client",
+		RedirectURIs:            []string{"https://example.com/callback"},
+		RegistrationType:        "dynamic",
+		ClientType:              models.OAuthServerClientTypePublic,
+		TokenEndpointAuthMethod: models.TokenEndpointAuthMethodNone,
+	}
+	client, _, err := ts.Server.registerOAuthServerClient(context.Background(), params)
+	require.NoError(ts.T(), err)
+
+	q := url.Values{
+		"client_id":     []string{client.ID.String()},
+		"redirect_uri":  []string{"https://example.com/callback"},
+		"scope":         []string{"openid"},
+		"response_type": []string{"code"},
+	}
+	req := httptest.NewRequest(http.MethodGet, "/oauth/authorize?"+q.Encode(), nil)
+	w := httptest.NewRecorder()
+	require.NoError(ts.T(), ts.Server.OAuthServerAuthorize(w, req))
+	assert.Equal(ts.T(), http.StatusFound, w.Code)
+
+	loc, err := url.Parse(w.Header().Get("Location"))
+	require.NoError(ts.T(), err)
+	assert.Equal(ts.T(), oAuth2ErrorInvalidRequest, loc.Query().Get("error"))
+	assert.Contains(ts.T(), loc.Query().Get("error_description"), "PKCE flow requires both code_challenge and code_challenge_method")
+}
+