From f4994b3a4fc41ce26e5b80a172d400ccb9bddc35 Mon Sep 17 00:00:00 2001
From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com>
Date: Thu, 8 Sep 2022 16:18:49 +0530
Subject: [PATCH] refactor: improved CORS support and caching (backport #18030)
 (#18065)

* refactor: improved CORS support and caching

(cherry picked from commit 23e8924a05e7eb60c878bb69831ec94c17b45b85)

* test: use `OPTIONS` method for CORS tests

(cherry picked from commit 5cb440c27f6230962c1f8b53eb306cea28c3b8b5)

* fix: only set allowed headers if required

(cherry picked from commit 51a39bd693f07316db2b4f4cee6ec685518cbd30)

* fix: set `Vary` header to tell browser that response differs based on origin

(cherry picked from commit 48196915f6e6b75b8929e8f91738cb1b3a4327b0)

* test: ensure that `Vary` header is specified

(cherry picked from commit 5db5396b49828aadc50c5b12ef5936ca0c60b54d)

Co-authored-by: Sagar Vora <sagar@resilient.tech>
---
 frappe/app.py             | 50 +++++++++++++++++++++++----------------
 frappe/tests/test_cors.py |  9 +++++--
 2 files changed, 37 insertions(+), 22 deletions(-)

diff --git a/frappe/app.py b/frappe/app.py
index 2ba6432a89..2176a1bc5a 100644
--- a/frappe/app.py
+++ b/frappe/app.py
@@ -157,35 +157,45 @@ def process_response(response):
 		response.headers.extend(frappe.local.rate_limiter.headers())
 
 	# CORS headers
-	if hasattr(frappe.local, "conf") and frappe.conf.allow_cors:
+	if hasattr(frappe.local, "conf"):
 		set_cors_headers(response)
 
 
 def set_cors_headers(response):
-	origin = frappe.request.headers.get("Origin")
-	allow_cors = frappe.conf.allow_cors
-	if not (origin and allow_cors):
+	if not (
+		(allowed_origins := frappe.conf.allow_cors)
+		and (request := frappe.local.request)
+		and (origin := request.headers.get("Origin"))
+	):
 		return
 
-	if allow_cors != "*":
-		if not isinstance(allow_cors, list):
-			allow_cors = [allow_cors]
+	if allowed_origins != "*":
+		if not isinstance(allowed_origins, list):
+			allowed_origins = [allowed_origins]
 
-		if origin not in allow_cors:
+		if origin not in allowed_origins:
 			return
 
-	response.headers.extend(
-		{
-			"Access-Control-Allow-Origin": origin,
-			"Access-Control-Allow-Credentials": "true",
-			"Access-Control-Allow-Methods": "GET, POST, PUT, DELETE, OPTIONS",
-			"Access-Control-Allow-Headers": (
-				"Authorization,DNT,X-Mx-ReqToken,"
-				"Keep-Alive,User-Agent,X-Requested-With,If-Modified-Since,"
-				"Cache-Control,Content-Type"
-			),
-		}
-	)
+	cors_headers = {
+		"Access-Control-Allow-Credentials": "true",
+		"Access-Control-Allow-Origin": origin,
+		"Vary": "Origin",
+	}
+
+	# only required for preflight requests
+	if request.method == "OPTIONS":
+		cors_headers["Access-Control-Allow-Methods"] = request.headers.get(
+			"Access-Control-Request-Method"
+		)
+
+		if allowed_headers := request.headers.get("Access-Control-Request-Headers"):
+			cors_headers["Access-Control-Allow-Headers"] = allowed_headers
+
+		# allow browsers to cache preflight requests for upto a day
+		if not frappe.conf.developer_mode:
+			cors_headers["Access-Control-Max-Age"] = "86400"
+
+	response.headers.extend(cors_headers)
 
 
 def make_form_dict(request):
diff --git a/frappe/tests/test_cors.py b/frappe/tests/test_cors.py
index 0712c8d24a..1974c174db 100644
--- a/frappe/tests/test_cors.py
+++ b/frappe/tests/test_cors.py
@@ -11,6 +11,7 @@ HEADERS = (
 	"Access-Control-Allow-Credentials",
 	"Access-Control-Allow-Methods",
 	"Access-Control-Allow-Headers",
+	"Vary",
 )
 
 
@@ -20,9 +21,13 @@ class TestCORS(FrappeTestCase):
 
 		headers = {}
 		if origin:
-			headers = {"Origin": origin}
+			headers = {
+				"Origin": origin,
+				"Access-Control-Request-Method": "POST",
+				"Access-Control-Request-Headers": "X-Test-Header",
+			}
 
-		frappe.utils.set_request(headers=headers)
+		frappe.utils.set_request(method="OPTIONS", headers=headers)
 
 		self.response = Response()
 		process_response(self.response)