From 3539418a4a4d6f5f25a6fdd75b4dc61987e113e0 Mon Sep 17 00:00:00 2001 From: Livio Spring Date: Tue, 6 Dec 2022 09:01:31 +0100 Subject: [PATCH] fix: handle UserLoginMustBeDomain changes correctly (#4765) * fix: handle UserLoginMustBeDomain changes correctly * fix: remove verified domains (and not only primary) as suffix * fix: ensure testability by changing map to slice * cleanup * reduce complexity of DomainPolicyUsernamesWriteModel.Reduce() * add test for removed org policy --- internal/api/grpc/admin/domain_policy.go | 98 +--- internal/api/grpc/admin/import.go | 2 +- internal/command/instance_policy_domain.go | 82 +++- .../command/instance_policy_domain_model.go | 62 ++- .../command/instance_policy_domain_test.go | 181 +++++-- internal/command/org_policy_domain.go | 184 +++++-- internal/command/org_policy_domain_model.go | 13 +- internal/command/org_policy_domain_test.go | 456 ++++++++++++++---- internal/command/policy_org_model.go | 140 ++++++ internal/command/user_domain_policy.go | 56 ++- internal/command/user_domain_policy_test.go | 63 ++- internal/repository/user/user.go | 37 +- 12 files changed, 1042 insertions(+), 332 deletions(-) diff --git a/internal/api/grpc/admin/domain_policy.go b/internal/api/grpc/admin/domain_policy.go index 3222f40d6c..7269fd3059 100644 --- a/internal/api/grpc/admin/domain_policy.go +++ b/internal/api/grpc/admin/domain_policy.go @@ -5,8 +5,6 @@ import ( "github.com/zitadel/zitadel/internal/api/grpc/object" policy_grpc "github.com/zitadel/zitadel/internal/api/grpc/policy" - "github.com/zitadel/zitadel/internal/domain" - "github.com/zitadel/zitadel/internal/eventstore/v1/models" admin_pb "github.com/zitadel/zitadel/pkg/grpc/admin" ) @@ -27,44 +25,32 @@ func (s *Server) GetCustomDomainPolicy(ctx context.Context, req *admin_pb.GetCus } func (s *Server) AddCustomDomainPolicy(ctx context.Context, req *admin_pb.AddCustomDomainPolicyRequest) (*admin_pb.AddCustomDomainPolicyResponse, error) { - policy, err := s.command.AddOrgDomainPolicy(ctx, req.OrgId, DomainPolicyToDomain(req.UserLoginMustBeDomain, req.ValidateOrgDomains, req.SmtpSenderAddressMatchesInstanceDomain)) + details, err := s.command.AddOrgDomainPolicy(ctx, req.OrgId, req.UserLoginMustBeDomain, req.ValidateOrgDomains, req.SmtpSenderAddressMatchesInstanceDomain) if err != nil { return nil, err } return &admin_pb.AddCustomDomainPolicyResponse{ - Details: object.AddToDetailsPb( - policy.Sequence, - policy.ChangeDate, - policy.ResourceOwner, - ), + Details: object.DomainToAddDetailsPb(details), }, nil } func (s *Server) UpdateDomainPolicy(ctx context.Context, req *admin_pb.UpdateDomainPolicyRequest) (*admin_pb.UpdateDomainPolicyResponse, error) { - config, err := s.command.ChangeDefaultDomainPolicy(ctx, updateDomainPolicyToDomain(req)) + details, err := s.command.ChangeDefaultDomainPolicy(ctx, req.UserLoginMustBeDomain, req.ValidateOrgDomains, req.SmtpSenderAddressMatchesInstanceDomain) if err != nil { return nil, err } return &admin_pb.UpdateDomainPolicyResponse{ - Details: object.ChangeToDetailsPb( - config.Sequence, - config.ChangeDate, - config.ResourceOwner, - ), + Details: object.DomainToChangeDetailsPb(details), }, nil } func (s *Server) UpdateCustomDomainPolicy(ctx context.Context, req *admin_pb.UpdateCustomDomainPolicyRequest) (*admin_pb.UpdateCustomDomainPolicyResponse, error) { - config, err := s.command.ChangeOrgDomainPolicy(ctx, req.OrgId, updateCustomDomainPolicyToDomain(req)) + details, err := s.command.ChangeOrgDomainPolicy(ctx, req.OrgId, req.UserLoginMustBeDomain, req.ValidateOrgDomains, req.SmtpSenderAddressMatchesInstanceDomain) if err != nil { return nil, err } return &admin_pb.UpdateCustomDomainPolicyResponse{ - Details: object.ChangeToDetailsPb( - config.Sequence, - config.ChangeDate, - config.ResourceOwner, - ), + Details: object.DomainToChangeDetailsPb(details), }, nil } @@ -76,72 +62,37 @@ func (s *Server) ResetCustomDomainPolicyToDefault(ctx context.Context, req *admi return &admin_pb.ResetCustomDomainPolicyToDefaultResponse{Details: object.DomainToChangeDetailsPb(details)}, nil } -func DomainPolicyToDomain(userLoginMustBeDomain, validateOrgDomains, smtpSenderAddressMatchesInstanceDomain bool) *domain.DomainPolicy { - return &domain.DomainPolicy{ - UserLoginMustBeDomain: userLoginMustBeDomain, - ValidateOrgDomains: validateOrgDomains, - SMTPSenderAddressMatchesInstanceDomain: smtpSenderAddressMatchesInstanceDomain, - } -} - -func updateDomainPolicyToDomain(req *admin_pb.UpdateDomainPolicyRequest) *domain.DomainPolicy { - return &domain.DomainPolicy{ - UserLoginMustBeDomain: req.UserLoginMustBeDomain, - ValidateOrgDomains: req.ValidateOrgDomains, - SMTPSenderAddressMatchesInstanceDomain: req.SmtpSenderAddressMatchesInstanceDomain, - } -} - -func updateCustomDomainPolicyToDomain(req *admin_pb.UpdateCustomDomainPolicyRequest) *domain.DomainPolicy { - return &domain.DomainPolicy{ - ObjectRoot: models.ObjectRoot{ - AggregateID: req.OrgId, - }, - UserLoginMustBeDomain: req.UserLoginMustBeDomain, - ValidateOrgDomains: req.ValidateOrgDomains, - SMTPSenderAddressMatchesInstanceDomain: req.SmtpSenderAddressMatchesInstanceDomain, - } -} +// the following requests only exist for backwards compatibility +// OrgIAMPolicy has been replaced by DomainPolicy, which also extends it with validateOrgDomains and smtpSenderAddressMatchesInstanceDomain +// Add and Update requests will therefore set the previous default (true) func (s *Server) AddCustomOrgIAMPolicy(ctx context.Context, req *admin_pb.AddCustomOrgIAMPolicyRequest) (*admin_pb.AddCustomOrgIAMPolicyResponse, error) { - policy, err := s.command.AddOrgDomainPolicy(ctx, req.OrgId, DomainPolicyToDomain(req.UserLoginMustBeDomain, true, true)) + details, err := s.command.AddOrgDomainPolicy(ctx, req.OrgId, req.UserLoginMustBeDomain, true, true) if err != nil { return nil, err } return &admin_pb.AddCustomOrgIAMPolicyResponse{ - Details: object.AddToDetailsPb( - policy.Sequence, - policy.ChangeDate, - policy.ResourceOwner, - ), + Details: object.DomainToAddDetailsPb(details), }, nil } func (s *Server) UpdateOrgIAMPolicy(ctx context.Context, req *admin_pb.UpdateOrgIAMPolicyRequest) (*admin_pb.UpdateOrgIAMPolicyResponse, error) { - config, err := s.command.ChangeDefaultDomainPolicy(ctx, updateOrgIAMPolicyToDomain(req)) + details, err := s.command.ChangeDefaultDomainPolicy(ctx, req.UserLoginMustBeDomain, true, true) if err != nil { return nil, err } return &admin_pb.UpdateOrgIAMPolicyResponse{ - Details: object.ChangeToDetailsPb( - config.Sequence, - config.ChangeDate, - config.ResourceOwner, - ), + Details: object.DomainToChangeDetailsPb(details), }, nil } func (s *Server) UpdateCustomOrgIAMPolicy(ctx context.Context, req *admin_pb.UpdateCustomOrgIAMPolicyRequest) (*admin_pb.UpdateCustomOrgIAMPolicyResponse, error) { - config, err := s.command.ChangeOrgDomainPolicy(ctx, req.OrgId, updateCustomOrgIAMPolicyToDomain(req)) + details, err := s.command.ChangeOrgDomainPolicy(ctx, req.OrgId, req.UserLoginMustBeDomain, true, true) if err != nil { return nil, err } return &admin_pb.UpdateCustomOrgIAMPolicyResponse{ - Details: object.ChangeToDetailsPb( - config.Sequence, - config.ChangeDate, - config.ResourceOwner, - ), + Details: object.DomainToChangeDetailsPb(details), }, nil } @@ -168,22 +119,3 @@ func (s *Server) ResetCustomOrgIAMPolicyToDefault(ctx context.Context, req *admi } return &admin_pb.ResetCustomOrgIAMPolicyToDefaultResponse{Details: object.DomainToChangeDetailsPb(details)}, nil } - -func updateOrgIAMPolicyToDomain(req *admin_pb.UpdateOrgIAMPolicyRequest) *domain.DomainPolicy { - return &domain.DomainPolicy{ - UserLoginMustBeDomain: req.UserLoginMustBeDomain, - ValidateOrgDomains: true, - SMTPSenderAddressMatchesInstanceDomain: true, - } -} - -func updateCustomOrgIAMPolicyToDomain(req *admin_pb.UpdateCustomOrgIAMPolicyRequest) *domain.DomainPolicy { - return &domain.DomainPolicy{ - ObjectRoot: models.ObjectRoot{ - AggregateID: req.OrgId, - }, - UserLoginMustBeDomain: req.UserLoginMustBeDomain, - ValidateOrgDomains: true, - SMTPSenderAddressMatchesInstanceDomain: true, - } -} diff --git a/internal/api/grpc/admin/import.go b/internal/api/grpc/admin/import.go index b3f0ec4493..ec84475b0f 100644 --- a/internal/api/grpc/admin/import.go +++ b/internal/api/grpc/admin/import.go @@ -377,7 +377,7 @@ func (s *Server) importData(ctx context.Context, orgs []*admin_pb.DataOrg) (*adm domainPolicy := org.GetDomainPolicy() if org.DomainPolicy != nil { - _, err := s.command.AddOrgDomainPolicy(ctx, org.GetOrgId(), DomainPolicyToDomain(domainPolicy.UserLoginMustBeDomain, domainPolicy.ValidateOrgDomains, domainPolicy.SmtpSenderAddressMatchesInstanceDomain)) + _, err := s.command.AddOrgDomainPolicy(ctx, org.GetOrgId(), domainPolicy.UserLoginMustBeDomain, domainPolicy.ValidateOrgDomains, domainPolicy.SmtpSenderAddressMatchesInstanceDomain) if err != nil { errors = append(errors, &admin_pb.ImportDataError{Type: "domain_policy", Id: org.GetOrgId(), Message: err.Error()}) } diff --git a/internal/command/instance_policy_domain.go b/internal/command/instance_policy_domain.go index a49907e352..21fe2557b8 100644 --- a/internal/command/instance_policy_domain.go +++ b/internal/command/instance_policy_domain.go @@ -25,30 +25,22 @@ func (c *Commands) AddDefaultDomainPolicy(ctx context.Context, userLoginMustBeDo return pushedEventsToObjectDetails(pushedEvents), nil } -func (c *Commands) ChangeDefaultDomainPolicy(ctx context.Context, policy *domain.DomainPolicy) (*domain.DomainPolicy, error) { - existingPolicy, err := c.defaultDomainPolicyWriteModelByID(ctx) +func (c *Commands) ChangeDefaultDomainPolicy(ctx context.Context, userLoginMustBeDomain, validateOrgDomains, smtpSenderAddressMatchesInstanceDomain bool) (*domain.ObjectDetails, error) { + instanceAgg := instance.NewAggregate(authz.GetInstance(ctx).InstanceID()) + cmds, err := preparation.PrepareCommands(ctx, c.eventstore.Filter, prepareChangeDefaultDomainPolicy(instanceAgg, userLoginMustBeDomain, validateOrgDomains, smtpSenderAddressMatchesInstanceDomain)) if err != nil { return nil, err } - if !existingPolicy.State.Exists() { - return nil, caos_errs.ThrowNotFound(nil, "INSTANCE-0Pl0d", "Errors.IAM.DomainPolicy.NotFound") - } - - instanceAgg := InstanceAggregateFromWriteModel(&existingPolicy.PolicyDomainWriteModel.WriteModel) - changedEvent, hasChanged := existingPolicy.NewChangedEvent(ctx, instanceAgg, policy.UserLoginMustBeDomain, policy.ValidateOrgDomains, policy.SMTPSenderAddressMatchesInstanceDomain) - if !hasChanged { - return nil, caos_errs.ThrowPreconditionFailed(nil, "INSTANCE-pl9fN", "Errors.IAM.DomainPolicy.NotChanged") - } - - pushedEvents, err := c.eventstore.Push(ctx, changedEvent) + pushedEvents, err := c.eventstore.Push(ctx, cmds...) if err != nil { return nil, err } - err = AppendAndReduce(existingPolicy, pushedEvents...) - if err != nil { - return nil, err - } - return writeModelToDomainPolicy(existingPolicy), nil + // returning the values of the first event as this is the one from the instance + return &domain.ObjectDetails{ + Sequence: pushedEvents[0].Sequence(), + EventDate: pushedEvents[0].CreationDate(), + ResourceOwner: pushedEvents[0].Aggregate().ResourceOwner, + }, nil } func (c *Commands) getDefaultDomainPolicy(ctx context.Context) (*domain.DomainPolicy, error) { @@ -84,15 +76,10 @@ func prepareAddDefaultDomainPolicy( ) preparation.Validation { return func() (preparation.CreateCommands, error) { return func(ctx context.Context, filter preparation.FilterToQueryReducer) ([]eventstore.Command, error) { - writeModel := NewInstanceDomainPolicyWriteModel(ctx) - events, err := filter(ctx, writeModel.Query()) + writeModel, err := instanceDomainPolicy(ctx, filter) if err != nil { return nil, err } - writeModel.AppendEvents(events...) - if err = writeModel.Reduce(); err != nil { - return nil, err - } if writeModel.State == domain.PolicyStateActive { return nil, caos_errs.ThrowAlreadyExists(nil, "INSTANCE-Lk0dS", "Errors.Instance.DomainPolicy.AlreadyExists") } @@ -106,3 +93,50 @@ func prepareAddDefaultDomainPolicy( }, nil } } + +func prepareChangeDefaultDomainPolicy( + a *instance.Aggregate, + userLoginMustBeDomain, + validateOrgDomains, + smtpSenderAddressMatchesInstanceDomain bool, +) preparation.Validation { + return func() (preparation.CreateCommands, error) { + return func(ctx context.Context, filter preparation.FilterToQueryReducer) ([]eventstore.Command, error) { + writeModel, err := instanceDomainPolicy(ctx, filter) + if err != nil { + return nil, err + } + if !writeModel.State.Exists() { + return nil, caos_errs.ThrowNotFound(nil, "INSTANCE-0Pl0d", "Errors.Instance.DomainPolicy.NotFound") + } + changedEvent, usernameChange, err := writeModel.NewChangedEvent(ctx, &a.Aggregate, + userLoginMustBeDomain, + validateOrgDomains, + smtpSenderAddressMatchesInstanceDomain, + ) + if err != nil { + return nil, err + } + cmds := []eventstore.Command{changedEvent} + // if the UserLoginMustBeDomain has not changed, no further changes are needed + if !usernameChange { + return cmds, err + } + // get all organisations without a custom domain policy + orgsWriteModel, err := domainPolicyOrgs(ctx, filter) + if err != nil { + return nil, err + } + // loop over all found organisations to get their usernames + // and to compute the username changed events + for _, orgID := range orgsWriteModel.OrgIDs { + usersWriteModel, err := domainPolicyUsernames(ctx, filter, orgID) + if err != nil { + return nil, err + } + cmds = append(cmds, usersWriteModel.NewUsernameChangedEvents(ctx, userLoginMustBeDomain)...) + } + return cmds, nil + }, nil + } +} diff --git a/internal/command/instance_policy_domain_model.go b/internal/command/instance_policy_domain_model.go index cb6b9ca311..9ce7665283 100644 --- a/internal/command/instance_policy_domain_model.go +++ b/internal/command/instance_policy_domain_model.go @@ -4,9 +4,10 @@ import ( "context" "github.com/zitadel/zitadel/internal/api/authz" + caos_errs "github.com/zitadel/zitadel/internal/errors" "github.com/zitadel/zitadel/internal/eventstore" - "github.com/zitadel/zitadel/internal/repository/instance" + "github.com/zitadel/zitadel/internal/repository/org" "github.com/zitadel/zitadel/internal/repository/policy" ) @@ -57,9 +58,10 @@ func (wm *InstanceDomainPolicyWriteModel) NewChangedEvent( aggregate *eventstore.Aggregate, userLoginMustBeDomain, validateOrgDomain, - smtpSenderAddresssMatchesInstanceDomain bool) (*instance.DomainPolicyChangedEvent, bool) { + smtpSenderAddresssMatchesInstanceDomain bool) (changedEvent *instance.DomainPolicyChangedEvent, usernameChange bool, err error) { changes := make([]policy.DomainPolicyChanges, 0) if wm.UserLoginMustBeDomain != userLoginMustBeDomain { + usernameChange = true changes = append(changes, policy.ChangeUserLoginMustBeDomain(userLoginMustBeDomain)) } if wm.ValidateOrgDomains != validateOrgDomain { @@ -69,11 +71,55 @@ func (wm *InstanceDomainPolicyWriteModel) NewChangedEvent( changes = append(changes, policy.ChangeSMTPSenderAddressMatchesInstanceDomain(smtpSenderAddresssMatchesInstanceDomain)) } if len(changes) == 0 { - return nil, false + return nil, false, caos_errs.ThrowPreconditionFailed(nil, "INSTANCE-pl9fN", "Errors.IAM.DomainPolicy.NotChanged") } - changedEvent, err := instance.NewDomainPolicyChangedEvent(ctx, aggregate, changes) - if err != nil { - return nil, false - } - return changedEvent, true + changedEvent, err = instance.NewDomainPolicyChangedEvent(ctx, aggregate, changes) + return changedEvent, usernameChange, err +} + +type DomainPolicyOrgsWriteModel struct { + eventstore.WriteModel + + OrgIDs []string +} + +func NewDomainPolicyOrgsWriteModel() *DomainPolicyOrgsWriteModel { + return &DomainPolicyOrgsWriteModel{ + WriteModel: eventstore.WriteModel{}, + } +} + +func (wm *DomainPolicyOrgsWriteModel) AppendEvents(events ...eventstore.Event) { + wm.WriteModel.AppendEvents(events...) +} + +func (wm *DomainPolicyOrgsWriteModel) Reduce() error { + for _, event := range wm.Events { + switch e := event.(type) { + case *org.OrgAddedEvent: + wm.OrgIDs = append(wm.OrgIDs, e.Aggregate().ID) + case *org.DomainPolicyAddedEvent: + for i, orgID := range wm.OrgIDs { + if orgID == e.Aggregate().ID { + wm.OrgIDs[i] = wm.OrgIDs[len(wm.OrgIDs)-1] + wm.OrgIDs = wm.OrgIDs[:len(wm.OrgIDs)-1] + break + } + } + case *org.DomainPolicyRemovedEvent: + wm.OrgIDs = append(wm.OrgIDs, e.Aggregate().ID) + } + } + return wm.WriteModel.Reduce() +} + +func (wm *DomainPolicyOrgsWriteModel) Query() *eventstore.SearchQueryBuilder { + return eventstore.NewSearchQueryBuilder(eventstore.ColumnsEvent). + AddQuery(). + AggregateTypes(org.AggregateType). + EventTypes( + org.OrgAddedEventType, + org.DomainPolicyAddedEventType, + org.DomainPolicyRemovedEventType). + Builder() } diff --git a/internal/command/instance_policy_domain_test.go b/internal/command/instance_policy_domain_test.go index 1a44b07166..e4820401e4 100644 --- a/internal/command/instance_policy_domain_test.go +++ b/internal/command/instance_policy_domain_test.go @@ -5,15 +5,17 @@ import ( "testing" "github.com/stretchr/testify/assert" + "golang.org/x/text/language" "github.com/zitadel/zitadel/internal/api/authz" "github.com/zitadel/zitadel/internal/domain" caos_errs "github.com/zitadel/zitadel/internal/errors" "github.com/zitadel/zitadel/internal/eventstore" "github.com/zitadel/zitadel/internal/eventstore/repository" - "github.com/zitadel/zitadel/internal/eventstore/v1/models" "github.com/zitadel/zitadel/internal/repository/instance" + "github.com/zitadel/zitadel/internal/repository/org" "github.com/zitadel/zitadel/internal/repository/policy" + "github.com/zitadel/zitadel/internal/repository/user" ) func TestCommandSide_AddDefaultDomainPolicy(t *testing.T) { @@ -121,11 +123,13 @@ func TestCommandSide_ChangeDefaultDomainPolicy(t *testing.T) { eventstore *eventstore.Eventstore } type args struct { - ctx context.Context - policy *domain.DomainPolicy + ctx context.Context + userLoginMustBeDomain bool + validateOrgDomains bool + smtpSenderAddressMatchesInstanceDomain bool } type res struct { - want *domain.DomainPolicy + want *domain.ObjectDetails err func(error) bool } tests := []struct { @@ -143,12 +147,10 @@ func TestCommandSide_ChangeDefaultDomainPolicy(t *testing.T) { ), }, args: args{ - ctx: context.Background(), - policy: &domain.DomainPolicy{ - UserLoginMustBeDomain: true, - ValidateOrgDomains: true, - SMTPSenderAddressMatchesInstanceDomain: true, - }, + ctx: context.Background(), + userLoginMustBeDomain: true, + validateOrgDomains: true, + smtpSenderAddressMatchesInstanceDomain: true, }, res: res{ err: caos_errs.IsNotFound, @@ -172,12 +174,10 @@ func TestCommandSide_ChangeDefaultDomainPolicy(t *testing.T) { ), }, args: args{ - ctx: context.Background(), - policy: &domain.DomainPolicy{ - UserLoginMustBeDomain: true, - ValidateOrgDomains: true, - SMTPSenderAddressMatchesInstanceDomain: true, - }, + ctx: context.Background(), + userLoginMustBeDomain: true, + validateOrgDomains: true, + smtpSenderAddressMatchesInstanceDomain: true, }, res: res{ err: caos_errs.IsPreconditionFailed, @@ -198,32 +198,149 @@ func TestCommandSide_ChangeDefaultDomainPolicy(t *testing.T) { ), ), ), + // domainPolicyOrgs + expectFilter( + eventFromEventPusher( + org.NewOrgAddedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + "org1", + ), + ), + eventFromEventPusher( + org.NewOrgAddedEvent(context.Background(), + &org.NewAggregate("org2").Aggregate, + "org2", + ), + ), + eventFromEventPusher( + org.NewDomainPolicyAddedEvent(context.Background(), + &org.NewAggregate("org2").Aggregate, + false, + false, + false, + ), + ), + eventFromEventPusher( + org.NewOrgAddedEvent(context.Background(), + &org.NewAggregate("org3").Aggregate, + "org3", + ), + ), + eventFromEventPusher( + org.NewDomainPolicyAddedEvent(context.Background(), + &org.NewAggregate("org3").Aggregate, + false, + false, + false, + ), + ), + eventFromEventPusher( + org.NewDomainPolicyRemovedEvent(context.Background(), + &org.NewAggregate("org3").Aggregate, + ), + ), + ), + // domainPolicyUsernames for each org + // org1 + expectFilter( + eventFromEventPusher( + org.NewDomainPrimarySetEvent( + context.Background(), + &org.NewAggregate("org1").Aggregate, + "org1.com", + ), + ), + eventFromEventPusher( + org.NewDomainPrimarySetEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + "org1.com", + ), + ), + eventFromEventPusher( + user.NewHumanAddedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "user1", + "firstname", + "lastname", + "nickname", + "displayname", + language.English, + domain.GenderUnspecified, + "user1@org1.com", + false, + ), + ), + ), + // org3 + expectFilter( + eventFromEventPusher( + org.NewDomainPrimarySetEvent( + context.Background(), + &org.NewAggregate("org3").Aggregate, + "org3.com", + ), + ), + eventFromEventPusher( + org.NewDomainPrimarySetEvent(context.Background(), + &org.NewAggregate("org3").Aggregate, + "org3.com", + ), + ), + eventFromEventPusher( + user.NewHumanAddedEvent(context.Background(), + &user.NewAggregate("user1", "org3").Aggregate, + "user1", + "firstname", + "lastname", + "nickname", + "displayname", + language.English, + domain.GenderUnspecified, + "user1@org3.com", + false, + ), + ), + ), expectPush( []*repository.Event{ - eventFromEventPusher( + eventFromEventPusherWithInstanceID("INSTANCE", newDefaultDomainPolicyChangedEvent(context.Background(), false, false, false), ), + eventFromEventPusherWithInstanceID("INSTANCE", + user.NewUsernameChangedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "user1", + "user1@org1.com", + false, + user.UsernameChangedEventWithPolicyChange(), + ), + ), + eventFromEventPusherWithInstanceID("INSTANCE", + user.NewUsernameChangedEvent(context.Background(), + &user.NewAggregate("user1", "org3").Aggregate, + "user1", + "user1@org3.com", + false, + user.UsernameChangedEventWithPolicyChange(), + ), + ), }, + uniqueConstraintsFromEventConstraintWithInstanceID("INSTANCE", user.NewRemoveUsernameUniqueConstraint("user1", "org1", true)), + uniqueConstraintsFromEventConstraintWithInstanceID("INSTANCE", user.NewAddUsernameUniqueConstraint("user1@org1.com", "org1", false)), + uniqueConstraintsFromEventConstraintWithInstanceID("INSTANCE", user.NewRemoveUsernameUniqueConstraint("user1", "org3", true)), + uniqueConstraintsFromEventConstraintWithInstanceID("INSTANCE", user.NewAddUsernameUniqueConstraint("user1@org3.com", "org3", false)), ), ), }, args: args{ - ctx: context.Background(), - policy: &domain.DomainPolicy{ - UserLoginMustBeDomain: false, - ValidateOrgDomains: false, - SMTPSenderAddressMatchesInstanceDomain: false, - }, + ctx: authz.WithInstanceID(context.Background(), "INSTANCE"), + userLoginMustBeDomain: false, + validateOrgDomains: false, + smtpSenderAddressMatchesInstanceDomain: false, }, res: res{ - want: &domain.DomainPolicy{ - ObjectRoot: models.ObjectRoot{ - AggregateID: "INSTANCE", - ResourceOwner: "INSTANCE", - }, - UserLoginMustBeDomain: false, - ValidateOrgDomains: false, - SMTPSenderAddressMatchesInstanceDomain: false, + want: &domain.ObjectDetails{ + ResourceOwner: "INSTANCE", }, }, }, @@ -233,7 +350,7 @@ func TestCommandSide_ChangeDefaultDomainPolicy(t *testing.T) { r := &Commands{ eventstore: tt.fields.eventstore, } - got, err := r.ChangeDefaultDomainPolicy(tt.args.ctx, tt.args.policy) + got, err := r.ChangeDefaultDomainPolicy(tt.args.ctx, tt.args.userLoginMustBeDomain, tt.args.validateOrgDomains, tt.args.smtpSenderAddressMatchesInstanceDomain) if tt.res.err == nil { assert.NoError(t, err) } diff --git a/internal/command/org_policy_domain.go b/internal/command/org_policy_domain.go index 7b49c02413..5e840fc4a3 100644 --- a/internal/command/org_policy_domain.go +++ b/internal/command/org_policy_domain.go @@ -3,6 +3,7 @@ package command import ( "context" + "github.com/zitadel/zitadel/internal/command/preparation" "github.com/zitadel/zitadel/internal/domain" caos_errs "github.com/zitadel/zitadel/internal/errors" "github.com/zitadel/zitadel/internal/eventstore" @@ -10,89 +11,52 @@ import ( "github.com/zitadel/zitadel/internal/telemetry/tracing" ) -func (c *Commands) AddOrgDomainPolicy(ctx context.Context, resourceOwner string, policy *domain.DomainPolicy) (*domain.DomainPolicy, error) { +func (c *Commands) AddOrgDomainPolicy(ctx context.Context, resourceOwner string, userLoginMustBeDomain, validateOrgDomains, smtpSenderAddressMatchesInstanceDomain bool) (*domain.ObjectDetails, error) { if resourceOwner == "" { return nil, caos_errs.ThrowInvalidArgument(nil, "Org-4Jfsf", "Errors.ResourceOwnerMissing") } - addedPolicy := NewOrgDomainPolicyWriteModel(resourceOwner) - orgAgg := OrgAggregateFromWriteModel(&addedPolicy.PolicyDomainWriteModel.WriteModel) - event, err := c.addOrgDomainPolicy(ctx, orgAgg, addedPolicy, policy) + orgAgg := org.NewAggregate(resourceOwner) + cmds, err := preparation.PrepareCommands(ctx, c.eventstore.Filter, prepareAddOrgDomainPolicy(orgAgg, userLoginMustBeDomain, validateOrgDomains, smtpSenderAddressMatchesInstanceDomain)) if err != nil { return nil, err } - pushedEvents, err := c.eventstore.Push(ctx, event) + pushedEvents, err := c.eventstore.Push(ctx, cmds...) if err != nil { return nil, err } - err = AppendAndReduce(addedPolicy, pushedEvents...) - if err != nil { - return nil, err - } - return orgWriteModelToDomainPolicy(addedPolicy), nil + return pushedEventsToObjectDetails(pushedEvents), nil } -func (c *Commands) addOrgDomainPolicy(ctx context.Context, orgAgg *eventstore.Aggregate, addedPolicy *OrgDomainPolicyWriteModel, policy *domain.DomainPolicy) (eventstore.Command, error) { - err := c.eventstore.FilterToQueryReducer(ctx, addedPolicy) - if err != nil { - return nil, err - } - if addedPolicy.State == domain.PolicyStateActive { - return nil, caos_errs.ThrowAlreadyExists(nil, "ORG-1M8ds", "Errors.Org.DomainPolicy.AlreadyExists") - } - return org.NewDomainPolicyAddedEvent(ctx, orgAgg, policy.UserLoginMustBeDomain, policy.ValidateOrgDomains, policy.SMTPSenderAddressMatchesInstanceDomain), nil -} - -func (c *Commands) ChangeOrgDomainPolicy(ctx context.Context, resourceOwner string, policy *domain.DomainPolicy) (*domain.DomainPolicy, error) { +func (c *Commands) ChangeOrgDomainPolicy(ctx context.Context, resourceOwner string, userLoginMustBeDomain, validateOrgDomains, smtpSenderAddressMatchesInstanceDomain bool) (*domain.ObjectDetails, error) { if resourceOwner == "" { return nil, caos_errs.ThrowInvalidArgument(nil, "Org-5H8fs", "Errors.ResourceOwnerMissing") } - existingPolicy, err := c.orgDomainPolicyWriteModelByID(ctx, resourceOwner) + orgAgg := org.NewAggregate(resourceOwner) + cmds, err := preparation.PrepareCommands(ctx, c.eventstore.Filter, prepareChangeOrgDomainPolicy(orgAgg, userLoginMustBeDomain, validateOrgDomains, smtpSenderAddressMatchesInstanceDomain)) if err != nil { return nil, err } - if existingPolicy.State == domain.PolicyStateUnspecified || existingPolicy.State == domain.PolicyStateRemoved { - return nil, caos_errs.ThrowNotFound(nil, "ORG-2N9sd", "Errors.Org.DomainPolicy.NotFound") - } - - orgAgg := OrgAggregateFromWriteModel(&existingPolicy.PolicyDomainWriteModel.WriteModel) - changedEvent, hasChanged := existingPolicy.NewChangedEvent(ctx, orgAgg, policy.UserLoginMustBeDomain, policy.ValidateOrgDomains, policy.SMTPSenderAddressMatchesInstanceDomain) - if !hasChanged { - return nil, caos_errs.ThrowPreconditionFailed(nil, "ORG-3M9ds", "Errors.Org.LabelPolicy.NotChanged") - } - - pushedEvents, err := c.eventstore.Push(ctx, changedEvent) + pushedEvents, err := c.eventstore.Push(ctx, cmds...) if err != nil { return nil, err } - err = AppendAndReduce(existingPolicy, pushedEvents...) - if err != nil { - return nil, err - } - return orgWriteModelToDomainPolicy(existingPolicy), nil + return pushedEventsToObjectDetails(pushedEvents), nil } func (c *Commands) RemoveOrgDomainPolicy(ctx context.Context, orgID string) (*domain.ObjectDetails, error) { if orgID == "" { return nil, caos_errs.ThrowInvalidArgument(nil, "Org-3H8fs", "Errors.ResourceOwnerMissing") } - existingPolicy, err := c.orgDomainPolicyWriteModelByID(ctx, orgID) + orgAgg := org.NewAggregate(orgID) + cmds, err := preparation.PrepareCommands(ctx, c.eventstore.Filter, prepareRemoveOrgDomainPolicy(orgAgg)) if err != nil { return nil, err } - if existingPolicy.State == domain.PolicyStateUnspecified || existingPolicy.State == domain.PolicyStateRemoved { - return nil, caos_errs.ThrowNotFound(nil, "ORG-Dvsh3", "Errors.Org.DomainPolicy.NotFound") - } - - orgAgg := OrgAggregateFromWriteModel(&existingPolicy.PolicyDomainWriteModel.WriteModel) - pushedEvents, err := c.eventstore.Push(ctx, org.NewDomainPolicyRemovedEvent(ctx, orgAgg)) + pushedEvents, err := c.eventstore.Push(ctx, cmds...) if err != nil { return nil, err } - err = AppendAndReduce(existingPolicy, pushedEvents...) - if err != nil { - return nil, err - } - return writeModelToObjectDetails(&existingPolicy.PolicyDomainWriteModel.WriteModel), nil + return pushedEventsToObjectDetails(pushedEvents), nil } func (c *Commands) getOrgDomainPolicy(ctx context.Context, orgID string) (*domain.DomainPolicy, error) { @@ -117,3 +81,121 @@ func (c *Commands) orgDomainPolicyWriteModelByID(ctx context.Context, orgID stri } return writeModel, nil } + +func prepareAddOrgDomainPolicy( + a *org.Aggregate, + userLoginMustBeDomain, + validateOrgDomains, + smtpSenderAddressMatchesInstanceDomain bool, +) preparation.Validation { + return func() (preparation.CreateCommands, error) { + return func(ctx context.Context, filter preparation.FilterToQueryReducer) ([]eventstore.Command, error) { + writeModel, err := orgDomainPolicy(ctx, filter) + if err != nil { + return nil, err + } + if writeModel.State == domain.PolicyStateActive { + return nil, caos_errs.ThrowAlreadyExists(nil, "ORG-1M8ds", "Errors.Org.DomainPolicy.AlreadyExists") + } + cmds := []eventstore.Command{ + org.NewDomainPolicyAddedEvent(ctx, &a.Aggregate, + userLoginMustBeDomain, + validateOrgDomains, + smtpSenderAddressMatchesInstanceDomain, + ), + } + instancePolicy, err := instanceDomainPolicy(ctx, filter) + if err != nil { + return nil, err + } + // regardless if the UserLoginMustBeDomain setting is true or false, + // if it will be the same value as currently on the instance, + // then there no further changes are needed + if instancePolicy.UserLoginMustBeDomain == userLoginMustBeDomain { + return cmds, nil + } + // the UserLoginMustBeDomain setting will be different from the instance + // therefore get all usernames and the current primary domain + usersWriteModel, err := domainPolicyUsernames(ctx, filter, a.ID) + if err != nil { + return nil, err + } + return append(cmds, usersWriteModel.NewUsernameChangedEvents(ctx, userLoginMustBeDomain)...), nil + }, nil + } +} + +func prepareChangeOrgDomainPolicy( + a *org.Aggregate, + userLoginMustBeDomain, + validateOrgDomains, + smtpSenderAddressMatchesInstanceDomain bool, +) preparation.Validation { + return func() (preparation.CreateCommands, error) { + return func(ctx context.Context, filter preparation.FilterToQueryReducer) ([]eventstore.Command, error) { + writeModel, err := orgDomainPolicy(ctx, filter) + if err != nil { + return nil, err + } + if !writeModel.State.Exists() { + return nil, caos_errs.ThrowNotFound(nil, "ORG-2N9sd", "Errors.Org.DomainPolicy.NotFound") + } + changedEvent, usernameChange, err := writeModel.NewChangedEvent(ctx, &a.Aggregate, + userLoginMustBeDomain, + validateOrgDomains, + smtpSenderAddressMatchesInstanceDomain, + ) + if err != nil { + return nil, err + } + cmds := []eventstore.Command{changedEvent} + // if the UserLoginMustBeDomain has not changed, no further changes are needed + if !usernameChange { + return cmds, err + } + // get all usernames and the primary domain + usersWriteModel, err := domainPolicyUsernames(ctx, filter, a.ID) + if err != nil { + return nil, err + } + // to compute the username changed events + return append(cmds, usersWriteModel.NewUsernameChangedEvents(ctx, userLoginMustBeDomain)...), nil + }, nil + } +} + +func prepareRemoveOrgDomainPolicy( + a *org.Aggregate, +) preparation.Validation { + return func() (preparation.CreateCommands, error) { + return func(ctx context.Context, filter preparation.FilterToQueryReducer) ([]eventstore.Command, error) { + writeModel, err := orgDomainPolicy(ctx, filter) + if err != nil { + return nil, err + } + if !writeModel.State.Exists() { + return nil, caos_errs.ThrowNotFound(nil, "ORG-Dvsh3", "Errors.Org.DomainPolicy.NotFound") + } + instancePolicy, err := instanceDomainPolicy(ctx, filter) + if err != nil { + return nil, err + } + cmds := []eventstore.Command{ + org.NewDomainPolicyRemovedEvent(ctx, &a.Aggregate), + } + // regardless if the UserLoginMustBeDomain setting is true or false, + // if it will be the same value as currently on the instance, + // then there no further changes are needed + if instancePolicy.UserLoginMustBeDomain == writeModel.UserLoginMustBeDomain { + return cmds, nil + } + // get all usernames and the primary domain + usersWriteModel, err := domainPolicyUsernames(ctx, filter, a.ID) + if err != nil { + return nil, err + } + // to compute the username changed events + return append(cmds, usersWriteModel.NewUsernameChangedEvents(ctx, instancePolicy.UserLoginMustBeDomain)...), nil + }, nil + } +} diff --git a/internal/command/org_policy_domain_model.go b/internal/command/org_policy_domain_model.go index 921e7faa66..328041ba00 100644 --- a/internal/command/org_policy_domain_model.go +++ b/internal/command/org_policy_domain_model.go @@ -3,6 +3,7 @@ package command import ( "context" + caos_errs "github.com/zitadel/zitadel/internal/errors" "github.com/zitadel/zitadel/internal/eventstore" "github.com/zitadel/zitadel/internal/repository/org" @@ -58,9 +59,10 @@ func (wm *OrgDomainPolicyWriteModel) NewChangedEvent( aggregate *eventstore.Aggregate, userLoginMustBeDomain, validateOrgDomains, - smtpSenderAddressMatchesInstanceDomain bool) (*org.DomainPolicyChangedEvent, bool) { + smtpSenderAddressMatchesInstanceDomain bool) (changedEvent *org.DomainPolicyChangedEvent, usernameChange bool, err error) { changes := make([]policy.DomainPolicyChanges, 0) if wm.UserLoginMustBeDomain != userLoginMustBeDomain { + usernameChange = true changes = append(changes, policy.ChangeUserLoginMustBeDomain(userLoginMustBeDomain)) } if wm.ValidateOrgDomains != validateOrgDomains { @@ -70,11 +72,8 @@ func (wm *OrgDomainPolicyWriteModel) NewChangedEvent( changes = append(changes, policy.ChangeSMTPSenderAddressMatchesInstanceDomain(smtpSenderAddressMatchesInstanceDomain)) } if len(changes) == 0 { - return nil, false + return nil, false, caos_errs.ThrowPreconditionFailed(nil, "ORG-3M9ds", "Errors.Org.LabelPolicy.NotChanged") } - changedEvent, err := org.NewDomainPolicyChangedEvent(ctx, aggregate, changes) - if err != nil { - return nil, false - } - return changedEvent, true + changedEvent, err = org.NewDomainPolicyChangedEvent(ctx, aggregate, changes) + return changedEvent, usernameChange, err } diff --git a/internal/command/org_policy_domain_test.go b/internal/command/org_policy_domain_test.go index 17f7420570..c034271c74 100644 --- a/internal/command/org_policy_domain_test.go +++ b/internal/command/org_policy_domain_test.go @@ -5,14 +5,16 @@ import ( "testing" "github.com/stretchr/testify/assert" + "golang.org/x/text/language" "github.com/zitadel/zitadel/internal/domain" caos_errs "github.com/zitadel/zitadel/internal/errors" "github.com/zitadel/zitadel/internal/eventstore" "github.com/zitadel/zitadel/internal/eventstore/repository" - "github.com/zitadel/zitadel/internal/eventstore/v1/models" + "github.com/zitadel/zitadel/internal/repository/instance" "github.com/zitadel/zitadel/internal/repository/org" "github.com/zitadel/zitadel/internal/repository/policy" + "github.com/zitadel/zitadel/internal/repository/user" ) func TestCommandSide_AddDomainPolicy(t *testing.T) { @@ -20,12 +22,14 @@ func TestCommandSide_AddDomainPolicy(t *testing.T) { eventstore *eventstore.Eventstore } type args struct { - ctx context.Context - orgID string - policy *domain.DomainPolicy + ctx context.Context + orgID string + userLoginMustBeDomain bool + validateOrgDomains bool + smtpSenderAddressMatchesInstanceDomain bool } type res struct { - want *domain.DomainPolicy + want *domain.ObjectDetails err func(error) bool } tests := []struct { @@ -42,18 +46,17 @@ func TestCommandSide_AddDomainPolicy(t *testing.T) { ), }, args: args{ - ctx: context.Background(), - policy: &domain.DomainPolicy{ - UserLoginMustBeDomain: true, - ValidateOrgDomains: true, - }, + ctx: context.Background(), + userLoginMustBeDomain: true, + validateOrgDomains: true, + smtpSenderAddressMatchesInstanceDomain: true, }, res: res{ err: caos_errs.IsErrorInvalidArgument, }, }, { - name: "mail template already existing, already exists error", + name: "policy already existing, already exists error", fields: fields{ eventstore: eventstoreExpect( t, @@ -70,24 +73,32 @@ func TestCommandSide_AddDomainPolicy(t *testing.T) { ), }, args: args{ - ctx: context.Background(), - orgID: "org1", - policy: &domain.DomainPolicy{ - UserLoginMustBeDomain: true, - ValidateOrgDomains: true, - SMTPSenderAddressMatchesInstanceDomain: true, - }, + ctx: context.Background(), + orgID: "org1", + userLoginMustBeDomain: true, + validateOrgDomains: true, + smtpSenderAddressMatchesInstanceDomain: true, }, res: res{ err: caos_errs.IsErrorAlreadyExists, }, }, { - name: "add policy,ok", + name: "add policy, no userLoginMustBeDomain change, ok", fields: fields{ eventstore: eventstoreExpect( t, expectFilter(), + expectFilter( + eventFromEventPusher( + instance.NewDomainPolicyAddedEvent(context.Background(), + &instance.NewAggregate("instanceID").Aggregate, + true, + false, + false, + ), + ), + ), expectPush( []*repository.Event{ eventFromEventPusher( @@ -103,23 +114,139 @@ func TestCommandSide_AddDomainPolicy(t *testing.T) { ), }, args: args{ - ctx: context.Background(), - orgID: "org1", - policy: &domain.DomainPolicy{ - UserLoginMustBeDomain: true, - ValidateOrgDomains: true, - SMTPSenderAddressMatchesInstanceDomain: true, - }, + ctx: context.Background(), + orgID: "org1", + userLoginMustBeDomain: true, + validateOrgDomains: true, + smtpSenderAddressMatchesInstanceDomain: true, }, res: res{ - want: &domain.DomainPolicy{ - ObjectRoot: models.ObjectRoot{ - AggregateID: "org1", - ResourceOwner: "org1", - }, - UserLoginMustBeDomain: true, - ValidateOrgDomains: true, - SMTPSenderAddressMatchesInstanceDomain: true, + want: &domain.ObjectDetails{ + ResourceOwner: "org1", + }, + }, + }, + { + name: "add policy, userLoginMustBeDomain changed, ok", + fields: fields{ + eventstore: eventstoreExpect( + t, + expectFilter(), + expectFilter( + eventFromEventPusher( + instance.NewDomainPolicyAddedEvent(context.Background(), + &instance.NewAggregate("instanceID").Aggregate, + false, + false, + false, + ), + ), + ), + expectFilter( + eventFromEventPusher( + org.NewDomainVerifiedEvent( + context.Background(), + &org.NewAggregate("org1").Aggregate, + "org.com", + ), + ), + eventFromEventPusher( + org.NewDomainVerifiedEvent( + context.Background(), + &org.NewAggregate("org1").Aggregate, + "test.com", + ), + ), + eventFromEventPusher( + org.NewDomainRemovedEvent( + context.Background(), + &org.NewAggregate("org1").Aggregate, + "test.com", + true, + ), + ), + eventFromEventPusher( + org.NewDomainPrimarySetEvent( + context.Background(), + &org.NewAggregate("org1").Aggregate, + "org.com", + ), + ), + eventFromEventPusher( + user.NewHumanAddedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "user1@org.com", + "firstname", + "lastname", + "nickname", + "displayname", + language.English, + domain.GenderUnspecified, + "user1@org.com", + false, + ), + ), + eventFromEventPusher( + user.NewHumanAddedEvent(context.Background(), + &user.NewAggregate("user2", "org1").Aggregate, + "user@test.com", + "firstname", + "lastname", + "nickname", + "displayname", + language.English, + domain.GenderUnspecified, + "user@test.com", + false, + ), + ), + ), + expectPush( + []*repository.Event{ + eventFromEventPusher( + org.NewDomainPolicyAddedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + true, + true, + true, + ), + ), + eventFromEventPusher( + user.NewUsernameChangedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "user1@org.com", + "user1", + true, + user.UsernameChangedEventWithPolicyChange(), + ), + ), + eventFromEventPusher( + user.NewUsernameChangedEvent(context.Background(), + &user.NewAggregate("user2", "org1").Aggregate, + "user@test.com", + "user@test.com", + true, + user.UsernameChangedEventWithPolicyChange(), + ), + ), + }, + uniqueConstraintsFromEventConstraint(user.NewRemoveUsernameUniqueConstraint("user1@org.com", "org1", false)), + uniqueConstraintsFromEventConstraint(user.NewAddUsernameUniqueConstraint("user1", "org1", true)), + uniqueConstraintsFromEventConstraint(user.NewRemoveUsernameUniqueConstraint("user@test.com", "org1", false)), + uniqueConstraintsFromEventConstraint(user.NewAddUsernameUniqueConstraint("user@test.com", "org1", true)), + ), + ), + }, + args: args{ + ctx: context.Background(), + orgID: "org1", + userLoginMustBeDomain: true, + validateOrgDomains: true, + smtpSenderAddressMatchesInstanceDomain: true, + }, + res: res{ + want: &domain.ObjectDetails{ + ResourceOwner: "org1", }, }, }, @@ -129,7 +256,7 @@ func TestCommandSide_AddDomainPolicy(t *testing.T) { r := &Commands{ eventstore: tt.fields.eventstore, } - got, err := r.AddOrgDomainPolicy(tt.args.ctx, tt.args.orgID, tt.args.policy) + got, err := r.AddOrgDomainPolicy(tt.args.ctx, tt.args.orgID, tt.args.userLoginMustBeDomain, tt.args.validateOrgDomains, tt.args.smtpSenderAddressMatchesInstanceDomain) if tt.res.err == nil { assert.NoError(t, err) } @@ -148,12 +275,14 @@ func TestCommandSide_ChangeDomainPolicy(t *testing.T) { eventstore *eventstore.Eventstore } type args struct { - ctx context.Context - orgID string - policy *domain.DomainPolicy + ctx context.Context + orgID string + userLoginMustBeDomain bool + validateOrgDomains bool + smtpSenderAddressMatchesInstanceDomain bool } type res struct { - want *domain.DomainPolicy + want *domain.ObjectDetails err func(error) bool } tests := []struct { @@ -170,11 +299,10 @@ func TestCommandSide_ChangeDomainPolicy(t *testing.T) { ), }, args: args{ - ctx: context.Background(), - policy: &domain.DomainPolicy{ - UserLoginMustBeDomain: true, - ValidateOrgDomains: true, - }, + ctx: context.Background(), + userLoginMustBeDomain: true, + validateOrgDomains: true, + smtpSenderAddressMatchesInstanceDomain: true, }, res: res{ err: caos_errs.IsErrorInvalidArgument, @@ -189,12 +317,11 @@ func TestCommandSide_ChangeDomainPolicy(t *testing.T) { ), }, args: args{ - ctx: context.Background(), - orgID: "org1", - policy: &domain.DomainPolicy{ - UserLoginMustBeDomain: true, - ValidateOrgDomains: true, - }, + ctx: context.Background(), + orgID: "org1", + userLoginMustBeDomain: true, + validateOrgDomains: true, + smtpSenderAddressMatchesInstanceDomain: true, }, res: res{ err: caos_errs.IsNotFound, @@ -218,20 +345,58 @@ func TestCommandSide_ChangeDomainPolicy(t *testing.T) { ), }, args: args{ - ctx: context.Background(), - orgID: "org1", - policy: &domain.DomainPolicy{ - UserLoginMustBeDomain: true, - ValidateOrgDomains: true, - SMTPSenderAddressMatchesInstanceDomain: true, - }, + ctx: context.Background(), + orgID: "org1", + userLoginMustBeDomain: true, + validateOrgDomains: true, + smtpSenderAddressMatchesInstanceDomain: true, }, res: res{ err: caos_errs.IsPreconditionFailed, }, }, { - name: "change, ok", + name: "change, no userLoginMustBeDomain change, ok", + fields: fields{ + eventstore: eventstoreExpect( + t, + expectFilter( + eventFromEventPusher( + org.NewDomainPolicyAddedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + false, + true, + true, + ), + ), + ), + expectPush( + []*repository.Event{ + eventFromEventPusher( + newDomainPolicyChangedEvent(context.Background(), "org1", + policy.ChangeValidateOrgDomains(false), + policy.ChangeSMTPSenderAddressMatchesInstanceDomain(false), + ), + ), + }, + ), + ), + }, + args: args{ + ctx: context.Background(), + orgID: "org1", + userLoginMustBeDomain: false, + validateOrgDomains: false, + smtpSenderAddressMatchesInstanceDomain: false, + }, + res: res{ + want: &domain.ObjectDetails{ + ResourceOwner: "org1", + }, + }, + }, + { + name: "change, userLoginMustBeDomain changed, ok", fields: fields{ eventstore: eventstoreExpect( t, @@ -245,33 +410,69 @@ func TestCommandSide_ChangeDomainPolicy(t *testing.T) { ), ), ), + expectFilter( + eventFromEventPusher( + org.NewDomainPrimarySetEvent( + context.Background(), + &org.NewAggregate("org1").Aggregate, + "org.com", + ), + ), + eventFromEventPusher( + org.NewDomainPrimarySetEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + "org.com", + ), + ), + eventFromEventPusher( + user.NewHumanAddedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "user1", + "firstname", + "lastname", + "nickname", + "displayname", + language.English, + domain.GenderUnspecified, + "user1@org.com", + false, + ), + ), + ), expectPush( []*repository.Event{ eventFromEventPusher( - newDomainPolicyChangedEvent(context.Background(), "org1", false, false, false), + newDomainPolicyChangedEvent(context.Background(), "org1", + policy.ChangeUserLoginMustBeDomain(false), + policy.ChangeValidateOrgDomains(false), + policy.ChangeSMTPSenderAddressMatchesInstanceDomain(false), + ), + ), + eventFromEventPusher( + user.NewUsernameChangedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "user1", + "user1@org.com", + false, + user.UsernameChangedEventWithPolicyChange(), + ), ), }, + uniqueConstraintsFromEventConstraint(user.NewRemoveUsernameUniqueConstraint("user1", "org1", true)), + uniqueConstraintsFromEventConstraint(user.NewAddUsernameUniqueConstraint("user1@org.com", "org1", false)), ), ), }, args: args{ - ctx: context.Background(), - orgID: "org1", - policy: &domain.DomainPolicy{ - UserLoginMustBeDomain: false, - ValidateOrgDomains: false, - SMTPSenderAddressMatchesInstanceDomain: false, - }, + ctx: context.Background(), + orgID: "org1", + userLoginMustBeDomain: false, + validateOrgDomains: false, + smtpSenderAddressMatchesInstanceDomain: false, }, res: res{ - want: &domain.DomainPolicy{ - ObjectRoot: models.ObjectRoot{ - AggregateID: "org1", - ResourceOwner: "org1", - }, - UserLoginMustBeDomain: false, - ValidateOrgDomains: false, - SMTPSenderAddressMatchesInstanceDomain: false, + want: &domain.ObjectDetails{ + ResourceOwner: "org1", }, }, }, @@ -281,7 +482,7 @@ func TestCommandSide_ChangeDomainPolicy(t *testing.T) { r := &Commands{ eventstore: tt.fields.eventstore, } - got, err := r.ChangeOrgDomainPolicy(tt.args.ctx, tt.args.orgID, tt.args.policy) + got, err := r.ChangeOrgDomainPolicy(tt.args.ctx, tt.args.orgID, tt.args.userLoginMustBeDomain, tt.args.validateOrgDomains, tt.args.smtpSenderAddressMatchesInstanceDomain) if tt.res.err == nil { assert.NoError(t, err) } @@ -344,7 +545,7 @@ func TestCommandSide_RemoveDomainPolicy(t *testing.T) { }, }, { - name: "remove, ok", + name: "remove, no userLoginMustBeDomain change, ok", fields: fields{ eventstore: eventstoreExpect( t, @@ -358,6 +559,16 @@ func TestCommandSide_RemoveDomainPolicy(t *testing.T) { ), ), ), + expectFilter( + eventFromEventPusher( + instance.NewDomainPolicyAddedEvent(context.Background(), + &instance.NewAggregate("instanceID").Aggregate, + true, + true, + true, + ), + ), + ), expectPush( []*repository.Event{ eventFromEventPusher( @@ -378,6 +589,91 @@ func TestCommandSide_RemoveDomainPolicy(t *testing.T) { }, }, }, + { + name: "remove, userLoginMustBeDomain changed, ok", + fields: fields{ + eventstore: eventstoreExpect( + t, + expectFilter( + eventFromEventPusher( + org.NewDomainPolicyAddedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + true, + true, + true, + ), + ), + ), + expectFilter( + eventFromEventPusher( + instance.NewDomainPolicyAddedEvent(context.Background(), + &instance.NewAggregate("instanceID").Aggregate, + false, + true, + true, + ), + ), + ), + expectFilter( + eventFromEventPusher( + org.NewDomainPrimarySetEvent( + context.Background(), + &org.NewAggregate("org1").Aggregate, + "org.com", + ), + ), + eventFromEventPusher( + org.NewDomainPrimarySetEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + "org.com", + ), + ), + eventFromEventPusher( + user.NewHumanAddedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "user1", + "firstname", + "lastname", + "nickname", + "displayname", + language.English, + domain.GenderUnspecified, + "user1@org.com", + false, + ), + ), + ), + expectPush( + []*repository.Event{ + eventFromEventPusher( + org.NewDomainPolicyRemovedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate), + ), + eventFromEventPusher( + user.NewUsernameChangedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "user1", + "user1@org.com", + false, + user.UsernameChangedEventWithPolicyChange(), + ), + ), + }, + uniqueConstraintsFromEventConstraint(user.NewRemoveUsernameUniqueConstraint("user1", "org1", true)), + uniqueConstraintsFromEventConstraint(user.NewAddUsernameUniqueConstraint("user1@org.com", "org1", false)), + ), + ), + }, + args: args{ + ctx: context.Background(), + orgID: "org1", + }, + res: res{ + want: &domain.ObjectDetails{ + ResourceOwner: "org1", + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -398,14 +694,10 @@ func TestCommandSide_RemoveDomainPolicy(t *testing.T) { } } -func newDomainPolicyChangedEvent(ctx context.Context, orgID string, userLoginMustBeDomain, validateOrgDomains, smtpSenderAddressMatchesInstanceDomain bool) *org.DomainPolicyChangedEvent { +func newDomainPolicyChangedEvent(ctx context.Context, orgID string, changes ...policy.DomainPolicyChanges) *org.DomainPolicyChangedEvent { event, _ := org.NewDomainPolicyChangedEvent(ctx, &org.NewAggregate(orgID).Aggregate, - []policy.DomainPolicyChanges{ - policy.ChangeUserLoginMustBeDomain(userLoginMustBeDomain), - policy.ChangeValidateOrgDomains(validateOrgDomains), - policy.ChangeSMTPSenderAddressMatchesInstanceDomain(smtpSenderAddressMatchesInstanceDomain), - }, + changes, ) return event } diff --git a/internal/command/policy_org_model.go b/internal/command/policy_org_model.go index 73434489ea..71b4a4015c 100644 --- a/internal/command/policy_org_model.go +++ b/internal/command/policy_org_model.go @@ -1,9 +1,14 @@ package command import ( + "context" + "strings" + "github.com/zitadel/zitadel/internal/domain" "github.com/zitadel/zitadel/internal/eventstore" + "github.com/zitadel/zitadel/internal/repository/org" "github.com/zitadel/zitadel/internal/repository/policy" + "github.com/zitadel/zitadel/internal/repository/user" ) type PolicyDomainWriteModel struct { @@ -39,3 +44,138 @@ func (wm *PolicyDomainWriteModel) Reduce() error { } return wm.WriteModel.Reduce() } + +type DomainPolicyUsernamesWriteModel struct { + eventstore.WriteModel + + PrimaryDomain string + VerifiedDomains []string + Users []*domainPolicyUsers +} + +type domainPolicyUsers struct { + id string + username string +} + +func NewDomainPolicyUsernamesWriteModel(orgID string) *DomainPolicyUsernamesWriteModel { + return &DomainPolicyUsernamesWriteModel{ + WriteModel: eventstore.WriteModel{ + ResourceOwner: orgID, + }, + Users: make([]*domainPolicyUsers, 0), + } +} + +func (wm *DomainPolicyUsernamesWriteModel) AppendEvents(events ...eventstore.Event) { + wm.WriteModel.AppendEvents(events...) +} + +func (wm *DomainPolicyUsernamesWriteModel) Reduce() error { + for _, event := range wm.Events { + switch e := event.(type) { + case *org.DomainVerifiedEvent: + wm.VerifiedDomains = append(wm.VerifiedDomains, e.Domain) + case *org.DomainRemovedEvent: + wm.removeDomain(e.Domain) + case *org.DomainPrimarySetEvent: + wm.PrimaryDomain = e.Domain + case *user.HumanAddedEvent: + wm.Users = append(wm.Users, &domainPolicyUsers{id: e.Aggregate().ID, username: e.UserName}) + case *user.HumanRegisteredEvent: + wm.Users = append(wm.Users, &domainPolicyUsers{id: e.Aggregate().ID, username: e.UserName}) + case *user.MachineAddedEvent: + wm.Users = append(wm.Users, &domainPolicyUsers{id: e.Aggregate().ID, username: e.UserName}) + case *user.UsernameChangedEvent: + for _, user := range wm.Users { + if user.id == e.Aggregate().ID { + user.username = e.UserName + break + } + } + case *user.DomainClaimedEvent: + for _, user := range wm.Users { + if user.id == e.Aggregate().ID { + user.username = e.UserName + break + } + } + case *user.UserRemovedEvent: + wm.removeUser(e.Aggregate().ID) + } + } + return wm.WriteModel.Reduce() +} + +func (wm *DomainPolicyUsernamesWriteModel) removeDomain(domain string) { + for i, verifiedDomain := range wm.VerifiedDomains { + if verifiedDomain != domain { + continue + } + wm.VerifiedDomains[i] = wm.VerifiedDomains[len(wm.VerifiedDomains)-1] + wm.VerifiedDomains[len(wm.VerifiedDomains)-1] = "" + wm.VerifiedDomains = wm.VerifiedDomains[:len(wm.VerifiedDomains)-1] + } +} + +func (wm *DomainPolicyUsernamesWriteModel) removeUser(userID string) { + for i, user := range wm.Users { + if user.id != userID { + continue + } + wm.Users[i] = wm.Users[len(wm.Users)-1] + wm.Users[len(wm.Users)-1] = nil + wm.Users = wm.Users[len(wm.Users)-1:] + } +} + +func (wm *DomainPolicyUsernamesWriteModel) Query() *eventstore.SearchQueryBuilder { + return eventstore.NewSearchQueryBuilder(eventstore.ColumnsEvent). + ResourceOwner(wm.ResourceOwner). + AddQuery(). + AggregateTypes(org.AggregateType, user.AggregateType). + EventTypes( + org.OrgDomainVerifiedEventType, + org.OrgDomainRemovedEventType, + org.OrgDomainPrimarySetEventType, + user.HumanAddedType, + user.HumanRegisteredType, + user.MachineAddedEventType, + user.UserUserNameChangedType, + user.UserDomainClaimedType, + user.UserRemovedType, + ). + Builder() +} + +func (wm *DomainPolicyUsernamesWriteModel) NewUsernameChangedEvents(ctx context.Context, userLoginMustBeDomain bool) []eventstore.Command { + events := make([]eventstore.Command, 0, len(wm.Users)) + for _, changeUser := range wm.Users { + events = append(events, user.NewUsernameChangedEvent(ctx, + &user.NewAggregate(changeUser.id, wm.ResourceOwner).Aggregate, + changeUser.username, + wm.newUsername(changeUser.username, userLoginMustBeDomain), + userLoginMustBeDomain, + user.UsernameChangedEventWithPolicyChange()), + ) + } + return events +} + +func (wm *DomainPolicyUsernamesWriteModel) newUsername(username string, userLoginMustBeDomain bool) string { + if !userLoginMustBeDomain { + // if the UserLoginMustBeDomain will be false, then it's currently true + // which means the usernames must be suffixed to ensure their uniqueness + // and the preferred login name remains the same + return username + "@" + wm.PrimaryDomain + } + // the UserLoginMustBeDomain is currently false + // which means the usernames might already be suffixed by a verified domain + // so let's remove a potential duplicate suffix + for _, verifiedDomain := range wm.VerifiedDomains { + if index := strings.LastIndex(username, "@"+verifiedDomain); index > 0 { + return username[:index] + } + } + return username +} diff --git a/internal/command/user_domain_policy.go b/internal/command/user_domain_policy.go index 341195c330..01961a50e0 100644 --- a/internal/command/user_domain_policy.go +++ b/internal/command/user_domain_policy.go @@ -10,40 +10,74 @@ import ( func domainPolicyWriteModel(ctx context.Context, filter preparation.FilterToQueryReducer) (*PolicyDomainWriteModel, error) { wm, err := orgDomainPolicy(ctx, filter) - if err != nil || wm != nil && wm.State.Exists() { - return wm, err + if err != nil { + return nil, err } - wm, err = instanceDomainPolicy(ctx, filter) - if err != nil || wm != nil { - return wm, err + if wm != nil && wm.State.Exists() { + return &wm.PolicyDomainWriteModel, err + } + instanceWriteModel, err := instanceDomainPolicy(ctx, filter) + if err != nil { + return nil, err + } + if instanceWriteModel != nil && instanceWriteModel.State.Exists() { + return &instanceWriteModel.PolicyDomainWriteModel, err } return nil, errors.ThrowInternal(nil, "USER-Ggk9n", "Errors.Internal") } -func orgDomainPolicy(ctx context.Context, filter preparation.FilterToQueryReducer) (*PolicyDomainWriteModel, error) { +func orgDomainPolicy(ctx context.Context, filter preparation.FilterToQueryReducer) (*OrgDomainPolicyWriteModel, error) { policy := NewOrgDomainPolicyWriteModel(authz.GetCtxData(ctx).OrgID) events, err := filter(ctx, policy.Query()) if err != nil { return nil, err } if len(events) == 0 { - return nil, nil + return policy, nil } policy.AppendEvents(events...) err = policy.Reduce() - return &policy.PolicyDomainWriteModel, err + return policy, err } -func instanceDomainPolicy(ctx context.Context, filter preparation.FilterToQueryReducer) (*PolicyDomainWriteModel, error) { +func instanceDomainPolicy(ctx context.Context, filter preparation.FilterToQueryReducer) (*InstanceDomainPolicyWriteModel, error) { policy := NewInstanceDomainPolicyWriteModel(ctx) events, err := filter(ctx, policy.Query()) if err != nil { return nil, err } if len(events) == 0 { - return nil, nil + return policy, nil } policy.AppendEvents(events...) err = policy.Reduce() - return &policy.PolicyDomainWriteModel, err + return policy, err +} + +func domainPolicyUsernames(ctx context.Context, filter preparation.FilterToQueryReducer, orgID string) (*DomainPolicyUsernamesWriteModel, error) { + policy := NewDomainPolicyUsernamesWriteModel(orgID) + events, err := filter(ctx, policy.Query()) + if err != nil { + return nil, err + } + if len(events) == 0 { + return policy, nil + } + policy.AppendEvents(events...) + err = policy.Reduce() + return policy, err +} + +func domainPolicyOrgs(ctx context.Context, filter preparation.FilterToQueryReducer) (*DomainPolicyOrgsWriteModel, error) { + policy := NewDomainPolicyOrgsWriteModel() + events, err := filter(ctx, policy.Query()) + if err != nil { + return nil, err + } + if len(events) == 0 { + return policy, nil + } + policy.AppendEvents(events...) + err = policy.Reduce() + return policy, err } diff --git a/internal/command/user_domain_policy_test.go b/internal/command/user_domain_policy_test.go index ed4a775729..9afe30e608 100644 --- a/internal/command/user_domain_policy_test.go +++ b/internal/command/user_domain_policy_test.go @@ -21,7 +21,7 @@ func Test_customDomainPolicy(t *testing.T) { tests := []struct { name string args args - want *PolicyDomainWriteModel + want *OrgDomainPolicyWriteModel wantErr bool }{ { @@ -41,7 +41,12 @@ func Test_customDomainPolicy(t *testing.T) { return []eventstore.Event{}, nil }, }, - want: nil, + want: &OrgDomainPolicyWriteModel{ + PolicyDomainWriteModel: PolicyDomainWriteModel{ + WriteModel: eventstore.WriteModel{}, + State: domain.PolicyStateUnspecified, + }, + }, wantErr: false, }, { @@ -59,16 +64,18 @@ func Test_customDomainPolicy(t *testing.T) { }, nil }, }, - want: &PolicyDomainWriteModel{ - WriteModel: eventstore.WriteModel{ - AggregateID: "id", - ResourceOwner: "id", - Events: []eventstore.Event{}, + want: &OrgDomainPolicyWriteModel{ + PolicyDomainWriteModel: PolicyDomainWriteModel{ + WriteModel: eventstore.WriteModel{ + AggregateID: "id", + ResourceOwner: "id", + Events: []eventstore.Event{}, + }, + UserLoginMustBeDomain: true, + ValidateOrgDomains: true, + SMTPSenderAddressMatchesInstanceDomain: true, + State: domain.PolicyStateActive, }, - UserLoginMustBeDomain: true, - ValidateOrgDomains: true, - SMTPSenderAddressMatchesInstanceDomain: true, - State: domain.PolicyStateActive, }, wantErr: false, }, @@ -94,7 +101,7 @@ func Test_defaultDomainPolicy(t *testing.T) { tests := []struct { name string args args - want *PolicyDomainWriteModel + want *InstanceDomainPolicyWriteModel wantErr bool }{ { @@ -114,7 +121,15 @@ func Test_defaultDomainPolicy(t *testing.T) { return []eventstore.Event{}, nil }, }, - want: nil, + want: &InstanceDomainPolicyWriteModel{ + PolicyDomainWriteModel: PolicyDomainWriteModel{ + WriteModel: eventstore.WriteModel{ + AggregateID: "INSTANCE", + ResourceOwner: "INSTANCE", + }, + State: domain.PolicyStateUnspecified, + }, + }, wantErr: false, }, { @@ -132,17 +147,19 @@ func Test_defaultDomainPolicy(t *testing.T) { }, nil }, }, - want: &PolicyDomainWriteModel{ - WriteModel: eventstore.WriteModel{ - AggregateID: "INSTANCE", - ResourceOwner: "INSTANCE", - Events: []eventstore.Event{}, - InstanceID: "INSTANCE", + want: &InstanceDomainPolicyWriteModel{ + PolicyDomainWriteModel: PolicyDomainWriteModel{ + WriteModel: eventstore.WriteModel{ + AggregateID: "INSTANCE", + ResourceOwner: "INSTANCE", + Events: []eventstore.Event{}, + InstanceID: "INSTANCE", + }, + UserLoginMustBeDomain: true, + ValidateOrgDomains: true, + SMTPSenderAddressMatchesInstanceDomain: true, + State: domain.PolicyStateActive, }, - UserLoginMustBeDomain: true, - ValidateOrgDomains: true, - SMTPSenderAddressMatchesInstanceDomain: true, - State: domain.PolicyStateActive, }, wantErr: false, }, diff --git a/internal/repository/user/user.go b/internal/repository/user/user.go index 97235d70ab..a35ebe8095 100644 --- a/internal/repository/user/user.go +++ b/internal/repository/user/user.go @@ -315,8 +315,8 @@ type DomainClaimedEvent struct { eventstore.BaseEvent `json:"-"` UserName string `json:"userName"` - oldUserName string `json:"-"` - userLoginMustBeDomain bool `json:"-"` + oldUserName string + userLoginMustBeDomain bool } func (e *DomainClaimedEvent) Data() interface{} { @@ -395,9 +395,10 @@ func DomainClaimedSentEventMapper(event *repository.Event) (eventstore.Event, er type UsernameChangedEvent struct { eventstore.BaseEvent `json:"-"` - UserName string `json:"userName"` - oldUserName string `json:"-"` - userLoginMustBeDomain bool `json:"-"` + UserName string `json:"userName"` + oldUserName string + userLoginMustBeDomain bool + oldUserLoginMustBeDomain bool } func (e *UsernameChangedEvent) Data() interface{} { @@ -406,7 +407,7 @@ func (e *UsernameChangedEvent) Data() interface{} { func (e *UsernameChangedEvent) UniqueConstraints() []*eventstore.EventUniqueConstraint { return []*eventstore.EventUniqueConstraint{ - NewRemoveUsernameUniqueConstraint(e.oldUserName, e.Aggregate().ResourceOwner, e.userLoginMustBeDomain), + NewRemoveUsernameUniqueConstraint(e.oldUserName, e.Aggregate().ResourceOwner, e.oldUserLoginMustBeDomain), NewAddUsernameUniqueConstraint(e.UserName, e.Aggregate().ResourceOwner, e.userLoginMustBeDomain), } } @@ -417,16 +418,32 @@ func NewUsernameChangedEvent( oldUserName, newUserName string, userLoginMustBeDomain bool, + opts ...UsernameChangedEventOption, ) *UsernameChangedEvent { - return &UsernameChangedEvent{ + event := &UsernameChangedEvent{ BaseEvent: *eventstore.NewBaseEventForPush( ctx, aggregate, UserUserNameChangedType, ), - UserName: newUserName, - oldUserName: oldUserName, - userLoginMustBeDomain: userLoginMustBeDomain, + UserName: newUserName, + oldUserName: oldUserName, + userLoginMustBeDomain: userLoginMustBeDomain, + oldUserLoginMustBeDomain: userLoginMustBeDomain, + } + for _, opt := range opts { + opt(event) + } + return event +} + +type UsernameChangedEventOption func(*UsernameChangedEvent) + +// UsernameChangedEventWithPolicyChange signals that the change occurs because of / during a domain policy change +// (will ensure the unique constraint change is handled correctly) +func UsernameChangedEventWithPolicyChange() UsernameChangedEventOption { + return func(e *UsernameChangedEvent) { + e.oldUserLoginMustBeDomain = !e.userLoginMustBeDomain } }