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

@@ -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,
-	}
-}

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()})
 			}

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)
-	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)
+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
 	}
-	err = AppendAndReduce(existingPolicy, pushedEvents...)
+	pushedEvents, err := c.eventstore.Push(ctx, cmds...)
 	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
+	}
+}

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
+	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{},
 	}
-	return changedEvent, true
+}
+
+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()
 }