feat: Desk User role (backport #22224)

frappe/core/doctype/doctype/doctype.py

@@ -30,6 +30,7 @@
 from frappe.model.meta import Meta
 from frappe.modules import get_doc_path, make_boilerplate
 from frappe.modules.import_file import get_file_path
+from frappe.permissions import ALL_USER_ROLE, AUTOMATIC_ROLES, SYSTEM_USER_ROLE
 from frappe.query_builder.functions import Concat
 from frappe.utils import cint, is_a_property, random_string
 from frappe.website.utils import clear_cache
@@ -1560,7 +1561,7 @@ def check_double(d):
 			)
 
 	def check_level_zero_is_set(d):
-		if cint(d.permlevel) > 0 and d.role != "All":
+		if cint(d.permlevel) > 0 and d.role not in (ALL_USER_ROLE, SYSTEM_USER_ROLE):
 			has_zero_perm = False
 			for p in permissions:
 				if p.role == d.role and (p.permlevel or 0) == 0 and p != d:
@@ -1619,11 +1620,11 @@ def validate_permission_for_all_role(d):
 			return
 
 		if doctype.custom:
-			if d.role == "All":
+			if d.role in AUTOMATIC_ROLES:
 				frappe.throw(
 					_(
 						"Row # {0}: Non administrator user can not set the role {1} to the custom doctype"
-					).format(d.idx, frappe.bold(_("All"))),
+					).format(d.idx, frappe.bold(_(d.role))),
 					title=_("Permissions Error"),
 				)
 
@@ -1673,8 +1674,7 @@ def make_module_and_roles(doc, perm_fieldname="permissions"):
 				m.custom = 1
 			m.insert()
 
-		default_roles = ["Administrator", "Guest", "All"]
-		roles = [p.role for p in doc.get("permissions") or []] + default_roles
+		roles = [p.role for p in doc.get("permissions") or []] + list(AUTOMATIC_ROLES)
 
 		for role in list(set(roles)):
 			if frappe.db.table_exists("Role", cached=False) and not frappe.db.exists("Role", role):

frappe/core/doctype/role/role.js

