diff --git a/backend/internal/controller/oidc_controller.go b/backend/internal/controller/oidc_controller.go index 1b5d3298..b76a8394 100644 --- a/backend/internal/controller/oidc_controller.go +++ b/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 { diff --git a/backend/internal/controller/oidc_controller_test.go b/backend/internal/controller/oidc_controller_test.go index 09c23e8f..28421448 100644 --- a/backend/internal/controller/oidc_controller_test.go +++ b/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) {