fix: only allowed idps in login step (#9136)

# Which Problems Are Solved

If a not allowed IDP is selected or now not allowed IDP was selected
before at login, the login will still try to use it as fallback.
The same goes for the linked IDPs which are not necessarily active
anymore, or disallowed through policies.

# How the Problems Are Solved

Check all possible or configured IDPs if they can be used.

# Additional Changes

None

# Additional Context

Addition to #6466

---------

Co-authored-by: Livio Spring <livio.a@gmail.com>
(cherry picked from commit 8d8f38fb4ca6e767f266ceffa07244c9c55448c2)
This commit is contained in:
Stefan Benz 2025-01-07 17:34:59 +01:00 committed by Livio Spring
parent 037384d5cc
commit 1129d7d7c3
No known key found for this signature in database
GPG Key ID: 26BB1C2FA5952CF0
2 changed files with 259 additions and 8 deletions

View File

@ -1065,8 +1065,10 @@ func (repo *AuthRequestRepo) nextSteps(ctx context.Context, request *domain.Auth
return nil, err
}
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)
allowedLinkedIDPs := checkForAllowedIDPs(request.AllowedExternalIDPs, idps.Links)
if (!isInternalLogin || len(allowedLinkedIDPs) > 0 || noLocalAuth) && len(request.LinkingUsers) == 0 {
step, err := repo.idpChecked(request, allowedLinkedIDPs, userSession)
if err != nil {
return nil, err
}
@ -1146,6 +1148,19 @@ func (repo *AuthRequestRepo) nextSteps(ctx context.Context, request *domain.Auth
return append(steps, &domain.RedirectToCallbackStep{}), nil
}
func checkForAllowedIDPs(allowedIDPs []*domain.IDPProvider, idps []*query.IDPUserLink) (_ []string) {
allowedLinkedIDPs := make([]string, 0, len(idps))
// only use allowed linked idps
for _, idp := range idps {
for _, allowedIdP := range allowedIDPs {
if idp.IDPID == allowedIdP.IDPConfigID {
allowedLinkedIDPs = append(allowedLinkedIDPs, allowedIdP.IDPConfigID)
}
}
}
return allowedLinkedIDPs
}
func passwordAgeChangeRequired(policy *domain.PasswordAgePolicy, changed time.Time) bool {
if policy == nil || policy.MaxAgeDays == 0 {
return false
@ -1299,7 +1314,7 @@ 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, error) {
func (repo *AuthRequestRepo) idpChecked(request *domain.AuthRequest, idps []string, userSession *user_model.UserSessionView) (domain.NextStep, error) {
if checkVerificationTimeMaxAge(userSession.ExternalLoginVerification, request.LoginPolicy.ExternalLoginCheckLifetime, request) {
request.IDPLoginChecked = true
request.AuthTime = userSession.ExternalLoginVerification
@ -1307,15 +1322,27 @@ func (repo *AuthRequestRepo) idpChecked(request *domain.AuthRequest, idps []*que
}
// use the explicitly set IdP first
if request.SelectedIDPConfigID != "" {
return &domain.ExternalLoginStep{SelectedIDPConfigID: request.SelectedIDPConfigID}, nil
// only use the explicitly set IdP if allowed
for _, allowedIdP := range request.AllowedExternalIDPs {
if request.SelectedIDPConfigID == allowedIdP.IDPConfigID {
return &domain.ExternalLoginStep{SelectedIDPConfigID: request.SelectedIDPConfigID}, nil
}
}
// error if the explicitly set IdP is not allowed, to avoid misinterpretation with usage of another IdP
return nil, zerrors.ThrowPreconditionFailed(nil, "LOGIN-LWif2", "Errors.Org.IdpNotExisting")
}
// reuse the previously used IdP from the session
if userSession.SelectedIDPConfigID != "" {
return &domain.ExternalLoginStep{SelectedIDPConfigID: userSession.SelectedIDPConfigID}, nil
// only use the previously used IdP if allowed
for _, allowedIdP := range request.AllowedExternalIDPs {
if userSession.SelectedIDPConfigID == allowedIdP.IDPConfigID {
return &domain.ExternalLoginStep{SelectedIDPConfigID: userSession.SelectedIDPConfigID}, nil
}
}
}
// then use an existing linked IdP of the user
// then use an existing linked and allowed IdP of the user
if len(idps) > 0 {
return &domain.ExternalLoginStep{SelectedIDPConfigID: idps[0].IDPID}, nil
return &domain.ExternalLoginStep{SelectedIDPConfigID: idps[0]}, nil
}
// if the user did not link one, then just use one of the configured IdPs of the org
if len(request.AllowedExternalIDPs) > 0 {

View File

@ -1247,6 +1247,7 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) {
args{&domain.AuthRequest{
UserID: "UserID",
SelectedIDPConfigID: "IDPConfigID",
AllowedExternalIDPs: []*domain.IDPProvider{{IDPConfigID: "IDPConfigID"}},
LoginPolicy: &domain.LoginPolicy{
AllowUsernamePassword: false,
SecondFactorCheckLifetime: 18 * time.Hour,
@ -1254,6 +1255,193 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) {
[]domain.NextStep{&domain.ExternalLoginStep{SelectedIDPConfigID: "IDPConfigID"}},
nil,
},
{
"external user (idp selected, not allowed, no external verification), error",
fields{
userSessionViewProvider: &mockViewUserSession{
SecondFactorVerification: testNow.Add(-5 * time.Minute),
},
userViewProvider: &mockViewUser{
IsEmailVerified: true,
MFAMaxSetUp: int32(domain.MFALevelSecondFactor),
},
userEventProvider: &mockEventUser{},
lockoutPolicyProvider: &mockLockoutPolicy{
policy: &query.LockoutPolicy{
ShowFailures: true,
},
},
orgViewProvider: &mockViewOrg{State: domain.OrgStateActive},
loginPolicyProvider: &mockLoginPolicy{
policy: &query.LoginPolicy{
SecondFactorCheckLifetime: database.Duration(18 * time.Hour),
},
},
idpUserLinksProvider: &mockIDPUserLinks{},
},
args{&domain.AuthRequest{
UserID: "UserID",
SelectedIDPConfigID: "IDPConfigID",
AllowedExternalIDPs: []*domain.IDPProvider{},
LoginPolicy: &domain.LoginPolicy{
AllowUsernamePassword: false,
SecondFactorCheckLifetime: 18 * time.Hour,
}}, false},
nil,
zerrors.IsPreconditionFailed,
},
{
"external user (idp link, no external verification), external login step",
fields{
userSessionViewProvider: &mockViewUserSession{
SecondFactorVerification: testNow.Add(-5 * time.Minute),
},
userViewProvider: &mockViewUser{
IsEmailVerified: true,
MFAMaxSetUp: int32(domain.MFALevelSecondFactor),
},
userEventProvider: &mockEventUser{},
lockoutPolicyProvider: &mockLockoutPolicy{
policy: &query.LockoutPolicy{
ShowFailures: true,
},
},
orgViewProvider: &mockViewOrg{State: domain.OrgStateActive},
loginPolicyProvider: &mockLoginPolicy{
policy: &query.LoginPolicy{
SecondFactorCheckLifetime: database.Duration(18 * time.Hour),
},
},
idpUserLinksProvider: &mockIDPUserLinks{
[]*query.IDPUserLink{
{IDPID: "IDPConfigID"},
},
},
},
args{&domain.AuthRequest{
UserID: "UserID",
SelectedIDPConfigID: "",
AllowedExternalIDPs: []*domain.IDPProvider{{IDPConfigID: "IDPConfigID"}},
LoginPolicy: &domain.LoginPolicy{
AllowUsernamePassword: false,
SecondFactorCheckLifetime: 18 * time.Hour,
}}, false},
[]domain.NextStep{&domain.ExternalLoginStep{SelectedIDPConfigID: "IDPConfigID"}},
nil,
},
{
"external user (idp link not allowed, no external verification), external login step",
fields{
userSessionViewProvider: &mockViewUserSession{
SecondFactorVerification: testNow.Add(-5 * time.Minute),
},
userViewProvider: &mockViewUser{
IsEmailVerified: true,
MFAMaxSetUp: int32(domain.MFALevelSecondFactor),
},
userEventProvider: &mockEventUser{},
lockoutPolicyProvider: &mockLockoutPolicy{
policy: &query.LockoutPolicy{
ShowFailures: true,
},
},
orgViewProvider: &mockViewOrg{State: domain.OrgStateActive},
loginPolicyProvider: &mockLoginPolicy{
policy: &query.LoginPolicy{
SecondFactorCheckLifetime: database.Duration(18 * time.Hour),
},
},
idpUserLinksProvider: &mockIDPUserLinks{
[]*query.IDPUserLink{
{IDPID: "IDPConfigID1"},
},
},
},
args{&domain.AuthRequest{
UserID: "UserID",
SelectedIDPConfigID: "",
AllowedExternalIDPs: []*domain.IDPProvider{{IDPConfigID: "IDPConfigID2"}},
LoginPolicy: &domain.LoginPolicy{
AllowUsernamePassword: false,
SecondFactorCheckLifetime: 18 * time.Hour,
}}, false},
[]domain.NextStep{&domain.ExternalLoginStep{SelectedIDPConfigID: "IDPConfigID2"}},
nil,
},
{
"external user (idp link not allowed, none allowed, no external verification), external login step",
fields{
userSessionViewProvider: &mockViewUserSession{
SecondFactorVerification: testNow.Add(-5 * time.Minute),
},
userViewProvider: &mockViewUser{
IsEmailVerified: true,
MFAMaxSetUp: int32(domain.MFALevelSecondFactor),
},
userEventProvider: &mockEventUser{},
lockoutPolicyProvider: &mockLockoutPolicy{
policy: &query.LockoutPolicy{
ShowFailures: true,
},
},
orgViewProvider: &mockViewOrg{State: domain.OrgStateActive},
loginPolicyProvider: &mockLoginPolicy{
policy: &query.LoginPolicy{
SecondFactorCheckLifetime: database.Duration(18 * time.Hour),
},
},
idpUserLinksProvider: &mockIDPUserLinks{
[]*query.IDPUserLink{
{IDPID: "IDPConfigID1"},
},
},
},
args{&domain.AuthRequest{
UserID: "UserID",
SelectedIDPConfigID: "",
AllowedExternalIDPs: []*domain.IDPProvider{},
LoginPolicy: &domain.LoginPolicy{
AllowUsernamePassword: false,
SecondFactorCheckLifetime: 18 * time.Hour,
}}, false},
nil,
zerrors.IsPreconditionFailed,
},
{
"external user (no idp allowed, no external verification), error",
fields{
userSessionViewProvider: &mockViewUserSession{
SecondFactorVerification: testNow.Add(-5 * time.Minute),
},
userViewProvider: &mockViewUser{
IsEmailVerified: true,
MFAMaxSetUp: int32(domain.MFALevelSecondFactor),
},
userEventProvider: &mockEventUser{},
lockoutPolicyProvider: &mockLockoutPolicy{
policy: &query.LockoutPolicy{
ShowFailures: true,
},
},
orgViewProvider: &mockViewOrg{State: domain.OrgStateActive},
loginPolicyProvider: &mockLoginPolicy{
policy: &query.LoginPolicy{
SecondFactorCheckLifetime: database.Duration(18 * time.Hour),
},
},
idpUserLinksProvider: &mockIDPUserLinks{},
},
args{&domain.AuthRequest{
UserID: "UserID",
SelectedIDPConfigID: "",
AllowedExternalIDPs: []*domain.IDPProvider{},
LoginPolicy: &domain.LoginPolicy{
AllowUsernamePassword: false,
SecondFactorCheckLifetime: 18 * time.Hour,
}}, false},
nil,
zerrors.IsPreconditionFailed,
},
{
"external user (only idp available, no external verification), external login step",
fields{
@ -1281,13 +1469,49 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) {
},
},
args{&domain.AuthRequest{
UserID: "UserID",
UserID: "UserID",
AllowedExternalIDPs: []*domain.IDPProvider{{IDPConfigID: "IDPConfigID"}},
LoginPolicy: &domain.LoginPolicy{
AllowUsernamePassword: false,
SecondFactorCheckLifetime: 18 * time.Hour,
}}, false},
[]domain.NextStep{&domain.ExternalLoginStep{SelectedIDPConfigID: "IDPConfigID"}},
nil,
}, {
"external user (only idp available, no allowed, no external verification), external login step",
fields{
userSessionViewProvider: &mockViewUserSession{
SecondFactorVerification: testNow.Add(-5 * time.Minute),
},
userViewProvider: &mockViewUser{
IsEmailVerified: true,
MFAMaxSetUp: int32(domain.MFALevelSecondFactor),
},
userEventProvider: &mockEventUser{},
lockoutPolicyProvider: &mockLockoutPolicy{
policy: &query.LockoutPolicy{
ShowFailures: true,
},
},
orgViewProvider: &mockViewOrg{State: domain.OrgStateActive},
loginPolicyProvider: &mockLoginPolicy{
policy: &query.LoginPolicy{
SecondFactorCheckLifetime: database.Duration(18 * time.Hour),
},
},
idpUserLinksProvider: &mockIDPUserLinks{
idps: []*query.IDPUserLink{{IDPID: "IDPConfigID"}},
},
},
args{&domain.AuthRequest{
UserID: "UserID",
AllowedExternalIDPs: []*domain.IDPProvider{},
LoginPolicy: &domain.LoginPolicy{
AllowUsernamePassword: false,
SecondFactorCheckLifetime: 18 * time.Hour,
}}, false},
nil,
zerrors.IsPreconditionFailed,
},
{
"external user (external verification set), callback",