From 09aafb35eb7a9625e884d4c46b14413b0305cc1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20M=C3=B6hlmann?= Date: Tue, 20 Jun 2023 12:36:21 +0200 Subject: [PATCH] feat(v2): implement user register OTP (#6030) * feat(v2): implement user register OTP * feat(v2): implement user verify OTP * session: retry on permission denied --- internal/api/grpc/user/v2/otp.go | 38 ++ .../api/grpc/user/v2/otp_integration_test.go | 155 ++++++ internal/api/grpc/user/v2/otp_test.go | 71 +++ internal/command/main_test.go | 12 + internal/command/user_human_otp.go | 46 +- internal/command/user_human_otp_test.go | 476 ++++++++++++++++++ internal/command/user_v2_otp.go | 33 ++ internal/command/user_v2_otp_test.go | 263 ++++++++++ internal/domain/human_otp.go | 9 +- .../repository/mock/repository.mock.impl.go | 22 + 10 files changed, 1113 insertions(+), 12 deletions(-) create mode 100644 internal/api/grpc/user/v2/otp.go create mode 100644 internal/api/grpc/user/v2/otp_integration_test.go create mode 100644 internal/api/grpc/user/v2/otp_test.go create mode 100644 internal/command/user_v2_otp.go create mode 100644 internal/command/user_v2_otp_test.go diff --git a/internal/api/grpc/user/v2/otp.go b/internal/api/grpc/user/v2/otp.go new file mode 100644 index 0000000000..80ea810076 --- /dev/null +++ b/internal/api/grpc/user/v2/otp.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) 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/otp_integration_test.go b/internal/api/grpc/user/v2/otp_integration_test.go new file mode 100644 index 0000000000..9bc788e066 --- /dev/null +++ b/internal/api/grpc/user/v2/otp_integration_test.go @@ -0,0 +1,155 @@ +//go:build integration + +package user_test + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/zitadel/zitadel/internal/integration" + user "github.com/zitadel/zitadel/pkg/grpc/user/v2alpha" +) + +func TestServer_RegisterOTP(t *testing.T) { + // userID := Tester.CreateHumanUser(CTX).GetUserId() + + type args struct { + ctx context.Context + req *user.RegisterOTPRequest + } + tests := []struct { + name string + args args + want *user.RegisterOTPResponse + wantErr bool + }{ + { + name: "missing user id", + args: args{ + ctx: CTX, + req: &user.RegisterOTPRequest{}, + }, + wantErr: true, + }, + { + name: "user mismatch", + args: args{ + ctx: CTX, + req: &user.RegisterOTPRequest{ + UserId: "wrong", + }, + }, + wantErr: true, + }, + /* TODO: after we are able to obtain a Bearer token for a human user + https://github.com/zitadel/zitadel/issues/6022 + { + name: "human user", + args: args{ + ctx: CTX, + req: &user.RegisterOTPRequest{ + UserId: userID, + }, + }, + want: &user.RegisterOTPResponse{ + Details: &object.Details{ + ResourceOwner: Tester.Organisation.ID, + }, + }, + }, + */ + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := Client.RegisterOTP(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) + assert.NotEmpty(t, got.GetUri()) + assert.NotEmpty(t, got.GetSecret()) + }) + } +} + +func TestServer_VerifyOTPRegistration(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{ + UserId: userID, + }) + require.NoError(t, err) + code, err := totp.GenerateCode(reg.Secret, time.Now()) + require.NoError(t, err) + */ + + type args struct { + ctx context.Context + req *user.VerifyOTPRegistrationRequest + } + tests := []struct { + name string + args args + want *user.VerifyOTPRegistrationResponse + wantErr bool + }{ + { + name: "user mismatch", + args: args{ + ctx: CTX, + req: &user.VerifyOTPRegistrationRequest{ + UserId: "wrong", + }, + }, + wantErr: true, + }, + { + name: "wrong code", + args: args{ + ctx: CTX, + req: &user.VerifyOTPRegistrationRequest{ + UserId: userID, + Code: "123", + }, + }, + wantErr: true, + }, + /* TODO: after we are able to obtain a Bearer token for a human user + https://github.com/zitadel/zitadel/issues/6022 + { + name: "success", + args: args{ + ctx: CTX, + req: &user.VerifyOTPRegistrationRequest{ + UserId: userID, + Code: code, + }, + }, + want: &user.VerifyOTPRegistrationResponse{ + Details: &object.Details{ + ResourceOwner: Tester.Organisation.ResourceOwner, + }, + }, + }, + */ + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := Client.VerifyOTPRegistration(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/api/grpc/user/v2/otp_test.go b/internal/api/grpc/user/v2/otp_test.go new file mode 100644 index 0000000000..29dcb59114 --- /dev/null +++ b/internal/api/grpc/user/v2/otp_test.go @@ -0,0 +1,71 @@ +package user + +import ( + "io" + "testing" + "time" + + "github.com/stretchr/testify/require" + "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/types/known/timestamppb" + + "github.com/zitadel/zitadel/internal/domain" + object "github.com/zitadel/zitadel/pkg/grpc/object/v2alpha" + user "github.com/zitadel/zitadel/pkg/grpc/user/v2alpha" +) + +func Test_otpDetailsToPb(t *testing.T) { + type args struct { + otp *domain.OTPv2 + err error + } + tests := []struct { + name string + args args + want *user.RegisterOTPResponse + wantErr error + }{ + { + name: "error", + args: args{ + err: io.ErrClosedPipe, + }, + wantErr: io.ErrClosedPipe, + }, + { + name: "success", + args: args{ + otp: &domain.OTPv2{ + ObjectDetails: &domain.ObjectDetails{ + Sequence: 123, + EventDate: time.Unix(456, 789), + ResourceOwner: "me", + }, + Secret: "secret", + URI: "URI", + }, + }, + want: &user.RegisterOTPResponse{ + Details: &object.Details{ + Sequence: 123, + ChangeDate: ×tamppb.Timestamp{ + Seconds: 456, + Nanos: 789, + }, + ResourceOwner: "me", + }, + Secret: "secret", + Uri: "URI", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := otpDetailsToPb(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) + } + }) + } +} diff --git a/internal/command/main_test.go b/internal/command/main_test.go index 5c7b42d3a7..7b37e0f8d5 100644 --- a/internal/command/main_test.go +++ b/internal/command/main_test.go @@ -125,6 +125,18 @@ func expectPushFailed(err error, events []*repository.Event, uniqueConstraints . } } +func expectRandomPush(events []*repository.Event, uniqueConstraints ...*repository.UniqueConstraint) expect { + return func(m *mock.MockRepository) { + m.ExpectRandomPush(events, uniqueConstraints...) + } +} + +func expectRandomPushFailed(err error, events []*repository.Event, uniqueConstraints ...*repository.UniqueConstraint) expect { + return func(m *mock.MockRepository) { + m.ExpectRandomPushFailed(err, events, uniqueConstraints...) + } +} + func expectFilter(events ...*repository.Event) expect { return func(m *mock.MockRepository) { m.ExpectFilterEvents(events...) diff --git a/internal/command/user_human_otp.go b/internal/command/user_human_otp.go index 96db142f75..7b779e37a9 100644 --- a/internal/command/user_human_otp.go +++ b/internal/command/user_human_otp.go @@ -3,12 +3,14 @@ package command import ( "context" + "github.com/pquerna/otp" "github.com/zitadel/logging" "github.com/zitadel/zitadel/internal/api/authz" "github.com/zitadel/zitadel/internal/crypto" "github.com/zitadel/zitadel/internal/domain" caos_errs "github.com/zitadel/zitadel/internal/errors" + "github.com/zitadel/zitadel/internal/eventstore" "github.com/zitadel/zitadel/internal/eventstore/v1/models" "github.com/zitadel/zitadel/internal/repository/user" "github.com/zitadel/zitadel/internal/telemetry/tracing" @@ -43,7 +45,32 @@ func (c *Commands) AddHumanOTP(ctx context.Context, userID, resourceowner string if userID == "" { return nil, caos_errs.ThrowInvalidArgument(nil, "COMMAND-5M0sd", "Errors.User.UserIDMissing") } - human, err := c.getHuman(ctx, userID, resourceowner) + prep, err := c.createHumanOTP(ctx, userID, resourceowner) + if err != nil { + return nil, err + } + _, err = c.eventstore.Push(ctx, prep.cmds...) + if err != nil { + return nil, err + } + return &domain.OTP{ + ObjectRoot: models.ObjectRoot{ + AggregateID: prep.userAgg.ID, + }, + SecretString: prep.key.Secret(), + Url: prep.key.URL(), + }, nil +} + +type preparedOTP struct { + wm *HumanOTPWriteModel + userAgg *eventstore.Aggregate + key *otp.Key + cmds []eventstore.Command +} + +func (c *Commands) createHumanOTP(ctx context.Context, userID, resourceOwner string) (*preparedOTP, 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") return nil, caos_errs.ThrowPreconditionFailed(err, "COMMAND-MM9fs", "Errors.User.NotFound") @@ -59,7 +86,7 @@ func (c *Commands) AddHumanOTP(ctx context.Context, userID, resourceowner string return nil, caos_errs.ThrowPreconditionFailed(err, "COMMAND-8ugTs", "Errors.Org.DomainPolicy.NotFound") } - otpWriteModel, err := c.otpWriteModelByID(ctx, userID, resourceowner) + otpWriteModel, err := c.otpWriteModelByID(ctx, userID, resourceOwner) if err != nil { return nil, err } @@ -80,16 +107,13 @@ func (c *Commands) AddHumanOTP(ctx context.Context, userID, resourceowner string if err != nil { return nil, err } - _, err = c.eventstore.Push(ctx, user.NewHumanOTPAddedEvent(ctx, userAgg, secret)) - if err != nil { - return nil, err - } - return &domain.OTP{ - ObjectRoot: models.ObjectRoot{ - AggregateID: human.AggregateID, + return &preparedOTP{ + wm: otpWriteModel, + userAgg: userAgg, + key: key, + cmds: []eventstore.Command{ + user.NewHumanOTPAddedEvent(ctx, userAgg, secret), }, - SecretString: key.Secret(), - Url: key.URL(), }, nil } diff --git a/internal/command/user_human_otp_test.go b/internal/command/user_human_otp_test.go index c894a63f6c..b04038bb32 100644 --- a/internal/command/user_human_otp_test.go +++ b/internal/command/user_human_otp_test.go @@ -2,11 +2,17 @@ package command import ( "context" + "io" "testing" + "time" + "github.com/golang/mock/gomock" + "github.com/pquerna/otp/totp" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "golang.org/x/text/language" + "github.com/zitadel/zitadel/internal/api/authz" "github.com/zitadel/zitadel/internal/crypto" "github.com/zitadel/zitadel/internal/domain" caos_errs "github.com/zitadel/zitadel/internal/errors" @@ -231,6 +237,476 @@ func TestCommandSide_AddHumanOTP(t *testing.T) { } } +func TestCommands_createHumanOTP(t *testing.T) { + type fields struct { + eventstore *eventstore.Eventstore + } + type args struct { + ctx context.Context + userID string + resourceOwner string + } + tests := []struct { + name string + fields fields + args args + want bool + wantErr error + }{ + { + name: "user not existing, not found error", + fields: fields{ + eventstore: eventstoreExpect( + t, + expectFilter(), + ), + }, + args: args{ + ctx: context.Background(), + resourceOwner: "org1", + userID: "user1", + }, + wantErr: caos_errs.ThrowPreconditionFailed(nil, "COMMAND-MM9fs", "Errors.User.NotFound"), + }, + { + name: "org not existing, not found error", + fields: fields{ + eventstore: eventstoreExpect( + t, + expectFilter( + eventFromEventPusher( + user.NewHumanAddedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "username", + "firstname", + "lastname", + "nickname", + "displayname", + language.German, + domain.GenderUnspecified, + "email@test.ch", + true, + ), + ), + ), + expectFilter(), + ), + }, + args: args{ + ctx: context.Background(), + resourceOwner: "org1", + userID: "user1", + }, + wantErr: caos_errs.ThrowPreconditionFailed(nil, "COMMAND-55M9f", "Errors.Org.NotFound"), + }, + { + name: "org iam policy not existing, not found error", + fields: fields{ + eventstore: eventstoreExpect( + t, + expectFilter( + eventFromEventPusher( + user.NewHumanAddedEvent(context.Background(), + &user.NewAggregate("user1", "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(), + expectFilter(), + ), + }, + args: args{ + ctx: context.Background(), + resourceOwner: "org1", + userID: "user1", + }, + wantErr: caos_errs.ThrowPreconditionFailed(nil, "COMMAND-8ugTs", "Errors.Org.DomainPolicy.NotFound"), + }, + { + name: "otp already exists, already exists error", + fields: fields{ + eventstore: eventstoreExpect( + t, + expectFilter( + eventFromEventPusher( + user.NewHumanAddedEvent(context.Background(), + &user.NewAggregate("user1", "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( + eventFromEventPusher( + user.NewHumanOTPAddedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + &crypto.CryptoValue{ + CryptoType: crypto.TypeEncryption, + Algorithm: "enc", + KeyID: "id", + Crypted: []byte("a"), + }), + ), + eventFromEventPusher( + user.NewHumanOTPVerifiedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "agent1")), + ), + ), + }, + args: args{ + ctx: context.Background(), + resourceOwner: "org1", + userID: "user1", + }, + wantErr: caos_errs.ThrowAlreadyExists(nil, "COMMAND-do9se", "Errors.User.MFA.OTP.AlreadyReady"), + }, + { + name: "issuer not in context", + fields: fields{ + eventstore: eventstoreExpect( + t, + expectFilter( + eventFromEventPusher( + user.NewHumanAddedEvent(context.Background(), + &user.NewAggregate("user1", "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(), + ), + }, + args: args{ + ctx: context.Background(), + resourceOwner: "org1", + userID: "user1", + }, + wantErr: caos_errs.ThrowInternal(nil, "TOTP-ieY3o", "Errors.Internal"), + }, + { + name: "success", + fields: fields{ + eventstore: eventstoreExpect( + t, + expectFilter( + eventFromEventPusher( + user.NewHumanAddedEvent(context.Background(), + &user.NewAggregate("user1", "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(), + ), + }, + args: args{ + ctx: authz.WithRequestedDomain(context.Background(), "zitadel.com"), + resourceOwner: "org1", + userID: "user1", + }, + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := &Commands{ + eventstore: tt.fields.eventstore, + multifactors: domain.MultifactorConfigs{ + OTP: domain.OTPConfig{ + CryptoMFA: crypto.CreateMockEncryptionAlg(gomock.NewController(t)), + }, + }, + } + got, err := c.createHumanOTP(tt.args.ctx, tt.args.userID, tt.args.resourceOwner) + require.ErrorIs(t, err, tt.wantErr) + if tt.want { + require.NotNil(t, got) + assert.NotNil(t, got.wm) + assert.NotNil(t, got.userAgg) + require.NotNil(t, got.key) + assert.NotEmpty(t, got.key.URL()) + assert.NotEmpty(t, got.key.Secret()) + assert.Len(t, got.cmds, 1) + } + }) + } +} + +func TestCommands_HumanCheckMFAOTPSetup(t *testing.T) { + ctx := authz.NewMockContext("inst1", "org1", "user1") + + cryptoAlg := crypto.CreateMockEncryptionAlg(gomock.NewController(t)) + key, secret, err := domain.NewOTPKey("example.com", "user1", cryptoAlg) + require.NoError(t, err) + userAgg := &user.NewAggregate("user1", "org1").Aggregate + + code, err := totp.GenerateCode(key.Secret(), time.Now()) + require.NoError(t, err) + + type fields struct { + eventstore *eventstore.Eventstore + } + type args struct { + userID string + code string + resourceOwner string + } + tests := []struct { + name string + fields fields + args args + want bool + wantErr error + }{ + { + name: "missing user id", + args: args{}, + wantErr: caos_errs.ThrowPreconditionFailed(nil, "COMMAND-8N9ds", "Errors.User.UserIDMissing"), + }, + { + name: "filter error", + fields: fields{ + eventstore: eventstoreExpect(t, + expectFilterError(io.ErrClosedPipe), + ), + }, + args: args{ + userID: "user1", + resourceOwner: "org1", + }, + wantErr: io.ErrClosedPipe, + }, + { + name: "otp not existing error", + fields: fields{ + eventstore: eventstoreExpect( + t, + expectFilter( + eventFromEventPusher( + user.NewHumanOTPAddedEvent(ctx, userAgg, secret), + ), + eventFromEventPusher( + user.NewHumanOTPRemovedEvent(ctx, userAgg), + ), + ), + ), + }, + args: args{ + resourceOwner: "org1", + userID: "user1", + }, + wantErr: caos_errs.ThrowNotFound(nil, "COMMAND-3Mif9s", "Errors.User.MFA.OTP.NotExisting"), + }, + { + name: "otp already ready error", + fields: fields{ + eventstore: eventstoreExpect( + t, + expectFilter( + eventFromEventPusher( + user.NewHumanOTPAddedEvent(ctx, userAgg, secret), + ), + eventFromEventPusher( + user.NewHumanOTPVerifiedEvent(context.Background(), + userAgg, + "agent1", + ), + ), + ), + ), + }, + args: args{ + resourceOwner: "org1", + userID: "user1", + }, + wantErr: caos_errs.ThrowPreconditionFailed(nil, "COMMAND-qx4ls", "Errors.Users.MFA.OTP.AlreadyReady"), + }, + { + name: "wrong code", + fields: fields{ + eventstore: eventstoreExpect( + t, + expectFilter( + eventFromEventPusher( + user.NewHumanOTPAddedEvent(ctx, userAgg, secret), + ), + ), + ), + }, + args: args{ + resourceOwner: "org1", + code: "wrong", + userID: "user1", + }, + wantErr: caos_errs.ThrowInvalidArgument(nil, "EVENT-8isk2", "Errors.User.MFA.OTP.InvalidCode"), + }, + { + name: "push error", + fields: fields{ + eventstore: eventstoreExpect( + t, + expectFilter( + eventFromEventPusher( + user.NewHumanOTPAddedEvent(ctx, userAgg, secret), + ), + ), + expectPushFailed(io.ErrClosedPipe, + []*repository.Event{eventFromEventPusher( + user.NewHumanOTPVerifiedEvent(ctx, + userAgg, + "agent1", + ), + )}, + ), + ), + }, + args: args{ + resourceOwner: "org1", + code: code, + userID: "user1", + }, + wantErr: io.ErrClosedPipe, + }, + { + name: "success", + fields: fields{ + eventstore: eventstoreExpect( + t, + expectFilter( + eventFromEventPusher( + user.NewHumanOTPAddedEvent(ctx, userAgg, secret), + ), + ), + expectPush([]*repository.Event{eventFromEventPusher( + user.NewHumanOTPVerifiedEvent(ctx, + userAgg, + "agent1", + ), + )}), + ), + }, + args: args{ + resourceOwner: "org1", + code: code, + userID: "user1", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := &Commands{ + eventstore: tt.fields.eventstore, + multifactors: domain.MultifactorConfigs{ + OTP: domain.OTPConfig{ + CryptoMFA: cryptoAlg, + }, + }, + } + got, err := c.HumanCheckMFAOTPSetup(ctx, tt.args.userID, tt.args.code, "agent1", tt.args.resourceOwner) + require.ErrorIs(t, err, tt.wantErr) + if tt.want { + require.NotNil(t, got) + assert.Equal(t, "org1", got.ResourceOwner) + } + }) + } +} + func TestCommandSide_RemoveHumanOTP(t *testing.T) { type fields struct { eventstore *eventstore.Eventstore diff --git a/internal/command/user_v2_otp.go b/internal/command/user_v2_otp.go new file mode 100644 index 0000000000..4a317c5e1a --- /dev/null +++ b/internal/command/user_v2_otp.go @@ -0,0 +1,33 @@ +package command + +import ( + "context" + + "github.com/zitadel/zitadel/internal/api/authz" + "github.com/zitadel/zitadel/internal/domain" +) + +func (c *Commands) AddUserOTP(ctx context.Context, userID, resourceowner string) (*domain.OTPv2, error) { + if err := authz.UserIDInCTX(ctx, userID); err != nil { + return nil, err + } + prep, err := c.createHumanOTP(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{ + 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) { + if err := authz.UserIDInCTX(ctx, userID); err != nil { + return nil, err + } + return c.HumanCheckMFAOTPSetup(ctx, userID, code, "", resourceOwner) +} diff --git a/internal/command/user_v2_otp_test.go b/internal/command/user_v2_otp_test.go new file mode 100644 index 0000000000..43cf6813f8 --- /dev/null +++ b/internal/command/user_v2_otp_test.go @@ -0,0 +1,263 @@ +package command + +import ( + "io" + "testing" + "time" + + "github.com/golang/mock/gomock" + "github.com/pquerna/otp/totp" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/text/language" + + "github.com/zitadel/zitadel/internal/api/authz" + "github.com/zitadel/zitadel/internal/crypto" + "github.com/zitadel/zitadel/internal/domain" + caos_errs "github.com/zitadel/zitadel/internal/errors" + "github.com/zitadel/zitadel/internal/eventstore" + "github.com/zitadel/zitadel/internal/eventstore/repository" + "github.com/zitadel/zitadel/internal/repository/org" + "github.com/zitadel/zitadel/internal/repository/user" +) + +func TestCommands_AddUserOTP(t *testing.T) { + ctx := authz.NewMockContext("inst1", "org1", "user1") + userAgg := &user.NewAggregate("user1", "org1").Aggregate + + type fields struct { + eventstore *eventstore.Eventstore + } + type args struct { + userID string + resourceowner string + } + tests := []struct { + name string + fields fields + args args + want bool + wantErr error + }{ + { + name: "wrong user", + args: args{ + userID: "foo", + resourceowner: "org1", + }, + wantErr: caos_errs.ThrowUnauthenticated(nil, "AUTH-Bohd2", "Errors.User.UserIDWrong"), + }, + { + name: "create otp error", + args: args{ + userID: "user1", + resourceowner: "org1", + }, + fields: fields{ + eventstore: eventstoreExpect( + t, + expectFilter(), + ), + }, + wantErr: caos_errs.ThrowPreconditionFailed(nil, "COMMAND-MM9fs", "Errors.User.NotFound"), + }, + { + name: "push error", + args: args{ + userID: "user1", + resourceowner: "org1", + }, + fields: fields{ + eventstore: eventstoreExpect( + t, + expectFilter( + eventFromEventPusher( + user.NewHumanAddedEvent(ctx, + userAgg, + "username", + "firstname", + "lastname", + "nickname", + "displayname", + language.German, + domain.GenderUnspecified, + "email@test.ch", + true, + ), + ), + ), + expectFilter( + eventFromEventPusher( + org.NewOrgAddedEvent(ctx, + userAgg, + "org", + ), + ), + ), + expectFilter( + eventFromEventPusher( + org.NewDomainPolicyAddedEvent(ctx, + userAgg, + true, + true, + true, + ), + ), + ), + expectFilter(), + expectRandomPushFailed(io.ErrClosedPipe, []*repository.Event{eventFromEventPusher( + user.NewHumanOTPAddedEvent(ctx, userAgg, nil), + )}), + ), + }, + wantErr: io.ErrClosedPipe, + }, + { + name: "success", + args: args{ + userID: "user1", + resourceowner: "org1", + }, + fields: fields{ + eventstore: eventstoreExpect( + t, + expectFilter( + eventFromEventPusher( + user.NewHumanAddedEvent(ctx, + userAgg, + "username", + "firstname", + "lastname", + "nickname", + "displayname", + language.German, + domain.GenderUnspecified, + "email@test.ch", + true, + ), + ), + ), + expectFilter( + eventFromEventPusher( + org.NewOrgAddedEvent(ctx, + userAgg, + "org", + ), + ), + ), + expectFilter( + eventFromEventPusher( + org.NewDomainPolicyAddedEvent(ctx, + userAgg, + true, + true, + true, + ), + ), + ), + expectFilter(), + expectRandomPush([]*repository.Event{eventFromEventPusher( + user.NewHumanOTPAddedEvent(ctx, userAgg, nil), + )}), + ), + }, + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := &Commands{ + eventstore: tt.fields.eventstore, + multifactors: domain.MultifactorConfigs{ + OTP: domain.OTPConfig{ + Issuer: "zitadel.com", + CryptoMFA: crypto.CreateMockEncryptionAlg(gomock.NewController(t)), + }, + }, + } + got, err := c.AddUserOTP(ctx, tt.args.userID, tt.args.resourceowner) + require.ErrorIs(t, err, tt.wantErr) + if tt.want { + require.NotNil(t, got) + assert.Equal(t, "org1", got.ResourceOwner) + assert.NotEmpty(t, got.Secret) + assert.NotEmpty(t, got.URI) + } + }) + } +} + +func TestCommands_CheckUserOTP(t *testing.T) { + ctx := authz.NewMockContext("inst1", "org1", "user1") + + cryptoAlg := crypto.CreateMockEncryptionAlg(gomock.NewController(t)) + key, secret, err := domain.NewOTPKey("example.com", "user1", cryptoAlg) + require.NoError(t, err) + userAgg := &user.NewAggregate("user1", "org1").Aggregate + + code, err := totp.GenerateCode(key.Secret(), time.Now()) + require.NoError(t, err) + + type fields struct { + eventstore *eventstore.Eventstore + } + type args struct { + userID string + code string + resourceOwner string + } + tests := []struct { + name string + fields fields + args args + want bool + wantErr error + }{ + { + name: "wrong user id", + args: args{ + userID: "foo", + }, + wantErr: caos_errs.ThrowUnauthenticated(nil, "AUTH-Bohd2", "Errors.User.UserIDWrong"), + }, + { + name: "success", + fields: fields{ + eventstore: eventstoreExpect( + t, + expectFilter( + eventFromEventPusher( + user.NewHumanOTPAddedEvent(ctx, userAgg, secret), + ), + ), + expectPush([]*repository.Event{eventFromEventPusher( + user.NewHumanOTPVerifiedEvent(ctx, userAgg, ""), + )}), + ), + }, + args: args{ + resourceOwner: "org1", + code: code, + userID: "user1", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := &Commands{ + eventstore: tt.fields.eventstore, + multifactors: domain.MultifactorConfigs{ + OTP: domain.OTPConfig{ + CryptoMFA: cryptoAlg, + }, + }, + } + got, err := c.CheckUserOTP(ctx, tt.args.userID, tt.args.code, tt.args.resourceOwner) + require.ErrorIs(t, err, tt.wantErr) + if tt.want { + require.NotNil(t, got) + assert.Equal(t, "org1", got.ResourceOwner) + } + }) + } +} diff --git a/internal/domain/human_otp.go b/internal/domain/human_otp.go index 354f9b1748..b35d0f4d34 100644 --- a/internal/domain/human_otp.go +++ b/internal/domain/human_otp.go @@ -17,10 +17,17 @@ type OTP struct { State MFAState } +type OTPv2 struct { + *ObjectDetails + + Secret string + URI string +} + func NewOTPKey(issuer, accountName string, cryptoAlg crypto.EncryptionAlgorithm) (*otp.Key, *crypto.CryptoValue, error) { key, err := totp.Generate(totp.GenerateOpts{Issuer: issuer, AccountName: accountName}) if err != nil { - return nil, nil, err + return nil, nil, caos_errs.ThrowInternal(err, "TOTP-ieY3o", "Errors.Internal") } encryptedSecret, err := crypto.Encrypt([]byte(key.Secret()), cryptoAlg) if err != nil { diff --git a/internal/eventstore/repository/mock/repository.mock.impl.go b/internal/eventstore/repository/mock/repository.mock.impl.go index 9121d1379f..e21f4fd7c3 100644 --- a/internal/eventstore/repository/mock/repository.mock.impl.go +++ b/internal/eventstore/repository/mock/repository.mock.impl.go @@ -70,3 +70,25 @@ func (m *MockRepository) ExpectPushFailed(err error, expectedEvents []*repositor ) return m } + +func (m *MockRepository) ExpectRandomPush(expectedEvents []*repository.Event, expectedUniqueConstraints ...*repository.UniqueConstraint) *MockRepository { + m.EXPECT().Push(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( + func(ctx context.Context, events []*repository.Event, uniqueConstraints ...*repository.UniqueConstraint) error { + assert.Len(m.ctrl.T, events, len(expectedEvents)) + assert.Len(m.ctrl.T, expectedUniqueConstraints, len(uniqueConstraints)) + return nil + }, + ) + return m +} + +func (m *MockRepository) ExpectRandomPushFailed(err error, expectedEvents []*repository.Event, expectedUniqueConstraints ...*repository.UniqueConstraint) *MockRepository { + m.EXPECT().Push(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( + func(ctx context.Context, events []*repository.Event, uniqueConstraints ...*repository.UniqueConstraint) error { + assert.Len(m.ctrl.T, events, len(expectedEvents)) + assert.Len(m.ctrl.T, expectedUniqueConstraints, len(uniqueConstraints)) + return err + }, + ) + return m +}