diff --git a/frappe/auth.py b/frappe/auth.py index 35e07236bb..4b53e76533 100644 --- a/frappe/auth.py +++ b/frappe/auth.py @@ -20,7 +20,7 @@ from frappe.twofactor import ( ) from frappe.utils import cint, date_diff, datetime, get_datetime, today from frappe.utils.deprecations import deprecation_warning -from frappe.utils.password import check_password +from frappe.utils.password import check_password, get_decrypted_password from frappe.website.utils import get_home_page SAFE_HTTP_METHODS = frozenset(("GET", "HEAD", "OPTIONS")) @@ -574,6 +574,11 @@ def validate_auth(): validate_oauth(authorization_header) validate_auth_via_api_keys(authorization_header) + # If login via bearer, basic or keypair didn't work then authentication failed and we + # should terminate here. + if frappe.session.user in ("", "Guest"): + raise frappe.AuthenticationError + validate_auth_via_hooks() @@ -588,6 +593,9 @@ def validate_oauth(authorization_header): from frappe.integrations.oauth2 import get_oauth_server from frappe.oauth import get_url_delimiter + if authorization_header[0].lower() != "bearer": + return + form_dict = frappe.local.form_dict token = authorization_header[1] req = frappe.request @@ -613,7 +621,7 @@ def validate_oauth(authorization_header): frappe.set_user(frappe.db.get_value("OAuth Bearer Token", token, "user")) frappe.local.form_dict = form_dict except AttributeError: - pass + raise frappe.AuthenticationError def validate_auth_via_api_keys(authorization_header): @@ -639,15 +647,17 @@ def validate_auth_via_api_keys(authorization_header): frappe.InvalidAuthorizationToken, ) except (AttributeError, TypeError, ValueError): - pass + raise frappe.AuthenticationError def validate_api_key_secret(api_key, api_secret, frappe_authorization_source=None): """frappe_authorization_source to provide api key and secret for a doctype apart from User""" doctype = frappe_authorization_source or "User" doc = frappe.db.get_value(doctype=doctype, filters={"api_key": api_key}, fieldname=["name"]) + if not doc: + raise frappe.AuthenticationError form_dict = frappe.local.form_dict - doc_secret = frappe.utils.password.get_decrypted_password(doctype, doc, fieldname="api_secret") + doc_secret = get_decrypted_password(doctype, doc, fieldname="api_secret") if api_secret == doc_secret: if doctype == "User": user = frappe.db.get_value(doctype="User", filters={"api_key": api_key}, fieldname=["name"]) @@ -656,6 +666,8 @@ def validate_api_key_secret(api_key, api_secret, frappe_authorization_source=Non if frappe.local.login_manager.user in ("", "Guest"): frappe.set_user(user) frappe.local.form_dict = form_dict + else: + raise frappe.AuthenticationError def validate_auth_via_hooks(): diff --git a/frappe/tests/test_api.py b/frappe/tests/test_api.py index 9b14b308ed..b415a57f77 100644 --- a/frappe/tests/test_api.py +++ b/frappe/tests/test_api.py @@ -278,6 +278,14 @@ class TestMethodAPI(FrappeAPITestCase): self.assertEqual(response.status_code, 200) self.assertEqual(response.json["message"], "Administrator") + authorization_token = f"{api_key}:INCORRECT" + response = self.get(self.method_path("frappe.auth.get_logged_user")) + self.assertEqual(response.status_code, 401) + + authorization_token = f"NonExistentKey:INCORRECT" + response = self.get(self.method_path("frappe.auth.get_logged_user")) + self.assertEqual(response.status_code, 401) + authorization_token = None def test_404s(self): diff --git a/frappe/tests/test_frappe_client.py b/frappe/tests/test_frappe_client.py index c8f3bee3d9..e1886fcbf0 100644 --- a/frappe/tests/test_frappe_client.py +++ b/frappe/tests/test_frappe_client.py @@ -196,7 +196,7 @@ class TestFrappeClient(FrappeTestCase): api_secret = "ksk&93nxoe3os" header = {"Authorization": f"token {api_key}:{api_secret}"} res = requests.post(get_url() + "/api/method/frappe.auth.get_logged_user", headers=header) - self.assertEqual(res.status_code, 403) + self.assertEqual(res.status_code, 401) # random api key and api secret api_key = "@3djdk3kld"