From cbd2ef0612a65720eed36a3843c1e7003b5d74a0 Mon Sep 17 00:00:00 2001 From: Livio Spring Date: Wed, 23 Aug 2023 10:04:29 +0200 Subject: [PATCH] fix: use system secret config if generator type does not exist on instance (#6420) * fix: use system secret config if generator type does not exist on instance * remove unused idGenerator --- cmd/setup/03.go | 1 + cmd/setup/config_change.go | 1 + cmd/start/start.go | 1 + internal/command/command.go | 21 ++-- internal/command/crypto.go | 23 ++++ internal/command/instance.go | 37 +++--- internal/command/user_human_otp.go | 5 +- internal/command/user_human_otp_test.go | 148 +++++++++++++++++++++--- 8 files changed, 195 insertions(+), 42 deletions(-) diff --git a/cmd/setup/03.go b/cmd/setup/03.go index 73d32fcab6..ffa4769a42 100644 --- a/cmd/setup/03.go +++ b/cmd/setup/03.go @@ -91,6 +91,7 @@ func (mig *FirstInstance) Execute(ctx context.Context) error { 0, 0, 0, + nil, ) if err != nil { return err diff --git a/cmd/setup/config_change.go b/cmd/setup/config_change.go index 28d66394c8..c6f107fcf6 100644 --- a/cmd/setup/config_change.go +++ b/cmd/setup/config_change.go @@ -57,6 +57,7 @@ func (mig *externalConfigChange) Execute(ctx context.Context) error { 0, 0, 0, + nil, ) if err != nil { diff --git a/cmd/start/start.go b/cmd/start/start.go index c8a5c20e32..1191e19b21 100644 --- a/cmd/start/start.go +++ b/cmd/start/start.go @@ -202,6 +202,7 @@ func startZitadel(config *Config, masterKey string, server chan<- *Server) error config.OIDC.DefaultAccessTokenLifetime, config.OIDC.DefaultRefreshTokenExpiration, config.OIDC.DefaultRefreshTokenIdleExpiration, + config.DefaultInstance.SecretGenerators, ) if err != nil { return fmt.Errorf("cannot start commands: %w", err) diff --git a/internal/command/command.go b/internal/command/command.go index 9a6a87cb75..5149652739 100644 --- a/internal/command/command.go +++ b/internal/command/command.go @@ -62,15 +62,16 @@ type Commands struct { defaultRefreshTokenLifetime time.Duration defaultRefreshTokenIdleLifetime time.Duration - multifactors domain.MultifactorConfigs - webauthnConfig *webauthn_helper.Config - keySize int - keyAlgorithm crypto.EncryptionAlgorithm - certificateAlgorithm crypto.EncryptionAlgorithm - certKeySize int - privateKeyLifetime time.Duration - publicKeyLifetime time.Duration - certificateLifetime time.Duration + multifactors domain.MultifactorConfigs + webauthnConfig *webauthn_helper.Config + keySize int + keyAlgorithm crypto.EncryptionAlgorithm + certificateAlgorithm crypto.EncryptionAlgorithm + certKeySize int + privateKeyLifetime time.Duration + publicKeyLifetime time.Duration + certificateLifetime time.Duration + defaultSecretGenerators *SecretGenerators } func StartCommands( @@ -89,6 +90,7 @@ func StartCommands( defaultAccessTokenLifetime, defaultRefreshTokenLifetime, defaultRefreshTokenIdleLifetime time.Duration, + defaultSecretGenerators *SecretGenerators, ) (repo *Commands, err error) { if externalDomain == "" { return nil, errors.ThrowInvalidArgument(nil, "COMMAND-Df21s", "no external domain specified") @@ -125,6 +127,7 @@ func StartCommands( defaultAccessTokenLifetime: defaultAccessTokenLifetime, defaultRefreshTokenLifetime: defaultRefreshTokenLifetime, defaultRefreshTokenIdleLifetime: defaultRefreshTokenIdleLifetime, + defaultSecretGenerators: defaultSecretGenerators, } instance_repo.RegisterEventMappers(repo.eventstore) diff --git a/internal/command/crypto.go b/internal/command/crypto.go index 8d145f2cc2..af9407e0a3 100644 --- a/internal/command/crypto.go +++ b/internal/command/crypto.go @@ -76,3 +76,26 @@ func secretGeneratorConfig(ctx context.Context, filter preparation.FilterToQuery IncludeSymbols: wm.IncludeSymbols, }, nil } + +func secretGeneratorConfigWithDefault(ctx context.Context, filter preparation.FilterToQueryReducer, typ domain.SecretGeneratorType, defaultGenerator *crypto.GeneratorConfig) (*crypto.GeneratorConfig, error) { + wm := NewInstanceSecretGeneratorConfigWriteModel(ctx, typ) + events, err := filter(ctx, wm.Query()) + if err != nil { + return nil, err + } + wm.AppendEvents(events...) + if err := wm.Reduce(); err != nil { + return nil, err + } + if wm.State != domain.SecretGeneratorStateActive { + return defaultGenerator, nil + } + return &crypto.GeneratorConfig{ + Length: wm.Length, + Expiry: wm.Expiry, + IncludeLowerLetters: wm.IncludeLowerLetters, + IncludeUpperLetters: wm.IncludeUpperLetters, + IncludeDigits: wm.IncludeDigits, + IncludeSymbols: wm.IncludeSymbols, + }, nil +} diff --git a/internal/command/instance.go b/internal/command/instance.go index e0a471fe93..40c035d3eb 100644 --- a/internal/command/instance.go +++ b/internal/command/instance.go @@ -33,24 +33,12 @@ const ( ) type InstanceSetup struct { - zitadel ZitadelConfig - idGenerator id.Generator - InstanceName string - CustomDomain string - DefaultLanguage language.Tag - Org InstanceOrgSetup - SecretGenerators struct { - PasswordSaltCost uint - ClientSecret *crypto.GeneratorConfig - InitializeUserCode *crypto.GeneratorConfig - EmailVerificationCode *crypto.GeneratorConfig - PhoneVerificationCode *crypto.GeneratorConfig - PasswordVerificationCode *crypto.GeneratorConfig - PasswordlessInitCode *crypto.GeneratorConfig - DomainVerification *crypto.GeneratorConfig - OTPSMS *crypto.GeneratorConfig - OTPEmail *crypto.GeneratorConfig - } + zitadel ZitadelConfig + InstanceName string + CustomDomain string + DefaultLanguage language.Tag + Org InstanceOrgSetup + SecretGenerators *SecretGenerators PasswordComplexityPolicy struct { MinLength uint64 HasLowercase bool @@ -126,6 +114,19 @@ type InstanceSetup struct { } } +type SecretGenerators struct { + PasswordSaltCost uint + ClientSecret *crypto.GeneratorConfig + InitializeUserCode *crypto.GeneratorConfig + EmailVerificationCode *crypto.GeneratorConfig + PhoneVerificationCode *crypto.GeneratorConfig + PasswordVerificationCode *crypto.GeneratorConfig + PasswordlessInitCode *crypto.GeneratorConfig + DomainVerification *crypto.GeneratorConfig + OTPSMS *crypto.GeneratorConfig + OTPEmail *crypto.GeneratorConfig +} + type ZitadelConfig struct { projectID string mgmtAppID string diff --git a/internal/command/user_human_otp.go b/internal/command/user_human_otp.go index c3a8e84f09..170dde17a7 100644 --- a/internal/command/user_human_otp.go +++ b/internal/command/user_human_otp.go @@ -278,6 +278,7 @@ func (c *Commands) HumanSendOTPSMS(ctx context.Context, userID, resourceOwner st authRequest, smsWriteModel, domain.SecretGeneratorTypeOTPSMS, + c.defaultSecretGenerators.OTPSMS, codeAddedEvent, ) } @@ -398,6 +399,7 @@ func (c *Commands) HumanSendOTPEmail(ctx context.Context, userID, resourceOwner authRequest, smsWriteModel, domain.SecretGeneratorTypeOTPEmail, + c.defaultSecretGenerators.OTPEmail, codeAddedEvent, ) } @@ -442,6 +444,7 @@ func (c *Commands) sendHumanOTP( authRequest *domain.AuthRequest, writeModelByID func(ctx context.Context, userID string, resourceOwner string) (OTPWriteModel, error), secretGeneratorType domain.SecretGeneratorType, + defaultSecretGenerator *crypto.GeneratorConfig, codeAddedEvent func(ctx context.Context, aggregate *eventstore.Aggregate, code *crypto.CryptoValue, expiry time.Duration, info *user.AuthRequestInfo) eventstore.Command, ) (err error) { if userID == "" { @@ -454,7 +457,7 @@ func (c *Commands) sendHumanOTP( if !existingOTP.OTPAdded() { return caos_errs.ThrowPreconditionFailed(nil, "COMMAND-SFD52", "Errors.User.MFA.OTP.NotReady") } - config, err := secretGeneratorConfig(ctx, c.eventstore.Filter, secretGeneratorType) + config, err := secretGeneratorConfigWithDefault(ctx, c.eventstore.Filter, secretGeneratorType, defaultSecretGenerator) if err != nil { return err } diff --git a/internal/command/user_human_otp_test.go b/internal/command/user_human_otp_test.go index 5ec52582a9..ba3a6dc7b9 100644 --- a/internal/command/user_human_otp_test.go +++ b/internal/command/user_human_otp_test.go @@ -1249,9 +1249,20 @@ func TestCommandSide_RemoveHumanOTPSMS(t *testing.T) { func TestCommandSide_HumanSendOTPSMS(t *testing.T) { ctx := authz.NewMockContext("inst1", "org1", "user1") + defaultGenerators := &SecretGenerators{ + OTPSMS: &crypto.GeneratorConfig{ + Length: 8, + Expiry: time.Hour, + IncludeLowerLetters: true, + IncludeUpperLetters: true, + IncludeDigits: true, + IncludeSymbols: true, + }, + } type fields struct { - eventstore func(*testing.T) *eventstore.Eventstore - userEncryption crypto.EncryptionAlgorithm + eventstore func(*testing.T) *eventstore.Eventstore + userEncryption crypto.EncryptionAlgorithm + defaultSecretGenerators *SecretGenerators } type ( args struct { @@ -1274,7 +1285,8 @@ func TestCommandSide_HumanSendOTPSMS(t *testing.T) { { name: "userid missing, invalid argument error", fields: fields{ - eventstore: expectEventstore(), + eventstore: expectEventstore(), + defaultSecretGenerators: defaultGenerators, }, args: args{ ctx: ctx, @@ -1291,6 +1303,7 @@ func TestCommandSide_HumanSendOTPSMS(t *testing.T) { eventstore: expectEventstore( expectFilter(), ), + defaultSecretGenerators: defaultGenerators, }, args: args{ ctx: ctx, @@ -1343,7 +1356,52 @@ func TestCommandSide_HumanSendOTPSMS(t *testing.T) { }, ), ), - userEncryption: crypto.CreateMockEncryptionAlgWithCode(gomock.NewController(t), "12345678"), + userEncryption: crypto.CreateMockEncryptionAlgWithCode(gomock.NewController(t), "12345678"), + defaultSecretGenerators: defaultGenerators, + }, + args: args{ + ctx: ctx, + userID: "user1", + resourceOwner: "org1", + }, + res: res{ + want: &domain.ObjectDetails{ + ResourceOwner: "org1", + }, + }, + }, + { + name: "successful add (without secret config)", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + user.NewHumanOTPSMSAddedEvent(ctx, + &user.NewAggregate("user1", "org1").Aggregate, + ), + ), + ), + expectFilter(), + expectPush( + []*repository.Event{ + eventFromEventPusherWithInstanceID("inst1", + user.NewHumanOTPSMSCodeAddedEvent(ctx, + &user.NewAggregate("user1", "org1").Aggregate, + &crypto.CryptoValue{ + CryptoType: crypto.TypeEncryption, + Algorithm: "enc", + KeyID: "id", + Crypted: []byte("12345678"), + }, + time.Hour, + nil, + ), + ), + }, + ), + ), + userEncryption: crypto.CreateMockEncryptionAlgWithCode(gomock.NewController(t), "12345678"), + defaultSecretGenerators: defaultGenerators, }, args: args{ ctx: ctx, @@ -1406,7 +1464,8 @@ func TestCommandSide_HumanSendOTPSMS(t *testing.T) { }, ), ), - userEncryption: crypto.CreateMockEncryptionAlgWithCode(gomock.NewController(t), "12345678"), + userEncryption: crypto.CreateMockEncryptionAlgWithCode(gomock.NewController(t), "12345678"), + defaultSecretGenerators: defaultGenerators, }, args: args{ ctx: ctx, @@ -1432,8 +1491,9 @@ func TestCommandSide_HumanSendOTPSMS(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { r := &Commands{ - eventstore: tt.fields.eventstore(t), - userEncryption: tt.fields.userEncryption, + eventstore: tt.fields.eventstore(t), + userEncryption: tt.fields.userEncryption, + defaultSecretGenerators: tt.fields.defaultSecretGenerators, } err := r.HumanSendOTPSMS(tt.args.ctx, tt.args.userID, tt.args.resourceOwner, tt.args.authRequest) assert.ErrorIs(t, err, tt.res.err) @@ -2176,9 +2236,20 @@ func TestCommandSide_RemoveHumanOTPEmail(t *testing.T) { func TestCommandSide_HumanSendOTPEmail(t *testing.T) { ctx := authz.NewMockContext("inst1", "org1", "user1") + defaultGenerators := &SecretGenerators{ + OTPEmail: &crypto.GeneratorConfig{ + Length: 8, + Expiry: time.Hour, + IncludeLowerLetters: true, + IncludeUpperLetters: true, + IncludeDigits: true, + IncludeSymbols: true, + }, + } type fields struct { - eventstore func(*testing.T) *eventstore.Eventstore - userEncryption crypto.EncryptionAlgorithm + eventstore func(*testing.T) *eventstore.Eventstore + userEncryption crypto.EncryptionAlgorithm + defaultSecretGenerators *SecretGenerators } type ( args struct { @@ -2201,7 +2272,8 @@ func TestCommandSide_HumanSendOTPEmail(t *testing.T) { { name: "userid missing, invalid argument error", fields: fields{ - eventstore: expectEventstore(), + eventstore: expectEventstore(), + defaultSecretGenerators: defaultGenerators, }, args: args{ ctx: ctx, @@ -2218,6 +2290,7 @@ func TestCommandSide_HumanSendOTPEmail(t *testing.T) { eventstore: expectEventstore( expectFilter(), ), + defaultSecretGenerators: defaultGenerators, }, args: args{ ctx: ctx, @@ -2270,7 +2343,52 @@ func TestCommandSide_HumanSendOTPEmail(t *testing.T) { }, ), ), - userEncryption: crypto.CreateMockEncryptionAlgWithCode(gomock.NewController(t), "12345678"), + userEncryption: crypto.CreateMockEncryptionAlgWithCode(gomock.NewController(t), "12345678"), + defaultSecretGenerators: defaultGenerators, + }, + args: args{ + ctx: ctx, + userID: "user1", + resourceOwner: "org1", + }, + res: res{ + want: &domain.ObjectDetails{ + ResourceOwner: "org1", + }, + }, + }, + { + name: "successful add (without secret config)", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + user.NewHumanOTPEmailAddedEvent(ctx, + &user.NewAggregate("user1", "org1").Aggregate, + ), + ), + ), + expectFilter(), + expectPush( + []*repository.Event{ + eventFromEventPusherWithInstanceID("inst1", + user.NewHumanOTPEmailCodeAddedEvent(ctx, + &user.NewAggregate("user1", "org1").Aggregate, + &crypto.CryptoValue{ + CryptoType: crypto.TypeEncryption, + Algorithm: "enc", + KeyID: "id", + Crypted: []byte("12345678"), + }, + time.Hour, + nil, + ), + ), + }, + ), + ), + userEncryption: crypto.CreateMockEncryptionAlgWithCode(gomock.NewController(t), "12345678"), + defaultSecretGenerators: defaultGenerators, }, args: args{ ctx: ctx, @@ -2333,7 +2451,8 @@ func TestCommandSide_HumanSendOTPEmail(t *testing.T) { }, ), ), - userEncryption: crypto.CreateMockEncryptionAlgWithCode(gomock.NewController(t), "12345678"), + userEncryption: crypto.CreateMockEncryptionAlgWithCode(gomock.NewController(t), "12345678"), + defaultSecretGenerators: defaultGenerators, }, args: args{ ctx: ctx, @@ -2359,8 +2478,9 @@ func TestCommandSide_HumanSendOTPEmail(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { r := &Commands{ - eventstore: tt.fields.eventstore(t), - userEncryption: tt.fields.userEncryption, + eventstore: tt.fields.eventstore(t), + userEncryption: tt.fields.userEncryption, + defaultSecretGenerators: tt.fields.defaultSecretGenerators, } err := r.HumanSendOTPEmail(tt.args.ctx, tt.args.userID, tt.args.resourceOwner, tt.args.authRequest) assert.ErrorIs(t, err, tt.res.err)