Improve integration tests:
- spliting the tests in TokenExchange to isolated instances and in
parallel
- corrected some test structure so that the check for Details is no done
anymore if the test already failed
- replace required-calls with assert-calls to not stop the testing
- add gofakeit for application, project and usernames(emails)
- add eventually checks for testing in actions v2, so the request only
get called when the execution is defined
- check for length of results in list/search endpoints to avoid index
errors
# Which Problems Are Solved
Calls with tokens issued through JWT Profile or Client Credentials
Grants were no longer possible and threw a "could not read projectid by
clientid (AUTH-GHpw2)" error.
ZITADEL checks the allowed origins of an application and load its
projectID into the context on any API call.
Tokens from service accounts did not contain any clientID and therefore
never did that check.
But due to a change in https://github.com/zitadel/zitadel/pull/8580,
were the service user id was set as client_id in the OIDC session to fix
the introspection response
(https://github.com/zitadel/zitadel/issues/8590).
# How the Problems Are Solved
- Check if the project and origin were retrieved and only then check the
origins
# Additional Changes
None.
# Additional Context
- closes https://github.com/zitadel/zitadel/issues/8676
- relates to https://github.com/zitadel/zitadel/pull/8580 (released on
2.62.0)
- relates to https://github.com/zitadel/zitadel/issues/8590
# Which Problems Are Solved
We identified the need of caching.
Currently we have a number of places where we use different ways of
caching, like go maps or LRU.
We might also want shared chaches in the future, like Redis-based or in
special SQL tables.
# How the Problems Are Solved
Define a generic Cache interface which allows different implementations.
- A noop implementation is provided and enabled as.
- An implementation using go maps is provided
- disabled in defaults.yaml
- enabled in integration tests
- Authz middleware instance objects are cached using the interface.
# Additional Changes
- Enabled integration test command raceflag
- Fix a race condition in the limits integration test client
- Fix a number of flaky integration tests. (Because zitadel is super
fast now!) 🎸🚀
# Additional Context
Related to https://github.com/zitadel/zitadel/issues/8648
# Which Problems Are Solved
ZITADEL's user account deactivation mechanism did not work correctly
with service accounts. Deactivated service accounts retained the ability
to request tokens, which could lead to unauthorized access to
applications and resources.
# How the Problems Are Solved
Additionally to checking the user state on the session API and login UI,
the state is checked on all oidc session methods resulting in a new
token or when returning the user information (userinfo, introspection,
id_token / access_token and saml attributes)
# Which Problems Are Solved
ZITADEL's user grants deactivation mechanism did not work correctly.
Deactivated user grants were still provided in token, which could lead
to unauthorized access to applications and resources.
Additionally, the management and auth API always returned the state as
active or did not provide any information about the state.
# How the Problems Are Solved
- Correctly check the user grant state on active for tokens and user
information (userinfo, introspection, saml attributes)
- Map state in API and display in Console
# Which Problems Are Solved
In Zitadel, even after an organization is deactivated, associated
projects, respectively their applications remain active. Users across
other organizations can still log in and access through these
applications, leading to unauthorized access.
Additionally, if a project was deactivated access to applications was
also still possible.
# How the Problems Are Solved
- Correctly check the status of the organization and related project.
(Corresponding functions have been renamed to `Active...`)
# Which Problems Are Solved
When the `openid` scope was not requested, as is possible in machine
authentication, we didn't set the `sub` (subject) claim to tokens and
possibly also userInfo and introspection.
This fix always sets the `sub` claim for all cases.
# How the Problems Are Solved
Set the `Subject` field to regardless of passed scopes.
# Additional Changes
- none
# Additional Context
According to standards:
- [RFC9068 - JSON Web Token (JWT) Profile for OAuth 2.0 Access
Tokens](https://datatracker.ietf.org/doc/html/rfc9068#name-data-structure)
this claim is **required**.
- [RFC7667 - OAuth 2.0 Token
Introspection](https://datatracker.ietf.org/doc/html/rfc7662#section-2.2)
the claim is optional, however there is no correlation to the `openid`
or OpenID Connect. Therefore it doesn't harm to always return this
claim.
- [OpenID connect, User Info
Response](https://openid.net/specs/openid-connect-core-1_0.html#UserInfoResponse):
"The sub (subject) Claim **MUST** always be returned in the UserInfo
Response."
Closes https://github.com/zitadel/zitadel/issues/8591
# Which Problems Are Solved
Improve performance by removing a GetUserByID call. The call also
executed a Trigger on projections, which significantly impacted
concurrent requests.
# How the Problems Are Solved
Token creation needs information from the user, such as the resource
owner and access token type.
For client credentials this is solved in a single search. By getting the
user by username (`client_id`), the user details and secret were
obtained in a single query. After that verification and token creation
can proceed. For JWT profile it is a bit more complex. We didn't know
anything about the user until after JWT verification.
The verification did a query for the AuthN key and after that we did a
GetUserByID to get remaining details.
This change uses a joined query when the OIDC library calls the
`GetKeyByIDAndClientID` method on the token storage. The found user
details are set to the verifieer object and returned after verification
is completed.
It is safe because the `jwtProfileKeyStorage` is a single-use object as
a wrapper around `query.Queries`.
This way getting the public key and user details are obtained in a
single query.
# Additional Changes
- Correctly set the `client_id` field with machine's username.
# Additional Context
- Related to: https://github.com/zitadel/zitadel/issues/8352
# Which Problems Are Solved
Return the user's project roles when the
`urn:zitadel:iam:org:projects:roles` scope is requested.
We alreayd returned it for access tokens, now also ID tokens.
# How the Problems Are Solved
Set `idTokenRoleAssertion` to `true` when calling
`accessTokenResponseFromSession` for service users. This parameter is
normally set to the client config. However, service user authentication
does not have a client.
# Additional Changes
- none
# Additional Context
- Introduced in https://github.com/zitadel/zitadel/pull/8046
- Closes https://github.com/zitadel/zitadel/issues/8107
Co-authored-by: Livio Spring <livio.a@gmail.com>
# Which Problems Are Solved
Use a single server instance for API integration tests. This optimizes
the time taken for the integration test pipeline,
because it allows running tests on multiple packages in parallel. Also,
it saves time by not start and stopping a zitadel server for every
package.
# How the Problems Are Solved
- Build a binary with `go build -race -cover ....`
- Integration tests only construct clients. The server remains running
in the background.
- The integration package and tested packages now fully utilize the API.
No more direct database access trough `query` and `command` packages.
- Use Makefile recipes to setup, start and stop the server in the
background.
- The binary has the race detector enabled
- Init and setup jobs are configured to halt immediately on race
condition
- Because the server runs in the background, races are only logged. When
the server is stopped and race logs exist, the Makefile recipe will
throw an error and print the logs.
- Makefile recipes include logic to print logs and convert coverage
reports after the server is stopped.
- Some tests need a downstream HTTP server to make requests, like quota
and milestones. A new `integration/sink` package creates an HTTP server
and uses websockets to forward HTTP request back to the test packages.
The package API uses Go channels for abstraction and easy usage.
# Additional Changes
- Integration test files already used the `//go:build integration`
directive. In order to properly split integration from unit tests,
integration test files need to be in a `integration_test` subdirectory
of their package.
- `UseIsolatedInstance` used to overwrite the `Tester.Client` for each
instance. Now a `Instance` object is returned with a gRPC client that is
connected to the isolated instance's hostname.
- The `Tester` type is now `Instance`. The object is created for the
first instance, used by default in any test. Isolated instances are also
`Instance` objects and therefore benefit from the same methods and
values. The first instance and any other us capable of creating an
isolated instance over the system API.
- All test packages run in an Isolated instance by calling
`NewInstance()`
- Individual tests that use an isolated instance use `t.Parallel()`
# Additional Context
- Closes#6684
- https://go.dev/doc/articles/race_detector
- https://go.dev/doc/build-cover
---------
Co-authored-by: Stefan Benz <46600784+stebenz@users.noreply.github.com>
# Which Problems Are Solved
Float64 which was used for the event.Position field is [not precise in
go and gets rounded](https://github.com/golang/go/issues/47300). This
can lead to unprecies position tracking of events and therefore
projections especially on cockcoachdb as the position used there is a
big number.
example of a unprecies position:
exact: 1725257931223002628
float64: 1725257931223002624.000000
# How the Problems Are Solved
The float64 was replaced by
[github.com/jackc/pgx-shopspring-decimal](https://github.com/jackc/pgx-shopspring-decimal).
# Additional Changes
Correct behaviour of makefile for load tests.
Rename `latestSequence`-queries to `latestPosition`
# Which Problems Are Solved
The end_session_endpoint currently always requires the userAgent cookie
to be able to terminate session created through the hosted login UI.
Only tokens issued through the Login V2 can be used to directly
terminate a specific session and without the need of a cookie.
This PR adds the possibility to terminate a single V1 session or all V1
sessions belonging to the same user agent without the need of the
userAgent cookie by providing an id_token as `id_token_hint` which
contains the id of a V1 session as `sid`.
# How the Problems Are Solved
- #8525 added the `sid` claim for id_tokens issued through the login UI
- The `sid` can now be checked for the `V1_` prefix and queries for
either the userAgentID and depending on the
`OIDCSingleV1SessionTermination` flag all userIDs of active session from
the same user agent id
- The `OIDCSingleV1SessionTermination` flag is added with default value
false to keep the existing behavior of terminating all sessions even in
case of providing an id_token_hint
# Additional Changes
- pass `context.Context` into session view functions for querying the
database with that context
# Additional Context
- relates to #8499
- closes#8501
# Which Problems Are Solved
id_tokens issued for auth requests created through the login UI
currently do not provide a sid claim.
This is due to the fact that (SSO) sessions for the login UI do not have
one and are only computed by the userAgent(ID), the user(ID) and the
authentication checks of the latter.
This prevents client to track sessions and terminate specific session on
the end_session_endpoint.
# How the Problems Are Solved
- An `id` column is added to the `auth.user_sessions` table.
- The `id` (prefixed with `V1_`) is set whenever a session is added or
updated to active (from terminated)
- The id is passed to the `oidc session` (as v2 sessionIDs), to expose
it as `sid` claim
# Additional Changes
- refactored `getUpdateCols` to handle different column value types and
add arguments for query
# Additional Context
- closes#8499
- relates to #8501
# Which Problems Are Solved
Do not push secret succeeded and failed events for API and OIDC clients
on the introspection endpoint.
On instances where introspection was fequently called, the pushed events
created issues on duplicate primary keys, due to collisions on the
`sequence` column in the eventstore. As the event pusher retries on this
collision and we pushed above mentioned events async, it would create a
backpressure of concurrent pushers and effectively cripple an instance.
We considered that pushing these events have little value with regards
to the audit trail, as we do not push similar events when client
assertion is used. Also, before #7657 the events were defined, but not
pushed.
# How the Problems Are Solved
- Removed API secret check succeeded and faild event definitions
- Removed OIDC secret check succeeded and faild event definitions
- Push only Hash Updated event when needed
# Additional Changes
- None
# Additional Context
- Fixes https://github.com/zitadel/zitadel/issues/8479
- Closes https://github.com/zitadel/zitadel/issues/8430
- Intoduced in https://github.com/zitadel/zitadel/pull/7657
# Which Problems Are Solved
Use web keys, managed by the `resources/v3alpha/web_keys` API, for OIDC
token signing and verification,
as well as serving the public web keys on the jwks / keys endpoint.
Response header on the keys endpoint now allows caching of the response.
This is now "safe" to do since keys can be created ahead of time and
caches have sufficient time to pickup the change before keys get
enabled.
# How the Problems Are Solved
- The web key format is used in the `getSignerOnce` function in the
`api/oidc` package.
- The public key cache is changed to get and store web keys.
- The jwks / keys endpoint returns the combined set of valid "legacy"
public keys and all available web keys.
- Cache-Control max-age default to 5 minutes and is configured in
`defaults.yaml`.
When the web keys feature is enabled, fallback mechanisms are in place
to obtain and convert "legacy" `query.PublicKey` as web keys when
needed. This allows transitioning to the feature without invalidating
existing tokens. A small performance overhead may be noticed on the keys
endpoint, because 2 queries need to be run sequentially. This will
disappear once the feature is stable and the legacy code gets cleaned
up.
# Additional Changes
- Extend legacy key lifetimes so that tests can be run on an existing
database with more than 6 hours apart.
- Discovery endpoint returns all supported algorithms when the Web Key
feature is enabled.
# Additional Context
- Closes https://github.com/zitadel/zitadel/issues/8031
- Part of https://github.com/zitadel/zitadel/issues/7809
- After https://github.com/zitadel/oidc/pull/637
- After https://github.com/zitadel/oidc/pull/638
# Which Problems Are Solved
Currently the OIDC API of ZITADEL only prints parent errors to the logs.
Where 4xx status are typically warn level and 5xx error level. This
makes it hard to debug certain errors for client in multi-instance
environments like ZITADEL cloud, where there is no direct access to
logs. In case of support requests we often can't correlate past
log-lines to the error that was reported.
This change adds the possibility to return the parent error in the
response to the OIDC client. For the moment this only applies to JSON
body responses, not error redirects to the RP.
# How the Problems Are Solved
- New instance-level feature flag: `debug_oidc_parent_error`
- Use the new `WithReturnParentToClient()` function from the oidc lib
introduced in https://github.com/zitadel/oidc/pull/629 for all cases
where `WithParent` was already used and the request context is
available.
# Additional Changes
none
# Additional Context
- Depends on: https://github.com/zitadel/oidc/pull/629
- Related to: https://github.com/zitadel/zitadel/issues/8362
---------
Co-authored-by: Livio Spring <livio.a@gmail.com>
# Which Problems Are Solved
Implement a new API service that allows management of OIDC signing web
keys.
This allows users to manage rotation of the instance level keys. which
are currently managed based on expiry.
The API accepts the generation of the following key types and
parameters:
- RSA keys with 2048, 3072 or 4096 bit in size and:
- Signing with SHA-256 (RS256)
- Signing with SHA-384 (RS384)
- Signing with SHA-512 (RS512)
- ECDSA keys with
- P256 curve
- P384 curve
- P512 curve
- ED25519 keys
# How the Problems Are Solved
Keys are serialized for storage using the JSON web key format from the
`jose` library. This is the format that will be used by OIDC for
signing, verification and publication.
Each instance can have a number of key pairs. All existing public keys
are meant to be used for token verification and publication the keys
endpoint. Keys can be activated and the active private key is meant to
sign new tokens. There is always exactly 1 active signing key:
1. When the first key for an instance is generated, it is automatically
activated.
2. Activation of the next key automatically deactivates the previously
active key.
3. Keys cannot be manually deactivated from the API
4. Active keys cannot be deleted
# Additional Changes
- Query methods that later will be used by the OIDC package are already
implemented. Preparation for #8031
- Fix indentation in french translation for instance event
- Move user_schema translations to consistent positions in all
translation files
# Additional Context
- Closes#8030
- Part of #7809
---------
Co-authored-by: Elio Bischof <elio@zitadel.com>
# Which Problems Are Solved
There was no default configuration for `DeviceAuth`, which makes it
impossible to override by environment variables.
Additionally, a custom `CharAmount` value would overwrite also the
`DashInterval`.
# How the Problems Are Solved
- added to defaults.yaml
- fixed customization
# Additional Changes
None.
# Additional Context
- noticed during a customer request
# Which Problems Are Solved
ZITADEL currently selects the instance context based on a HTTP header
(see https://github.com/zitadel/zitadel/issues/8279#issue-2399959845 and
checks it against the list of instance domains. Let's call it instance
or API domain.
For any context based URL (e.g. OAuth, OIDC, SAML endpoints, links in
emails, ...) the requested domain (instance domain) will be used. Let's
call it the public domain.
In cases of proxied setups, all exposed domains (public domains) require
the domain to be managed as instance domain.
This can either be done using the "ExternalDomain" in the runtime config
or via system API, which requires a validation through CustomerPortal on
zitadel.cloud.
# How the Problems Are Solved
- Two new headers / header list are added:
- `InstanceHostHeaders`: an ordered list (first sent wins), which will
be used to match the instance.
(For backward compatibility: the `HTTP1HostHeader`, `HTTP2HostHeader`
and `forwarded`, `x-forwarded-for`, `x-forwarded-host` are checked
afterwards as well)
- `PublicHostHeaders`: an ordered list (first sent wins), which will be
used as public host / domain. This will be checked against a list of
trusted domains on the instance.
- The middleware intercepts all requests to the API and passes a
`DomainCtx` object with the hosts and protocol into the context
(previously only a computed `origin` was passed)
- HTTP / GRPC server do not longer try to match the headers to instances
themself, but use the passed `http.DomainContext` in their interceptors.
- The `RequestedHost` and `RequestedDomain` from authz.Instance are
removed in favor of the `http.DomainContext`
- When authenticating to or signing out from Console UI, the current
`http.DomainContext(ctx).Origin` (already checked by instance
interceptor for validity) is used to compute and dynamically add a
`redirect_uri` and `post_logout_redirect_uri`.
- Gateway passes all configured host headers (previously only did
`x-zitadel-*`)
- Admin API allows to manage trusted domain
# Additional Changes
None
# Additional Context
- part of #8279
- open topics:
- "single-instance" mode
- Console UI
# Which Problems Are Solved
The v2beta services are stable but not GA.
# How the Problems Are Solved
The v2beta services are copied to v2. The corresponding v1 and v2beta
services are deprecated.
# Additional Context
Closes#7236
---------
Co-authored-by: Elio Bischof <elio@zitadel.com>
# Which Problems Are Solved
We found multiple cases where either the error was not properly handled,
which led to panics.
# How the Problems Are Solved
Handle the errors.
# Additional Changes
None.
# Additional Context
- noticed internally
# Which Problems Are Solved
When we switched to V2 tokens (#7822), the user agent was incorrectly
set for sessions created though the login UI.
Additionally, when calling the ListMyUserSessions from the AuthService,
any session without the fingerprint ID (e.g. created through the session
API) would be listed.
# How the Problems Are Solved
- Use the intended ID of the user agent (fingerprint)
- Ignore empty user agent IDs when listing the user sessions
# Additional Changes
None.
# Additional Context
- relates #7822
- closes#8213
# Which Problems Are Solved
Zitadel never stored or returned the requested `response_mode` in oidc
Auth Requests. This caused the oidc library to fallback to the default
based on the response_type.
# How the Problems Are Solved
- Store the `response_mode` in the Auth request repo
- Store the `response_mode` in the Auth request v2 events
- Return the `resonse_mode` from the Auth Request v1 and v2
`ResponseMode()` methods. (Was hard-coded to an empty string)
# Additional Changes
- Populate the `response_modes_supported` to the oidc Discovery
Configuration. When it was empty, the standard specifies the default of
`query` and `fragment`. However, our oidc library also supports
`form_post` and by this fix, zitadel now also supports this.
# Additional Context
- Closes#6586
- Reported
https://discord.com/channels/927474939156643850/1151508313717084220
---------
Co-authored-by: Livio Spring <livio.a@gmail.com>
# Which Problems Are Solved
Introduced with #6909, the authentication check (API client) and the
token verification on the introspection endpoint where parallelized to
improve performance. Only the first error would be considered and
returned (and the second completely ignored).
This could lead to situations where both the client authentication and
token verification failed and the response would result in a 200 OK with
`active: false`.
# How the Problems Are Solved
- The client authentication check error will always be prioritized.
- An error in the token check will no longer terminate the client
authentication check.
# Additional Changes
None.
# Additional Context
- reported in Discord:
https://discord.com/channels/927474939156643850/1242770807105781760
# Which Problems Are Solved
An admin / application might want to be able to reduce the amount of
roles returned in the token, for example if a user is granted to many
organizations or for specific cases where the application want to narrow
down the access for that token to a specific organization or multiple.
This can now be achieved by providing a scope with the id of the
organization, resp. multiple scopes for every organization, which should
be included.
```
urn:zitadel:iam:org:roles🆔{orgID}
```
**Note:** the new scope does not work when Introspection / Userinfo are
set to legacy mode.
# How the Problems Are Solved
The user info query now has two variants:
1. Variant that returns all organization authorization grants if the new
scope wasn't provided for backward compatibility.
2. Variant that filters the organizations based on the IDs passed in one
or more of the above scopes and returns only those authorization grants.
The query is defined as a `text/template` and both variants are rendered
once in package `init()`.
# Additional Changes
- In the integration tests `assertProjectRoleClaims` now also checks the
org IDs in the roles.
# Additional Context
- Closes#7996
# Which Problems Are Solved
Request to the ZITADEL API currently require multi factor authentication
if the user has set up any second factor.
However, the login UI will only prompt the user to check factors that
are allowed by the login policy.
This can lead to situations, where the user has set up a factor (e.g.
some OTP) which was not allowed by the policy, therefore will not have
to verify the factor, the ZITADEL API however will require the check
since the user has set it up.
# How the Problems Are Solved
The requirement for multi factor authentication based on the user's
authentication methods is removed when accessing the ZITADEL APIs.
Those requests will only require MFA in case the login policy does so
because of `requireMFA` or `requireMFAForLocalUsers`.
# Additional Changes
None.
# Additional Context
- a customer reached out to support
- discussed internally
- relates #7822
- backport to 2.53.x
# Which Problems Are Solved
Introduced with #7822 the access token response incorrectly returned the
`state` parameter.
# How the Problems Are Solved
The `state` will only be returned for access token responses in an
implicit_flow.
# Additional Changes
None.
# Additional Context
- relates to #7822
- relates to
https://github.com/zitadel/oidc/issues/446#issuecomment-2144999644
- backport to 2.53.x
---------
Co-authored-by: Tim Möhlmann <tim+github@zitadel.com>
# Which Problems Are Solved
When an error occurred during the oidc session creation from
client_credentials or jwt_profile, the error was ignored.
# How the Problems Are Solved
Return the error.
# Additional Changes
None.
# Additional Context
- relates to #7822
- noticed internally
- backport to 2.53.x
# Which Problems Are Solved
During tests of 2.53.3 we noticed that in cases where the
`idTokenRoleAssertion` was disabled, claims set in the
preAccessTokenTrigger where also set in the id_token.
# How the Problems Are Solved
The userinfo of the id_token now uses a correct copy of their own.
# Additional Changes
None.
# Additional Context
- relates to #7822
- relates to #8046
# Which Problems Are Solved
After deployment of 2.53.x, customers noted that the roles claims where
always present in the tokens even if the corresponding option on the
client (accessTokenRoleAssertion, idTokenRoleAsseriton) was disabled.
Only the project flag (assertRolesOnAuthentication) would be considered.
Further it was noted, that the action on the preAccessTokenCreation
trigger would not be executed.
Additionally, while testing those issues we found out, that the user
information (name, givenname, family name, ...) where always present in
the id_token even if the option (idTokenUserInfo) was not enabled.
# How the Problems Are Solved
- The `getUserinfoOnce` which was used for access and id_tokens is
refactored to `getUserInfo` and no longer only queries the info once
from the database, but still provides a mechanism to be reused for
access and id_token where the corresponding `roleAssertion` and action
`triggerType` can be passed.
- `userInfo` on the other hand now directly makes sure the information
is only queried once from the database. Role claims are asserted every
time and action triggers are executed on every call.
- `userInfo` now also checks if the profile information need to be
returned.
# Additional Changes
None.
# Additional Context
- relates to #7822
- reported by customers
# Which Problems Are Solved
Introspection errors such as invalid audience and errors in the login UI
such as invalid user agents where all logged as severity error.
# How the Problems Are Solved
Log level for both general loggers is changed to `info`.
# Additional Changes
None
# Additional Context
- internal discussion
# Which Problems Are Solved
As already mentioned and (partially) fixed in #7992 we discovered,
issues with v2 tokens that where obtained through an IDP, with
passwordless authentication or with password authentication (wihtout any
2FA set up) using the v1 login for zitadel API calls
- (Previous) authentication through an IdP is now correctly treated as
auth method in case of a reauth even when the user is not redirected to
the IdP
- There were some cases where passwordless authentication was
successfully checked but not correctly set as auth method, which denied
access to ZITADEL API
- Users with password and passwordless, but no 2FA set up which
authenticate just wich password can access the ZITADEL API again
Additionally while testing we found out that because of #7969 the login
UI could completely break / block with the following error:
`sql: Scan error on column index 3, name "state": converting NULL to
int32 is unsupported (Internal)`
# How the Problems Are Solved
- IdP checks are treated the same way as other factors and it's ensured
that a succeeded check within the configured timeframe will always
provide the idp auth method
- `MFATypesAllowed` checks for possible passwordless authentication
- As with the v1 login, the token check now only requires MFA if the
policy is set or the user has 2FA set up
- UserAuthMethodsRequirements now always uses the correctly policy to
check for MFA enforcement
- `State` column is handled as nullable and additional events set the
state to active (as before #7969)
# Additional Changes
- Console now also checks for 403 (mfa required) errors (e.g. after
setting up the first 2FA in console) and redirects the user to the login
UI (with the current id_token as id_token_hint)
- Possible duplicates in auth methods / AMRs are removed now as well.
# Additional Context
- Bugs were introduced in #7822 and # and 7969 and only part of a
pre-release.
- partially already fixed with #7992
- Reported internally.
# Which Problems Are Solved
After https://github.com/zitadel/zitadel/pull/7822 was merged we
discovered that
v2 tokens that where obtained through an IDP using the v1 login, can't
be used for
zitadel API calls.
- Because we used to store the AMR claim on the auth request, but
internally use the domain.UserAuthMethod type. AMR has no notion of an
IDP login, so that "factor" was lost
during conversion. Rendering those v2 tokens invalid on the zitadel API.
- A wrong check on machine user tokens falsly allowed some tokens to be
valid
- The client ID was set to tokens from client credentials and JWT
profile, which made client queries fail in the validation middleware.
The middleware expects client ID unset for machine users.
# How the Problems Are Solved
Store the domain.AuthMethods directly in the auth requests and session,
instead of using AMR claims with lossy conversion.
- IDPs have seperate auth method, which is not an AMR claim
- Machine users are treated specialy, eg auth methods are not required.
- Do not set the client ID for client credentials and JWT profile
# Additional Changes
Cleaned up mostly unused `oidc.getInfoFromRequest()`.
# Additional Context
- Bugs were introduced in https://github.com/zitadel/zitadel/pull/7822
and not yet part of a release.
- Reported internally.
* implement code exchange
* port tokenexchange to v2 tokens
* implement refresh token
* implement client credentials
* implement jwt profile
* implement device token
* cleanup unused code
* fix current unit tests
* add user agent unit test
* unit test domain package
* need refresh token as argument
* test commands create oidc session
* test commands device auth
* fix device auth build error
* implicit for oidc session API
* implement authorize callback handler for legacy implicit mode
* upgrade oidc module to working draft
* add missing auth methods and time
* handle all errors in defer
* do not fail auth request on error
the oauth2 Go client automagically retries on any error. If we fail the auth request on the first error, the next attempt will always fail with the Errors.AuthRequest.NoCode, because the auth request state is already set to failed.
The original error is then already lost and the oauth2 library does not return the original error.
Therefore we should not fail the auth request.
Might be worth discussing and perhaps send a bug report to Oauth2?
* fix code flow tests by explicitly setting code exchanged
* fix unit tests in command package
* return allowed scope from client credential client
* add device auth done reducer
* carry nonce thru session into ID token
* fix token exchange integration tests
* allow project role scope prefix in client credentials client
* gci formatting
* do not return refresh token in client credentials and jwt profile
* check org scope
* solve linting issue on authorize callback error
* end session based on v2 session ID
* use preferred language and user agent ID for v2 access tokens
* pin oidc v3.23.2
* add integration test for jwt profile and client credentials with org scopes
* refresh token v1 to v2
* add user token v2 audit event
* add activity trigger
* cleanup and set panics for unused methods
* use the encrypted code for v1 auth request get by code
* add missing event translation
* fix pipeline errors (hopefully)
* fix another test
* revert pointer usage of preferred language
* solve browser info panic in device auth
* remove duplicate entries in AMRToAuthMethodTypes to prevent future `mfa` claim
* revoke v1 refresh token to prevent reuse
* fix terminate oidc session
* always return a new refresh toke in refresh token grant
---------
Co-authored-by: Livio Spring <livio.a@gmail.com>
We are trying to reproduce a few 500 responses we observe on zitadel cloud's token endpoint.
As in the past these were caused by wrongly encoded or encrypted refresh tokens, I created a integration test which tries to reproduce 500 errors by sending invalid refresh tokens.
The added test does not reproduce 500s, all returned errors are in the 400 range as they should. However, as the test is already written, we might as well include them.
Related to #7765
Co-authored-by: Livio Spring <livio.a@gmail.com>
* fix(oidc): roles in userinfo for client credentials token
When tokens were obtained using the client credentials grant,
with audience and role scopes, userinfo would not return the role claims. This had multiple causes:
1. There is no auth request flow, so for legacy userinfo project data was never attached to the token
2. For optimized userinfo, there is no client ID that maps to an application. The client ID for client credentials is the machine user's name. There we can't obtain a project ID. When the project ID remained empty, we always ignored the roleAudience.
This PR fixes situation 2, by always taking the roleAudience into account, even when the projectID is empty. The code responsible for the bug is also refactored to be more readable and understandable, including additional godoc.
The fix only applies to the optimized userinfo code introduced in #7706 and released in v2.50 (currently in RC). Therefore it can't be back-ported to earlier versions.
Fixes#6662
* chore(deps): update all go deps (#7764)
This change updates all go modules, including oidc, a major version of go-jose and the go 1.22 release.
* Revert "chore(deps): update all go deps" (#7772)
Revert "chore(deps): update all go deps (#7764)"
This reverts commit 6893e7d060.
---------
Co-authored-by: Livio Spring <livio.a@gmail.com>
* feat(oidc): optimize the userinfo endpoint
* store project ID in the access token
* query for projectID if not in token
* add scope based tests
* Revert "store project ID in the access token"
This reverts commit 5f0262f239.
* query project role assertion
* use project role assertion setting to return roles
* workaround eventual consistency and handle PAT
* do not append empty project id
* fix(oidc): return bad request for base64 errors
We've recently noticed an increased amount of 500: internal server error status returns on zitadel cloud.
The source of these errors appear to be erroneous input in fields that are supposed to be bas64 formatted.
```
time=2024-04-08T14:05:47.600Z level=ERROR msg="request error" oidc_error.parent="ID=OIDC-AhX2u Message=Errors.Internal Parent=(illegal base64 data at input byte 8)" oidc_error.description=Errors.Internal oidc_error.type=server_error status_code=500
```
Within the possible code paths of the token endpoint there are a couple of uses of base64.Encoding.DecodeString of which a returned error was not properly wrapped, but returned as-is.
This causes the oidc error handler to return a 500 with the `OIDC-AhX2u` ID.
We were not able to pinpoint the exact errors that are happening to any one call of `DecodeString`.
This fix wraps all errors from `DecodeString` so that proper 400: bad request is returned with information about the error. Each wrapper now has an unique error ID, so that logs will contain the source of the error as well.
This bug was reported internally by the ops team.
* catch op.ErrInvalidRefreshToken
* fix: add resource owner as query for user v2 ListUsers and clean up deprecated attribute
* fix: add resource owner as query for user v2 ListUsers and clean up deprecated attribute
* fix: add resource owner as query for user v2 ListUsers and clean up deprecated attribute
* fix: review changes
* fix: review changes
* fix: review changes
* fix: review changes
* fix: add password change required to user v2 get and list
* fix: update unit tests for query side with new column and projection
* fix: change projection in setup steps
* fix: change projection in setup steps
* fix: remove setup step 25
* fix: add password_change_required into ListUsers response
* fix: correct SetUserPassword parameters
* fix: rollback to change setup instead of projection directly
* fix: rollback to change setup instead of projection directly
---------
Co-authored-by: Livio Spring <livio.a@gmail.com>
fix(oidc): define audience inside auth request instead off token creation
When using the v1 OIDC Code flow, tokens would not carry the correct audience when returned as JWT. This applies to access tokens as JWT and ID tokens.
Introspection would still show the correct audience.
This happened because project audience was appended at token creation time. This stored the appended audience, used later in introspection or token refresh. However, the OIDC library still only had a view of the original auth request with the original audience.
When signing JWTs it would use this outdated information.
This change moves audience modifications to the auth request creation. This is was already the way it was done for v2 login and now v1 follows the same method.
Co-authored-by: Livio Spring <livio.a@gmail.com>
