fix: #1251 - fix return_to for expired flows

selfservice/flow/login/error.go

@@ -79,7 +79,7 @@ func (s *ErrorHandler) WriteFlowError(w http.ResponseWriter, r *http.Request, f
 
 	if e := new(FlowExpiredError); errors.As(err, &e) {
 		// create new flow because the old one is not valid
-		a, err := s.d.LoginHandler().NewLoginFlow(w, r, f.Type)
+		a, err := s.d.LoginHandler().FromOldFlow(w, r, *f)
 		if err != nil {
 			// failed to create a new session and redirect to it, handle that error as a new one
 			s.WriteFlowError(w, r, f, group, err)

selfservice/flow/login/handler.go

@@ -109,6 +109,16 @@ func (h *Handler) NewLoginFlow(w http.ResponseWriter, r *http.Request, flow flow
 	return f, nil
 }
 
+func (h *Handler) FromOldFlow(w http.ResponseWriter, r *http.Request, of Flow) (*Flow, error) {
+	nf, err := h.NewLoginFlow(w, r, of.Type)
+	if err != nil {
+		return nil, err
+	}
+
+	nf.RequestURL = of.RequestURL
+	return nf, nil
+}
+
 // nolint:deadcode,unused
 // swagger:parameters initializeSelfServiceLoginFlowForBrowsers initializeSelfServiceLoginFlowWithoutBrowser
 type initializeSelfServiceLoginFlowWithoutBrowser struct {

selfservice/flow/login/handler_test.go

@@ -185,6 +185,7 @@ func TestGetFlow(t *testing.T) {
 			require.NoError(t, err)
 		}))
 		conf.MustSet(config.ViperKeySelfServiceLoginUI, ts.URL)
+		conf.MustSet(config.ViperKeySelfServiceBrowserDefaultReturnTo, "https://www.ory.sh")
 		t.Cleanup(ts.Close)
 		return ts
 	}
@@ -229,4 +230,27 @@ func TestGetFlow(t *testing.T) {
 		assert.EqualValues(t, http.StatusGone, res.StatusCode)
 		assert.Equal(t, public.URL+login.RouteInitBrowserFlow, gjson.GetBytes(body, "error.details.redirect_to").String(), "%s", body)
 	})
+
+	t.Run("case=expired with return_to", func(t *testing.T) {
+		client := testhelpers.NewClientWithCookies(t)
+		setupLoginUI(t, client)
+		body := x.EasyGetBody(t, client, public.URL+login.RouteInitBrowserFlow+"?return_to=https://www.ory.sh")
+
+		// Expire the flow
+		f, err := reg.LoginFlowPersister().GetLoginFlow(context.Background(), uuid.FromStringOrNil(gjson.GetBytes(body, "id").String()))
+		require.NoError(t, err)
+		f.ExpiresAt = time.Now().Add(-time.Second)
+		require.NoError(t, reg.LoginFlowPersister().UpdateLoginFlow(context.Background(), f))
+
+		// submit the flow but it is expired
+		u := public.URL + login.RouteSubmitFlow + "?flow=" + f.ID.String()
+		res, err := client.PostForm(u, url.Values{"password_identifier": {"email@ory.sh"}, "csrf_token": {f.CSRFToken}, "password": {"password"}, "method": {"password"}})
+		resBody, err := ioutil.ReadAll(res.Body)
+		require.NoError(t, err)
+		require.NoError(t, res.Body.Close())
+
+		f, err = reg.LoginFlowPersister().GetLoginFlow(context.Background(), uuid.FromStringOrNil(gjson.GetBytes(resBody, "id").String()))
+		require.NoError(t, err)
+		assert.Equal(t, public.URL+login.RouteInitBrowserFlow+"?return_to=https://www.ory.sh", f.RequestURL)
+	})
 }

selfservice/flow/recovery/error.go

