fix!: use secret for auth. between servers (#36778)

* fix: use secret for auth. between servers

* fix(security): use redis for server auth.

* fix: use socket.io directly to fetch secret from redis

* refactor: Socket secret can be bench specific

- No need to keep it site specific.

* fix: don't return anything if secrets dont match

* test: rewrite test to factor in server-to-server communication only

---------

Co-authored-by: Ankush Menat <ankush@frappe.io>

frappe/realtime.py

@@ -121,9 +121,31 @@ def has_permission(doctype: str, name: str) -> bool:
 	return True
 
 
+SOCKETIO_SECRET_KEY = "socketio_auth_secret"
+
+
+def get_socketio_secret():
+	"""Generate socket.io secret and store in redis"""
+
+	from frappe.utils.background_jobs import get_redis_connection_without_auth
+
+	r = get_redis_connection_without_auth()
+	secret = r.get(SOCKETIO_SECRET_KEY)
+	if secret:
+		return secret.decode()
+
+	secret = frappe.generate_hash(length=32)
+	r.set(SOCKETIO_SECRET_KEY, secret)
+	return secret
+
+
 @frappe.whitelist(allow_guest=True)
 def get_user_info():
 	user_type = frappe.session.data.user_type
+	trusted_secret = get_socketio_secret()
+	provided_secret = frappe.get_request_header("X-Frappe-Socket-Secret")
+	if trusted_secret != provided_secret:
+		return {}
 	# For requests with Bearer tokens, user_type is not set in the session data
 	if not user_type:
 		user_type = frappe.get_cached_value("User", frappe.session.user, "user_type")

frappe/tests/test_api.py

@@ -305,11 +305,11 @@ class TestMethodAPI(FrappeAPITestCase):
 		self.assertEqual(response.json["message"], "pong")
 
 	def test_get_user_info(self):
-		# test 3: test for /api/method/frappe.realtime.get_user_info
+		# test 3: test for /api/method/frappe.realtime.get_user_info (server-to-server only)
 		response = self.get(self.method("frappe.realtime.get_user_info"))
 		self.assertEqual(response.status_code, 200)
-		self.assertIsInstance(response.json, dict)
+		message = response.json.get("message")
-		self.assertIn(response.json.get("message").get("user"), ("Administrator", "Guest"))
+		self.assertEqual(message, {})
 
 	def test_auth_cycle(self):
 		# test 4: Pass authorization token in request

frappe/tests/test_api_v2.py

@@ -161,10 +161,10 @@ class TestMethodAPIV2(FrappeAPITestCase):
 		self.assertEqual(response.json["data"], "pong")
 
 	def test_get_user_info(self):
+		# server-to-server only
 		response = self.get(self.method("frappe.realtime.get_user_info"))
 		self.assertEqual(response.status_code, 200)
-		self.assertIsInstance(response.json, dict)
+		self.assertEqual(response.json.get("data"), {})
-		self.assertIn(response.json.get("data").get("user"), ("Administrator", "Guest"))
 
 	def test_auth_cycle(self):
 		global authorization_token

realtime/middlewares/authenticate.js

@@ -1,7 +1,14 @@
 const cookie = require("cookie");
-const { get_conf } = require("../../node_utils");
+const { get_conf, get_redis_subscriber } = require("../../node_utils");
 const { get_url } = require("../utils");
 const conf = get_conf();
+const redisClient = get_redis_subscriber("redis_queue");
+
+async function getSecretFromRedis() {
+	if (!redisClient.isOpen) await redisClient.connect();
+	const val = await redisClient.get("socketio_auth_secret");
+	return val;
+}
 
 function authenticate_with_frappe(socket, next) {
 	let namespace = socket.nsp.name;
@@ -35,7 +42,7 @@ function authenticate_with_frappe(socket, next) {
 	socket.sid = cookies.sid;
 	socket.authorization_header = authorization_header;
 
-	socket.frappe_request = (path, args = {}, opts = {}) => {
+	socket.frappe_request = async (path, args = {}, opts = {}) => {
 		let query_args = new URLSearchParams(args);
 		if (query_args.toString()) {
 			path = path + "?" + query_args.toString();
@@ -47,7 +54,10 @@ function authenticate_with_frappe(socket, next) {
 		} else if (socket.sid) {
 			headers["Cookie"] = `sid=${socket.sid}`;
 		}
-
+		const secret = await getSecretFromRedis();
+		if (secret) {
+			headers["X-Frappe-Socket-Secret"] = secret;
+		}
 		return fetch(get_url(socket, path), {
 			...opts,
 			headers,
@@ -57,10 +67,18 @@ function authenticate_with_frappe(socket, next) {
 	socket
 		.frappe_request("/api/method/frappe.realtime.get_user_info")
 		.then((res) => res.json())
-		.then(({ message }) => {
+		.then(async ({ message }) => {
+			if (socket.user !== "Guest" && !message.installed_apps) {
+				const retry_res = await socket.frappe_request(
+					"/api/method/frappe.realtime.get_user_info"
+				);
+				const retry_data = await retry_res.json();
+				message = retry_data.message;
+			}
+
 			socket.user = message.user;
 			socket.user_type = message.user_type;
-			socket.installed_apps = message.installed_apps;
+			socket.installed_apps = message.installed_apps || [];
 			next();
 		})
 		.catch((e) => {