From af1f10b7cac7f438b161cbc508e19fe69c4f2149 Mon Sep 17 00:00:00 2001 From: Livio Amstutz Date: Mon, 8 Nov 2021 08:42:07 +0100 Subject: [PATCH] fix: check login policy before register and password check (#2611) * fix: check login policy before register and password check * remove accidentally pushed overwrite * Update en.yaml --- internal/command/org_policy_login.go | 11 ++ internal/command/user_human.go | 7 + internal/command/user_human_password.go | 9 + internal/command/user_human_password_test.go | 114 +++++++++++ internal/command/user_human_test.go | 194 +++++++++++++++++++ internal/static/i18n/de.yaml | 2 + internal/static/i18n/en.yaml | 2 + 7 files changed, 339 insertions(+) diff --git a/internal/command/org_policy_login.go b/internal/command/org_policy_login.go index 49538aedd..d3a293d58 100644 --- a/internal/command/org_policy_login.go +++ b/internal/command/org_policy_login.go @@ -62,6 +62,17 @@ func (c *Commands) orgLoginPolicyWriteModelByID(ctx context.Context, orgID strin return policyWriteModel, nil } +func (c *Commands) getOrgLoginPolicy(ctx context.Context, orgID string) (*domain.LoginPolicy, error) { + policy, err := c.orgLoginPolicyWriteModelByID(ctx, orgID) + if err != nil { + return nil, err + } + if policy.State == domain.PolicyStateActive { + return writeModelToLoginPolicy(&policy.LoginPolicyWriteModel), nil + } + return c.getDefaultLoginPolicy(ctx) +} + func (c *Commands) ChangeLoginPolicy(ctx context.Context, resourceOwner string, policy *domain.LoginPolicy) (*domain.LoginPolicy, error) { if resourceOwner == "" { return nil, caos_errs.ThrowInvalidArgument(nil, "Org-Mf9sf", "Errors.ResourceOwnerMissing") diff --git a/internal/command/user_human.go b/internal/command/user_human.go index c07701ef6..13d8ed510 100644 --- a/internal/command/user_human.go +++ b/internal/command/user_human.go @@ -129,6 +129,13 @@ func (c *Commands) RegisterHuman(ctx context.Context, orgID string, human *domai if err != nil { return nil, caos_errs.ThrowPreconditionFailed(err, "COMMAND-M5Fsd", "Errors.Org.PasswordComplexity.NotFound") } + loginPolicy, err := c.getOrgLoginPolicy(ctx, orgID) + if err != nil { + return nil, caos_errs.ThrowPreconditionFailed(err, "COMMAND-Dfg3g", "Errors.Org.LoginPolicy.NotFound") + } + if !loginPolicy.AllowRegister { + return nil, caos_errs.ThrowPreconditionFailed(err, "COMMAND-SAbr3", "Errors.Org.LoginPolicy.RegistrationNotAllowed") + } userEvents, registeredHuman, err := c.registerHuman(ctx, orgID, human, link, orgIAMPolicy, pwPolicy) if err != nil { return nil, err diff --git a/internal/command/user_human_password.go b/internal/command/user_human_password.go index 7d0ff8c8e..0ef71a100 100644 --- a/internal/command/user_human_password.go +++ b/internal/command/user_human_password.go @@ -4,6 +4,7 @@ import ( "context" "github.com/caos/logging" + "github.com/caos/zitadel/internal/crypto" "github.com/caos/zitadel/internal/domain" caos_errs "github.com/caos/zitadel/internal/errors" @@ -206,6 +207,14 @@ func (c *Commands) HumanCheckPassword(ctx context.Context, orgID, userID, passwo return caos_errs.ThrowInvalidArgument(nil, "COMMAND-3n8fs", "Errors.User.Password.Empty") } + loginPolicy, err := c.getOrgLoginPolicy(ctx, orgID) + if err != nil { + return caos_errs.ThrowPreconditionFailed(err, "COMMAND-Edf3g", "Errors.Org.LoginPolicy.NotFound") + } + if !loginPolicy.AllowUsernamePassword { + return caos_errs.ThrowPreconditionFailed(err, "COMMAND-Dft32", "Errors.Org.LoginPolicy.UsernamePasswordNotAllowed") + } + existingPassword, err := c.passwordWriteModel(ctx, userID, orgID) if err != nil { return err diff --git a/internal/command/user_human_password_test.go b/internal/command/user_human_password_test.go index c43dcf529..a33ee1a2c 100644 --- a/internal/command/user_human_password_test.go +++ b/internal/command/user_human_password_test.go @@ -1125,11 +1125,73 @@ func TestCommandSide_CheckPassword(t *testing.T) { err: caos_errs.IsErrorInvalidArgument, }, }, + { + name: "login policy not found, precondition error", + fields: fields{ + eventstore: eventstoreExpect( + t, + expectFilter(), + expectFilter(), + ), + }, + args: args{ + ctx: context.Background(), + userID: "user1", + resourceOwner: "org1", + password: "password", + }, + res: res{ + err: caos_errs.IsPreconditionFailed, + }, + }, + { + name: "login policy login password not allowed, precondition error", + fields: fields{ + eventstore: eventstoreExpect( + t, + expectFilter( + eventFromEventPusher( + org.NewLoginPolicyAddedEvent(context.Background(), + &org.NewAggregate("org1", "org1").Aggregate, + false, + false, + false, + false, + false, + domain.PasswordlessTypeNotAllowed, + ), + ), + ), + ), + }, + args: args{ + ctx: context.Background(), + userID: "user1", + resourceOwner: "org1", + password: "password", + }, + res: res{ + err: caos_errs.IsPreconditionFailed, + }, + }, { name: "user not existing, precondition error", fields: fields{ eventstore: eventstoreExpect( t, + expectFilter( + eventFromEventPusher( + org.NewLoginPolicyAddedEvent(context.Background(), + &org.NewAggregate("org1", "org1").Aggregate, + true, + false, + false, + false, + false, + domain.PasswordlessTypeNotAllowed, + ), + ), + ), expectFilter(), ), }, @@ -1148,6 +1210,19 @@ func TestCommandSide_CheckPassword(t *testing.T) { fields: fields{ eventstore: eventstoreExpect( t, + expectFilter( + eventFromEventPusher( + org.NewLoginPolicyAddedEvent(context.Background(), + &org.NewAggregate("org1", "org1").Aggregate, + true, + false, + false, + false, + false, + domain.PasswordlessTypeNotAllowed, + ), + ), + ), expectFilter( eventFromEventPusher( user.NewHumanAddedEvent(context.Background(), @@ -1182,6 +1257,19 @@ func TestCommandSide_CheckPassword(t *testing.T) { fields: fields{ eventstore: eventstoreExpect( t, + expectFilter( + eventFromEventPusher( + org.NewLoginPolicyAddedEvent(context.Background(), + &org.NewAggregate("org1", "org1").Aggregate, + true, + false, + false, + false, + false, + domain.PasswordlessTypeNotAllowed, + ), + ), + ), expectFilter( eventFromEventPusher( user.NewHumanAddedEvent(context.Background(), @@ -1250,6 +1338,19 @@ func TestCommandSide_CheckPassword(t *testing.T) { fields: fields{ eventstore: eventstoreExpect( t, + expectFilter( + eventFromEventPusher( + org.NewLoginPolicyAddedEvent(context.Background(), + &org.NewAggregate("org1", "org1").Aggregate, + true, + false, + false, + false, + false, + domain.PasswordlessTypeNotAllowed, + ), + ), + ), expectFilter( eventFromEventPusher( user.NewHumanAddedEvent(context.Background(), @@ -1325,6 +1426,19 @@ func TestCommandSide_CheckPassword(t *testing.T) { fields: fields{ eventstore: eventstoreExpect( t, + expectFilter( + eventFromEventPusher( + org.NewLoginPolicyAddedEvent(context.Background(), + &org.NewAggregate("org1", "org1").Aggregate, + true, + false, + false, + false, + false, + domain.PasswordlessTypeNotAllowed, + ), + ), + ), expectFilter( eventFromEventPusher( user.NewHumanAddedEvent(context.Background(), diff --git a/internal/command/user_human_test.go b/internal/command/user_human_test.go index cf8cf279e..f17df3ec2 100644 --- a/internal/command/user_human_test.go +++ b/internal/command/user_human_test.go @@ -1531,6 +1531,109 @@ func TestCommandSide_RegisterHuman(t *testing.T) { err: caos_errs.IsPreconditionFailed, }, }, + { + name: "login policy not found, precondition error", + fields: fields{ + eventstore: eventstoreExpect( + t, + expectFilter( + eventFromEventPusher( + org.NewOrgIAMPolicyAddedEvent(context.Background(), + &org.NewAggregate("org1", "org1").Aggregate, + true, + ), + ), + ), + expectFilter( + eventFromEventPusher( + org.NewPasswordComplexityPolicyAddedEvent(context.Background(), + &org.NewAggregate("org1", "org1").Aggregate, + 1, + false, + false, + false, + false, + ), + ), + ), + expectFilter(), + expectFilter(), + ), + }, + args: args{ + ctx: context.Background(), + orgID: "org1", + human: &domain.Human{ + Username: "username", + Profile: &domain.Profile{ + FirstName: "firstname", + }, + Password: &domain.Password{ + SecretString: "password", + }, + }, + }, + res: res{ + err: caos_errs.IsPreconditionFailed, + }, + }, + { + name: "login policy registration not allowed, precondition error", + fields: fields{ + eventstore: eventstoreExpect( + t, + expectFilter( + eventFromEventPusher( + org.NewOrgIAMPolicyAddedEvent(context.Background(), + &org.NewAggregate("org1", "org1").Aggregate, + true, + ), + ), + ), + expectFilter( + eventFromEventPusher( + org.NewPasswordComplexityPolicyAddedEvent(context.Background(), + &org.NewAggregate("org1", "org1").Aggregate, + 1, + false, + false, + false, + false, + ), + ), + ), + expectFilter( + eventFromEventPusher( + org.NewLoginPolicyAddedEvent(context.Background(), + &org.NewAggregate("org1", "org1").Aggregate, + false, + false, + false, + false, + false, + domain.PasswordlessTypeNotAllowed, + ), + ), + ), + ), + }, + args: args{ + ctx: context.Background(), + orgID: "org1", + human: &domain.Human{ + Username: "username", + Profile: &domain.Profile{ + FirstName: "firstname", + }, + Password: &domain.Password{ + SecretString: "password", + }, + }, + }, + res: res{ + err: caos_errs.IsPreconditionFailed, + }, + }, { name: "user invalid, invalid argument error", fields: fields{ @@ -1556,6 +1659,19 @@ func TestCommandSide_RegisterHuman(t *testing.T) { ), ), ), + expectFilter( + eventFromEventPusher( + org.NewLoginPolicyAddedEvent(context.Background(), + &org.NewAggregate("org1", "org1").Aggregate, + false, + true, + false, + false, + false, + domain.PasswordlessTypeNotAllowed, + ), + ), + ), ), }, args: args{ @@ -1600,6 +1716,19 @@ func TestCommandSide_RegisterHuman(t *testing.T) { ), ), ), + expectFilter( + eventFromEventPusher( + org.NewLoginPolicyAddedEvent(context.Background(), + &org.NewAggregate("org1", "org1").Aggregate, + false, + true, + false, + false, + false, + domain.PasswordlessTypeNotAllowed, + ), + ), + ), expectFilter( eventFromEventPusher( org.NewDomainAddedEvent(context.Background(), @@ -1661,6 +1790,19 @@ func TestCommandSide_RegisterHuman(t *testing.T) { ), ), ), + expectFilter( + eventFromEventPusher( + org.NewLoginPolicyAddedEvent(context.Background(), + &org.NewAggregate("org1", "org1").Aggregate, + false, + true, + false, + false, + false, + domain.PasswordlessTypeNotAllowed, + ), + ), + ), expectFilter( eventFromEventPusher( org.NewDomainAddedEvent(context.Background(), @@ -1780,6 +1922,19 @@ func TestCommandSide_RegisterHuman(t *testing.T) { ), ), ), + expectFilter( + eventFromEventPusher( + org.NewLoginPolicyAddedEvent(context.Background(), + &org.NewAggregate("org1", "org1").Aggregate, + false, + true, + false, + false, + false, + domain.PasswordlessTypeNotAllowed, + ), + ), + ), expectPush( []*repository.Event{ eventFromEventPusher( @@ -1867,6 +2022,19 @@ func TestCommandSide_RegisterHuman(t *testing.T) { ), ), ), + expectFilter( + eventFromEventPusher( + org.NewLoginPolicyAddedEvent(context.Background(), + &org.NewAggregate("org1", "org1").Aggregate, + false, + true, + false, + false, + false, + domain.PasswordlessTypeNotAllowed, + ), + ), + ), expectPush( []*repository.Event{ eventFromEventPusher( @@ -1948,6 +2116,19 @@ func TestCommandSide_RegisterHuman(t *testing.T) { ), ), ), + expectFilter( + eventFromEventPusher( + org.NewLoginPolicyAddedEvent(context.Background(), + &org.NewAggregate("org1", "org1").Aggregate, + false, + true, + false, + false, + false, + domain.PasswordlessTypeNotAllowed, + ), + ), + ), expectPush( []*repository.Event{ eventFromEventPusher( @@ -2051,6 +2232,19 @@ func TestCommandSide_RegisterHuman(t *testing.T) { ), ), ), + expectFilter( + eventFromEventPusher( + org.NewLoginPolicyAddedEvent(context.Background(), + &org.NewAggregate("org1", "org1").Aggregate, + false, + true, + false, + false, + false, + domain.PasswordlessTypeNotAllowed, + ), + ), + ), expectPush( []*repository.Event{ eventFromEventPusher( diff --git a/internal/static/i18n/de.yaml b/internal/static/i18n/de.yaml index e7144e994..ac81c3c85 100644 --- a/internal/static/i18n/de.yaml +++ b/internal/static/i18n/de.yaml @@ -154,6 +154,8 @@ Errors: AlreadyExists: Login Policy existiert bereits IdpProviderAlreadyExisting: Idp Provider existiert bereits IdpProviderNotExisting: Idp Provider existiert nicht + RegistrationNotAllowed: Registrierung ist nicht erlaubt + UsernamePasswordNotAllowed: Login mit Username / Passwort nicht erlaubt MFA: AlreadyExists: Multifaktor existiert bereits NotExisting: Multifaktor existiert nicht diff --git a/internal/static/i18n/en.yaml b/internal/static/i18n/en.yaml index 30b4b69bd..831415696 100644 --- a/internal/static/i18n/en.yaml +++ b/internal/static/i18n/en.yaml @@ -154,6 +154,8 @@ Errors: AlreadyExists: Login Policy already exists IdpProviderAlreadyExisting: Idp Provider already existing IdpProviderNotExisting: Idp Provider not existing + RegistrationNotAllowed: Registration is not allowed + UsernamePasswordNotAllowed: Login with Username / Password is not allowed MFA: AlreadyExists: Multifactor already exists NotExisting: Multifactor not existing