fix: remove http.Redirect from show_verification_ui hook (#3238)

ory · Apr 17, 2023 · 054705b · 054705b
1 parent 6f908b9
commit 054705b
Show file tree

Hide file tree

Showing 5 changed files with 38 additions and 3 deletions.
diff --git a/selfservice/flow/registration/flow.go b/selfservice/flow/registration/flow.go
@@ -80,6 +80,9 @@ type Flow struct {
 	// ReturnTo contains the requested return_to URL.
 	ReturnTo string `json:"return_to,omitempty" db:"-"`
 
+	// ReturnToVerification contains the redirect URL for the verification flow.
+	ReturnToVerification string `json:"-" db:"-"`
+
 	// Active, if set, contains the registration method that is being used. It is initially
 	// not set.
 	Active identity.CredentialsType `json:"active,omitempty" faker:"identity_credentials_type" db:"active_method"`

diff --git a/selfservice/flow/registration/hook.go b/selfservice/flow/registration/hook.go
@@ -247,6 +247,8 @@ func (e *HookExecutor) PostRegistrationHook(w http.ResponseWriter, r *http.Reque
 			return err
 		}
 		finalReturnTo = cr
+	} else if a.ReturnToVerification != "" {
+		finalReturnTo = a.ReturnToVerification
 	}
 
 	x.ContentNegotiationRedirection(w, r, s.Declassified(), e.d.Writer(), finalReturnTo)

diff --git a/selfservice/flow/registration/hook_test.go b/selfservice/flow/registration/hook_test.go
@@ -198,6 +198,36 @@ func TestRegistrationExecutor(t *testing.T) {
 					assert.Contains(t, res.Request.URL.String(), verificationTS.URL)
 					assert.NotEmpty(t, res.Request.URL.Query().Get("flow"))
 				})
+
+				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.Set(ctx, config.ViperKeySelfServiceVerificationEnabled, true)
+					conf.Set(ctx, config.ViperKeySelfServiceRegistrationAfter+".hooks", []map[string]interface{}{
+						{
+							"hook": hook.KeyVerificationUI,
+						},
+						{
+							"hook": hook.KeySessionIssuer,
+						},
+					})
+
+					i := testhelpers.SelfServiceHookFakeIdentity(t)
+					i.Traits = identity.Traits(`{"email": "verifiable4@ory.sh"}`)
+
+					jar := x.EasyCookieJar(t, nil)
+					s := newServer(t, i, flow.TypeBrowser)
+					s.Client().Jar = jar
+					res, _ := makeRequestPost(t, s, false, url.Values{})
+					assert.EqualValues(t, http.StatusOK, res.StatusCode)
+					assert.Contains(t, res.Request.URL.String(), verificationTS.URL)
+					assert.NotEmpty(t, res.Request.URL.Query().Get("flow"))
+					u, err := url.Parse(s.URL)
+					require.NoError(t, err)
+					cookies := jar.Cookies(u)
+					require.Len(t, cookies, 1)
+					assert.Equal(t, "ory_kratos_session", cookies[0].Name)
+				})
 			})
 
 			for _, kind := range []flow.Type{flow.TypeBrowser, flow.TypeAPI} {

diff --git a/selfservice/hook/show_verification_ui.go b/selfservice/hook/show_verification_ui.go
@@ -63,7 +63,7 @@ func (e *ShowVerificationUIHook) execute(w http.ResponseWriter, r *http.Request,
 
 	if vf != nil {
 		redir := e.d.Config().SelfServiceFlowVerificationUI(r.Context())
-		http.Redirect(w, r, vf.AppendTo(redir).String(), http.StatusSeeOther)
+		f.ReturnToVerification = vf.AppendTo(redir).String()
 	}
 
 	return nil

diff --git a/selfservice/hook/show_verification_ui_test.go b/selfservice/hook/show_verification_ui_test.go
@@ -56,8 +56,8 @@ func TestExecutePostRegistrationPostPersistHook(t *testing.T) {
 		}
 		rec := httptest.NewRecorder()
 		require.NoError(t, h.ExecutePostRegistrationPostPersistHook(rec, browserRequest, rf, nil))
-		assert.Equal(t, 303, rec.Code)
-		assert.Equal(t, "/verification?flow="+vf.ID.String(), rec.Header().Get("Location"))
+		assert.Equal(t, 200, rec.Code)
+		assert.Equal(t, "/verification?flow="+vf.ID.String(), rf.ReturnToVerification)
 	})
 
 	t.Run("case=no verification ui in continue with item returns 200 OK", func(t *testing.T) {