feat: limit oauth client to roles (backport #26193) (#26196)

* fix(UX): bulk delete error messages

(cherry picked from commit 926c888)

* feat: Limit OAuth Client by roles

(cherry picked from commit ae4eb87)

# Conflicts:
#	frappe/integrations/doctype/oauth_client/oauth_client.json
#	frappe/patches.txt

* chore: conflicts

---------

Co-authored-by: Ankush Menat <ankush@frappe.io>

frappe/core/doctype/user/user.js

@@ -286,7 +286,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());
 		}
 	},

frappe/core/doctype/user/user.py

@@ -152,7 +152,7 @@ def validate(self):
 			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.populate_role_profile_roles()

frappe/desk/reportview.py

@@ -513,6 +513,16 @@ def delete_bulk(doctype, items):
 	if undeleted_items and len(items) != len(undeleted_items):
 		frappe.clear_messages()
 		delete_bulk(doctype, undeleted_items)
+	elif undeleted_items:
+		frappe.msgprint(
+			_("Failed to delete {0} documents: {1}").format(len(undeleted_items), ", ".join(undeleted_items)),
+			realtime=True,
+			title=_("Bulk Operation Failed"),
+		)
+	else:
+		frappe.msgprint(
+			_("Deleted all documents successfully"), realtime=True, title=_("Bulk Operation Successful")
+		)
 
 
 @frappe.whitelist()

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": "2023-07-17 07:06:35.765981",
+ "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
-}
+}

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 @@ def validate(self):
 		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 @@ def validate_grant_and_response(self):
 					"Combination of Grant Type (<code>{0}</code>) and Response Type (<code>{1}</code>) 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()))

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

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": []
+}

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

frappe/oauth.py

@@ -20,10 +20,11 @@ def validate_client_id(self, client_id, request, *args, **kwargs):
 		# 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

frappe/patches.txt

@@ -234,3 +234,4 @@ frappe.core.doctype.data_import.patches.remove_stale_docfields_from_legacy_versi
 frappe.patches.v15_0.validate_newsletter_recipients
 frappe.patches.v15_0.sanitize_workspace_titles
 frappe.custom.doctype.property_setter.patches.remove_invalid_fetch_from_expressions
+frappe.integrations.doctype.oauth_client.patches.set_default_allowed_role_in_oauth_client