From 51594fc4965a7a1c413532c9a637fffbb0d8d162 Mon Sep 17 00:00:00 2001
From: Andrew Walker <awalker@ixsystems.com>
Date: Fri, 10 Jan 2025 12:34:29 -0600
Subject: [PATCH] require role_prefix for public CRUD / Config services

This commit adds a role_prefix for all public CRUDService and
ConfigService plugins, and raises an exception in a plugin
of this type is created without a defined role_prefix.
---
 .../middlewared/plugins/acme_protocol.py           |  1 +
 src/middlewared/middlewared/plugins/auth_/2fa.py   |  1 +
 src/middlewared/middlewared/plugins/cron.py        |  1 +
 src/middlewared/middlewared/plugins/disk.py        |  1 +
 .../middlewared/plugins/init_shutdown_script.py    |  1 +
 src/middlewared/middlewared/plugins/ipmi_/lan.py   |  1 +
 src/middlewared/middlewared/plugins/mail.py        |  1 +
 .../middlewared/plugins/network_/global_config.py  |  1 +
 .../middlewared/plugins/network_/static_routes.py  |  1 +
 src/middlewared/middlewared/plugins/ntp.py         |  2 ++
 .../middlewared/plugins/pool_/user_props.py        |  1 +
 src/middlewared/middlewared/plugins/rsync.py       |  1 +
 src/middlewared/middlewared/plugins/smart.py       |  2 ++
 src/middlewared/middlewared/plugins/snmp.py        |  1 +
 src/middlewared/middlewared/plugins/ssh.py         |  1 +
 src/middlewared/middlewared/plugins/sysdataset.py  |  1 +
 src/middlewared/middlewared/plugins/tunables.py    |  1 +
 src/middlewared/middlewared/plugins/ups.py         |  1 +
 src/middlewared/middlewared/plugins/vmware.py      |  1 +
 src/middlewared/middlewared/role.py                | 14 ++++++++++++++
 .../middlewared/service/config_service.py          |  3 +++
 .../middlewared/service/crud_service.py            |  3 +++
 22 files changed, 41 insertions(+)

diff --git a/src/middlewared/middlewared/plugins/acme_protocol.py b/src/middlewared/middlewared/plugins/acme_protocol.py
index 39c9f6860d354..6701f50d59756 100644
--- a/src/middlewared/middlewared/plugins/acme_protocol.py
+++ b/src/middlewared/middlewared/plugins/acme_protocol.py
@@ -206,6 +206,7 @@ class Config:
         datastore = 'system.acmednsauthenticator'
         cli_namespace = 'system.acme.dns_auth'
         entry = ACMEDNSAuthenticatorEntry
+        role_prefix = 'NETWORK_INTERFACE'
 
     @private
     async def common_validation(self, data, schema_name, old=None):
diff --git a/src/middlewared/middlewared/plugins/auth_/2fa.py b/src/middlewared/middlewared/plugins/auth_/2fa.py
index ded6eb11155ec..c6bdab44ce198 100644
--- a/src/middlewared/middlewared/plugins/auth_/2fa.py
+++ b/src/middlewared/middlewared/plugins/auth_/2fa.py
@@ -37,6 +37,7 @@ class Config:
         datastore_extend = 'auth.twofactor.two_factor_extend'
         namespace = 'auth.twofactor'
         cli_namespace = 'auth.two_factor'
