mirror of
https://github.com/zitadel/zitadel.git
synced 2024-12-14 11:58:02 +00:00
fix(login): make sure first email verification is done before MFA check (#9039)
# Which Problems Are Solved During authentication in the login UI, there is a check if the user's MFA is already checked or needs to be setup. In cases where the user was just set up or especially, if the user was just federated without a verified email address, this can lead to the problem, where OTP Email cannot be setup as there's no verified email address. # How the Problems Are Solved - Added a check if there's no verified email address on the user and require a mail verification check before checking for MFA. Note: that if the user had a verified email address, but changed it and has not verified it, they will still be prompted with an MFA check before the email verification. This is make sure, we don't break the existing behavior and the user's authentication is properly checked. # Additional Changes None # Additional Context - closes https://github.com/zitadel/zitadel/issues/9035
This commit is contained in:
parent
fd70a7de5f
commit
f20539ef8f
@ -1081,6 +1081,15 @@ func (repo *AuthRequestRepo) nextSteps(ctx context.Context, request *domain.Auth
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// If the user never had a verified email, we need to verify it.
|
||||||
|
// This prevents situations, where OTP email is the only MFA method and no verified email is set.
|
||||||
|
// If the user had a verified email, but change it and has not yet verified the new one, we'll verify it after we checked the MFA methods.
|
||||||
|
if user.VerifiedEmail == "" && !user.IsEmailVerified {
|
||||||
|
return append(steps, &domain.VerifyEMailStep{
|
||||||
|
InitPassword: !user.PasswordSet && len(idps.Links) == 0,
|
||||||
|
}), nil
|
||||||
|
}
|
||||||
|
|
||||||
step, ok, err := repo.mfaChecked(userSession, request, user, isInternalLogin && len(request.LinkingUsers) == 0)
|
step, ok, err := repo.mfaChecked(userSession, request, user, isInternalLogin && len(request.LinkingUsers) == 0)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
|
@ -156,6 +156,7 @@ type mockViewUser struct {
|
|||||||
PasswordChanged time.Time
|
PasswordChanged time.Time
|
||||||
PasswordChangeRequired bool
|
PasswordChangeRequired bool
|
||||||
IsEmailVerified bool
|
IsEmailVerified bool
|
||||||
|
VerifiedEmail string
|
||||||
OTPState int32
|
OTPState int32
|
||||||
MFAMaxSetUp int32
|
MFAMaxSetUp int32
|
||||||
MFAInitSkipped time.Time
|
MFAInitSkipped time.Time
|
||||||
@ -222,6 +223,7 @@ func (m *mockViewUser) UserByID(context.Context, string, string) (*user_view_mod
|
|||||||
PasswordSet: m.PasswordSet,
|
PasswordSet: m.PasswordSet,
|
||||||
PasswordChangeRequired: m.PasswordChangeRequired,
|
PasswordChangeRequired: m.PasswordChangeRequired,
|
||||||
IsEmailVerified: m.IsEmailVerified,
|
IsEmailVerified: m.IsEmailVerified,
|
||||||
|
VerifiedEmail: m.VerifiedEmail,
|
||||||
OTPState: m.OTPState,
|
OTPState: m.OTPState,
|
||||||
MFAMaxSetUp: m.MFAMaxSetUp,
|
MFAMaxSetUp: m.MFAMaxSetUp,
|
||||||
MFAInitSkipped: m.MFAInitSkipped,
|
MFAInitSkipped: m.MFAInitSkipped,
|
||||||
@ -1403,6 +1405,7 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) {
|
|||||||
PasswordVerification: testNow.Add(-5 * time.Minute),
|
PasswordVerification: testNow.Add(-5 * time.Minute),
|
||||||
},
|
},
|
||||||
userViewProvider: &mockViewUser{
|
userViewProvider: &mockViewUser{
|
||||||
|
VerifiedEmail: "verified",
|
||||||
PasswordSet: true,
|
PasswordSet: true,
|
||||||
PasswordlessTokens: user_view_model.WebAuthNTokens{&user_view_model.WebAuthNView{ID: "id", State: int32(user_model.MFAStateReady)}},
|
PasswordlessTokens: user_view_model.WebAuthNTokens{&user_view_model.WebAuthNView{ID: "id", State: int32(user_model.MFAStateReady)}},
|
||||||
OTPState: int32(user_model.MFAStateReady),
|
OTPState: int32(user_model.MFAStateReady),
|
||||||
@ -1439,6 +1442,7 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) {
|
|||||||
PasswordVerification: testNow.Add(-5 * time.Minute),
|
PasswordVerification: testNow.Add(-5 * time.Minute),
|
||||||
},
|
},
|
||||||
userViewProvider: &mockViewUser{
|
userViewProvider: &mockViewUser{
|
||||||
|
VerifiedEmail: "verified",
|
||||||
PasswordSet: true,
|
PasswordSet: true,
|
||||||
OTPState: int32(user_model.MFAStateReady),
|
OTPState: int32(user_model.MFAStateReady),
|
||||||
MFAMaxSetUp: int32(domain.MFALevelSecondFactor),
|
MFAMaxSetUp: int32(domain.MFALevelSecondFactor),
|
||||||
@ -1475,6 +1479,7 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) {
|
|||||||
ExternalLoginVerification: testNow.Add(-5 * time.Minute),
|
ExternalLoginVerification: testNow.Add(-5 * time.Minute),
|
||||||
},
|
},
|
||||||
userViewProvider: &mockViewUser{
|
userViewProvider: &mockViewUser{
|
||||||
|
VerifiedEmail: "verified",
|
||||||
PasswordSet: true,
|
PasswordSet: true,
|
||||||
OTPState: int32(user_model.MFAStateReady),
|
OTPState: int32(user_model.MFAStateReady),
|
||||||
MFAMaxSetUp: int32(domain.MFALevelSecondFactor),
|
MFAMaxSetUp: int32(domain.MFALevelSecondFactor),
|
||||||
@ -1505,6 +1510,44 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) {
|
|||||||
}},
|
}},
|
||||||
nil,
|
nil,
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
"external user, mfa not verified, email never verified, email verification step",
|
||||||
|
fields{
|
||||||
|
userSessionViewProvider: &mockViewUserSession{
|
||||||
|
PasswordVerification: testNow.Add(-5 * time.Minute),
|
||||||
|
ExternalLoginVerification: testNow.Add(-5 * time.Minute),
|
||||||
|
},
|
||||||
|
userViewProvider: &mockViewUser{
|
||||||
|
PasswordSet: true,
|
||||||
|
OTPState: int32(user_model.MFAStateReady),
|
||||||
|
MFAMaxSetUp: int32(domain.MFALevelSecondFactor),
|
||||||
|
},
|
||||||
|
userEventProvider: &mockEventUser{},
|
||||||
|
orgViewProvider: &mockViewOrg{State: domain.OrgStateActive},
|
||||||
|
lockoutPolicyProvider: &mockLockoutPolicy{
|
||||||
|
policy: &query.LockoutPolicy{
|
||||||
|
ShowFailures: true,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
idpUserLinksProvider: &mockIDPUserLinks{},
|
||||||
|
},
|
||||||
|
args{
|
||||||
|
&domain.AuthRequest{
|
||||||
|
UserID: "UserID",
|
||||||
|
SelectedIDPConfigID: "IDPConfigID",
|
||||||
|
LoginPolicy: &domain.LoginPolicy{
|
||||||
|
AllowUsernamePassword: true,
|
||||||
|
SecondFactors: []domain.SecondFactorType{domain.SecondFactorTypeTOTP},
|
||||||
|
PasswordCheckLifetime: 10 * 24 * time.Hour,
|
||||||
|
ExternalLoginCheckLifetime: 10 * 24 * time.Hour,
|
||||||
|
SecondFactorCheckLifetime: 18 * time.Hour,
|
||||||
|
},
|
||||||
|
}, false},
|
||||||
|
[]domain.NextStep{&domain.VerifyEMailStep{
|
||||||
|
InitPassword: false,
|
||||||
|
}},
|
||||||
|
nil,
|
||||||
|
},
|
||||||
{
|
{
|
||||||
"password change required and email verified, password change step",
|
"password change required and email verified, password change step",
|
||||||
fields{
|
fields{
|
||||||
@ -1573,13 +1616,14 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) {
|
|||||||
nil,
|
nil,
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"email not verified and password change required, mail verification step",
|
"email not verified (but had before) and password change required, mail verification step",
|
||||||
fields{
|
fields{
|
||||||
userSessionViewProvider: &mockViewUserSession{
|
userSessionViewProvider: &mockViewUserSession{
|
||||||
PasswordVerification: testNow.Add(-5 * time.Minute),
|
PasswordVerification: testNow.Add(-5 * time.Minute),
|
||||||
SecondFactorVerification: testNow.Add(-5 * time.Minute),
|
SecondFactorVerification: testNow.Add(-5 * time.Minute),
|
||||||
},
|
},
|
||||||
userViewProvider: &mockViewUser{
|
userViewProvider: &mockViewUser{
|
||||||
|
VerifiedEmail: "verified",
|
||||||
PasswordSet: true,
|
PasswordSet: true,
|
||||||
PasswordChangeRequired: true,
|
PasswordChangeRequired: true,
|
||||||
MFAMaxSetUp: int32(domain.MFALevelSecondFactor),
|
MFAMaxSetUp: int32(domain.MFALevelSecondFactor),
|
||||||
|
@ -40,6 +40,7 @@ type HumanView struct {
|
|||||||
Gender Gender
|
Gender Gender
|
||||||
Email string
|
Email string
|
||||||
IsEmailVerified bool
|
IsEmailVerified bool
|
||||||
|
VerifiedEmail string
|
||||||
Phone string
|
Phone string
|
||||||
IsPhoneVerified bool
|
IsPhoneVerified bool
|
||||||
Country string
|
Country string
|
||||||
|
@ -79,6 +79,7 @@ type HumanView struct {
|
|||||||
AvatarKey string `json:"storeKey" gorm:"column:avatar_key"`
|
AvatarKey string `json:"storeKey" gorm:"column:avatar_key"`
|
||||||
Email string `json:"email" gorm:"column:email"`
|
Email string `json:"email" gorm:"column:email"`
|
||||||
IsEmailVerified bool `json:"-" gorm:"column:is_email_verified"`
|
IsEmailVerified bool `json:"-" gorm:"column:is_email_verified"`
|
||||||
|
VerifiedEmail string `json:"-" gorm:"column:verified_email"`
|
||||||
Phone string `json:"phone" gorm:"column:phone"`
|
Phone string `json:"phone" gorm:"column:phone"`
|
||||||
IsPhoneVerified bool `json:"-" gorm:"column:is_phone_verified"`
|
IsPhoneVerified bool `json:"-" gorm:"column:is_phone_verified"`
|
||||||
Country string `json:"country" gorm:"column:country"`
|
Country string `json:"country" gorm:"column:country"`
|
||||||
@ -170,6 +171,7 @@ func UserToModel(user *UserView) *model.UserView {
|
|||||||
Gender: model.Gender(user.Gender),
|
Gender: model.Gender(user.Gender),
|
||||||
Email: user.Email,
|
Email: user.Email,
|
||||||
IsEmailVerified: user.IsEmailVerified,
|
IsEmailVerified: user.IsEmailVerified,
|
||||||
|
VerifiedEmail: user.VerifiedEmail,
|
||||||
Phone: user.Phone,
|
Phone: user.Phone,
|
||||||
IsPhoneVerified: user.IsPhoneVerified,
|
IsPhoneVerified: user.IsPhoneVerified,
|
||||||
Country: user.Country,
|
Country: user.Country,
|
||||||
|
@ -42,6 +42,7 @@ SELECT
|
|||||||
, h.gender
|
, h.gender
|
||||||
, h.email
|
, h.email
|
||||||
, h.is_email_verified
|
, h.is_email_verified
|
||||||
|
, n.verified_email
|
||||||
, h.phone
|
, h.phone
|
||||||
, h.is_phone_verified
|
, h.is_phone_verified
|
||||||
, (SELECT COALESCE((SELECT state FROM auth_methods WHERE method_type = 1), 0)) AS otp_state
|
, (SELECT COALESCE((SELECT state FROM auth_methods WHERE method_type = 1), 0)) AS otp_state
|
||||||
@ -77,6 +78,9 @@ FROM projections.users13 u
|
|||||||
LEFT JOIN projections.users13_humans h
|
LEFT JOIN projections.users13_humans h
|
||||||
ON u.instance_id = h.instance_id
|
ON u.instance_id = h.instance_id
|
||||||
AND u.id = h.user_id
|
AND u.id = h.user_id
|
||||||
|
LEFT JOIN projections.users13_notifications n
|
||||||
|
ON u.instance_id = n.instance_id
|
||||||
|
AND u.id = n.user_id
|
||||||
LEFT JOIN projections.login_names3 l
|
LEFT JOIN projections.login_names3 l
|
||||||
ON u.instance_id = l.instance_id
|
ON u.instance_id = l.instance_id
|
||||||
AND u.id = l.user_id
|
AND u.id = l.user_id
|
||||||
|
Loading…
Reference in New Issue
Block a user