From c9490c8927209b686aafe54b8a16207a8ef47ebe Mon Sep 17 00:00:00 2001
From: Fabian Meyer <3982806+meyfa@users.noreply.github.com>
Date: Tue, 4 Jan 2022 12:02:22 +0100
Subject: [PATCH] fix: send 404 instead of null response for unknown
 verification flows (#2102)

Fixes the verification handler to write the error, instead of nil object, when the flow does not exist. Adds tests for every handler to check proper behavior in that regard.

Closes #2099
---
 selfservice/flow/login/handler_test.go        | 8 ++++++++
 selfservice/flow/recovery/handler_test.go     | 8 ++++++++
 selfservice/flow/registration/handler_test.go | 8 ++++++++
 selfservice/flow/settings/handler_test.go     | 2 +-
 selfservice/flow/verification/handler.go      | 2 +-
 selfservice/flow/verification/handler_test.go | 9 +++++++++
 6 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/selfservice/flow/login/handler_test.go b/selfservice/flow/login/handler_test.go
index 02de9b64c17b..d19c7844b22d 100644
--- a/selfservice/flow/login/handler_test.go
+++ b/selfservice/flow/login/handler_test.go
@@ -557,4 +557,12 @@ func TestGetFlow(t *testing.T) {
 		require.NoError(t, err)
 		assert.Equal(t, public.URL+login.RouteInitBrowserFlow+"?return_to=https://www.ory.sh", f.RequestURL)
 	})
+
+	t.Run("case=not found", func(t *testing.T) {
+		client := testhelpers.NewClientWithCookies(t)
+		setupLoginUI(t, client)
+
+		res, _ := x.EasyGet(t, client, public.URL+login.RouteGetFlow+"?id="+x.NewUUID().String())
+		assert.EqualValues(t, http.StatusNotFound, res.StatusCode)
+	})
 }
diff --git a/selfservice/flow/recovery/handler_test.go b/selfservice/flow/recovery/handler_test.go
index 5dae11ac3657..a2d6091f2bf5 100644
--- a/selfservice/flow/recovery/handler_test.go
+++ b/selfservice/flow/recovery/handler_test.go
@@ -249,4 +249,12 @@ func TestGetFlow(t *testing.T) {
 		require.NoError(t, err)
 		assert.Equal(t, public.URL+recovery.RouteInitBrowserFlow+"?return_to=https://www.ory.sh", f.RequestURL)
 	})
+
+	t.Run("case=not found", func(t *testing.T) {
+		client := testhelpers.NewClientWithCookies(t)
+		setupRecoveryTS(t, client)
+
+		res, _ := x.EasyGet(t, client, public.URL+recovery.RouteGetFlow+"?id="+x.NewUUID().String())
+		assert.EqualValues(t, http.StatusNotFound, res.StatusCode)
+	})
 }
diff --git a/selfservice/flow/registration/handler_test.go b/selfservice/flow/registration/handler_test.go
index 27d0a23611f3..ed3fefe398b4 100644
--- a/selfservice/flow/registration/handler_test.go
+++ b/selfservice/flow/registration/handler_test.go
@@ -307,4 +307,12 @@ func TestGetFlow(t *testing.T) {
 		require.NoError(t, err)
 		assert.Equal(t, public.URL+registration.RouteInitBrowserFlow+"?return_to=https://www.ory.sh", f.RequestURL)
 	})
+
+	t.Run("case=not found", func(t *testing.T) {
+		client := testhelpers.NewClientWithCookies(t)
+		setupRegistrationUI(t, client)
+
+		res, _ := x.EasyGet(t, client, public.URL+registration.RouteGetFlow+"?id="+x.NewUUID().String())
+		assert.EqualValues(t, http.StatusNotFound, res.StatusCode)
+	})
 }
diff --git a/selfservice/flow/settings/handler_test.go b/selfservice/flow/settings/handler_test.go
index 1c5e6fdcea2b..dd89846df845 100644
--- a/selfservice/flow/settings/handler_test.go
+++ b/selfservice/flow/settings/handler_test.go
@@ -186,7 +186,7 @@ func TestHandler(t *testing.T) {
 	})
 
 	t.Run("endpoint=fetch", func(t *testing.T) {
-		t.Run("description=fetching a non-existent flow should return a 403 error", func(t *testing.T) {
+		t.Run("description=fetching a non-existent flow should return a 404 error", func(t *testing.T) {
 			_, _, err := testhelpers.NewSDKCustomClient(publicTS, otherUser).V0alpha2Api.GetSelfServiceSettingsFlow(context.Background()).Id("i-do-not-exist").Execute()
 			require.Error(t, err)
 
diff --git a/selfservice/flow/verification/handler.go b/selfservice/flow/verification/handler.go
index 3585f109daf8..8465f0589a09 100644
--- a/selfservice/flow/verification/handler.go
+++ b/selfservice/flow/verification/handler.go
@@ -234,7 +234,7 @@ func (h *Handler) fetch(w http.ResponseWriter, r *http.Request, _ httprouter.Par
 	rid := x.ParseUUID(r.URL.Query().Get("id"))
 	req, err := h.d.VerificationFlowPersister().GetVerificationFlow(r.Context(), rid)
 	if err != nil {
-		h.d.Writer().Write(w, r, req)
+		h.d.Writer().WriteError(w, r, err)
 		return
 	}
 
diff --git a/selfservice/flow/verification/handler_test.go b/selfservice/flow/verification/handler_test.go
index 49c6d55dc1f3..71333777bc73 100644
--- a/selfservice/flow/verification/handler_test.go
+++ b/selfservice/flow/verification/handler_test.go
@@ -132,6 +132,7 @@ func TestGetFlow(t *testing.T) {
 		require.NoError(t, err)
 		assert.Equal(t, public.URL+verification.RouteInitBrowserFlow+"?return_to=https://www.ory.sh", f.RequestURL)
 	})
+
 	t.Run("case=relative redirect when self-service verification ui is a relative URL", func(t *testing.T) {
 		router := x.NewRouterPublic()
 		ts, _ := testhelpers.NewKratosServerWithRouters(t, reg, router, x.NewRouterAdmin())
@@ -142,4 +143,12 @@ func TestGetFlow(t *testing.T) {
 			testhelpers.GetSelfServiceRedirectLocation(t, ts.URL+verification.RouteInitBrowserFlow),
 		)
 	})
+
+	t.Run("case=not found", func(t *testing.T) {
+		client := testhelpers.NewClientWithCookies(t)
+		_ = setupVerificationUI(t, client)
+
+		res, _ := x.EasyGet(t, client, public.URL+verification.RouteGetFlow+"?id="+x.NewUUID().String())
+		assert.EqualValues(t, http.StatusNotFound, res.StatusCode)
+	})
 }