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
2025-08-12 01:37:31 +00:00 · 2022-12-06 09:01:31 +01:00
parent 97fe041a86
commit 3539418a4a
12 changed files with 1042 additions and 332 deletions
--- 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
+	}
+}