fix: add missing totp remove endpoint in user v2 API (#8256)

# Which Problems Are Solved

TOTP remove endpoint available in management API, not in user v2 API.

# How the Problems Are Solved

Add endpoint RemoveTOTP to user v2 API.

# Additional Changes

None

# Additional Context

close #6605

---------

Co-authored-by: Livio Spring <livio.a@gmail.com>
This commit is contained in:
Stefan Benz 2024-07-10 14:31:28 +02:00 committed by GitHub
parent 82d950019f
commit 19a8ab02ad
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 160 additions and 6 deletions

View File

@ -12,7 +12,6 @@ func (s *Server) RegisterTOTP(ctx context.Context, req *user.RegisterTOTPRequest
return totpDetailsToPb( return totpDetailsToPb(
s.command.AddUserTOTP(ctx, req.GetUserId(), ""), s.command.AddUserTOTP(ctx, req.GetUserId(), ""),
) )
} }
func totpDetailsToPb(totp *domain.TOTP, err error) (*user.RegisterTOTPResponse, error) { 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), Details: object.DomainToDetailsPb(objectDetails),
}, nil }, 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
}

View File

@ -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)
})
}
}

View File

@ -239,6 +239,11 @@ func (c *Commands) HumanRemoveTOTP(ctx context.Context, userID, resourceOwner st
if existingOTP.State == domain.MFAStateUnspecified || existingOTP.State == domain.MFAStateRemoved { if existingOTP.State == domain.MFAStateUnspecified || existingOTP.State == domain.MFAStateRemoved {
return nil, zerrors.ThrowNotFound(nil, "COMMAND-Hd9sd", "Errors.User.MFA.OTP.NotExisting") 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) userAgg := UserAggregateFromWriteModel(&existingOTP.WriteModel)
pushedEvents, err := c.eventstore.Push(ctx, user.NewHumanOTPRemovedEvent(ctx, userAgg)) pushedEvents, err := c.eventstore.Push(ctx, user.NewHumanOTPRemovedEvent(ctx, userAgg))
if err != nil { if err != nil {

View File

@ -841,7 +841,8 @@ func TestCommands_HumanCheckMFATOTPSetup(t *testing.T) {
func TestCommandSide_RemoveHumanTOTP(t *testing.T) { func TestCommandSide_RemoveHumanTOTP(t *testing.T) {
type fields struct { type fields struct {
eventstore func(t *testing.T) *eventstore.Eventstore eventstore func(t *testing.T) *eventstore.Eventstore
checkPermission domain.PermissionCheck
} }
type ( type (
args struct { 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{ fields: fields{
eventstore: expectEventstore( eventstore: expectEventstore(
expectFilter( expectFilter(
@ -908,6 +933,7 @@ func TestCommandSide_RemoveHumanTOTP(t *testing.T) {
), ),
), ),
), ),
checkPermission: newMockPermissionCheckAllowed(),
}, },
args: args{ args: args{
ctx: context.Background(), ctx: context.Background(),
@ -924,7 +950,8 @@ func TestCommandSide_RemoveHumanTOTP(t *testing.T) {
for _, tt := range tests { for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
r := &Commands{ 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) got, err := r.HumanRemoveTOTP(tt.args.ctx, tt.args.userID, tt.args.orgID)
if tt.res.err == nil { if tt.res.err == nil {

View File

@ -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) { rpc AddOTPSMS (AddOTPSMSRequest) returns (AddOTPSMSResponse) {
option (google.api.http) = { option (google.api.http) = {
post: "/v2beta/users/{user_id}/otp_sms" post: "/v2beta/users/{user_id}/otp_sms"
@ -690,7 +712,7 @@ service UserService {
}; };
option (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_operation) = { option (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_operation) = {
summary: "Remove One-Time-Password (OTP) SMS from a user"; 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: { responses: {
key: "200" key: "200"
value: { value: {
@ -735,7 +757,7 @@ service UserService {
}; };
option (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_operation) = { option (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_operation) = {
summary: "Remove One-Time-Password (OTP) Email from a user"; 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: { responses: {
key: "200" key: "200"
value: { value: {
@ -1471,6 +1493,22 @@ message VerifyTOTPRegistrationResponse {
zitadel.object.v2beta.Details details = 1; 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 { message AddOTPSMSRequest {
string user_id = 1 [ string user_id = 1 [
(validate.rules).string = {min_len: 1, max_len: 200}, (validate.rules).string = {min_len: 1, max_len: 200},