diff --git a/driver/registry_default_registration.go b/driver/registry_default_registration.go index 89ed5e656c74..ec248d8451c8 100644 --- a/driver/registry_default_registration.go +++ b/driver/registry_default_registration.go @@ -26,19 +26,13 @@ func (m *RegistryDefault) PostRegistrationPrePersistHooks(ctx context.Context, c } func (m *RegistryDefault) PostRegistrationPostPersistHooks(ctx context.Context, credentialsType identity.CredentialsType) (b []registration.PostHookPostPersistExecutor) { - initialHookCount := 0 - if m.Config().SelfServiceFlowVerificationEnabled(ctx) { - b = append(b, m.HookVerifier()) - initialHookCount = 1 - } - for _, v := range m.getHooks(string(credentialsType), m.Config().SelfServiceFlowRegistrationAfterHooks(ctx, string(credentialsType))) { if hook, ok := v.(registration.PostHookPostPersistExecutor); ok { b = append(b, hook) } } - if len(b) == initialHookCount { + if len(b) == 0 { // since we don't want merging hooks defined in a specific strategy and // global hooks are added only if no strategy specific hooks are defined for _, v := range m.getHooks(config.HookGlobal, m.Config().SelfServiceFlowRegistrationAfterHooks(ctx, config.HookGlobal)) { diff --git a/driver/registry_default_settings.go b/driver/registry_default_settings.go index 5afba50d38ac..549334fdd11c 100644 --- a/driver/registry_default_settings.go +++ b/driver/registry_default_settings.go @@ -29,19 +29,13 @@ func (m *RegistryDefault) PreSettingsHooks(ctx context.Context) (b []settings.Pr } func (m *RegistryDefault) PostSettingsPostPersistHooks(ctx context.Context, settingsType string) (b []settings.PostHookPostPersistExecutor) { - initialHookCount := 0 - if m.Config().SelfServiceFlowVerificationEnabled(ctx) { - b = append(b, m.HookVerifier()) - initialHookCount = 1 - } - for _, v := range m.getHooks(settingsType, m.Config().SelfServiceFlowSettingsAfterHooks(ctx, settingsType)) { if hook, ok := v.(settings.PostHookPostPersistExecutor); ok { b = append(b, hook) } } - if len(b) == initialHookCount { + if len(b) == 0 { // since we don't want merging hooks defined in a specific strategy and global hooks // global hooks are added only if no strategy specific hooks are defined for _, v := range m.getHooks(config.HookGlobal, m.Config().SelfServiceFlowSettingsAfterHooks(ctx, config.HookGlobal)) { diff --git a/driver/registry_default_test.go b/driver/registry_default_test.go index a52b4fc6072c..1763cef8757e 100644 --- a/driver/registry_default_test.go +++ b/driver/registry_default_test.go @@ -263,11 +263,24 @@ func TestDriverDefault_Hooks(t *testing.T) { { uc: "Only session hook configured for password strategy", config: map[string]any{ - config.ViperKeySelfServiceVerificationEnabled: true, config.ViperKeySelfServiceRegistrationAfter + ".password.hooks": []map[string]any{ {"hook": "session"}, }, }, + expect: func(reg *driver.RegistryDefault) []registration.PostHookPostPersistExecutor { + return []registration.PostHookPostPersistExecutor{ + hook.NewSessionIssuer(reg), + } + }, + }, + { + uc: "Session hook and verification hook configured for password strategy", + config: map[string]any{ + config.ViperKeySelfServiceRegistrationAfter + ".password.hooks": []map[string]any{ + {"hook": "verification"}, + {"hook": "session"}, + }, + }, expect: func(reg *driver.RegistryDefault) []registration.PostHookPostPersistExecutor { return []registration.PostHookPostPersistExecutor{ hook.NewVerifier(reg), @@ -278,7 +291,6 @@ func TestDriverDefault_Hooks(t *testing.T) { { uc: "A session hook and a web_hook are configured for password strategy", config: map[string]any{ - config.ViperKeySelfServiceVerificationEnabled: true, config.ViperKeySelfServiceRegistrationAfter + ".password.hooks": []map[string]any{ {"hook": "web_hook", "config": map[string]any{"headers": map[string]string{"X-Custom-Header": "test"}, "url": "foo", "method": "POST", "body": "bar"}}, {"hook": "session"}, @@ -286,7 +298,6 @@ func TestDriverDefault_Hooks(t *testing.T) { }, expect: func(reg *driver.RegistryDefault) []registration.PostHookPostPersistExecutor { return []registration.PostHookPostPersistExecutor{ - hook.NewVerifier(reg), hook.NewWebHook(reg, json.RawMessage(`{"body":"bar","headers":{"X-Custom-Header":"test"},"method":"POST","url":"foo"}`)), hook.NewSessionIssuer(reg), } @@ -317,11 +328,9 @@ func TestDriverDefault_Hooks(t *testing.T) { config.ViperKeySelfServiceRegistrationAfter + ".hooks": []map[string]any{ {"hook": "web_hook", "config": map[string]any{"url": "bar", "method": "POST", "headers": map[string]string{"X-Custom-Header": "test"}}}, }, - config.ViperKeySelfServiceVerificationEnabled: true, }, expect: func(reg *driver.RegistryDefault) []registration.PostHookPostPersistExecutor { return []registration.PostHookPostPersistExecutor{ - hook.NewVerifier(reg), hook.NewWebHook(reg, json.RawMessage(`{"headers":{"X-Custom-Header":"test"},"method":"GET","url":"foo"}`)), hook.NewSessionIssuer(reg), } @@ -553,7 +562,9 @@ func TestDriverDefault_Hooks(t *testing.T) { { uc: "Only verify hook configured for the strategy", config: map[string]any{ - config.ViperKeySelfServiceVerificationEnabled: true, + config.ViperKeySelfServiceSettingsAfter + ".profile.hooks": []map[string]any{ + {"hook": "verification"}, + }, // I think this is a bug as there is a hook named verify defined for both profile and password // strategies. Instead of using it, the code makes use of the property used above and which // is defined in an entirely different flow (verification). @@ -570,11 +581,9 @@ func TestDriverDefault_Hooks(t *testing.T) { config.ViperKeySelfServiceSettingsAfter + ".profile.hooks": []map[string]any{ {"hook": "web_hook", "config": map[string]any{"headers": []map[string]string{{"X-Custom-Header": "test"}}, "url": "foo", "method": "POST", "body": "bar"}}, }, - config.ViperKeySelfServiceVerificationEnabled: true, }, expect: func(reg *driver.RegistryDefault) []settings.PostHookPostPersistExecutor { return []settings.PostHookPostPersistExecutor{ - hook.NewVerifier(reg), hook.NewWebHook(reg, json.RawMessage(`{"body":"bar","headers":[{"X-Custom-Header":"test"}],"method":"POST","url":"foo"}`)), } }, @@ -597,7 +606,6 @@ func TestDriverDefault_Hooks(t *testing.T) { { uc: "Hooks are configured on a global level, as well as on a strategy level", config: map[string]any{ - config.ViperKeySelfServiceVerificationEnabled: true, config.ViperKeySelfServiceSettingsAfter + ".profile.hooks": []map[string]any{ {"hook": "web_hook", "config": map[string]any{"url": "foo", "method": "GET", "headers": map[string]string{"X-Custom-Header": "test"}}}, }, @@ -607,7 +615,6 @@ func TestDriverDefault_Hooks(t *testing.T) { }, expect: func(reg *driver.RegistryDefault) []settings.PostHookPostPersistExecutor { return []settings.PostHookPostPersistExecutor{ - hook.NewVerifier(reg), hook.NewWebHook(reg, json.RawMessage(`{"headers":{"X-Custom-Header":"test"},"method":"GET","url":"foo"}`)), } }, diff --git a/selfservice/flow/registration/hook_test.go b/selfservice/flow/registration/hook_test.go index 9a65b05a0eeb..6717d9f795d9 100644 --- a/selfservice/flow/registration/hook_test.go +++ b/selfservice/flow/registration/hook_test.go @@ -187,9 +187,12 @@ func TestRegistrationExecutor(t *testing.T) { t.Run("case=should redirect to verification UI if show_verification_ui hook is set", func(t *testing.T) { verificationTS := testhelpers.NewVerificationUIFlowEchoServer(t, reg) t.Cleanup(testhelpers.SelfServiceHookConfigReset(t, conf)) - conf.MustSet(ctx, config.ViperKeySelfServiceVerificationEnabled, true) conf.MustSet(ctx, config.ViperKeySelfServiceRegistrationAfter+".hooks", []map[string]interface{}{ { + "hook": hook.KeyVerifier, + }, + { + "hook": hook.KeyVerificationUI, }, }) @@ -205,10 +208,14 @@ func TestRegistrationExecutor(t *testing.T) { t.Run("case=should redirect to verification UI if there is a login_challenge", func(t *testing.T) { verificationTS := testhelpers.NewVerificationUIFlowEchoServer(t, reg) t.Cleanup(testhelpers.SelfServiceHookConfigReset(t, conf)) - conf.MustSet(ctx, config.ViperKeySelfServiceVerificationEnabled, true) - conf.MustSet(ctx, config.ViperKeySelfServiceRegistrationAfter+".hooks", []map[string]interface{}{{ - "hook": hook.KeyVerificationUI, - }}) + conf.MustSet(ctx, config.ViperKeySelfServiceRegistrationAfter+".hooks", []map[string]interface{}{ + { + "hook": hook.KeyVerifier, + }, + { + "hook": hook.KeyVerificationUI, + }, + }) i := testhelpers.SelfServiceHookFakeIdentity(t) i.Traits = identity.Traits(`{"email": "verifiable-valid-login_challenge@ory.sh"}`) @@ -228,8 +235,10 @@ func TestRegistrationExecutor(t *testing.T) { t.Run("case=should redirect to first verification UI if show_verification_ui hook is set and multiple verifiable addresses", func(t *testing.T) { verificationTS := testhelpers.NewVerificationUIFlowEchoServer(t, reg) t.Cleanup(testhelpers.SelfServiceHookConfigReset(t, conf)) - conf.MustSet(ctx, config.ViperKeySelfServiceVerificationEnabled, true) conf.MustSet(ctx, config.ViperKeySelfServiceRegistrationAfter+".hooks", []map[string]interface{}{ + { + "hook": hook.KeyVerifier, + }, { "hook": hook.KeyVerificationUI, }, @@ -248,8 +257,10 @@ func TestRegistrationExecutor(t *testing.T) { t.Run("case=should still sent session if show_verification_ui is set after session hook", func(t *testing.T) { verificationTS := testhelpers.NewVerificationUIFlowEchoServer(t, reg) t.Cleanup(testhelpers.SelfServiceHookConfigReset(t, conf)) - conf.MustSet(ctx, config.ViperKeySelfServiceVerificationEnabled, true) conf.MustSet(ctx, config.ViperKeySelfServiceRegistrationAfter+".hooks", []map[string]interface{}{ + { + "hook": hook.KeyVerifier, + }, { "hook": hook.KeyVerificationUI, }, diff --git a/selfservice/strategy/profile/strategy_test.go b/selfservice/strategy/profile/strategy_test.go index be9b448a0215..968933ad041e 100644 --- a/selfservice/strategy/profile/strategy_test.go +++ b/selfservice/strategy/profile/strategy_test.go @@ -17,6 +17,8 @@ import ( "testing" "time" + "github.com/ory/kratos/selfservice/hook" + "github.com/ory/x/jsonx" kratos "github.com/ory/kratos/internal/httpclient" @@ -532,7 +534,7 @@ func TestStrategyTraits(t *testing.T) { t.Run("description=should send email with verifiable address", func(t *testing.T) { setPrivileged(t) - conf.MustSet(ctx, config.ViperKeySelfServiceVerificationEnabled, true) + conf.MustSet(ctx, config.ViperKeySelfServiceSettingsAfter+".profile.hooks", []map[string]any{{"hook": hook.KeyVerifier}}) conf.MustSet(ctx, config.ViperKeyCourierSMTPURL, "smtp://foo:bar@irrelevant.com/") t.Cleanup(func() { conf.MustSet(ctx, config.HookStrategyKey(config.ViperKeySelfServiceSettingsAfter, settings.StrategyProfile), nil)