From 34002ec83460c026a8bef7631912d10d1ed38ca1 Mon Sep 17 00:00:00 2001 From: Livio Amstutz Date: Wed, 25 Aug 2021 11:12:24 +0200 Subject: [PATCH] fix: check domain of username not claimed by other organisation and cleanup (#2265) * fix: register human * fix: check domain of username not claimed by other organisation * fix: create setup step to create domain claimed events for invalid users * Update setup_step19.go --- internal/command/org_domain_model.go | 54 ++++++ internal/command/setup_step19.go | 212 ++++++++++++++++++++++ internal/command/user_human.go | 18 +- internal/command/user_human_test.go | 260 +++++++++++++++++++++++---- internal/domain/step.go | 1 + internal/setup/config.go | 2 + 6 files changed, 515 insertions(+), 32 deletions(-) create mode 100644 internal/command/setup_step19.go diff --git a/internal/command/org_domain_model.go b/internal/command/org_domain_model.go index 7772f5af7e..50f9395b82 100644 --- a/internal/command/org_domain_model.go +++ b/internal/command/org_domain_model.go @@ -170,3 +170,57 @@ func (wm *OrgDomainsWriteModel) Query() *eventstore.SearchQueryBuilder { org.OrgDomainRemovedEventType). Builder() } + +type OrgDomainVerifiedWriteModel struct { + eventstore.WriteModel + + Domain string + Verified bool +} + +func NewOrgDomainVerifiedWriteModel(domain string) *OrgDomainVerifiedWriteModel { + return &OrgDomainVerifiedWriteModel{ + WriteModel: eventstore.WriteModel{}, + Domain: domain, + } +} + +func (wm *OrgDomainVerifiedWriteModel) AppendEvents(events ...eventstore.EventReader) { + for _, event := range events { + switch e := event.(type) { + case *org.DomainVerifiedEvent: + if e.Domain != wm.Domain { + continue + } + wm.WriteModel.AppendEvents(e) + case *org.DomainRemovedEvent: + if e.Domain != wm.Domain { + continue + } + wm.WriteModel.AppendEvents(e) + } + } +} + +func (wm *OrgDomainVerifiedWriteModel) Reduce() error { + for _, event := range wm.Events { + switch e := event.(type) { + case *org.DomainVerifiedEvent: + wm.Verified = true + wm.ResourceOwner = e.Aggregate().ResourceOwner + case *org.DomainRemovedEvent: + wm.Verified = false + } + } + return nil +} + +func (wm *OrgDomainVerifiedWriteModel) Query() *eventstore.SearchQueryBuilder { + return eventstore.NewSearchQueryBuilder(eventstore.ColumnsEvent). + AddQuery(). + AggregateTypes(org.AggregateType). + EventTypes( + org.OrgDomainVerifiedEventType, + org.OrgDomainRemovedEventType). + Builder() +} diff --git a/internal/command/setup_step19.go b/internal/command/setup_step19.go new file mode 100644 index 0000000000..7a1e9e6464 --- /dev/null +++ b/internal/command/setup_step19.go @@ -0,0 +1,212 @@ +package command + +import ( + "context" + "fmt" + "strings" + + "github.com/caos/logging" + "github.com/caos/zitadel/internal/domain" + "github.com/caos/zitadel/internal/eventstore" + "github.com/caos/zitadel/internal/repository/org" + "github.com/caos/zitadel/internal/repository/user" +) + +type Step19 struct{} + +func (s *Step19) Step() domain.Step { + return domain.Step19 +} + +func (s *Step19) execute(ctx context.Context, commandSide *Commands) error { + return commandSide.SetupStep19(ctx, s) +} + +func (c *Commands) SetupStep19(ctx context.Context, step *Step19) error { + fn := func(iam *IAMWriteModel) ([]eventstore.EventPusher, error) { + events := make([]eventstore.EventPusher, 0) + orgs := newOrgsWithUsernameNotDomain() + if err := c.eventstore.FilterToQueryReducer(ctx, orgs); err != nil { + return nil, err + } + for orgID, usernameCheck := range orgs.orgs { + if !usernameCheck { + continue + } + users := newDomainClaimedUsernames(orgID) + if err := c.eventstore.FilterToQueryReducer(ctx, users); err != nil { + return nil, err + } + for userID, username := range users.users { + split := strings.Split(username, "@") + if len(split) != 2 { + continue + } + domainVerified := NewOrgDomainVerifiedWriteModel(split[1]) + if err := c.eventstore.FilterToQueryReducer(ctx, domainVerified); err != nil { + return nil, err + } + if domainVerified.Verified && domainVerified.ResourceOwner != orgID { + id, err := c.idGenerator.Next() + if err != nil { + return nil, err + } + events = append(events, user.NewDomainClaimedEvent( + ctx, + &user.NewAggregate(userID, orgID).Aggregate, + fmt.Sprintf("%s@temporary.%s", id, c.iamDomain), + username, + false)) + } + } + } + + if length := len(events); length > 0 { + logging.Log("SETUP-dFG2t").WithField("count", length).Info("domain claimed events created") + } + return events, nil + } + return c.setup(ctx, step, fn) +} + +func newOrgsWithUsernameNotDomain() *orgsWithUsernameNotDomain { + return &orgsWithUsernameNotDomain{ + orgEvents: make(map[string][]eventstore.EventReader), + orgs: make(map[string]bool), + } +} + +type orgsWithUsernameNotDomain struct { + eventstore.WriteModel + + orgEvents map[string][]eventstore.EventReader + orgs map[string]bool +} + +func (wm *orgsWithUsernameNotDomain) AppendEvents(events ...eventstore.EventReader) { + for _, event := range events { + switch e := event.(type) { + case *org.OrgAddedEvent: + wm.orgEvents[e.Aggregate().ID] = append(wm.orgEvents[e.Aggregate().ID], e) + case *org.OrgRemovedEvent: + delete(wm.orgEvents, e.Aggregate().ID) + case *org.OrgIAMPolicyAddedEvent: + wm.orgEvents[e.Aggregate().ID] = append(wm.orgEvents[e.Aggregate().ID], e) + case *org.OrgIAMPolicyChangedEvent: + if e.UserLoginMustBeDomain == nil { + continue + } + wm.orgEvents[e.Aggregate().ID] = append(wm.orgEvents[e.Aggregate().ID], e) + case *org.OrgIAMPolicyRemovedEvent: + delete(wm.orgEvents, e.Aggregate().ID) + } + } +} + +func (wm *orgsWithUsernameNotDomain) Reduce() error { + for _, events := range wm.orgEvents { + for _, event := range events { + switch e := event.(type) { + case *org.OrgIAMPolicyAddedEvent: + if !e.UserLoginMustBeDomain { + wm.orgs[e.Aggregate().ID] = true + } + case *org.OrgIAMPolicyChangedEvent: + if !*e.UserLoginMustBeDomain { + wm.orgs[e.Aggregate().ID] = true + } + delete(wm.orgs, e.Aggregate().ID) + } + } + } + return nil +} + +func (wm *orgsWithUsernameNotDomain) Query() *eventstore.SearchQueryBuilder { + return eventstore.NewSearchQueryBuilder(eventstore.ColumnsEvent). + AddQuery(). + AggregateTypes(org.AggregateType). + EventTypes( + org.OrgAddedEventType, + org.OrgRemovedEventType, + org.OrgIAMPolicyAddedEventType, + org.OrgIAMPolicyChangedEventType, + org.OrgIAMPolicyRemovedEventType). + Builder() +} + +func newDomainClaimedUsernames(orgID string) *domainClaimedUsernames { + return &domainClaimedUsernames{ + WriteModel: eventstore.WriteModel{ + ResourceOwner: orgID, + }, + userEvents: make(map[string][]eventstore.EventReader), + users: make(map[string]string), + } +} + +type domainClaimedUsernames struct { + eventstore.WriteModel + + userEvents map[string][]eventstore.EventReader + users map[string]string +} + +func (wm *domainClaimedUsernames) AppendEvents(events ...eventstore.EventReader) { + for _, event := range events { + switch e := event.(type) { + case *user.HumanAddedEvent: + if !strings.Contains(e.UserName, "@") { + continue + } + wm.userEvents[e.Aggregate().ID] = append(wm.userEvents[e.Aggregate().ID], e) + case *user.HumanRegisteredEvent: + if !strings.Contains(e.UserName, "@") { + continue + } + wm.userEvents[e.Aggregate().ID] = append(wm.userEvents[e.Aggregate().ID], e) + case *user.UsernameChangedEvent: + if !strings.Contains(e.UserName, "@") { + delete(wm.userEvents, e.Aggregate().ID) + continue + } + wm.userEvents[e.Aggregate().ID] = append(wm.userEvents[e.Aggregate().ID], e) + case *user.DomainClaimedEvent: + delete(wm.userEvents, e.Aggregate().ID) + case *user.UserRemovedEvent: + delete(wm.userEvents, e.Aggregate().ID) + } + } +} + +func (wm *domainClaimedUsernames) Reduce() error { + for _, events := range wm.userEvents { + for _, event := range events { + switch e := event.(type) { + case *user.HumanAddedEvent: + wm.users[e.Aggregate().ID] = e.UserName + case *user.HumanRegisteredEvent: + wm.users[e.Aggregate().ID] = e.UserName + case *user.UsernameChangedEvent: + wm.users[e.Aggregate().ID] = e.UserName + } + } + } + return nil +} + +func (wm *domainClaimedUsernames) Query() *eventstore.SearchQueryBuilder { + return eventstore.NewSearchQueryBuilder(eventstore.ColumnsEvent). + ResourceOwner(wm.ResourceOwner). + AddQuery(). + AggregateTypes(user.AggregateType). + EventTypes( + user.UserV1AddedType, + user.UserV1RegisteredType, + user.HumanAddedType, + user.HumanRegisteredType, + user.UserUserNameChangedType, + user.UserDomainClaimedType, + user.UserRemovedType). + Builder() +} diff --git a/internal/command/user_human.go b/internal/command/user_human.go index 16172f6674..06381be215 100644 --- a/internal/command/user_human.go +++ b/internal/command/user_human.go @@ -2,6 +2,7 @@ package command import ( "context" + "strings" "github.com/caos/zitadel/internal/eventstore" @@ -117,8 +118,8 @@ func (c *Commands) importHuman(ctx context.Context, orgID string, human *domain. } func (c *Commands) RegisterHuman(ctx context.Context, orgID string, human *domain.Human, externalIDP *domain.ExternalIDP, orgMemberRoles []string) (*domain.Human, error) { - if orgID == "" || !human.IsValid() || externalIDP == nil && (human.Password == nil || human.SecretString == "") { - return nil, caos_errs.ThrowInvalidArgument(nil, "COMMAND-GEdf2", "Errors.User.Invalid") + if orgID == "" { + return nil, caos_errs.ThrowInvalidArgument(nil, "COMMAND-GEdf2", "Errors.ResourceOwnerMissing") } orgIAMPolicy, err := c.getOrgIAMPolicy(ctx, orgID) if err != nil { @@ -179,6 +180,19 @@ func (c *Commands) createHuman(ctx context.Context, orgID string, human *domain. if err := human.CheckOrgIAMPolicy(orgIAMPolicy); err != nil { return nil, nil, err } + if !orgIAMPolicy.UserLoginMustBeDomain { + usernameSplit := strings.Split(human.Username, "@") + if len(usernameSplit) != 2 { + return nil, nil, caos_errs.ThrowInvalidArgument(nil, "COMMAND-Dfd21", "Errors.User.Invalid") + } + domainCheck := NewOrgDomainVerifiedWriteModel(usernameSplit[1]) + if err := c.eventstore.FilterToQueryReducer(ctx, domainCheck); err != nil { + return nil, nil, err + } + if domainCheck.Verified && domainCheck.ResourceOwner != orgID { + return nil, nil, caos_errs.ThrowInvalidArgument(nil, "COMMAND-SFd21", "Errors.User.DomainNotAllowedAsUsername") + } + } userID, err := c.idGenerator.Next() if err != nil { return nil, nil, err diff --git a/internal/command/user_human_test.go b/internal/command/user_human_test.go index d24e104f17..3d12f26a41 100644 --- a/internal/command/user_human_test.go +++ b/internal/command/user_human_test.go @@ -1463,30 +1463,6 @@ func TestCommandSide_RegisterHuman(t *testing.T) { err: caos_errs.IsErrorInvalidArgument, }, }, - { - name: "user invalid, invalid argument error", - fields: fields{ - eventstore: eventstoreExpect( - t, - ), - }, - 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.IsErrorInvalidArgument, - }, - }, { name: "org policy not found, precondition error", fields: fields{ @@ -1555,6 +1531,230 @@ func TestCommandSide_RegisterHuman(t *testing.T) { err: caos_errs.IsPreconditionFailed, }, }, + { + name: "user invalid, invalid argument 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, + ), + ), + ), + ), + }, + 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.IsErrorInvalidArgument, + }, + }, + { + name: "email domain reserved, invalid argument error", + fields: fields{ + eventstore: eventstoreExpect( + t, + expectFilter( + eventFromEventPusher( + org.NewOrgIAMPolicyAddedEvent(context.Background(), + &org.NewAggregate("org1", "org1").Aggregate, + false, + ), + ), + ), + expectFilter( + eventFromEventPusher( + org.NewPasswordComplexityPolicyAddedEvent(context.Background(), + &org.NewAggregate("org1", "org1").Aggregate, + 1, + false, + false, + false, + false, + ), + ), + ), + expectFilter( + eventFromEventPusher( + org.NewDomainAddedEvent(context.Background(), + &org.NewAggregate("org2", "org2").Aggregate, + "test.ch", + ), + ), + eventFromEventPusher( + org.NewDomainVerifiedEvent(context.Background(), + &org.NewAggregate("org2", "org2").Aggregate, + "test.ch", + ), + ), + ), + ), + }, + args: args{ + ctx: context.Background(), + orgID: "org1", + human: &domain.Human{ + Password: &domain.Password{ + SecretString: "password", + }, + Profile: &domain.Profile{ + FirstName: "firstname", + LastName: "lastname", + }, + Email: &domain.Email{ + EmailAddress: "email@test.ch", + }, + }, + }, + res: res{ + err: caos_errs.IsErrorInvalidArgument, + }, + }, + { + name: "email domain reserved, same org, ok", + fields: fields{ + eventstore: eventstoreExpect( + t, + expectFilter( + eventFromEventPusher( + org.NewOrgIAMPolicyAddedEvent(context.Background(), + &org.NewAggregate("org1", "org1").Aggregate, + false, + ), + ), + ), + expectFilter( + eventFromEventPusher( + org.NewPasswordComplexityPolicyAddedEvent(context.Background(), + &org.NewAggregate("org1", "org1").Aggregate, + 1, + false, + false, + false, + false, + ), + ), + ), + expectFilter( + eventFromEventPusher( + org.NewDomainAddedEvent(context.Background(), + &org.NewAggregate("org2", "org2").Aggregate, + "test.ch", + ), + ), + eventFromEventPusher( + org.NewDomainVerifiedEvent(context.Background(), + &org.NewAggregate("org2", "org2").Aggregate, + "test.ch", + ), + ), + eventFromEventPusher( + org.NewDomainRemovedEvent(context.Background(), + &org.NewAggregate("org2", "org2").Aggregate, + "test.ch", + true, + ), + ), + eventFromEventPusher( + org.NewDomainAddedEvent(context.Background(), + &org.NewAggregate("org1", "org1").Aggregate, + "test.ch", + ), + ), + eventFromEventPusher( + org.NewDomainVerifiedEvent(context.Background(), + &org.NewAggregate("org1", "org1").Aggregate, + "test.ch", + ), + ), + ), + expectPush( + []*repository.Event{ + eventFromEventPusher( + newRegisterHumanEvent("email@test.ch", "password", false, ""), + ), + eventFromEventPusher( + user.NewHumanInitialCodeAddedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + &crypto.CryptoValue{ + CryptoType: crypto.TypeEncryption, + Algorithm: "enc", + KeyID: "id", + Crypted: []byte("a"), + }, + time.Hour*1, + ), + ), + }, + uniqueConstraintsFromEventConstraint(user.NewAddUsernameUniqueConstraint("email@test.ch", "org1", false)), + ), + ), + idGenerator: id_mock.NewIDGeneratorExpectIDs(t, "user1"), + secretGenerator: GetMockSecretGenerator(t), + userPasswordAlg: crypto.CreateMockHashAlg(gomock.NewController(t)), + }, + args: args{ + ctx: context.Background(), + orgID: "org1", + human: &domain.Human{ + Password: &domain.Password{ + SecretString: "password", + }, + Profile: &domain.Profile{ + FirstName: "firstname", + LastName: "lastname", + }, + Email: &domain.Email{ + EmailAddress: "email@test.ch", + }, + }, + }, + res: res{ + want: &domain.Human{ + ObjectRoot: models.ObjectRoot{ + AggregateID: "user1", + ResourceOwner: "org1", + }, + Username: "email@test.ch", + Profile: &domain.Profile{ + FirstName: "firstname", + LastName: "lastname", + DisplayName: "firstname lastname", + PreferredLanguage: language.Und, + }, + Email: &domain.Email{ + EmailAddress: "email@test.ch", + }, + State: domain.UserStateInitial, + }, + }, + }, { name: "add human (with password and initial code), ok", fields: fields{ @@ -1583,7 +1783,7 @@ func TestCommandSide_RegisterHuman(t *testing.T) { expectPush( []*repository.Event{ eventFromEventPusher( - newRegisterHumanEvent("password", false, ""), + newRegisterHumanEvent("username", "password", false, ""), ), eventFromEventPusher( user.NewHumanInitialCodeAddedEvent(context.Background(), @@ -1670,7 +1870,7 @@ func TestCommandSide_RegisterHuman(t *testing.T) { expectPush( []*repository.Event{ eventFromEventPusher( - newRegisterHumanEvent("password", false, ""), + newRegisterHumanEvent("username", "password", false, ""), ), eventFromEventPusher( user.NewHumanEmailVerifiedEvent(context.Background(), @@ -1751,7 +1951,7 @@ func TestCommandSide_RegisterHuman(t *testing.T) { expectPush( []*repository.Event{ eventFromEventPusher( - newRegisterHumanEvent("password", false, "+41711234567"), + newRegisterHumanEvent("username", "password", false, "+41711234567"), ), eventFromEventPusher( user.NewHumanInitialCodeAddedEvent(context.Background(), @@ -1854,7 +2054,7 @@ func TestCommandSide_RegisterHuman(t *testing.T) { expectPush( []*repository.Event{ eventFromEventPusher( - newRegisterHumanEvent("password", false, "+41711234567"), + newRegisterHumanEvent("username", "password", false, "+41711234567"), ), eventFromEventPusher( user.NewHumanInitialCodeAddedEvent(context.Background(), @@ -2285,10 +2485,10 @@ func newAddHumanEvent(password string, changeRequired bool, phone string) *user. return event } -func newRegisterHumanEvent(password string, changeRequired bool, phone string) *user.HumanRegisteredEvent { +func newRegisterHumanEvent(username, password string, changeRequired bool, phone string) *user.HumanRegisteredEvent { event := user.NewHumanRegisteredEvent(context.Background(), &user.NewAggregate("user1", "org1").Aggregate, - "username", + username, "firstname", "lastname", "", diff --git a/internal/domain/step.go b/internal/domain/step.go index b4fd3ddc61..bdfd1bf259 100644 --- a/internal/domain/step.go +++ b/internal/domain/step.go @@ -21,6 +21,7 @@ const ( Step16 Step17 Step18 + Step19 //StepCount marks the the length of possible steps (StepCount-1 == last possible step) StepCount ) diff --git a/internal/setup/config.go b/internal/setup/config.go index 535268af66..a169d38b8c 100644 --- a/internal/setup/config.go +++ b/internal/setup/config.go @@ -24,6 +24,7 @@ type IAMSetUp struct { Step16 *command.Step16 Step17 *command.Step17 Step18 *command.Step18 + Step19 *command.Step19 } func (setup *IAMSetUp) Steps(currentDone domain.Step) ([]command.Step, error) { @@ -48,6 +49,7 @@ func (setup *IAMSetUp) Steps(currentDone domain.Step) ([]command.Step, error) { setup.Step16, setup.Step17, setup.Step18, + setup.Step19, } { if step.Step() <= currentDone { continue