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
This commit is contained in:
phot0n 2022-07-17 11:14:01 +05:30
parent 8edae2ce09
commit f679dc3fdd
5 changed files with 23 additions and 11 deletions

View file

@ -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,

View file

@ -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)}",
},

View file

@ -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')}",
},
)

View file

@ -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}"

View file

@ -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')}",
},
)