From 1189a17b707f77f2cc0a70afe76f95e4d9e18a36 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 (cherry picked from commit 650c21f18af91b0056f1e337e5d3aa21946e84b6) --- .../eventsourcing/eventstore/auth_request.go | 33 +++-- .../eventstore/auth_request_test.go | 114 +++++++++++++++--- 2 files changed, 123 insertions(+), 24 deletions(-) diff --git a/internal/auth/repository/eventsourcing/eventstore/auth_request.go b/internal/auth/repository/eventsourcing/eventstore/auth_request.go index 1b05932067..7a2ad12991 100644 --- a/internal/auth/repository/eventsourcing/eventstore/auth_request.go +++ b/internal/auth/repository/eventsourcing/eventstore/auth_request.go @@ -1053,8 +1053,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 } @@ -1254,20 +1258,29 @@ func (repo *AuthRequestRepo) firstFactorChecked(request *domain.AuthRequest, use 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 10528a8a25..b72b0ef8c3 100644 --- a/internal/auth/repository/eventsourcing/eventstore/auth_request_test.go +++ b/internal/auth/repository/eventsourcing/eventstore/auth_request_test.go @@ -538,6 +538,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), @@ -559,6 +560,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, @@ -783,7 +785,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, }, @@ -820,9 +830,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, }}, @@ -849,7 +872,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, }, @@ -874,7 +906,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, }, @@ -900,7 +940,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, }, @@ -927,14 +975,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, }, @@ -954,7 +1006,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.InitPasswordStep{}}, nil, }, @@ -986,6 +1046,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"}}, @@ -1020,6 +1081,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"}}, @@ -1054,6 +1116,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, }, @@ -1083,7 +1146,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, }, @@ -1117,6 +1188,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, }, @@ -1149,6 +1221,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, @@ -1183,6 +1256,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, @@ -1219,6 +1293,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, @@ -1256,6 +1331,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, @@ -1287,6 +1363,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, @@ -1319,6 +1396,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, @@ -1358,6 +1436,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, @@ -1397,6 +1476,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, @@ -1434,6 +1514,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, @@ -1471,6 +1552,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, @@ -1510,6 +1592,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, @@ -1550,6 +1633,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, @@ -1590,6 +1674,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, @@ -1631,6 +1716,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,