From 5bf195d37479090577b635cbb980cf9ec43621dc Mon Sep 17 00:00:00 2001 From: Livio Spring Date: Tue, 7 May 2024 07:38:26 +0200 Subject: [PATCH] fix: allow other users to set up MFAs (#7914) * fix: allow other users to set up MFAs * update tests * update integration tests --- internal/api/grpc/auth/multi_factor.go | 2 +- internal/api/grpc/auth/passwordless.go | 2 +- .../api/grpc/user/v2/otp_integration_test.go | 153 ++++--- .../api/grpc/user/v2/totp_integration_test.go | 56 ++- .../api/grpc/user/v2/u2f_integration_test.go | 20 +- internal/api/ui/login/mfa_init_u2f.go | 4 +- .../api/ui/login/mfa_init_verify_handler.go | 2 +- internal/api/ui/login/mfa_prompt_handler.go | 2 +- .../passwordless_registration_handler.go | 4 +- .../eventsourcing/eventstore/auth_request.go | 2 +- internal/command/user_human_otp.go | 23 +- internal/command/user_human_otp_test.go | 384 ++++++++++++++---- internal/command/user_human_webauthn.go | 20 +- internal/command/user_v2_passkey_test.go | 2 +- internal/command/user_v2_totp.go | 11 +- internal/command/user_v2_totp_test.go | 135 +++++- internal/command/user_v2_u2f.go | 4 - internal/command/user_v2_u2f_test.go | 55 ++- internal/domain/permission.go | 11 +- internal/webauthn/webauthn.go | 2 +- 20 files changed, 701 insertions(+), 193 deletions(-) diff --git a/internal/api/grpc/auth/multi_factor.go b/internal/api/grpc/auth/multi_factor.go index b7c23da1ad..ed3cf421d5 100644 --- a/internal/api/grpc/auth/multi_factor.go +++ b/internal/api/grpc/auth/multi_factor.go @@ -120,7 +120,7 @@ func (s *Server) RemoveMyAuthFactorOTPEmail(ctx context.Context, _ *auth_pb.Remo func (s *Server) AddMyAuthFactorU2F(ctx context.Context, _ *auth_pb.AddMyAuthFactorU2FRequest) (*auth_pb.AddMyAuthFactorU2FResponse, error) { ctxData := authz.GetCtxData(ctx) - u2f, err := s.command.HumanAddU2FSetup(ctx, ctxData.UserID, ctxData.ResourceOwner, false) + u2f, err := s.command.HumanAddU2FSetup(ctx, ctxData.UserID, ctxData.ResourceOwner) if err != nil { return nil, err } diff --git a/internal/api/grpc/auth/passwordless.go b/internal/api/grpc/auth/passwordless.go index 3b0aa5357d..b7b914d2c2 100644 --- a/internal/api/grpc/auth/passwordless.go +++ b/internal/api/grpc/auth/passwordless.go @@ -41,7 +41,7 @@ func (s *Server) ListMyPasswordless(ctx context.Context, _ *auth_pb.ListMyPasswo func (s *Server) AddMyPasswordless(ctx context.Context, _ *auth_pb.AddMyPasswordlessRequest) (*auth_pb.AddMyPasswordlessResponse, error) { ctxData := authz.GetCtxData(ctx) - token, err := s.command.HumanAddPasswordlessSetup(ctx, ctxData.UserID, ctxData.ResourceOwner, false, domain.AuthenticatorAttachmentUnspecified) + token, err := s.command.HumanAddPasswordlessSetup(ctx, ctxData.UserID, ctxData.ResourceOwner, domain.AuthenticatorAttachmentUnspecified) if err != nil { return nil, err } diff --git a/internal/api/grpc/user/v2/otp_integration_test.go b/internal/api/grpc/user/v2/otp_integration_test.go index 7471675c7e..7f4c4a0f43 100644 --- a/internal/api/grpc/user/v2/otp_integration_test.go +++ b/internal/api/grpc/user/v2/otp_integration_test.go @@ -19,12 +19,26 @@ func TestServer_AddOTPSMS(t *testing.T) { Tester.RegisterUserPasskey(CTX, userID) _, sessionToken, _, _ := Tester.CreateVerifiedWebAuthNSession(t, CTX, userID) - // TODO: add when phone can be added to user - /* - userIDPhone := Tester.CreateHumanUser(CTX).GetUserId() - Tester.RegisterUserPasskey(CTX, userIDPhone) - _, sessionTokenPhone, _, _ := Tester.CreateVerifiedWebAuthNSession(t, CTX, userIDPhone) - */ + otherUser := Tester.CreateHumanUser(CTX).GetUserId() + Tester.RegisterUserPasskey(CTX, otherUser) + _, sessionTokenOtherUser, _, _ := Tester.CreateVerifiedWebAuthNSession(t, CTX, otherUser) + + userVerified := Tester.CreateHumanUser(CTX) + _, err := Tester.Client.UserV2.VerifyPhone(CTX, &user.VerifyPhoneRequest{ + UserId: userVerified.GetUserId(), + VerificationCode: userVerified.GetPhoneCode(), + }) + require.NoError(t, err) + Tester.RegisterUserPasskey(CTX, userVerified.GetUserId()) + _, sessionTokenVerified, _, _ := Tester.CreateVerifiedWebAuthNSession(t, CTX, userVerified.GetUserId()) + + userVerified2 := Tester.CreateHumanUser(CTX) + _, err = Tester.Client.UserV2.VerifyPhone(CTX, &user.VerifyPhoneRequest{ + UserId: userVerified2.GetUserId(), + VerificationCode: userVerified2.GetPhoneCode(), + }) + require.NoError(t, err) + type args struct { ctx context.Context req *user.AddOTPSMSRequest @@ -46,9 +60,9 @@ func TestServer_AddOTPSMS(t *testing.T) { { name: "user mismatch", args: args{ - ctx: CTX, + ctx: Tester.WithAuthorizationToken(context.Background(), sessionTokenOtherUser), req: &user.AddOTPSMSRequest{ - UserId: "wrong", + UserId: userID, }, }, wantErr: true, @@ -63,23 +77,34 @@ func TestServer_AddOTPSMS(t *testing.T) { }, wantErr: true, }, - // TODO: add when phone can be added to user - /* - { - name: "add success", - args: args{ - ctx: Tester.WithAuthorizationToken(context.Background(), sessionTokenPhone), - req: &user.AddOTPSMSRequest{ - UserId: userID, - }, - }, - want: &user.AddOTPSMSResponse{ - Details: &object.Details{ - ResourceOwner: Tester.Organisation.ID, - }, + { + name: "add success", + args: args{ + ctx: Tester.WithAuthorizationToken(context.Background(), sessionTokenVerified), + req: &user.AddOTPSMSRequest{ + UserId: userVerified.GetUserId(), }, }, - */ + want: &user.AddOTPSMSResponse{ + Details: &object.Details{ + ResourceOwner: Tester.Organisation.ID, + }, + }, + }, + { + name: "add success, admin", + args: args{ + ctx: CTX, + req: &user.AddOTPSMSRequest{ + UserId: userVerified2.GetUserId(), + }, + }, + want: &user.AddOTPSMSResponse{ + Details: &object.Details{ + ResourceOwner: Tester.Organisation.ID, + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -96,12 +121,21 @@ func TestServer_AddOTPSMS(t *testing.T) { } func TestServer_RemoveOTPSMS(t *testing.T) { - // TODO: add when phone can be added to user - /* - userID := Tester.CreateHumanUser(CTX).GetUserId() - Tester.RegisterUserPasskey(CTX, userID) - _, sessionToken, _, _ := Tester.CreateVerifiedWebAuthNSession(t, CTX, userID) - */ + userID := Tester.CreateHumanUser(CTX).GetUserId() + Tester.RegisterUserPasskey(CTX, userID) + _, sessionToken, _, _ := Tester.CreateVerifiedWebAuthNSession(t, CTX, userID) + + userVerified := Tester.CreateHumanUser(CTX) + Tester.RegisterUserPasskey(CTX, userVerified.GetUserId()) + _, sessionTokenVerified, _, _ := Tester.CreateVerifiedWebAuthNSession(t, CTX, userVerified.GetUserId()) + userVerifiedCtx := Tester.WithAuthorizationToken(context.Background(), sessionTokenVerified) + _, err := Tester.Client.UserV2.VerifyPhone(userVerifiedCtx, &user.VerifyPhoneRequest{ + UserId: userVerified.GetUserId(), + VerificationCode: userVerified.GetPhoneCode(), + }) + require.NoError(t, err) + _, err = Tester.Client.UserV2.AddOTPSMS(userVerifiedCtx, &user.AddOTPSMSRequest{UserId: userVerified.GetUserId()}) + require.NoError(t, err) type args struct { ctx context.Context @@ -116,30 +150,27 @@ func TestServer_RemoveOTPSMS(t *testing.T) { { name: "not added", args: args{ - ctx: CTX, + ctx: Tester.WithAuthorizationToken(context.Background(), sessionToken), req: &user.RemoveOTPSMSRequest{ - UserId: "wrong", + UserId: userID, }, }, wantErr: true, }, - // TODO: add when phone can be added to user - /* - { - name: "success", - args: args{ - ctx: Tester.WithAuthorizationToken(context.Background(), sessionToken), - req: &user.RemoveOTPSMSRequest{ - UserId: userID, - }, - }, - want: &user.RemoveOTPSMSResponse{ - Details: &object.Details{ - ResourceOwner: Tester.Organisation.ResourceOwner, - }, + { + name: "success", + args: args{ + ctx: userVerifiedCtx, + req: &user.RemoveOTPSMSRequest{ + UserId: userVerified.GetUserId(), }, }, - */ + want: &user.RemoveOTPSMSResponse{ + Details: &object.Details{ + ResourceOwner: Tester.Organisation.ResourceOwner, + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -160,6 +191,10 @@ func TestServer_AddOTPEmail(t *testing.T) { Tester.RegisterUserPasskey(CTX, userID) _, sessionToken, _, _ := Tester.CreateVerifiedWebAuthNSession(t, CTX, userID) + otherUser := Tester.CreateHumanUser(CTX).GetUserId() + Tester.RegisterUserPasskey(CTX, otherUser) + _, sessionTokenOtherUser, _, _ := Tester.CreateVerifiedWebAuthNSession(t, CTX, otherUser) + userVerified := Tester.CreateHumanUser(CTX) _, err := Tester.Client.UserV2.VerifyEmail(CTX, &user.VerifyEmailRequest{ UserId: userVerified.GetUserId(), @@ -169,6 +204,13 @@ func TestServer_AddOTPEmail(t *testing.T) { Tester.RegisterUserPasskey(CTX, userVerified.GetUserId()) _, sessionTokenVerified, _, _ := Tester.CreateVerifiedWebAuthNSession(t, CTX, userVerified.GetUserId()) + userVerified2 := Tester.CreateHumanUser(CTX) + _, err = Tester.Client.UserV2.VerifyEmail(CTX, &user.VerifyEmailRequest{ + UserId: userVerified2.GetUserId(), + VerificationCode: userVerified2.GetEmailCode(), + }) + require.NoError(t, err) + type args struct { ctx context.Context req *user.AddOTPEmailRequest @@ -190,9 +232,9 @@ func TestServer_AddOTPEmail(t *testing.T) { { name: "user mismatch", args: args{ - ctx: CTX, + ctx: Tester.WithAuthorizationToken(context.Background(), sessionTokenOtherUser), req: &user.AddOTPEmailRequest{ - UserId: "wrong", + UserId: userID, }, }, wantErr: true, @@ -222,6 +264,21 @@ func TestServer_AddOTPEmail(t *testing.T) { }, }, }, + { + name: "add success, admin", + args: args{ + ctx: CTX, + req: &user.AddOTPEmailRequest{ + UserId: userVerified2.GetUserId(), + }, + }, + want: &user.AddOTPEmailResponse{ + Details: &object.Details{ + ChangeDate: timestamppb.Now(), + ResourceOwner: Tester.Organisation.ID, + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/internal/api/grpc/user/v2/totp_integration_test.go b/internal/api/grpc/user/v2/totp_integration_test.go index 086a281af9..cf7e6ffa3a 100644 --- a/internal/api/grpc/user/v2/totp_integration_test.go +++ b/internal/api/grpc/user/v2/totp_integration_test.go @@ -23,6 +23,11 @@ func TestServer_RegisterTOTP(t *testing.T) { _, sessionToken, _, _ := Tester.CreateVerifiedWebAuthNSession(t, CTX, userID) ctx := Tester.WithAuthorizationToken(CTX, sessionToken) + otherUser := Tester.CreateHumanUser(CTX).GetUserId() + Tester.RegisterUserPasskey(CTX, otherUser) + _, sessionTokenOtherUser, _, _ := Tester.CreateVerifiedWebAuthNSession(t, CTX, otherUser) + ctxOtherUser := Tester.WithAuthorizationToken(CTX, sessionTokenOtherUser) + type args struct { ctx context.Context req *user.RegisterTOTPRequest @@ -44,13 +49,28 @@ func TestServer_RegisterTOTP(t *testing.T) { { name: "user mismatch", args: args{ - ctx: ctx, + ctx: ctxOtherUser, req: &user.RegisterTOTPRequest{ - UserId: "wrong", + UserId: userID, }, }, wantErr: true, }, + { + name: "admin", + args: args{ + ctx: CTX, + req: &user.RegisterTOTPRequest{ + UserId: userID, + }, + }, + want: &user.RegisterTOTPResponse{ + Details: &object.Details{ + ChangeDate: timestamppb.Now(), + ResourceOwner: Tester.Organisation.ID, + }, + }, + }, { name: "success", args: args{ @@ -96,6 +116,18 @@ func TestServer_VerifyTOTPRegistration(t *testing.T) { code, err := totp.GenerateCode(reg.Secret, time.Now()) require.NoError(t, err) + otherUser := Tester.CreateHumanUser(CTX).GetUserId() + Tester.RegisterUserPasskey(CTX, otherUser) + _, sessionTokenOtherUser, _, _ := Tester.CreateVerifiedWebAuthNSession(t, CTX, otherUser) + ctxOtherUser := Tester.WithAuthorizationToken(CTX, sessionTokenOtherUser) + + regOtherUser, err := Client.RegisterTOTP(CTX, &user.RegisterTOTPRequest{ + UserId: otherUser, + }) + require.NoError(t, err) + codeOtherUser, err := totp.GenerateCode(regOtherUser.Secret, time.Now()) + require.NoError(t, err) + type args struct { ctx context.Context req *user.VerifyTOTPRegistrationRequest @@ -109,9 +141,9 @@ func TestServer_VerifyTOTPRegistration(t *testing.T) { { name: "user mismatch", args: args{ - ctx: ctx, + ctx: ctxOtherUser, req: &user.VerifyTOTPRegistrationRequest{ - UserId: "wrong", + UserId: userID, }, }, wantErr: true, @@ -143,6 +175,22 @@ func TestServer_VerifyTOTPRegistration(t *testing.T) { }, }, }, + { + name: "success, admin", + args: args{ + ctx: CTX, + req: &user.VerifyTOTPRegistrationRequest{ + UserId: otherUser, + Code: codeOtherUser, + }, + }, + want: &user.VerifyTOTPRegistrationResponse{ + Details: &object.Details{ + ChangeDate: timestamppb.Now(), + ResourceOwner: Tester.Organisation.ResourceOwner, + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/internal/api/grpc/user/v2/u2f_integration_test.go b/internal/api/grpc/user/v2/u2f_integration_test.go index 78c620e3c2..3b7fbd293c 100644 --- a/internal/api/grpc/user/v2/u2f_integration_test.go +++ b/internal/api/grpc/user/v2/u2f_integration_test.go @@ -18,10 +18,13 @@ import ( func TestServer_RegisterU2F(t *testing.T) { userID := Tester.CreateHumanUser(CTX).GetUserId() + otherUser := Tester.CreateHumanUser(CTX).GetUserId() // We also need a user session Tester.RegisterUserPasskey(CTX, userID) _, sessionToken, _, _ := Tester.CreateVerifiedWebAuthNSession(t, CTX, userID) + Tester.RegisterUserPasskey(CTX, otherUser) + _, sessionTokenOtherUser, _, _ := Tester.CreateVerifiedWebAuthNSession(t, CTX, otherUser) type args struct { ctx context.Context @@ -42,13 +45,28 @@ func TestServer_RegisterU2F(t *testing.T) { wantErr: true, }, { - name: "user mismatch", + name: "admin user", args: args{ ctx: CTX, req: &user.RegisterU2FRequest{ UserId: userID, }, }, + want: &user.RegisterU2FResponse{ + Details: &object.Details{ + ChangeDate: timestamppb.Now(), + ResourceOwner: Tester.Organisation.ID, + }, + }, + }, + { + name: "other user, no permission", + args: args{ + ctx: Tester.WithAuthorizationToken(CTX, sessionTokenOtherUser), + req: &user.RegisterU2FRequest{ + UserId: userID, + }, + }, wantErr: true, }, { diff --git a/internal/api/ui/login/mfa_init_u2f.go b/internal/api/ui/login/mfa_init_u2f.go index 2cd1029ee5..0b7718bb80 100644 --- a/internal/api/ui/login/mfa_init_u2f.go +++ b/internal/api/ui/login/mfa_init_u2f.go @@ -21,7 +21,7 @@ func (l *Login) renderRegisterU2F(w http.ResponseWriter, r *http.Request, authRe var errID, errMessage, credentialData string var u2f *domain.WebAuthNToken if err == nil { - u2f, err = l.command.HumanAddU2FSetup(setContext(r.Context(), authReq.UserOrgID), authReq.UserID, authReq.UserOrgID, true) + u2f, err = l.command.HumanAddU2FSetup(setUserContext(r.Context(), authReq.UserID, authReq.UserOrgID), authReq.UserID, authReq.UserOrgID) } if err != nil { errID, errMessage = l.getErrorMessage(r, err) @@ -54,7 +54,7 @@ func (l *Login) handleRegisterU2F(w http.ResponseWriter, r *http.Request) { } userAgentID, _ := http_mw.UserAgentIDFromCtx(r.Context()) - if _, err = l.command.HumanVerifyU2FSetup(setContext(r.Context(), authReq.UserOrgID), authReq.UserID, authReq.UserOrgID, data.Name, userAgentID, credData); err != nil { + if _, err = l.command.HumanVerifyU2FSetup(setUserContext(r.Context(), authReq.UserID, authReq.UserOrgID), authReq.UserID, authReq.UserOrgID, data.Name, userAgentID, credData); err != nil { l.renderRegisterU2F(w, r, authReq, err) return } diff --git a/internal/api/ui/login/mfa_init_verify_handler.go b/internal/api/ui/login/mfa_init_verify_handler.go index d8e930b19b..0b66451434 100644 --- a/internal/api/ui/login/mfa_init_verify_handler.go +++ b/internal/api/ui/login/mfa_init_verify_handler.go @@ -50,7 +50,7 @@ func (l *Login) handleMFAInitVerify(w http.ResponseWriter, r *http.Request) { func (l *Login) handleOTPVerify(w http.ResponseWriter, r *http.Request, authReq *domain.AuthRequest, data *mfaInitVerifyData) *mfaVerifyData { userAgentID, _ := http_mw.UserAgentIDFromCtx(r.Context()) - _, err := l.command.HumanCheckMFATOTPSetup(setContext(r.Context(), authReq.UserOrgID), authReq.UserID, data.Code, userAgentID, authReq.UserOrgID) + _, err := l.command.HumanCheckMFATOTPSetup(setUserContext(r.Context(), authReq.UserID, authReq.UserOrgID), authReq.UserID, data.Code, userAgentID, authReq.UserOrgID) if err == nil { return nil } diff --git a/internal/api/ui/login/mfa_prompt_handler.go b/internal/api/ui/login/mfa_prompt_handler.go index bee8df2160..ade1b53229 100644 --- a/internal/api/ui/login/mfa_prompt_handler.go +++ b/internal/api/ui/login/mfa_prompt_handler.go @@ -96,7 +96,7 @@ func (l *Login) handleMFACreation(w http.ResponseWriter, r *http.Request, authRe } func (l *Login) handleTOTPCreation(w http.ResponseWriter, r *http.Request, authReq *domain.AuthRequest, data *mfaVerifyData) { - otp, err := l.command.AddHumanTOTP(setContext(r.Context(), authReq.UserOrgID), authReq.UserID, authReq.UserOrgID) + otp, err := l.command.AddHumanTOTP(setUserContext(r.Context(), authReq.UserID, authReq.UserOrgID), authReq.UserID, authReq.UserOrgID) if err != nil { l.renderError(w, r, authReq, err) return diff --git a/internal/api/ui/login/passwordless_registration_handler.go b/internal/api/ui/login/passwordless_registration_handler.go index 4c2d379b48..0346374cee 100644 --- a/internal/api/ui/login/passwordless_registration_handler.go +++ b/internal/api/ui/login/passwordless_registration_handler.go @@ -87,9 +87,9 @@ func (l *Login) renderPasswordlessRegistration(w http.ResponseWriter, r *http.Re var webAuthNToken *domain.WebAuthNToken if err == nil { if authReq != nil { - webAuthNToken, err = l.authRepo.BeginPasswordlessSetup(setContext(r.Context(), authReq.UserOrgID), userID, authReq.UserOrgID, domain.AuthenticatorAttachment(requestedPlatformType)) + webAuthNToken, err = l.authRepo.BeginPasswordlessSetup(setUserContext(r.Context(), userID, authReq.UserOrgID), userID, authReq.UserOrgID, domain.AuthenticatorAttachment(requestedPlatformType)) } else { - webAuthNToken, err = l.authRepo.BeginPasswordlessInitCodeSetup(setContext(r.Context(), orgID), userID, orgID, codeID, code, domain.AuthenticatorAttachment(requestedPlatformType)) + webAuthNToken, err = l.authRepo.BeginPasswordlessInitCodeSetup(setUserContext(r.Context(), userID, orgID), userID, orgID, codeID, code, domain.AuthenticatorAttachment(requestedPlatformType)) } } if err != nil { diff --git a/internal/auth/repository/eventsourcing/eventstore/auth_request.go b/internal/auth/repository/eventsourcing/eventstore/auth_request.go index efdb20505a..129369a71f 100644 --- a/internal/auth/repository/eventsourcing/eventstore/auth_request.go +++ b/internal/auth/repository/eventsourcing/eventstore/auth_request.go @@ -447,7 +447,7 @@ func (repo *AuthRequestRepo) VerifyMFAU2F(ctx context.Context, userID, resourceO func (repo *AuthRequestRepo) BeginPasswordlessSetup(ctx context.Context, userID, resourceOwner string, authenticatorPlatform domain.AuthenticatorAttachment) (login *domain.WebAuthNToken, err error) { ctx, span := tracing.NewSpan(ctx) defer func() { span.EndWithError(err) }() - return repo.Command.HumanAddPasswordlessSetup(ctx, userID, resourceOwner, true, authenticatorPlatform) + return repo.Command.HumanAddPasswordlessSetup(ctx, userID, resourceOwner, authenticatorPlatform) } func (repo *AuthRequestRepo) VerifyPasswordlessSetup(ctx context.Context, userID, resourceOwner, userAgentID, tokenName string, credentialData []byte) (err error) { diff --git a/internal/command/user_human_otp.go b/internal/command/user_human_otp.go index fd717c0de4..a24751f570 100644 --- a/internal/command/user_human_otp.go +++ b/internal/command/user_human_otp.go @@ -73,6 +73,11 @@ 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 + } + } org, err := c.getOrg(ctx, human.ResourceOwner) if err != nil { logging.WithError(err).WithField("traceID", tracing.TraceIDFromCtx(ctx)).Debug("unable to get org for loginname") @@ -124,6 +129,11 @@ 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 existingOTP.State == domain.MFAStateUnspecified || existingOTP.State == domain.MFAStateRemoved { return nil, zerrors.ThrowNotFound(nil, "COMMAND-3Mif9s", "Errors.User.MFA.OTP.NotExisting") } @@ -238,13 +248,15 @@ func (c *Commands) addHumanOTPSMS(ctx context.Context, userID, resourceOwner str if userID == "" { return nil, zerrors.ThrowInvalidArgument(nil, "COMMAND-QSF2s", "Errors.User.UserIDMissing") } - if err := authz.UserIDInCTX(ctx, userID); err != nil { - return nil, err - } otpWriteModel, err := c.otpSMSWriteModelByID(ctx, userID, resourceOwner) 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 otpWriteModel.otpAdded { return nil, zerrors.ThrowAlreadyExists(nil, "COMMAND-Ad3g2", "Errors.User.MFA.OTP.AlreadyReady") } @@ -365,6 +377,11 @@ 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 otpWriteModel.otpAdded { return nil, zerrors.ThrowAlreadyExists(nil, "COMMAND-MKL2s", "Errors.User.MFA.OTP.AlreadyReady") } diff --git a/internal/command/user_human_otp_test.go b/internal/command/user_human_otp_test.go index 838e2357c6..0cfba917c7 100644 --- a/internal/command/user_human_otp_test.go +++ b/internal/command/user_human_otp_test.go @@ -25,7 +25,8 @@ import ( func TestCommandSide_AddHumanTOTP(t *testing.T) { type fields struct { - eventstore *eventstore.Eventstore + eventstore func(t *testing.T) *eventstore.Eventstore + permissionCheck domain.PermissionCheck } type ( args struct { @@ -47,12 +48,10 @@ func TestCommandSide_AddHumanTOTP(t *testing.T) { { name: "userid missing, invalid argument error", fields: fields{ - eventstore: eventstoreExpect( - t, - ), + eventstore: expectEventstore(), }, args: args{ - ctx: context.Background(), + ctx: authz.NewMockContext("instanceID", "org1", "user1"), orgID: "org1", userID: "", }, @@ -63,13 +62,12 @@ func TestCommandSide_AddHumanTOTP(t *testing.T) { { name: "user not existing, not found error", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter(), ), }, args: args{ - ctx: context.Background(), + ctx: authz.NewMockContext("instanceID", "org1", "user1"), orgID: "org1", userID: "user1", }, @@ -77,11 +75,40 @@ func TestCommandSide_AddHumanTOTP(t *testing.T) { err: zerrors.IsPreconditionFailed, }, }, + { + name: "other user, no permission error", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + user.NewHumanAddedEvent(context.Background(), + &user.NewAggregate("user2", "org1").Aggregate, + "username", + "firstname", + "lastname", + "nickname", + "displayname", + language.German, + domain.GenderUnspecified, + "email@test.ch", + true, + ), + ), + ), + permissionCheck: newMockPermissionCheckNotAllowed(), + }, + args: args{ + ctx: authz.NewMockContext("instanceID", "org1", "user1"), + orgID: "org1", + userID: "user2", + }, + res: res{ + err: zerrors.IsPermissionDenied, + }, + }, { name: "org not existing, not found error", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( user.NewHumanAddedEvent(context.Background(), &user.NewAggregate("user1", "org1").Aggregate, @@ -100,7 +127,7 @@ func TestCommandSide_AddHumanTOTP(t *testing.T) { ), }, args: args{ - ctx: context.Background(), + ctx: authz.NewMockContext("instanceID", "org1", "user1"), orgID: "org1", userID: "user1", }, @@ -111,8 +138,7 @@ func TestCommandSide_AddHumanTOTP(t *testing.T) { { name: "org iam policy not existing, not found error", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( user.NewHumanAddedEvent(context.Background(), &user.NewAggregate("user1", "org1").Aggregate, @@ -138,7 +164,7 @@ func TestCommandSide_AddHumanTOTP(t *testing.T) { ), }, args: args{ - ctx: context.Background(), + ctx: authz.NewMockContext("instanceID", "org1", "user1"), orgID: "org1", userID: "user1", }, @@ -149,8 +175,7 @@ func TestCommandSide_AddHumanTOTP(t *testing.T) { { name: "otp already exists, already exists error", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( user.NewHumanAddedEvent(context.Background(), &user.NewAggregate("user1", "org1").Aggregate, @@ -197,7 +222,7 @@ func TestCommandSide_AddHumanTOTP(t *testing.T) { ), }, args: args{ - ctx: context.Background(), + ctx: authz.NewMockContext("instanceID", "org1", "user1"), orgID: "org1", userID: "user1", }, @@ -209,7 +234,8 @@ func TestCommandSide_AddHumanTOTP(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { r := &Commands{ - eventstore: tt.fields.eventstore, + eventstore: tt.fields.eventstore(t), + checkPermission: tt.fields.permissionCheck, } got, err := r.AddHumanTOTP(tt.args.ctx, tt.args.userID, tt.args.orgID) if tt.res.err == nil { @@ -227,7 +253,8 @@ func TestCommandSide_AddHumanTOTP(t *testing.T) { func TestCommands_createHumanTOTP(t *testing.T) { type fields struct { - eventstore *eventstore.Eventstore + eventstore func(t *testing.T) *eventstore.Eventstore + checkPermission domain.PermissionCheck } type args struct { ctx context.Context @@ -244,23 +271,51 @@ func TestCommands_createHumanTOTP(t *testing.T) { { name: "user not existing, not found error", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter(), ), }, args: args{ - ctx: context.Background(), + ctx: authz.NewMockContext("instanceID", "org1", "user1"), resourceOwner: "org1", userID: "user1", }, wantErr: zerrors.ThrowPreconditionFailed(nil, "COMMAND-SqyJz", "Errors.User.NotFound"), }, + { + name: "other user, no permission error", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + user.NewHumanAddedEvent(context.Background(), + &user.NewAggregate("user2", "org1").Aggregate, + "username", + "firstname", + "lastname", + "nickname", + "displayname", + language.German, + domain.GenderUnspecified, + "email@test.ch", + true, + ), + ), + ), + ), + checkPermission: newMockPermissionCheckNotAllowed(), + }, + args: args{ + ctx: authz.NewMockContext("instanceID", "org1", "user1"), + resourceOwner: "org1", + userID: "user2", + }, + wantErr: zerrors.ThrowPermissionDenied(nil, "AUTHZ-HKJD33", "Errors.PermissionDenied"), + }, { name: "org not existing, not found error", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( user.NewHumanAddedEvent(context.Background(), @@ -281,7 +336,7 @@ func TestCommands_createHumanTOTP(t *testing.T) { ), }, args: args{ - ctx: context.Background(), + ctx: authz.NewMockContext("instanceID", "org1", "user1"), resourceOwner: "org1", userID: "user1", }, @@ -290,8 +345,7 @@ func TestCommands_createHumanTOTP(t *testing.T) { { name: "org iam policy not existing, not found error", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( user.NewHumanAddedEvent(context.Background(), @@ -321,7 +375,7 @@ func TestCommands_createHumanTOTP(t *testing.T) { ), }, args: args{ - ctx: context.Background(), + ctx: authz.NewMockContext("instanceID", "org1", "user1"), resourceOwner: "org1", userID: "user1", }, @@ -330,8 +384,7 @@ func TestCommands_createHumanTOTP(t *testing.T) { { name: "otp already exists, already exists error", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( user.NewHumanAddedEvent(context.Background(), @@ -385,7 +438,7 @@ func TestCommands_createHumanTOTP(t *testing.T) { ), }, args: args{ - ctx: context.Background(), + ctx: authz.NewMockContext("instanceID", "org1", "user1"), resourceOwner: "org1", userID: "user1", }, @@ -394,8 +447,7 @@ func TestCommands_createHumanTOTP(t *testing.T) { { name: "issuer not in context", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( user.NewHumanAddedEvent(context.Background(), @@ -434,7 +486,7 @@ func TestCommands_createHumanTOTP(t *testing.T) { ), }, args: args{ - ctx: context.Background(), + ctx: authz.NewMockContext("instanceID", "org1", "user1"), resourceOwner: "org1", userID: "user1", }, @@ -443,8 +495,7 @@ func TestCommands_createHumanTOTP(t *testing.T) { { name: "success", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( user.NewHumanAddedEvent(context.Background(), @@ -483,17 +534,67 @@ func TestCommands_createHumanTOTP(t *testing.T) { ), }, args: args{ - ctx: authz.WithRequestedDomain(context.Background(), "zitadel.com"), + ctx: authz.WithRequestedDomain(authz.NewMockContext("instanceID", "org1", "user1"), "zitadel.com"), resourceOwner: "org1", userID: "user1", }, want: true, }, + { + name: "success, other user", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + user.NewHumanAddedEvent(context.Background(), + &user.NewAggregate("user2", "org1").Aggregate, + "username", + "firstname", + "lastname", + "nickname", + "displayname", + language.German, + domain.GenderUnspecified, + "email@test.ch", + true, + ), + ), + ), + expectFilter( + eventFromEventPusher( + org.NewOrgAddedEvent(context.Background(), + &user.NewAggregate("org1", "org1").Aggregate, + "org", + ), + ), + ), + expectFilter( + eventFromEventPusher( + org.NewDomainPolicyAddedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + true, + true, + true, + ), + ), + ), + expectFilter(), + ), + checkPermission: newMockPermissionCheckAllowed(), + }, + args: args{ + ctx: authz.WithRequestedDomain(authz.NewMockContext("instanceID", "org1", "user1"), "zitadel.com"), + resourceOwner: "org1", + userID: "user2", + }, + want: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { c := &Commands{ - eventstore: tt.fields.eventstore, + eventstore: tt.fields.eventstore(t), + checkPermission: tt.fields.checkPermission, multifactors: domain.MultifactorConfigs{ OTP: domain.OTPConfig{ CryptoMFA: crypto.CreateMockEncryptionAlg(gomock.NewController(t)), @@ -522,12 +623,14 @@ func TestCommands_HumanCheckMFATOTPSetup(t *testing.T) { key, secret, err := domain.NewTOTPKey("example.com", "user1", cryptoAlg) require.NoError(t, err) userAgg := &user.NewAggregate("user1", "org1").Aggregate + userAgg2 := &user.NewAggregate("user2", "org1").Aggregate code, err := totp.GenerateCode(key.Secret(), time.Now()) require.NoError(t, err) type fields struct { - eventstore *eventstore.Eventstore + eventstore func(t *testing.T) *eventstore.Eventstore + checkPermission domain.PermissionCheck } type args struct { userID string @@ -542,14 +645,17 @@ func TestCommands_HumanCheckMFATOTPSetup(t *testing.T) { wantErr error }{ { - name: "missing user id", + name: "missing user id", + fields: fields{ + eventstore: expectEventstore(), + }, args: args{}, wantErr: zerrors.ThrowPreconditionFailed(nil, "COMMAND-8N9ds", "Errors.User.UserIDMissing"), }, { name: "filter error", fields: fields{ - eventstore: eventstoreExpect(t, + eventstore: expectEventstore( expectFilterError(io.ErrClosedPipe), ), }, @@ -559,11 +665,28 @@ func TestCommands_HumanCheckMFATOTPSetup(t *testing.T) { }, wantErr: io.ErrClosedPipe, }, + { + name: "other user, no permission error", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + user.NewHumanOTPAddedEvent(ctx, userAgg2, secret), + ), + ), + ), + checkPermission: newMockPermissionCheckNotAllowed(), + }, + args: args{ + resourceOwner: "org1", + userID: "user2", + }, + wantErr: zerrors.ThrowPermissionDenied(nil, "AUTHZ-HKJD33", "Errors.PermissionDenied"), + }, { name: "otp not existing error", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( user.NewHumanOTPAddedEvent(ctx, userAgg, secret), @@ -583,8 +706,7 @@ func TestCommands_HumanCheckMFATOTPSetup(t *testing.T) { { name: "otp already ready error", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( user.NewHumanOTPAddedEvent(ctx, userAgg, secret), @@ -607,8 +729,7 @@ func TestCommands_HumanCheckMFATOTPSetup(t *testing.T) { { name: "wrong code", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( user.NewHumanOTPAddedEvent(ctx, userAgg, secret), @@ -626,8 +747,7 @@ func TestCommands_HumanCheckMFATOTPSetup(t *testing.T) { { name: "push error", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( user.NewHumanOTPAddedEvent(ctx, userAgg, secret), @@ -651,8 +771,7 @@ func TestCommands_HumanCheckMFATOTPSetup(t *testing.T) { { name: "success", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( user.NewHumanOTPAddedEvent(ctx, userAgg, secret), @@ -672,11 +791,36 @@ func TestCommands_HumanCheckMFATOTPSetup(t *testing.T) { userID: "user1", }, }, + { + name: "success, other user", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + user.NewHumanOTPAddedEvent(ctx, userAgg2, secret), + ), + ), + expectPush( + user.NewHumanOTPVerifiedEvent(ctx, + userAgg2, + "agent1", + ), + ), + ), + checkPermission: newMockPermissionCheckAllowed(), + }, + args: args{ + resourceOwner: "org1", + code: code, + userID: "user2", + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { c := &Commands{ - eventstore: tt.fields.eventstore, + eventstore: tt.fields.eventstore(t), + checkPermission: tt.fields.checkPermission, multifactors: domain.MultifactorConfigs{ OTP: domain.OTPConfig{ CryptoMFA: cryptoAlg, @@ -695,7 +839,7 @@ func TestCommands_HumanCheckMFATOTPSetup(t *testing.T) { func TestCommandSide_RemoveHumanTOTP(t *testing.T) { type fields struct { - eventstore *eventstore.Eventstore + eventstore func(t *testing.T) *eventstore.Eventstore } type ( args struct { @@ -717,9 +861,7 @@ func TestCommandSide_RemoveHumanTOTP(t *testing.T) { { name: "userid missing, invalid argument error", fields: fields{ - eventstore: eventstoreExpect( - t, - ), + eventstore: expectEventstore(), }, args: args{ ctx: context.Background(), @@ -733,8 +875,7 @@ func TestCommandSide_RemoveHumanTOTP(t *testing.T) { { name: "otp not existing, not found error", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter(), ), }, @@ -750,8 +891,7 @@ func TestCommandSide_RemoveHumanTOTP(t *testing.T) { { name: "otp not existing, not found error", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( user.NewHumanOTPAddedEvent(context.Background(), @@ -782,7 +922,7 @@ func TestCommandSide_RemoveHumanTOTP(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { r := &Commands{ - eventstore: tt.fields.eventstore, + eventstore: tt.fields.eventstore(t), } got, err := r.HumanRemoveTOTP(tt.args.ctx, tt.args.userID, tt.args.orgID) if tt.res.err == nil { @@ -801,7 +941,8 @@ func TestCommandSide_RemoveHumanTOTP(t *testing.T) { func TestCommandSide_AddHumanOTPSMS(t *testing.T) { ctx := authz.NewMockContext("inst1", "org1", "user1") type fields struct { - eventstore func(*testing.T) *eventstore.Eventstore + eventstore func(*testing.T) *eventstore.Eventstore + checkPermission domain.PermissionCheck } type ( args struct { @@ -837,15 +978,24 @@ func TestCommandSide_AddHumanOTPSMS(t *testing.T) { { name: "wrong user, permission denied error", fields: fields{ - eventstore: expectEventstore(), + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + user.NewHumanOTPSMSAddedEvent(ctx, + &user.NewAggregate("user2", "org1").Aggregate, + ), + ), + ), + ), + checkPermission: newMockPermissionCheckNotAllowed(), }, args: args{ ctx: ctx, - userID: "other", + userID: "user2", resourceOwner: "org1", }, res: res{ - err: zerrors.ThrowPermissionDenied(nil, "AUTH-Bohd2", "Errors.User.UserIDWrong"), + err: zerrors.ThrowPermissionDenied(nil, "AUTHZ-HKJD33", "Errors.PermissionDenied"), }, }, { @@ -954,11 +1104,48 @@ func TestCommandSide_AddHumanOTPSMS(t *testing.T) { }, }, }, + { + name: "successful add, other user", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + user.NewHumanPhoneChangedEvent(ctx, + &user.NewAggregate("user2", "org1").Aggregate, + "+4179654321", + ), + ), + eventFromEventPusher( + user.NewHumanPhoneVerifiedEvent(ctx, + &user.NewAggregate("user2", "org1").Aggregate, + ), + ), + ), + expectPush( + user.NewHumanOTPSMSAddedEvent(ctx, + &user.NewAggregate("user2", "org1").Aggregate, + ), + ), + ), + checkPermission: newMockPermissionCheckAllowed(), + }, + args: args{ + ctx: ctx, + userID: "user2", + resourceOwner: "org1", + }, + res: res{ + want: &domain.ObjectDetails{ + ResourceOwner: "org1", + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { r := &Commands{ - eventstore: tt.fields.eventstore(t), + eventstore: tt.fields.eventstore(t), + checkPermission: tt.fields.checkPermission, } got, err := r.AddHumanOTPSMS(tt.args.ctx, tt.args.userID, tt.args.resourceOwner) assert.ErrorIs(t, err, tt.res.err) @@ -1942,7 +2129,8 @@ func TestCommandSide_HumanCheckOTPSMS(t *testing.T) { func TestCommandSide_AddHumanOTPEmail(t *testing.T) { ctx := authz.NewMockContext("inst1", "org1", "user1") type fields struct { - eventstore func(*testing.T) *eventstore.Eventstore + eventstore func(*testing.T) *eventstore.Eventstore + checkPermission domain.PermissionCheck } type ( args struct { @@ -1975,6 +2163,29 @@ func TestCommandSide_AddHumanOTPEmail(t *testing.T) { err: zerrors.ThrowInvalidArgument(nil, "COMMAND-Sg1hz", "Errors.User.UserIDMissing"), }, }, + { + name: "wrong user, permission denied error", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + user.NewHumanOTPEmailAddedEvent(ctx, + &user.NewAggregate("user2", "org1").Aggregate, + ), + ), + ), + ), + checkPermission: newMockPermissionCheckNotAllowed(), + }, + args: args{ + ctx: ctx, + userID: "user2", + resourceOwner: "org1", + }, + res: res{ + err: zerrors.ThrowPermissionDenied(nil, "AUTHZ-HKJD33", "Errors.PermissionDenied"), + }, + }, { name: "otp email already exists, already exists error", fields: fields{ @@ -2048,11 +2259,48 @@ func TestCommandSide_AddHumanOTPEmail(t *testing.T) { }, }, }, + { + name: "successful add, other user", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + user.NewHumanEmailChangedEvent(ctx, + &user.NewAggregate("user2", "org1").Aggregate, + "email@test.ch", + ), + ), + eventFromEventPusher( + user.NewHumanEmailVerifiedEvent(ctx, + &user.NewAggregate("user2", "org1").Aggregate, + ), + ), + ), + expectPush( + user.NewHumanOTPEmailAddedEvent(ctx, + &user.NewAggregate("user2", "org1").Aggregate, + ), + ), + ), + checkPermission: newMockPermissionCheckAllowed(), + }, + args: args{ + ctx: ctx, + userID: "user2", + resourceOwner: "org1", + }, + res: res{ + want: &domain.ObjectDetails{ + ResourceOwner: "org1", + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { r := &Commands{ - eventstore: tt.fields.eventstore(t), + eventstore: tt.fields.eventstore(t), + checkPermission: tt.fields.checkPermission, } got, err := r.AddHumanOTPEmail(tt.args.ctx, tt.args.userID, tt.args.resourceOwner) assert.ErrorIs(t, err, tt.res.err) diff --git a/internal/command/user_human_webauthn.go b/internal/command/user_human_webauthn.go index 5588ed7286..6988b0052a 100644 --- a/internal/command/user_human_webauthn.go +++ b/internal/command/user_human_webauthn.go @@ -6,6 +6,7 @@ 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" @@ -77,7 +78,7 @@ func (c *Commands) getHumanPasswordlessLogin(ctx context.Context, userID, authRe }, nil } -func (c *Commands) HumanAddU2FSetup(ctx context.Context, userID, resourceowner string, isLoginUI bool) (*domain.WebAuthNToken, error) { +func (c *Commands) HumanAddU2FSetup(ctx context.Context, userID, resourceowner string) (*domain.WebAuthNToken, error) { u2fTokens, err := c.getHumanU2FTokens(ctx, userID, resourceowner) if err != nil { return nil, err @@ -103,7 +104,7 @@ func (c *Commands) HumanAddU2FSetup(ctx context.Context, userID, resourceowner s return createdWebAuthN, nil } -func (c *Commands) HumanAddPasswordlessSetup(ctx context.Context, userID, resourceowner string, isLoginUI bool, authenticatorPlatform domain.AuthenticatorAttachment) (*domain.WebAuthNToken, error) { +func (c *Commands) HumanAddPasswordlessSetup(ctx context.Context, userID, resourceowner string, authenticatorPlatform domain.AuthenticatorAttachment) (*domain.WebAuthNToken, error) { passwordlessTokens, err := c.getHumanPasswordlessTokens(ctx, userID, resourceowner) if err != nil { return nil, err @@ -134,7 +135,7 @@ func (c *Commands) HumanAddPasswordlessSetupInitCode(ctx context.Context, userID if err != nil { return nil, err } - return c.HumanAddPasswordlessSetup(ctx, userID, resourceowner, true, preferredPlatformType) + return c.HumanAddPasswordlessSetup(ctx, userID, resourceowner, preferredPlatformType) } func (c *Commands) addHumanWebAuthN(ctx context.Context, userID, resourceowner, rpID string, tokens []*domain.WebAuthNToken, authenticatorPlatform domain.AuthenticatorAttachment, userVerification domain.UserVerificationRequirement) (*HumanWebAuthNWriteModel, *eventstore.Aggregate, *domain.WebAuthNToken, error) { @@ -145,6 +146,11 @@ 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 + } + } org, err := c.getOrg(ctx, user.ResourceOwner) if err != nil { return nil, nil, nil, err @@ -179,7 +185,7 @@ func (c *Commands) HumanVerifyU2FSetup(ctx context.Context, userID, resourceowne if err != nil { return nil, err } - userAgg, webAuthN, verifyWebAuthN, err := c.verifyHumanWebAuthN(ctx, userID, resourceowner, tokenName, userAgentID, credentialData, u2fTokens) + userAgg, webAuthN, verifyWebAuthN, err := c.verifyHumanWebAuthN(ctx, userID, resourceowner, tokenName, credentialData, u2fTokens) if err != nil { return nil, err } @@ -230,7 +236,7 @@ func (c *Commands) humanHumanPasswordlessSetup(ctx context.Context, userID, reso if err != nil { return nil, err } - userAgg, webAuthN, verifyWebAuthN, err := c.verifyHumanWebAuthN(ctx, userID, resourceowner, tokenName, userAgentID, credentialData, u2fTokens) + userAgg, webAuthN, verifyWebAuthN, err := c.verifyHumanWebAuthN(ctx, userID, resourceowner, tokenName, credentialData, u2fTokens) if err != nil { return nil, err } @@ -263,7 +269,7 @@ func (c *Commands) humanHumanPasswordlessSetup(ctx context.Context, userID, reso return writeModelToObjectDetails(&verifyWebAuthN.WriteModel), nil } -func (c *Commands) verifyHumanWebAuthN(ctx context.Context, userID, resourceowner, tokenName, userAgentID string, credentialData []byte, tokens []*domain.WebAuthNToken) (*eventstore.Aggregate, *domain.WebAuthNToken, *HumanWebAuthNWriteModel, error) { +func (c *Commands) verifyHumanWebAuthN(ctx context.Context, userID, resourceowner, tokenName string, credentialData []byte, tokens []*domain.WebAuthNToken) (*eventstore.Aggregate, *domain.WebAuthNToken, *HumanWebAuthNWriteModel, error) { if userID == "" { return nil, nil, nil, zerrors.ThrowPreconditionFailed(nil, "COMMAND-3M0od", "Errors.IDMissing") } @@ -272,7 +278,7 @@ func (c *Commands) verifyHumanWebAuthN(ctx context.Context, userID, resourceowne return nil, nil, nil, err } _, token := domain.GetTokenToVerify(tokens) - webAuthN, err := c.webauthnConfig.FinishRegistration(ctx, user, token, tokenName, credentialData, userAgentID != "") + webAuthN, err := c.webauthnConfig.FinishRegistration(ctx, user, token, tokenName, credentialData) if err != nil { return nil, nil, nil, err } diff --git a/internal/command/user_v2_passkey_test.go b/internal/command/user_v2_passkey_test.go index 15286bea11..7b1343b862 100644 --- a/internal/command/user_v2_passkey_test.go +++ b/internal/command/user_v2_passkey_test.go @@ -339,7 +339,7 @@ func TestCommands_verifyUserPasskeyCode(t *testing.T) { } func TestCommands_pushUserPasskey(t *testing.T) { - ctx := authz.WithRequestedDomain(context.Background(), "example.com") + ctx := authz.WithRequestedDomain(authz.NewMockContext("instance1", "org1", "user1"), "example.com") webauthnConfig := &webauthn_helper.Config{ DisplayName: "test", ExternalSecure: true, diff --git a/internal/command/user_v2_totp.go b/internal/command/user_v2_totp.go index 61514b0e7e..0ceca9b073 100644 --- a/internal/command/user_v2_totp.go +++ b/internal/command/user_v2_totp.go @@ -3,15 +3,11 @@ package command import ( "context" - "github.com/zitadel/zitadel/internal/api/authz" "github.com/zitadel/zitadel/internal/domain" ) -func (c *Commands) AddUserTOTP(ctx context.Context, userID, resourceowner string) (*domain.TOTP, error) { - if err := authz.UserIDInCTX(ctx, userID); err != nil { - return nil, err - } - prep, err := c.createHumanTOTP(ctx, userID, resourceowner) +func (c *Commands) AddUserTOTP(ctx context.Context, userID, resourceOwner string) (*domain.TOTP, error) { + prep, err := c.createHumanTOTP(ctx, userID, resourceOwner) if err != nil { return nil, err } @@ -26,8 +22,5 @@ func (c *Commands) AddUserTOTP(ctx context.Context, userID, resourceowner string } func (c *Commands) CheckUserTOTP(ctx context.Context, userID, code, resourceOwner string) (*domain.ObjectDetails, error) { - if err := authz.UserIDInCTX(ctx, userID); err != nil { - return nil, err - } return c.HumanCheckMFATOTPSetup(ctx, userID, code, "", resourceOwner) } diff --git a/internal/command/user_v2_totp_test.go b/internal/command/user_v2_totp_test.go index 4e1df191d5..d05bdc9647 100644 --- a/internal/command/user_v2_totp_test.go +++ b/internal/command/user_v2_totp_test.go @@ -23,9 +23,11 @@ import ( func TestCommands_AddUserTOTP(t *testing.T) { ctx := authz.NewMockContext("inst1", "org1", "user1") userAgg := &user.NewAggregate("user1", "org1").Aggregate + userAgg2 := &user.NewAggregate("user2", "org1").Aggregate type fields struct { - eventstore *eventstore.Eventstore + eventstore func(t *testing.T) *eventstore.Eventstore + checkPermission domain.PermissionCheck } type args struct { userID string @@ -39,12 +41,33 @@ func TestCommands_AddUserTOTP(t *testing.T) { wantErr error }{ { - name: "wrong user", + name: "other user, permission error", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + user.NewHumanAddedEvent(ctx, + userAgg, + "username", + "firstname", + "lastname", + "nickname", + "displayname", + language.German, + domain.GenderUnspecified, + "email@test.ch", + true, + ), + ), + ), + ), + checkPermission: newMockPermissionCheckNotAllowed(), + }, args: args{ userID: "foo", resourceowner: "org1", }, - wantErr: zerrors.ThrowPermissionDenied(nil, "AUTH-Bohd2", "Errors.User.UserIDWrong"), + wantErr: zerrors.ThrowPermissionDenied(nil, "AUTHZ-HKJD33", "Errors.PermissionDenied"), }, { name: "create otp error", @@ -53,8 +76,7 @@ func TestCommands_AddUserTOTP(t *testing.T) { resourceowner: "org1", }, fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter(), ), }, @@ -67,8 +89,7 @@ func TestCommands_AddUserTOTP(t *testing.T) { resourceowner: "org1", }, fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( user.NewHumanAddedEvent(ctx, @@ -118,8 +139,7 @@ func TestCommands_AddUserTOTP(t *testing.T) { resourceowner: "org1", }, fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( user.NewHumanAddedEvent(ctx, @@ -162,11 +182,63 @@ func TestCommands_AddUserTOTP(t *testing.T) { }, want: true, }, + { + name: "success, other user", + args: args{ + userID: "user2", + resourceowner: "org1", + }, + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + user.NewHumanAddedEvent(ctx, + userAgg2, + "username", + "firstname", + "lastname", + "nickname", + "displayname", + language.German, + domain.GenderUnspecified, + "email@test.ch", + true, + ), + ), + ), + expectFilter( + eventFromEventPusher( + org.NewOrgAddedEvent(ctx, + userAgg2, + "org", + ), + ), + ), + expectFilter( + eventFromEventPusher( + org.NewDomainPolicyAddedEvent(ctx, + userAgg2, + true, + true, + true, + ), + ), + ), + expectFilter(), + expectRandomPush([]eventstore.Command{ + user.NewHumanOTPAddedEvent(ctx, userAgg2, nil), + }), + ), + checkPermission: newMockPermissionCheckAllowed(), + }, + want: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { c := &Commands{ - eventstore: tt.fields.eventstore, + eventstore: tt.fields.eventstore(t), + checkPermission: tt.fields.checkPermission, multifactors: domain.MultifactorConfigs{ OTP: domain.OTPConfig{ Issuer: "zitadel.com", @@ -198,7 +270,8 @@ func TestCommands_CheckUserTOTP(t *testing.T) { require.NoError(t, err) type fields struct { - eventstore *eventstore.Eventstore + eventstore func(t *testing.T) *eventstore.Eventstore + checkPermission domain.PermissionCheck } type args struct { userID string @@ -213,17 +286,46 @@ func TestCommands_CheckUserTOTP(t *testing.T) { wantErr error }{ { - name: "wrong user id", + name: "other user, no permission, error", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + user.NewHumanOTPAddedEvent(ctx, userAgg, secret), + ), + ), + ), + checkPermission: newMockPermissionCheckNotAllowed(), + }, args: args{ userID: "foo", }, - wantErr: zerrors.ThrowPermissionDenied(nil, "AUTH-Bohd2", "Errors.User.UserIDWrong"), + wantErr: zerrors.ThrowPermissionDenied(nil, "AUTHZ-HKJD33", "Errors.PermissionDenied"), + }, + { + name: "user id, with permission, success", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + user.NewHumanOTPAddedEvent(ctx, &user.NewAggregate("foo", "org1").Aggregate, secret), + ), + ), + expectPush( + user.NewHumanOTPVerifiedEvent(ctx, &user.NewAggregate("foo", "org1").Aggregate, ""), + ), + ), + checkPermission: newMockPermissionCheckAllowed(), + }, + args: args{ + userID: "foo", + code: code, + }, }, { name: "success", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( user.NewHumanOTPAddedEvent(ctx, userAgg, secret), @@ -244,7 +346,8 @@ func TestCommands_CheckUserTOTP(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { c := &Commands{ - eventstore: tt.fields.eventstore, + eventstore: tt.fields.eventstore(t), + checkPermission: tt.fields.checkPermission, multifactors: domain.MultifactorConfigs{ OTP: domain.OTPConfig{ CryptoMFA: cryptoAlg, diff --git a/internal/command/user_v2_u2f.go b/internal/command/user_v2_u2f.go index 50c16aa30f..41ab47373d 100644 --- a/internal/command/user_v2_u2f.go +++ b/internal/command/user_v2_u2f.go @@ -3,16 +3,12 @@ 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/user" ) func (c *Commands) RegisterUserU2F(ctx context.Context, userID, resourceOwner, rpID string) (*domain.WebAuthNRegistrationDetails, error) { - if err := authz.UserIDInCTX(ctx, userID); err != nil { - return nil, err - } return c.registerUserU2F(ctx, userID, resourceOwner, rpID) } diff --git a/internal/command/user_v2_u2f_test.go b/internal/command/user_v2_u2f_test.go index a69dac062e..b2a5788d00 100644 --- a/internal/command/user_v2_u2f_test.go +++ b/internal/command/user_v2_u2f_test.go @@ -1,7 +1,6 @@ package command import ( - "context" "io" "testing" @@ -30,8 +29,9 @@ func TestCommands_RegisterUserU2F(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 + permissionCheck domain.PermissionCheck } type args struct { userID string @@ -45,18 +45,10 @@ func TestCommands_RegisterUserU2F(t *testing.T) { want *domain.WebAuthNRegistrationDetails wantErr error }{ - { - name: "wrong user", - args: args{ - userID: "foo", - resourceOwner: "org1", - }, - wantErr: zerrors.ThrowPermissionDenied(nil, "AUTH-Bohd2", "Errors.User.UserIDWrong"), - }, { name: "get human passwordless error", fields: fields{ - eventstore: eventstoreExpect(t, + eventstore: expectEventstore( expectFilterError(io.ErrClosedPipe), ), }, @@ -66,10 +58,38 @@ func TestCommands_RegisterUserU2F(t *testing.T) { }, wantErr: io.ErrClosedPipe, }, + { + name: "other user, no permission", + fields: fields{ + eventstore: expectEventstore( + expectFilter(), + expectFilter(eventFromEventPusher( + user.NewHumanAddedEvent(ctx, + userAgg, + "username", + "firstname", + "lastname", + "nickname", + "displayname", + language.German, + domain.GenderUnspecified, + "email@test.ch", + true, + ), + )), + ), + permissionCheck: newMockPermissionCheckNotAllowed(), + }, + args: args{ + userID: "foo", + resourceOwner: "org1", + }, + wantErr: zerrors.ThrowPermissionDenied(nil, "AUTHZ-HKJD33", "Errors.PermissionDenied"), + }, { name: "id generator error", fields: fields{ - eventstore: eventstoreExpect(t, + eventstore: expectEventstore( expectFilter(), // getHumanPasswordlessTokens expectFilter(eventFromEventPusher( user.NewHumanAddedEvent(ctx, @@ -110,9 +130,10 @@ func TestCommands_RegisterUserU2F(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, + checkPermission: tt.fields.permissionCheck, + webauthnConfig: webauthnConfig, } _, err := c.RegisterUserU2F(ctx, tt.args.userID, tt.args.resourceOwner, tt.args.rpID) require.ErrorIs(t, err, tt.wantErr) @@ -122,7 +143,7 @@ func TestCommands_RegisterUserU2F(t *testing.T) { } func TestCommands_pushUserU2F(t *testing.T) { - ctx := authz.WithRequestedDomain(context.Background(), "example.com") + ctx := authz.WithRequestedDomain(authz.NewMockContext("instance1", "org1", "user1"), "example.com") webauthnConfig := &webauthn_helper.Config{ DisplayName: "test", ExternalSecure: true, diff --git a/internal/domain/permission.go b/internal/domain/permission.go index fd20f77cf0..cf2d02d426 100644 --- a/internal/domain/permission.go +++ b/internal/domain/permission.go @@ -27,9 +27,10 @@ func (p *Permissions) appendPermission(ctxID, permission string) { type PermissionCheck func(ctx context.Context, permission, orgID, resourceID string) (err error) const ( - PermissionUserWrite = "user.write" - PermissionUserRead = "user.read" - PermissionUserDelete = "user.delete" - PermissionSessionWrite = "session.write" - PermissionSessionDelete = "session.delete" + PermissionUserWrite = "user.write" + PermissionUserRead = "user.read" + PermissionUserDelete = "user.delete" + PermissionUserCredentialWrite = "user.credential.write" + PermissionSessionWrite = "session.write" + PermissionSessionDelete = "session.delete" ) diff --git a/internal/webauthn/webauthn.go b/internal/webauthn/webauthn.go index 329d533631..bb13d28bd9 100644 --- a/internal/webauthn/webauthn.go +++ b/internal/webauthn/webauthn.go @@ -95,7 +95,7 @@ func (w *Config) BeginRegistration(ctx context.Context, user *domain.Human, acco }, nil } -func (w *Config) FinishRegistration(ctx context.Context, user *domain.Human, webAuthN *domain.WebAuthNToken, tokenName string, credData []byte, isLoginUI bool) (*domain.WebAuthNToken, error) { +func (w *Config) FinishRegistration(ctx context.Context, user *domain.Human, webAuthN *domain.WebAuthNToken, tokenName string, credData []byte) (*domain.WebAuthNToken, error) { if webAuthN == nil { return nil, zerrors.ThrowInternal(nil, "WEBAU-5M9so", "Errors.User.WebAuthN.NotFound") }