From ae4eb8745824dcd86bc943f23d01156258d78a27 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 29 Apr 2024 12:09:26 +0530 Subject: [PATCH] feat: Limit OAuth Client by roles --- frappe/core/doctype/user/user.js | 2 +- frappe/core/doctype/user/user.py | 2 +- .../doctype/oauth_client/oauth_client.json | 11 +++++-- .../doctype/oauth_client/oauth_client.py | 13 ++++++++ .../doctype/oauth_client/patches/__init__.py | 0 ...et_default_allowed_role_in_oauth_client.py | 11 +++++++ .../doctype/oauth_client_role/__init__.py | 0 .../oauth_client_role/oauth_client_role.json | 31 +++++++++++++++++++ .../oauth_client_role/oauth_client_role.py | 23 ++++++++++++++ frappe/oauth.py | 9 +++--- frappe/patches.txt | 1 + 11 files changed, 95 insertions(+), 8 deletions(-) create mode 100644 frappe/integrations/doctype/oauth_client/patches/__init__.py create mode 100644 frappe/integrations/doctype/oauth_client/patches/set_default_allowed_role_in_oauth_client.py create mode 100644 frappe/integrations/doctype/oauth_client_role/__init__.py create mode 100644 frappe/integrations/doctype/oauth_client_role/oauth_client_role.json create mode 100644 frappe/integrations/doctype/oauth_client_role/oauth_client_role.py diff --git a/frappe/core/doctype/user/user.js b/frappe/core/doctype/user/user.js index b37ebda9ee..cc34d015d6 100644 --- a/frappe/core/doctype/user/user.js +++ b/frappe/core/doctype/user/user.js @@ -280,7 +280,7 @@ frappe.ui.form.on("User", { frm.set_df_property("enabled", "read_only", 0); } - if (frappe.session.user !== "Administrator") { + if (frm.doc.name !== "Administrator") { frm.toggle_enable("email", frm.is_new()); } }, diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py index d856fd0c0f..f4ca1b0de4 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -157,7 +157,7 @@ class User(Document): self.password_strength_test() if self.name not in STANDARD_USERS: - self.validate_email_type(self.email) + self.email = self.name self.validate_email_type(self.name) self.add_system_manager_role() self.move_role_profile_name_to_role_profiles() diff --git a/frappe/integrations/doctype/oauth_client/oauth_client.json b/frappe/integrations/doctype/oauth_client/oauth_client.json index 5c93809111..e60cc1f5f1 100644 --- a/frappe/integrations/doctype/oauth_client/oauth_client.json +++ b/frappe/integrations/doctype/oauth_client/oauth_client.json @@ -9,6 +9,7 @@ "client_id", "app_name", "user", + "allowed_roles", "cb_1", "client_secret", "skip_authorization", @@ -114,10 +115,16 @@ "in_standard_filter": 1, "label": "Response Type", "options": "Code\nToken" + }, + { + "fieldname": "allowed_roles", + "fieldtype": "Table MultiSelect", + "label": "Allowed Roles", + "options": "OAuth Client Role" } ], "links": [], - "modified": "2024-03-23 16:03:32.679227", + "modified": "2024-04-29 12:07:07.946980", "modified_by": "Administrator", "module": "Integrations", "name": "OAuth Client", @@ -141,4 +148,4 @@ "states": [], "title_field": "app_name", "track_changes": 1 -} \ No newline at end of file +} diff --git a/frappe/integrations/doctype/oauth_client/oauth_client.py b/frappe/integrations/doctype/oauth_client/oauth_client.py index 604dd02d89..4b54320bdc 100644 --- a/frappe/integrations/doctype/oauth_client/oauth_client.py +++ b/frappe/integrations/doctype/oauth_client/oauth_client.py @@ -4,6 +4,7 @@ import frappe from frappe import _ from frappe.model.document import Document +from frappe.permissions import SYSTEM_USER_ROLE class OAuthClient(Document): @@ -13,8 +14,10 @@ class OAuthClient(Document): from typing import TYPE_CHECKING if TYPE_CHECKING: + from frappe.integrations.doctype.oauth_client_role.oauth_client_role import OAuthClientRole from frappe.types import DF + allowed_roles: DF.TableMultiSelect[OAuthClientRole] app_name: DF.Data client_id: DF.Data | None client_secret: DF.Data | None @@ -32,6 +35,7 @@ class OAuthClient(Document): if not self.client_secret: self.client_secret = frappe.generate_hash(length=10) self.validate_grant_and_response() + self.add_default_role() def validate_grant_and_response(self): if ( @@ -45,3 +49,12 @@ class OAuthClient(Document): "Combination of Grant Type ({0}) and Response Type ({1}) not allowed" ).format(self.grant_type, self.response_type) ) + + def add_default_role(self): + if not self.allowed_roles: + self.append("allowed_roles", {"role": SYSTEM_USER_ROLE}) + + def user_has_allowed_role(self) -> bool: + """Returns true if session user is allowed to use this client.""" + allowed_roles = {d.role for d in self.allowed_roles} + return bool(allowed_roles & set(frappe.get_roles())) diff --git a/frappe/integrations/doctype/oauth_client/patches/__init__.py b/frappe/integrations/doctype/oauth_client/patches/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/frappe/integrations/doctype/oauth_client/patches/set_default_allowed_role_in_oauth_client.py b/frappe/integrations/doctype/oauth_client/patches/set_default_allowed_role_in_oauth_client.py new file mode 100644 index 0000000000..0e877f4129 --- /dev/null +++ b/frappe/integrations/doctype/oauth_client/patches/set_default_allowed_role_in_oauth_client.py @@ -0,0 +1,11 @@ +import frappe + + +def execute(): + """Set default allowed role in OAuth Client""" + for client in frappe.get_all("OAuth Client", pluck="name"): + doc = frappe.get_doc("OAuth Client", client) + if doc.allowed_roles: + continue + row = doc.append("allowed_roles", {"role": "All"}) # Current default + row.db_insert() diff --git a/frappe/integrations/doctype/oauth_client_role/__init__.py b/frappe/integrations/doctype/oauth_client_role/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/frappe/integrations/doctype/oauth_client_role/oauth_client_role.json b/frappe/integrations/doctype/oauth_client_role/oauth_client_role.json new file mode 100644 index 0000000000..34352012d5 --- /dev/null +++ b/frappe/integrations/doctype/oauth_client_role/oauth_client_role.json @@ -0,0 +1,31 @@ +{ + "actions": [], + "creation": "2024-04-29 12:08:19.459404", + "doctype": "DocType", + "editable_grid": 1, + "engine": "InnoDB", + "field_order": [ + "role" + ], + "fields": [ + { + "fieldname": "role", + "fieldtype": "Link", + "in_list_view": 1, + "label": "Role", + "options": "Role" + } + ], + "index_web_pages_for_search": 1, + "istable": 1, + "links": [], + "modified": "2024-04-29 12:16:48.018031", + "modified_by": "Administrator", + "module": "Integrations", + "name": "OAuth Client Role", + "owner": "Administrator", + "permissions": [], + "sort_field": "creation", + "sort_order": "DESC", + "states": [] +} \ No newline at end of file diff --git a/frappe/integrations/doctype/oauth_client_role/oauth_client_role.py b/frappe/integrations/doctype/oauth_client_role/oauth_client_role.py new file mode 100644 index 0000000000..99cd8fef49 --- /dev/null +++ b/frappe/integrations/doctype/oauth_client_role/oauth_client_role.py @@ -0,0 +1,23 @@ +# Copyright (c) 2024, Frappe Technologies and contributors +# For license information, please see license.txt + +# import frappe +from frappe.model.document import Document + + +class OAuthClientRole(Document): + # begin: auto-generated types + # This code is auto-generated. Do not modify anything in this block. + + from typing import TYPE_CHECKING + + if TYPE_CHECKING: + from frappe.types import DF + + parent: DF.Data + parentfield: DF.Data + parenttype: DF.Data + role: DF.Link | None + # end: auto-generated types + + pass diff --git a/frappe/oauth.py b/frappe/oauth.py index 36ec026ca4..25f058017d 100644 --- a/frappe/oauth.py +++ b/frappe/oauth.py @@ -20,10 +20,11 @@ class OAuthWebRequestValidator(RequestValidator): # Simple validity check, does client exist? Not banned? cli_id = frappe.db.get_value("OAuth Client", {"name": client_id}) if cli_id: - request.client = frappe.get_doc("OAuth Client", client_id).as_dict() - return True - else: - return False + client = frappe.get_doc("OAuth Client", client_id) + if client.user_has_allowed_role(): + request.client = client.as_dict() + return True + return False def validate_redirect_uri(self, client_id, redirect_uri, request, *args, **kwargs): # Is the client allowed to use the supplied redirect_uri? i.e. has diff --git a/frappe/patches.txt b/frappe/patches.txt index 5b99aa226c..a937f535fc 100644 --- a/frappe/patches.txt +++ b/frappe/patches.txt @@ -236,3 +236,4 @@ frappe.patches.v15_0.migrate_role_profile_to_table_multi_select frappe.patches.v15_0.migrate_session_data frappe.custom.doctype.property_setter.patches.remove_invalid_fetch_from_expressions frappe.patches.v16_0.switch_default_sort_order +frappe.integrations.doctype.oauth_client.patches.set_default_allowed_role_in_oauth_client