fix: properly setup CSRF errors

ory · Dec 31, 2024 · b6a8f80 · b6a8f80
1 parent 18d7f5e
commit b6a8f80
Show file tree

Hide file tree

Showing 6 changed files with 337 additions and 118 deletions.
diff --git a/driver/registry_default.go b/driver/registry_default.go
@@ -826,7 +826,7 @@ func (m *RegistryDefault) WithCSRFTokenGenerator(cg x.CSRFToken) {
 
 func (m *RegistryDefault) GenerateCSRFToken(r *http.Request) string {
 	if m.csrfTokenGenerator == nil {
-		m.csrfTokenGenerator = x.DefaultCSRFToken
+		m.csrfTokenGenerator = x.DefaultCSRFTokenGenerator
 	}
 	return m.csrfTokenGenerator(r)
 }

diff --git a/selfservice/strategy/idfirst/strategy_login_test.go b/selfservice/strategy/idfirst/strategy_login_test.go
@@ -236,8 +236,7 @@ func TestCompleteLogin(t *testing.T) {
 
 			actual, res := testhelpers.LoginMakeRequest(t, false, false, f, browserClient, values.Encode())
 			assert.EqualValues(t, http.StatusOK, res.StatusCode)
-			assertx.EqualAsJSON(t, x.ErrInvalidCSRFToken,
-				json.RawMessage(actual), "%s", actual)
+			assertx.EqualAsJSON(t, x.ErrInvalidCSRFTokenServerNoCookies, json.RawMessage(actual), "%s", actual)
 		})
 
 		t.Run("case=should fail because of missing CSRF token/type=spa", func(t *testing.T) {

diff --git a/selfservice/strategy/profile/strategy_test.go b/selfservice/strategy/profile/strategy_test.go
@@ -85,7 +85,8 @@ func TestStrategyTraits(t *testing.T) {
 	ui := testhelpers.NewSettingsUIEchoServer(t, reg)
 	_ = testhelpers.NewErrorTestServer(t, reg)
 
-	publicTS, _ := testhelpers.NewKratosServer(t, reg)
+	publicRouter := x.NewRouterPublic()
+	publicTS, _ := testhelpers.NewKratosServerWithRouters(t, reg, publicRouter, x.NewRouterAdmin())
 
 	browserIdentity1 := newIdentityWithPassword("[email protected]")
 	apiIdentity1 := newIdentityWithPassword("[email protected]")
@@ -122,75 +123,85 @@ func TestStrategyTraits(t *testing.T) {
 		})
 	})
 
