fix(oidc): fall back to Basic auth when PKCE puts client_id in body (#1466)

Co-authored-by: mgabor3141 <@mgabor3141>
Co-authored-by: Gabor Mezo <git@mgabor.hu>
Co-authored-by: Alessandro (Ale) Segala <43508+ItalyPaleAle@users.noreply.github.com>

backend/internal/controller/oidc_controller.go

@@ -202,10 +202,8 @@ func (oc *OidcController) createTokensHandler(c *gin.Context) {
 		return
 	}
 
-	// Client id and secret can also be passed over the Authorization header
-	if input.ClientID == "" && input.ClientSecret == "" {
-		input.ClientID, input.ClientSecret, _ = utils.OAuthClientBasicAuth(c.Request)
-	}
+	// Check if the client ID / secret are passed in the Authorization header (RFC 6749)
+	parseBasicAuth(c.Request, &input.ClientSecret, &input.ClientID)
 
 	tokens, err := oc.createTokens(c.Request.Context(), input)
 
@@ -692,6 +690,22 @@ func (oc *OidcController) updateAllowedUserGroupsHandler(c *gin.Context) {
 	c.JSON(http.StatusOK, oidcClientDto)
 }
 
+// This method can modify the value of clientSecret and clientID
+func parseBasicAuth(r *http.Request, clientSecret *string, clientID *string) {
+	// Client id and secret can also be passed via the Authorization header (RFC 6749, section 2.3.1 "client_secret_basic")
+	// When PKCE is used, some libraries send client_id in the body for the code_verifier binding while keeping the secret only in the Authorization header
+	// We therefore fall back to Basic auth whenever the secret is missing, not only when both fields are empty
+	if *clientSecret == "" {
+		basicID, basicSecret, ok := utils.OAuthClientBasicAuth(r)
+		if ok {
+			if *clientID == "" {
+				*clientID = basicID
+			}
+			*clientSecret = basicSecret
+		}
+	}
+}
+
 func (oc *OidcController) deviceAuthorizationHandler(c *gin.Context) {
 	// Per RFC 8628 (OAuth 2.0 Device Authorization Grant), parameters for the device authorization request MUST be sent in the body of the POST request
 	// Gin's "ShouldBind" by default reads from the query string too, so we need to reset all query string args before invoking ShouldBind
@@ -704,10 +718,8 @@ func (oc *OidcController) deviceAuthorizationHandler(c *gin.Context) {
 		return
 	}
 
-	// Client id and secret can also be passed over the Authorization header
-	if input.ClientID == "" && input.ClientSecret == "" {
-		input.ClientID, input.ClientSecret, _ = utils.OAuthClientBasicAuth(c.Request)
-	}
+	// Check if the client ID / secret are passed in the Authorization header (RFC 6749)
+	parseBasicAuth(c.Request, &input.ClientSecret, &input.ClientID)
 
 	response, err := oc.oidcService.CreateDeviceAuthorization(c.Request.Context(), input)
 	if err != nil {

backend/internal/controller/oidc_controller_test.go

@@ -146,6 +146,43 @@ func TestCreateTokensHandler(t *testing.T) {
 		assert.Equal(t, 120, response.ExpiresIn)
 	})
 
+	t.Run("Uses Basic Auth Secret When Body Has Only Client ID (PKCE)", func(t *testing.T) {
+		// Some OIDC libraries (e.g. jumbojett/openid-connect-php) combine client_secret_basic with PKCE by sending client_id in the body alongside code_verifier while keeping the secret only in the Authorization header
+		// RFC 6749 §2.3.1 permits this; the body client_id must not block the Basic auth fallback for the secret.
+		var capturedInput dto.OidcCreateTokensDto
+		oc := &OidcController{
+			createTokens: func(_ context.Context, input dto.OidcCreateTokensDto) (service.CreatedTokens, error) {
+				capturedInput = input
+				return service.CreatedTokens{
+					AccessToken:  "access-token",
+					IdToken:      "id-token",
+					RefreshToken: "refresh-token",
+					ExpiresIn:    2 * time.Minute,
+				}, nil
+			},
+		}
+
+		basicAuth := "Basic " + base64.StdEncoding.EncodeToString([]byte("client-id:client-secret"))
+		c, recorder := createTestContext(
+			t,
+			"http://example.com/oidc/token",
+			url.Values{
+				"grant_type":    {service.GrantTypeRefreshToken},
+				"refresh_token": {"input-refresh-token"},
+				"client_id":     {"client-id"},
+			},
+			basicAuth,
+			false,
+		)
+
+		oc.createTokensHandler(c)
+
+		require.Empty(t, c.Errors)
+		assert.Equal(t, "client-id", capturedInput.ClientID)
+		assert.Equal(t, "client-secret", capturedInput.ClientSecret)
+		require.Equal(t, http.StatusOK, recorder.Code)
+	})
+
 	t.Run("Maps Authorization Pending Error", func(t *testing.T) {
 		oc := &OidcController{
 			createTokens: func(context.Context, dto.OidcCreateTokensDto) (service.CreatedTokens, error) {