refactor: improved CORS support and caching (backport #18030) (#18065)

* refactor: improved CORS support and caching

(cherry picked from commit 23e8924a05)

* test: use `OPTIONS` method for CORS tests

(cherry picked from commit 5cb440c27f)

* fix: only set allowed headers if required

(cherry picked from commit 51a39bd693)

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

(cherry picked from commit 48196915f6)

* test: ensure that `Vary` header is specified

(cherry picked from commit 5db5396b49)

Co-authored-by: Sagar Vora <sagar@resilient.tech>

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):

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)