-	t.Run("description=should fail to post data if CSRF is invalid/type=browser", func(t *testing.T) {
-		setUnprivileged(t)
+	t.Run("csrf issues", func(t *testing.T) {
+		reg.WithCSRFHandler(x.NewCSRFHandler(publicRouter, reg))
+		reg.WithCSRFTokenGenerator(x.DefaultCSRFTokenGenerator)
 
-		f := testhelpers.InitializeSettingsFlowViaBrowser(t, browserUser1, false, publicTS)
+		t.Cleanup(func() {
+			reg.WithCSRFHandler(new(x.FakeCSRFHandler))
+			reg.WithCSRFTokenGenerator(x.FakeCSRFTokenGenerator)
+		})
 
-		actual, res := testhelpers.SettingsMakeRequest(t, false, false, f, browserUser1,
-			url.Values{"traits.booly": {"true"}, "csrf_token": {"invalid"}, "method": {"profile"}}.Encode())
-		assert.EqualValues(t, http.StatusOK, res.StatusCode, "should return a 400 error because CSRF token is not set\n\t%s", actual)
-		assertx.EqualAsJSON(t, x.ErrInvalidCSRFToken, json.RawMessage(actual), "%s", actual)
-	})
+		t.Run("description=should fail to post data if CSRF is invalid/type=browser", func(t *testing.T) {
+			setUnprivileged(t)
 
-	t.Run("description=should fail to post data if CSRF is invalid/type=spa", func(t *testing.T) {
-		setUnprivileged(t)
+			f := testhelpers.InitializeSettingsFlowViaBrowser(t, browserUser1, false, publicTS)
 
-		f := testhelpers.InitializeSettingsFlowViaBrowser(t, browserUser1, true, publicTS)
+			actual, res := testhelpers.SettingsMakeRequest(t, false, false, f, browserUser1,
+				url.Values{"traits.booly": {"true"}, "csrf_token": {"invalid"}, "method": {"profile"}}.Encode())
+			assert.EqualValues(t, http.StatusOK, res.StatusCode, "should return a 400 error because CSRF token is not set\n\t%s", actual)
+			assertx.EqualAsJSON(t, x.ErrInvalidCSRFToken, json.RawMessage(actual), "%s", actual)
+		})
 
-		actual, res := testhelpers.SettingsMakeRequest(t, false, true, f, browserUser1,
-			testhelpers.EncodeFormAsJSON(t, true, url.Values{"traits.booly": {"true"}, "csrf_token": {"invalid"}, "method": {"profile"}}))
-		assert.EqualValues(t, http.StatusForbidden, res.StatusCode, "should return a 400 error because CSRF token is not set\n\t%s", actual)
-		assertx.EqualAsJSON(t, x.ErrInvalidCSRFToken, json.RawMessage(gjson.Get(actual, "error").Raw), "%s", actual)
-	})
+		t.Run("description=should fail to post data if CSRF is invalid/type=spa", func(t *testing.T) {
+			setUnprivileged(t)
 
-	t.Run("description=should not fail because of CSRF token but because of unprivileged/type=api", func(t *testing.T) {
-		setUnprivileged(t)
+			f := testhelpers.InitializeSettingsFlowViaBrowser(t, browserUser1, true, publicTS)
 
-		f := testhelpers.InitializeSettingsFlowViaAPI(t, apiUser1, publicTS)
+			actual, res := testhelpers.SettingsMakeRequest(t, false, true, f, browserUser1,
+				testhelpers.EncodeFormAsJSON(t, true, url.Values{"traits.booly": {"true"}, "csrf_token": {"invalid"}, "method": {"profile"}}))
+			assert.EqualValues(t, http.StatusForbidden, res.StatusCode, "should return a 400 error because CSRF token is not set\n\t%s", actual)
+			assertx.EqualAsJSON(t, x.ErrInvalidCSRFToken, json.RawMessage(gjson.Get(actual, "error").Raw), "%s", actual)
+		})
 
-		actual, res := testhelpers.SettingsMakeRequest(t, true, false, f, apiUser1, `{"traits.booly":true,"method":"profile","csrf_token":"`+x.FakeCSRFToken+`"}`)
-		require.Len(t, res.Cookies(), 1)
-		assert.Equal(t, "ory_kratos_continuity", res.Cookies()[0].Name)
-		assert.EqualValues(t, http.StatusForbidden, res.StatusCode)
-		assert.Contains(t, gjson.Get(actual, "error.reason").String(), "login session is too old", actual)
-	})
+		t.Run("description=should not fail because of CSRF token but because of unprivileged/type=api", func(t *testing.T) {
+			setUnprivileged(t)
 
-	t.Run("case=should fail with correct CSRF error cause/type=api", func(t *testing.T) {
-		setPrivileged(t)
+			f := testhelpers.InitializeSettingsFlowViaAPI(t, apiUser1, publicTS)
+
+			actual, res := testhelpers.SettingsMakeRequest(t, true, false, f, apiUser1, `{"traits.booly":true,"method":"profile","csrf_token":"`+x.FakeCSRFToken+`"}`)
+			require.Len(t, res.Cookies(), 1)
+			assert.Equal(t, "ory_kratos_continuity", res.Cookies()[0].Name)
+			assert.EqualValues(t, http.StatusForbidden, res.StatusCode)
+			assert.Contains(t, gjson.Get(actual, "error.reason").String(), "login session is too old", actual)
+		})
 
-		for k, tc := range []struct {
-			mod func(http.Header)
-			exp string
-		}{
-			{
-				mod: func(h http.Header) {
-					h.Add("Cookie", "name=bar")
+		t.Run("case=should fail with correct CSRF error cause/type=api", func(t *testing.T) {
+			setPrivileged(t)
+
+			for k, tc := range []struct {
+				mod func(http.Header)
+				exp string
+			}{
+				{
+					mod: func(h http.Header) {
+						h.Add("Cookie", "name=bar")
+					},
+					exp: "The HTTP Request Header included the \\\"Cookie\\\" key",
 				},
-				exp: "The HTTP Request Header included the \\\"Cookie\\\" key",
-			},
-			{
-				mod: func(h http.Header) {
-					h.Add("Origin", "www.bar.com")
+				{
+					mod: func(h http.Header) {
+						h.Add("Origin", "www.bar.com")
+					},
+					exp: "The HTTP Request Header included the \\\"Origin\\\" key",
 				},
-				exp: "The HTTP Request Header included the \\\"Origin\\\" key",
-			},
-		} {
-			t.Run(fmt.Sprintf("case=%d", k), func(t *testing.T) {
-				f := testhelpers.InitializeSettingsFlowViaAPI(t, apiUser1, publicTS)
+			} {
+				t.Run(fmt.Sprintf("case=%d", k), func(t *testing.T) {
+					f := testhelpers.InitializeSettingsFlowViaAPI(t, apiUser1, publicTS)
 
-				req := testhelpers.NewRequest(t, true, "POST", f.Ui.Action, bytes.NewBufferString(`{"traits.booly":true,"method":"profile","csrf_token":"invalid"}`))
-				tc.mod(req.Header)
+					req := testhelpers.NewRequest(t, true, "POST", f.Ui.Action, bytes.NewBufferString(`{"traits.booly":true,"method":"profile","csrf_token":"invalid"}`))
+					tc.mod(req.Header)
 
-				res, err := apiUser1.Do(req)
-				require.NoError(t, err)
-				defer res.Body.Close()
+					res, err := apiUser1.Do(req)
+					require.NoError(t, err)
+					defer res.Body.Close()
 
-				actual := string(ioutilx.MustReadAll(res.Body))
-				assert.EqualValues(t, http.StatusBadRequest, res.StatusCode)
-				assert.Contains(t, actual, tc.exp, "%s", actual)
-			})
-		}
+					actual := string(ioutilx.MustReadAll(res.Body))
+					assert.EqualValues(t, http.StatusBadRequest, res.StatusCode)
+					assert.Contains(t, actual, tc.exp, "%s", actual)
+				})
+			}
+		})
 	})
 
 	t.Run("description=hydrate the proper fields", func(t *testing.T) {

diff --git a/x/.snapshots/TestSnapshotCsrfErrors.json b/x/.snapshots/TestSnapshotCsrfErrors.json
@@ -0,0 +1,162 @@
+[
+  {
+    "Id": "ErrInvalidCSRFToken",
+    "Err": {
+      "id": "security_csrf_violation",
+      "code": 403,
+      "status": "Forbidden",
+      "reason": "Please retry the flow and optionally clear your cookies. The request was rejected to protect you from Cross-Site-Request-Forgery (CSRF) which could cause account takeover, leaking personal information, and other serious security issues.",
+      "details": {
+        "docs": "https://www.ory.sh/docs/kratos/debug/csrf",
+        "hint": ""
+      },
+      "message": "The request was rejected to protect you from Cross-Site-Request-Forgery (CSRF) which could cause account takeover, leaking personal information, and other serious security issues."
+    }
+  },
+  {
+    "Id": "ErrInvalidCSRFTokenAJAX",
+    "Err": {
+      "id": "security_csrf_violation",
+      "code": 403,
+      "status": "Forbidden",
+      "reason": "Please retry the flow and optionally clear your cookies. The request was rejected to protect you from Cross-Site-Request-Forgery (CSRF) which could cause account takeover, leaking personal information, and other serious security issues.",
+      "details": {
+        "docs": "https://www.ory.sh/docs/kratos/debug/csrf",
+        "hint": "We detected an AJAX call, please ensure that CORS is enabled and configured correctly, and that your AJAX code sends cookies and has credentials enabled. For further debugging, check your browser's network tab to see what cookies are included or excluded."
+      },
+      "message": "The request was rejected to protect you from Cross-Site-Request-Forgery (CSRF) which could cause account takeover, leaking personal information, and other serious security issues."
+    }
+  },
+  {
+    "Id": "ErrInvalidCSRFTokenAJAXCookieMissing",
+    "Err": {
+      "id": "security_csrf_violation",
+      "code": 403,
+      "status": "Forbidden",
+      "reason": "Please retry the flow and optionally clear your cookies. The request was rejected to protect you from Cross-Site-Request-Forgery (CSRF) which could cause account takeover, leaking personal information, and other serious security issues.",
+      "details": {
+        "docs": "https://www.ory.sh/docs/kratos/debug/csrf",
+        "hint": "We detected an AJAX call, please ensure that CORS is enabled and configured correctly, and that your AJAX code sends cookies and has credentials enabled. For further debugging, check your browser's network tab to see what cookies are included or excluded.",
+        "reject_reason": "The HTTP cookie header was set but did not include the anti-CSRF cookie."
+      },
+      "message": "The request was rejected to protect you from Cross-Site-Request-Forgery (CSRF) which could cause account takeover, leaking personal information, and other serious security issues."
+    }
+  },
+  {
+    "Id": "ErrInvalidCSRFTokenAJAXNoCookies",
+    "Err": {
+      "id": "security_csrf_violation",
+      "code": 403,
+      "status": "Forbidden",
+      "reason": "Please retry the flow and optionally clear your cookies. The request was rejected to protect you from Cross-Site-Request-Forgery (CSRF) which could cause account takeover, leaking personal information, and other serious security issues.",
+      "details": {
+        "docs": "https://www.ory.sh/docs/kratos/debug/csrf",
+        "hint": "We detected an AJAX call, please ensure that CORS is enabled and configured correctly, and that your AJAX code sends cookies and has credentials enabled. For further debugging, check your browser's network tab to see what cookies are included or excluded.",
+        "reject_reason": "The HTTP cookie header is empty or not set."
+      },
+      "message": "The request was rejected to protect you from Cross-Site-Request-Forgery (CSRF) which could cause account takeover, leaking personal information, and other serious security issues."
+    }
+  },
+  {
+    "Id": "ErrInvalidCSRFTokenAJAXTokenMismatch",
+    "Err": {
+      "id": "security_csrf_violation",
+      "code": 403,
+      "status": "Forbidden",
+      "reason": "Please retry the flow and optionally clear your cookies. The request was rejected to protect you from Cross-Site-Request-Forgery (CSRF) which could cause account takeover, leaking personal information, and other serious security issues.",
+      "details": {
+        "docs": "https://www.ory.sh/docs/kratos/debug/csrf",
+        "hint": "We detected an AJAX call, please ensure that CORS is enabled and configured correctly, and that your AJAX code sends cookies and has credentials enabled. For further debugging, check your browser's network tab to see what cookies are included or excluded.",
+        "reject_reason": "The HTTP cookie header was set and a CSRF token was sent but they do not match. We recommend deleting all cookies for this domain and retrying the flow."
+      },
+      "message": "The request was rejected to protect you from Cross-Site-Request-Forgery (CSRF) which could cause account takeover, leaking personal information, and other serious security issues."
+    }
+  },
+  {
+    "Id": "ErrInvalidCSRFTokenAJAXTokenNotSent",
+    "Err": {
+      "id": "security_csrf_violation",
+      "code": 403,
+      "status": "Forbidden",
+      "reason": "Please retry the flow and optionally clear your cookies. The request was rejected to protect you from Cross-Site-Request-Forgery (CSRF) which could cause account takeover, leaking personal information, and other serious security issues.",
+      "details": {
+        "docs": "https://www.ory.sh/docs/kratos/debug/csrf",
+        "hint": "The anti-CSRF cookie was found but the CSRF token was not included in the HTTP request body (csrf_token) nor in the HTTP header (X-CSRF-Token)."
+      },
+      "message": "The request was rejected to protect you from Cross-Site-Request-Forgery (CSRF) which could cause account takeover, leaking personal information, and other serious security issues."
+    }
+  },
+  {
+    "Id": "ErrInvalidCSRFTokenServer",
+    "Err": {
+      "id": "security_csrf_violation",
+      "code": 403,
+      "status": "Forbidden",
+      "reason": "Please retry the flow and optionally clear your cookies. The request was rejected to protect you from Cross-Site-Request-Forgery (CSRF) which could cause account takeover, leaking personal information, and other serious security issues.",
+      "details": {
+        "docs": "https://www.ory.sh/docs/kratos/debug/csrf",
+        "hint": "We detected a regular browser or server-side call. To debug browser calls check your browser's network tab to see what cookies are included or excluded. If you are calling from a server ensure that the appropriate cookies are being forwarded and that the SDK method is called correctly."
+      },
+      "message": "The request was rejected to protect you from Cross-Site-Request-Forgery (CSRF) which could cause account takeover, leaking personal information, and other serious security issues."
+    }
+  },
+  {
+    "Id": "ErrInvalidCSRFTokenServerCookieMissing",
+    "Err": {
+      "id": "security_csrf_violation",
+      "code": 403,
+      "status": "Forbidden",
+      "reason": "Please retry the flow and optionally clear your cookies. The request was rejected to protect you from Cross-Site-Request-Forgery (CSRF) which could cause account takeover, leaking personal information, and other serious security issues.",
+      "details": {
+        "docs": "https://www.ory.sh/docs/kratos/debug/csrf",
+        "hint": "We detected a regular browser or server-side call. To debug browser calls check your browser's network tab to see what cookies are included or excluded. If you are calling from a server ensure that the appropriate cookies are being forwarded and that the SDK method is called correctly.",
+        "reject_reason": "The HTTP cookie header was set but did not include the anti-CSRF cookie."
+      },
+      "message": "The request was rejected to protect you from Cross-Site-Request-Forgery (CSRF) which could cause account takeover, leaking personal information, and other serious security issues."
+    }
+  },
+  {
+    "Id": "ErrInvalidCSRFTokenServerNoCookies",
+    "Err": {
+      "id": "security_csrf_violation",
+      "code": 403,
+      "status": "Forbidden",
+      "reason": "Please retry the flow and optionally clear your cookies. The request was rejected to protect you from Cross-Site-Request-Forgery (CSRF) which could cause account takeover, leaking personal information, and other serious security issues.",
+      "details": {
+        "docs": "https://www.ory.sh/docs/kratos/debug/csrf",
+        "hint": "We detected a regular browser or server-side call. To debug browser calls check your browser's network tab to see what cookies are included or excluded. If you are calling from a server ensure that the appropriate cookies are being forwarded and that the SDK method is called correctly.",
+        "reject_reason": "The HTTP cookie header is empty or not set."
+      },
+      "message": "The request was rejected to protect you from Cross-Site-Request-Forgery (CSRF) which could cause account takeover, leaking personal information, and other serious security issues."
+    }
+  },
+  {
+    "Id": "ErrInvalidCSRFTokenServerTokenMismatch",
+    "Err": {
+      "id": "security_csrf_violation",
+      "code": 403,
+      "status": "Forbidden",
+      "reason": "Please retry the flow and optionally clear your cookies. The request was rejected to protect you from Cross-Site-Request-Forgery (CSRF) which could cause account takeover, leaking personal information, and other serious security issues.",
+      "details": {
+        "docs": "https://www.ory.sh/docs/kratos/debug/csrf",
+        "hint": "We detected a regular browser or server-side call. To debug browser calls check your browser's network tab to see what cookies are included or excluded. If you are calling from a server ensure that the appropriate cookies are being forwarded and that the SDK method is called correctly.",
+        "reject_reason": "The HTTP cookie header was set and a CSRF token was sent but they do not match. We recommend deleting all cookies for this domain and retrying the flow."
+      },
+      "message": "The request was rejected to protect you from Cross-Site-Request-Forgery (CSRF) which could cause account takeover, leaking personal information, and other serious security issues."
+    }
+  },
+  {
+    "Id": "ErrInvalidCSRFTokenServerTokenNotSent",
+    "Err": {
+      "id": "security_csrf_violation",
+      "code": 403,
+      "status": "Forbidden",
+      "reason": "Please retry the flow and optionally clear your cookies. The request was rejected to protect you from Cross-Site-Request-Forgery (CSRF) which could cause account takeover, leaking personal information, and other serious security issues.",
+      "details": {
+        "docs": "https://www.ory.sh/docs/kratos/debug/csrf",
+        "hint": "The anti-CSRF cookie was found but the CSRF token was not included in the HTTP request body (csrf_token) nor in the HTTP header (X-CSRF-Token)."
+      },
+      "message": "The request was rejected to protect you from Cross-Site-Request-Forgery (CSRF) which could cause account takeover, leaking personal information, and other serious security issues."
+    }
+  }
+]