diff --git a/internal/api/grpc/user/v2/totp.go b/internal/api/grpc/user/v2/totp.go index 691f31a83..e426f5788 100644 --- a/internal/api/grpc/user/v2/totp.go +++ b/internal/api/grpc/user/v2/totp.go @@ -12,7 +12,6 @@ func (s *Server) RegisterTOTP(ctx context.Context, req *user.RegisterTOTPRequest return totpDetailsToPb( s.command.AddUserTOTP(ctx, req.GetUserId(), ""), ) - } func totpDetailsToPb(totp *domain.TOTP, err error) (*user.RegisterTOTPResponse, error) { @@ -35,3 +34,11 @@ func (s *Server) VerifyTOTPRegistration(ctx context.Context, req *user.VerifyTOT Details: object.DomainToDetailsPb(objectDetails), }, nil } + +func (s *Server) RemoveTOTP(ctx context.Context, req *user.RemoveTOTPRequest) (*user.RemoveTOTPResponse, error) { + objectDetails, err := s.command.HumanRemoveTOTP(ctx, req.GetUserId(), "") + if err != nil { + return nil, err + } + return &user.RemoveTOTPResponse{Details: object.DomainToDetailsPb(objectDetails)}, nil +} diff --git a/internal/api/grpc/user/v2/totp_integration_test.go b/internal/api/grpc/user/v2/totp_integration_test.go index cf7e6ffa3..489c189b0 100644 --- a/internal/api/grpc/user/v2/totp_integration_test.go +++ b/internal/api/grpc/user/v2/totp_integration_test.go @@ -205,3 +205,80 @@ func TestServer_VerifyTOTPRegistration(t *testing.T) { }) } } + +func TestServer_RemoveTOTP(t *testing.T) { + 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) + + regOtherUser, err := Client.RegisterTOTP(CTX, &user.RegisterTOTPRequest{ + UserId: userVerified.GetUserId(), + }) + require.NoError(t, err) + codeOtherUser, err := totp.GenerateCode(regOtherUser.Secret, time.Now()) + require.NoError(t, err) + _, err = Client.VerifyTOTPRegistration(userVerifiedCtx, &user.VerifyTOTPRegistrationRequest{ + UserId: userVerified.GetUserId(), + Code: codeOtherUser, + }, + ) + require.NoError(t, err) + + type args struct { + ctx context.Context + req *user.RemoveTOTPRequest + } + tests := []struct { + name string + args args + want *user.RemoveTOTPResponse + wantErr bool + }{ + { + name: "not added", + args: args{ + ctx: Tester.WithAuthorizationToken(context.Background(), sessionToken), + req: &user.RemoveTOTPRequest{ + UserId: userID, + }, + }, + wantErr: true, + }, + { + name: "success", + args: args{ + ctx: userVerifiedCtx, + req: &user.RemoveTOTPRequest{ + UserId: userVerified.GetUserId(), + }, + }, + want: &user.RemoveTOTPResponse{ + Details: &object.Details{ + ResourceOwner: Tester.Organisation.ResourceOwner, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := Client.RemoveTOTP(tt.args.ctx, tt.args.req) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + require.NotNil(t, got) + integration.AssertDetails(t, tt.want, got) + }) + } +} diff --git a/internal/command/user_human_otp.go b/internal/command/user_human_otp.go index bc65ba724..8ecd51751 100644 --- a/internal/command/user_human_otp.go +++ b/internal/command/user_human_otp.go @@ -239,6 +239,11 @@ 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 + } + } userAgg := UserAggregateFromWriteModel(&existingOTP.WriteModel) pushedEvents, err := c.eventstore.Push(ctx, user.NewHumanOTPRemovedEvent(ctx, userAgg)) if err != nil { diff --git a/internal/command/user_human_otp_test.go b/internal/command/user_human_otp_test.go index 81615a4f6..806d24dd4 100644 --- a/internal/command/user_human_otp_test.go +++ b/internal/command/user_human_otp_test.go @@ -841,7 +841,8 @@ func TestCommands_HumanCheckMFATOTPSetup(t *testing.T) { func TestCommandSide_RemoveHumanTOTP(t *testing.T) { type fields struct { - eventstore func(t *testing.T) *eventstore.Eventstore + eventstore func(t *testing.T) *eventstore.Eventstore + checkPermission domain.PermissionCheck } type ( args struct { @@ -891,7 +892,31 @@ func TestCommandSide_RemoveHumanTOTP(t *testing.T) { }, }, { - name: "otp not existing, not found error", + name: "otp, no permission error", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + user.NewHumanOTPAddedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + nil, + ), + ), + ), + ), + checkPermission: newMockPermissionCheckNotAllowed(), + }, + args: args{ + ctx: context.Background(), + orgID: "org1", + userID: "user1", + }, + res: res{ + err: zerrors.IsPermissionDenied, + }, + }, + { + name: "otp remove, ok", fields: fields{ eventstore: expectEventstore( expectFilter( @@ -908,6 +933,7 @@ func TestCommandSide_RemoveHumanTOTP(t *testing.T) { ), ), ), + checkPermission: newMockPermissionCheckAllowed(), }, args: args{ ctx: context.Background(), @@ -924,7 +950,8 @@ func TestCommandSide_RemoveHumanTOTP(t *testing.T) { 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.HumanRemoveTOTP(tt.args.ctx, tt.args.userID, tt.args.orgID) if tt.res.err == nil { diff --git a/proto/zitadel/user/v2beta/user_service.proto b/proto/zitadel/user/v2beta/user_service.proto index f3d8de747..3a0fb2dfa 100644 --- a/proto/zitadel/user/v2beta/user_service.proto +++ b/proto/zitadel/user/v2beta/user_service.proto @@ -655,6 +655,28 @@ service UserService { }; } + rpc RemoveTOTP (RemoveTOTPRequest) returns (RemoveTOTPResponse) { + option (google.api.http) = { + delete: "/v2beta/users/{user_id}/totp" + }; + + option (zitadel.protoc_gen_zitadel.v2.options) = { + auth_option: { + permission: "authenticated" + } + }; + option (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_operation) = { + summary: "Remove TOTP generator from a user"; + description: "Remove the configured TOTP generator of a user. As only one TOTP generator per user is allowed, the user will not have TOTP as a second-factor afterward." + responses: { + key: "200" + value: { + description: "OK"; + } + }; + }; + } + rpc AddOTPSMS (AddOTPSMSRequest) returns (AddOTPSMSResponse) { option (google.api.http) = { post: "/v2beta/users/{user_id}/otp_sms" @@ -690,7 +712,7 @@ service UserService { }; option (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_operation) = { summary: "Remove One-Time-Password (OTP) SMS from a user"; - description: "Remove the configured One-Time-Password (OTP) SMS factor of the authenticated user. As only one OTP SMS per user is allowed, the user will not have OTP SMS as a second-factor afterward." + description: "Remove the configured One-Time-Password (OTP) SMS factor of a user. As only one OTP SMS per user is allowed, the user will not have OTP SMS as a second-factor afterward." responses: { key: "200" value: { @@ -735,7 +757,7 @@ service UserService { }; option (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_operation) = { summary: "Remove One-Time-Password (OTP) Email from a user"; - description: "Remove the configured One-Time-Password (OTP) Email factor of the authenticated user. As only one OTP Email per user is allowed, the user will not have OTP Email as a second-factor afterward." + description: "Remove the configured One-Time-Password (OTP) Email factor of a user. As only one OTP Email per user is allowed, the user will not have OTP Email as a second-factor afterward." responses: { key: "200" value: { @@ -1471,6 +1493,22 @@ message VerifyTOTPRegistrationResponse { zitadel.object.v2beta.Details details = 1; } +message RemoveTOTPRequest { + string user_id = 1 [ + (validate.rules).string = {min_len: 1, max_len: 200}, + (google.api.field_behavior) = REQUIRED, + (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = { + min_length: 1; + max_length: 200; + example: "\"163840776835432705\""; + } + ]; +} + +message RemoveTOTPResponse { + zitadel.object.v2beta.Details details = 1; +} + message AddOTPSMSRequest { string user_id = 1 [ (validate.rules).string = {min_len: 1, max_len: 200},