mirror of
https://github.com/zitadel/zitadel.git
synced 2026-08-19 01:14:48 -05:00
Marco A.andClaude Opus 5
8a61aa4f36
* 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * chore: trigger checks Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
215 lines
11 KiB
Go
215 lines
11 KiB
Go
package command
|
|
|
|
import (
|
|
"context"
|
|
|
|
"github.com/zitadel/zitadel/internal/api/authz"
|
|
"github.com/zitadel/zitadel/internal/domain"
|
|
"github.com/zitadel/zitadel/internal/eventstore"
|
|
"github.com/zitadel/zitadel/internal/repository/group"
|
|
"github.com/zitadel/zitadel/internal/repository/instance"
|
|
"github.com/zitadel/zitadel/internal/repository/org"
|
|
"github.com/zitadel/zitadel/internal/repository/project"
|
|
"github.com/zitadel/zitadel/internal/v2/user"
|
|
"github.com/zitadel/zitadel/internal/zerrors"
|
|
)
|
|
|
|
type PermissionCheck func(resourceOwner, aggregateID string) error
|
|
|
|
type UserGrantPermissionCheck func(projectID, projectGrantID string) PermissionCheck
|
|
|
|
func (c *Commands) newPermissionCheck(ctx context.Context, permission string, aggregateType eventstore.AggregateType) PermissionCheck {
|
|
return func(resourceOwner, aggregateID string) error {
|
|
if aggregateID == "" {
|
|
return zerrors.ThrowInternal(nil, "COMMAND-ulBlS", "Errors.IDMissing")
|
|
}
|
|
// For example if a write model didn't query any events, the resource owner is probably empty.
|
|
// In this case, we have to query an event on the given aggregate to get the resource owner.
|
|
if resourceOwner == "" {
|
|
r := NewResourceOwnerModel(authz.GetInstance(ctx).InstanceID(), aggregateType, aggregateID)
|
|
err := c.eventstore.FilterToQueryReducer(ctx, r)
|
|
if err != nil {
|
|
return err
|
|
}
|
|
resourceOwner = r.resourceOwner
|
|
}
|
|
if resourceOwner == "" {
|
|
return zerrors.ThrowNotFound(nil, "COMMAND-4g3xq", "Errors.NotFound")
|
|
}
|
|
return c.checkPermission(ctx, permission, resourceOwner, aggregateID)
|
|
}
|
|
}
|
|
|
|
func (c *Commands) checkPermissionOnUser(ctx context.Context, permission string, allowSelfManagement bool) PermissionCheck {
|
|
return func(resourceOwner, aggregateID string) error {
|
|
if allowSelfManagement && aggregateID != "" && aggregateID == authz.GetCtxData(ctx).UserID {
|
|
return nil
|
|
}
|
|
return c.newPermissionCheck(ctx, permission, user.AggregateType)(resourceOwner, aggregateID)
|
|
}
|
|
}
|
|
|
|
func (c *Commands) NewPermissionCheckUserWrite(ctx context.Context, allowSelfManagement bool) PermissionCheck {
|
|
return c.checkPermissionOnUser(ctx, domain.PermissionUserWrite, allowSelfManagement)
|
|
}
|
|
|
|
func (c *Commands) checkPermissionDeleteUser(ctx context.Context, resourceOwner, userID string) error {
|
|
err := c.checkPermissionOnUser(ctx, domain.PermissionUserDelete, false)(resourceOwner, userID)
|
|
if err == nil {
|
|
return nil
|
|
}
|
|
if userID != authz.GetCtxData(ctx).UserID {
|
|
return err
|
|
}
|
|
return c.checkPermissionOnUser(ctx, domain.PermissionUserDeleteSelf, false)(resourceOwner, userID)
|
|
}
|
|
|
|
func (c *Commands) checkPermissionUpdateUser(ctx context.Context, resourceOwner, userID string, allowSelfManagement bool) error {
|
|
return c.NewPermissionCheckUserWrite(ctx, allowSelfManagement)(resourceOwner, userID)
|
|
}
|
|
|
|
func (c *Commands) checkPermissionUpdateUserCredentials(ctx context.Context, resourceOwner, userID string) error {
|
|
return c.checkPermissionOnUser(ctx, domain.PermissionUserCredentialWrite, true)(resourceOwner, userID)
|
|
}
|
|
|
|
// checkPermissionUpdateUserPasskey checks the same permission the passkey RPCs declare in their
|
|
// auth annotation, so that no role loses access. The API interceptor only verifies it in the
|
|
// caller-supplied request-header org, therefore commands must re-check it against the target
|
|
// user's actual resource owner.
|
|
func (c *Commands) checkPermissionUpdateUserPasskey(ctx context.Context, resourceOwner, userID string) error {
|
|
return c.checkPermissionOnUser(ctx, domain.PermissionUserPasskeyWrite, true)(resourceOwner, userID)
|
|
}
|
|
|
|
func (c *Commands) checkPermissionCreateProject(ctx context.Context, resourceOwner, projectID string) error {
|
|
return c.newPermissionCheck(ctx, domain.PermissionProjectCreate, project.AggregateType)(resourceOwner, projectID)
|
|
}
|
|
|
|
func (c *Commands) checkPermissionDeleteProject(ctx context.Context, resourceOwner, projectID string) error {
|
|
return c.newPermissionCheck(ctx, domain.PermissionProjectDelete, project.AggregateType)(resourceOwner, projectID)
|
|
}
|
|
|
|
func (c *Commands) checkPermissionUpdateProject(ctx context.Context, resourceOwner, projectID string) error {
|
|
return c.newPermissionCheck(ctx, domain.PermissionProjectWrite, project.AggregateType)(resourceOwner, projectID)
|
|
}
|
|
|
|
func (c *Commands) checkPermissionUpdateProjectGrant(ctx context.Context, resourceOwner, projectID, projectGrantID string) (err error) {
|
|
if err := c.newPermissionCheck(ctx, domain.PermissionProjectGrantWrite, project.AggregateType)(resourceOwner, projectGrantID); err != nil {
|
|
if err := c.newPermissionCheck(ctx, domain.PermissionProjectGrantWrite, project.AggregateType)(resourceOwner, projectID); err != nil {
|
|
return err
|
|
}
|
|
}
|
|
return nil
|
|
}
|
|
|
|
func (c *Commands) checkPermissionDeleteProjectGrant(ctx context.Context, resourceOwner, projectID, projectGrantID string) (err error) {
|
|
if err := c.newPermissionCheck(ctx, domain.PermissionProjectGrantDelete, project.AggregateType)(resourceOwner, projectGrantID); err != nil {
|
|
if err := c.newPermissionCheck(ctx, domain.PermissionProjectGrantDelete, project.AggregateType)(resourceOwner, projectID); err != nil {
|
|
return err
|
|
}
|
|
}
|
|
return nil
|
|
}
|
|
|
|
func (c *Commands) checkPermissionUpdateApplication(ctx context.Context, resourceOwner, appID string) error {
|
|
return c.newPermissionCheck(ctx, domain.PermissionProjectAppWrite, project.AggregateType)(resourceOwner, appID)
|
|
}
|
|
|
|
func (c *Commands) checkPermissionDeleteApp(ctx context.Context, resourceOwner, appID string) error {
|
|
return c.newPermissionCheck(ctx, domain.PermissionProjectAppDelete, project.AggregateType)(resourceOwner, appID)
|
|
}
|
|
|
|
func (c *Commands) checkPermissionUpdateInstanceMember(ctx context.Context, instanceID string) error {
|
|
return c.newPermissionCheck(ctx, domain.PermissionInstanceMemberWrite, instance.AggregateType)(instanceID, instanceID)
|
|
}
|
|
|
|
func (c *Commands) checkPermissionDeleteInstanceMember(ctx context.Context, instanceID string) error {
|
|
return c.newPermissionCheck(ctx, domain.PermissionInstanceMemberDelete, instance.AggregateType)(instanceID, instanceID)
|
|
}
|
|
|
|
func (c *Commands) checkPermissionUpdateOrgMember(ctx context.Context, instanceID, orgID string) error {
|
|
return c.newPermissionCheck(ctx, domain.PermissionOrgMemberWrite, org.AggregateType)(instanceID, orgID)
|
|
}
|
|
func (c *Commands) checkPermissionDeleteOrgMember(ctx context.Context, instanceID, orgID string) error {
|
|
return c.newPermissionCheck(ctx, domain.PermissionOrgMemberDelete, org.AggregateType)(instanceID, orgID)
|
|
}
|
|
|
|
func (c *Commands) checkPermissionUpdateProjectMember(ctx context.Context, resourceOwner, projectID string) error {
|
|
return c.newPermissionCheck(ctx, domain.PermissionProjectMemberWrite, project.AggregateType)(resourceOwner, projectID)
|
|
}
|
|
|
|
func (c *Commands) checkPermissionDeleteProjectMember(ctx context.Context, resourceOwner, projectID string) error {
|
|
return c.newPermissionCheck(ctx, domain.PermissionProjectMemberDelete, project.AggregateType)(resourceOwner, projectID)
|
|
}
|
|
|
|
func (c *Commands) checkPermissionUpdateProjectGrantMember(ctx context.Context, grantedOrgID, projectGrantID string) (err error) {
|
|
return c.newPermissionCheck(ctx, domain.PermissionProjectGrantMemberWrite, project.AggregateType)(grantedOrgID, projectGrantID)
|
|
}
|
|
|
|
func (c *Commands) checkPermissionDeleteProjectGrantMember(ctx context.Context, grantedOrgID, projectGrantID string) (err error) {
|
|
return c.newPermissionCheck(ctx, domain.PermissionProjectGrantMemberDelete, project.AggregateType)(grantedOrgID, projectGrantID)
|
|
}
|
|
|
|
func (c *Commands) newUserGrantPermissionCheck(ctx context.Context, permission string) UserGrantPermissionCheck {
|
|
check := c.newPermissionCheck(ctx, permission, project.AggregateType)
|
|
return func(projectID, projectGrantID string) PermissionCheck {
|
|
return func(resourceOwner, _ string) error {
|
|
if projectGrantID != "" {
|
|
return check(resourceOwner, projectGrantID)
|
|
}
|
|
return check(resourceOwner, projectID)
|
|
}
|
|
}
|
|
}
|
|
|
|
func (c *Commands) NewPermissionCheckUserGrantWrite(ctx context.Context) UserGrantPermissionCheck {
|
|
return c.newUserGrantPermissionCheck(ctx, domain.PermissionUserGrantWrite)
|
|
}
|
|
|
|
func (c *Commands) NewPermissionCheckUserGrantDelete(ctx context.Context) UserGrantPermissionCheck {
|
|
return c.newUserGrantPermissionCheck(ctx, domain.PermissionUserGrantDelete)
|
|
}
|
|
|
|
func (c *Commands) CheckPermissionOrganizationCreate(ctx context.Context, organizationID string) error {
|
|
return c.newPermissionCheck(ctx, domain.PermissionOrganizationWrite, org.AggregateType)(organizationID, organizationID)
|
|
}
|
|
|
|
func (c *Commands) CheckPermissionOrganizationWrite(ctx context.Context, organizationID string) error {
|
|
return c.newPermissionCheck(ctx, domain.PermissionOrganizationWrite, org.AggregateType)(organizationID, organizationID)
|
|
}
|
|
|
|
func (c *Commands) CheckPermissionOrganizationDelete(ctx context.Context, organizationID string) error {
|
|
return c.newPermissionCheck(ctx, domain.PermissionOrganizationDelete, org.AggregateType)(organizationID, organizationID)
|
|
}
|
|
|
|
// CheckPermissionRegisterDynamicClient authorizes token-gated OAuth 2.0 Dynamic Client
|
|
// Registration (RFC 7591) in the given organization. It is org-scoped and deliberately
|
|
// distinct from project.app.write, so a service user can be granted the ability to
|
|
// self-register clients without gaining write access to existing applications.
|
|
//
|
|
// It is called once by the registration endpoint before any state is created, so that it
|
|
// also gates the auto-provisioning of the organization's dedicated DCR project. Open
|
|
// registration does not require the permission; see the endpoint for the mode selection.
|
|
func (c *Commands) CheckPermissionRegisterDynamicClient(ctx context.Context, organizationID string) error {
|
|
return c.newPermissionCheck(ctx, domain.PermissionProjectAppRegisterDynamic, org.AggregateType)(organizationID, organizationID)
|
|
}
|
|
|
|
func (c *Commands) checkPermissionCreateGroup(ctx context.Context, resourceOwner, groupID string) error {
|
|
return c.newPermissionCheck(ctx, domain.PermissionGroupCreate, group.AggregateType)(resourceOwner, groupID)
|
|
}
|
|
|
|
func (c *Commands) checkPermissionUpdateGroup(ctx context.Context, resourceOwner, groupID string) error {
|
|
return c.newPermissionCheck(ctx, domain.PermissionGroupWrite, group.AggregateType)(resourceOwner, groupID)
|
|
}
|
|
|
|
func (c *Commands) checkPermissionDeleteGroup(ctx context.Context, resourceOwner, groupID string) error {
|
|
return c.newPermissionCheck(ctx, domain.PermissionGroupDelete, group.AggregateType)(resourceOwner, groupID)
|
|
}
|
|
|
|
func (c *Commands) checkPermissionAddUserToGroup(ctx context.Context, resourceOwner, groupID string) error {
|
|
return c.newPermissionCheck(ctx, domain.PermissionGroupUserWrite, group.AggregateType)(resourceOwner, groupID)
|
|
}
|
|
|
|
func (c *Commands) checkPermissionRemoveUserFromGroup(ctx context.Context, resourceOwner, groupID string) error {
|
|
return c.newPermissionCheck(ctx, domain.PermissionGroupUserDelete, group.AggregateType)(resourceOwner, groupID)
|
|
}
|