fix(oauthserver): make PKCE optional for confidential clients on /oauth/authorize

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,16 +468,32 @@ 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 (OAuth 2.1 §4.1.1, RFC 8252 §8.1).
+	// For confidential clients the client_secret authenticates the token
+	// exchange and PKCE is OPTIONAL — many large IdP/connector consumers
+	// (Shopify, Microsoft Power Platform Connectors) never send a
+	// `code_challenge` and were rejected with `invalid_request` here. Issue #2585.
+	isConfidential := client != nil && client.ClientType == models.OAuthServerClientTypeConfidential
+	pkceProvided := codeChallenge != "" || codeChallengeMethod != ""
+
+	if !pkceProvided {
+		if isConfidential {
+			// Confidential client opted out of PKCE — the client_secret presented
+			// at /oauth/token will authenticate the exchange instead.
+			return nil
+		}
+		return errors.New("PKCE flow requires both code_challenge and code_challenge_method")
+	}
+
+	// PKCE was at least partially supplied — both halves must be present.
 	if codeChallenge == "" || codeChallengeMethod == "" {
 		return errors.New("PKCE flow requires both code_challenge and code_challenge_method")
 	}

internal/api/oauthserver/authorize_test.go

@@ -666,3 +666,86 @@ func (ts *OAuthAuthorizeTestSuite) TestConsent_InvalidActionRejected() {
 	err := ts.Server.OAuthServerConsent(w, req)
 	ts.assertHTTPError(err, http.StatusBadRequest, apierrors.ErrorCodeValidationFailed)
 }
+
+// Regression for https://github.com/supabase/auth/issues/2585: PKCE was
+// rejected on the /oauth/authorize endpoint even for confidential clients,
+// which made the OAuth server incompatible with consumers that don't send
+// `code_challenge` (Microsoft Power Platform Connectors, Shopify, …) even
+// though those clients authenticate with a client_secret at the token
+// exchange. OAuth 2.1 §4.1.1 only mandates PKCE for public clients.
+func (ts *OAuthAuthorizeTestSuite) TestAuthorize_ConfidentialClientWithoutPKCEAccepted() {
+	client := ts.createClient()
+	require.Equal(ts.T(), models.OAuthServerClientTypeConfidential, client.ClientType,
+		"createClient() should default to a confidential client")
+
+	q := url.Values{
+		"client_id":    []string{client.ID.String()},
+		"redirect_uri": []string{"https://example.com/callback"},
+		"scope":        []string{"openid"},
+		// No code_challenge / code_challenge_method — confidential client opts out.
+	}
+	req := httptest.NewRequest(http.MethodGet, "/oauth/authorize?"+q.Encode(), nil)
+	w := httptest.NewRecorder()
+	require.NoError(ts.T(), ts.Server.OAuthServerAuthorize(w, req))
+
+	require.Equal(ts.T(), http.StatusFound, w.Code, "authorize: %s", w.Body.String())
+	loc, err := url.Parse(w.Header().Get("Location"))
+	require.NoError(ts.T(), err)
+	require.Empty(ts.T(), loc.Query().Get("error"),
+		"expected no error redirect, got %s", w.Header().Get("Location"))
+	require.NotEmpty(ts.T(), loc.Query().Get("authorization_id"),
+		"expected an authorization_id, got %s", w.Header().Get("Location"))
+}
+
+func (ts *OAuthAuthorizeTestSuite) TestAuthorize_PublicClientWithoutPKCERejected() {
+	publicParams := &OAuthServerClientRegisterParams{
+		ClientName:              "Test Public Client",
+		RedirectURIs:            []string{"https://example.com/callback"},
+		RegistrationType:        "dynamic",
+		ClientType:              models.OAuthServerClientTypePublic,
+		TokenEndpointAuthMethod: models.TokenEndpointAuthMethodNone,
+	}
+	publicClient, _, err := ts.Server.registerOAuthServerClient(context.Background(), publicParams)
+	require.NoError(ts.T(), err)
+	require.Equal(ts.T(), models.OAuthServerClientTypePublic, publicClient.ClientType)
+
+	q := url.Values{
+		"client_id":    []string{publicClient.ID.String()},
+		"redirect_uri": []string{"https://example.com/callback"},
+		"scope":        []string{"openid"},
+		// No PKCE — must still error for public clients per OAuth 2.1.
+	}
+	req := httptest.NewRequest(http.MethodGet, "/oauth/authorize?"+q.Encode(), nil)
+	w := httptest.NewRecorder()
+	require.NoError(ts.T(), ts.Server.OAuthServerAuthorize(w, req))
+
+	require.Equal(ts.T(), http.StatusFound, w.Code)
+	loc, err := url.Parse(w.Header().Get("Location"))
+	require.NoError(ts.T(), err)
+	require.Equal(ts.T(), oAuth2ErrorInvalidRequest, loc.Query().Get("error"),
+		"expected invalid_request error redirect for public client missing PKCE, got %s",
+		w.Header().Get("Location"))
+	require.Contains(ts.T(), loc.Query().Get("error_description"), "PKCE flow requires")
+}
+
+func (ts *OAuthAuthorizeTestSuite) TestAuthorize_ConfidentialClientWithPartialPKCERejected() {
+	client := ts.createClient()
+
+	q := url.Values{
+		"client_id":      []string{client.ID.String()},
+		"redirect_uri":   []string{"https://example.com/callback"},
+		"scope":          []string{"openid"},
+		"code_challenge": []string{"abcdefghijklmnopqrstuvwxyz0123456789ABCDEFG"},
+		// `code_challenge_method` deliberately omitted — half-supplied PKCE.
+	}
+	req := httptest.NewRequest(http.MethodGet, "/oauth/authorize?"+q.Encode(), nil)
+	w := httptest.NewRecorder()
+	require.NoError(ts.T(), ts.Server.OAuthServerAuthorize(w, req))
+
+	require.Equal(ts.T(), http.StatusFound, w.Code)
+	loc, err := url.Parse(w.Header().Get("Location"))
+	require.NoError(ts.T(), err)
+	require.Equal(ts.T(), oAuth2ErrorInvalidRequest, loc.Query().Get("error"),
+		"expected invalid_request when a confidential client provides only half of PKCE, got %s",
+		w.Header().Get("Location"))
+}