Skip to content

Commit 5eac736

Browse files
David-WobrockDavid-Wobrock
committed
feat: do not automatically add verification hook
Instead, it should be configured explicitly if we want emails to be sent during registration. In the current state, this is a breaking change.
1 parent b96f0ce commit 5eac736

File tree

5 files changed

+40
-32
lines changed

5 files changed

+40
-32
lines changed

‎driver/registry_default_registration.go

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,19 +26,13 @@ func (m *RegistryDefault) PostRegistrationPrePersistHooks(ctx context.Context, c
2626
}
2727

2828
func (m *RegistryDefault) PostRegistrationPostPersistHooks(ctx context.Context, credentialsType identity.CredentialsType) (b []registration.PostHookPostPersistExecutor) {
29-
initialHookCount := 0
30-
if m.Config().SelfServiceFlowVerificationEnabled(ctx) {
31-
b = append(b, m.HookVerifier())
32-
initialHookCount = 1
33-
}
34-
3529
for _, v := range m.getHooks(string(credentialsType), m.Config().SelfServiceFlowRegistrationAfterHooks(ctx, string(credentialsType))) {
3630
if hook, ok := v.(registration.PostHookPostPersistExecutor); ok {
3731
b = append(b, hook)
3832
}
3933
}
4034

41-
if len(b) == initialHookCount {
35+
if len(b) == 0 {
4236
// since we don't want merging hooks defined in a specific strategy and
4337
// global hooks are added only if no strategy specific hooks are defined
4438
for _, v := range m.getHooks(config.HookGlobal, m.Config().SelfServiceFlowRegistrationAfterHooks(ctx, config.HookGlobal)) {

‎driver/registry_default_settings.go

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,19 +29,13 @@ func (m *RegistryDefault) PreSettingsHooks(ctx context.Context) (b []settings.Pr
2929
}
3030

3131
func (m *RegistryDefault) PostSettingsPostPersistHooks(ctx context.Context, settingsType string) (b []settings.PostHookPostPersistExecutor) {
32-
initialHookCount := 0
33-
if m.Config().SelfServiceFlowVerificationEnabled(ctx) {
34-
b = append(b, m.HookVerifier())
35-
initialHookCount = 1
36-
}
37-
3832
for _, v := range m.getHooks(settingsType, m.Config().SelfServiceFlowSettingsAfterHooks(ctx, settingsType)) {
3933
if hook, ok := v.(settings.PostHookPostPersistExecutor); ok {
4034
b = append(b, hook)
4135
}
4236
}
4337

44-
if len(b) == initialHookCount {
38+
if len(b) == 0 {
4539
// since we don't want merging hooks defined in a specific strategy and global hooks
4640
// global hooks are added only if no strategy specific hooks are defined
4741
for _, v := range m.getHooks(config.HookGlobal, m.Config().SelfServiceFlowSettingsAfterHooks(ctx, config.HookGlobal)) {

‎driver/registry_default_test.go

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -263,11 +263,24 @@ func TestDriverDefault_Hooks(t *testing.T) {
263263
{
264264
uc: "Only session hook configured for password strategy",
265265
config: map[string]any{
266-
config.ViperKeySelfServiceVerificationEnabled: true,
267266
config.ViperKeySelfServiceRegistrationAfter + ".password.hooks": []map[string]any{
268267
{"hook": "session"},
269268
},
270269
},
270+
expect: func(reg *driver.RegistryDefault) []registration.PostHookPostPersistExecutor {
271+
return []registration.PostHookPostPersistExecutor{
272+
hook.NewSessionIssuer(reg),
273+
}
274+
},
275+
},
276+
{
277+
uc: "Session hook and verification hook configured for password strategy",
278+
config: map[string]any{
279+
config.ViperKeySelfServiceRegistrationAfter + ".password.hooks": []map[string]any{
280+
{"hook": "verification"},
281+
{"hook": "session"},
282+
},
283+
},
271284
expect: func(reg *driver.RegistryDefault) []registration.PostHookPostPersistExecutor {
272285
return []registration.PostHookPostPersistExecutor{
273286
hook.NewVerifier(reg),
@@ -278,15 +291,13 @@ func TestDriverDefault_Hooks(t *testing.T) {
278291
{
279292
uc: "A session hook and a web_hook are configured for password strategy",
280293
config: map[string]any{
281-
config.ViperKeySelfServiceVerificationEnabled: true,
282294
config.ViperKeySelfServiceRegistrationAfter + ".password.hooks": []map[string]any{
283295
{"hook": "web_hook", "config": map[string]any{"headers": map[string]string{"X-Custom-Header": "test"}, "url": "foo", "method": "POST", "body": "bar"}},
284296
{"hook": "session"},
285297
},
286298
},
287299
expect: func(reg *driver.RegistryDefault) []registration.PostHookPostPersistExecutor {
288300
return []registration.PostHookPostPersistExecutor{
289-
hook.NewVerifier(reg),
290301
hook.NewWebHook(reg, json.RawMessage(`{"body":"bar","headers":{"X-Custom-Header":"test"},"method":"POST","url":"foo"}`)),
291302
hook.NewSessionIssuer(reg),
292303
}
@@ -317,11 +328,9 @@ func TestDriverDefault_Hooks(t *testing.T) {
317328
config.ViperKeySelfServiceRegistrationAfter + ".hooks": []map[string]any{
318329
{"hook": "web_hook", "config": map[string]any{"url": "bar", "method": "POST", "headers": map[string]string{"X-Custom-Header": "test"}}},
319330
},
320-
config.ViperKeySelfServiceVerificationEnabled: true,
321331
},
322332
expect: func(reg *driver.RegistryDefault) []registration.PostHookPostPersistExecutor {
323333
return []registration.PostHookPostPersistExecutor{
324-
hook.NewVerifier(reg),
325334
hook.NewWebHook(reg, json.RawMessage(`{"headers":{"X-Custom-Header":"test"},"method":"GET","url":"foo"}`)),
326335
hook.NewSessionIssuer(reg),
327336
}
@@ -553,7 +562,9 @@ func TestDriverDefault_Hooks(t *testing.T) {
553562
{
554563
uc: "Only verify hook configured for the strategy",
555564
config: map[string]any{
556-
config.ViperKeySelfServiceVerificationEnabled: true,
565+
config.ViperKeySelfServiceSettingsAfter + ".profile.hooks": []map[string]any{
566+
{"hook": "verification"},
567+
},
557568
// I think this is a bug as there is a hook named verify defined for both profile and password
558569
// strategies. Instead of using it, the code makes use of the property used above and which
559570
// is defined in an entirely different flow (verification).
@@ -570,11 +581,9 @@ func TestDriverDefault_Hooks(t *testing.T) {
570581
config.ViperKeySelfServiceSettingsAfter + ".profile.hooks": []map[string]any{
571582
{"hook": "web_hook", "config": map[string]any{"headers": []map[string]string{{"X-Custom-Header": "test"}}, "url": "foo", "method": "POST", "body": "bar"}},
572583
},
573-
config.ViperKeySelfServiceVerificationEnabled: true,
574584
},
575585
expect: func(reg *driver.RegistryDefault) []settings.PostHookPostPersistExecutor {
576586
return []settings.PostHookPostPersistExecutor{
577-
hook.NewVerifier(reg),
578587
hook.NewWebHook(reg, json.RawMessage(`{"body":"bar","headers":[{"X-Custom-Header":"test"}],"method":"POST","url":"foo"}`)),
579588
}
580589
},
@@ -597,7 +606,6 @@ func TestDriverDefault_Hooks(t *testing.T) {
597606
{
598607
uc: "Hooks are configured on a global level, as well as on a strategy level",
599608
config: map[string]any{
600-
config.ViperKeySelfServiceVerificationEnabled: true,
601609
config.ViperKeySelfServiceSettingsAfter + ".profile.hooks": []map[string]any{
602610
{"hook": "web_hook", "config": map[string]any{"url": "foo", "method": "GET", "headers": map[string]string{"X-Custom-Header": "test"}}},
603611
},
@@ -607,7 +615,6 @@ func TestDriverDefault_Hooks(t *testing.T) {
607615
},
608616
expect: func(reg *driver.RegistryDefault) []settings.PostHookPostPersistExecutor {
609617
return []settings.PostHookPostPersistExecutor{
610-
hook.NewVerifier(reg),
611618
hook.NewWebHook(reg, json.RawMessage(`{"headers":{"X-Custom-Header":"test"},"method":"GET","url":"foo"}`)),
612619
}
613620
},

‎selfservice/flow/registration/hook_test.go

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -187,9 +187,12 @@ func TestRegistrationExecutor(t *testing.T) {
187187
t.Run("case=should redirect to verification UI if show_verification_ui hook is set", func(t *testing.T) {
188188
verificationTS := testhelpers.NewVerificationUIFlowEchoServer(t, reg)
189189
t.Cleanup(testhelpers.SelfServiceHookConfigReset(t, conf))
190-
conf.MustSet(ctx, config.ViperKeySelfServiceVerificationEnabled, true)
191190
conf.MustSet(ctx, config.ViperKeySelfServiceRegistrationAfter+".hooks", []map[string]interface{}{
192191
{
192+
"hook": hook.KeyVerifier,
193+
},
194+
{
195+
193196
"hook": hook.KeyVerificationUI,
194197
},
195198
})
@@ -205,10 +208,14 @@ func TestRegistrationExecutor(t *testing.T) {
205208
t.Run("case=should redirect to verification UI if there is a login_challenge", func(t *testing.T) {
206209
verificationTS := testhelpers.NewVerificationUIFlowEchoServer(t, reg)
207210
t.Cleanup(testhelpers.SelfServiceHookConfigReset(t, conf))
208-
conf.MustSet(ctx, config.ViperKeySelfServiceVerificationEnabled, true)
209-
conf.MustSet(ctx, config.ViperKeySelfServiceRegistrationAfter+".hooks", []map[string]interface{}{{
210-
"hook": hook.KeyVerificationUI,
211-
}})
211+
conf.MustSet(ctx, config.ViperKeySelfServiceRegistrationAfter+".hooks", []map[string]interface{}{
212+
{
213+
"hook": hook.KeyVerifier,
214+
},
215+
{
216+
"hook": hook.KeyVerificationUI,
217+
},
218+
})
212219
i := testhelpers.SelfServiceHookFakeIdentity(t)
213220
i.Traits = identity.Traits(`{"email": "[email protected]"}`)
214221

@@ -228,8 +235,10 @@ func TestRegistrationExecutor(t *testing.T) {
228235
t.Run("case=should redirect to first verification UI if show_verification_ui hook is set and multiple verifiable addresses", func(t *testing.T) {
229236
verificationTS := testhelpers.NewVerificationUIFlowEchoServer(t, reg)
230237
t.Cleanup(testhelpers.SelfServiceHookConfigReset(t, conf))
231-
conf.MustSet(ctx, config.ViperKeySelfServiceVerificationEnabled, true)
232238
conf.MustSet(ctx, config.ViperKeySelfServiceRegistrationAfter+".hooks", []map[string]interface{}{
239+
{
240+
"hook": hook.KeyVerifier,
241+
},
233242
{
234243
"hook": hook.KeyVerificationUI,
235244
},
@@ -248,8 +257,10 @@ func TestRegistrationExecutor(t *testing.T) {
248257
t.Run("case=should still sent session if show_verification_ui is set after session hook", func(t *testing.T) {
249258
verificationTS := testhelpers.NewVerificationUIFlowEchoServer(t, reg)
250259
t.Cleanup(testhelpers.SelfServiceHookConfigReset(t, conf))
251-
conf.MustSet(ctx, config.ViperKeySelfServiceVerificationEnabled, true)
252260
conf.MustSet(ctx, config.ViperKeySelfServiceRegistrationAfter+".hooks", []map[string]interface{}{
261+
{
262+
"hook": hook.KeyVerifier,
263+
},
253264
{
254265
"hook": hook.KeyVerificationUI,
255266
},

‎selfservice/strategy/profile/strategy_test.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ import (
1717
"testing"
1818
"time"
1919

20+
"github.com/ory/kratos/selfservice/hook"
21+
2022
"github.com/ory/x/jsonx"
2123

2224
kratos "github.com/ory/kratos/internal/httpclient"
@@ -532,7 +534,7 @@ func TestStrategyTraits(t *testing.T) {
532534
t.Run("description=should send email with verifiable address", func(t *testing.T) {
533535
setPrivileged(t)
534536

535-
conf.MustSet(ctx, config.ViperKeySelfServiceVerificationEnabled, true)
537+
conf.MustSet(ctx, config.ViperKeySelfServiceSettingsAfter+".profile.hooks", []map[string]any{{"hook": hook.KeyVerifier}})
536538
conf.MustSet(ctx, config.ViperKeyCourierSMTPURL, "smtp://foo:[email protected]/")
537539
t.Cleanup(func() {
538540
conf.MustSet(ctx, config.HookStrategyKey(config.ViperKeySelfServiceSettingsAfter, settings.StrategyProfile), nil)

0 commit comments

Comments
 (0)