@@ -5,7 +5,12 @@ frappe.ui.form.on("Role", {
 	refresh: function (frm) {
 		if (frm.doc.name === "All") {
 			frm.dashboard.add_comment(
-				__("Role 'All' will be given to all System Users."),
+				__("Role 'All' will be given to all system + website users."),
+				"yellow"
+			);
+		} else if (frm.doc.name === "Desk User") {
+			frm.dashboard.add_comment(
+				__("Role 'Desk User' will be given to all system users."),
 				"yellow"
 			);
 		}

frappe/core/doctype/role_permission_for_page_and_report/role_permission_for_page_and_report.py

@@ -4,6 +4,8 @@
 import frappe
 from frappe.core.doctype.report.report import is_prepared_report_disabled
 from frappe.model.document import Document
+from frappe.permissions import ALL_USER_ROLE
+from frappe.utils import cint
 
 
 class RolePermissionforPageandReport(Document):
@@ -79,11 +81,11 @@ def get_args(self, row=None):
 		return {check_for_field: name}
 
 	def get_roles(self):
-		roles = []
-		for data in self.roles:
-			if data.role != "All":
-				roles.append({"role": data.role, "parenttype": "Custom Role"})
-		return roles
+		return [
+			{"role": data.role, "parenttype": "Custom Role"}
+			for data in self.roles
+			if data.role != ALL_USER_ROLE
+		]
 
 	def update_status(self):
 		return frappe.render_template

frappe/core/doctype/user/user.py

@@ -699,7 +699,10 @@ def get_all_roles(arg=None):
 
 	roles = frappe.get_all(
 		"Role",
-		filters={"name": ("not in", "Administrator,Guest,All"), "disabled": 0},
+		filters={
+			"name": ("not in", frappe.permissions.AUTOMATIC_ROLES),
+			"disabled": 0,
+		},
 		or_filters={"ifnull(restrict_to_domain, '')": "", "restrict_to_domain": ("in", active_domains)},
 		order_by="name",
 	)

frappe/core/page/permission_manager/permission_manager.py

@@ -12,6 +12,7 @@
 from frappe.exceptions import DoesNotExistError
 from frappe.modules.import_file import get_file_path, read_doc_from_file
 from frappe.permissions import (
+	AUTOMATIC_ROLES,
 	add_permission,
 	get_all_perms,
 	get_linked_doctypes,
@@ -43,10 +44,8 @@ def get_roles_and_doctypes():
 	restricted_roles = ["Administrator"]
 	if frappe.session.user != "Administrator":
 		custom_user_type_roles = frappe.get_all("User Type", filters={"is_standard": 0}, fields=["role"])
-		for row in custom_user_type_roles:
-			restricted_roles.append(row.role)
-
-		restricted_roles.append("All")
+		restricted_roles.extend(row.role for row in custom_user_type_roles)
+		restricted_roles.extend(AUTOMATIC_ROLES)
 
 	roles = frappe.get_all(
 		"Role",

frappe/desk/doctype/todo/todo.py

@@ -5,6 +5,7 @@
 
 import frappe
 from frappe.model.document import Document
+from frappe.permissions import AUTOMATIC_ROLES
 from frappe.utils import get_fullname, parse_addr
 
 exclude_from_linked_with = True
@@ -117,8 +118,7 @@ def get_permission_query_conditions(user):
 		user = frappe.session.user
 
 	todo_roles = frappe.permissions.get_doctype_roles("ToDo")
-	if "All" in todo_roles:
-		todo_roles.remove("All")
+	todo_roles = set(todo_roles) - set(AUTOMATIC_ROLES)
 
 	if any(check in todo_roles for check in frappe.get_roles(user)):
 		return None
@@ -131,8 +131,7 @@ def get_permission_query_conditions(user):
 def has_permission(doc, ptype="read", user=None):
 	user = user or frappe.session.user
 	todo_roles = frappe.permissions.get_doctype_roles("ToDo", ptype)
-	if "All" in todo_roles:
-		todo_roles.remove("All")
+	todo_roles = set(todo_roles) - set(AUTOMATIC_ROLES)
 
 	if any(check in todo_roles for check in frappe.get_roles(user)):
 		return True

frappe/desk/page/setup_wizard/setup_wizard.py

@@ -5,6 +5,7 @@
 
 import frappe
 from frappe.geo.country_info import get_country_info
+from frappe.permissions import AUTOMATIC_ROLES
 from frappe.translate import get_messages_for_boot, send_translations, set_default_language
 from frappe.utils import cint, strip
 from frappe.utils.password import update_password
@@ -254,13 +255,11 @@ def add_all_roles_to(name):
 	user = frappe.get_doc("User", name)
 	for role in frappe.db.sql("""select name from tabRole"""):
 		if role[0] not in [
-			"Administrator",
-			"Guest",
-			"All",
 			"Customer",
 			"Supplier",
 			"Partner",
 			"Employee",
+			*AUTOMATIC_ROLES,
 		]:
 			d = user.append("roles")
 			d.role = role[0]

frappe/integrations/doctype/ldap_settings/test_ldap_settings.py

@@ -371,13 +371,15 @@ def test_sync_roles(self):
 					"default_role",
 					"frappe_default_all",
 					"frappe_default_guest",
+					"frappe_default_desk_user",
 				],
 				"posix.user2": [
 					"Users",
 					"Group3",
 					"default_role",
 					"frappe_default_all",
 					"frappe_default_guest",
+					"frappe_default_desk_user",
 				],
 			}
 
@@ -389,13 +391,15 @@ def test_sync_roles(self):
 					"default_role",
 					"frappe_default_all",
 					"frappe_default_guest",
+					"frappe_default_desk_user",
 				],
 				"posix.user2": [
 					"Domain Users",
 					"Enterprise Administrators",
 					"default_role",
 					"frappe_default_all",
 					"frappe_default_guest",
+					"frappe_default_desk_user",
 				],
 			}
 
@@ -406,6 +410,7 @@ def test_sync_roles(self):
 			"Newsletter Manager": "default_role",
 			"All": "frappe_default_all",
 			"Guest": "frappe_default_guest",
+			"Desk User": "frappe_default_desk_user",
 		}
 
 		# re-create user1 to ensure clean

frappe/permissions.py

@@ -27,6 +27,15 @@
 	"share",
 )
 
+GUEST_ROLE = "Guest"
+ALL_USER_ROLE = "All"  # This includes website users too.
+SYSTEM_USER_ROLE = "Desk User"
+ADMIN_ROLE = "Administrator"
+
+
+# These roles are automatically assigned based on user type
+AUTOMATIC_ROLES = (GUEST_ROLE, ALL_USER_ROLE, SYSTEM_USER_ROLE, ADMIN_ROLE)
+
 
 def check_admin_or_system_manager(user=None):
 	from frappe.utils.commands import warn
@@ -426,7 +435,7 @@ def get_roles(user=None, with_standard=True):
 		user = frappe.session.user
 
 	if user == "Guest" or not user:
-		return ["Guest"]
+		return [GUEST_ROLE]
 
 	def get():
 		if user == "Administrator":
@@ -438,18 +447,21 @@ def get():
 				.where(
 					(table.parenttype == "User")
 					& (table.parent == user)
-					& (table.role.notin(["All", "Guest"]))
+					& (table.role.notin(AUTOMATIC_ROLES))
 				)
 				.select(table.role)
 				.run(pluck=True)
 			)
-			return [*roles, "All", "Guest"]
+			roles += [ALL_USER_ROLE, GUEST_ROLE]
+			if is_system_user(user):
+				roles.append(SYSTEM_USER_ROLE)
+			return roles
 
 	roles = frappe.cache().hget("roles", user, get)
 
 	# filter standard if required
 	if not with_standard:
-		roles = [r for r in roles if r not in ["All", "Guest", "Administrator"]]
+		roles = [r for r in roles if r not in AUTOMATIC_ROLES]
 
 	return roles
 
