From 25dcad757a7d7670bfd8d4d4ac209e26dc919596 Mon Sep 17 00:00:00 2001 From: Elias Schneider Date: Wed, 8 Jul 2026 10:42:05 +0200 Subject: [PATCH] feat: add support for unencrypted OIDC request parameter --- backend/go.mod | 2 +- backend/go.sum | 4 +- .../controller/well_known_controller.go | 3 + .../internal/oidc/authorization_handler.go | 5 +- backend/internal/oidc/provider.go | 40 +++++---- backend/internal/oidc/provider_test.go | 90 +++++++++++++++++++ 6 files changed, 122 insertions(+), 22 deletions(-) diff --git a/backend/go.mod b/backend/go.mod index 89e9609b..f30c16dd 100644 --- a/backend/go.mod +++ b/backend/go.mod @@ -248,4 +248,4 @@ require ( modernc.org/memory v1.11.0 // indirect ) -replace github.com/ory/fosite => github.com/pocket-id/fosite v0.0.0-20260706212719-8e923341c5c1 +replace github.com/ory/fosite => github.com/pocket-id/fosite v0.0.0-20260708083902-56a3c0f378d6 diff --git a/backend/go.sum b/backend/go.sum index 6e1b02a5..d91d42d1 100644 --- a/backend/go.sum +++ b/backend/go.sum @@ -426,8 +426,8 @@ github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINE github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= -github.com/pocket-id/fosite v0.0.0-20260706212719-8e923341c5c1 h1:xDSTZCWHidOOIuLryG1GCdzaZhL6XtJ9WO+DMIeJHLY= -github.com/pocket-id/fosite v0.0.0-20260706212719-8e923341c5c1/go.mod h1:KeQ7tTIBm3DyeBnKcKLnPbSdrd6ttM6w3TD3yy9x8rM= +github.com/pocket-id/fosite v0.0.0-20260708083902-56a3c0f378d6 h1:gD1LKfqFkbIHkE6d308IFJkZbHw4VwL7buxEnblNQTs= +github.com/pocket-id/fosite v0.0.0-20260708083902-56a3c0f378d6/go.mod h1:KeQ7tTIBm3DyeBnKcKLnPbSdrd6ttM6w3TD3yy9x8rM= github.com/prometheus/client_golang v1.23.2 h1:Je96obch5RDVy3FDMndoUsjAhG5Edi49h0RJWRi/o0o= github.com/prometheus/client_golang v1.23.2/go.mod h1:Tb1a6LWHB3/SPIzCoaDXI4I8UHKeFTEQ1YCr+0Gyqmg= github.com/prometheus/client_model v0.6.2 h1:oBsgwpGs7iVziMvrGhE53c/GrLUsZdHnqNwqPLxwZyk= diff --git a/backend/internal/controller/well_known_controller.go b/backend/internal/controller/well_known_controller.go index 249fc341..cf8c1c84 100644 --- a/backend/internal/controller/well_known_controller.go +++ b/backend/internal/controller/well_known_controller.go @@ -91,6 +91,9 @@ func (wkc *WellKnownController) computeOIDCConfiguration() ([]byte, error) { "id_token_signing_alg_values_supported": []string{alg.String()}, "authorization_response_iss_parameter_supported": true, "code_challenge_methods_supported": []string{"plain", "S256"}, + "request_parameter_supported": true, + "request_uri_parameter_supported": false, + "request_object_signing_alg_values_supported": []string{"none"}, "prompt_values_supported": []string{"none", "login", "consent", "select_account"}, "token_endpoint_auth_methods_supported": []string{"client_secret_basic", "client_secret_post", "none"}, "pushed_authorization_request_endpoint": internalAppUrl + "/api/oidc/par", diff --git a/backend/internal/oidc/authorization_handler.go b/backend/internal/oidc/authorization_handler.go index d8a41b27..4d11be49 100644 --- a/backend/internal/oidc/authorization_handler.go +++ b/backend/internal/oidc/authorization_handler.go @@ -172,7 +172,10 @@ func requestMetaFromGin(c *gin.Context) requestMeta { func authorizeRequestParams(requester fosite.AuthorizeRequester) map[string]string { params := make(map[string]string) for key, values := range requester.GetRequestForm() { - if len(values) == 0 || key == "request_uri" || key == "interaction" { + // The raw "request" object is dropped alongside "request_uri": its claims are already merged + // into the form, and replaying the JWT on interaction resume would re-validate its "exp" + // against the resume time, failing logins that took longer than the object's lifetime. + if len(values) == 0 || key == "request" || key == "request_uri" || key == "interaction" { continue } params[key] = values[0] diff --git a/backend/internal/oidc/provider.go b/backend/internal/oidc/provider.go index a0be1bc8..83ba0283 100644 --- a/backend/internal/oidc/provider.go +++ b/backend/internal/oidc/provider.go @@ -37,24 +37,28 @@ func newProvider(store *Store, authenticator *federatedClientAuthenticator, sign } var fositeConfig = &fosite.Config{ - RefreshTokenLifespan: 30 * 24 * time.Hour, - DeviceAndUserCodeLifespan: 15 * time.Minute, - DeviceAuthTokenPollingInterval: 5 * time.Second, - DeviceVerificationURL: config.BaseURL + "/device", - PushedAuthorizeContextLifespan: 90 * time.Second, - IDTokenIssuer: config.BaseURL, - AccessTokenIssuer: config.BaseURL, - TokenURL: config.TokenBaseURL + "/api/oidc/token", - ScopeStrategy: fosite.ExactScopeStrategy, - IgnoreUnknownScopes: true, - AudienceMatchingStrategy: fosite.ExactAudienceMatchingStrategy, - RedirectURIMatcher: matchRedirectURI, - EnforcePKCEForPublicClients: true, - EnablePKCEPlainChallengeMethod: true, - FormPostHTMLTemplate: formPostTemplate, - RefreshTokenScopes: []string{}, - GlobalSecret: secret, - JWTScopeClaimKey: jwt.JWTScopeFieldBoth, + RefreshTokenLifespan: 30 * 24 * time.Hour, + DeviceAndUserCodeLifespan: 15 * time.Minute, + DeviceAuthTokenPollingInterval: 5 * time.Second, + DeviceVerificationURL: config.BaseURL + "/device", + PushedAuthorizeContextLifespan: 90 * time.Second, + IDTokenIssuer: config.BaseURL, + AccessTokenIssuer: config.BaseURL, + TokenURL: config.TokenBaseURL + "/api/oidc/token", + ScopeStrategy: fosite.ExactScopeStrategy, + IgnoreUnknownScopes: true, + AudienceMatchingStrategy: fosite.ExactAudienceMatchingStrategy, + RedirectURIMatcher: matchRedirectURI, + EnforcePKCEForPublicClients: true, + EnablePKCEPlainChallengeMethod: true, + FormPostHTMLTemplate: formPostTemplate, + RefreshTokenScopes: []string{}, + GlobalSecret: secret, + SupportedRequestObjectSigningAlgorithms: []string{"none"}, + FormPostHTMLTemplate: formPostTemplate, + RefreshTokenScopes: []string{}, + GlobalSecret: secret, + JWTScopeClaimKey: jwt.JWTScopeFieldBoth, } keyGetter := func(context.Context) (interface{}, error) { diff --git a/backend/internal/oidc/provider_test.go b/backend/internal/oidc/provider_test.go index 41d42c13..6f25a3e7 100644 --- a/backend/internal/oidc/provider_test.go +++ b/backend/internal/oidc/provider_test.go @@ -179,6 +179,96 @@ func TestProviderRejectsUnmatchedWildcardRedirectURI(t *testing.T) { require.ErrorIs(t, err, fosite.ErrInvalidRequest) } +// encodeRequestObject builds a compact JWS request object from the given header and claims, +// with an empty signature segment (as used by unsigned "alg": "none" request objects). +func encodeRequestObject(t *testing.T, header map[string]any, claims map[string]any) string { + t.Helper() + headerJSON, err := json.Marshal(header) + require.NoError(t, err) + claimsJSON, err := json.Marshal(claims) + require.NoError(t, err) + return base64.RawURLEncoding.EncodeToString(headerJSON) + "." + base64.RawURLEncoding.EncodeToString(claimsJSON) + "." +} + +func TestProviderAcceptsUnsignedRequestObject(t *testing.T) { + db := testutils.NewDatabaseForTest(t) + signerKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + require.NoError(t, db.Create(&model.OidcClient{ + Base: model.Base{ID: "test-client"}, + Name: "Test Client", + CallbackURLs: model.UrlList{"https://client.example.com/callback"}, + }).Error) + + provider, err := newProvider(NewStore(db, nil), nil, testTokenSigner{key: signerKey}, Config{ //nolint:gosec // static test-only provider secret + BaseURL: "https://issuer.example.com", + TokenBaseURL: "https://issuer.example.com", + Secret: []byte("test-secret"), + }) + require.NoError(t, err) + + requestObject := encodeRequestObject(t, + map[string]any{"alg": "none"}, + map[string]any{ + "client_id": "test-client", + "redirect_uri": "https://client.example.com/callback", + "nonce": "nonce-from-request-object", + "max_age": 300, + }, + ) + + req := httptest.NewRequestWithContext( + t.Context(), + http.MethodGet, + "/api/oidc/authorize?client_id=test-client&response_type=code&scope=openid&state=state-with-enough-entropy&request="+requestObject, + nil, + ) + + ar, err := provider.NewAuthorizeRequest(req.Context(), req) + require.NoError(t, err) + require.Equal(t, "https://client.example.com/callback", ar.GetRedirectURI().String()) + require.Equal(t, "nonce-from-request-object", ar.GetRequestForm().Get("nonce")) + require.Equal(t, "300", ar.GetRequestForm().Get("max_age")) +} + +func TestProviderRejectsSignedRequestObject(t *testing.T) { + db := testutils.NewDatabaseForTest(t) + signerKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + require.NoError(t, db.Create(&model.OidcClient{ + Base: model.Base{ID: "test-client"}, + Name: "Test Client", + CallbackURLs: model.UrlList{"https://client.example.com/callback"}, + }).Error) + + provider, err := newProvider(NewStore(db, nil), nil, testTokenSigner{key: signerKey}, Config{ //nolint:gosec // static test-only provider secret + BaseURL: "https://issuer.example.com", + TokenBaseURL: "https://issuer.example.com", + Secret: []byte("test-secret"), + }) + require.NoError(t, err) + + // The signature is never verified: the request object must already be rejected because only + // "none" is a supported request object signing algorithm. + requestObject := encodeRequestObject(t, + map[string]any{"alg": "RS256"}, + map[string]any{"redirect_uri": "https://client.example.com/callback"}, + ) + base64.RawURLEncoding.EncodeToString([]byte("signature")) + + req := httptest.NewRequestWithContext( + t.Context(), + http.MethodGet, + "/api/oidc/authorize?client_id=test-client&response_type=code&scope=openid&state=state-with-enough-entropy&request="+requestObject, + nil, + ) + + _, err = provider.NewAuthorizeRequest(req.Context(), req) + require.ErrorIs(t, err, fosite.ErrInvalidRequestObject) + // Pin the rejection to the SupportedRequestObjectSigningAlgorithms allowlist: a signed object + // would also fail with invalid_request_object for other reasons, e.g. missing client JWKS. + require.Contains(t, fosite.ErrorToRFC6749Error(err).Reason(), "the authorization server only supports [none]") +} + // decodeJWTPart base64url-decodes the header (index 0) or claims (index 1) segment of a // JWT without verifying the signature, for assertions in tests. func decodeJWTPart(t *testing.T, token string, index int) map[string]any {