@@ -96,7 +96,7 @@ func (s *ErrorHandler) WriteFlowError(
 
 	if e := new(FlowExpiredError); errors.As(err, &e) {
 		// create new flow because the old one is not valid
-		a, err := NewFlow(s.d.Config(r.Context()), s.d.Config(r.Context()).SelfServiceFlowRecoveryRequestLifespan(), s.d.GenerateCSRFToken(r), r, s.d.RecoveryStrategies(r.Context()), f.Type)
+		a, err := FromOldFlow(s.d.Config(r.Context()), s.d.Config(r.Context()).SelfServiceFlowRecoveryRequestLifespan(), s.d.GenerateCSRFToken(r), r, s.d.RecoveryStrategies(r.Context()), *f)
 		if err != nil {
 			// failed to create a new session and redirect to it, handle that error as a new one
 			s.WriteFlowError(w, r, f, group, err)

selfservice/flow/recovery/flow.go

@@ -111,6 +111,16 @@ func NewFlow(conf *config.Config, exp time.Duration, csrf string, r *http.Reques
 	return req, nil
 }
 
+func FromOldFlow(conf *config.Config, exp time.Duration, csrf string, r *http.Request, strategies Strategies, of Flow) (*Flow, error) {
+	nf, err := NewFlow(conf, exp, csrf, r, strategies, of.Type)
+	if err != nil {
+		return nil, err
+	}
+
+	nf.RequestURL = of.RequestURL
+	return nf, nil
+}
+
 func (f *Flow) GetType() flow.Type {
 	return f.Type
 }

selfservice/flow/recovery/handler_test.go

@@ -6,6 +6,7 @@ import (
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
+	"net/url"
 	"testing"
 	"time"
 
@@ -215,4 +216,27 @@ func TestGetFlow(t *testing.T) {
 		assert.EqualValues(t, http.StatusGone, res.StatusCode)
 		assert.Equal(t, public.URL+recovery.RouteInitBrowserFlow, gjson.GetBytes(body, "error.details.redirect_to").String(), "%s", body)
 	})
+
+	t.Run("case=expired with return_to", func(t *testing.T) {
+		client := testhelpers.NewClientWithCookies(t)
+		setupRecoveryTS(t, client)
+		body := x.EasyGetBody(t, client, public.URL+recovery.RouteInitBrowserFlow+"?return_to=https://www.ory.sh")
+
+		// Expire the flow
+		f, err := reg.RecoveryFlowPersister().GetRecoveryFlow(context.Background(), uuid.FromStringOrNil(gjson.GetBytes(body, "id").String()))
+		require.NoError(t, err)
+		f.ExpiresAt = time.Now().Add(-time.Second)
+		require.NoError(t, reg.RecoveryFlowPersister().UpdateRecoveryFlow(context.Background(), f))
+
+		// submit the flow but it is expired
+		u := public.URL + recovery.RouteSubmitFlow + "?flow=" + f.ID.String()
+		res, err := client.PostForm(u, url.Values{"email": {"email@ory.sh"}, "csrf_token": {f.CSRFToken}, "method": {"link"}})
+		resBody, err := ioutil.ReadAll(res.Body)
+		require.NoError(t, err)
+		require.NoError(t, res.Body.Close())
+
+		f, err = reg.RecoveryFlowPersister().GetRecoveryFlow(context.Background(), uuid.FromStringOrNil(gjson.GetBytes(resBody, "id").String()))
+		require.NoError(t, err)
+		assert.Equal(t, public.URL+recovery.RouteInitBrowserFlow+"?return_to=https://www.ory.sh", f.RequestURL)
+	})
 }

selfservice/flow/registration/error.go

@@ -83,7 +83,7 @@ func (s *ErrorHandler) WriteFlowError(
 
 	if e := new(FlowExpiredError); errors.As(err, &e) {
 		// create new flow because the old one is not valid
-		a, err := s.d.RegistrationHandler().NewRegistrationFlow(w, r, f.Type)
+		a, err := s.d.RegistrationHandler().FromOldFlow(w, r, *f)
 		if err != nil {
 			// failed to create a new session and redirect to it, handle that error as a new one
 			s.WriteFlowError(w, r, f, group, err)

selfservice/flow/registration/handler.go

@@ -112,6 +112,16 @@ func (h *Handler) NewRegistrationFlow(w http.ResponseWriter, r *http.Request, ft
 	return f, nil
 }
 
+func (h *Handler) FromOldFlow(w http.ResponseWriter, r *http.Request, of Flow) (*Flow, error) {
+	nf, err := h.NewRegistrationFlow(w, r, of.Type)
+	if err != nil {
+		return nil, err
+	}
+
+	nf.RequestURL = of.RequestURL
+	return nf, nil
+}
+
 // swagger:route GET /self-service/registration/api v0alpha1 initializeSelfServiceRegistrationFlowWithoutBrowser
 //
 // Initialize Registration Flow for APIs, Services, Apps, ...

selfservice/flow/registration/handler_test.go

@@ -6,6 +6,7 @@ import (
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
+	"net/url"
 	"testing"
 	"time"
 
@@ -213,4 +214,27 @@ func TestGetFlow(t *testing.T) {
 		assert.EqualValues(t, http.StatusGone, res.StatusCode)
 		assert.Equal(t, public.URL+registration.RouteInitBrowserFlow, gjson.GetBytes(body, "error.details.redirect_to").String(), "%s", body)
 	})
+
+	t.Run("case=expired with return_to", func(t *testing.T) {
+		client := testhelpers.NewClientWithCookies(t)
+		setupRegistrationUI(t, client)
+		body := x.EasyGetBody(t, client, public.URL+registration.RouteInitBrowserFlow+"?return_to=https://www.ory.sh")
+
+		// Expire the flow
+		f, err := reg.RegistrationFlowPersister().GetRegistrationFlow(context.Background(), uuid.FromStringOrNil(gjson.GetBytes(body, "id").String()))
+		require.NoError(t, err)
+		f.ExpiresAt = time.Now().Add(-time.Second)
+		require.NoError(t, reg.RegistrationFlowPersister().UpdateRegistrationFlow(context.Background(), f))
+
+		// submit the flow but it is expired
+		u := public.URL + registration.RouteSubmitFlow + "?flow=" + f.ID.String()
+		res, err := client.PostForm(u, url.Values{"method": {"password"}, "csrf_token": {f.CSRFToken}, "password": {"password"}, "traits.email": {"email@ory.sh"}})
+		resBody, err := ioutil.ReadAll(res.Body)
+		require.NoError(t, err)
+		require.NoError(t, res.Body.Close())
+
+		f, err = reg.RegistrationFlowPersister().GetRegistrationFlow(context.Background(), uuid.FromStringOrNil(gjson.GetBytes(resBody, "id").String()))
+		require.NoError(t, err)
+		assert.Equal(t, public.URL+registration.RouteInitBrowserFlow+"?return_to=https://www.ory.sh", f.RequestURL)
+	})
 }

selfservice/flow/settings/error.go

@@ -118,7 +118,7 @@ func (s *ErrorHandler) WriteFlowError(
 		}
 
 		// create new flow because the old one is not valid
-		a, err := s.d.SettingsHandler().NewFlow(w, r, id, f.Type)
+		a, err := s.d.SettingsHandler().FromOldFlow(w, r, id, *f)
 		if err != nil {
 			// failed to create a new session and redirect to it, handle that error as a new one
 			s.WriteFlowError(w, r, group, f, id, err)

selfservice/flow/settings/handler.go

@@ -128,6 +128,16 @@ func (h *Handler) NewFlow(w http.ResponseWriter, r *http.Request, i *identity.Id
 	return f, nil
 }
 
+func (h *Handler) FromOldFlow(w http.ResponseWriter, r *http.Request, i *identity.Identity, of Flow) (*Flow, error) {
+	nf, err := h.NewFlow(w, r, i, of.Type)
+	if err != nil {
+		return nil, err
+	}
+
+	nf.RequestURL = of.RequestURL
+	return nf, nil
+}
+
 // swagger:parameters initializeSelfServiceSettingsFlowWithoutBrowser
 // nolint:deadcode,unused
 type initializeSelfServiceSettingsFlowWithoutBrowser struct {

selfservice/flow/settings/handler_test.go

@@ -4,9 +4,12 @@ import (
 	"context"
 	"io/ioutil"
 	"net/http"
+	"net/url"
 	"testing"
 	"time"
 
+	"github.com/gofrs/uuid"
+
 	kratos "github.com/ory/kratos-client-go"
 
 	"github.com/ory/kratos/corpx"
@@ -131,6 +134,28 @@ func TestHandler(t *testing.T) {
 		assert.Equal(t, int64(http.StatusGone), gjson.GetBytes(err.(*kratos.GenericOpenAPIError).Body(), "error.code").Int())
 	})
 
+	t.Run("case=expired with return_to", func(t *testing.T) {
+		client := testhelpers.NewHTTPClientWithArbitrarySessionToken(t, reg)
+		body := x.EasyGetBody(t, client, publicTS.URL+settings.RouteInitBrowserFlow+"?return_to=https://www.ory.sh")
+
+		// Expire the flow
+		f, err := reg.SettingsFlowPersister().GetSettingsFlow(context.Background(), uuid.FromStringOrNil(gjson.GetBytes(body, "id").String()))
+		require.NoError(t, err)
+		f.ExpiresAt = time.Now().Add(-time.Second)
+		require.NoError(t, reg.SettingsFlowPersister().UpdateSettingsFlow(context.Background(), f))
+
+		// submit the flow but it is expired
+		u := publicTS.URL + settings.RouteSubmitFlow + "?flow=" + f.ID.String()
+		res, err := client.PostForm(u, url.Values{"method": {"password"}, "csrf_token": {"csrf"}, "password": {"password"}})
+		resBody, err := ioutil.ReadAll(res.Body)
+		require.NoError(t, err)
+		require.NoError(t, res.Body.Close())
+
+		f, err = reg.SettingsFlowPersister().GetSettingsFlow(context.Background(), uuid.FromStringOrNil(gjson.GetBytes(resBody, "id").String()))
+		require.NoError(t, err)
+		assert.Equal(t, publicTS.URL+settings.RouteInitBrowserFlow+"?return_to=https://www.ory.sh", f.RequestURL)
+	})
+
 	t.Run("description=should fail to fetch request if identity changed", func(t *testing.T) {
 		t.Run("type=api", func(t *testing.T) {
 			user1 := testhelpers.NewHTTPClientWithArbitrarySessionToken(t, reg)

selfservice/flow/verification/error.go

@@ -85,8 +85,8 @@ func (s *ErrorHandler) WriteFlowError(
 
 	if e := new(FlowExpiredError); errors.As(err, &e) {
 		// create new flow because the old one is not valid
-		a, err := NewFlow(s.d.Config(r.Context()), s.d.Config(r.Context()).SelfServiceFlowVerificationRequestLifespan(),
-			s.d.GenerateCSRFToken(r), r, s.d.VerificationStrategies(r.Context()), f.Type)
+		a, err := FromOldFlow(s.d.Config(r.Context()), s.d.Config(r.Context()).SelfServiceFlowVerificationRequestLifespan(),
+			s.d.GenerateCSRFToken(r), r, s.d.VerificationStrategies(r.Context()), f)
 		if err != nil {
 			// failed to create a new session and redirect to it, handle that error as a new one
 			s.WriteFlowError(w, r, f, group, err)

selfservice/flow/verification/flow.go

@@ -115,6 +115,16 @@ func NewFlow(conf *config.Config, exp time.Duration, csrf string, r *http.Reques
 	return f, nil
 }
 
+func FromOldFlow(conf *config.Config, exp time.Duration, csrf string, r *http.Request, strategies Strategies, of *Flow) (*Flow, error) {
+	nf, err := NewFlow(conf, exp, csrf, r, strategies, of.Type)
+	if err != nil {
+		return nil, err
+	}
+
+	nf.RequestURL = of.RequestURL
+	return nf, nil
+}
+
 func NewPostHookFlow(conf *config.Config, exp time.Duration, csrf string, r *http.Request, strategies Strategies, original flow.Flow) (*Flow, error) {
 	f, err := NewFlow(conf, exp, csrf, r, strategies, original.GetType())
 	if err != nil {

selfservice/flow/verification/handler_test.go

@@ -2,7 +2,9 @@ package verification_test
 
 import (
 	"context"
+	"io/ioutil"
 	"net/http"
+	"net/url"
 	"testing"
 	"time"
 
@@ -105,4 +107,27 @@ func TestGetFlow(t *testing.T) {
 		assert.EqualValues(t, http.StatusGone, res.StatusCode)
 		assert.Equal(t, public.URL+verification.RouteInitBrowserFlow, gjson.GetBytes(body, "error.details.redirect_to").String(), "%s", body)
 	})
+
+	t.Run("case=expired with return_to", func(t *testing.T) {
+		client := testhelpers.NewClientWithCookies(t)
+		_ = setupVerificationUI(t, client)
+		body := x.EasyGetBody(t, client, public.URL+verification.RouteInitBrowserFlow+"?return_to=https://www.ory.sh")
+
+		// Expire the flow
+		f, err := reg.VerificationFlowPersister().GetVerificationFlow(context.Background(), uuid.FromStringOrNil(gjson.GetBytes(body, "id").String()))
+		require.NoError(t, err)
+		f.ExpiresAt = time.Now().Add(-time.Second)
+		require.NoError(t, reg.VerificationFlowPersister().UpdateVerificationFlow(context.Background(), f))
+
+		// submit the flow but it is expired
+		u := public.URL + verification.RouteSubmitFlow + "?flow=" + f.ID.String()
+		res, err := client.PostForm(u, url.Values{"method": {"link"}, "csrf_token": {f.CSRFToken}, "email": {"email@ory.sh"}})
+		resBody, err := ioutil.ReadAll(res.Body)
+		require.NoError(t, err)
+		require.NoError(t, res.Body.Close())
+
+		f, err = reg.VerificationFlowPersister().GetVerificationFlow(context.Background(), uuid.FromStringOrNil(gjson.GetBytes(resBody, "id").String()))
+		require.NoError(t, err)
+		assert.Equal(t, public.URL+verification.RouteInitBrowserFlow+"?return_to=https://www.ory.sh", f.RequestURL)
+	})
 }

test/e2e/cypress/integration/profiles/email/login/success.spec.js

@@ -1,4 +1,4 @@
-import { APP_URL, gen, password, website } from '../../../../helpers'
+import { APP_URL, gen, website } from '../../../../helpers'
 
 context('Email Profile', () => {
   describe('Login Flow Success', () => {
@@ -48,4 +48,46 @@ context('Email Profile', () => {
       })
     })
   })
+  describe('Login Flow Success with return_to url after flow expires', () => {
+    before(() => {
+      cy.useConfigProfile('email')
+    })
+
+    const email = gen.email()
+    const password = gen.password()
+
+    before(() => {
+      cy.registerApi({ email, password, fields: { 'traits.website': website } })
+    })
+
+    beforeEach(() => {
+      cy.shortLoginLifespan()
+      cy.browserReturnUrlOry()
+      cy.clearCookies()
+      cy.visit(
+        APP_URL + '/self-service/login/browser?return_to=https://www.ory.sh/'
+      )
+    })
+
+    it('should redirect to return_to after flow expires', () => {
+      cy.wait(105)
+      cy.get('input[name="password_identifier"]').type(email.toUpperCase())
+      cy.get('input[name="password"]').type(password)
+
+      cy.longLoginLifespan()
+      cy.get('button[type="submit"]').click()
+      cy.get('.messages .message').should(
+        'contain.text',
+        'The login flow expired'
+      )
+
+      // try again with long lifespan set
+      cy.get('input[name="password_identifier"]').type(email.toUpperCase())
+      cy.get('input[name="password"]').type(password)
+      cy.get('button[type="submit"]').click()
+
+      // check that redirection has happened
+      cy.url().should('eq', 'https://www.ory.sh/')
+    })
+  })
 })