From f679dc3fdd64e8515a03f0d2d855a24bf41cd6b1 Mon Sep 17 00:00:00 2001
From: phot0n <ritwikpuri5678@gmail.com>
Date: Sun, 17 Jul 2022 11:14:01 +0530
Subject: [PATCH] fix(security): restrict the god google callback

the common google callback can be used to trigger any method in the whole codebase
restrict it by only allowing domain specific callback method and raise
an error if the domain is not found
---
 frappe/email/oauth.py                         |  1 -
 .../google_contacts/google_contacts.py        |  1 -
 .../doctype/google_drive/google_drive.py      |  1 -
 frappe/integrations/google_oauth.py           | 30 ++++++++++++++-----
 .../website_settings/google_indexing.py       |  1 -
 5 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/frappe/email/oauth.py b/frappe/email/oauth.py
index 9adeb2f9de..89b6df15d8 100644
--- a/frappe/email/oauth.py
+++ b/frappe/email/oauth.py
@@ -150,7 +150,6 @@ def authorize_google_access(email_account, doctype: str = "Email Account", code:
 	if not code:
 		return oauth_obj.get_authentication_url(
 			{
-				"method": "frappe.email.oauth.authorize_google_access",
 				"redirect": f"/app/Form/{quote(doctype)}/{quote(email_account)}",
 				"success_query_param": "successful_authorization=1",
 				"email_account": email_account,
diff --git a/frappe/integrations/doctype/google_contacts/google_contacts.py b/frappe/integrations/doctype/google_contacts/google_contacts.py
index c1f445b599..9a20d5e905 100644
--- a/frappe/integrations/doctype/google_contacts/google_contacts.py
+++ b/frappe/integrations/doctype/google_contacts/google_contacts.py
@@ -45,7 +45,6 @@ def authorize_access(g_contact, reauthorize=False, code=None):
 	if not oauth_code or reauthorize:
 		return oauth_obj.get_authentication_url(
 			{
-				"method": "frappe.integrations.doctype.google_contacts.google_contacts.authorize_access",
 				"g_contact": g_contact,
 				"redirect": f"/app/Form/{quote('Google Contacts')}/{quote(g_contact)}",
 			},
diff --git a/frappe/integrations/doctype/google_drive/google_drive.py b/frappe/integrations/doctype/google_drive/google_drive.py
index 62100ae7c5..6b1e03eccb 100644
--- a/frappe/integrations/doctype/google_drive/google_drive.py
+++ b/frappe/integrations/doctype/google_drive/google_drive.py
@@ -57,7 +57,6 @@ def authorize_access(reauthorize=False, code=None):
 			frappe.db.set_value("Google Drive", None, "backup_folder_id", "")
 		return oauth_obj.get_authentication_url(
 			{
-				"method": "frappe.integrations.doctype.google_drive.google_drive.authorize_access",
 				"redirect": f"/app/Form/{quote('Google Drive')}",
 			},
 		)
diff --git a/frappe/integrations/google_oauth.py b/frappe/integrations/google_oauth.py
index edce63493e..1d5ed3723f 100644
--- a/frappe/integrations/google_oauth.py
+++ b/frappe/integrations/google_oauth.py
@@ -19,6 +19,12 @@ _SERVICES = {
 	"drive": ("drive", "v3"),
 	"indexing": ("indexing", "v3"),
 }
+_DOMAIN_CALLBACK_METHODS = {
+	"mail": "frappe.email.oauth.authorize_google_access",
+	"contacts": "frappe.integrations.doctype.google_contacts.google_contacts.authorize_access",
+	"drive": "frappe.integrations.doctype.google_drive.google_drive.authorize_access",
+	"indexing": "frappe.website.doctype.website_settings.google_indexing.authorize_access",
+}
 
 
 class GoogleAuthenticationError(Exception):
@@ -100,6 +106,7 @@ class GoogleOAuth:
 		:param state: [optional] dict of values which you need on callback (for calling methods, redirection back to the form, doc name, etc)
 		"""
 
+		state.update({"domain": self.domain})
 		state = json.dumps(state)
 		callback_url = get_request_site_address(True) + CALLBACK_METHOD
 
@@ -175,13 +182,22 @@ def callback(state: str, code: str = None, error: str = None) -> None:
 	failure_query_param = state.pop("failure_query_param", "")
 
 	if not error:
-		state.update({"code": code})
-		frappe.get_attr(state.pop("method"))(**state)
+		if (domain := state.pop("domain")) in _DOMAIN_CALLBACK_METHODS:
+			state.update({"code": code})
+			frappe.get_attr(_DOMAIN_CALLBACK_METHODS[domain])(**state)
 
-		# GET request, hence using commit to persist changes
-		frappe.db.commit()  # nosemgrep
-
-	redirect = f"{redirect}?{failure_query_param if error else success_query_param}"
+			# GET request, hence using commit to persist changes
+			frappe.db.commit()  # nosemgrep
+		else:
+			return frappe.respond_as_web_page(
+				"Invalid Google Callback",
+				"The callback domain provided is not valid for Google Authentication",
+				http_status_code=400,
+				indicator_color="red",
+				width=640,
+			)
 
 	frappe.local.response["type"] = "redirect"
-	frappe.local.response["location"] = redirect
+	frappe.local.response[
+		"location"
+	] = f"{redirect}?{failure_query_param if error else success_query_param}"
diff --git a/frappe/website/doctype/website_settings/google_indexing.py b/frappe/website/doctype/website_settings/google_indexing.py
index 4f67949f86..9dbd415b02 100644
--- a/frappe/website/doctype/website_settings/google_indexing.py
+++ b/frappe/website/doctype/website_settings/google_indexing.py
@@ -26,7 +26,6 @@ def authorize_access(reauthorize=False, code=None):
 	if not oauth_code or reauthorize:
 		return oauth_obj.get_authentication_url(
 			{
-				"method": "frappe.website.doctype.website_settings.google_indexing.authorize_access",
 				"redirect": f"/app/Form/{quote('Website Settings')}",
 			},
 		)