mirror of
https://github.com/andwati/zitadel.git
synced 2026-10-18 05:52:51 +00:00
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]>
This commit is contained in:
co-authored by
Claude Opus 5
parent
5f3a0de33a
commit
8a61aa4f36
@@ -0,0 +1,80 @@
|
||||
//go:build integration
|
||||
|
||||
package management_test
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"google.golang.org/grpc/codes"
|
||||
"google.golang.org/grpc/status"
|
||||
|
||||
"github.com/zitadel/zitadel/internal/integration"
|
||||
"github.com/zitadel/zitadel/pkg/grpc/management"
|
||||
)
|
||||
|
||||
// TestPasswordlessRegistration_CrossOrg simulates issuing a passwordless enrollment link for a
|
||||
// user of a foreign organization through the deprecated v1 management API.
|
||||
//
|
||||
// The caller passes only its own organization, which the interceptor is happy with because that
|
||||
// is where the caller holds the permission. The write lands on the target's aggregate regardless,
|
||||
// so the denial has to come from the command, authorizing against the target's real owner.
|
||||
func TestPasswordlessRegistration_CrossOrg(t *testing.T) {
|
||||
orgB := Instance.CreateOrganization(IAMOwnerCTX, integration.OrganizationName(), integration.Email())
|
||||
victimID := Instance.CreateHumanUserVerified(
|
||||
IAMOwnerCTX, orgB.GetOrganizationId(), integration.Email(), integration.Phone(),
|
||||
).GetUserId()
|
||||
|
||||
// Either code is acceptable: the default permission backend reports the caller's missing
|
||||
// membership in the target organization as not found, PermissionCheckV2 as permission denied.
|
||||
deniedCodes := []codes.Code{codes.NotFound, codes.PermissionDenied}
|
||||
|
||||
t.Run("org owner of another org is denied", func(t *testing.T) {
|
||||
// ReturnCode equivalent: on success the link with the plaintext code is in the response,
|
||||
// so asserting it stays empty proves nothing leaked.
|
||||
got, err := Client.AddPasswordlessRegistration(OrgCTX, &management.AddPasswordlessRegistrationRequest{
|
||||
UserId: victimID,
|
||||
})
|
||||
require.Error(t, err)
|
||||
assert.Contains(t, deniedCodes, status.Code(err))
|
||||
assert.Empty(t, got.GetLink())
|
||||
|
||||
_, err = Client.SendPasswordlessRegistration(OrgCTX, &management.SendPasswordlessRegistrationRequest{
|
||||
UserId: victimID,
|
||||
})
|
||||
require.Error(t, err)
|
||||
assert.Contains(t, deniedCodes, status.Code(err))
|
||||
})
|
||||
|
||||
t.Run("org owner of the same org is allowed", func(t *testing.T) {
|
||||
ownUserID := Instance.CreateHumanUserVerified(
|
||||
IAMOwnerCTX, Instance.DefaultOrg.GetId(), integration.Email(), integration.Phone(),
|
||||
).GetUserId()
|
||||
|
||||
got, err := Client.AddPasswordlessRegistration(OrgCTX, &management.AddPasswordlessRegistrationRequest{
|
||||
UserId: ownUserID,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
assert.NotEmpty(t, got.GetLink())
|
||||
|
||||
_, err = Client.SendPasswordlessRegistration(OrgCTX, &management.SendPasswordlessRegistrationRequest{
|
||||
UserId: ownUserID,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
})
|
||||
|
||||
// SendPasswordlessRegistration is annotated with user.write, which IAM_USER_MANAGER holds
|
||||
// instance wide. Gating the command on user.credential.write instead - the annotation of the
|
||||
// sibling AddPasswordlessRegistration - would lock this role out of the endpoint.
|
||||
t.Run("instance wide user manager may still send", func(t *testing.T) {
|
||||
_, pat, err := Instance.CreateMachineUserPATWithMembership(IAMOwnerCTX, "IAM_USER_MANAGER")
|
||||
require.NoError(t, err)
|
||||
managerCTX := integration.WithAuthorizationToken(CTX, pat)
|
||||
|
||||
_, err = Client.SendPasswordlessRegistration(managerCTX, &management.SendPasswordlessRegistrationRequest{
|
||||
UserId: victimID,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
})
|
||||
}
|
||||
@@ -10,6 +10,8 @@ import (
|
||||
"github.com/muhlemmer/gu"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"google.golang.org/grpc/codes"
|
||||
"google.golang.org/grpc/status"
|
||||
"google.golang.org/protobuf/types/known/structpb"
|
||||
"google.golang.org/protobuf/types/known/timestamppb"
|
||||
|
||||
@@ -322,6 +324,51 @@ func TestServer_CreatePasskeyRegistrationLink(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestServer_CreatePasskeyRegistrationLink_CrossOrg simulates the cross-organization passkey
|
||||
// enrollment code issuance: the RPC's auth annotation carries user.passkey.write with no
|
||||
// org_field, so the interceptor only checks the caller's permission in the request-header org.
|
||||
// An org owner of one organization must not be able to mint an enrollment code - and with it a
|
||||
// takeover of the account - for a user in another organization.
|
||||
func TestServer_CreatePasskeyRegistrationLink_CrossOrg(t *testing.T) {
|
||||
// The victim lives in another organization than the caller.
|
||||
orgB := Instance.CreateOrganization(IamCTX, integration.OrganizationName(), integration.Email())
|
||||
victimID := Instance.CreateHumanUserVerified(
|
||||
IamCTX, orgB.GetOrganizationId(), integration.Email(), integration.Phone(),
|
||||
).GetUserId()
|
||||
|
||||
t.Run("org owner of another org is denied", func(t *testing.T) {
|
||||
// The attacker is ORG_OWNER of the default org only, and pins the request-header org
|
||||
// to it - exactly the scope the interceptor verifies.
|
||||
attackerCTX := integration.SetOrgID(OrgCTX, Instance.DefaultOrg.GetId())
|
||||
|
||||
got, err := Client.CreatePasskeyRegistrationLink(attackerCTX, &user.CreatePasskeyRegistrationLinkRequest{
|
||||
UserId: victimID,
|
||||
Medium: &user.CreatePasskeyRegistrationLinkRequest_ReturnCode{},
|
||||
})
|
||||
require.Error(t, err)
|
||||
// The default permission path reports "membership not found" (NotFound) because the
|
||||
// caller has no membership in the victim's org at all; the v2 permission check reports
|
||||
// PermissionDenied. Either is a correct denial.
|
||||
assert.Contains(t, []codes.Code{codes.NotFound, codes.PermissionDenied}, status.Code(err))
|
||||
assert.Empty(t, got.GetCode().GetCode())
|
||||
})
|
||||
|
||||
t.Run("instance wide user manager is allowed", func(t *testing.T) {
|
||||
// IAM_USER_MANAGER holds user.passkey.write instance wide, so it must keep working
|
||||
// across organizations - the check narrows the org, it must not narrow the role set.
|
||||
_, pat, err := Instance.CreateMachineUserPATWithMembership(IamCTX, "IAM_USER_MANAGER")
|
||||
require.NoError(t, err)
|
||||
managerCTX := integration.WithAuthorizationToken(CTX, pat)
|
||||
|
||||
got, err := Client.CreatePasskeyRegistrationLink(managerCTX, &user.CreatePasskeyRegistrationLinkRequest{
|
||||
UserId: victimID,
|
||||
Medium: &user.CreatePasskeyRegistrationLinkRequest_ReturnCode{},
|
||||
})
|
||||
require.NoError(t, err)
|
||||
assert.NotEmpty(t, got.GetCode().GetId())
|
||||
})
|
||||
}
|
||||
|
||||
func userWithPasskeyRegistered(t *testing.T) (string, *user.RegisterPasskeyResponse) {
|
||||
userID := Instance.CreateHumanUser(OrgCTX).GetUserId()
|
||||
return userID, passkeyRegister(t, userID)
|
||||
|
||||
@@ -34,6 +34,15 @@ func mockEncryptedCode(code string, exp time.Duration) encrypedCodeFunc {
|
||||
}
|
||||
}
|
||||
|
||||
// newEncryptedCodeNoCall fails the test if a code is generated, e.g. before a permission check
|
||||
// denied the request.
|
||||
func newEncryptedCodeNoCall(t *testing.T) encrypedCodeFunc {
|
||||
return func(context.Context, preparation.FilterToQueryReducer, domain.SecretGeneratorType, crypto.EncryptionAlgorithm) (*EncryptedCode, error) {
|
||||
t.Error("unexpected code generation")
|
||||
return nil, nil
|
||||
}
|
||||
}
|
||||
|
||||
func mockEncryptedCodeWithDefault(code string, exp time.Duration) encryptedCodeWithDefaultFunc {
|
||||
return func(ctx context.Context, filter preparation.FilterToQueryReducer, _ domain.SecretGeneratorType, alg crypto.EncryptionAlgorithm, _ *crypto.GeneratorConfig) (*EncryptedCode, error) {
|
||||
return &EncryptedCode{
|
||||
|
||||
@@ -72,6 +72,14 @@ func (c *Commands) checkPermissionUpdateUserCredentials(ctx context.Context, res
|
||||
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)
|
||||
}
|
||||
|
||||
@@ -486,7 +486,22 @@ func (c *Commands) HumanRemovePasswordless(ctx context.Context, userID, webAuthN
|
||||
return c.removeHumanWebAuthN(ctx, userID, webAuthNID, resourceOwner, event)
|
||||
}
|
||||
|
||||
// HumanAddPasswordlessInitCode issues a passwordless init code and returns it to the caller.
|
||||
//
|
||||
// The permission is deliberately checked with an empty resource owner. Callers pass their own
|
||||
// organization, which only scopes the write model's read: for a user of another organization
|
||||
// that read matches no events, so the write model keeps the caller's organization while the
|
||||
// pushed event is re-owned to the target's organization by the eventstore. Handing an empty
|
||||
// resource owner to the check makes it resolve the target's real owner instead, and fail with
|
||||
// not found when the user does not exist at all.
|
||||
//
|
||||
// user.credential.write mirrors the auth annotation of the management RPC calling this
|
||||
// (AddPasswordlessRegistration), so no role gains or loses access. Self management stays
|
||||
// allowed for the auth API's AddMyPasswordlessLink.
|
||||
func (c *Commands) HumanAddPasswordlessInitCode(ctx context.Context, userID, resourceOwner string, passwordlessCodeGenerator crypto.Generator) (*domain.PasswordlessInitCode, error) {
|
||||
if err := c.checkPermissionUpdateUserCredentials(ctx, "", userID); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
codeEvent, initCode, code, err := c.humanAddPasswordlessInitCode(ctx, userID, resourceOwner, true, passwordlessCodeGenerator)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
@@ -502,7 +517,16 @@ func (c *Commands) HumanAddPasswordlessInitCode(ctx context.Context, userID, res
|
||||
return writeModelToPasswordlessInitCode(initCode, code), nil
|
||||
}
|
||||
|
||||
// HumanSendPasswordlessInitCode issues a passwordless init code and has it mailed to the user.
|
||||
//
|
||||
// It checks user.write, the annotation of the management RPC calling this
|
||||
// (SendPasswordlessRegistration), which a wider set of roles holds than the
|
||||
// user.credential.write of HumanAddPasswordlessInitCode. See there for why the resource owner
|
||||
// passed to the check is empty.
|
||||
func (c *Commands) HumanSendPasswordlessInitCode(ctx context.Context, userID, resourceOwner string, passwordlessCodeGenerator crypto.Generator) (*domain.PasswordlessInitCode, error) {
|
||||
if err := c.checkPermissionUpdateUser(ctx, "", userID, true); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
codeEvent, initCode, code, err := c.humanAddPasswordlessInitCode(ctx, userID, resourceOwner, false, passwordlessCodeGenerator)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
@@ -518,6 +542,9 @@ func (c *Commands) HumanSendPasswordlessInitCode(ctx context.Context, userID, re
|
||||
return writeModelToPasswordlessInitCode(initCode, code), nil
|
||||
}
|
||||
|
||||
// humanAddPasswordlessInitCode is unauthorized on purpose: importHuman calls it while creating
|
||||
// the user, whose aggregate has no events yet, so the target's owner cannot be resolved here.
|
||||
// Authorization belongs to the exported callers.
|
||||
func (c *Commands) humanAddPasswordlessInitCode(ctx context.Context, userID, resourceOwner string, direct bool, passwordlessCodeGenerator crypto.Generator) (eventstore.Command, *HumanPasswordlessInitCodeWriteModel, string, error) {
|
||||
if userID == "" {
|
||||
return nil, nil, "", zerrors.ThrowPreconditionFailed(nil, "COMMAND-GVfg3", "Errors.IDMissing")
|
||||
|
||||
@@ -0,0 +1,109 @@
|
||||
package command
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"golang.org/x/text/language"
|
||||
|
||||
"github.com/zitadel/zitadel/internal/api/authz"
|
||||
"github.com/zitadel/zitadel/internal/domain"
|
||||
id_mock "github.com/zitadel/zitadel/internal/id/mock"
|
||||
"github.com/zitadel/zitadel/internal/repository/user"
|
||||
"github.com/zitadel/zitadel/internal/zerrors"
|
||||
)
|
||||
|
||||
// victimHumanAddedEvent is a human in org2, targeted by a caller acting in org1.
|
||||
func victimHumanAddedEvent() *user.HumanAddedEvent {
|
||||
return user.NewHumanAddedEvent(context.Background(),
|
||||
&user.NewAggregate("user1", "org2").Aggregate,
|
||||
"username",
|
||||
"firstname",
|
||||
"lastname",
|
||||
"nickname",
|
||||
"displayname",
|
||||
language.German,
|
||||
domain.GenderUnspecified,
|
||||
"[email protected]",
|
||||
true,
|
||||
)
|
||||
}
|
||||
|
||||
// TestCommands_HumanPasswordlessInitCode_permission guards cross-organization issuance of v1
|
||||
// passwordless init codes. The deprecated management RPCs pass the *caller's* organization as
|
||||
// resource owner, which only scopes the read: the init code write model then matches no events
|
||||
// for a user of another organization and keeps the caller's organization, while the pushed event
|
||||
// is re-owned to the target's organization by the eventstore.
|
||||
//
|
||||
// Authorizing against that write model would therefore authorize against the attacker's own
|
||||
// organization. The assertion on the organization handed to checkPermission is what this test
|
||||
// exists for - asserting only that a denial happened would pass even against the wrong org.
|
||||
func TestCommands_HumanPasswordlessInitCode_permission(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
// wantPermission mirrors the auth annotation of the RPC calling the command; a change
|
||||
// here means roles gained or lost access.
|
||||
wantPermission string
|
||||
call func(*Commands) error
|
||||
}{
|
||||
{
|
||||
name: "HumanAddPasswordlessInitCode",
|
||||
wantPermission: domain.PermissionUserCredentialWrite,
|
||||
call: func(c *Commands) error {
|
||||
_, err := c.HumanAddPasswordlessInitCode(context.Background(), "user1", "org1", nil)
|
||||
return err
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "HumanSendPasswordlessInitCode",
|
||||
wantPermission: domain.PermissionUserWrite,
|
||||
call: func(c *Commands) error {
|
||||
_, err := c.HumanSendPasswordlessInitCode(context.Background(), "user1", "org1", nil)
|
||||
return err
|
||||
},
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
var gotPermission, gotOrgID, gotResourceID string
|
||||
c := &Commands{
|
||||
checkPermission: func(_ context.Context, permission, orgID, resourceID string) error {
|
||||
gotPermission, gotOrgID, gotResourceID = permission, orgID, resourceID
|
||||
return zerrors.ThrowPermissionDenied(nil, "AUTHZ-HKJD33", "Errors.PermissionDenied")
|
||||
},
|
||||
// Only the resource owner resolution is expected: a denied caller gets no code
|
||||
// generated and no event pushed.
|
||||
eventstore: expectEventstore(expectFilter(eventFromEventPusher(victimHumanAddedEvent())))(t),
|
||||
idGenerator: id_mock.NewIDGeneratorExpectIDs(t),
|
||||
}
|
||||
|
||||
err := tt.call(c)
|
||||
assert.True(t, zerrors.IsPermissionDenied(err), "want permission denied, got %v", err)
|
||||
assert.Equal(t, tt.wantPermission, gotPermission)
|
||||
assert.Equal(t, "org2", gotOrgID, "must authorize against the target user's org, not the caller-supplied one")
|
||||
assert.Equal(t, "user1", gotResourceID)
|
||||
})
|
||||
}
|
||||
|
||||
// internal/api/grpc/auth/passwordless.go passes ctxData.UserID, so self management has to
|
||||
// short-circuit before any permission lookup. Filtering is failed on purpose to end the call
|
||||
// right after the check, which spares this test mocking the whole push.
|
||||
t.Run("self management needs no permission", func(t *testing.T) {
|
||||
ctx := authz.SetCtxData(context.Background(), authz.CtxData{UserID: "user1"})
|
||||
filterErr := zerrors.ThrowInternal(nil, "TEST-Ei0oh", "filter failed")
|
||||
checked := false
|
||||
c := &Commands{
|
||||
checkPermission: func(context.Context, string, string, string) error {
|
||||
checked = true
|
||||
return zerrors.ThrowPermissionDenied(nil, "AUTHZ-HKJD33", "Errors.PermissionDenied")
|
||||
},
|
||||
eventstore: expectEventstore(expectFilterError(filterErr))(t),
|
||||
idGenerator: id_mock.NewIDGeneratorExpectIDs(t, "code1"),
|
||||
}
|
||||
|
||||
_, err := c.HumanAddPasswordlessInitCode(ctx, "user1", "org1", nil)
|
||||
assert.False(t, checked, "self management must not require a permission")
|
||||
assert.True(t, zerrors.IsInternal(err), "want the call to proceed past the permission check, got %v", err)
|
||||
})
|
||||
}
|
||||
@@ -128,19 +128,31 @@ func (c *Commands) AddUserPasskeyCodeReturn(ctx context.Context, userID, resourc
|
||||
}
|
||||
|
||||
func (c *Commands) addUserPasskeyCode(ctx context.Context, userID, resourceOwner string, alg crypto.EncryptionAlgorithm, urlTmpl string, returnCode bool) (*domain.PasskeyCodeDetails, error) {
|
||||
// The code ID only serves to match events of an already existing code, of which there are
|
||||
// none for a code that is about to be created. It is therefore generated after the
|
||||
// permission check, so an unauthorized caller triggers neither ID generation nor the
|
||||
// secret generator lookup behind newPasskeyCode.
|
||||
wm := NewHumanPasswordlessInitCodeWriteModel(userID, "", resourceOwner)
|
||||
err := c.eventstore.FilterToQueryReducer(ctx, wm)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
// Authorize against the target user's actual resource owner (resolved from the write
|
||||
// model), not a caller-supplied org. The API auth interceptor only verifies the
|
||||
// permission in the request-header org, so without this check an ORG_OWNER of one org
|
||||
// could mint a passkey enrollment code for a user in another org.
|
||||
if err := c.checkPermissionUpdateUserPasskey(ctx, wm.ResourceOwner, userID); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
codeID, err := c.idGenerator.Next()
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
wm.CodeID = codeID
|
||||
code, err := c.newPasskeyCode(ctx, c.eventstore.Filter, alg)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
wm := NewHumanPasswordlessInitCodeWriteModel(userID, codeID, resourceOwner)
|
||||
err = c.eventstore.FilterToQueryReducer(ctx, wm)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
agg := UserAggregateFromWriteModel(&wm.WriteModel)
|
||||
|
||||
if urlTmpl == "" {
|
||||
|
||||
@@ -0,0 +1,94 @@
|
||||
package command
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"go.uber.org/mock/gomock"
|
||||
"golang.org/x/text/language"
|
||||
|
||||
"github.com/zitadel/zitadel/internal/crypto"
|
||||
"github.com/zitadel/zitadel/internal/domain"
|
||||
id_mock "github.com/zitadel/zitadel/internal/id/mock"
|
||||
"github.com/zitadel/zitadel/internal/repository/user"
|
||||
"github.com/zitadel/zitadel/internal/zerrors"
|
||||
)
|
||||
|
||||
// TestCommands_addUserPasskeyCode_permission guards the cross-organization passkey enrollment
|
||||
// code issuance. The v2 gRPC handlers pass an empty resourceOwner and the RPC's auth annotation
|
||||
// carries no org_field, so the API interceptor only verifies the caller's permission in the
|
||||
// request-header org. The authorization therefore has to happen here, against the target user's
|
||||
// actual resource owner resolved from the write model - not against anything the caller supplied.
|
||||
func TestCommands_addUserPasskeyCode_permission(t *testing.T) {
|
||||
alg := crypto.CreateMockEncryptionAlg(gomock.NewController(t))
|
||||
|
||||
// The victim lives in org2. The caller supplies no resource owner at all, exactly like
|
||||
// internal/api/grpc/user/v2/passkey.go does.
|
||||
victimAgg := &user.NewAggregate("user1", "org2").Aggregate
|
||||
victimAdded := eventFromEventPusher(user.NewHumanAddedEvent(context.Background(),
|
||||
victimAgg,
|
||||
"username",
|
||||
"firstname",
|
||||
"lastname",
|
||||
"nickname",
|
||||
"displayname",
|
||||
language.German,
|
||||
domain.GenderUnspecified,
|
||||
"[email protected]",
|
||||
true,
|
||||
))
|
||||
|
||||
const urlTmpl = "https://example.com/passkey/register?userID={{.UserID}}&orgID={{.OrgID}}&codeID={{.CodeID}}&code={{.Code}}"
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
call func(*Commands) error
|
||||
}{
|
||||
{
|
||||
name: "AddUserPasskeyCode",
|
||||
call: func(c *Commands) error {
|
||||
_, err := c.AddUserPasskeyCode(context.Background(), "user1", "", alg)
|
||||
return err
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "AddUserPasskeyCodeURLTemplate",
|
||||
call: func(c *Commands) error {
|
||||
_, err := c.AddUserPasskeyCodeURLTemplate(context.Background(), "user1", "", alg, urlTmpl)
|
||||
return err
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "AddUserPasskeyCodeReturn",
|
||||
call: func(c *Commands) error {
|
||||
_, err := c.AddUserPasskeyCodeReturn(context.Background(), "user1", "", alg)
|
||||
return err
|
||||
},
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
var gotPermission, gotOrgID, gotResourceID string
|
||||
c := &Commands{
|
||||
checkPermission: func(_ context.Context, permission, orgID, resourceID string) error {
|
||||
gotPermission, gotOrgID, gotResourceID = permission, orgID, resourceID
|
||||
return zerrors.ThrowPermissionDenied(nil, "AUTHZ-HKJD33", "Errors.PermissionDenied")
|
||||
},
|
||||
// No push is expected: an unauthorized caller must not get a code issued.
|
||||
// Neither ID generation nor code generation runs before the check, so both
|
||||
// mocks are declared without any expected call.
|
||||
eventstore: expectEventstore(expectFilter(victimAdded))(t),
|
||||
idGenerator: id_mock.NewIDGeneratorExpectIDs(t),
|
||||
newEncryptedCode: newEncryptedCodeNoCall(t),
|
||||
loginPaths: expectLoginPathsNoCall(t),
|
||||
}
|
||||
|
||||
err := tt.call(c)
|
||||
assert.True(t, zerrors.IsPermissionDenied(err), "want permission denied, got %v", err)
|
||||
assert.Equal(t, domain.PermissionUserPasskeyWrite, gotPermission)
|
||||
assert.Equal(t, "org2", gotOrgID, "must authorize against the target user's org, not the caller-supplied one")
|
||||
assert.Equal(t, "user1", gotResourceID)
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -542,7 +542,7 @@ func TestCommands_AddUserPasskeyCode(t *testing.T) {
|
||||
name: "id generator error",
|
||||
fields: fields{
|
||||
newCode: mockEncryptedCode("passkey1", time.Hour),
|
||||
eventstore: expectEventstore(),
|
||||
eventstore: expectEventstore(expectFilter()), // init code write model
|
||||
idGenerator: id_mock.NewIDGeneratorExpectError(t, io.ErrClosedPipe),
|
||||
loginPaths: expectLoginPathsNoCall,
|
||||
},
|
||||
@@ -640,6 +640,7 @@ func TestCommands_AddUserPasskeyCode(t *testing.T) {
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
c := &Commands{
|
||||
checkPermission: newMockPermissionCheckAllowed(),
|
||||
newEncryptedCode: tt.fields.newCode,
|
||||
eventstore: tt.fields.eventstore(t),
|
||||
idGenerator: tt.fields.idGenerator,
|
||||
@@ -690,7 +691,7 @@ func TestCommands_AddUserPasskeyCodeURLTemplate(t *testing.T) {
|
||||
name: "id generator error",
|
||||
fields: fields{
|
||||
newCode: newEncryptedCode,
|
||||
eventstore: expectEventstore(),
|
||||
eventstore: expectEventstore(expectFilter()), // init code write model
|
||||
idGenerator: id_mock.NewIDGeneratorExpectError(t, io.ErrClosedPipe),
|
||||
},
|
||||
args: args{
|
||||
@@ -749,6 +750,7 @@ func TestCommands_AddUserPasskeyCodeURLTemplate(t *testing.T) {
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
c := &Commands{
|
||||
checkPermission: newMockPermissionCheckAllowed(),
|
||||
newEncryptedCode: tt.fields.newCode,
|
||||
eventstore: tt.fields.eventstore(t),
|
||||
idGenerator: tt.fields.idGenerator,
|
||||
@@ -784,7 +786,7 @@ func TestCommands_AddUserPasskeyCodeReturn(t *testing.T) {
|
||||
name: "id generator error",
|
||||
fields: fields{
|
||||
newCode: newEncryptedCode,
|
||||
eventstore: expectEventstore(),
|
||||
eventstore: expectEventstore(expectFilter()), // init code write model
|
||||
idGenerator: id_mock.NewIDGeneratorExpectError(t, io.ErrClosedPipe),
|
||||
loginPaths: expectLoginPathsNoCall,
|
||||
},
|
||||
@@ -890,6 +892,7 @@ func TestCommands_AddUserPasskeyCodeReturn(t *testing.T) {
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
c := &Commands{
|
||||
checkPermission: newMockPermissionCheckAllowed(),
|
||||
newEncryptedCode: tt.fields.newCode,
|
||||
eventstore: tt.fields.eventstore(t),
|
||||
idGenerator: tt.fields.idGenerator,
|
||||
@@ -931,7 +934,7 @@ func TestCommands_addUserPasskeyCode(t *testing.T) {
|
||||
name: "id generator error",
|
||||
fields: fields{
|
||||
newCode: newEncryptedCode,
|
||||
eventstore: expectEventstore(),
|
||||
eventstore: expectEventstore(expectFilter()), // init code write model
|
||||
idGenerator: id_mock.NewIDGeneratorExpectError(t, io.ErrClosedPipe),
|
||||
loginPaths: expectLoginPathsNoCall,
|
||||
},
|
||||
@@ -944,8 +947,11 @@ func TestCommands_addUserPasskeyCode(t *testing.T) {
|
||||
{
|
||||
name: "crypto error",
|
||||
fields: fields{
|
||||
newCode: newEncryptedCode,
|
||||
eventstore: expectEventstore(expectFilterError(io.ErrClosedPipe)),
|
||||
newCode: newEncryptedCode,
|
||||
eventstore: expectEventstore(
|
||||
expectFilter(), // init code write model
|
||||
expectFilterError(io.ErrClosedPipe),
|
||||
),
|
||||
idGenerator: id_mock.NewIDGeneratorExpectIDs(t, "123"),
|
||||
loginPaths: expectLoginPathsNoCall,
|
||||
},
|
||||
@@ -1107,6 +1113,7 @@ func TestCommands_addUserPasskeyCode(t *testing.T) {
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
c := &Commands{
|
||||
checkPermission: newMockPermissionCheckAllowed(),
|
||||
newEncryptedCode: tt.fields.newCode,
|
||||
eventstore: tt.fields.eventstore(t),
|
||||
idGenerator: tt.fields.idGenerator,
|
||||
|
||||
@@ -32,6 +32,7 @@ const (
|
||||
PermissionUserDelete = "user.delete"
|
||||
PermissionUserDeleteSelf = "user.self.delete"
|
||||
PermissionUserCredentialWrite = "user.credential.write"
|
||||
PermissionUserPasskeyWrite = "user.passkey.write"
|
||||
PermissionSessionWrite = "session.write"
|
||||
PermissionSessionRead = "session.read"
|
||||
PermissionSessionLink = "session.link"
|
||||
|
||||
Reference in New Issue
Block a user