From 56e33ce1a7b1f30bd8deb5f62fe5fb1937f554df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20M=C3=B6hlmann?= Date: Thu, 22 Jun 2023 12:06:32 +0200 Subject: [PATCH] fix: rename OTP to TOTP in v2 alpha user api This change renames the v2 user OTP registration endpoints and objects to TOTP. Also the v2 related code paths have been renamed to TOTP. This change was discussed during the sprint review. --- internal/api/grpc/user/v2/otp.go | 38 ------------------- internal/api/grpc/user/v2/totp.go | 38 +++++++++++++++++++ ...ation_test.go => totp_integration_test.go} | 34 ++++++++--------- .../user/v2/{otp_test.go => totp_test.go} | 14 +++---- internal/command/user_human_otp.go | 8 ++-- internal/command/user_human_otp_test.go | 2 +- .../{user_v2_otp.go => user_v2_totp.go} | 8 ++-- ...er_v2_otp_test.go => user_v2_totp_test.go} | 8 ++-- internal/domain/human_otp.go | 2 +- proto/zitadel/user/v2alpha/user_service.proto | 26 ++++++------- 10 files changed, 89 insertions(+), 89 deletions(-) delete mode 100644 internal/api/grpc/user/v2/otp.go create mode 100644 internal/api/grpc/user/v2/totp.go rename internal/api/grpc/user/v2/{otp_integration_test.go => totp_integration_test.go} (75%) rename internal/api/grpc/user/v2/{otp_test.go => totp_test.go} (80%) rename internal/command/{user_v2_otp.go => user_v2_totp.go} (67%) rename internal/command/{user_v2_otp_test.go => user_v2_totp_test.go} (95%) diff --git a/internal/api/grpc/user/v2/otp.go b/internal/api/grpc/user/v2/otp.go deleted file mode 100644 index 80ea810076..0000000000 --- a/internal/api/grpc/user/v2/otp.go +++ /dev/null @@ -1,38 +0,0 @@ -package user - -import ( - "context" - - "github.com/zitadel/zitadel/internal/api/authz" - "github.com/zitadel/zitadel/internal/api/grpc/object/v2" - "github.com/zitadel/zitadel/internal/domain" - user "github.com/zitadel/zitadel/pkg/grpc/user/v2alpha" -) - -func (s *Server) RegisterOTP(ctx context.Context, req *user.RegisterOTPRequest) (*user.RegisterOTPResponse, error) { - return otpDetailsToPb( - s.command.AddUserOTP(ctx, req.GetUserId(), authz.GetCtxData(ctx).ResourceOwner), - ) - -} - -func otpDetailsToPb(otp *domain.OTPv2, err error) (*user.RegisterOTPResponse, error) { - if err != nil { - return nil, err - } - return &user.RegisterOTPResponse{ - Details: object.DomainToDetailsPb(otp.ObjectDetails), - Uri: otp.URI, - Secret: otp.Secret, - }, nil -} - -func (s *Server) VerifyOTPRegistration(ctx context.Context, req *user.VerifyOTPRegistrationRequest) (*user.VerifyOTPRegistrationResponse, error) { - objectDetails, err := s.command.CheckUserOTP(ctx, req.GetUserId(), req.GetCode(), authz.GetCtxData(ctx).ResourceOwner) - if err != nil { - return nil, err - } - return &user.VerifyOTPRegistrationResponse{ - Details: object.DomainToDetailsPb(objectDetails), - }, nil -} diff --git a/internal/api/grpc/user/v2/totp.go b/internal/api/grpc/user/v2/totp.go new file mode 100644 index 0000000000..494c558983 --- /dev/null +++ b/internal/api/grpc/user/v2/totp.go @@ -0,0 +1,38 @@ +package user + +import ( + "context" + + "github.com/zitadel/zitadel/internal/api/authz" + "github.com/zitadel/zitadel/internal/api/grpc/object/v2" + "github.com/zitadel/zitadel/internal/domain" + user "github.com/zitadel/zitadel/pkg/grpc/user/v2alpha" +) + +func (s *Server) RegisterTOTP(ctx context.Context, req *user.RegisterTOTPRequest) (*user.RegisterTOTPResponse, error) { + return totpDetailsToPb( + s.command.AddUserTOTP(ctx, req.GetUserId(), authz.GetCtxData(ctx).ResourceOwner), + ) + +} + +func totpDetailsToPb(totp *domain.TOTP, err error) (*user.RegisterTOTPResponse, error) { + if err != nil { + return nil, err + } + return &user.RegisterTOTPResponse{ + Details: object.DomainToDetailsPb(totp.ObjectDetails), + Uri: totp.URI, + Secret: totp.Secret, + }, nil +} + +func (s *Server) VerifyTOTPRegistration(ctx context.Context, req *user.VerifyTOTPRegistrationRequest) (*user.VerifyTOTPRegistrationResponse, error) { + objectDetails, err := s.command.CheckUserTOTP(ctx, req.GetUserId(), req.GetCode(), authz.GetCtxData(ctx).ResourceOwner) + if err != nil { + return nil, err + } + return &user.VerifyTOTPRegistrationResponse{ + Details: object.DomainToDetailsPb(objectDetails), + }, nil +} diff --git a/internal/api/grpc/user/v2/otp_integration_test.go b/internal/api/grpc/user/v2/totp_integration_test.go similarity index 75% rename from internal/api/grpc/user/v2/otp_integration_test.go rename to internal/api/grpc/user/v2/totp_integration_test.go index 9bc788e066..66d286cff4 100644 --- a/internal/api/grpc/user/v2/otp_integration_test.go +++ b/internal/api/grpc/user/v2/totp_integration_test.go @@ -13,24 +13,24 @@ import ( user "github.com/zitadel/zitadel/pkg/grpc/user/v2alpha" ) -func TestServer_RegisterOTP(t *testing.T) { +func TestServer_RegisterTOTP(t *testing.T) { // userID := Tester.CreateHumanUser(CTX).GetUserId() type args struct { ctx context.Context - req *user.RegisterOTPRequest + req *user.RegisterTOTPRequest } tests := []struct { name string args args - want *user.RegisterOTPResponse + want *user.RegisterTOTPResponse wantErr bool }{ { name: "missing user id", args: args{ ctx: CTX, - req: &user.RegisterOTPRequest{}, + req: &user.RegisterTOTPRequest{}, }, wantErr: true, }, @@ -38,7 +38,7 @@ func TestServer_RegisterOTP(t *testing.T) { name: "user mismatch", args: args{ ctx: CTX, - req: &user.RegisterOTPRequest{ + req: &user.RegisterTOTPRequest{ UserId: "wrong", }, }, @@ -50,11 +50,11 @@ func TestServer_RegisterOTP(t *testing.T) { name: "human user", args: args{ ctx: CTX, - req: &user.RegisterOTPRequest{ + req: &user.RegisterTOTPRequest{ UserId: userID, }, }, - want: &user.RegisterOTPResponse{ + want: &user.RegisterTOTPResponse{ Details: &object.Details{ ResourceOwner: Tester.Organisation.ID, }, @@ -64,7 +64,7 @@ func TestServer_RegisterOTP(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := Client.RegisterOTP(tt.args.ctx, tt.args.req) + got, err := Client.RegisterTOTP(tt.args.ctx, tt.args.req) if tt.wantErr { require.Error(t, err) return @@ -78,11 +78,11 @@ func TestServer_RegisterOTP(t *testing.T) { } } -func TestServer_VerifyOTPRegistration(t *testing.T) { +func TestServer_VerifyTOTPRegistration(t *testing.T) { userID := Tester.CreateHumanUser(CTX).GetUserId() /* TODO: after we are able to obtain a Bearer token for a human user - reg, err := Client.RegisterOTP(CTX, &user.RegisterOTPRequest{ + reg, err := Client.RegisterTOTP(CTX, &user.RegisterTOTPRequest{ UserId: userID, }) require.NoError(t, err) @@ -92,19 +92,19 @@ func TestServer_VerifyOTPRegistration(t *testing.T) { type args struct { ctx context.Context - req *user.VerifyOTPRegistrationRequest + req *user.VerifyTOTPRegistrationRequest } tests := []struct { name string args args - want *user.VerifyOTPRegistrationResponse + want *user.VerifyTOTPRegistrationResponse wantErr bool }{ { name: "user mismatch", args: args{ ctx: CTX, - req: &user.VerifyOTPRegistrationRequest{ + req: &user.VerifyTOTPRegistrationRequest{ UserId: "wrong", }, }, @@ -114,7 +114,7 @@ func TestServer_VerifyOTPRegistration(t *testing.T) { name: "wrong code", args: args{ ctx: CTX, - req: &user.VerifyOTPRegistrationRequest{ + req: &user.VerifyTOTPRegistrationRequest{ UserId: userID, Code: "123", }, @@ -127,12 +127,12 @@ func TestServer_VerifyOTPRegistration(t *testing.T) { name: "success", args: args{ ctx: CTX, - req: &user.VerifyOTPRegistrationRequest{ + req: &user.VerifyTOTPRegistrationRequest{ UserId: userID, Code: code, }, }, - want: &user.VerifyOTPRegistrationResponse{ + want: &user.VerifyTOTPRegistrationResponse{ Details: &object.Details{ ResourceOwner: Tester.Organisation.ResourceOwner, }, @@ -142,7 +142,7 @@ func TestServer_VerifyOTPRegistration(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := Client.VerifyOTPRegistration(tt.args.ctx, tt.args.req) + got, err := Client.VerifyTOTPRegistration(tt.args.ctx, tt.args.req) if tt.wantErr { require.Error(t, err) return diff --git a/internal/api/grpc/user/v2/otp_test.go b/internal/api/grpc/user/v2/totp_test.go similarity index 80% rename from internal/api/grpc/user/v2/otp_test.go rename to internal/api/grpc/user/v2/totp_test.go index 29dcb59114..021717283b 100644 --- a/internal/api/grpc/user/v2/otp_test.go +++ b/internal/api/grpc/user/v2/totp_test.go @@ -14,15 +14,15 @@ import ( user "github.com/zitadel/zitadel/pkg/grpc/user/v2alpha" ) -func Test_otpDetailsToPb(t *testing.T) { +func Test_totpDetailsToPb(t *testing.T) { type args struct { - otp *domain.OTPv2 + otp *domain.TOTP err error } tests := []struct { name string args args - want *user.RegisterOTPResponse + want *user.RegisterTOTPResponse wantErr error }{ { @@ -35,7 +35,7 @@ func Test_otpDetailsToPb(t *testing.T) { { name: "success", args: args{ - otp: &domain.OTPv2{ + otp: &domain.TOTP{ ObjectDetails: &domain.ObjectDetails{ Sequence: 123, EventDate: time.Unix(456, 789), @@ -45,7 +45,7 @@ func Test_otpDetailsToPb(t *testing.T) { URI: "URI", }, }, - want: &user.RegisterOTPResponse{ + want: &user.RegisterTOTPResponse{ Details: &object.Details{ Sequence: 123, ChangeDate: ×tamppb.Timestamp{ @@ -61,10 +61,10 @@ func Test_otpDetailsToPb(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := otpDetailsToPb(tt.args.otp, tt.args.err) + got, err := totpDetailsToPb(tt.args.otp, tt.args.err) require.ErrorIs(t, err, tt.wantErr) if !proto.Equal(tt.want, got) { - t.Errorf("RegisterOTPResponse =\n%v\nwant\n%v", got, tt.want) + t.Errorf("RegisterTOTPResponse =\n%v\nwant\n%v", got, tt.want) } }) } diff --git a/internal/command/user_human_otp.go b/internal/command/user_human_otp.go index 7b779e37a9..8bf6f04685 100644 --- a/internal/command/user_human_otp.go +++ b/internal/command/user_human_otp.go @@ -45,7 +45,7 @@ func (c *Commands) AddHumanOTP(ctx context.Context, userID, resourceowner string if userID == "" { return nil, caos_errs.ThrowInvalidArgument(nil, "COMMAND-5M0sd", "Errors.User.UserIDMissing") } - prep, err := c.createHumanOTP(ctx, userID, resourceowner) + prep, err := c.createHumanTOTP(ctx, userID, resourceowner) if err != nil { return nil, err } @@ -62,14 +62,14 @@ func (c *Commands) AddHumanOTP(ctx context.Context, userID, resourceowner string }, nil } -type preparedOTP struct { +type preparedTOTP struct { wm *HumanOTPWriteModel userAgg *eventstore.Aggregate key *otp.Key cmds []eventstore.Command } -func (c *Commands) createHumanOTP(ctx context.Context, userID, resourceOwner string) (*preparedOTP, error) { +func (c *Commands) createHumanTOTP(ctx context.Context, userID, resourceOwner string) (*preparedTOTP, error) { human, err := c.getHuman(ctx, userID, resourceOwner) if err != nil { logging.Log("COMMAND-DAqe1").WithError(err).WithField("traceID", tracing.TraceIDFromCtx(ctx)).Debug("unable to get human for loginname") @@ -107,7 +107,7 @@ func (c *Commands) createHumanOTP(ctx context.Context, userID, resourceOwner str if err != nil { return nil, err } - return &preparedOTP{ + return &preparedTOTP{ wm: otpWriteModel, userAgg: userAgg, key: key, diff --git a/internal/command/user_human_otp_test.go b/internal/command/user_human_otp_test.go index b04038bb32..e11f771e6f 100644 --- a/internal/command/user_human_otp_test.go +++ b/internal/command/user_human_otp_test.go @@ -512,7 +512,7 @@ func TestCommands_createHumanOTP(t *testing.T) { }, }, } - got, err := c.createHumanOTP(tt.args.ctx, tt.args.userID, tt.args.resourceOwner) + got, err := c.createHumanTOTP(tt.args.ctx, tt.args.userID, tt.args.resourceOwner) require.ErrorIs(t, err, tt.wantErr) if tt.want { require.NotNil(t, got) diff --git a/internal/command/user_v2_otp.go b/internal/command/user_v2_totp.go similarity index 67% rename from internal/command/user_v2_otp.go rename to internal/command/user_v2_totp.go index 4a317c5e1a..76840bac13 100644 --- a/internal/command/user_v2_otp.go +++ b/internal/command/user_v2_totp.go @@ -7,25 +7,25 @@ import ( "github.com/zitadel/zitadel/internal/domain" ) -func (c *Commands) AddUserOTP(ctx context.Context, userID, resourceowner string) (*domain.OTPv2, error) { +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.createHumanOTP(ctx, userID, resourceowner) + prep, err := c.createHumanTOTP(ctx, userID, resourceowner) if err != nil { return nil, err } if err = c.pushAppendAndReduce(ctx, prep.wm, prep.cmds...); err != nil { return nil, err } - return &domain.OTPv2{ + return &domain.TOTP{ ObjectDetails: writeModelToObjectDetails(&prep.wm.WriteModel), Secret: prep.key.Secret(), URI: prep.key.URL(), }, nil } -func (c *Commands) CheckUserOTP(ctx context.Context, userID, code, resourceOwner string) (*domain.ObjectDetails, error) { +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 } diff --git a/internal/command/user_v2_otp_test.go b/internal/command/user_v2_totp_test.go similarity index 95% rename from internal/command/user_v2_otp_test.go rename to internal/command/user_v2_totp_test.go index 43cf6813f8..d7b9f82463 100644 --- a/internal/command/user_v2_otp_test.go +++ b/internal/command/user_v2_totp_test.go @@ -21,7 +21,7 @@ import ( "github.com/zitadel/zitadel/internal/repository/user" ) -func TestCommands_AddUserOTP(t *testing.T) { +func TestCommands_AddUserTOTP(t *testing.T) { ctx := authz.NewMockContext("inst1", "org1", "user1") userAgg := &user.NewAggregate("user1", "org1").Aggregate @@ -175,7 +175,7 @@ func TestCommands_AddUserOTP(t *testing.T) { }, }, } - got, err := c.AddUserOTP(ctx, tt.args.userID, tt.args.resourceowner) + got, err := c.AddUserTOTP(ctx, tt.args.userID, tt.args.resourceowner) require.ErrorIs(t, err, tt.wantErr) if tt.want { require.NotNil(t, got) @@ -187,7 +187,7 @@ func TestCommands_AddUserOTP(t *testing.T) { } } -func TestCommands_CheckUserOTP(t *testing.T) { +func TestCommands_CheckUserTOTP(t *testing.T) { ctx := authz.NewMockContext("inst1", "org1", "user1") cryptoAlg := crypto.CreateMockEncryptionAlg(gomock.NewController(t)) @@ -252,7 +252,7 @@ func TestCommands_CheckUserOTP(t *testing.T) { }, }, } - got, err := c.CheckUserOTP(ctx, tt.args.userID, tt.args.code, tt.args.resourceOwner) + got, err := c.CheckUserTOTP(ctx, tt.args.userID, tt.args.code, tt.args.resourceOwner) require.ErrorIs(t, err, tt.wantErr) if tt.want { require.NotNil(t, got) diff --git a/internal/domain/human_otp.go b/internal/domain/human_otp.go index b35d0f4d34..479814340b 100644 --- a/internal/domain/human_otp.go +++ b/internal/domain/human_otp.go @@ -17,7 +17,7 @@ type OTP struct { State MFAState } -type OTPv2 struct { +type TOTP struct { *ObjectDetails Secret string diff --git a/proto/zitadel/user/v2alpha/user_service.proto b/proto/zitadel/user/v2alpha/user_service.proto index ad30412d00..5c548f9178 100644 --- a/proto/zitadel/user/v2alpha/user_service.proto +++ b/proto/zitadel/user/v2alpha/user_service.proto @@ -271,9 +271,9 @@ service UserService { }; } - rpc RegisterOTP (RegisterOTPRequest) returns (RegisterOTPResponse) { + rpc RegisterTOTP (RegisterTOTPRequest) returns (RegisterTOTPResponse) { option (google.api.http) = { - post: "/v2alpha/users/{user_id}/otp" + post: "/v2alpha/users/{user_id}/totp" body: "*" }; @@ -283,8 +283,8 @@ service UserService { } }; option (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_operation) = { - summary: "Start the registration of an OTP generator for a user"; - description: "Start the registration of a OTP generator for a user, as a response a secret returned, which is used to initialize a TOTP app or device." + summary: "Start the registration of a TOTP generator for a user"; + description: "Start the registration of a TOTP generator for a user, as a response a secret returned, which is used to initialize a TOTP app or device." responses: { key: "200" value: { @@ -294,9 +294,9 @@ service UserService { }; } - rpc VerifyOTPRegistration (VerifyOTPRegistrationRequest) returns (VerifyOTPRegistrationResponse) { + rpc VerifyTOTPRegistration (VerifyTOTPRegistrationRequest) returns (VerifyTOTPRegistrationResponse) { option (google.api.http) = { - post: "/v2alpha/users/{user_id}/otp/_verify" // Why underscore here?? + post: "/v2alpha/users/{user_id}/totp/_verify" body: "*" }; @@ -306,8 +306,8 @@ service UserService { } }; option (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_operation) = { - summary: "Verify a u2f token for a user"; - description: "Verify the OTP registration with a generated code." + summary: "Verify a TOTP generator for a user"; + description: "Verify the TOTP registration with a generated code." responses: { key: "200" value: { @@ -719,7 +719,7 @@ message VerifyU2FRegistrationResponse{ zitadel.object.v2alpha.Details details = 1; } -message RegisterOTPRequest { +message RegisterTOTPRequest { string user_id = 1 [ (validate.rules).string = {min_len: 1, max_len: 200}, (google.api.field_behavior) = REQUIRED, @@ -731,7 +731,7 @@ message RegisterOTPRequest { ]; } -message RegisterOTPResponse { +message RegisterTOTPResponse { zitadel.object.v2alpha.Details details = 1; string uri = 2 [ (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = { @@ -745,7 +745,7 @@ message RegisterOTPResponse { ]; } -message VerifyOTPRegistrationRequest { +message VerifyTOTPRegistrationRequest { string user_id = 1 [ (validate.rules).string = {min_len: 1, max_len: 200}, (google.api.field_behavior) = REQUIRED, @@ -759,13 +759,13 @@ message VerifyOTPRegistrationRequest { (validate.rules).string = {min_len: 1, max_len: 200}, (google.api.field_behavior) = REQUIRED, (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = { - description: "Code generated by OTP app or device" + description: "Code generated by TOTP app or device" example: "\"123456\""; } ]; } -message VerifyOTPRegistrationResponse { +message VerifyTOTPRegistrationResponse { zitadel.object.v2alpha.Details details = 1; }