fix!: Don't silently fail API auth

This commit is contained in:
Ankush Menat 2023-11-01 14:23:28 +05:30
parent b56cbdfff8
commit c4815ff987
3 changed files with 25 additions and 5 deletions

View file

@ -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():

View file

@ -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):

View file

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