fix: on verification required after registration, preserve return_to (#…

…3589)

* fix: on verification required after registration, preserve return_to

* test: return_to on verification flow

* chore: refactor

Co-authored-by: Jonas Hungershausen <[email protected]>

---------

Co-authored-by: Jonas Hungershausen <[email protected]>

selfservice/flow/verification/flow.go

@@ -185,7 +185,11 @@ func NewPostHookFlow(conf *config.Config, exp time.Duration, csrf string, r *htt
 		requestURL = new(url.URL)
 	}
 	query := requestURL.Query()
-	query.Set("return_to", query.Get("after_verification_return_to"))
+	// we need to keep the return_to in-tact if the `after_verification_return_to` is empty
+	// otherwise we take the `after_verification_return_to` query parameter over the current `return_to`
+	if afterVerificationReturn := query.Get("after_verification_return_to"); afterVerificationReturn != "" {
+		query.Set("return_to", afterVerificationReturn)
+	}
 	query.Del("after_verification_return_to")
 	requestURL.RawQuery = query.Encode()
 	f.RequestURL = requestURL.String()

selfservice/flow/verification/flow_test.go

@@ -108,7 +108,7 @@ func TestNewPostHookFlow(t *testing.T) {
 	})
 
 	t.Run("case=return_to supplied", func(t *testing.T) {
-		expectReturnTo(t, url.Values{"return_to": {"http://foo.com/original_flow_callback"}}, "")
+		expectReturnTo(t, url.Values{"return_to": {"http://foo.com/original_flow_callback"}}, "http://foo.com/original_flow_callback")
 	})
 
 	t.Run("case=return_to and after_verification_return_to supplied", func(t *testing.T) {

selfservice/hook/show_verification_ui.go

@@ -59,9 +59,10 @@ func (e *ShowVerificationUIHook) execute(r *http.Request, f *registration.Flow)
 		}
 	}
 
+	ctx := r.Context()
 	if vf != nil {
-		redir := e.d.Config().SelfServiceFlowVerificationUI(r.Context())
-		f.ReturnToVerification = vf.AppendTo(redir).String()
+		redirURL := e.d.Config().SelfServiceFlowVerificationUI(ctx)
+		f.ReturnToVerification = vf.AppendTo(redirURL).String()
 	}
 
 	return nil

selfservice/hook/verification.go

@@ -70,29 +70,37 @@ func (e *Verifier) do(
 ) error {
 	// This is called after the identity has been created so we can safely assume that all addresses are available
 	// already.
+	ctx := r.Context()
 
-	strategy, err := e.r.GetActiveVerificationStrategy(r.Context())
+	strategy, err := e.r.GetActiveVerificationStrategy(ctx)
 	if err != nil {
 		return err
 	}
 
+	isBrowserFlow := f.GetType() == flow.TypeBrowser
+	isRegistrationFlow := f.GetFlowName() == flow.RegistrationFlow
+
 	for k := range i.VerifiableAddresses {
 		address := &i.VerifiableAddresses[k]
 		if address.Status != identity.VerifiableAddressStatusPending {
 			continue
 		}
-		csrf := ""
+
+		var csrf string
+
 		// TODO: this is pretty ugly, we should probably have a better way to handle CSRF tokens here.
-		if f.GetType() != flow.TypeBrowser {
-		} else if _, ok := f.(*registration.Flow); ok {
-			// If this hook is executed from a registration flow, we need to regenerate the CSRF token.
-			csrf = e.r.CSRFHandler().RegenerateToken(w, r)
-		} else {
-			// If it came from a settings flow, there already is a CSRF token, so we can just use that.
-			csrf = e.r.GenerateCSRFToken(r)
+		if isBrowserFlow {
+			if isRegistrationFlow {
+				// If this hook is executed from a registration flow, we need to regenerate the CSRF token.
+				csrf = e.r.CSRFHandler().RegenerateToken(w, r)
+			} else {
+				// If it came from a settings flow, there already is a CSRF token, so we can just use that.
+				csrf = e.r.GenerateCSRFToken(r)
+			}
 		}
+
 		verificationFlow, err := verification.NewPostHookFlow(e.r.Config(),
-			e.r.Config().SelfServiceFlowVerificationRequestLifespan(r.Context()),
+			e.r.Config().SelfServiceFlowVerificationRequestLifespan(ctx),
 			csrf, r, strategy, f)
 		if err != nil {
 			return err
@@ -108,17 +116,17 @@ func (e *Verifier) do(
 			return err
 		}
 
-		if err := e.r.VerificationFlowPersister().CreateVerificationFlow(r.Context(), verificationFlow); err != nil {
+		if err := e.r.VerificationFlowPersister().CreateVerificationFlow(ctx, verificationFlow); err != nil {
 			return err
 		}
 
-		if err := strategy.SendVerificationEmail(r.Context(), verificationFlow, i, address); err != nil {
+		if err := strategy.SendVerificationEmail(ctx, verificationFlow, i, address); err != nil {
 			return err
 		}
 
 		flowURL := ""
 		if verificationFlow.Type == flow.TypeBrowser {
-			flowURL = verificationFlow.AppendTo(e.r.Config().SelfServiceFlowVerificationUI(r.Context())).String()
+			flowURL = verificationFlow.AppendTo(e.r.Config().SelfServiceFlowVerificationUI(ctx)).String()
 		}
 
 		f.AddContinueWith(flow.NewContinueWithVerificationUI(verificationFlow, address.Value, flowURL))

test/e2e/cypress/integration/profiles/verification/registration/success.spec.ts

@@ -70,6 +70,7 @@ context("Account Verification Registration Success", () => {
           email,
           password,
           query: {
+            return_to: "http://localhost:4455/verification_return_to_callback",
             after_verification_return_to:
               "http://localhost:4455/verification_callback",
           },
@@ -83,6 +84,26 @@ context("Account Verification Registration Success", () => {
           },
         })
       })
+
+      it("is redirected to `return_to` after verification", () => {
+        cy.clearAllCookies()
+        const { email, password } = gen.identity()
+        cy.register({
+          email,
+          password,
+          query: {
+            return_to: "http://localhost:4455/verification_return_to_callback",
+          },
+        })
+        cy.login({ email, password })
+        cy.verifyEmail({
+          expect: {
+            email,
+            password,
+            redirectTo: "http://localhost:4455/verification_return_to_callback",
+          },
+        })
+      })
     })
   })
 })