From 07b2bac463ad9f96afde5b0cc6c9d2f9f8bec9dd Mon Sep 17 00:00:00 2001 From: Livio Spring Date: Wed, 17 Jul 2024 06:43:07 +0200 Subject: [PATCH] fix: allow login with user created through v2 api without password (#8291) # Which Problems Are Solved User created through the User V2 API without any authentication method and possibly unverified email address was not able to login through the current hosted login UI. An unverified email address would result in a mail verification and not an initialization mail like it would with the management API. Also the login UI would then require the user to enter the init code, which the user never received. # How the Problems Are Solved - When verifying the email through the login UI, it will check for existing auth methods (password, IdP, passkeys). In case there are none, the user will be prompted to set a password. - When a user was created through the V2 API with a verified email and no auth method, the user will be prompted to set a password in the login UI. - Since setting a password requires a corresponding code, the code will be generated and sent when login in. # Additional Changes - Changed `RequestSetPassword` to get the codeGenerator from the eventstore instead of getting it from query. # Additional Context - closes https://github.com/zitadel/zitadel/issues/6600 - closes https://github.com/zitadel/zitadel/issues/8235 --------- Co-authored-by: Stefan Benz <46600784+stebenz@users.noreply.github.com> --- internal/api/grpc/auth/email.go | 2 +- internal/api/grpc/management/user.go | 6 +- .../api/ui/login/init_password_handler.go | 7 +- internal/api/ui/login/mail_verify_handler.go | 101 +++++++++++++---- .../api/ui/login/password_reset_handler.go | 10 +- internal/api/ui/login/renderer.go | 2 +- .../static/templates/mail_verification.html | 23 +++- .../eventsourcing/eventstore/auth_request.go | 23 +++- .../eventstore/auth_request_test.go | 101 +++++++++++++++-- .../eventsourcing/eventstore/user.go | 39 +++++++ .../repository/eventsourcing/repository.go | 1 + internal/command/user_human_email.go | 39 ++++--- internal/command/user_human_email_test.go | 103 ++++++++++++++++-- internal/command/user_human_password.go | 6 +- internal/command/user_human_password_test.go | 35 +++--- internal/command/user_v2_human_test.go | 70 +++++++++++- internal/domain/next_step.go | 4 +- 17 files changed, 465 insertions(+), 107 deletions(-) diff --git a/internal/api/grpc/auth/email.go b/internal/api/grpc/auth/email.go index c6153546e7..ced1ca42da 100644 --- a/internal/api/grpc/auth/email.go +++ b/internal/api/grpc/auth/email.go @@ -50,7 +50,7 @@ func (s *Server) VerifyMyEmail(ctx context.Context, req *auth_pb.VerifyMyEmailRe return nil, err } ctxData := authz.GetCtxData(ctx) - objectDetails, err := s.command.VerifyHumanEmail(ctx, ctxData.UserID, req.Code, ctxData.ResourceOwner, emailCodeGenerator) + objectDetails, err := s.command.VerifyHumanEmail(ctx, ctxData.UserID, req.Code, ctxData.ResourceOwner, "", "", emailCodeGenerator) if err != nil { return nil, err } diff --git a/internal/api/grpc/management/user.go b/internal/api/grpc/management/user.go index f662b8c592..d3cab62961 100644 --- a/internal/api/grpc/management/user.go +++ b/internal/api/grpc/management/user.go @@ -586,11 +586,7 @@ func (s *Server) SetHumanPassword(ctx context.Context, req *mgmt_pb.SetHumanPass } func (s *Server) SendHumanResetPasswordNotification(ctx context.Context, req *mgmt_pb.SendHumanResetPasswordNotificationRequest) (*mgmt_pb.SendHumanResetPasswordNotificationResponse, error) { - passwordCodeGenerator, err := s.query.InitEncryptionGenerator(ctx, domain.SecretGeneratorTypePasswordResetCode, s.userCodeAlg) - if err != nil { - return nil, err - } - objectDetails, err := s.command.RequestSetPassword(ctx, req.UserId, authz.GetCtxData(ctx).OrgID, notifyTypeToDomain(req.Type), passwordCodeGenerator, "") + objectDetails, err := s.command.RequestSetPassword(ctx, req.UserId, authz.GetCtxData(ctx).OrgID, notifyTypeToDomain(req.Type), "") if err != nil { return nil, err } diff --git a/internal/api/ui/login/init_password_handler.go b/internal/api/ui/login/init_password_handler.go index 91a197ef64..4b9c173a2f 100644 --- a/internal/api/ui/login/init_password_handler.go +++ b/internal/api/ui/login/init_password_handler.go @@ -97,12 +97,7 @@ func (l *Login) resendPasswordSet(w http.ResponseWriter, r *http.Request, authRe userID = authReq.UserID authReqID = authReq.ID } - passwordCodeGenerator, err := l.query.InitEncryptionGenerator(r.Context(), domain.SecretGeneratorTypePasswordResetCode, l.userCodeAlg) - if err != nil { - l.renderInitPassword(w, r, authReq, userID, "", err) - return - } - _, err = l.command.RequestSetPassword(setContext(r.Context(), userOrg), userID, userOrg, domain.NotificationTypeEmail, passwordCodeGenerator, authReqID) + _, err := l.command.RequestSetPassword(setContext(r.Context(), userOrg), userID, userOrg, domain.NotificationTypeEmail, authReqID) l.renderInitPassword(w, r, authReq, userID, "", err) } diff --git a/internal/api/ui/login/mail_verify_handler.go b/internal/api/ui/login/mail_verify_handler.go index aa14969008..85c6114570 100644 --- a/internal/api/ui/login/mail_verify_handler.go +++ b/internal/api/ui/login/mail_verify_handler.go @@ -1,10 +1,16 @@ package login import ( + "context" "net/http" "net/url" + "github.com/zitadel/logging" + + http_mw "github.com/zitadel/zitadel/internal/api/http/middleware" "github.com/zitadel/zitadel/internal/domain" + "github.com/zitadel/zitadel/internal/query" + "github.com/zitadel/zitadel/internal/zerrors" ) const ( @@ -16,15 +22,25 @@ const ( ) type mailVerificationFormData struct { - Code string `schema:"code"` - UserID string `schema:"userID"` - Resend bool `schema:"resend"` + Code string `schema:"code"` + UserID string `schema:"userID"` + Resend bool `schema:"resend"` + PasswordInit bool `schema:"passwordInit"` + Password string `schema:"password"` + PasswordConfirm string `schema:"passwordconfirm"` } type mailVerificationData struct { baseData profileData - UserID string + UserID string + Code string + PasswordInit bool + MinLength uint64 + HasUppercase string + HasLowercase string + HasNumber string + HasSymbol string } func MailVerificationLink(origin, userID, code, orgID, authRequestID string) string { @@ -40,11 +56,32 @@ func (l *Login) handleMailVerification(w http.ResponseWriter, r *http.Request) { authReq := l.checkOptionalAuthRequestOfEmailLinks(r) userID := r.FormValue(queryUserID) code := r.FormValue(queryCode) - if code != "" { - l.checkMailCode(w, r, authReq, userID, code) + if userID == "" && authReq == nil { + l.renderError(w, r, authReq, nil) return } - l.renderMailVerification(w, r, authReq, userID, nil) + if userID == "" { + userID = authReq.UserID + } + passwordInit := l.checkUserNoFirstFactor(r.Context(), userID) + if code != "" && !passwordInit { + l.checkMailCode(w, r, authReq, userID, code, "") + return + } + l.renderMailVerification(w, r, authReq, userID, code, passwordInit, nil) +} + +func (l *Login) checkUserNoFirstFactor(ctx context.Context, userID string) bool { + userIDQuery, err := query.NewUserAuthMethodUserIDSearchQuery(userID) + logging.WithFields("userID", userID).OnError(err).Warn("error creating NewUserAuthMethodUserIDSearchQuery") + authMethodsQuery, err := query.NewUserAuthMethodTypesSearchQuery(domain.UserAuthMethodTypeIDP, domain.UserAuthMethodTypePassword, domain.UserAuthMethodTypePasswordless) + logging.WithFields("userID", userID).OnError(err).Warn("error creating NewUserAuthMethodTypesSearchQuery") + authMethods, err := l.query.SearchUserAuthMethods(ctx, &query.UserAuthMethodSearchQueries{Queries: []query.SearchQuery{userIDQuery, authMethodsQuery}}, false) + if err != nil { + logging.WithFields("userID", userID).OnError(err).Warn("unable to load user's auth methods for mail verification") + return false + } + return len(authMethods.AuthMethods) == 0 } func (l *Login) handleMailVerificationCheck(w http.ResponseWriter, r *http.Request) { @@ -55,7 +92,12 @@ func (l *Login) handleMailVerificationCheck(w http.ResponseWriter, r *http.Reque return } if !data.Resend { - l.checkMailCode(w, r, authReq, data.UserID, data.Code) + if data.PasswordInit && data.Password != data.PasswordConfirm { + err := zerrors.ThrowInvalidArgument(nil, "VIEW-fsdfd", "Errors.User.Password.ConfirmationWrong") + l.renderMailVerification(w, r, authReq, data.UserID, data.Code, data.PasswordInit, err) + return + } + l.checkMailCode(w, r, authReq, data.UserID, data.Code, data.Password) return } var userOrg, authReqID string @@ -65,14 +107,14 @@ func (l *Login) handleMailVerificationCheck(w http.ResponseWriter, r *http.Reque } emailCodeGenerator, err := l.query.InitEncryptionGenerator(r.Context(), domain.SecretGeneratorTypeVerifyEmailCode, l.userCodeAlg) if err != nil { - l.checkMailCode(w, r, authReq, data.UserID, data.Code) + l.renderMailVerification(w, r, authReq, data.UserID, "", data.PasswordInit, err) return } _, err = l.command.CreateHumanEmailVerificationCode(setContext(r.Context(), userOrg), data.UserID, userOrg, emailCodeGenerator, authReqID) - l.renderMailVerification(w, r, authReq, data.UserID, err) + l.renderMailVerification(w, r, authReq, data.UserID, "", data.PasswordInit, err) } -func (l *Login) checkMailCode(w http.ResponseWriter, r *http.Request, authReq *domain.AuthRequest, userID, code string) { +func (l *Login) checkMailCode(w http.ResponseWriter, r *http.Request, authReq *domain.AuthRequest, userID, code, password string) { userOrg := "" if authReq != nil { userID = authReq.UserID @@ -80,31 +122,52 @@ func (l *Login) checkMailCode(w http.ResponseWriter, r *http.Request, authReq *d } emailCodeGenerator, err := l.query.InitEncryptionGenerator(r.Context(), domain.SecretGeneratorTypeVerifyEmailCode, l.userCodeAlg) if err != nil { - l.renderMailVerification(w, r, authReq, userID, err) + l.renderMailVerification(w, r, authReq, userID, "", password != "", err) return } - _, err = l.command.VerifyHumanEmail(setContext(r.Context(), userOrg), userID, code, userOrg, emailCodeGenerator) + userAgentID, _ := http_mw.UserAgentIDFromCtx(r.Context()) + _, err = l.command.VerifyHumanEmail(setContext(r.Context(), userOrg), userID, code, userOrg, password, userAgentID, emailCodeGenerator) if err != nil { - l.renderMailVerification(w, r, authReq, userID, err) + l.renderMailVerification(w, r, authReq, userID, "", password != "", err) return } l.renderMailVerified(w, r, authReq, userOrg) } -func (l *Login) renderMailVerification(w http.ResponseWriter, r *http.Request, authReq *domain.AuthRequest, userID string, err error) { +func (l *Login) renderMailVerification(w http.ResponseWriter, r *http.Request, authReq *domain.AuthRequest, userID, code string, passwordInit bool, err error) { var errID, errMessage string if err != nil { errID, errMessage = l.getErrorMessage(r, err) } - if userID == "" { + if userID == "" && authReq != nil { userID = authReq.UserID } translator := l.getTranslator(r.Context(), authReq) data := mailVerificationData{ - baseData: l.getBaseData(r, authReq, translator, "EmailVerification.Title", "EmailVerification.Description", errID, errMessage), - UserID: userID, - profileData: l.getProfileData(authReq), + baseData: l.getBaseData(r, authReq, translator, "EmailVerification.Title", "EmailVerification.Description", errID, errMessage), + UserID: userID, + profileData: l.getProfileData(authReq), + Code: code, + PasswordInit: passwordInit, + } + if passwordInit { + policy := l.getPasswordComplexityPolicyByUserID(r, userID) + if policy != nil { + data.MinLength = policy.MinLength + if policy.HasUppercase { + data.HasUppercase = UpperCaseRegex + } + if policy.HasLowercase { + data.HasLowercase = LowerCaseRegex + } + if policy.HasSymbol { + data.HasSymbol = SymbolRegex + } + if policy.HasNumber { + data.HasNumber = NumberRegex + } + } } if authReq == nil { user, err := l.query.GetUserByID(r.Context(), false, userID) diff --git a/internal/api/ui/login/password_reset_handler.go b/internal/api/ui/login/password_reset_handler.go index bc9a5069d1..f4f98806c7 100644 --- a/internal/api/ui/login/password_reset_handler.go +++ b/internal/api/ui/login/password_reset_handler.go @@ -25,15 +25,7 @@ func (l *Login) handlePasswordReset(w http.ResponseWriter, r *http.Request) { l.renderPasswordResetDone(w, r, authReq, err) return } - passwordCodeGenerator, err := l.query.InitEncryptionGenerator(r.Context(), domain.SecretGeneratorTypePasswordResetCode, l.userCodeAlg) - if err != nil { - if authReq.LoginPolicy.IgnoreUnknownUsernames && zerrors.IsNotFound(err) { - err = nil - } - l.renderPasswordResetDone(w, r, authReq, err) - return - } - _, err = l.command.RequestSetPassword(setContext(r.Context(), authReq.UserOrgID), user.ID, authReq.UserOrgID, domain.NotificationTypeEmail, passwordCodeGenerator, authReq.ID) + _, err = l.command.RequestSetPassword(setContext(r.Context(), authReq.UserOrgID), user.ID, authReq.UserOrgID, domain.NotificationTypeEmail, authReq.ID) l.renderPasswordResetDone(w, r, authReq, err) } diff --git a/internal/api/ui/login/renderer.go b/internal/api/ui/login/renderer.go index 40339954d0..462e3fed93 100644 --- a/internal/api/ui/login/renderer.go +++ b/internal/api/ui/login/renderer.go @@ -313,7 +313,7 @@ func (l *Login) chooseNextStep(w http.ResponseWriter, r *http.Request, authReq * case *domain.ChangePasswordStep: l.renderChangePassword(w, r, authReq, err) case *domain.VerifyEMailStep: - l.renderMailVerification(w, r, authReq, "", err) + l.renderMailVerification(w, r, authReq, authReq.UserID, "", step.InitPassword, err) case *domain.MFAPromptStep: l.renderMFAPrompt(w, r, authReq, step, err) case *domain.InitUserStep: diff --git a/internal/api/ui/login/static/templates/mail_verification.html b/internal/api/ui/login/static/templates/mail_verification.html index 8c7ddee8c4..011947c173 100644 --- a/internal/api/ui/login/static/templates/mail_verification.html +++ b/internal/api/ui/login/static/templates/mail_verification.html @@ -17,13 +17,29 @@
- +
+ {{ if .PasswordInit }} +
+ + +
+
+ + + {{ template "password-complexity-policy-description" . }} +
+ {{ end }} + {{ template "error-message" .}}
- @@ -40,6 +56,7 @@
- + + {{template "main-bottom" .}} diff --git a/internal/auth/repository/eventsourcing/eventstore/auth_request.go b/internal/auth/repository/eventsourcing/eventstore/auth_request.go index 79d986dc5e..ec8e85bb60 100644 --- a/internal/auth/repository/eventsourcing/eventstore/auth_request.go +++ b/internal/auth/repository/eventsourcing/eventstore/auth_request.go @@ -52,6 +52,7 @@ type AuthRequestRepo struct { ProjectProvider projectProvider ApplicationProvider applicationProvider CustomTextProvider customTextProvider + PasswordReset passwordReset IdGenerator id.Generator } @@ -96,6 +97,7 @@ type idpUserLinksProvider interface { type userEventProvider interface { UserEventsByID(ctx context.Context, id string, changeDate time.Time, eventTypes []eventstore.EventType) ([]eventstore.Event, error) + PasswordCodeExists(ctx context.Context, userID string) (exists bool, err error) } type userCommandProvider interface { @@ -125,6 +127,10 @@ type customTextProvider interface { CustomTextListByTemplate(ctx context.Context, aggregateID string, text string, withOwnerRemoved bool) (texts *query.CustomTexts, err error) } +type passwordReset interface { + RequestSetPassword(ctx context.Context, userID, resourceOwner string, notifyType domain.NotificationType, authRequestID string) (objectDetails *domain.ObjectDetails, err error) +} + func (repo *AuthRequestRepo) Health(ctx context.Context) error { return repo.AuthRequests.Health(ctx) } @@ -1046,7 +1052,7 @@ func (repo *AuthRequestRepo) nextSteps(ctx context.Context, request *domain.Auth } } if isInternalLogin || (!isInternalLogin && len(request.LinkingUsers) > 0) { - step := repo.firstFactorChecked(request, user, userSession) + step := repo.firstFactorChecked(ctx, request, user, userSession) if step != nil { return append(steps, step), nil } @@ -1065,7 +1071,9 @@ func (repo *AuthRequestRepo) nextSteps(ctx context.Context, request *domain.Auth steps = append(steps, &domain.ChangePasswordStep{Expired: expired}) } if !user.IsEmailVerified { - steps = append(steps, &domain.VerifyEMailStep{}) + steps = append(steps, &domain.VerifyEMailStep{ + InitPassword: !user.PasswordSet, + }) } if user.UsernameChangeRequired { steps = append(steps, &domain.ChangeUsernameStep{}) @@ -1204,7 +1212,7 @@ func (repo *AuthRequestRepo) usersForUserSelection(ctx context.Context, request return users, nil } -func (repo *AuthRequestRepo) firstFactorChecked(request *domain.AuthRequest, user *user_model.UserView, userSession *user_model.UserSessionView) domain.NextStep { +func (repo *AuthRequestRepo) firstFactorChecked(ctx context.Context, request *domain.AuthRequest, user *user_model.UserView, userSession *user_model.UserSessionView) domain.NextStep { if user.InitRequired { return &domain.InitUserStep{PasswordSet: user.PasswordSet} } @@ -1226,6 +1234,15 @@ func (repo *AuthRequestRepo) firstFactorChecked(request *domain.AuthRequest, use } if user.PasswordInitRequired { + if !user.IsEmailVerified { + return &domain.VerifyEMailStep{InitPassword: true} + } + exists, err := repo.UserEventProvider.PasswordCodeExists(ctx, user.ID) + logging.WithFields("userID", user.ID).OnError(err).Error("unable to check if password code exists") + if err == nil && !exists { + _, err = repo.PasswordReset.RequestSetPassword(ctx, user.ID, user.ResourceOwner, domain.NotificationTypeEmail, request.ID) + logging.WithFields("userID", user.ID).OnError(err).Error("unable to create password code") + } return &domain.InitPasswordStep{} } diff --git a/internal/auth/repository/eventsourcing/eventstore/auth_request_test.go b/internal/auth/repository/eventsourcing/eventstore/auth_request_test.go index e64c7fed91..4f05d75b45 100644 --- a/internal/auth/repository/eventsourcing/eventstore/auth_request_test.go +++ b/internal/auth/repository/eventsourcing/eventstore/auth_request_test.go @@ -108,7 +108,8 @@ func (m *mockViewNoUser) UserByID(string, string) (*user_view_model.UserView, er } type mockEventUser struct { - Event eventstore.Event + Event eventstore.Event + CodeExists bool } func (m *mockEventUser) UserEventsByID(ctx context.Context, id string, changeDate time.Time, types []eventstore.EventType) ([]eventstore.Event, error) { @@ -118,6 +119,10 @@ func (m *mockEventUser) UserEventsByID(ctx context.Context, id string, changeDat return nil, nil } +func (m *mockEventUser) PasswordCodeExists(ctx context.Context, userID string) (bool, error) { + return m.CodeExists, nil +} + func (m *mockEventUser) GetLatestUserSessionSequence(ctx context.Context, instanceID string) (*query.CurrentState, error) { return &query.CurrentState{State: query.State{Sequence: 0}}, nil } @@ -132,8 +137,8 @@ func (m *mockEventErrUser) UserEventsByID(ctx context.Context, id string, change return nil, zerrors.ThrowInternal(nil, "id", "internal error") } -func (m *mockEventErrUser) BulkAddExternalIDPs(ctx context.Context, userID string, externalIDPs []*user_model.ExternalIDP) error { - return zerrors.ThrowInternal(nil, "id", "internal error") +func (m *mockEventErrUser) PasswordCodeExists(ctx context.Context, userID string) (bool, error) { + return false, zerrors.ThrowInternal(nil, "id", "internal error") } type mockViewUser struct { @@ -298,6 +303,28 @@ func (m *mockIDPUserLinks) IDPUserLinks(ctx context.Context, queries *query.IDPU return &query.IDPUserLinks{Links: m.idps}, nil } +type mockPasswordReset struct { + t *testing.T + expectCall bool +} + +func newMockPasswordReset(expectCall bool) func(*testing.T) passwordReset { + return func(t *testing.T) passwordReset { + return &mockPasswordReset{ + t: t, + expectCall: expectCall, + } + } +} + +func (m *mockPasswordReset) RequestSetPassword(ctx context.Context, userID, resourceOwner string, notifyType domain.NotificationType, authRequestID string) (objectDetails *domain.ObjectDetails, err error) { + if !m.expectCall { + m.t.Error("unexpected call to RequestSetPassword") + return nil, nil + } + return nil, err +} + func TestAuthRequestRepo_nextSteps(t *testing.T) { type fields struct { AuthRequests cache.AuthRequestCache @@ -316,6 +343,7 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) { labelPolicyProvider labelPolicyProvider passwordAgePolicyProvider passwordAgePolicyProvider customTextProvider customTextProvider + passwordReset func(t *testing.T) passwordReset } type args struct { request *domain.AuthRequest @@ -687,7 +715,7 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) { fields{ userViewProvider: &mockViewUser{}, userEventProvider: &mockEventUser{ - &es_models.Event{ + Event: &es_models.Event{ AggregateType: user_repo.AggregateType, Typ: user_repo.UserDeactivatedType, }, @@ -709,7 +737,7 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) { fields{ userViewProvider: &mockViewUser{}, userEventProvider: &mockEventUser{ - &es_models.Event{ + Event: &es_models.Event{ AggregateType: user_repo.AggregateType, Typ: user_repo.UserLockedType, }, @@ -929,7 +957,7 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) { nil, }, { - "password not set, init password step", + "password not set (email not verified), init password step", fields{ userSessionViewProvider: &mockViewUserSession{}, userViewProvider: &mockViewUser{ @@ -945,6 +973,54 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) { idpUserLinksProvider: &mockIDPUserLinks{}, }, args{&domain.AuthRequest{UserID: "UserID", LoginPolicy: &domain.LoginPolicy{}}, false}, + []domain.NextStep{&domain.VerifyEMailStep{InitPassword: true}}, + nil, + }, + { + "password not set (email verified), init password step", + fields{ + userSessionViewProvider: &mockViewUserSession{}, + userViewProvider: &mockViewUser{ + PasswordInitRequired: true, + IsEmailVerified: true, + }, + userEventProvider: &mockEventUser{ + CodeExists: true, + }, + lockoutPolicyProvider: &mockLockoutPolicy{ + policy: &query.LockoutPolicy{ + ShowFailures: true, + }, + }, + orgViewProvider: &mockViewOrg{State: domain.OrgStateActive}, + idpUserLinksProvider: &mockIDPUserLinks{}, + passwordReset: newMockPasswordReset(false), + }, + args{&domain.AuthRequest{UserID: "UserID", LoginPolicy: &domain.LoginPolicy{}}, false}, + []domain.NextStep{&domain.InitPasswordStep{}}, + nil, + }, + { + "password not set (email verified, password code not exists), create code, init password step", + fields{ + userSessionViewProvider: &mockViewUserSession{}, + userViewProvider: &mockViewUser{ + PasswordInitRequired: true, + IsEmailVerified: true, + }, + userEventProvider: &mockEventUser{ + CodeExists: false, + }, + lockoutPolicyProvider: &mockLockoutPolicy{ + policy: &query.LockoutPolicy{ + ShowFailures: true, + }, + }, + orgViewProvider: &mockViewOrg{State: domain.OrgStateActive}, + idpUserLinksProvider: &mockIDPUserLinks{}, + passwordReset: newMockPasswordReset(true), + }, + args{&domain.AuthRequest{UserID: "UserID", LoginPolicy: &domain.LoginPolicy{}}, false}, []domain.NextStep{&domain.InitPasswordStep{}}, nil, }, @@ -1720,6 +1796,9 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) { PasswordAgePolicyProvider: tt.fields.passwordAgePolicyProvider, CustomTextProvider: tt.fields.customTextProvider, } + if tt.fields.passwordReset != nil { + repo.PasswordReset = tt.fields.passwordReset(t) + } got, err := repo.nextSteps(context.Background(), tt.args.request, tt.args.checkLoggedIn) if (err != nil && tt.wantErr == nil) || (tt.wantErr != nil && !tt.wantErr(err)) { t.Errorf("nextSteps() wrong error = %v", err) @@ -2201,7 +2280,7 @@ func Test_userSessionByIDs(t *testing.T) { agentID: "agentID", user: &user_model.UserView{ID: "id", HumanView: &user_model.HumanView{FirstName: "FirstName"}}, eventProvider: &mockEventUser{ - &es_models.Event{ + Event: &es_models.Event{ AggregateType: user_repo.AggregateType, Typ: user_repo.UserV1MFAOTPCheckSucceededType, CreationDate: testNow, @@ -2224,7 +2303,7 @@ func Test_userSessionByIDs(t *testing.T) { agentID: "agentID", user: &user_model.UserView{ID: "id"}, eventProvider: &mockEventUser{ - &es_models.Event{ + Event: &es_models.Event{ AggregateType: user_repo.AggregateType, Typ: user_repo.UserV1MFAOTPCheckSucceededType, CreationDate: testNow, @@ -2251,7 +2330,7 @@ func Test_userSessionByIDs(t *testing.T) { agentID: "agentID", user: &user_model.UserView{ID: "id", HumanView: &user_model.HumanView{FirstName: "FirstName"}}, eventProvider: &mockEventUser{ - &es_models.Event{ + Event: &es_models.Event{ AggregateType: user_repo.AggregateType, Typ: user_repo.UserV1MFAOTPCheckSucceededType, CreationDate: testNow, @@ -2278,7 +2357,7 @@ func Test_userSessionByIDs(t *testing.T) { agentID: "agentID", user: &user_model.UserView{ID: "id"}, eventProvider: &mockEventUser{ - &es_models.Event{ + Event: &es_models.Event{ AggregateType: user_repo.AggregateType, Typ: user_repo.UserRemovedType, }, @@ -2367,7 +2446,7 @@ func Test_userByID(t *testing.T) { PasswordChangeRequired: true, }, eventProvider: &mockEventUser{ - &es_models.Event{ + Event: &es_models.Event{ AggregateType: user_repo.AggregateType, Typ: user_repo.UserV1PasswordChangedType, CreationDate: testNow, diff --git a/internal/auth/repository/eventsourcing/eventstore/user.go b/internal/auth/repository/eventsourcing/eventstore/user.go index cfa573e7e3..f35b2f0cf9 100644 --- a/internal/auth/repository/eventsourcing/eventstore/user.go +++ b/internal/auth/repository/eventsourcing/eventstore/user.go @@ -10,7 +10,9 @@ import ( "github.com/zitadel/zitadel/internal/domain" "github.com/zitadel/zitadel/internal/eventstore" "github.com/zitadel/zitadel/internal/query" + "github.com/zitadel/zitadel/internal/repository/user" usr_view "github.com/zitadel/zitadel/internal/user/repository/view" + "github.com/zitadel/zitadel/internal/zerrors" ) type UserRepo struct { @@ -46,3 +48,40 @@ func (repo *UserRepo) UserEventsByID(ctx context.Context, id string, changeDate } return repo.Eventstore.Filter(ctx, query) //nolint:staticcheck } + +type passwordCodeCheck struct { + userID string + + exists bool + events int +} + +func (p *passwordCodeCheck) Reduce() error { + p.exists = p.events > 0 + return nil +} + +func (p *passwordCodeCheck) AppendEvents(events ...eventstore.Event) { + p.events += len(events) +} + +func (p *passwordCodeCheck) Query() *eventstore.SearchQueryBuilder { + return eventstore.NewSearchQueryBuilder(eventstore.ColumnsEvent). + AddQuery(). + AggregateTypes(user.AggregateType). + AggregateIDs(p.userID). + EventTypes(user.UserV1PasswordCodeAddedType, user.UserV1PasswordCodeSentType, + user.HumanPasswordCodeAddedType, user.HumanPasswordCodeSentType). + Builder() +} + +func (repo *UserRepo) PasswordCodeExists(ctx context.Context, userID string) (exists bool, err error) { + model := &passwordCodeCheck{ + userID: userID, + } + err = repo.Eventstore.FilterToQueryReducer(ctx, model) + if err != nil { + return false, zerrors.ThrowPermissionDenied(err, "EVENT-SJ642", "Errors.Internal") + } + return model.exists, nil +} diff --git a/internal/auth/repository/eventsourcing/repository.go b/internal/auth/repository/eventsourcing/repository.go index a9ec6aa058..c76efab284 100644 --- a/internal/auth/repository/eventsourcing/repository.go +++ b/internal/auth/repository/eventsourcing/repository.go @@ -78,6 +78,7 @@ func Start(ctx context.Context, conf Config, systemDefaults sd.SystemDefaults, c ProjectProvider: queryView, ApplicationProvider: queries, CustomTextProvider: queries, + PasswordReset: command, IdGenerator: id.SonyFlakeGenerator(), }, eventstore.TokenRepo{ diff --git a/internal/command/user_human_email.go b/internal/command/user_human_email.go index 18bdc1caad..c954b32469 100644 --- a/internal/command/user_human_email.go +++ b/internal/command/user_human_email.go @@ -64,7 +64,7 @@ func (c *Commands) ChangeHumanEmail(ctx context.Context, email *domain.Email, em return writeModelToEmail(existingEmail), nil } -func (c *Commands) VerifyHumanEmail(ctx context.Context, userID, code, resourceowner string, emailCodeGenerator crypto.Generator) (*domain.ObjectDetails, error) { +func (c *Commands) VerifyHumanEmail(ctx context.Context, userID, code, resourceowner, optionalPassword, optionalUserAgentID string, emailCodeGenerator crypto.Generator) (*domain.ObjectDetails, error) { if userID == "" { return nil, zerrors.ThrowInvalidArgument(nil, "COMMAND-4M0ds", "Errors.User.UserIDMissing") } @@ -82,21 +82,30 @@ func (c *Commands) VerifyHumanEmail(ctx context.Context, userID, code, resourceo userAgg := UserAggregateFromWriteModel(&existingCode.WriteModel) err = crypto.VerifyCode(existingCode.CodeCreationDate, existingCode.CodeExpiry, existingCode.Code, code, emailCodeGenerator.Alg()) - if err == nil { - pushedEvents, err := c.eventstore.Push(ctx, user.NewHumanEmailVerifiedEvent(ctx, userAgg)) - if err != nil { - return nil, err - } - err = AppendAndReduce(existingCode, pushedEvents...) - if err != nil { - return nil, err - } - return writeModelToObjectDetails(&existingCode.WriteModel), nil + if err != nil { + _, err = c.eventstore.Push(ctx, user.NewHumanEmailVerificationFailedEvent(ctx, userAgg)) + logging.WithFields("userID", userAgg.ID).OnError(err).Error("NewHumanEmailVerificationFailedEvent push failed") + return nil, zerrors.ThrowInvalidArgument(err, "COMMAND-Gdsgs", "Errors.User.Code.Invalid") } - - _, err = c.eventstore.Push(ctx, user.NewHumanEmailVerificationFailedEvent(ctx, userAgg)) - logging.LogWithFields("COMMAND-Dg2z5", "userID", userAgg.ID).OnError(err).Error("NewHumanEmailVerificationFailedEvent push failed") - return nil, zerrors.ThrowInvalidArgument(err, "COMMAND-Gdsgs", "Errors.User.Code.Invalid") + commands := []eventstore.Command{ + user.NewHumanEmailVerifiedEvent(ctx, userAgg), + } + if optionalPassword != "" { + passwordCommand, err := c.setPasswordCommand(ctx, userAgg, domain.UserStateActive, optionalPassword, "", optionalUserAgentID, false, nil) + if err != nil { + return nil, err + } + commands = append(commands, passwordCommand) + } + pushedEvents, err := c.eventstore.Push(ctx, commands...) + if err != nil { + return nil, err + } + err = AppendAndReduce(existingCode, pushedEvents...) + if err != nil { + return nil, err + } + return writeModelToObjectDetails(&existingCode.WriteModel), nil } func (c *Commands) CreateHumanEmailVerificationCode(ctx context.Context, userID, resourceOwner string, emailCodeGenerator crypto.Generator, authRequestID string) (*domain.ObjectDetails, error) { diff --git a/internal/command/user_human_email_test.go b/internal/command/user_human_email_test.go index 53b95ac802..fca5ba2606 100644 --- a/internal/command/user_human_email_test.go +++ b/internal/command/user_human_email_test.go @@ -12,6 +12,7 @@ import ( "github.com/zitadel/zitadel/internal/domain" "github.com/zitadel/zitadel/internal/eventstore" "github.com/zitadel/zitadel/internal/eventstore/v1/models" + "github.com/zitadel/zitadel/internal/repository/org" "github.com/zitadel/zitadel/internal/repository/user" "github.com/zitadel/zitadel/internal/zerrors" ) @@ -387,14 +388,17 @@ func TestCommandSide_ChangeHumanEmail(t *testing.T) { func TestCommandSide_VerifyHumanEmail(t *testing.T) { type fields struct { - eventstore func(*testing.T) *eventstore.Eventstore + eventstore func(*testing.T) *eventstore.Eventstore + userPasswordHasher *crypto.Hasher } type args struct { - ctx context.Context - userID string - code string - resourceOwner string - secretGenerator crypto.Generator + ctx context.Context + userID string + code string + resourceOwner string + optionalUserAgentID string + optionalPassword string + secretGenerator crypto.Generator } type res struct { want *domain.ObjectDetails @@ -587,13 +591,96 @@ func TestCommandSide_VerifyHumanEmail(t *testing.T) { }, }, }, + { + name: "valid code (with password and user agent), ok", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + user.NewHumanAddedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "username", + "firstname", + "lastname", + "nickname", + "displayname", + language.German, + domain.GenderUnspecified, + "email@test.ch", + true, + ), + ), + eventFromEventPusherWithCreationDateNow( + user.NewHumanEmailCodeAddedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + &crypto.CryptoValue{ + CryptoType: crypto.TypeEncryption, + Algorithm: "enc", + KeyID: "id", + Crypted: []byte("a"), + }, + time.Hour*1, + "", + ), + ), + ), + expectFilter( + eventFromEventPusher( + org.NewPasswordComplexityPolicyAddedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + 1, + false, + false, + false, + false, + ), + ), + ), + expectPush( + user.NewHumanEmailVerifiedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + ), + user.NewHumanPasswordChangedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "$plain$x$password", + false, + "userAgentID", + ), + ), + ), + userPasswordHasher: mockPasswordHasher("x"), + }, + args: args{ + ctx: context.Background(), + userID: "user1", + code: "a", + resourceOwner: "org1", + optionalPassword: "password", + optionalUserAgentID: "userAgentID", + secretGenerator: GetMockSecretGenerator(t), + }, + res: res{ + want: &domain.ObjectDetails{ + ResourceOwner: "org1", + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { r := &Commands{ - eventstore: tt.fields.eventstore(t), + eventstore: tt.fields.eventstore(t), + userPasswordHasher: tt.fields.userPasswordHasher, } - got, err := r.VerifyHumanEmail(tt.args.ctx, tt.args.userID, tt.args.code, tt.args.resourceOwner, tt.args.secretGenerator) + got, err := r.VerifyHumanEmail( + tt.args.ctx, + tt.args.userID, + tt.args.code, + tt.args.resourceOwner, + tt.args.optionalPassword, + tt.args.optionalUserAgentID, + tt.args.secretGenerator, + ) if tt.res.err == nil { assert.NoError(t, err) } diff --git a/internal/command/user_human_password.go b/internal/command/user_human_password.go index 839977b384..6978f81ced 100644 --- a/internal/command/user_human_password.go +++ b/internal/command/user_human_password.go @@ -228,7 +228,7 @@ func (c *Commands) checkPasswordComplexity(ctx context.Context, newPassword stri } // RequestSetPassword generate and send out new code to change password for a specific user -func (c *Commands) RequestSetPassword(ctx context.Context, userID, resourceOwner string, notifyType domain.NotificationType, passwordVerificationCode crypto.Generator, authRequestID string) (objectDetails *domain.ObjectDetails, err error) { +func (c *Commands) RequestSetPassword(ctx context.Context, userID, resourceOwner string, notifyType domain.NotificationType, authRequestID string) (objectDetails *domain.ObjectDetails, err error) { if userID == "" { return nil, zerrors.ThrowInvalidArgument(nil, "COMMAND-M00oL", "Errors.User.UserIDMissing") } @@ -244,11 +244,11 @@ func (c *Commands) RequestSetPassword(ctx context.Context, userID, resourceOwner return nil, zerrors.ThrowPreconditionFailed(nil, "COMMAND-2M9sd", "Errors.User.NotInitialised") } userAgg := UserAggregateFromWriteModel(&existingHuman.WriteModel) - passwordCode, err := domain.NewPasswordCode(passwordVerificationCode) + passwordCode, err := c.newEncryptedCode(ctx, c.eventstore.Filter, domain.SecretGeneratorTypePasswordResetCode, c.userEncryption) //nolint:staticcheck if err != nil { return nil, err } - pushedEvents, err := c.eventstore.Push(ctx, user.NewHumanPasswordCodeAddedEvent(ctx, userAgg, passwordCode.Code, passwordCode.Expiry, notifyType, authRequestID)) + pushedEvents, err := c.eventstore.Push(ctx, user.NewHumanPasswordCodeAddedEvent(ctx, userAgg, passwordCode.Crypted, passwordCode.Expiry, notifyType, authRequestID)) if err != nil { return nil, err } diff --git a/internal/command/user_human_password_test.go b/internal/command/user_human_password_test.go index d944f5e1cc..0326967498 100644 --- a/internal/command/user_human_password_test.go +++ b/internal/command/user_human_password_test.go @@ -1111,14 +1111,14 @@ func TestCommandSide_ChangePassword(t *testing.T) { func TestCommandSide_RequestSetPassword(t *testing.T) { type fields struct { eventstore func(*testing.T) *eventstore.Eventstore + newCode encrypedCodeFunc } type args struct { - ctx context.Context - userID string - resourceOwner string - notifyType domain.NotificationType - secretGenerator crypto.Generator - authRequestID string + ctx context.Context + userID string + resourceOwner string + notifyType domain.NotificationType + authRequestID string } type res struct { want *domain.ObjectDetails @@ -1251,12 +1251,12 @@ func TestCommandSide_RequestSetPassword(t *testing.T) { ), ), ), + newCode: mockEncryptedCode("a", 1*time.Hour), }, args: args{ - ctx: context.Background(), - userID: "user1", - resourceOwner: "org1", - secretGenerator: GetMockSecretGenerator(t), + ctx: context.Background(), + userID: "user1", + resourceOwner: "org1", }, res: res{ want: &domain.ObjectDetails{ @@ -1307,13 +1307,13 @@ func TestCommandSide_RequestSetPassword(t *testing.T) { ), ), ), + newCode: mockEncryptedCode("a", 1*time.Hour), }, args: args{ - ctx: context.Background(), - userID: "user1", - resourceOwner: "org1", - secretGenerator: GetMockSecretGenerator(t), - authRequestID: "authRequestID", + ctx: context.Background(), + userID: "user1", + resourceOwner: "org1", + authRequestID: "authRequestID", }, res: res{ want: &domain.ObjectDetails{ @@ -1325,9 +1325,10 @@ func TestCommandSide_RequestSetPassword(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), + newEncryptedCode: tt.fields.newCode, } - got, err := r.RequestSetPassword(tt.args.ctx, tt.args.userID, tt.args.resourceOwner, tt.args.notifyType, tt.args.secretGenerator, tt.args.authRequestID) + got, err := r.RequestSetPassword(tt.args.ctx, tt.args.userID, tt.args.resourceOwner, tt.args.notifyType, tt.args.authRequestID) if tt.res.err == nil { assert.NoError(t, err) } diff --git a/internal/command/user_v2_human_test.go b/internal/command/user_v2_human_test.go index e3dccde360..997124e1f1 100644 --- a/internal/command/user_v2_human_test.go +++ b/internal/command/user_v2_human_test.go @@ -319,7 +319,7 @@ func TestCommandSide_AddUserHuman(t *testing.T) { }, }, { - name: "add human (with initial code), ok", + name: "add human (email not verified, no password), ok (init code)", fields: fields{ eventstore: expectEventstore( expectFilter(), @@ -389,7 +389,7 @@ func TestCommandSide_AddUserHuman(t *testing.T) { }, }, { - name: "add human (with password and initial code), ok", + name: "add human (email not verified, with password), ok (init code)", fields: fields{ eventstore: expectEventstore( expectFilter(), @@ -459,6 +459,65 @@ func TestCommandSide_AddUserHuman(t *testing.T) { wantID: "user1", }, }, + { + name: "add human (email not verified, no password, no allowInitMail), ok (email verification with passwordInit)", + fields: fields{ + eventstore: expectEventstore( + expectFilter(), + expectFilter( + eventFromEventPusher( + org.NewDomainPolicyAddedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + true, + true, + true, + ), + ), + ), + expectPush( + newAddHumanEvent("", false, true, "", language.English), + user.NewHumanEmailCodeAddedEventV2(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + &crypto.CryptoValue{ + CryptoType: crypto.TypeEncryption, + Algorithm: "enc", + KeyID: "id", + Crypted: []byte("emailverify"), + }, + 1*time.Hour, + "", + false, + "", + ), + ), + ), + checkPermission: newMockPermissionCheckAllowed(), + idGenerator: id_mock.NewIDGeneratorExpectIDs(t, "user1"), + newCode: mockEncryptedCode("emailverify", time.Hour), + }, + args: args{ + ctx: context.Background(), + orgID: "org1", + human: &AddHuman{ + Username: "username", + FirstName: "firstname", + LastName: "lastname", + Email: Email{ + Address: "email@test.ch", + }, + PreferredLanguage: language.English, + }, + secretGenerator: GetMockSecretGenerator(t), + allowInitMail: false, + codeAlg: crypto.CreateMockEncryptionAlg(gomock.NewController(t)), + }, + res: res{ + want: &domain.ObjectDetails{ + ResourceOwner: "org1", + }, + wantID: "user1", + }, + }, { name: "add human (with password and email code custom template), ok", fields: fields{ @@ -609,7 +668,7 @@ func TestCommandSide_AddUserHuman(t *testing.T) { }, }, { - name: "add human email verified, ok", + name: "add human email verified and password, ok", fields: fields{ eventstore: expectEventstore( expectFilter(), @@ -1084,8 +1143,9 @@ func TestCommandSide_AddUserHuman(t *testing.T) { }, wantID: "user1", }, - }, { - name: "add human (with return code), ok", + }, + { + name: "add human (with phone return code), ok", fields: fields{ eventstore: expectEventstore( expectFilter(), diff --git a/internal/domain/next_step.go b/internal/domain/next_step.go index c871017588..009989956f 100644 --- a/internal/domain/next_step.go +++ b/internal/domain/next_step.go @@ -137,7 +137,9 @@ func (s *ChangeUsernameStep) Type() NextStepType { return NextStepChangeUsername } -type VerifyEMailStep struct{} +type VerifyEMailStep struct { + InitPassword bool +} func (s *VerifyEMailStep) Type() NextStepType { return NextStepVerifyEmail