Tags: zitadel/oidc
Tags
fix(op): allow dynamic loopback ports in post_logout_redirect_uri per… … RFC 8252 §7.3 (#875) # Which Problems Are Solved - The end-session endpoint rejects valid `post_logout_redirect_uri` values from native (desktop) apps that use the loopback interface with a dynamic port. Registering `http://127.0.0.1/callback` and logging out via `http://127.0.0.1:54321/callback` returns `{"error":"invalid_request","error_description":"post_logout_redirect_uri invalid"}`, even though the same client can authenticate via that exact loopback URI without issue. - This breaks RP-initiated logout for any native app that follows [RFC 8252 §7.3](https://datatracker.ietf.org/doc/html/rfc8252#section-7.3) ("Loopback Interface Redirection"), which states that the authorization server *MUST* allow any port to be specified at request time for loopback IP redirect URIs. - The asymmetry exists because the auth-request side already implements RFC 8252 §7.3 for `redirect_uri` (see `validateAuthReqRedirectURINative`), but `ValidateEndSessionPostLogoutRedirectURI` was a plain string-equality check. # How the Problems Are Solved - `ValidateEndSessionPostLogoutRedirectURI` in `pkg/op/session.go` gains a native-app loopback branch placed between the existing exact-match loop and the existing `HasRedirectGlobs` (DevMode) branch. - The new branch reuses the helpers already used by the auth-request validator: `HTTPLoopbackOrLocalhost` (parses an http/https URI and reports whether the host is `127.0.0.1`, `[::1]`, or `localhost`) and `equalURI` (compares only path + raw query). When the client is `ApplicationTypeNative` and the incoming `post_logout_redirect_uri` resolves to a loopback host, each registered URI is parsed the same way and compared without considering the port — matching the auth-side semantics exactly. - No change to public API. No change to the Client/HasRedirectGlobs interfaces. # Additional Changes - Adds `pkg/op/session_test.go` with table-driven coverage for `ValidateEndSessionPostLogoutRedirectURI`. The table covers: existing exact / no-match / glob cases (regression guard), native loopback v4/v6/localhost dynamic ports, native HTTPS loopback, native cross-loopback equivalence (v4 ↔ v6, inherited from `equalURI`), wrong-path negatives, web/user-agent dynamic-port rejection, and the literal RFC 8252 §7.3 example URIs (`http://127.0.0.1:51004/oauth2redirect/example-provider`, `http://[::1]:61023/oauth2redirect/example-provider`) plus the §8.3 `localhost` variant. Total: 24 cases, all passing. # Additional Context - Closes zitadel/zitadel#6615 (filed 2023-09-25). - Mirrors the existing native-app handling in `pkg/op/auth_request.go` (`validateAuthReqRedirectURINative`, lines 352-382 in v3.47.0) so that auth and logout treat loopback URIs consistently.
fix: URL-encode client credentials in Basic Auth per RFC 6749 §2.3.1 (#… …873) ## Summary Four `Auth()` methods call `req.SetBasicAuth()` with raw client credentials. Per [RFC 6749 Section 2.3.1](https://datatracker.ietf.org/doc/html/rfc6749#section-2.3.1), client credentials MUST be encoded using the `application/x-www-form-urlencoded` encoding algorithm before being sent via HTTP Basic Authentication. `net/http.SetBasicAuth` only base64-encodes the credentials — it does not URL-encode them first. When a client secret contains characters like `%`, authorization servers that URL-decode per the RFC (e.g., Keycloak) fail with errors like `URLDecoder: Incomplete trailing escape (%) pattern`. The existing `AuthorizeBasic()` helper in `pkg/http/http.go` already correctly applies `url.QueryEscape`. This PR applies the same encoding to the four `Auth()` methods that were missing it: - `RefreshTokenRequest.Auth` (`pkg/client/rp/relying_party.go`) - `RevokeRequest.Auth` (`pkg/client/client.go`) - `DeviceAccessTokenRequest.Auth` (`pkg/client/client.go`) - `ClientCredentialsRequest.Auth` (`pkg/oidc/token_request.go`) ## Related This is distinct from #857 / #858 (duplicate credentials issue), though both stem from the same `Auth()` methods. This PR fixes the encoding bug without removing the `ClientSecretBasicAuthRequest` interface, so it is not a breaking change. ## Testing All existing tests pass. The fix is a one-line change per method, consistent with the pattern already used by `AuthorizeBasic()`.
fix: propagate signature verification errors correctly (#872) # Which Problems Are Solved The error returned after the signature verification fails is not propagated correctly to the calling function. # How the Problems Are Solved The error is wrapped (using the %w verb) instead of embedding (with the %v verb) the error message before returning to the caller. # Additional Changes Update test cases to assert the error type. # Additional Context - Closes #864
fix: tolerate string amr claims from external providers (#855) ## Summary - accept non-compliant `amr` values when providers return a single string instead of an array - normalize `amr` to `[]string` for ID token, access token, and introspection unmarshalling without changing the public field types - add focused tests for string and array `amr` values during JSON unmarshalling ## Testing - `go test ./pkg/oidc` - `go test ./...` *(currently fails in unrelated existing example/server route tests in `pkg/client/rp` and `pkg/op` under Go 1.26 in this environment)* --------- Co-authored-by: Wim Van Laer <[email protected]>
feat: Allow for reuse of cookie creation + decouple creation from htt… …p writer (#848) The cookie handler used for .CheckCookie() and .SetCookie() is needlessly constrained and not reusable outside of the scope of an http resp writer. See #847 ### Definition of Ready - [X] I am happy with the code - [X] Short description of the feature/issue is added in the pr description - [X] PR is linked to the corresponding user story - [X] Acceptance criteria are met - [X] All open todos and follow ups are defined in a new ticket and justified - [X] Deviations from the acceptance criteria and design are agreed with the PO and documented. - [X] No debug or dead code - [X] My code has no repetitions - [X] Critical parts are tested automatically - [X] Where possible E2E tests are implemented - [X] Documentation/examples are up-to-date - [X] All non-functional requirements are met - [X] Functionality of the acceptance criteria is checked manually on the dev system.
fix: add auth_methods client_secret_basic for refresh token and revok… ( #803) I have an authentication provider that only accepts client secrets from the request headers. Because of this, the refresh token and revoke token requests fail — the Authorization header is missing. This PR adds the Authorization header with the client credentials to those requests. Additionally, I’ve updated how the header is added in other requests to follow the same approach. I’d appreciate any feedback and am happy to adjust the PR as needed. --------- Co-authored-by: Tim Möhlmann <[email protected]>
fix: grant type in device code validation error message (#845) Use `GrantTypeDeviceCode` in device code error message The error message was incorrectly referencing `GrantTypeCode`(authorization_code) instead of `GrantTypeDeviceCode` when validation failed.
PreviousNext