+        role_prefix = 'SYSTEM_SECURITY'
 
     ENTRY = Dict(
         'auth_twofactor_entry',
diff --git a/src/middlewared/middlewared/plugins/cron.py b/src/middlewared/middlewared/plugins/cron.py
index 08ef284eb330d..68a0d7b7df27b 100644
--- a/src/middlewared/middlewared/plugins/cron.py
+++ b/src/middlewared/middlewared/plugins/cron.py
@@ -38,6 +38,7 @@ class Config:
         namespace = 'cronjob'
         cli_namespace = 'task.cron_job'
         entry = CronJobEntry
+        role_prefix = 'SYSTEM_CRON'
 
     @private
     def cron_extend(self, data):
diff --git a/src/middlewared/middlewared/plugins/disk.py b/src/middlewared/middlewared/plugins/disk.py
index 4a87cb8bdacd0..1f720a8020788 100644
--- a/src/middlewared/middlewared/plugins/disk.py
+++ b/src/middlewared/middlewared/plugins/disk.py
@@ -48,6 +48,7 @@ class Config:
         event_register = False
         event_send = False
         cli_namespace = 'storage.disk'
+        role_prefix = 'DISK'
 
     ENTRY = Dict(
         'disk_entry',
diff --git a/src/middlewared/middlewared/plugins/init_shutdown_script.py b/src/middlewared/middlewared/plugins/init_shutdown_script.py
index 40d55d989ce46..112b43a75ec76 100644
--- a/src/middlewared/middlewared/plugins/init_shutdown_script.py
+++ b/src/middlewared/middlewared/plugins/init_shutdown_script.py
@@ -37,6 +37,7 @@ class Config:
         datastore_extend = 'initshutdownscript.init_shutdown_script_extend'
         cli_namespace = 'system.init_shutdown_script'
         entry = InitShutdownScriptEntry
+        role_prefix = 'SYSTEM_CRON'
 
     @api_method(InitShutdownScriptCreateArgs, InitShutdownScriptCreateResult, roles=['FULL_ADMIN'])
     async def do_create(self, data):
diff --git a/src/middlewared/middlewared/plugins/ipmi_/lan.py b/src/middlewared/middlewared/plugins/ipmi_/lan.py
index 2078dce77bde5..987f55186bbea 100644
--- a/src/middlewared/middlewared/plugins/ipmi_/lan.py
+++ b/src/middlewared/middlewared/plugins/ipmi_/lan.py
@@ -87,6 +87,7 @@ class IPMILanService(CRUDService):
     class Config:
         namespace = 'ipmi.lan'
         cli_namespace = 'network.ipmi'
+        role_prefix = 'IPMI'
 
     @accepts(roles=['IPMI_READ'])
     @returns(List('lan_channels', items=[Int('lan_channel')]))
diff --git a/src/middlewared/middlewared/plugins/mail.py b/src/middlewared/middlewared/plugins/mail.py
index 440de1daacb22..31048fd4ef17c 100644
--- a/src/middlewared/middlewared/plugins/mail.py
+++ b/src/middlewared/middlewared/plugins/mail.py
@@ -78,6 +78,7 @@ class Config:
         datastore_prefix = 'em_'
         datastore_extend = 'mail.mail_extend'
         cli_namespace = 'system.mail'
+        role_prefix = 'ALERT'
 
     ENTRY = Dict(
         'mail_entry',
diff --git a/src/middlewared/middlewared/plugins/network_/global_config.py b/src/middlewared/middlewared/plugins/network_/global_config.py
index d432d0c4379ef..8cda8a6a3ebbe 100644
--- a/src/middlewared/middlewared/plugins/network_/global_config.py
+++ b/src/middlewared/middlewared/plugins/network_/global_config.py
@@ -36,6 +36,7 @@ class Config:
         datastore_prefix = 'gc_'
         datastore_extend = 'network.configuration.network_config_extend'
         cli_namespace = 'network.configuration'
+        role_prefix = 'NETWORK_GENERAL'
 
     ENTRY = Dict(
         'network_configuration_entry',
diff --git a/src/middlewared/middlewared/plugins/network_/static_routes.py b/src/middlewared/middlewared/plugins/network_/static_routes.py
index fcc28ec070727..bf6532e96caab 100644
--- a/src/middlewared/middlewared/plugins/network_/static_routes.py
+++ b/src/middlewared/middlewared/plugins/network_/static_routes.py
@@ -31,6 +31,7 @@ class Config:
         datastore_prefix = 'sr_'
         cli_namespace = 'network.static_route'
         entry = StaticRouteEntry
+        role_prefix = 'NETWORK_INTERFACE'
 
     @api_method(StaticRouteCreateArgs, StaticRouteCreateResult)
     async def do_create(self, data):
diff --git a/src/middlewared/middlewared/plugins/ntp.py b/src/middlewared/middlewared/plugins/ntp.py
index ef94c40562e82..2f1e266ed1f21 100644
--- a/src/middlewared/middlewared/plugins/ntp.py
+++ b/src/middlewared/middlewared/plugins/ntp.py
@@ -100,6 +100,8 @@ class Config:
         datastore_prefix = 'ntp_'
         cli_namespace = 'system.ntp_server'
         entry = NTPServerEntry
+        prefix = 'SYSTEM_ADVANCED'
+        role_prefix = 'NETWORK_INTERFACE'
 
     @api_method(NTPServerCreateArgs, NTPServerCreateResult)
     async def do_create(self, data):
diff --git a/src/middlewared/middlewared/plugins/pool_/user_props.py b/src/middlewared/middlewared/plugins/pool_/user_props.py
index 2bf4d6f0af352..e4708f4edcd95 100644
--- a/src/middlewared/middlewared/plugins/pool_/user_props.py
+++ b/src/middlewared/middlewared/plugins/pool_/user_props.py
@@ -10,6 +10,7 @@ class Config:
         datastore_primary_key_type = 'string'
         namespace = 'pool.dataset.userprop'
         cli_namespace = 'storage.dataset.user_prop'
+        role_prefix = 'DATASET'
 
     ENTRY = Dict(
         'pool_dataset_userprop_entry',
diff --git a/src/middlewared/middlewared/plugins/rsync.py b/src/middlewared/middlewared/plugins/rsync.py
index 44bca0fcd01a2..3da29c275767e 100644
--- a/src/middlewared/middlewared/plugins/rsync.py
+++ b/src/middlewared/middlewared/plugins/rsync.py
@@ -128,6 +128,7 @@ class Config:
         datastore_extend = 'rsynctask.rsync_task_extend'
         datastore_extend_context = 'rsynctask.rsync_task_extend_context'
         cli_namespace = 'task.rsync'
+        role_prefix = 'SNAPSHOT_TASK'
 
     ENTRY = Patch(
         'rsync_task_create', 'rsync_task_entry',
diff --git a/src/middlewared/middlewared/plugins/smart.py b/src/middlewared/middlewared/plugins/smart.py
index e92791461afd9..2509f4c90d746 100644
--- a/src/middlewared/middlewared/plugins/smart.py
+++ b/src/middlewared/middlewared/plugins/smart.py
@@ -266,6 +266,7 @@ class Config:
         datastore_prefix = 'smarttest_'
         namespace = 'smart.test'
         cli_namespace = 'task.smart_test'
+        role_prefix = 'DISK'
 
     ENTRY = Patch(
         'smart_task_create', 'smart_task_entry',
@@ -773,6 +774,7 @@ class Config:
         datastore_extend = "smart.smart_extend"
         datastore_prefix = "smart_"
         cli_namespace = "service.smart"
+        role_prefix = 'DISK'
 
     ENTRY = Dict(
         'smart_entry',
diff --git a/src/middlewared/middlewared/plugins/snmp.py b/src/middlewared/middlewared/plugins/snmp.py
index a513f0b0f30a0..3fc18b0ee6926 100644
--- a/src/middlewared/middlewared/plugins/snmp.py
+++ b/src/middlewared/middlewared/plugins/snmp.py
@@ -45,6 +45,7 @@ class Config:
         datastore_prefix = 'snmp_'
         cli_namespace = 'service.snmp'
         entry = SnmpEntry
+        role_prefix = 'SYSTEM_GENERAL'
 
     @private
     def get_snmp_users(self):
diff --git a/src/middlewared/middlewared/plugins/ssh.py b/src/middlewared/middlewared/plugins/ssh.py
index 3bba21c00a56a..08280ff70f177 100644
--- a/src/middlewared/middlewared/plugins/ssh.py
+++ b/src/middlewared/middlewared/plugins/ssh.py
@@ -50,6 +50,7 @@ class Config:
         service = "ssh"
         datastore_prefix = "ssh_"
         cli_namespace = 'service.ssh'
+        role_prefix = 'SSH'
 
     ENTRY = Dict(
         'ssh_entry',
diff --git a/src/middlewared/middlewared/plugins/sysdataset.py b/src/middlewared/middlewared/plugins/sysdataset.py
index 25ddef151e7a1..6ccac62db0818 100644
--- a/src/middlewared/middlewared/plugins/sysdataset.py
+++ b/src/middlewared/middlewared/plugins/sysdataset.py
@@ -39,6 +39,7 @@ class Config:
         datastore_extend = 'systemdataset.config_extend'
         datastore_prefix = 'sys_'
         cli_namespace = 'system.system_dataset'
+        role_prefix = 'DATASET'
 
     ENTRY = Dict(
         'systemdataset_entry',
diff --git a/src/middlewared/middlewared/plugins/tunables.py b/src/middlewared/middlewared/plugins/tunables.py
index f0f60b50efc53..5f26240a7428a 100644
--- a/src/middlewared/middlewared/plugins/tunables.py
+++ b/src/middlewared/middlewared/plugins/tunables.py
@@ -38,6 +38,7 @@ class Config:
         datastore = 'system.tunable'
         datastore_prefix = 'tun_'
         cli_namespace = 'system.tunable'
+        role_prefix = 'SYSTEM_TUNABLE'
 
     SYSCTLS = set()
 
diff --git a/src/middlewared/middlewared/plugins/ups.py b/src/middlewared/middlewared/plugins/ups.py
index 5f20cb0224d35..411df642bb02e 100644
--- a/src/middlewared/middlewared/plugins/ups.py
+++ b/src/middlewared/middlewared/plugins/ups.py
@@ -83,6 +83,7 @@ class Config:
         service = 'ups'
         service_verb = 'restart'
         cli_namespace = 'service.ups'
+        role_prefix = 'SYSTEM_GENERAL'
 
     @private
     async def ups_config_extend(self, data):
diff --git a/src/middlewared/middlewared/plugins/vmware.py b/src/middlewared/middlewared/plugins/vmware.py
index 9744285122bb0..c0b499b1d43dd 100644
--- a/src/middlewared/middlewared/plugins/vmware.py
+++ b/src/middlewared/middlewared/plugins/vmware.py
@@ -34,6 +34,7 @@ class VMWareService(CRUDService):
     class Config:
         datastore = 'storage.vmwareplugin'
         cli_namespace = 'storage.vmware'
+        role_prefix = 'POOL'
 
     ENTRY = Patch(
         "vmware_create",
diff --git a/src/middlewared/middlewared/role.py b/src/middlewared/middlewared/role.py
index 557b79ff20a10..332ebb36db216 100644
--- a/src/middlewared/middlewared/role.py
+++ b/src/middlewared/middlewared/role.py
@@ -38,6 +38,9 @@ class Role:
     'DIRECTORY_SERVICE_READ': Role(),
     'DIRECTORY_SERVICE_WRITE': Role(includes=['DIRECTORY_SERVICE_READ']),
 
+    'DISK_READ': Role(),
+    'DISK_WRITE': Role(includes=['DISK_READ']),
+
     'KMIP_READ': Role(),
     'KMIP_WRITE': Role(includes=['KMIP_READ']),
 
@@ -61,6 +64,9 @@ class Role:
     'SUPPORT_READ': Role(),
     'SUPPORT_WRITE': Role(includes=['SUPPORT_READ']),
 
+    'SSH_READ': Role(),
+    'SSH_WRITE': Role(includes=['SSH_READ']),
+
     'SYSTEM_AUDIT_READ': Role(),
     'SYSTEM_AUDIT_WRITE': Role(),
 
@@ -89,6 +95,8 @@ class Role:
 
     # Network roles
     'NETWORK_GENERAL_READ': Role(),
+    'NETWORK_GENERAL_WRITE': Role(includes=['NETWORK_GENERAL_READ']),
+
     'NETWORK_INTERFACE_READ': Role(),
     'NETWORK_INTERFACE_WRITE': Role(includes=['NETWORK_INTERFACE_READ']),
 
@@ -220,6 +228,12 @@ class Role:
     'SYSTEM_SECURITY_READ': Role(),
     'SYSTEM_SECURITY_WRITE': Role(includes=['SYSTEM_SECURITY_READ']),
 
+    'SYSTEM_TUNABLE_READ': Role(),
+    'SYSTEM_TUNABLE_WRITE': Role(includes=['SYSTEM_TUNABLE_READ']),
+
+    'SYSTEM_CRON_READ': Role(),
+    'SYSTEM_CRON_WRITE': Role(includes=['SYSTEM_CRON_READ']),
+
     # Virtualization
     'VIRT_GLOBAL_READ': Role(),
     'VIRT_GLOBAL_WRITE': Role(includes=['VIRT_GLOBAL_READ'], stig=None),
diff --git a/src/middlewared/middlewared/service/config_service.py b/src/middlewared/middlewared/service/config_service.py
index 7c266adce093f..435c5c8e128f3 100644
--- a/src/middlewared/middlewared/service/config_service.py
+++ b/src/middlewared/middlewared/service/config_service.py
@@ -36,6 +36,9 @@ def __new__(cls, name, bases, attrs):
         config_entry_key = f'{namespace}_entry'
         config_model_name = f'{namespace.capitalize()}Config'
 
+        if not klass._config.private and not klass._config.role_prefix:
+            raise ValueError(f'{klass._config.namespace}: public ConfigService must have role_prefix defined')
+
         if klass._config.entry is not None and not hasattr(klass.config, 'new_style_accepts'):
             klass.ENTRY = None
             result_model = create_model(
diff --git a/src/middlewared/middlewared/service/crud_service.py b/src/middlewared/middlewared/service/crud_service.py
index 5087c50248469..8e1ad4847e6e6 100644
--- a/src/middlewared/middlewared/service/crud_service.py
+++ b/src/middlewared/middlewared/service/crud_service.py
@@ -62,6 +62,9 @@ def __new__(cls, name, bases, attrs):
         ):
             return klass
 
+        if not klass._config.private and not klass._config.role_prefix:
+            raise ValueError(f'{klass._config.namespace}: public CRUDService must have role_prefix defined')
+
         if klass._config.entry is not None:
             # FIXME: This is to prevent `Method cloudsync.credentials.ENTRY is public but has no @accepts()`, remove
             # eventually.