From b0681a0bbe229280ee0362f2268ee75bbe9ab89c Mon Sep 17 00:00:00 2001 From: Livio Amstutz Date: Thu, 15 Apr 2021 15:30:19 +0200 Subject: [PATCH] fix: ensure event order in setDefaultAuthFactorsInCustomLoginPolicy (for testability) (#1595) * fix: ensure event order (for testability) * fix: error handling (incl. imports of wrong pkgs) --- .../repository/eventsourcing/handler/user.go | 17 ++++++++--------- .../repository/eventsourcing/handler/token.go | 5 ++--- internal/command/org_features.go | 11 +++++++---- internal/command/org_features_test.go | 4 ++-- internal/domain/factors.go | 16 ++++++++++++++++ .../eventsourcing/handler/project_member.go | 2 +- 6 files changed, 36 insertions(+), 19 deletions(-) diff --git a/internal/admin/repository/eventsourcing/handler/user.go b/internal/admin/repository/eventsourcing/handler/user.go index b4b445448b..d0731e9545 100644 --- a/internal/admin/repository/eventsourcing/handler/user.go +++ b/internal/admin/repository/eventsourcing/handler/user.go @@ -2,24 +2,23 @@ package handler import ( "context" + + "github.com/caos/logging" + "github.com/caos/zitadel/internal/config/systemdefaults" "github.com/caos/zitadel/internal/domain" caos_errs "github.com/caos/zitadel/internal/errors" "github.com/caos/zitadel/internal/eventstore/v1" + es_models "github.com/caos/zitadel/internal/eventstore/v1/models" + "github.com/caos/zitadel/internal/eventstore/v1/query" es_sdk "github.com/caos/zitadel/internal/eventstore/v1/sdk" + "github.com/caos/zitadel/internal/eventstore/v1/spooler" iam_model "github.com/caos/zitadel/internal/iam/model" "github.com/caos/zitadel/internal/iam/repository/eventsourcing/model" iam_view "github.com/caos/zitadel/internal/iam/repository/view" - "github.com/caos/zitadel/internal/org/repository/view" - "k8s.io/apimachinery/pkg/api/errors" - - "github.com/caos/logging" - - es_models "github.com/caos/zitadel/internal/eventstore/v1/models" - "github.com/caos/zitadel/internal/eventstore/v1/query" - "github.com/caos/zitadel/internal/eventstore/v1/spooler" org_model "github.com/caos/zitadel/internal/org/model" org_es_model "github.com/caos/zitadel/internal/org/repository/eventsourcing/model" + "github.com/caos/zitadel/internal/org/repository/view" es_model "github.com/caos/zitadel/internal/user/repository/eventsourcing/model" view_model "github.com/caos/zitadel/internal/user/repository/view/model" ) @@ -266,7 +265,7 @@ func (u *User) getOrgByID(ctx context.Context, orgID string) (*org_model.Org, er }, } err = es_sdk.Filter(ctx, u.Eventstore().FilterEvents, esOrg.AppendEvents, query) - if err != nil && !errors.IsNotFound(err) { + if err != nil && !caos_errs.IsNotFound(err) { return nil, err } if esOrg.Sequence == 0 { diff --git a/internal/auth/repository/eventsourcing/handler/token.go b/internal/auth/repository/eventsourcing/handler/token.go index 50de1ebbe6..c1b05e0faa 100644 --- a/internal/auth/repository/eventsourcing/handler/token.go +++ b/internal/auth/repository/eventsourcing/handler/token.go @@ -3,12 +3,11 @@ package handler import ( "context" "encoding/json" - "github.com/caos/zitadel/internal/eventstore/v1" "github.com/caos/logging" - "k8s.io/apimachinery/pkg/api/errors" caos_errs "github.com/caos/zitadel/internal/errors" + "github.com/caos/zitadel/internal/eventstore/v1" es_models "github.com/caos/zitadel/internal/eventstore/v1/models" "github.com/caos/zitadel/internal/eventstore/v1/query" es_sdk "github.com/caos/zitadel/internal/eventstore/v1/sdk" @@ -169,7 +168,7 @@ func (t *Token) getProjectByID(ctx context.Context, projID string) (*proj_model. }, } err = es_sdk.Filter(ctx, t.Eventstore().FilterEvents, esProject.AppendEvents, query) - if err != nil && !errors.IsNotFound(err) { + if err != nil && !caos_errs.IsNotFound(err) { return nil, err } if esProject.Sequence == 0 { diff --git a/internal/command/org_features.go b/internal/command/org_features.go index 7aef8488df..d30682943e 100644 --- a/internal/command/org_features.go +++ b/internal/command/org_features.go @@ -168,8 +168,9 @@ func (c *Commands) setDefaultAuthFactorsInCustomLoginPolicy(ctx context.Context, return nil, err } events := make([]eventstore.EventPusher, 0) - for factor, state := range orgAuthFactors.SecondFactors { - if state.IAM == state.Org { + for _, factor := range domain.SecondFactorTypes() { + state := orgAuthFactors.SecondFactors[factor] + if state == nil || state.IAM == state.Org { continue } secondFactorWriteModel := orgAuthFactors.ToSecondFactorWriteModel(factor) @@ -191,8 +192,10 @@ func (c *Commands) setDefaultAuthFactorsInCustomLoginPolicy(ctx context.Context, events = append(events, event) } } - for factor, state := range orgAuthFactors.MultiFactors { - if state.IAM == state.Org { + + for _, factor := range domain.MultiFactorTypes() { + state := orgAuthFactors.MultiFactors[factor] + if state == nil || state.IAM == state.Org { continue } multiFactorWriteModel := orgAuthFactors.ToMultiFactorWriteModel(factor) diff --git a/internal/command/org_features_test.go b/internal/command/org_features_test.go index 9f37bf4099..7536df86ab 100644 --- a/internal/command/org_features_test.go +++ b/internal/command/org_features_test.go @@ -284,10 +284,10 @@ func TestCommandSide_SetOrgFeatures(t *testing.T) { expectPush( []*repository.Event{ eventFromEventPusher( - org.NewLoginPolicySecondFactorAddedEvent(context.Background(), &org.NewAggregate("org1", "org1").Aggregate, domain.SecondFactorTypeU2F), + org.NewLoginPolicySecondFactorRemovedEvent(context.Background(), &org.NewAggregate("org1", "org1").Aggregate, domain.SecondFactorTypeOTP), ), eventFromEventPusher( - org.NewLoginPolicySecondFactorRemovedEvent(context.Background(), &org.NewAggregate("org1", "org1").Aggregate, domain.SecondFactorTypeOTP), + org.NewLoginPolicySecondFactorAddedEvent(context.Background(), &org.NewAggregate("org1", "org1").Aggregate, domain.SecondFactorTypeU2F), ), eventFromEventPusher( org.NewLoginPolicyMultiFactorAddedEvent(context.Background(), &org.NewAggregate("org1", "org1").Aggregate, domain.MultiFactorTypeU2FWithPIN), diff --git a/internal/domain/factors.go b/internal/domain/factors.go index 1f8f0ec41d..172b20c665 100644 --- a/internal/domain/factors.go +++ b/internal/domain/factors.go @@ -10,6 +10,14 @@ const ( secondFactorCount ) +func SecondFactorTypes() []SecondFactorType { + types := make([]SecondFactorType, 0, secondFactorCount-1) + for i := SecondFactorTypeUnspecified + 1; i < secondFactorCount; i++ { + types = append(types, i) + } + return types +} + type MultiFactorType int32 const ( @@ -19,6 +27,14 @@ const ( multiFactorCount ) +func MultiFactorTypes() []MultiFactorType { + types := make([]MultiFactorType, 0, multiFactorCount-1) + for i := MultiFactorTypeUnspecified + 1; i < multiFactorCount; i++ { + types = append(types, i) + } + return types +} + type FactorState int32 const ( diff --git a/internal/management/repository/eventsourcing/handler/project_member.go b/internal/management/repository/eventsourcing/handler/project_member.go index 9b7e6b8f15..c9592bb0ac 100644 --- a/internal/management/repository/eventsourcing/handler/project_member.go +++ b/internal/management/repository/eventsourcing/handler/project_member.go @@ -214,7 +214,7 @@ func (u *ProjectMember) getUserByID(userID string) (*usr_view_model.UserView, er return user, nil } } - if userCopy.State == int32(usr_model.UserStateDeleted) { + if userCopy.State == int32(usr_model.UserStateUnspecified) || userCopy.State == int32(usr_model.UserStateDeleted) { return nil, caos_errs.ThrowNotFound(nil, "HANDLER-m9dos", "Errors.User.NotFound") } return &userCopy, nil