mirror of
https://github.com/andwati/zitadel.git
synced 2026-11-08 15:15:12 +00:00
main
15
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
8a61aa4f36 |
Merge commit from fork
* 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]> |
||
|
|
f34cc45eac |
feat(oidc): add RFC 7591 dynamic client registration (#12313)
# Which Problems Are Solved ZITADEL cannot currently be used as an OAuth 2.0 Authorization Server by clients that need to register themselves at runtime: - There is no [OAuth 2.0 Dynamic Client Registration (RFC 7591)](https://datatracker.ietf.org/doc/html/rfc7591) endpoint, and no `registration_endpoint` is advertised in the discovery document. - This blocks [Model Context Protocol (MCP)](https://modelcontextprotocol.io) clients (Claude Desktop, claude.ai, Cursor, the MCP SDKs), whose OAuth 2.1 authorization profile expects an Authorization Server that supports dynamic client registration. They self-register before any user context exists and only then start the authorization code + PKCE flow. # How the Problems Are Solved ## Control plane: instance security settings No feature flag and no runtime config. A `DynamicClientRegistrationSettings` message is nested under the instance `SecuritySettings` in `settings/v2`, next to `embedded_iframe` and `enable_impersonation`: | `enabled` | `allow_unauthenticated` | Behaviour | | --- | --- | --- | | `false` (default) | — | `POST /oauth/v2/register` returns `404`, `registration_endpoint` absent from discovery | | `true` | `false` (default) | Token-gated registration, client homed in the token's organization | | `true` | `true` | Open registration for MCP hosts, client homed in the instance default organization | Both values are carried on `authz.Instance` and loaded with the instance, exactly like `EnableImpersonation`, so the discovery document and the endpoint read them per request without an extra query. `allow_unauthenticated` implies `enabled` in both implementations, so a single setting closes the endpoint. Adding the two columns bumps the projection table to `projections.security_policies3`, following what the impersonation setting did for `security_policies2`. ## Authorization: a dedicated permission, not `project.app.write` - New org-scoped permission **`project.app.register_dynamic`**, granted by default to `ORG_OWNER`, `IAM_OWNER` and `IAM_ORG_MANAGER` — in both `InternalAuthZ` and `SystemAuthZ`, mirroring where `project.app.write` already lives. - New built-in role **`ORG_DYNAMIC_CLIENT_REGISTRAR`** carrying only that permission, so a service user can self-register clients without gaining write access to the organization's existing applications. - Open mode (`allow_unauthenticated = true`) requires no token and no permission: that is the MCP self-register path. - A valid token whose user lacks the permission is answered with **`403 insufficient_scope`** (RFC 6750 §3.1) rather than `401 invalid_token`, so a caller can tell "your token is bad" from "your token may not do this". `verifyAccessToken` also reports invalid tokens as permission denied, so the authorization failure is marked explicitly to keep the two apart. The check runs once, at the endpoint, right after the token is verified and before any state is created, so it also gates the auto-provisioning of the organization's DCR project. The permission itself lives in the command layer next to the others (`Commands.CheckPermissionRegisterDynamicClient`). As the OIDC endpoints are not behind the authorization interceptor, the caller identity is derived from the verified token, the way token exchange does for the impersonating actor. ## Registration itself - **Registered clients are ordinary OIDC applications.** The endpoint reuses the existing application-creation events (`NewApplicationAddedEvent` and `NewOIDCConfigAddedEvent`), so the whole token, authorization and introspection flow keeps working unchanged. They are stored in a dedicated, auto-provisioned project named `ZITADEL DCR` per organization (name lookup), so they do not pollute the `IAMProject` or other projects. - Input is validated with the existing `GetOIDCV1Compliance` rules; `private_key_jwt` and `jwks`/`jwks_uri` are rejected with `invalid_client_metadata`. - **Endpoint wiring without forking the library.** `NewServer` calls `op.RegisterServer` instead of `op.RegisterLegacyServer`, so the new route can be added through the same `op.WithSetRouter` hook as the authorize callback. `op.RegisterLegacyServer` appends a middleware after the caller options, which chi forbids once a route has been registered; `op.NewIssuerInterceptor` reproduces the issuer middleware it would otherwise add. The `/oauth/v2` routing prefix is unchanged. # Additional Changes - **`application_type` inference for custom-scheme redirects** (last commit, isolated). `application_type` is an OpenID Connect member; RFC 7591 does not define it, so clients that only implement RFC 7591 omit it. Native MCP hosts doing so register custom-scheme redirect URIs, which ZITADEL reserves for native applications, and the OIDC default of `web` rejected them with `invalid_redirect_uri`. Inference is limited to custom schemes and to an absent `application_type`, so every request accepted today keeps its current application type and auth method — an `http` loopback redirect still yields a web application. Happy to drop this commit if you would rather keep it out of this PR. - **Docs**: a Dynamic Client Registration integration guide, a `registration_endpoint` section on the OpenID Connect endpoints page, and the new role in the administrators table (plus its description in the Console translations). # Additional Context - Part of #9810. This is **Phase 1 (DCR)**; #12315 adds RFC 7592 and #12316 adds CIMD, both stacked on this branch and rebased once this lands. - Implements the control-plane and authorization model requested in the review of this PR, itself a refinement of the direction in https://github.com/zitadel/zitadel/issues/9810#issuecomment-4891176997. - **Out of scope (deliberately)**: RFC 7592, CIMD, persisting free-form metadata (`logo_uri`, `contacts`, and similar), and `private_key_jwt` / `jwks_uri` (rejected cleanly). No new event types. - Follow-ups explicitly left out, per the review: DCR client lifecycle and richer audit, consent and custom scopes, JWT / `aud` (RFC 8707), and RFC 8414 AS metadata (currently absent from both ZITADEL and `zitadel/oidc`). |
||
|
|
ffd9b39bdb |
feat: handle Zitadel provider IdP login via login v1 (#12469)
# Which Problems Are Solved This PR adds support for using a ZITADEL IdP provider in the login v1 UI flow and conditionally grants a support-user instance membership based on a ZITADEL-specific project-roles token claim. # How the Problems Are Solved - Introduced ZITADEL IdP handling in the login v1 handler, including extracting the `urn:zitadel:iam:org:project:roles` claim and granting `IAM_OWNER_VIEWER` instance membership when configured orgs match. - Added a new `AddInstanceMemberFromLogin` to assign an instance membership during the login v1 ZITADEL-IdP flow without permission checks (as the authorization is established based on the project roles in the token claim) - Added unit tests # Additional Changes N/A # Additional Context - Closes https://github.com/zitadel/zitadel/issues/12446 |
||
|
|
5bdfd96be5 |
feat: extend DeleteProvider to include Zitadel IdP (#12396)
# Which Problems Are Solved This PR extends `DeleteProvider` to include Zitadel provider enabling the deletion of Zitadel IdP. # How the Problems Are Solved - Extend org/instance IDP remove write models to include `ZitadelIDPAddedEvent` in event appends and queries. - Extend command-side IDP reduction/type handling for Zitadel IDP add events. - Add management/admin integration tests for deleting Zitadel providers - Add error translation key in all language locales for org-level “IDP config not existing”. # Additional Changes N/A # Additional Context. Closes https://github.com/zitadel/zitadel/issues/12397 --------- Co-authored-by: Copilot Autofix powered by AI <[email protected]> Co-authored-by: Livio Spring <[email protected]> |
||
|
|
77169181a3 |
feat: get/list Zitadel IdP (#12394)
# Which Problems Are Solved This PR extends the functionality of `GetProviderByID`, `ListProviders` v1 endpoints and `GetIDPByID` v2 endpoint to also return Zitadel IdP when queried. # How the Problems Are Solved - Add `PROVIDER_TYPE_ZITADEL` and a `ZitadelConfig` to `zitadel.idp.v1.ProviderConfig` (v1 APIs). - Add `IDP_TYPE_ZITADEL`, `ZitadelConfig`, `InstanceRolesInfo` in v2 `idp.proto` - Extend internal/query IDP template querying to include a `ZitadelIDPTemplate` (incl. issuer, client credentials, scopes, instance roles info). - Add/extend integration tests for updating + fetching providers by ID and listing providers. # Additional Changes N/A # Additional Context Closes https://github.com/zitadel/zitadel/issues/12051 --------- Co-authored-by: Copilot Autofix powered by AI <[email protected]> |
||
|
|
9d60e83d6f |
Merge commit from fork
* Use slices.Contains over custom function * Correctly remove roles from granted roles * fix(setup): repair user grants with stale roles (GHSA-v859-c572-qh5p) Add setup step 73 that reconciles existing user grants whose roles were left too broad by the buggy cascade removal in removeRoleFromUserGrant. The corruption lives in the eventstore event payloads, so the step pushes a corrective user.grant.cascade.changed event per affected grant (roles intersected with the currently valid set) and re-triggers the user grant projection. Runs in the second setup slice, after the projection tables it reads have been created. Co-Authored-By: Claude Opus 4.8 <[email protected]> * fix(setup): scope GHSA-v859-c572-qh5p repair to grant-based user grants Direct user grants can never be hit by this bug (only ChangeProjectGrant's multi-role cascade to grant-based grants can trigger it), so drop the direct-grant branch from the finder query to avoid stripping unrelated, legitimate roles that merely mismatch for other reasons (e.g. stale role_key drift). Also exclude removed instances from the migration scope, and log the number of grants fixed per instance. Co-authored-by: Cursor <[email protected]> --------- Co-authored-by: Claude Opus 4.8 <[email protected]> Co-authored-by: Livio Spring <[email protected]> Co-authored-by: Cursor <[email protected]> |
||
|
|
942607e0e9 |
feat: implement UpdateZitadelProvider in ManagementService (#12384)
# Which Problems Are Solved This PR adds support for updating ZITADEL identity provider (IDP) templates. # How the Problems Are Solved - Implements `UpdateZitadelProvider` in the Management gRPC server plus converter - Introduces an org-scoped `ZitadelIDPChanged` event and registers its event mapper. - Adds command-side support to update org Zitadel providers and appends the new event to the org write model. - Extends the IDP template projection reducer to accept org change events. # Additional Changes N/A # Additional Context Closes https://github.com/zitadel/zitadel/issues/11922 Follow up for PRs: - https://github.com/zitadel/zitadel/pull/12018 - https://github.com/zitadel/zitadel/pull/12020 - https://github.com/zitadel/zitadel/pull/12055 - https://github.com/zitadel/zitadel/pull/12056 - https://github.com/zitadel/zitadel/pull/12371 - https://github.com/zitadel/zitadel/pull/12378 --------- Co-authored-by: Copilot Autofix powered by AI <[email protected]> Co-authored-by: Livio Spring <[email protected]> |
||
|
|
e19bb7a173 |
feat: implement AddZitadelProvider in ManagementService (#12056)
# Which Problems Are Solved This PR adds implementation to add a Zitadel IdP at the organization-level. # How the Problems Are Solved - Added handling/converters for the `AddZitadelProvider` endpoint in `ManagementService` in the server layer - Registered a new `org.idp.zitadel.added` event for org-level Zitadel providers - Added `AddOrgZitadelProvider` command to validate the request and push `org.idp.zitadel.added` event to the eventstore - Added the `org.idp.zitadel.added` event to the projection reducer - Added unit and integration tests # Additional Changes added more tests for the ZitadelProvider in AdminService # Additional Context - Closes https://github.com/zitadel/zitadel/issues/11823 - Follow-up for PRs https://github.com/zitadel/zitadel/pull/12018, https://github.com/zitadel/zitadel/pull/12020, https://github.com/zitadel/zitadel/pull/12055 |
||
|
|
5a2181bcec |
chore: rename IAM to instance (#11351)
# Which Problems Are Solved In many cases IAM was used where instance was meant. # How the Problems Are Solved - Rename wrong usage of IAM to instance all over the code base. - Add an agents.md which explains the terms used in Zitadel's architecture. # Additional Context - closes #11280 --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: muhlemmer <[email protected]> Co-authored-by: Copilot <[email protected]> Co-authored-by: Marco A. <[email protected]> |
||
|
|
a45908b364 |
feat(rt): project repository (#10789)
# Which Problems Are Solved Add projects to the relational tables # How the Problems Are Solved - Define table migrations - Define and implement Project and Project Role repositories. - Provide projection handlers to populate the relational tables. # Additional Changes - Statement Builder now has a constructor which allows setting of a base query with arguments. - Certain operations, like Get, Update and Delete require the Primary Key to be set as conditions. However, this requires knowledge of the implementation and table definition. This PR proposes an additional condition for repositories: `PrimaryKeyCondition`. This gives clarity on the required IDs for these operations. - Added couple of helpers to the repository package, to allow more DRY code. - getOne / getMany: generic functions for query execution and scanning. - checkRestrictingColumns, checkPkCondition: simplify condition checking, instead of using ladders of conditionals. - Added a couple of helpers to the repository test package: - Transaction, savepoint and rollback helpers. - Create instance and organization helpers for objects that depend on them (like projects). # Additional Context - after https://github.com/zitadel/zitadel/pull/10809 - closes #10765 |
||
|
|
492f1826ee |
chore: move gofakeit integration testing calls (#10684)
# Which Problems Are Solved Flakiness and conflicts in value from gofakeit. # How the Problems Are Solved Move Gofakeit calls to the integration package, to guarantee proper usage and values for integration testing. # Additional Changes None # Additional Context None |
||
|
|
a637ae5aa5 |
fix(api): fix for ListAppKeys() not returning app keys (#10465)
# Which Problems Are Solved `ListAppKeys()` does not work properly, in that it does not return any app keys. # How the Problems Are Solved The issue stems from a mistake SQL query not joining the `projections.authn_keys2` table to `projections.projects4` instead of joining to `projections.apps7` # Additional Changes `ListAppKeys()` returns the app key IDs in order of their creation - Closes https://github.com/zitadel/zitadel/issues/10420 - backport to v4.x --------- Co-authored-by: Livio Spring <[email protected]> |
||
|
|
e1f0d46393 |
fix(listUsers): Add Search User By Phone to User Service V2 (#9052)
# Which Problems Are Solved
Added search by phone to user Service V2.
```
curl --request POST \
--url https://<zitadel_domain>/v2/users \
--header 'Accept: application/json' \
--header 'Authorization: Bearer <Token>' \
--header 'Content-Type: application/json' \
--header 'content-type: application/json' \
--data '{
"query": {
"offset": "0",
"limit": 100,
"asc": true
},
"sortingColumn": "USER_FIELD_NAME_UNSPECIFIED",
"queries": [
{
"phoneQuery": {
"number": "+12011223313",
"method": "TEXT_QUERY_METHOD_EQUALS"
}
}
]
}'
```
Why?
Searching for a user by phone was missing from User Service V2 and V2
beta.
# How the Problems Are Solved
* Added to the SearchQuery proto
* Added code to filter users by phone
# Additional Changes
N/A
# Additional Context
Search by phone is present in V3 User Service
---------
Co-authored-by: Stefan Benz <[email protected]>
|
||
|
|
8d97363642 |
chore: improve integration tests (#8727)
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 |
||
|
|
d2e0ac07f1 |
chore(tests): use a coverage server binary (#8407)
# 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]> |