From f3f01d197893cfdfac6e4cd3e979d30d59c336ac Mon Sep 17 00:00:00 2001 From: Jannat Patel Date: Wed, 3 Nov 2021 14:58:16 +0530 Subject: [PATCH 1/6] fix: github api endpoint --- .../doctype/social_login_key/social_login_key.py | 6 ++++-- frappe/patches.txt | 1 + frappe/patches/v14_0/update_github_endpoints.py | 11 +++++++++++ frappe/utils/oauth.py | 3 +++ 4 files changed, 19 insertions(+), 2 deletions(-) create mode 100644 frappe/patches/v14_0/update_github_endpoints.py diff --git a/frappe/integrations/doctype/social_login_key/social_login_key.py b/frappe/integrations/doctype/social_login_key/social_login_key.py index d6f55e5758..92bdacde72 100644 --- a/frappe/integrations/doctype/social_login_key/social_login_key.py +++ b/frappe/integrations/doctype/social_login_key/social_login_key.py @@ -78,9 +78,11 @@ class SocialLoginKey(Document): "authorize_url":"https://github.com/login/oauth/authorize", "access_token_url":"https://github.com/login/oauth/access_token", "redirect_url":"/api/method/frappe.www.login.login_via_github", - "api_endpoint":"user", + "api_endpoint":"user/emails", "api_endpoint_args":None, - "auth_url_data":None + "auth_url_data": json.dumps({ + "scope": "user:email" + }) } providers["Google"] = { diff --git a/frappe/patches.txt b/frappe/patches.txt index c1b654d0e8..29dc4f77b7 100644 --- a/frappe/patches.txt +++ b/frappe/patches.txt @@ -184,3 +184,4 @@ frappe.patches.v13_0.update_notification_channel_if_empty frappe.patches.v14_0.drop_data_import_legacy frappe.patches.v14_0.rename_cancelled_documents frappe.patches.v14_0.update_workspace2 # 20.09.2021 +frappe.patches.v14_0.update_github_endpoints diff --git a/frappe/patches/v14_0/update_github_endpoints.py b/frappe/patches/v14_0/update_github_endpoints.py new file mode 100644 index 0000000000..b7ac67655e --- /dev/null +++ b/frappe/patches/v14_0/update_github_endpoints.py @@ -0,0 +1,11 @@ +import frappe +import json + +def execute(): + if frappe.db.exists("Social Login Key", "github"): + frappe.db.set_value("Social Login Key", "github", "api_endpoint", "user/emails") + frappe.db.set_value("Social Login Key", "github", "auth_url_data", + json.dumps({ + "scope": "user:email" + }) + ) diff --git a/frappe/utils/oauth.py b/frappe/utils/oauth.py index c28663d138..04d0cb1b1a 100644 --- a/frappe/utils/oauth.py +++ b/frappe/utils/oauth.py @@ -140,6 +140,9 @@ def get_info_via_oauth(provider, code, decoder=None, id_token=False): api_endpoint_args = oauth2_providers[provider].get("api_endpoint_args") info = session.get(api_endpoint, params=api_endpoint_args).json() + if provider == "github": + info = list(filter(lambda x: x.get("primary") == True, info))[0] + if not (info.get("email_verified") or info.get("email")): frappe.throw(_("Email not verified with {0}").format(provider.title())) From 49e084868b8e066defd71ef060f867ad7bb79315 Mon Sep 17 00:00:00 2001 From: Jannat Patel Date: Wed, 3 Nov 2021 15:24:23 +0530 Subject: [PATCH 2/6] fix: indentation and filter condition --- frappe/patches/v14_0/update_github_endpoints.py | 2 +- frappe/utils/oauth.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/patches/v14_0/update_github_endpoints.py b/frappe/patches/v14_0/update_github_endpoints.py index b7ac67655e..c3a9285a78 100644 --- a/frappe/patches/v14_0/update_github_endpoints.py +++ b/frappe/patches/v14_0/update_github_endpoints.py @@ -6,6 +6,6 @@ def execute(): frappe.db.set_value("Social Login Key", "github", "api_endpoint", "user/emails") frappe.db.set_value("Social Login Key", "github", "auth_url_data", json.dumps({ - "scope": "user:email" + "scope": "user:email" }) ) diff --git a/frappe/utils/oauth.py b/frappe/utils/oauth.py index 04d0cb1b1a..68d62129bd 100644 --- a/frappe/utils/oauth.py +++ b/frappe/utils/oauth.py @@ -141,7 +141,7 @@ def get_info_via_oauth(provider, code, decoder=None, id_token=False): info = session.get(api_endpoint, params=api_endpoint_args).json() if provider == "github": - info = list(filter(lambda x: x.get("primary") == True, info))[0] + info = list(filter(lambda x: x.get("primary"), info))[0] if not (info.get("email_verified") or info.get("email")): frappe.throw(_("Email not verified with {0}").format(provider.title())) From 88d8c1e56a7227c18deb12901cd96a4d855c0a82 Mon Sep 17 00:00:00 2001 From: Jannat Patel Date: Mon, 8 Nov 2021 20:17:07 +0530 Subject: [PATCH 3/6] fix: public info api --- .../doctype/social_login_key/social_login_key.py | 2 +- frappe/patches.txt | 2 +- frappe/patches/v14_0/update_github_endpoints.py | 1 - frappe/utils/oauth.py | 7 +++++-- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/frappe/integrations/doctype/social_login_key/social_login_key.py b/frappe/integrations/doctype/social_login_key/social_login_key.py index 92bdacde72..195d6800be 100644 --- a/frappe/integrations/doctype/social_login_key/social_login_key.py +++ b/frappe/integrations/doctype/social_login_key/social_login_key.py @@ -78,7 +78,7 @@ class SocialLoginKey(Document): "authorize_url":"https://github.com/login/oauth/authorize", "access_token_url":"https://github.com/login/oauth/access_token", "redirect_url":"/api/method/frappe.www.login.login_via_github", - "api_endpoint":"user/emails", + "api_endpoint":"user", "api_endpoint_args":None, "auth_url_data": json.dumps({ "scope": "user:email" diff --git a/frappe/patches.txt b/frappe/patches.txt index 29dc4f77b7..8309b2df57 100644 --- a/frappe/patches.txt +++ b/frappe/patches.txt @@ -184,4 +184,4 @@ frappe.patches.v13_0.update_notification_channel_if_empty frappe.patches.v14_0.drop_data_import_legacy frappe.patches.v14_0.rename_cancelled_documents frappe.patches.v14_0.update_workspace2 # 20.09.2021 -frappe.patches.v14_0.update_github_endpoints +frappe.patches.v14_0.update_github_endpoints #08-11-2021 diff --git a/frappe/patches/v14_0/update_github_endpoints.py b/frappe/patches/v14_0/update_github_endpoints.py index c3a9285a78..8f9a06a043 100644 --- a/frappe/patches/v14_0/update_github_endpoints.py +++ b/frappe/patches/v14_0/update_github_endpoints.py @@ -3,7 +3,6 @@ import json def execute(): if frappe.db.exists("Social Login Key", "github"): - frappe.db.set_value("Social Login Key", "github", "api_endpoint", "user/emails") frappe.db.set_value("Social Login Key", "github", "auth_url_data", json.dumps({ "scope": "user:email" diff --git a/frappe/utils/oauth.py b/frappe/utils/oauth.py index 68d62129bd..df2f5dca62 100644 --- a/frappe/utils/oauth.py +++ b/frappe/utils/oauth.py @@ -138,10 +138,13 @@ def get_info_via_oauth(provider, code, decoder=None, id_token=False): else: api_endpoint = oauth2_providers[provider].get("api_endpoint") api_endpoint_args = oauth2_providers[provider].get("api_endpoint_args") + info = session.get(api_endpoint, params=api_endpoint_args).json() - if provider == "github": - info = list(filter(lambda x: x.get("primary"), info))[0] + if provider == "github" and not info.get("email"): + emails = session.get("/user/emails", params=api_endpoint_args).json() + email_dict = list(filter(lambda x: x.get("primary"), emails))[0] + info["email"] = email_dict.get("email") if not (info.get("email_verified") or info.get("email")): frappe.throw(_("Email not verified with {0}").format(provider.title())) From 08faf731e0b4ac005c302d57d40472306d049a20 Mon Sep 17 00:00:00 2001 From: Jannat Patel Date: Tue, 9 Nov 2021 13:38:17 +0530 Subject: [PATCH 4/6] test: github login --- .../social_login_key/test_social_login_key.py | 86 +++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/frappe/integrations/doctype/social_login_key/test_social_login_key.py b/frappe/integrations/doctype/social_login_key/test_social_login_key.py index 880f1ee99c..60d660a701 100644 --- a/frappe/integrations/doctype/social_login_key/test_social_login_key.py +++ b/frappe/integrations/doctype/social_login_key/test_social_login_key.py @@ -4,6 +4,12 @@ import frappe from frappe.integrations.doctype.social_login_key.social_login_key import BaseUrlNotSetError import unittest +from frappe.utils.oauth import login_via_oauth2 +from unittest.mock import patch, Mock, MagicMock +from rauth import OAuth2Service +from frappe.auth import LoginManager, CookieManager +from frappe.utils import set_request + class TestSocialLoginKey(unittest.TestCase): def test_adding_frappe_social_login_provider(self): @@ -14,6 +20,41 @@ class TestSocialLoginKey(unittest.TestCase): social_login_key.get_social_login_provider(provider_name, initialize=True) self.assertRaises(BaseUrlNotSetError, social_login_key.insert) + def test_github_login_with_private_email(self): + github_social_login_setup() + + mock_session = MagicMock() + mock_session.get.side_effect = github_response_for_private_email + + with patch.object(OAuth2Service, "get_auth_session", return_value=mock_session): + login_via_oauth2("github", "iwriu", {"token": "ewrwerwer"}) # Dummy code and state token + + def test_github_login_with_public_email(self): + github_social_login_setup() + + mock_session = MagicMock() + mock_session.get.side_effect = github_response_for_public_email + + with patch.object(OAuth2Service, "get_auth_session", return_value=mock_session): + login_via_oauth2("github", "iwriu", {"token": "ewrwerwer"}) # Dummy code and state token + + def test_normal_signup_and_github_login(self): + github_social_login_setup() + + if not frappe.db.exists("User", "githublogin@example.com"): + user = frappe.get_doc({ + "doctype": "User", + "email": "githublogin@example.com", + "first_name": "GitHub Login" + }) + user.save(ignore_permissions=True) + + mock_session = MagicMock() + mock_session.get.side_effect = github_response_for_login + + with patch.object(OAuth2Service, "get_auth_session", return_value=mock_session): + login_via_oauth2("github", "iwriu", {"token": "ewrwerwer"}) + def make_social_login_key(**kwargs): kwargs["doctype"] = "Social Login Key" if not "provider_name" in kwargs: @@ -34,3 +75,48 @@ def create_or_update_social_login_key(): frappe.db.commit() return social_login_key + +def create_github_social_login_key(): + if frappe.db.exists("Social Login Key", "github"): + return frappe.get_doc("Social Login Key", "github") + else: + provider_name = "GitHub" + social_login_key = make_social_login_key( + social_login_provider=provider_name + ) + social_login_key.get_social_login_provider(provider_name, initialize=True) + + # Dummy client_id and client_secret + social_login_key.client_id = "h6htd6q" + social_login_key.client_secret = "keoererk988ekkhf8w9e8ewrjhhkjer9889" + social_login_key.insert() + return social_login_key + +def github_response_for_private_email(url, *args, **kwargs): + if url == "user": + return_value = {"login": "dummy_username", "id": "223342", "email": None, "first_name": "Github Private"} + else: + return_value = [{"email": "github@example.com", "primary": True, "verified": True}] + + return MagicMock(status_code=200, json=MagicMock(return_value=return_value)) + +def github_response_for_public_email(url, *args, **kwargs): + if url == "user": + return_value = {"login": "dummy_username", "id": "223343", "email": "github_public@example.com", "first_name": "Github Public"} + + return MagicMock(status_code=200, json=MagicMock(return_value=return_value)) + +def github_response_for_login(url, *args, **kwargs): + if url == "user": + return_value = {"login": "dummy_username", "id": "223346", "email": None, "first_name": "Github Login"} + else: + return_value = [{"email": "githublogin@example.com", "primary": True, "verified": True}] + + return MagicMock(status_code=200, json=MagicMock(return_value=return_value)) + +def github_social_login_setup(): + set_request(path="/random") + frappe.local.cookie_manager = CookieManager() + frappe.local.login_manager = LoginManager() + + create_github_social_login_key() From 845c5894979c93a26703242cc59587ebc4228965 Mon Sep 17 00:00:00 2001 From: Jannat Patel Date: Tue, 9 Nov 2021 14:33:41 +0530 Subject: [PATCH 5/6] test: ignore permissions --- .../doctype/social_login_key/test_social_login_key.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/integrations/doctype/social_login_key/test_social_login_key.py b/frappe/integrations/doctype/social_login_key/test_social_login_key.py index 60d660a701..e85752bdda 100644 --- a/frappe/integrations/doctype/social_login_key/test_social_login_key.py +++ b/frappe/integrations/doctype/social_login_key/test_social_login_key.py @@ -89,7 +89,7 @@ def create_github_social_login_key(): # Dummy client_id and client_secret social_login_key.client_id = "h6htd6q" social_login_key.client_secret = "keoererk988ekkhf8w9e8ewrjhhkjer9889" - social_login_key.insert() + social_login_key.insert(ignore_permissions=True) return social_login_key def github_response_for_private_email(url, *args, **kwargs): From 7771e0b2c9f5a7e5a0a835d5c62552f6a61b2002 Mon Sep 17 00:00:00 2001 From: Jannat Patel Date: Tue, 9 Nov 2021 14:49:27 +0530 Subject: [PATCH 6/6] fix: removed unused imports --- .../doctype/social_login_key/test_social_login_key.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/integrations/doctype/social_login_key/test_social_login_key.py b/frappe/integrations/doctype/social_login_key/test_social_login_key.py index e85752bdda..73e6a072cb 100644 --- a/frappe/integrations/doctype/social_login_key/test_social_login_key.py +++ b/frappe/integrations/doctype/social_login_key/test_social_login_key.py @@ -5,7 +5,7 @@ import frappe from frappe.integrations.doctype.social_login_key.social_login_key import BaseUrlNotSetError import unittest from frappe.utils.oauth import login_via_oauth2 -from unittest.mock import patch, Mock, MagicMock +from unittest.mock import patch, MagicMock from rauth import OAuth2Service from frappe.auth import LoginManager, CookieManager from frappe.utils import set_request