@@ -777,3 +789,7 @@ def has_child_permission(
 		user=user,
 		raise_exception=raise_exception,
 	)
+
+
+def is_system_user(user: str | None = None) -> bool:
+	return frappe.get_cached_value("User", user or frappe.session.user, "user_type") == "System User"

frappe/tests/test_permissions.py

@@ -10,6 +10,10 @@
 from frappe.core.page.permission_manager.permission_manager import add, remove, reset, update
 from frappe.desk.form.load import getdoc
 from frappe.permissions import (
+	ALL_USER_ROLE,
+	AUTOMATIC_ROLES,
+	GUEST_ROLE,
+	SYSTEM_USER_ROLE,
 	add_permission,
 	add_user_permission,
 	clear_user_permissions_for_doctype,
@@ -712,6 +716,31 @@ def test_select_user(self):
 		self.assertIn("test2@example.com", users)
 		self.assertIn("test3@example.com", users)
 
+	def test_automatic_permissions(self):
+		def assertHasRole(*roles: str | tuple[str, ...]):
+			for role in roles:
+				self.assertIn(role, frappe.get_roles())
+
+		frappe.set_user("Administrator")
+		assertHasRole(*AUTOMATIC_ROLES)
+
+		frappe.set_user("Guest")
+		assertHasRole(GUEST_ROLE)
+
+		website_user = frappe.db.get_value(
+			"User",
+			{"user_type": "Website User", "enabled": 1, "name": ("not in", AUTOMATIC_ROLES)},
+		)
+		frappe.set_user(website_user)
+		assertHasRole(GUEST_ROLE, ALL_USER_ROLE)
+
+		system_user = frappe.db.get_value(
+			"User",
+			{"user_type": "System User", "enabled": 1, "name": ("not in", AUTOMATIC_ROLES)},
+		)
+		frappe.set_user(system_user)
+		assertHasRole(GUEST_ROLE, ALL_USER_ROLE, SYSTEM_USER_ROLE)
+
 	def test_get_doctypes_with_read(self):
 		with self.set_user("Administrator"):
 			doctype = new_doctype(permissions=[{"select": 1, "role": "_Test Role", "read": 0}]).insert().name

frappe/tests/ui_test_helpers.py

@@ -1,5 +1,6 @@
 import frappe
 from frappe import _
+from frappe.permissions import AUTOMATIC_ROLES
 from frappe.utils import add_to_date, now
 
 UI_TEST_USER = "frappe@example.com"
@@ -441,10 +442,9 @@ def create_test_user(username=None):
 
 	user.reload()
 
-	blocked_roles = {"Administrator", "Guest", "All"}
 	all_roles = set(frappe.get_all("Role", pluck="name"))
 
-	for role in all_roles - blocked_roles:
+	for role in all_roles - set(AUTOMATIC_ROLES):
 		user.append("roles", {"role": role})
 
 	user.save()

frappe/twofactor.py

@@ -9,6 +9,7 @@
 import frappe
 import frappe.defaults
 from frappe import _
+from frappe.permissions import ALL_USER_ROLE
 from frappe.utils import cint, get_datetime, get_url, time_diff_in_seconds
 from frappe.utils.background_jobs import enqueue
 from frappe.utils.password import decrypt, encrypt
@@ -116,8 +117,7 @@ def two_factor_is_enabled_for_(user):
 
 	if isinstance(user, str):
 		user = frappe.get_doc("User", user)
-
-	roles = [d.role for d in user.roles or []] + ["All"]
+	roles = [d.role for d in user.roles or []] + [ALL_USER_ROLE]
 
 	role_doctype = frappe.qb.DocType("Role")
 	no_of_users = frappe.db.count(

frappe/utils/user.py

@@ -9,7 +9,7 @@
 from frappe import _dict
 from frappe.boot import get_allowed_reports
 from frappe.core.doctype.domain_settings.domain_settings import get_active_modules
-from frappe.permissions import get_roles, get_valid_perms
+from frappe.permissions import AUTOMATIC_ROLES, get_roles, get_valid_perms
 from frappe.query_builder import DocType, Order
 from frappe.query_builder.functions import Concat_ws
 
@@ -354,7 +354,7 @@ def add_system_manager(
 	roles = frappe.get_all(
 		"Role",
 		fields=["name"],
-		filters={"name": ["not in", ("Administrator", "Guest", "All")]},
+		filters={"name": ["not in", AUTOMATIC_ROLES]},
 	)
 	roles = [role.name for role in roles]
 	user.add_roles(*roles)
@@ -383,13 +383,16 @@ def is_website_user(username: str | None = None) -> str | None:
 
 
 def is_system_user(username: str | None = None) -> str | None:
+	# TODO: Depracate this. Inefficient, incorrect. This function is meant to be used in emails only.
+	# Problem: Filters on email instead of PK, implicitly filters out disabled users.
 	return frappe.db.get_value(
 		"User",
 		{
 			"email": username or frappe.session.user,
 			"enabled": 1,
 			"user_type": "System User",
 		},
+		cache=True,
 	)