* fix(command): enforce permission check when issuing passkey enrollment codes
addUserPasskeyCode - behind CreatePasskeyRegistrationLink via
AddUserPasskeyCode / AddUserPasskeyCodeURLTemplate / AddUserPasskeyCodeReturn -
issued a passkey enrollment code without any authorization check.
The RPC's auth annotation carries user.passkey.write with no org_field, so the
API interceptor only verifies the caller's permission in the org taken from the
caller-supplied x-zitadel-orgid header, never the org that owns the target user.
The gRPC handlers additionally pass an empty resourceOwner, and the write model
lookup is instance-wide, so the code was minted against the victim's aggregate
regardless of org. An org owner of one organization could therefore issue a
passkey enrollment code for a user in another organization, enroll an
attacker-controlled authenticator and take over the account.
Authorize against the target user's actual resource owner, resolved from the
write model, before pushing the code requested event.
The check uses user.passkey.write - the permission the RPC already declares -
rather than user.credential.write used by the sibling RegisterUserPasskey. This
narrows the organization the permission is evaluated in without narrowing the
set of roles that may call the endpoint: IAM_USER_MANAGER holds
user.passkey.write but not user.credential.write, and would otherwise lose the
endpoint instance-wide.
The enrollment code stays a bearer credential by design, so the unauthenticated
login flows redeeming it are unaffected; issuance is the only place this can be
enforced.
Co-Authored-By: Claude Opus 5 <[email protected]>
* test(command): guard cross-org passkey enrollment code issuance
The existing AddUserPasskeyCode* tests all use an allowed permission check, so
none of them fails if the authorization is removed again.
Assert the denial path instead: with the caller passing an empty resourceOwner,
as the v2 gRPC handlers do, the check must receive the target user's real
resource owner resolved from the write model, and no event may be pushed.
Without the check in addUserPasskeyCode all three subtests fail, one of them on
an unexpected Push - a code would actually have been issued.
Co-Authored-By: Claude Opus 5 <[email protected]>
* test(user/v2): simulate cross-org passkey registration link attack
End-to-end reproduction against a live instance: an org owner of one
organization requests a passkey registration link, with the request-header org
pinned to its own org, for a user that lives in another organization. Uses the
return-code medium so a successful response would carry the plaintext code,
making a missing denial impossible to pass silently.
The second subtest pins the other half of the contract: IAM_USER_MANAGER holds
user.passkey.write instance-wide and must keep working across organizations, so
the fix cannot be tightened into user.credential.write without regressing it.
No equivalent test for user/v2beta: it shares the same internal/command layer
and is fixed by the same change, but the API is deprecated, so the duplicate is
deliberately omitted.
Co-Authored-By: Claude Opus 5 <[email protected]>
* refactor(command): generate passkey code only after the permission check
Review feedback: the code ID and the passkey code - the latter costs an extra
eventstore filter for the secret generator config - were produced before the
permission check, so an unauthorized caller still paid for both.
The write model is now built with an empty code ID, which is equivalent for
the lookup: the ID only matches events of an already existing code, and a code
about to be created has none. It is assigned right after the check so the
pushed event is still appended to the write model.
Co-Authored-By: Claude Opus 5 <[email protected]>
* fix(command): enforce permission check on v1 passwordless init codes
The deprecated management RPCs AddPasswordlessRegistration and
SendPasswordlessRegistration accept an attacker-controlled user_id and pass the
caller's own organization to the command. That organization only scopes the
write model's read, not the write: for a user of another organization the read
matches nothing, the write model keeps the caller's organization, and on push
the eventstore re-owns the event to the target's real organization
(internal/eventstore/v3/sequence.go). An org admin could therefore mint a
passwordless enrollment link for a user of any other organization and, after
correcting the orgID in the returned link, take the account over.
Same class of flaw as the v2 CreatePasskeyRegistrationLink issue, but a
different command, so the earlier fix does not reach it. Copying that fix
verbatim would not close it either: authorizing against the write model's
resource owner would authorize against the attacker's own organization, where
they legitimately hold the permission. The check is instead handed an empty
resource owner, which makes it resolve the target's real owner from the
eventstore.
The check sits in the two exported commands rather than in the shared inner
function, for two reasons:
- the RPCs declare different permissions, user.credential.write and
user.write. Hardcoding one would either lock IAM_USER_MANAGER and
ORG_USER_MANAGER out of SendPasswordlessRegistration or widen
AddPasswordlessRegistration to them. Access is preserved exactly.
- the inner function is also called by importHuman while creating a user,
whose aggregate has no events yet and whose owner cannot be resolved.
Self management stays allowed, so the auth API's AddMyPasswordlessLink and
SendMyPasswordlessLink are unaffected, as is anonymous redemption of the code
through the login UI - the init code is a bearer credential by design and
issuance is the only place this can be enforced.
Co-Authored-By: Claude Opus 5 <[email protected]>
* test(command): guard cross-org v1 passwordless init code issuance
Asserts the organization handed to checkPermission, not merely that a denial
happened. A caller acting in org1 targeting a user of org2 must be authorized
against org2; a test that only asserted PermissionDenied would pass against the
attacker's own organization, where they hold the permission - which is exactly
how a fix copied from the v2 command would fail.
Also pins the permission per command, so a later attempt to hoist the check
into the shared inner function shows up as roles gaining or losing access, and
asserts that self management still skips the check for the auth API.
Negative controls, both confirmed:
- authorizing against the caller-supplied resource owner: the two RPC
subtests fail on org1 != org2.
- dropping the check: they fail on an unexpected ID generation and an
unfulfilled resource owner lookup, i.e. a code would have been issued.
Co-Authored-By: Claude Opus 5 <[email protected]>
* test(management): simulate cross-org passwordless registration link attack
Reproduces the attack against a live instance through the deprecated v1
management API, and pins the two access guarantees the fix has to keep.
Verified by running the three builds against a real instance:
| build | cross-org denied | IAM_USER_MANAGER can send |
|--------------------------------|------------------|---------------------------|
| no check | FAIL, link with | PASS |
| | plaintext code | |
| | returned | |
| user.credential.write for both | PASS | FAIL, AUTH-AWfge |
| per-RPC permissions (shipped) | PASS | PASS |
Row 1 is the vulnerability. Row 2 is why SendPasswordlessRegistration keeps
user.write: IAM_USER_MANAGER holds it instance wide, and instance memberships
resolve in every organization, so the stricter permission would take the
endpoint away from that role everywhere.
ORG_USER_MANAGER gets no separate case: it holds the same user.write through an
org-scoped membership, which the same-org subtest already covers. The auth API's
self-service equivalents have no integration test suite; they are covered by the
self-management unit test instead.
Co-Authored-By: Claude Opus 5 <[email protected]>
* chore: trigger checks
Co-Authored-By: Claude Opus 5 <[email protected]>
---------
Co-authored-by: Claude Opus 5 <[email protected]>
# Which Problems Are Solved
Flakiness in integration tests because of eventual consistentcy.
# How the Problems Are Solved
Split tests related to feature flags and other eventual consistent
resources.
# Additional Changes
None
# Additional Context
None
Co-authored-by: Marco A. <[email protected]>
# Which Problems Are Solved
Some user v2 API calls checked for permission only on the user itself.
# How the Problems Are Solved
Consistent check for permissions on user v2 API.
# Additional Changes
None
# Additional Context
Closes#7944
---------
Co-authored-by: Livio Spring <[email protected]>
# Which Problems Are Solved
Integration tests are flaky due to eventual consistency.
# How the Problems Are Solved
Remove t.Parallel so that less concurrent requests on multiple instance
happen. This allows the projections to catch up more easily.
# Additional Changes
- none
# Additional Context
- none
# Which Problems Are Solved
Eventual consistency is handled wrongly in the newly improved
integration tests.
# How the Problems Are Solved
Correct the usage of the require package with the assert package where
necessary, to remove the panics where the EventuallyWithT functions can
rerun.
# Additional Changes
Modify the timeout values for some EventuallyWithT which can vary when a
instance is freshly setup.
# Additional Context
None
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
There is no option to only query auth methods related to specific
domains.
# How the Problems Are Solved
Add domain as attribute to the ListAuthenticationMethodTypes request.
# Additional Changes
OwnerRemoved column removed from the projection.
# Additional Context
Closes#8615
---------
Co-authored-by: Livio Spring <[email protected]>
# 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 <[email protected]>
Livio SpringandGitHub
Tim MöhlmannandGitHub