From c07a5f4277277284d6ddd31a1b0f4c86da39c005 Mon Sep 17 00:00:00 2001 From: Stefan Benz <46600784+stebenz@users.noreply.github.com> Date: Tue, 3 Dec 2024 11:14:04 +0100 Subject: [PATCH] fix: consistent permission check on user v2 (#8807) # 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 --- .../grpc/user/v2/integration_test/otp_test.go | 73 ++++++++++++++++--- .../user/v2/integration_test/passkey_test.go | 19 ++++- internal/api/grpc/user/v2/otp.go | 1 - .../user/v2beta/integration_test/otp_test.go | 71 +++++++++++++++--- .../v2beta/integration_test/passkey_test.go | 19 ++++- internal/command/user_human_otp.go | 43 ++++------- internal/command/user_human_password.go | 2 +- internal/command/user_human_webauthn.go | 14 ++-- internal/command/user_v2.go | 10 +++ internal/command/user_v2_email.go | 13 +--- internal/command/user_v2_invite.go | 7 +- internal/command/user_v2_passkey.go | 3 +- internal/command/user_v2_passkey_test.go | 24 +++--- internal/command/user_v2_password.go | 7 +- internal/command/user_v2_phone.go | 12 +-- 15 files changed, 213 insertions(+), 105 deletions(-) diff --git a/internal/api/grpc/user/v2/integration_test/otp_test.go b/internal/api/grpc/user/v2/integration_test/otp_test.go index ae7c040427..01e6c07a40 100644 --- a/internal/api/grpc/user/v2/integration_test/otp_test.go +++ b/internal/api/grpc/user/v2/integration_test/otp_test.go @@ -58,7 +58,7 @@ func TestServer_AddOTPSMS(t *testing.T) { wantErr: true, }, { - name: "user mismatch", + name: "no permission", args: args{ ctx: integration.WithAuthorizationToken(context.Background(), sessionTokenOtherUser), req: &user.AddOTPSMSRequest{ @@ -127,14 +127,24 @@ func TestServer_RemoveOTPSMS(t *testing.T) { userVerified := Instance.CreateHumanUser(CTX) Instance.RegisterUserPasskey(CTX, userVerified.GetUserId()) - _, sessionTokenVerified, _, _ := Instance.CreateVerifiedWebAuthNSession(t, CTX, userVerified.GetUserId()) - userVerifiedCtx := integration.WithAuthorizationToken(context.Background(), sessionTokenVerified) - _, err := Instance.Client.UserV2.VerifyPhone(userVerifiedCtx, &user.VerifyPhoneRequest{ + _, err := Instance.Client.UserV2.VerifyPhone(CTX, &user.VerifyPhoneRequest{ UserId: userVerified.GetUserId(), VerificationCode: userVerified.GetPhoneCode(), }) require.NoError(t, err) - _, err = Instance.Client.UserV2.AddOTPSMS(userVerifiedCtx, &user.AddOTPSMSRequest{UserId: userVerified.GetUserId()}) + _, err = Instance.Client.UserV2.AddOTPSMS(CTX, &user.AddOTPSMSRequest{UserId: userVerified.GetUserId()}) + require.NoError(t, err) + + userSelf := Instance.CreateHumanUser(CTX) + Instance.RegisterUserPasskey(CTX, userSelf.GetUserId()) + _, sessionTokenSelf, _, _ := Instance.CreateVerifiedWebAuthNSession(t, CTX, userSelf.GetUserId()) + userSelfCtx := integration.WithAuthorizationToken(context.Background(), sessionTokenSelf) + _, err = Instance.Client.UserV2.VerifyPhone(CTX, &user.VerifyPhoneRequest{ + UserId: userSelf.GetUserId(), + VerificationCode: userSelf.GetPhoneCode(), + }) + require.NoError(t, err) + _, err = Instance.Client.UserV2.AddOTPSMS(CTX, &user.AddOTPSMSRequest{UserId: userSelf.GetUserId()}) require.NoError(t, err) type args struct { @@ -157,10 +167,24 @@ func TestServer_RemoveOTPSMS(t *testing.T) { }, wantErr: true, }, + { + name: "success, self", + args: args{ + ctx: userSelfCtx, + req: &user.RemoveOTPSMSRequest{ + UserId: userSelf.GetUserId(), + }, + }, + want: &user.RemoveOTPSMSResponse{ + Details: &object.Details{ + ResourceOwner: Instance.DefaultOrg.Details.ResourceOwner, + }, + }, + }, { name: "success", args: args{ - ctx: userVerifiedCtx, + ctx: CTX, req: &user.RemoveOTPSMSRequest{ UserId: userVerified.GetUserId(), }, @@ -230,7 +254,7 @@ func TestServer_AddOTPEmail(t *testing.T) { wantErr: true, }, { - name: "user mismatch", + name: "no permission", args: args{ ctx: integration.WithAuthorizationToken(context.Background(), sessionTokenOtherUser), req: &user.AddOTPEmailRequest{ @@ -301,14 +325,24 @@ func TestServer_RemoveOTPEmail(t *testing.T) { userVerified := Instance.CreateHumanUser(CTX) Instance.RegisterUserPasskey(CTX, userVerified.GetUserId()) - _, sessionTokenVerified, _, _ := Instance.CreateVerifiedWebAuthNSession(t, CTX, userVerified.GetUserId()) - userVerifiedCtx := integration.WithAuthorizationToken(context.Background(), sessionTokenVerified) - _, err := Instance.Client.UserV2.VerifyEmail(userVerifiedCtx, &user.VerifyEmailRequest{ + _, err := Instance.Client.UserV2.VerifyEmail(CTX, &user.VerifyEmailRequest{ UserId: userVerified.GetUserId(), VerificationCode: userVerified.GetEmailCode(), }) require.NoError(t, err) - _, err = Instance.Client.UserV2.AddOTPEmail(userVerifiedCtx, &user.AddOTPEmailRequest{UserId: userVerified.GetUserId()}) + _, err = Instance.Client.UserV2.AddOTPEmail(CTX, &user.AddOTPEmailRequest{UserId: userVerified.GetUserId()}) + require.NoError(t, err) + + userSelf := Instance.CreateHumanUser(CTX) + Instance.RegisterUserPasskey(CTX, userSelf.GetUserId()) + _, sessionTokenSelf, _, _ := Instance.CreateVerifiedWebAuthNSession(t, CTX, userSelf.GetUserId()) + userSelfCtx := integration.WithAuthorizationToken(context.Background(), sessionTokenSelf) + _, err = Instance.Client.UserV2.VerifyEmail(CTX, &user.VerifyEmailRequest{ + UserId: userSelf.GetUserId(), + VerificationCode: userSelf.GetEmailCode(), + }) + require.NoError(t, err) + _, err = Instance.Client.UserV2.AddOTPEmail(CTX, &user.AddOTPEmailRequest{UserId: userSelf.GetUserId()}) require.NoError(t, err) type args struct { @@ -331,10 +365,25 @@ func TestServer_RemoveOTPEmail(t *testing.T) { }, wantErr: true, }, + { + name: "success, self", + args: args{ + ctx: userSelfCtx, + req: &user.RemoveOTPEmailRequest{ + UserId: userSelf.GetUserId(), + }, + }, + want: &user.RemoveOTPEmailResponse{ + Details: &object.Details{ + ChangeDate: timestamppb.Now(), + ResourceOwner: Instance.DefaultOrg.Details.ResourceOwner, + }, + }, + }, { name: "success", args: args{ - ctx: userVerifiedCtx, + ctx: CTX, req: &user.RemoveOTPEmailRequest{ UserId: userVerified.GetUserId(), }, diff --git a/internal/api/grpc/user/v2/integration_test/passkey_test.go b/internal/api/grpc/user/v2/integration_test/passkey_test.go index 881ab17c09..055a47ec46 100644 --- a/internal/api/grpc/user/v2/integration_test/passkey_test.go +++ b/internal/api/grpc/user/v2/integration_test/passkey_test.go @@ -93,15 +93,30 @@ func TestServer_RegisterPasskey(t *testing.T) { wantErr: true, }, { - name: "user mismatch", + name: "user no permission", args: args{ - ctx: CTX, + ctx: UserCTX, req: &user.RegisterPasskeyRequest{ UserId: userID, }, }, wantErr: true, }, + { + name: "user permission", + args: args{ + ctx: IamCTX, + req: &user.RegisterPasskeyRequest{ + UserId: userID, + }, + }, + want: &user.RegisterPasskeyResponse{ + Details: &object.Details{ + ChangeDate: timestamppb.Now(), + ResourceOwner: Instance.DefaultOrg.Id, + }, + }, + }, { name: "user setting its own passkey", args: args{ diff --git a/internal/api/grpc/user/v2/otp.go b/internal/api/grpc/user/v2/otp.go index e2fe6b794d..fd76cf2b93 100644 --- a/internal/api/grpc/user/v2/otp.go +++ b/internal/api/grpc/user/v2/otp.go @@ -13,7 +13,6 @@ func (s *Server) AddOTPSMS(ctx context.Context, req *user.AddOTPSMSRequest) (*us return nil, err } return &user.AddOTPSMSResponse{Details: object.DomainToDetailsPb(details)}, nil - } func (s *Server) RemoveOTPSMS(ctx context.Context, req *user.RemoveOTPSMSRequest) (*user.RemoveOTPSMSResponse, error) { diff --git a/internal/api/grpc/user/v2beta/integration_test/otp_test.go b/internal/api/grpc/user/v2beta/integration_test/otp_test.go index 6d6e2eff3e..fae6c069a4 100644 --- a/internal/api/grpc/user/v2beta/integration_test/otp_test.go +++ b/internal/api/grpc/user/v2beta/integration_test/otp_test.go @@ -58,7 +58,7 @@ func TestServer_AddOTPSMS(t *testing.T) { wantErr: true, }, { - name: "user mismatch", + name: "no permission", args: args{ ctx: integration.WithAuthorizationToken(context.Background(), sessionTokenOtherUser), req: &user.AddOTPSMSRequest{ @@ -127,14 +127,24 @@ func TestServer_RemoveOTPSMS(t *testing.T) { userVerified := Instance.CreateHumanUser(CTX) Instance.RegisterUserPasskey(CTX, userVerified.GetUserId()) - _, sessionTokenVerified, _, _ := Instance.CreateVerifiedWebAuthNSession(t, CTX, userVerified.GetUserId()) - userVerifiedCtx := integration.WithAuthorizationToken(context.Background(), sessionTokenVerified) - _, err := Client.VerifyPhone(userVerifiedCtx, &user.VerifyPhoneRequest{ + _, err := Instance.Client.UserV2beta.VerifyPhone(CTX, &user.VerifyPhoneRequest{ UserId: userVerified.GetUserId(), VerificationCode: userVerified.GetPhoneCode(), }) require.NoError(t, err) - _, err = Client.AddOTPSMS(userVerifiedCtx, &user.AddOTPSMSRequest{UserId: userVerified.GetUserId()}) + _, err = Instance.Client.UserV2beta.AddOTPSMS(CTX, &user.AddOTPSMSRequest{UserId: userVerified.GetUserId()}) + require.NoError(t, err) + + userSelf := Instance.CreateHumanUser(CTX) + Instance.RegisterUserPasskey(CTX, userSelf.GetUserId()) + _, sessionTokenSelf, _, _ := Instance.CreateVerifiedWebAuthNSession(t, CTX, userSelf.GetUserId()) + userSelfCtx := integration.WithAuthorizationToken(context.Background(), sessionTokenSelf) + _, err = Instance.Client.UserV2beta.VerifyPhone(CTX, &user.VerifyPhoneRequest{ + UserId: userSelf.GetUserId(), + VerificationCode: userSelf.GetPhoneCode(), + }) + require.NoError(t, err) + _, err = Instance.Client.UserV2beta.AddOTPSMS(CTX, &user.AddOTPSMSRequest{UserId: userSelf.GetUserId()}) require.NoError(t, err) type args struct { @@ -157,10 +167,24 @@ func TestServer_RemoveOTPSMS(t *testing.T) { }, wantErr: true, }, + { + name: "success, self", + args: args{ + ctx: userSelfCtx, + req: &user.RemoveOTPSMSRequest{ + UserId: userSelf.GetUserId(), + }, + }, + want: &user.RemoveOTPSMSResponse{ + Details: &object.Details{ + ResourceOwner: Instance.DefaultOrg.Details.ResourceOwner, + }, + }, + }, { name: "success", args: args{ - ctx: userVerifiedCtx, + ctx: CTX, req: &user.RemoveOTPSMSRequest{ UserId: userVerified.GetUserId(), }, @@ -301,14 +325,24 @@ func TestServer_RemoveOTPEmail(t *testing.T) { userVerified := Instance.CreateHumanUser(CTX) Instance.RegisterUserPasskey(CTX, userVerified.GetUserId()) - _, sessionTokenVerified, _, _ := Instance.CreateVerifiedWebAuthNSession(t, CTX, userVerified.GetUserId()) - userVerifiedCtx := integration.WithAuthorizationToken(context.Background(), sessionTokenVerified) - _, err := Client.VerifyEmail(userVerifiedCtx, &user.VerifyEmailRequest{ + _, err := Client.VerifyEmail(CTX, &user.VerifyEmailRequest{ UserId: userVerified.GetUserId(), VerificationCode: userVerified.GetEmailCode(), }) require.NoError(t, err) - _, err = Client.AddOTPEmail(userVerifiedCtx, &user.AddOTPEmailRequest{UserId: userVerified.GetUserId()}) + _, err = Client.AddOTPEmail(CTX, &user.AddOTPEmailRequest{UserId: userVerified.GetUserId()}) + require.NoError(t, err) + + userSelf := Instance.CreateHumanUser(CTX) + Instance.RegisterUserPasskey(CTX, userSelf.GetUserId()) + _, sessionTokenSelf, _, _ := Instance.CreateVerifiedWebAuthNSession(t, IamCTX, userSelf.GetUserId()) + userSelfCtx := integration.WithAuthorizationToken(context.Background(), sessionTokenSelf) + _, err = Client.VerifyEmail(CTX, &user.VerifyEmailRequest{ + UserId: userSelf.GetUserId(), + VerificationCode: userSelf.GetEmailCode(), + }) + require.NoError(t, err) + _, err = Client.AddOTPEmail(CTX, &user.AddOTPEmailRequest{UserId: userSelf.GetUserId()}) require.NoError(t, err) type args struct { @@ -331,10 +365,25 @@ func TestServer_RemoveOTPEmail(t *testing.T) { }, wantErr: true, }, + { + name: "success, self", + args: args{ + ctx: userSelfCtx, + req: &user.RemoveOTPEmailRequest{ + UserId: userSelf.GetUserId(), + }, + }, + want: &user.RemoveOTPEmailResponse{ + Details: &object.Details{ + ChangeDate: timestamppb.Now(), + ResourceOwner: Instance.DefaultOrg.Details.ResourceOwner, + }, + }, + }, { name: "success", args: args{ - ctx: userVerifiedCtx, + ctx: CTX, req: &user.RemoveOTPEmailRequest{ UserId: userVerified.GetUserId(), }, diff --git a/internal/api/grpc/user/v2beta/integration_test/passkey_test.go b/internal/api/grpc/user/v2beta/integration_test/passkey_test.go index acca01885c..7bc0465956 100644 --- a/internal/api/grpc/user/v2beta/integration_test/passkey_test.go +++ b/internal/api/grpc/user/v2beta/integration_test/passkey_test.go @@ -92,15 +92,30 @@ func TestServer_RegisterPasskey(t *testing.T) { wantErr: true, }, { - name: "user mismatch", + name: "user no permission", args: args{ - ctx: CTX, + ctx: UserCTX, req: &user.RegisterPasskeyRequest{ UserId: userID, }, }, wantErr: true, }, + { + name: "user permission", + args: args{ + ctx: IamCTX, + req: &user.RegisterPasskeyRequest{ + UserId: userID, + }, + }, + want: &user.RegisterPasskeyResponse{ + Details: &object.Details{ + ChangeDate: timestamppb.Now(), + ResourceOwner: Instance.DefaultOrg.Id, + }, + }, + }, { name: "user setting its own passkey", args: args{ diff --git a/internal/command/user_human_otp.go b/internal/command/user_human_otp.go index e505288cbd..97596aabd8 100644 --- a/internal/command/user_human_otp.go +++ b/internal/command/user_human_otp.go @@ -7,7 +7,6 @@ import ( "github.com/pquerna/otp" "github.com/zitadel/logging" - "github.com/zitadel/zitadel/internal/api/authz" http_util "github.com/zitadel/zitadel/internal/api/http" "github.com/zitadel/zitadel/internal/command/preparation" "github.com/zitadel/zitadel/internal/crypto" @@ -79,10 +78,8 @@ func (c *Commands) createHumanTOTP(ctx context.Context, userID, resourceOwner st logging.WithError(err).WithField("traceID", tracing.TraceIDFromCtx(ctx)).Debug("unable to get human for loginname") return nil, zerrors.ThrowPreconditionFailed(err, "COMMAND-SqyJz", "Errors.User.NotFound") } - if authz.GetCtxData(ctx).UserID != userID { - if err := c.checkPermission(ctx, domain.PermissionUserCredentialWrite, human.ResourceOwner, userID); err != nil { - return nil, err - } + if err := c.checkPermissionUpdateUserCredentials(ctx, human.ResourceOwner, userID); err != nil { + return nil, err } org, err := c.getOrg(ctx, human.ResourceOwner) if err != nil { @@ -139,10 +136,8 @@ func (c *Commands) HumanCheckMFATOTPSetup(ctx context.Context, userID, code, use if err != nil { return nil, err } - if authz.GetCtxData(ctx).UserID != userID { - if err := c.checkPermission(ctx, domain.PermissionUserCredentialWrite, existingOTP.ResourceOwner, userID); err != nil { - return nil, err - } + if err := c.checkPermissionUpdateUserCredentials(ctx, existingOTP.ResourceOwner, userID); err != nil { + return nil, err } if existingOTP.State == domain.MFAStateUnspecified || existingOTP.State == domain.MFAStateRemoved { return nil, zerrors.ThrowNotFound(nil, "COMMAND-3Mif9s", "Errors.User.MFA.OTP.NotExisting") @@ -242,10 +237,8 @@ func (c *Commands) HumanRemoveTOTP(ctx context.Context, userID, resourceOwner st if existingOTP.State == domain.MFAStateUnspecified || existingOTP.State == domain.MFAStateRemoved { return nil, zerrors.ThrowNotFound(nil, "COMMAND-Hd9sd", "Errors.User.MFA.OTP.NotExisting") } - if userID != authz.GetCtxData(ctx).UserID { - if err := c.checkPermission(ctx, domain.PermissionUserWrite, existingOTP.ResourceOwner, userID); err != nil { - return nil, err - } + if err := c.checkPermissionUpdateUser(ctx, existingOTP.ResourceOwner, userID); err != nil { + return nil, err } userAgg := UserAggregateFromWriteModel(&existingOTP.WriteModel) pushedEvents, err := c.eventstore.Push(ctx, user.NewHumanOTPRemovedEvent(ctx, userAgg)) @@ -286,10 +279,8 @@ func (c *Commands) addHumanOTPSMS(ctx context.Context, userID, resourceOwner str if err != nil { return nil, err } - if authz.GetCtxData(ctx).UserID != userID { - if err := c.checkPermission(ctx, domain.PermissionUserCredentialWrite, otpWriteModel.ResourceOwner(), userID); err != nil { - return nil, err - } + if err := c.checkPermissionUpdateUserCredentials(ctx, otpWriteModel.ResourceOwner(), userID); err != nil { + return nil, err } if otpWriteModel.otpAdded { return nil, zerrors.ThrowAlreadyExists(nil, "COMMAND-Ad3g2", "Errors.User.MFA.OTP.AlreadyReady") @@ -318,10 +309,8 @@ func (c *Commands) RemoveHumanOTPSMS(ctx context.Context, userID, resourceOwner if err != nil { return nil, err } - if userID != authz.GetCtxData(ctx).UserID { - if err := c.checkPermission(ctx, domain.PermissionUserWrite, existingOTP.WriteModel.ResourceOwner, userID); err != nil { - return nil, err - } + if err := c.checkPermissionUpdateUser(ctx, existingOTP.WriteModel.ResourceOwner, userID); err != nil { + return nil, err } if !existingOTP.otpAdded { return nil, zerrors.ThrowNotFound(nil, "COMMAND-Sr3h3", "Errors.User.MFA.OTP.NotExisting") @@ -420,10 +409,8 @@ func (c *Commands) addHumanOTPEmail(ctx context.Context, userID, resourceOwner s if err != nil { return nil, err } - if authz.GetCtxData(ctx).UserID != userID { - if err := c.checkPermission(ctx, domain.PermissionUserCredentialWrite, otpWriteModel.ResourceOwner(), userID); err != nil { - return nil, err - } + if err := c.checkPermissionUpdateUserCredentials(ctx, otpWriteModel.ResourceOwner(), userID); err != nil { + return nil, err } if otpWriteModel.otpAdded { return nil, zerrors.ThrowAlreadyExists(nil, "COMMAND-MKL2s", "Errors.User.MFA.OTP.AlreadyReady") @@ -452,10 +439,8 @@ func (c *Commands) RemoveHumanOTPEmail(ctx context.Context, userID, resourceOwne if err != nil { return nil, err } - if userID != authz.GetCtxData(ctx).UserID { - if err := c.checkPermission(ctx, domain.PermissionUserWrite, existingOTP.WriteModel.ResourceOwner, userID); err != nil { - return nil, err - } + if err := c.checkPermissionUpdateUser(ctx, existingOTP.WriteModel.ResourceOwner, userID); err != nil { + return nil, err } if !existingOTP.otpAdded { return nil, zerrors.ThrowNotFound(nil, "COMMAND-b312D", "Errors.User.MFA.OTP.NotExisting") diff --git a/internal/command/user_human_password.go b/internal/command/user_human_password.go index 9b686f88b7..a67a4b91da 100644 --- a/internal/command/user_human_password.go +++ b/internal/command/user_human_password.go @@ -110,7 +110,7 @@ type setPasswordVerification func(ctx context.Context) (newEncodedPassword strin // setPasswordWithPermission returns a permission check as [setPasswordVerification] implementation func (c *Commands) setPasswordWithPermission(userID, orgID string) setPasswordVerification { return func(ctx context.Context) (_ string, err error) { - return "", c.checkPermission(ctx, domain.PermissionUserWrite, orgID, userID) + return "", c.checkPermissionUpdateUser(ctx, orgID, userID) } } diff --git a/internal/command/user_human_webauthn.go b/internal/command/user_human_webauthn.go index 3555466359..3b8a66e0d5 100644 --- a/internal/command/user_human_webauthn.go +++ b/internal/command/user_human_webauthn.go @@ -6,7 +6,6 @@ import ( "github.com/zitadel/logging" - "github.com/zitadel/zitadel/internal/api/authz" "github.com/zitadel/zitadel/internal/crypto" "github.com/zitadel/zitadel/internal/domain" "github.com/zitadel/zitadel/internal/eventstore" @@ -146,10 +145,8 @@ func (c *Commands) addHumanWebAuthN(ctx context.Context, userID, resourceowner, if err != nil { return nil, nil, nil, err } - if authz.GetCtxData(ctx).UserID != userID { - if err = c.checkPermission(ctx, domain.PermissionUserCredentialWrite, user.ResourceOwner, userID); err != nil { - return nil, nil, nil, err - } + if err := c.checkPermissionUpdateUserCredentials(ctx, user.ResourceOwner, userID); err != nil { + return nil, nil, nil, err } org, err := c.getOrg(ctx, user.ResourceOwner) if err != nil { @@ -603,10 +600,9 @@ func (c *Commands) removeHumanWebAuthN(ctx context.Context, userID, webAuthNID, if existingWebAuthN.State == domain.MFAStateUnspecified || existingWebAuthN.State == domain.MFAStateRemoved { return nil, zerrors.ThrowNotFound(nil, "COMMAND-DAfb2", "Errors.User.WebAuthN.NotFound") } - if userID != authz.GetCtxData(ctx).UserID { - if err := c.checkPermission(ctx, domain.PermissionUserWrite, existingWebAuthN.ResourceOwner, existingWebAuthN.AggregateID); err != nil { - return nil, err - } + + if err := c.checkPermissionUpdateUser(ctx, existingWebAuthN.ResourceOwner, existingWebAuthN.AggregateID); err != nil { + return nil, err } userAgg := UserAggregateFromWriteModel(&existingWebAuthN.WriteModel) diff --git a/internal/command/user_v2.go b/internal/command/user_v2.go index 032ac0b8f7..033a16eb9a 100644 --- a/internal/command/user_v2.go +++ b/internal/command/user_v2.go @@ -127,6 +127,16 @@ func (c *Commands) checkPermissionUpdateUser(ctx context.Context, resourceOwner, return nil } +func (c *Commands) checkPermissionUpdateUserCredentials(ctx context.Context, resourceOwner, userID string) error { + if userID != "" && userID == authz.GetCtxData(ctx).UserID { + return nil + } + if err := c.checkPermission(ctx, domain.PermissionUserCredentialWrite, resourceOwner, userID); err != nil { + return err + } + return nil +} + func (c *Commands) checkPermissionDeleteUser(ctx context.Context, resourceOwner, userID string) error { if userID != "" && userID == authz.GetCtxData(ctx).UserID { return nil diff --git a/internal/command/user_v2_email.go b/internal/command/user_v2_email.go index cc81f7399c..1618e2cd48 100644 --- a/internal/command/user_v2_email.go +++ b/internal/command/user_v2_email.go @@ -6,7 +6,6 @@ import ( "github.com/zitadel/logging" - "github.com/zitadel/zitadel/internal/api/authz" "github.com/zitadel/zitadel/internal/crypto" "github.com/zitadel/zitadel/internal/domain" "github.com/zitadel/zitadel/internal/eventstore" @@ -118,10 +117,8 @@ func (c *Commands) changeUserEmailWithGeneratorEvents(ctx context.Context, userI if err != nil { return nil, err } - if authz.GetCtxData(ctx).UserID != userID { - if err = c.checkPermission(ctx, domain.PermissionUserWrite, cmd.aggregate.ResourceOwner, userID); err != nil { - return nil, err - } + if err = c.checkPermissionUpdateUser(ctx, cmd.aggregate.ResourceOwner, userID); err != nil { + return nil, err } if err = cmd.Change(ctx, domain.EmailAddress(email)); err != nil { return nil, err @@ -137,10 +134,8 @@ func (c *Commands) resendUserEmailCodeWithGeneratorEvents(ctx context.Context, u if err != nil { return nil, err } - if authz.GetCtxData(ctx).UserID != userID { - if err = c.checkPermission(ctx, domain.PermissionUserWrite, cmd.aggregate.ResourceOwner, userID); err != nil { - return nil, err - } + if err = c.checkPermissionUpdateUser(ctx, cmd.aggregate.ResourceOwner, userID); err != nil { + return nil, err } if cmd.model.Code == nil { return nil, zerrors.ThrowPreconditionFailed(err, "EMAIL-5w5ilin4yt", "Errors.User.Code.Empty") diff --git a/internal/command/user_v2_invite.go b/internal/command/user_v2_invite.go index 78b46a530e..1325d2e0c9 100644 --- a/internal/command/user_v2_invite.go +++ b/internal/command/user_v2_invite.go @@ -6,7 +6,6 @@ import ( "github.com/zitadel/logging" - "github.com/zitadel/zitadel/internal/api/authz" "github.com/zitadel/zitadel/internal/crypto" "github.com/zitadel/zitadel/internal/domain" "github.com/zitadel/zitadel/internal/eventstore" @@ -74,10 +73,8 @@ func (c *Commands) ResendInviteCode(ctx context.Context, userID, resourceOwner, if err != nil { return nil, err } - if authz.GetCtxData(ctx).UserID != userID { - if err := c.checkPermission(ctx, domain.PermissionUserWrite, existingCode.ResourceOwner, userID); err != nil { - return nil, err - } + if err := c.checkPermissionUpdateUser(ctx, existingCode.ResourceOwner, userID); err != nil { + return nil, err } if !existingCode.UserState.Exists() { return nil, zerrors.ThrowPreconditionFailed(nil, "COMMAND-H3b2a", "Errors.User.NotFound") diff --git a/internal/command/user_v2_passkey.go b/internal/command/user_v2_passkey.go index 897a1ab41d..a386049744 100644 --- a/internal/command/user_v2_passkey.go +++ b/internal/command/user_v2_passkey.go @@ -6,7 +6,6 @@ import ( "github.com/zitadel/logging" - "github.com/zitadel/zitadel/internal/api/authz" "github.com/zitadel/zitadel/internal/command/preparation" "github.com/zitadel/zitadel/internal/crypto" "github.com/zitadel/zitadel/internal/domain" @@ -18,7 +17,7 @@ import ( // RegisterUserPasskey creates a passkey registration for the current authenticated user. // UserID, usually taken from the request is compared against the user ID in the context. func (c *Commands) RegisterUserPasskey(ctx context.Context, userID, resourceOwner, rpID string, authenticator domain.AuthenticatorAttachment) (*domain.WebAuthNRegistrationDetails, error) { - if err := authz.UserIDInCTX(ctx, userID); err != nil { + if err := c.checkPermissionUpdateUserCredentials(ctx, resourceOwner, userID); err != nil { return nil, err } return c.registerUserPasskey(ctx, userID, resourceOwner, rpID, authenticator) diff --git a/internal/command/user_v2_passkey_test.go b/internal/command/user_v2_passkey_test.go index a6ba470d2b..0d1009862c 100644 --- a/internal/command/user_v2_passkey_test.go +++ b/internal/command/user_v2_passkey_test.go @@ -34,8 +34,9 @@ func TestCommands_RegisterUserPasskey(t *testing.T) { } userAgg := &user.NewAggregate("user1", "org1").Aggregate type fields struct { - eventstore *eventstore.Eventstore - idGenerator id.Generator + eventstore func(t *testing.T) *eventstore.Eventstore + idGenerator id.Generator + checkPermission domain.PermissionCheck } type args struct { userID string @@ -51,18 +52,22 @@ func TestCommands_RegisterUserPasskey(t *testing.T) { wantErr error }{ { - name: "wrong user", + name: "no permission", + fields: fields{ + eventstore: expectEventstore(), + checkPermission: newMockPermissionCheckNotAllowed(), + }, args: args{ userID: "foo", resourceOwner: "org1", authenticator: domain.AuthenticatorAttachmentCrossPlattform, }, - wantErr: zerrors.ThrowPermissionDenied(nil, "AUTH-Bohd2", "Errors.User.UserIDWrong"), + wantErr: zerrors.ThrowPermissionDenied(nil, "AUTHZ-HKJD33", "Errors.PermissionDenied"), }, { name: "get human passwordless error", fields: fields{ - eventstore: eventstoreExpect(t, + eventstore: expectEventstore( expectFilterError(io.ErrClosedPipe), ), }, @@ -76,7 +81,7 @@ func TestCommands_RegisterUserPasskey(t *testing.T) { { name: "id generator error", fields: fields{ - eventstore: eventstoreExpect(t, + eventstore: expectEventstore( expectFilter(), // getHumanPasswordlessTokens expectFilter(eventFromEventPusher( user.NewHumanAddedEvent(ctx, @@ -118,9 +123,10 @@ func TestCommands_RegisterUserPasskey(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { c := &Commands{ - eventstore: tt.fields.eventstore, - idGenerator: tt.fields.idGenerator, - webauthnConfig: webauthnConfig, + eventstore: tt.fields.eventstore(t), + idGenerator: tt.fields.idGenerator, + webauthnConfig: webauthnConfig, + checkPermission: tt.fields.checkPermission, } _, err := c.RegisterUserPasskey(ctx, tt.args.userID, tt.args.resourceOwner, tt.args.rpID, tt.args.authenticator) require.ErrorIs(t, err, tt.wantErr) diff --git a/internal/command/user_v2_password.go b/internal/command/user_v2_password.go index 67bee2c28f..faa1fe14a6 100644 --- a/internal/command/user_v2_password.go +++ b/internal/command/user_v2_password.go @@ -4,7 +4,6 @@ import ( "context" "io" - "github.com/zitadel/zitadel/internal/api/authz" "github.com/zitadel/zitadel/internal/domain" "github.com/zitadel/zitadel/internal/repository/user" "github.com/zitadel/zitadel/internal/zerrors" @@ -50,10 +49,8 @@ func (c *Commands) requestPasswordReset(ctx context.Context, userID string, retu if model.UserState == domain.UserStateInitial { return nil, nil, zerrors.ThrowPreconditionFailed(nil, "COMMAND-Sfe4g", "Errors.User.NotInitialised") } - if authz.GetCtxData(ctx).UserID != userID { - if err = c.checkPermission(ctx, domain.PermissionUserWrite, model.ResourceOwner, userID); err != nil { - return nil, nil, err - } + if err = c.checkPermissionUpdateUser(ctx, model.ResourceOwner, userID); err != nil { + return nil, nil, err } var passwordCode *EncryptedCode var generatorID string diff --git a/internal/command/user_v2_phone.go b/internal/command/user_v2_phone.go index 8b754b36f3..8648f9a564 100644 --- a/internal/command/user_v2_phone.go +++ b/internal/command/user_v2_phone.go @@ -82,10 +82,8 @@ func (c *Commands) changeUserPhoneWithGenerator(ctx context.Context, userID, pho if err != nil { return nil, err } - if authz.GetCtxData(ctx).UserID != userID { - if err = c.checkPermission(ctx, domain.PermissionUserWrite, cmd.aggregate.ResourceOwner, userID); err != nil { - return nil, err - } + if err = c.checkPermissionUpdateUser(ctx, cmd.aggregate.ResourceOwner, userID); err != nil { + return nil, err } if err = cmd.Change(ctx, domain.PhoneNumber(phone)); err != nil { return nil, err @@ -104,10 +102,8 @@ func (c *Commands) resendUserPhoneCodeWithGenerator(ctx context.Context, userID if err != nil { return nil, err } - if authz.GetCtxData(ctx).UserID != userID { - if err = c.checkPermission(ctx, domain.PermissionUserWrite, cmd.aggregate.ResourceOwner, userID); err != nil { - return nil, err - } + if err = c.checkPermissionUpdateUser(ctx, cmd.aggregate.ResourceOwner, userID); err != nil { + return nil, err } if cmd.model.Code == nil && cmd.model.GeneratorID == "" { return nil, zerrors.ThrowPreconditionFailed(err, "PHONE-5xrra88eq8", "Errors.User.Code.Empty")