From 650c21f18af91b0056f1e337e5d3aa21946e84b6 Mon Sep 17 00:00:00 2001 From: Livio Spring Date: Tue, 10 Sep 2024 12:55:32 +0200 Subject: [PATCH] fix: check if pw login allowed (#8584) # Which Problems Are Solved When checking for the next step for the login UI and a user did not yet have an IdP linked, they would always be presented the password check screen, even if the local authentication was disabled. # How the Problems Are Solved - Correctly check the login policy for the `Allow Username Password` option - In case the user has no IdP linked yet, fallback to the organizations configuration (and redirect if possible) - the user can be auto-linked based on the username / email after successfully authenticating at the IdP # Additional Changes None # Additional Context - closes https://github.com/zitadel/zitadel/issues/5106 - closes https://github.com/zitadel/zitadel/issues/7502 --- .../eventsourcing/eventstore/auth_request.go | 33 +++-- .../eventstore/auth_request_test.go | 134 +++++++++++++++--- 2 files changed, 141 insertions(+), 26 deletions(-) diff --git a/internal/auth/repository/eventsourcing/eventstore/auth_request.go b/internal/auth/repository/eventsourcing/eventstore/auth_request.go index ca1cc5a03..9a9c52012 100644 --- a/internal/auth/repository/eventsourcing/eventstore/auth_request.go +++ b/internal/auth/repository/eventsourcing/eventstore/auth_request.go @@ -1060,8 +1060,12 @@ func (repo *AuthRequestRepo) nextSteps(ctx context.Context, request *domain.Auth if err != nil { return nil, err } - if (!isInternalLogin || len(idps.Links) > 0) && len(request.LinkingUsers) == 0 { - step := repo.idpChecked(request, idps.Links, userSession) + noLocalAuth := request.LoginPolicy != nil && !request.LoginPolicy.AllowUsernamePassword + if (!isInternalLogin || len(idps.Links) > 0 || noLocalAuth) && len(request.LinkingUsers) == 0 { + step, err := repo.idpChecked(request, idps.Links, userSession) + if err != nil { + return nil, err + } if step != nil { return append(steps, step), nil } @@ -1272,20 +1276,29 @@ func (repo *AuthRequestRepo) firstFactorChecked(ctx context.Context, request *do return &domain.PasswordStep{} } -func (repo *AuthRequestRepo) idpChecked(request *domain.AuthRequest, idps []*query.IDPUserLink, userSession *user_model.UserSessionView) domain.NextStep { +func (repo *AuthRequestRepo) idpChecked(request *domain.AuthRequest, idps []*query.IDPUserLink, userSession *user_model.UserSessionView) (domain.NextStep, error) { if checkVerificationTimeMaxAge(userSession.ExternalLoginVerification, request.LoginPolicy.ExternalLoginCheckLifetime, request) { request.IDPLoginChecked = true request.AuthTime = userSession.ExternalLoginVerification - return nil + return nil, nil } - selectedIDPConfigID := request.SelectedIDPConfigID - if selectedIDPConfigID == "" { - selectedIDPConfigID = userSession.SelectedIDPConfigID + // use the explicitly set IdP first + if request.SelectedIDPConfigID != "" { + return &domain.ExternalLoginStep{SelectedIDPConfigID: request.SelectedIDPConfigID}, nil } - if selectedIDPConfigID == "" && len(idps) > 0 { - selectedIDPConfigID = idps[0].IDPID + // reuse the previously used IdP from the session + if userSession.SelectedIDPConfigID != "" { + return &domain.ExternalLoginStep{SelectedIDPConfigID: userSession.SelectedIDPConfigID}, nil } - return &domain.ExternalLoginStep{SelectedIDPConfigID: selectedIDPConfigID} + // then use an existing linked IdP of the user + if len(idps) > 0 { + return &domain.ExternalLoginStep{SelectedIDPConfigID: idps[0].IDPID}, nil + } + // if the user did not link one, then just use one of the configured IdPs of the org + if len(request.AllowedExternalIDPs) > 0 { + return &domain.ExternalLoginStep{SelectedIDPConfigID: request.AllowedExternalIDPs[0].IDPConfigID}, nil + } + return nil, zerrors.ThrowPreconditionFailed(nil, "LOGIN-5Hm8s", "Errors.Org.IdpNotExisting") } func (repo *AuthRequestRepo) mfaChecked(userSession *user_model.UserSessionView, request *domain.AuthRequest, user *user_model.UserView, isInternalAuthentication bool) (domain.NextStep, bool, error) { diff --git a/internal/auth/repository/eventsourcing/eventstore/auth_request_test.go b/internal/auth/repository/eventsourcing/eventstore/auth_request_test.go index 9ed47762b..96a7fc8e4 100644 --- a/internal/auth/repository/eventsourcing/eventstore/auth_request_test.go +++ b/internal/auth/repository/eventsourcing/eventstore/auth_request_test.go @@ -563,6 +563,7 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) { idpUserLinksProvider: &mockIDPUserLinks{}, loginPolicyProvider: &mockLoginPolicy{ policy: &query.LoginPolicy{ + AllowUsernamePassword: true, SecondFactors: []domain.SecondFactorType{domain.SecondFactorTypeTOTP}, PasswordCheckLifetime: database.Duration(10 * 24 * time.Hour), SecondFactorCheckLifetime: database.Duration(18 * time.Hour), @@ -584,6 +585,7 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) { args{&domain.AuthRequest{ Request: &domain.AuthRequestOIDC{}, LoginPolicy: &domain.LoginPolicy{ + AllowUsernamePassword: true, SecondFactors: []domain.SecondFactorType{domain.SecondFactorTypeTOTP}, PasswordCheckLifetime: 10 * 24 * time.Hour, SecondFactorCheckLifetime: 18 * time.Hour, @@ -812,7 +814,15 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) { }, idpUserLinksProvider: &mockIDPUserLinks{}, }, - args{&domain.AuthRequest{UserID: "UserID", LoginPolicy: &domain.LoginPolicy{}}, false}, + args{ + &domain.AuthRequest{ + UserID: "UserID", + LoginPolicy: &domain.LoginPolicy{ + AllowUsernamePassword: true, + }, + }, + false, + }, []domain.NextStep{&domain.PasswordStep{}}, nil, }, @@ -849,9 +859,22 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) { ShowFailures: true, }, }, + loginPolicyProvider: &mockLoginPolicy{ + policy: &query.LoginPolicy{ + AllowUsernamePassword: true, + }, + }, idpUserLinksProvider: &mockIDPUserLinks{}, }, - args{&domain.AuthRequest{UserID: "UserID", LoginPolicy: &domain.LoginPolicy{}}, false}, + args{ + &domain.AuthRequest{ + UserID: "UserID", + LoginPolicy: &domain.LoginPolicy{ + AllowUsernamePassword: true, + }, + }, + false, + }, []domain.NextStep{&domain.InitUserStep{ PasswordSet: true, }}, @@ -878,7 +901,16 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) { }, idpUserLinksProvider: &mockIDPUserLinks{}, }, - args{&domain.AuthRequest{UserID: "UserID", LoginPolicy: &domain.LoginPolicy{PasswordlessType: domain.PasswordlessTypeAllowed}}, false}, + args{ + &domain.AuthRequest{ + UserID: "UserID", + LoginPolicy: &domain.LoginPolicy{ + AllowUsernamePassword: true, + PasswordlessType: domain.PasswordlessTypeAllowed, + }, + }, + false, + }, []domain.NextStep{&domain.PasswordlessRegistrationPromptStep{}}, nil, }, @@ -903,7 +935,15 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) { }, idpUserLinksProvider: &mockIDPUserLinks{}, }, - args{&domain.AuthRequest{UserID: "UserID", LoginPolicy: &domain.LoginPolicy{PasswordlessType: domain.PasswordlessTypeAllowed}}, false}, + args{ + &domain.AuthRequest{ + UserID: "UserID", + LoginPolicy: &domain.LoginPolicy{ + AllowUsernamePassword: true, + PasswordlessType: domain.PasswordlessTypeAllowed, + }, + }, false, + }, []domain.NextStep{&domain.PasswordlessStep{}}, nil, }, @@ -929,7 +969,15 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) { }, idpUserLinksProvider: &mockIDPUserLinks{}, }, - args{&domain.AuthRequest{UserID: "UserID", LoginPolicy: &domain.LoginPolicy{PasswordlessType: domain.PasswordlessTypeAllowed}}, false}, + args{ + &domain.AuthRequest{ + UserID: "UserID", + LoginPolicy: &domain.LoginPolicy{ + AllowUsernamePassword: true, + PasswordlessType: domain.PasswordlessTypeAllowed, + }, + }, false, + }, []domain.NextStep{&domain.PasswordlessStep{PasswordSet: true}}, nil, }, @@ -956,14 +1004,18 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) { orgViewProvider: &mockViewOrg{State: domain.OrgStateActive}, idpUserLinksProvider: &mockIDPUserLinks{}, }, - args{&domain.AuthRequest{ - UserID: "UserID", - LoginPolicy: &domain.LoginPolicy{ - PasswordlessType: domain.PasswordlessTypeAllowed, - MultiFactors: []domain.MultiFactorType{domain.MultiFactorTypeU2FWithPIN}, - MultiFactorCheckLifetime: 10 * time.Hour, + args{ + &domain.AuthRequest{ + UserID: "UserID", + LoginPolicy: &domain.LoginPolicy{ + AllowUsernamePassword: true, + PasswordlessType: domain.PasswordlessTypeAllowed, + MultiFactors: []domain.MultiFactorType{domain.MultiFactorTypeU2FWithPIN}, + MultiFactorCheckLifetime: 10 * time.Hour, + }, }, - }, false}, + false, + }, []domain.NextStep{&domain.VerifyEMailStep{}}, nil, }, @@ -983,7 +1035,15 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) { orgViewProvider: &mockViewOrg{State: domain.OrgStateActive}, idpUserLinksProvider: &mockIDPUserLinks{}, }, - args{&domain.AuthRequest{UserID: "UserID", LoginPolicy: &domain.LoginPolicy{}}, false}, + args{ + &domain.AuthRequest{ + UserID: "UserID", + LoginPolicy: &domain.LoginPolicy{ + AllowUsernamePassword: true, + }, + }, + false, + }, []domain.NextStep{&domain.VerifyEMailStep{InitPassword: true}}, nil, }, @@ -1007,7 +1067,15 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) { idpUserLinksProvider: &mockIDPUserLinks{}, passwordReset: newMockPasswordReset(false), }, - args{&domain.AuthRequest{UserID: "UserID", LoginPolicy: &domain.LoginPolicy{}}, false}, + args{ + &domain.AuthRequest{ + UserID: "UserID", + LoginPolicy: &domain.LoginPolicy{ + AllowUsernamePassword: true, + }, + }, + false, + }, []domain.NextStep{&domain.InitPasswordStep{}}, nil, }, @@ -1031,7 +1099,15 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) { idpUserLinksProvider: &mockIDPUserLinks{}, passwordReset: newMockPasswordReset(true), }, - args{&domain.AuthRequest{UserID: "UserID", LoginPolicy: &domain.LoginPolicy{}}, false}, + args{ + &domain.AuthRequest{ + UserID: "UserID", + LoginPolicy: &domain.LoginPolicy{ + AllowUsernamePassword: true, + }, + }, + false, + }, []domain.NextStep{&domain.InitPasswordStep{}}, nil, }, @@ -1063,6 +1139,7 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) { UserID: "UserID", SelectedIDPConfigID: "IDPConfigID", LoginPolicy: &domain.LoginPolicy{ + AllowUsernamePassword: false, SecondFactorCheckLifetime: 18 * time.Hour, }}, false}, []domain.NextStep{&domain.ExternalLoginStep{SelectedIDPConfigID: "IDPConfigID"}}, @@ -1097,6 +1174,7 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) { args{&domain.AuthRequest{ UserID: "UserID", LoginPolicy: &domain.LoginPolicy{ + AllowUsernamePassword: false, SecondFactorCheckLifetime: 18 * time.Hour, }}, false}, []domain.NextStep{&domain.ExternalLoginStep{SelectedIDPConfigID: "IDPConfigID"}}, @@ -1131,6 +1209,7 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) { SelectedIDPConfigID: "IDPConfigID", Request: &domain.AuthRequestOIDC{}, LoginPolicy: &domain.LoginPolicy{ + AllowUsernamePassword: false, ExternalLoginCheckLifetime: 10 * 24 * time.Hour, SecondFactorCheckLifetime: 18 * time.Hour, }, @@ -1160,7 +1239,15 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) { }, idpUserLinksProvider: &mockIDPUserLinks{}, }, - args{&domain.AuthRequest{UserID: "UserID", LoginPolicy: &domain.LoginPolicy{}}, false}, + args{ + &domain.AuthRequest{ + UserID: "UserID", + LoginPolicy: &domain.LoginPolicy{ + AllowUsernamePassword: true, + }, + }, + false, + }, []domain.NextStep{&domain.PasswordStep{}}, nil, }, @@ -1194,6 +1281,7 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) { SelectedIDPConfigID: "IDPConfigID", Request: &domain.AuthRequestOIDC{}, LoginPolicy: &domain.LoginPolicy{ + AllowUsernamePassword: true, SecondFactorCheckLifetime: 18 * time.Hour, ExternalLoginCheckLifetime: 10 * 24 * time.Hour, }, @@ -1226,6 +1314,7 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) { &domain.AuthRequest{ UserID: "UserID", LoginPolicy: &domain.LoginPolicy{ + AllowUsernamePassword: true, SecondFactors: []domain.SecondFactorType{domain.SecondFactorTypeTOTP}, PasswordCheckLifetime: 10 * 24 * time.Hour, SecondFactorCheckLifetime: 18 * time.Hour, @@ -1260,6 +1349,7 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) { &domain.AuthRequest{ UserID: "UserID", LoginPolicy: &domain.LoginPolicy{ + AllowUsernamePassword: true, SecondFactors: []domain.SecondFactorType{domain.SecondFactorTypeTOTP}, PasswordCheckLifetime: 10 * 24 * time.Hour, SecondFactorCheckLifetime: 18 * time.Hour, @@ -1296,6 +1386,7 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) { UserID: "UserID", SelectedIDPConfigID: "IDPConfigID", LoginPolicy: &domain.LoginPolicy{ + AllowUsernamePassword: true, SecondFactors: []domain.SecondFactorType{domain.SecondFactorTypeTOTP}, PasswordCheckLifetime: 10 * 24 * time.Hour, ExternalLoginCheckLifetime: 10 * 24 * time.Hour, @@ -1333,6 +1424,7 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) { &domain.AuthRequest{ UserID: "UserID", LoginPolicy: &domain.LoginPolicy{ + AllowUsernamePassword: true, SecondFactors: []domain.SecondFactorType{domain.SecondFactorTypeTOTP}, PasswordCheckLifetime: 10 * 24 * time.Hour, SecondFactorCheckLifetime: 18 * time.Hour, @@ -1364,6 +1456,7 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) { args{&domain.AuthRequest{ UserID: "UserID", LoginPolicy: &domain.LoginPolicy{ + AllowUsernamePassword: true, SecondFactors: []domain.SecondFactorType{domain.SecondFactorTypeTOTP}, PasswordCheckLifetime: 10 * 24 * time.Hour, SecondFactorCheckLifetime: 18 * time.Hour, @@ -1396,6 +1489,7 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) { args{&domain.AuthRequest{ UserID: "UserID", LoginPolicy: &domain.LoginPolicy{ + AllowUsernamePassword: true, SecondFactors: []domain.SecondFactorType{domain.SecondFactorTypeTOTP}, PasswordCheckLifetime: 10 * 24 * time.Hour, SecondFactorCheckLifetime: 18 * time.Hour, @@ -1435,6 +1529,7 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) { args{&domain.AuthRequest{ UserID: "UserID", LoginPolicy: &domain.LoginPolicy{ + AllowUsernamePassword: true, SecondFactors: []domain.SecondFactorType{domain.SecondFactorTypeTOTP}, PasswordCheckLifetime: 10 * 24 * time.Hour, SecondFactorCheckLifetime: 18 * time.Hour, @@ -1474,6 +1569,7 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) { UserID: "UserID", Request: &domain.AuthRequestOIDC{}, LoginPolicy: &domain.LoginPolicy{ + AllowUsernamePassword: true, SecondFactors: []domain.SecondFactorType{domain.SecondFactorTypeTOTP}, PasswordCheckLifetime: 10 * 24 * time.Hour, SecondFactorCheckLifetime: 18 * time.Hour, @@ -1511,6 +1607,7 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) { Prompt: []domain.Prompt{domain.PromptNone}, Request: &domain.AuthRequestOIDC{}, LoginPolicy: &domain.LoginPolicy{ + AllowUsernamePassword: true, SecondFactors: []domain.SecondFactorType{domain.SecondFactorTypeTOTP}, PasswordCheckLifetime: 10 * 24 * time.Hour, SecondFactorCheckLifetime: 18 * time.Hour, @@ -1548,6 +1645,7 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) { Prompt: []domain.Prompt{domain.PromptNone}, Request: &domain.AuthRequestOIDC{}, LoginPolicy: &domain.LoginPolicy{ + AllowUsernamePassword: true, SecondFactors: []domain.SecondFactorType{domain.SecondFactorTypeTOTP}, PasswordCheckLifetime: 10 * 24 * time.Hour, SecondFactorCheckLifetime: 18 * time.Hour, @@ -1587,6 +1685,7 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) { Prompt: []domain.Prompt{domain.PromptNone}, Request: &domain.AuthRequestOIDC{}, LoginPolicy: &domain.LoginPolicy{ + AllowUsernamePassword: true, SecondFactors: []domain.SecondFactorType{domain.SecondFactorTypeTOTP}, PasswordCheckLifetime: 10 * 24 * time.Hour, SecondFactorCheckLifetime: 18 * time.Hour, @@ -1627,6 +1726,7 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) { Prompt: []domain.Prompt{domain.PromptNone}, Request: &domain.AuthRequestOIDC{}, LoginPolicy: &domain.LoginPolicy{ + AllowUsernamePassword: true, SecondFactors: []domain.SecondFactorType{domain.SecondFactorTypeTOTP}, PasswordCheckLifetime: 10 * 24 * time.Hour, SecondFactorCheckLifetime: 18 * time.Hour, @@ -1667,6 +1767,7 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) { Prompt: []domain.Prompt{domain.PromptNone}, Request: &domain.AuthRequestOIDC{}, LoginPolicy: &domain.LoginPolicy{ + AllowUsernamePassword: true, SecondFactors: []domain.SecondFactorType{domain.SecondFactorTypeTOTP}, PasswordCheckLifetime: 10 * 24 * time.Hour, SecondFactorCheckLifetime: 18 * time.Hour, @@ -1708,6 +1809,7 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) { Prompt: []domain.Prompt{domain.PromptNone}, Request: &domain.AuthRequestOIDC{}, LoginPolicy: &domain.LoginPolicy{ + AllowUsernamePassword: true, SecondFactors: []domain.SecondFactorType{domain.SecondFactorTypeTOTP}, PasswordCheckLifetime: 10 * 24 * time.Hour, SecondFactorCheckLifetime: 18 * time.Hour,