fix: correct oidcsettings management (#4413)

* fix(oidcsettings): corrected projection, unittests and added the add endpoint

* fix(oidcsettings): corrected default handling and instance setup

* fix: set oidc settings correctly in console

* cleanup

* e2e test

* improve e2e test

* lint e2e

Co-authored-by: Livio Spring <livio.a@gmail.com>
Co-authored-by: Fabi <38692350+hifabienne@users.noreply.github.com>

internal/command/instance.go

@@ -104,6 +104,12 @@ type InstanceSetup struct {
 	EmailTemplate     []byte
 	MessageTexts      []*domain.CustomMessageText
 	SMTPConfiguration *smtp.EmailConfig
+	OIDCSettings      *struct {
+		AccessTokenLifetime        time.Duration
+		IdTokenLifetime            time.Duration
+		RefreshTokenIdleExpiration time.Duration
+		RefreshTokenExpiration     time.Duration
+	}
 }
 
 type ZitadelConfig struct {
@@ -346,6 +352,18 @@ func (c *Commands) SetUpInstance(ctx context.Context, setup *InstanceSetup) (str
 		)
 	}
 
+	if setup.OIDCSettings != nil {
+		validations = append(validations,
+			c.prepareAddOIDCSettings(
+				instanceAgg,
+				setup.OIDCSettings.AccessTokenLifetime,
+				setup.OIDCSettings.IdTokenLifetime,
+				setup.OIDCSettings.RefreshTokenIdleExpiration,
+				setup.OIDCSettings.RefreshTokenExpiration,
+			),
+		)
+	}
+
 	cmds, err := preparation.PrepareCommands(ctx, c.eventstore.Filter, validations...)
 	if err != nil {
 		return "", nil, err

internal/command/instance_oidc_settings.go

@@ -2,78 +2,131 @@ package command
 
 import (
 	"context"
+	"time"
 
+	"github.com/zitadel/zitadel/internal/api/authz"
+	"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/errors"
+	"github.com/zitadel/zitadel/internal/eventstore"
 	"github.com/zitadel/zitadel/internal/repository/instance"
 )
 
+func (c *Commands) prepareAddOIDCSettings(a *instance.Aggregate, accessTokenLifetime, idTokenLifetime, refreshTokenIdleExpiration, refreshTokenExpiration time.Duration) preparation.Validation {
+	return func() (preparation.CreateCommands, error) {
+		if accessTokenLifetime == time.Duration(0) ||
+			idTokenLifetime == time.Duration(0) ||
+			refreshTokenIdleExpiration == time.Duration(0) ||
+			refreshTokenExpiration == time.Duration(0) {
+			return nil, errors.ThrowInvalidArgument(nil, "INST-10s82j", "Errors.Invalid.Argument")
+		}
+
+		return func(ctx context.Context, filter preparation.FilterToQueryReducer) ([]eventstore.Command, error) {
+			writeModel, err := c.getOIDCSettingsWriteModel(ctx, filter)
+			if err != nil {
+				return nil, err
+			}
+			if writeModel.State == domain.OIDCSettingsStateActive {
+				return nil, errors.ThrowAlreadyExists(nil, "INST-0aaj1o", "Errors.OIDCSettings.AlreadyExists")
+			}
+			return []eventstore.Command{
+				instance.NewOIDCSettingsAddedEvent(
+					ctx,
+					&a.Aggregate,
+					accessTokenLifetime,
+					idTokenLifetime,
+					refreshTokenIdleExpiration,
+					refreshTokenExpiration,
+				),
+			}, nil
+		}, nil
+	}
+}
+
+func (c *Commands) prepareUpdateOIDCSettings(a *instance.Aggregate, accessTokenLifetime, idTokenLifetime, refreshTokenIdleExpiration, refreshTokenExpiration time.Duration) preparation.Validation {
+	return func() (preparation.CreateCommands, error) {
+		if accessTokenLifetime == time.Duration(0) ||
+			idTokenLifetime == time.Duration(0) ||
+			refreshTokenIdleExpiration == time.Duration(0) ||
+			refreshTokenExpiration == time.Duration(0) {
+			return nil, errors.ThrowInvalidArgument(nil, "INST-10sxks", "Errors.Invalid.Argument")
+		}
+
+		return func(ctx context.Context, filter preparation.FilterToQueryReducer) ([]eventstore.Command, error) {
+			writeModel, err := c.getOIDCSettingsWriteModel(ctx, filter)
+			if err != nil {
+				return nil, err
+			}
+			if writeModel.State != domain.OIDCSettingsStateActive {
+				return nil, errors.ThrowNotFound(nil, "INST-90s32oj", "Errors.OIDCSettings.NotFound")
+			}
+			changedEvent, hasChanged, err := writeModel.NewChangedEvent(
+				ctx,
+				&a.Aggregate,
+				accessTokenLifetime,
+				idTokenLifetime,
+				refreshTokenIdleExpiration,
+				refreshTokenExpiration,
+			)
+			if err != nil {
+				return nil, err
+			}
+			if !hasChanged {
+				return nil, errors.ThrowPreconditionFailed(nil, "COMMAND-0pk2nu", "Errors.NoChangesFound")
+			}
+			return []eventstore.Command{
+				changedEvent,
+			}, nil
+		}, nil
+	}
+}
+
 func (c *Commands) AddOIDCSettings(ctx context.Context, settings *domain.OIDCSettings) (*domain.ObjectDetails, error) {
-	oidcSettingWriteModel, err := c.getOIDCSettings(ctx)
+	instanceAgg := instance.NewAggregate(authz.GetInstance(ctx).InstanceID())
+	validation := c.prepareAddOIDCSettings(instanceAgg, settings.AccessTokenLifetime, settings.IdTokenLifetime, settings.RefreshTokenIdleExpiration, settings.RefreshTokenExpiration)
+	cmds, err := preparation.PrepareCommands(ctx, c.eventstore.Filter, validation)
 	if err != nil {
 		return nil, err
 	}
-	if oidcSettingWriteModel.State.Exists() {
-		return nil, caos_errs.ThrowAlreadyExists(nil, "COMMAND-d9nlw", "Errors.OIDCSettings.AlreadyExists")
-	}
-	instanceAgg := InstanceAggregateFromWriteModel(&oidcSettingWriteModel.WriteModel)
-	pushedEvents, err := c.eventstore.Push(ctx, instance.NewOIDCSettingsAddedEvent(
-		ctx,
-		instanceAgg,
-		settings.AccessTokenLifetime,
-		settings.IdTokenLifetime,
-		settings.RefreshTokenIdleExpiration,
-		settings.RefreshTokenExpiration))
+	events, err := c.eventstore.Push(ctx, cmds...)
 	if err != nil {
 		return nil, err
 	}
-	err = AppendAndReduce(oidcSettingWriteModel, pushedEvents...)
-	if err != nil {
-		return nil, err
-	}
-	return writeModelToObjectDetails(&oidcSettingWriteModel.WriteModel), nil
+	return &domain.ObjectDetails{
+		Sequence:      events[len(events)-1].Sequence(),
+		EventDate:     events[len(events)-1].CreationDate(),
+		ResourceOwner: events[len(events)-1].Aggregate().InstanceID,
+	}, nil
 }
 
 func (c *Commands) ChangeOIDCSettings(ctx context.Context, settings *domain.OIDCSettings) (*domain.ObjectDetails, error) {
-	oidcSettingWriteModel, err := c.getOIDCSettings(ctx)
+	instanceAgg := instance.NewAggregate(authz.GetInstance(ctx).InstanceID())
+	validation := c.prepareUpdateOIDCSettings(instanceAgg, settings.AccessTokenLifetime, settings.IdTokenLifetime, settings.RefreshTokenIdleExpiration, settings.RefreshTokenExpiration)
+	cmds, err := preparation.PrepareCommands(ctx, c.eventstore.Filter, validation)
 	if err != nil {
 		return nil, err
 	}
-	if !oidcSettingWriteModel.State.Exists() {
-		return nil, caos_errs.ThrowNotFound(nil, "COMMAND-8snEr", "Errors.OIDCSettings.NotFound")
-	}
-	instanceAgg := InstanceAggregateFromWriteModel(&oidcSettingWriteModel.WriteModel)
-
-	changedEvent, hasChanged, err := oidcSettingWriteModel.NewChangedEvent(
-		ctx,
-		instanceAgg,
-		settings.AccessTokenLifetime,
-		settings.IdTokenLifetime,
-		settings.RefreshTokenIdleExpiration,
-		settings.RefreshTokenExpiration)
+	events, err := c.eventstore.Push(ctx, cmds...)
 	if err != nil {
 		return nil, err
 	}
-	if !hasChanged {
-		return nil, caos_errs.ThrowPreconditionFailed(nil, "COMMAND-398uF", "Errors.NoChangesFound")
-	}
-	pushedEvents, err := c.eventstore.Push(ctx, changedEvent)
-	if err != nil {
-		return nil, err
-	}
-	err = AppendAndReduce(oidcSettingWriteModel, pushedEvents...)
-	if err != nil {
-		return nil, err
-	}
-	return writeModelToObjectDetails(&oidcSettingWriteModel.WriteModel), nil
+	return &domain.ObjectDetails{
+		Sequence:      events[len(events)-1].Sequence(),
+		EventDate:     events[len(events)-1].CreationDate(),
+		ResourceOwner: events[len(events)-1].Aggregate().InstanceID,
+	}, nil
 }
 
-func (c *Commands) getOIDCSettings(ctx context.Context) (_ *InstanceOIDCSettingsWriteModel, err error) {
+func (c *Commands) getOIDCSettingsWriteModel(ctx context.Context, filter preparation.FilterToQueryReducer) (_ *InstanceOIDCSettingsWriteModel, err error) {
 	writeModel := NewInstanceOIDCSettingsWriteModel(ctx)
-	err = c.eventstore.FilterToQueryReducer(ctx, writeModel)
+	events, err := filter(ctx, writeModel.Query())
 	if err != nil {
 		return nil, err
 	}
-
-	return writeModel, nil
+	if len(events) == 0 {
+		return writeModel, nil
+	}
+	writeModel.AppendEvents(events...)
+	err = writeModel.Reduce()
+	return writeModel, err
 }

internal/command/instance_oidc_settings_test.go

@@ -6,6 +6,7 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/assert"
+
 	"github.com/zitadel/zitadel/internal/api/authz"
 
 	"github.com/zitadel/zitadel/internal/domain"
@@ -34,7 +35,7 @@ func TestCommandSide_AddOIDCConfig(t *testing.T) {
 		res    res
 	}{
 		{
-			name: "oidc config, error already exists",
+			name: "oidc settings, error already exists",
 			fields: fields{
 				eventstore: eventstoreExpect(
 					t,
@@ -52,7 +53,7 @@ func TestCommandSide_AddOIDCConfig(t *testing.T) {
 				),
 			},
 			args: args{
-				ctx: context.Background(),
+				ctx: authz.WithInstanceID(context.Background(), "INSTANCE"),
 				oidcConfig: &domain.OIDCSettings{
 					AccessTokenLifetime:        1 * time.Hour,
 					IdTokenLifetime:            1 * time.Hour,
@@ -65,7 +66,7 @@ func TestCommandSide_AddOIDCConfig(t *testing.T) {
 			},
 		},
 		{
-			name: "add secret generator, ok",
+			name: "add oidc settings, ok",
 			fields: fields{
 				eventstore: eventstoreExpect(
 					t,
@@ -102,6 +103,86 @@ func TestCommandSide_AddOIDCConfig(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "add oidc settings, invalid argument 1",
+			fields: fields{
+				eventstore: eventstoreExpect(
+					t,
+				),
+			},
+			args: args{
+				ctx: authz.WithInstanceID(context.Background(), "INSTANCE"),
+				oidcConfig: &domain.OIDCSettings{
+					AccessTokenLifetime:        0 * time.Hour,
+					IdTokenLifetime:            1 * time.Hour,
+					RefreshTokenIdleExpiration: 1 * time.Hour,
+					RefreshTokenExpiration:     1 * time.Hour,
+				},
+			},
+			res: res{
+				err: caos_errs.IsErrorInvalidArgument,
+			},
+		},
+		{
+			name: "add oidc settings, invalid argument 2",
+			fields: fields{
+				eventstore: eventstoreExpect(
+					t,
+				),
+			},
+			args: args{
+				ctx: authz.WithInstanceID(context.Background(), "INSTANCE"),
+				oidcConfig: &domain.OIDCSettings{
+					AccessTokenLifetime:        1 * time.Hour,
+					IdTokenLifetime:            0 * time.Hour,
+					RefreshTokenIdleExpiration: 1 * time.Hour,
+					RefreshTokenExpiration:     1 * time.Hour,
+				},
+			},
+			res: res{
+				err: caos_errs.IsErrorInvalidArgument,
+			},
+		},
+		{
+			name: "add oidc settings, invalid argument 3",
+			fields: fields{
+				eventstore: eventstoreExpect(
+					t,
+				),
+			},
+			args: args{
+				ctx: authz.WithInstanceID(context.Background(), "INSTANCE"),
+				oidcConfig: &domain.OIDCSettings{
+					AccessTokenLifetime:        1 * time.Hour,
+					IdTokenLifetime:            1 * time.Hour,
+					RefreshTokenIdleExpiration: 0 * time.Hour,
+					RefreshTokenExpiration:     1 * time.Hour,
+				},
+			},
+			res: res{
+				err: caos_errs.IsErrorInvalidArgument,
+			},
+		},
+		{
+			name: "add oidc settings, invalid argument 4",
+			fields: fields{
+				eventstore: eventstoreExpect(
+					t,
+				),
+			},
+			args: args{
+				ctx: authz.WithInstanceID(context.Background(), "INSTANCE"),
+				oidcConfig: &domain.OIDCSettings{
+					AccessTokenLifetime:        1 * time.Hour,
+					IdTokenLifetime:            1 * time.Hour,
+					RefreshTokenIdleExpiration: 1 * time.Hour,
+					RefreshTokenExpiration:     0 * time.Hour,
+				},
+			},
+			res: res{
+				err: caos_errs.IsErrorInvalidArgument,
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -141,7 +222,7 @@ func TestCommandSide_ChangeOIDCConfig(t *testing.T) {
 		res    res
 	}{
 		{
-			name: "oidc config not existing, not found error",
+			name: "oidc settings not existing, not found error",
 			fields: fields{
 				eventstore: eventstoreExpect(
 					t,
@@ -150,11 +231,97 @@ func TestCommandSide_ChangeOIDCConfig(t *testing.T) {
 			},
 			args: args{
 				ctx: context.Background(),
+				oidcConfig: &domain.OIDCSettings{
+					AccessTokenLifetime:        1 * time.Hour,
+					IdTokenLifetime:            1 * time.Hour,
+					RefreshTokenIdleExpiration: 1 * time.Hour,
+					RefreshTokenExpiration:     1 * time.Hour,
+				},
 			},
 			res: res{
 				err: caos_errs.IsNotFound,
 			},
 		},
+		{
+			name: "no changes, invalid argument error 1",
+			fields: fields{
+				eventstore: eventstoreExpect(
+					t,
+				),
+			},
+			args: args{
+				ctx: authz.WithInstanceID(context.Background(), "INSTANCE"),
+				oidcConfig: &domain.OIDCSettings{
+					AccessTokenLifetime:        0 * time.Hour,
+					IdTokenLifetime:            1 * time.Hour,
+					RefreshTokenIdleExpiration: 1 * time.Hour,
+					RefreshTokenExpiration:     1 * time.Hour,
+				},
+			},
+			res: res{
+				err: caos_errs.IsErrorInvalidArgument,
+			},
+		},
+		{
+			name: "no changes, invalid argument error 2",
+			fields: fields{
+				eventstore: eventstoreExpect(
+					t,
+				),
+			},
+			args: args{
+				ctx: authz.WithInstanceID(context.Background(), "INSTANCE"),
+				oidcConfig: &domain.OIDCSettings{
+					AccessTokenLifetime:        1 * time.Hour,
+					IdTokenLifetime:            0 * time.Hour,
+					RefreshTokenIdleExpiration: 1 * time.Hour,
+					RefreshTokenExpiration:     1 * time.Hour,
+				},
+			},
+			res: res{
+				err: caos_errs.IsErrorInvalidArgument,
+			},
+		},
+		{
+			name: "no changes, invalid argument error 3",
+			fields: fields{
+				eventstore: eventstoreExpect(
+					t,
+				),
+			},
+			args: args{
+				ctx: authz.WithInstanceID(context.Background(), "INSTANCE"),
+				oidcConfig: &domain.OIDCSettings{
+					AccessTokenLifetime:        1 * time.Hour,
+					IdTokenLifetime:            1 * time.Hour,
+					RefreshTokenIdleExpiration: 0 * time.Hour,
+					RefreshTokenExpiration:     1 * time.Hour,
+				},
+			},
+			res: res{
+				err: caos_errs.IsErrorInvalidArgument,
+			},
+		},
+		{
+			name: "no changes, invalid argument error 4",
+			fields: fields{
+				eventstore: eventstoreExpect(
+					t,
+				),
+			},
+			args: args{
+				ctx: authz.WithInstanceID(context.Background(), "INSTANCE"),
+				oidcConfig: &domain.OIDCSettings{
+					AccessTokenLifetime:        1 * time.Hour,
+					IdTokenLifetime:            1 * time.Hour,
+					RefreshTokenIdleExpiration: 1 * time.Hour,
+					RefreshTokenExpiration:     0 * time.Hour,
+				},
+			},
+			res: res{
+				err: caos_errs.IsErrorInvalidArgument,
+			},
+		},
 		{
 			name: "no changes, precondition error",
 			fields: fields{
@@ -175,7 +342,7 @@ func TestCommandSide_ChangeOIDCConfig(t *testing.T) {
 				),
 			},
 			args: args{
-				ctx: context.Background(),
+				ctx: authz.WithInstanceID(context.Background(), "INSTANCE"),
 				oidcConfig: &domain.OIDCSettings{
 					AccessTokenLifetime:        1 * time.Hour,
 					IdTokenLifetime:            1 * time.Hour,
@@ -188,7 +355,7 @@ func TestCommandSide_ChangeOIDCConfig(t *testing.T) {
 			},
 		},
 		{
-			name: "secret generator change, ok",
+			name: "oidc settings change, ok",
 			fields: fields{
 				eventstore: eventstoreExpect(
 					t,
@@ -206,8 +373,9 @@ func TestCommandSide_ChangeOIDCConfig(t *testing.T) {
 					),
 					expectPush(
 						[]*repository.Event{
-							eventFromEventPusher(
-								newOIDCConfigChangedEvent(context.Background(),
+							eventFromEventPusherWithInstanceID("INSTANCE",
+								newOIDCConfigChangedEvent(
+									context.Background(),
 									time.Hour*2,
 									time.Hour*2,
 									time.Hour*2,
@@ -218,7 +386,7 @@ func TestCommandSide_ChangeOIDCConfig(t *testing.T) {
 				),
 			},
 			args: args{
-				ctx: context.Background(),
+				ctx: authz.WithInstanceID(context.Background(), "INSTANCE"),
 				oidcConfig: &domain.OIDCSettings{
 					AccessTokenLifetime:        2 * time.Hour,
 					IdTokenLifetime:            2 * time.Hour,

internal/command/smtp.go

@@ -9,7 +9,6 @@ import (
 	"github.com/zitadel/zitadel/internal/crypto"
 	"github.com/zitadel/zitadel/internal/domain"
 	"github.com/zitadel/zitadel/internal/errors"
-	caos_errs "github.com/zitadel/zitadel/internal/errors"
 	"github.com/zitadel/zitadel/internal/eventstore"
 	"github.com/zitadel/zitadel/internal/notification/channels/smtp"
 	"github.com/zitadel/zitadel/internal/repository/instance"
@@ -58,7 +57,7 @@ func (c *Commands) ChangeSMTPConfigPassword(ctx context.Context, password string
 		return nil, err
 	}
 	if smtpConfigWriteModel.State != domain.SMTPConfigStateActive {
-		return nil, caos_errs.ThrowNotFound(nil, "COMMAND-3n9ls", "Errors.SMTPConfig.NotFound")
+		return nil, errors.ThrowNotFound(nil, "COMMAND-3n9ls", "Errors.SMTPConfig.NotFound")
 	}
 	var smtpPassword *crypto.CryptoValue
 	if password != "" {
@@ -180,7 +179,7 @@ func (c *Commands) prepareChangeSMTPConfig(a *instance.Aggregate, from, name, ho
 				return nil, err
 			}
 			if !hasChanged {
-				return nil, caos_errs.ThrowPreconditionFailed(nil, "COMMAND-m0o3f", "Errors.NoChangesFound")
+				return nil, errors.ThrowPreconditionFailed(nil, "COMMAND-m0o3f", "Errors.NoChangesFound")
 			}
 			return []eventstore.Command{
 				changedEvent,