AWS Cognito (and potentially other providers) return `email_verified`
and `phone_number_verified` as strings (`"true"`/`"false"`) instead of
proper JSON booleans, violating the [OIDC
specification](https://openid.net/specs/openid-connect-basic-1_0.html#StandardClaims).
AWS Documentation confirms this:
> Currently, Amazon Cognito returns the values for email_verified and
phone_number_verified as strings.
_Source:
https://docs.aws.amazon.com/cognito/latest/developerguide/userinfo-endpoint.html#get-userinfo-response-sample_
### The Problem
The `zitadel/oidc` library currently handles this inconsistently:
- ✅ `EmailVerified` uses the custom `Bool` type (added in #139)
- ❌ `PhoneNumberVerified` uses Go's standard `bool`
This forces developers to handle semantically identical fields
differently:
```go
// Currently inconsistent code path
userInfo.EmailVerified = oidc.Bool(emailValue) // Cast
userInfo.PhoneNumberVerified = phoneValue // No cast
```
Additionally, the existing `Bool.UnmarshalJSON` implementation meant
that false values couldn't overwrite true.
### Solution
Applied `Bool` type consistently to both fields and simplified
`Bool.UnmarshalJSON` using a direct switch statement to:
- Handle standard JSON booleans (true/false)
- Handle AWS Cognito string format ("true"/"false")
- Return errors on invalid input instead of silently failing
- Allow false to overwrite true
Updated tests to match codebase conventions, as well.
### Impact
`PhoneNumberVerified` changes from `bool` to `Bool` (type alias of
`bool`). Most consumer code should work as-is since `Bool` is just a
type alias. Direct type assertions would need updating.
### Definition of Ready
- [X] I am happy with the code
- [X] Short description of the feature/issue is added in the pr
description
- [ ] PR is linked to the corresponding user story
- [X] Acceptance criteria are met
- [ ] All open todos and follow ups are defined in a new ticket and
justified
- [ ] 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
- [ ] All non-functional requirements are met
- [x] Functionality of the acceptance criteria is checked manually on
the dev system.
Co-authored-by: Wim Van Laer <wim07101993@users.noreply.github.com>
This safely ignores unknown key type errors on JWKS while returning all
other errors. Returned errors are wrap to easily identify which key in
the set is problematic if any.
Jose v4.0.3 was handling this correctly according to spec, but it was
reverted in v4.0.4 as the implementation was a breaking change due to
the custom UnmarshalJSON on the key set. For details see:
- https://github.com/go-jose/go-jose/issues/136
- https://github.com/go-jose/go-jose/pull/137
Jose v4.0.4 also provided a handy static error to check for unknown web
key types. Sadly this was removed: a prefix match on the error message
is the best option until Jose improves it's error handling.
Hopefully, Jose will not change the error message in a patch or minor
version release. But just in case, test cases have been added to detect
it.
Closes#541
### 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
- [ ] Acceptance criteria are met
- [ ] All open todos and follow ups are defined in a new ticket and
justified
- [ ] 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
- [ ] Functionality of the acceptance criteria is checked manually on
the dev system.
Co-authored-by: Wim Van Laer <wim07101993@users.noreply.github.com>
Add the WithPKCEFromDiscovery option to create a relying party with PKCE
enabled if it is supported when query the discovery endpoint as
discussed in #506.
This only works when creating an OIDC RP which performs a discovery
call. With an OAuth2-only RP, an error is returned as no discovery call
is performed.
Closes#506
### 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
- [ ] Acceptance criteria are met
- [ ] All open todos and follow ups are defined in a new ticket and
justified
- [ ] 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
- [ ] Functionality of the acceptance criteria is checked manually on
the dev system.
While reviewing #750, we noticed that the `KeyFile` struct and
corresponding methods are proprietary to Zitadel and should have never
been part of the pure OIDC library.
This PR deprecates the corresponding parts. For users of Zitadel, the
corresponding code is moved to zitadel/zitadel-go#516
### 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
- [ ] All open todos and follow ups are defined in a new ticket and
justified
- [ ] 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
- [ ] 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.
- Add Go 1.25 to the test matrix
- Oldest supported Go version is now 1.24, as required for
https://github.com/zitadel/oidc/pull/796
- Fix non-constant format string build errors
### 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 parameter name typo in `GetKeyByIDAndClientID`
### Definition of Ready
- [x] I am happy with the code
- [x] Short description of the feature/issue is added in the pr
description
- [ ] PR is linked to the corresponding user story
- [ ] Acceptance criteria are met
- [ ] All open todos and follow ups are defined in a new ticket and
justified
- [ ] Deviations from the acceptance criteria and design are agreed with
the PO and documented.
- [ ] No debug or dead code
- [ ] My code has no repetitions
- [ ] Critical parts are tested automatically
- [ ] Where possible E2E tests are implemented
- [ ] Documentation/examples are up-to-date
- [ ] All non-functional requirements are met
- [ ] Functionality of the acceptance criteria is checked manually on
the dev system.
Co-authored-by: Stefan Benz <46600784+stebenz@users.noreply.github.com>
Add a `op.WithCrypto` `op.Option` that allows developers to specify
their custom `op.Crypto` implementations during setup. If the
`op.Option` is used, it will override `op.Config.CryptoKey`.
Closes https://github.com/zitadel/oidc/issues/736.
### 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
- [ ] Acceptance criteria are met
- [ ] All open todos and follow ups are defined in a new ticket and
justified
- [ ] Deviations from the acceptance criteria and design are agreed with
the PO and documented.
- [x] No debug or dead code
- [ ] My code has no repetitions
- [ ] Critical parts are tested automatically
- [ ] Where possible E2E tests are implemented
- [x] Documentation/examples are up-to-date
- [ ] All non-functional requirements are met
- [ ] Functionality of the acceptance criteria is checked manually on
the dev system.
---------
Signed-off-by: mqf20 <mingqingfoo@gmail.com>
Co-authored-by: Tim Möhlmann <tim+github@zitadel.com>
### Definition of Ready
This PR introduces a redirect_uri decoding step (url.QueryUnescape) in
the authorization request validation logic.
Libraries such as
[golang.org/x/oauth2](https://cs.opensource.google/go/x/oauth2/+/refs/tags/v0.30.0:oauth2.go;l=184)
automatically encode the redirect_uri using url.Values.Encode(). This
means the incoming URI is percent-encoded (e.g.,
https%3A%2F%2Fclient.example.com%2Fcallback), and the server must decode
it before performing string comparisons.
- [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.
Co-authored-by: sianao <me@sianao.site>
### Context
While implementing the Storage interface, I discovered that several
parameter names were misleading:
- Parameters named `refreshTokenID` and `newRefreshTokenID` actually
contain the full token values, not IDs
- This naming inconsistency caused confusion about what values should be
passed/returned
- The example implementations already use the semantically correct names
(`refreshToken`, `newRefreshToken`), creating a mismatch with the
interface definition
## Solution
This PR aligns the interface parameter names with their actual purpose
and with the existing example implementations.
## Changes
1. **Storage interface parameter renames:**
- `TokenRequestByRefreshToken`: `refreshTokenID` → `refreshToken`
- `CreateAccessAndRefreshTokens`: `newRefreshTokenID` →
`newRefreshToken`
2. **Improved code readability in token.go:**
- Made bare returns explicit for better clarity
- Added documentation explaining the token creation flow
- Clarified why `CreateAccessToken` also returns refresh tokens
## Impact
- **Breaking change**: No - these are parameter name changes in the
interface definition only
- **Behavior change**: No - all logic remains unchanged
- **Documentation**: Improved with clearer parameter names and added
explanations
## Testing
- Ran existing tests (some timing-related test failures are pre-existing
and unrelated to these changes)
- Verified example implementations already use the new parameter names
### Definition of Ready
- [X] I am happy with the code
- [X] Short description of the feature/issue is added in the pr
description
- [ ] PR is linked to the corresponding user story
- [ ] Acceptance criteria are met
- [ ] All open todos and follow ups are defined in a new ticket and
justified
- [ ] 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
- [ ] Critical parts are tested automatically
- [ ] Where possible E2E tests are implemented
- [X] Documentation/examples are up-to-date
- [ ] All non-functional requirements are met
- [ ] Functionality of the acceptance criteria is checked manually on
the dev system.
This PR makes the default Authorized Party check in `rp.VerifyIDToken`
optional by adding an options parameter for dynamic verification
functions. This check is meant to be an optional validation requirement,
so some providers (including GCP) do not adhere to it.
See https://github.com/zitadel/oidc/issues/405 for more context.
Closes https://github.com/zitadel/oidc/issues/405
### 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.
# Context
PR https://github.com/zitadel/oidc/pull/754 has introduced the optional
logout hint and UI locales to the end session request. However, while
working on https://github.com/zitadel/zitadel/pull/10039 , I have
noticed that the integration tests on Zitadel side call
`relying_party.EndSession()` without the possibility of specifying any
logout hint nor ui locales.
This PR adds these 2 parameters to `relying_party.EndSession()`
function.
* pkg/http: Add `secureCookieFunc` field to CookieHandler.
Signed-off-by: Mark Laing <mark.laing@canonical.com>
* pkg/http: Add `IsRequestAware` method CookieHandler.
Signed-off-by: Mark Laing <mark.laing@canonical.com>
* pkg/http: Use `secureCookieFunc` when checking a cookie (if set).
Signed-off-by: Mark Laing <mark.laing@canonical.com>
* pkg/http: Error on `SetCookie` if cookie handler is request aware.
Signed-off-by: Mark Laing <mark.laing@canonical.com>
* pkg/http: Add method to set request aware cookies.
Signed-off-by: Mark Laing <mark.laing@canonical.com>
* pkg/http: Add function to create a new request aware cookie handler.
Signed-off-by: Mark Laing <mark.laing@canonical.com>
* pkg/client/rp: Update `trySetStateCookie` function signature.
Use `SetRequestAwareCookie` if the cookie handle is request aware.
This function signature can be updated because it is not exported.
Signed-off-by: Mark Laing <mark.laing@canonical.com>
* pkg/client/rp: Add `GenerateAndStoreCodeChallengeWithRequest` function.
It's not possible to add a `http.Request` argument to
`GenerateAndStoreCodeChallenge` as this would be a breaking change.
Instead, add a new function that accepts a request argument and call
`SetRequestAwareCookie` here.
Signed-off-by: Mark Laing <mark.laing@canonical.com>
* pkg/client/rp: Update PKCE logic to pass request if required by cookie handler.
Signed-off-by: Mark Laing <mark.laing@canonical.com>
* pkg/http: Don't set MaxAge if cookie handler is request aware.
The securecookie field can be nil. Expect the caller to set max age on
the securecookie returned by the secureCookieFunc.
Signed-off-by: Mark Laing <mark.laing@canonical.com>
* pkg/client: Add integration tests for request aware cookie handling.
Adds a new type `cookieSpec` which is accepted as an argument to
`RunAuthorizationCodeFlow`. `TestRelyingPartySession` now runs with
`wrapServer` true/false and with two cookie handlers, one static and one
request aware.
The request aware handler extracts encryption keys from a secret using a
salt from a "login_id" cookie.
Signed-off-by: Mark Laing <mark.laing@canonical.com>
---------
Signed-off-by: Mark Laing <mark.laing@canonical.com>
- Introduced CodeResponseType struct to encapsulate response data.
- Added handleFormPostResponse and handleRedirectResponse functions to manage different response modes.
- Created BuildAuthResponseCodeResponsePayload and BuildAuthResponseCallbackURL functions for better modularity in response generation.
Finally the RFC Best Current Practice for OAuth 2.0 Security has been approved.
According to the RFC:
> Authorization servers MUST support PKCE [RFC7636].
>
> If a client sends a valid PKCE code_challenge parameter in the authorization request, the authorization server MUST enforce the correct usage of code_verifier at the token endpoint.
Isn’t it time we strengthen PKCE support a bit more?
This PR updates the logic so that PKCE is always verified, even when the Auth Method is not "none".
* chore: updating go to 1.24
* fixup! chore: updating go to 1.24
* fixup! fixup! chore: updating go to 1.24
* fix device test (drop read error)
* drop older go versions
* drop unrelated formatter changes
---------
Co-authored-by: Iraq Jaber <IraqJaber@gmail.com>
Co-authored-by: Tim Möhlmann <tim+github@zitadel.com>
* add default signature algorithm
* implements session_state in auth_request.go
* add test
* Update pkg/op/auth_request.go
link to the standard
Co-authored-by: Tim Möhlmann <muhlemmer@gmail.com>
* add check_session_iframe
---------
Co-authored-by: Tim Möhlmann <tim+github@zitadel.com>
Co-authored-by: Tim Möhlmann <muhlemmer@gmail.com>
* feat(oidc): return defined error when discovery failed
* Use errors.Join() to join errors
Co-authored-by: Tim Möhlmann <muhlemmer@gmail.com>
* Remove unnecessary field
Co-authored-by: Tim Möhlmann <muhlemmer@gmail.com>
* Fix order and message
Co-authored-by: Tim Möhlmann <muhlemmer@gmail.com>
* Fix error order
* Simplify error assertion
Co-authored-by: Tim Möhlmann <muhlemmer@gmail.com>
---------
Co-authored-by: Tim Möhlmann <muhlemmer@gmail.com>
This changes removes the requirement of the openid scope to be set for all token requests.
As this library also support OAuth2-only authentication mechanisms we still want to sanitize requested scopes, but not enforce the openid scope.
Related to https://github.com/zitadel/zitadel/discussions/8068
This change requires an additional argument to the op.RegisterLegacyServer constructor which passes the Authorize Callback Handler.
This allows implementations to use their own handler instead of the one provided by the package.
The current handler is exported for legacy behavior.
This change is not considered breaking, as RegisterLegacyServer is flagged experimental.
Related to https://github.com/zitadel/zitadel/issues/6882
* feat(rp): to use signing algorithms from discovery configuration (#574)
* feat: WithSigningAlgsFromDiscovery to verify IDTokenVerifier() behavior in RP with
This change updates to go-jose v4, which was a new major release.
jose.ParseSigned now expects the supported signing algorithms to be passed, on which we previously did our own check. As they use a dedicated type for this, the slice of string needs to be converted. The returned error also need to be handled in a non-standard way in order to stay compatible.
For OIDC v4 we should use the jose.SignatureAlgorithm type directly and wrap errors, instead of returned static defined errors.
Closes#583
