From 87aa97b9c70fad55202224f33405185b396e616f Mon Sep 17 00:00:00 2001 From: Fabi <38692350+fgerschwiler@users.noreply.github.com> Date: Wed, 26 Aug 2020 10:17:43 +0200 Subject: [PATCH] fix: reread user mfas, preferred loginname as otp account name (#636) * fix: reread user mfas * fix: use preferred login name as otp account name * fix: tests --- .../eventsourcing/eventstore/user.go | 27 ++++++++++++++++--- .../eventsourcing/eventstore/user.go | 9 ++++++- .../repository/eventsourcing/eventstore.go | 10 ++++--- .../eventsourcing/eventstore_test.go | 16 ++++++----- 4 files changed, 47 insertions(+), 15 deletions(-) diff --git a/internal/auth/repository/eventsourcing/eventstore/user.go b/internal/auth/repository/eventsourcing/eventstore/user.go index 998dba85c3..6ce456a9aa 100644 --- a/internal/auth/repository/eventsourcing/eventstore/user.go +++ b/internal/auth/repository/eventsourcing/eventstore/user.go @@ -176,15 +176,36 @@ func (repo *UserRepo) ChangePassword(ctx context.Context, userID, old, new strin } func (repo *UserRepo) MyUserMfas(ctx context.Context) ([]*model.MultiFactor, error) { - return repo.View.UserMfas(authz.GetCtxData(ctx).UserID) + user, err := repo.UserByID(ctx, authz.GetCtxData(ctx).UserID) + if err != nil { + return nil, err + } + if user.OTPState == model.MfaStateUnspecified { + return []*model.MultiFactor{}, nil + } + return []*model.MultiFactor{{Type: model.MfaTypeOTP, State: user.OTPState}}, nil } func (repo *UserRepo) AddMfaOTP(ctx context.Context, userID string) (*model.OTP, error) { - return repo.UserEvents.AddOTP(ctx, userID) + accountName := "" + user, err := repo.UserByID(ctx, userID) + if err != nil { + logging.Log("EVENT-Fk93s").OnError(err).Debug("unable to get user for loginname") + } else { + accountName = user.PreferredLoginName + } + return repo.UserEvents.AddOTP(ctx, userID, accountName) } func (repo *UserRepo) AddMyMfaOTP(ctx context.Context) (*model.OTP, error) { - return repo.UserEvents.AddOTP(ctx, authz.GetCtxData(ctx).UserID) + accountName := "" + user, err := repo.UserByID(ctx, authz.GetCtxData(ctx).UserID) + if err != nil { + logging.Log("EVENT-Ml0sd").OnError(err).Debug("unable to get user for loginname") + } else { + accountName = user.PreferredLoginName + } + return repo.UserEvents.AddOTP(ctx, authz.GetCtxData(ctx).UserID, accountName) } func (repo *UserRepo) VerifyMfaOTPSetup(ctx context.Context, userID, code string) error { diff --git a/internal/management/repository/eventsourcing/eventstore/user.go b/internal/management/repository/eventsourcing/eventstore/user.go index b774ba63f4..853a8bb6ac 100644 --- a/internal/management/repository/eventsourcing/eventstore/user.go +++ b/internal/management/repository/eventsourcing/eventstore/user.go @@ -145,7 +145,14 @@ func (repo *UserRepo) IsUserUnique(ctx context.Context, userName, email string) } func (repo *UserRepo) UserMfas(ctx context.Context, userID string) ([]*usr_model.MultiFactor, error) { - return repo.View.UserMfas(userID) + user, err := repo.UserByID(ctx, userID) + if err != nil { + return nil, err + } + if user.OTPState == usr_model.MfaStateUnspecified { + return []*usr_model.MultiFactor{}, nil + } + return []*usr_model.MultiFactor{{Type: usr_model.MfaTypeOTP, State: user.OTPState}}, nil } func (repo *UserRepo) SetOneTimePassword(ctx context.Context, password *usr_model.Password) (*usr_model.Password, error) { diff --git a/internal/user/repository/eventsourcing/eventstore.go b/internal/user/repository/eventsourcing/eventstore.go index 37c4603ee5..a790a97a17 100644 --- a/internal/user/repository/eventsourcing/eventstore.go +++ b/internal/user/repository/eventsourcing/eventstore.go @@ -946,7 +946,7 @@ func (es *UserEventstore) ChangeAddress(ctx context.Context, address *usr_model. return model.AddressToModel(repoExisting.Address), nil } -func (es *UserEventstore) AddOTP(ctx context.Context, userID string) (*usr_model.OTP, error) { +func (es *UserEventstore) AddOTP(ctx context.Context, userID, accountName string) (*usr_model.OTP, error) { existing, err := es.UserByID(ctx, userID) if err != nil { return nil, err @@ -954,9 +954,11 @@ func (es *UserEventstore) AddOTP(ctx context.Context, userID string) (*usr_model if existing.IsOTPReady() { return nil, caos_errs.ThrowAlreadyExists(nil, "EVENT-do9se", "Errors.User.Mfa.Otp.AlreadyReady") } - accountName := existing.UserName - if existing.Email != nil { - accountName = existing.EmailAddress + if accountName == "" { + accountName = existing.UserName + if existing.Email != nil { + accountName = existing.EmailAddress + } } key, err := totp.Generate(totp.GenerateOpts{Issuer: es.Multifactors.OTP.Issuer, AccountName: accountName}) if err != nil { diff --git a/internal/user/repository/eventsourcing/eventstore_test.go b/internal/user/repository/eventsourcing/eventstore_test.go index 260b558042..12f4c0388c 100644 --- a/internal/user/repository/eventsourcing/eventstore_test.go +++ b/internal/user/repository/eventsourcing/eventstore_test.go @@ -2947,9 +2947,10 @@ func TestChangeAddress(t *testing.T) { func TestAddOTP(t *testing.T) { ctrl := gomock.NewController(t) type args struct { - es *UserEventstore - ctx context.Context - userID string + es *UserEventstore + ctx context.Context + userID string + accountName string } type res struct { errFunc func(err error) bool @@ -2962,9 +2963,10 @@ func TestAddOTP(t *testing.T) { { name: "add ok", args: args{ - es: GetMockManipulateUserWithOTPGen(ctrl), - ctx: authz.NewMockContext("orgID", "userID"), - userID: "AggregateID", + es: GetMockManipulateUserWithOTPGen(ctrl), + ctx: authz.NewMockContext("orgID", "userID"), + userID: "AggregateID", + accountName: "AccountName", }, }, { @@ -2992,7 +2994,7 @@ func TestAddOTP(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result, err := tt.args.es.AddOTP(tt.args.ctx, tt.args.userID) + result, err := tt.args.es.AddOTP(tt.args.ctx, tt.args.userID, tt.args.accountName) if tt.res.errFunc == nil && result.AggregateID == "" { t.Errorf("result has no id")