netcmd: auth policy: remove old service-allowed-to-authenticate-from-silo and group

Signed-off-by: Rob van der Linde <rob@catalyst.net.nz>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

diff --git a/python/samba/netcmd/domain/auth/policy/policy.py b/python/samba/netcmd/domain/auth/policy/policy.py
index 701d6e40aa21c0518804a813eafb012b14473cf0..fc06fd27705a2241d04b25e5ca3da1a2744b3bd5 100644 (file)
--- a/python/samba/netcmd/domain/auth/policy/policy.py
+++ b/python/samba/netcmd/domain/auth/policy/policy.py
@@ -29,17 +29,6 @@ from samba.netcmd import Command, CommandError, Option
 from samba.netcmd.validators import Range
 
 
-def check_similar_args(option, args):
-    """Helper method for checking similar mutually exclusive args.
-
-    Example: --user-allowed-to-authenticate-from and
-             --user-allowed-to-authenticate-from-device-silo
-    """
-    num = sum(arg is not None for arg in args)
-    if num > 1:
-        raise CommandError(f"{option} argument repeated {num} times.")
-
-
 class UserOptions(options.OptionGroup):
     """User options used by policy create and policy modify commands."""
 
@@ -91,16 +80,6 @@ class ServiceOptions(options.OptionGroup):
                         type=str, dest="allowed_to_authenticate_from",
                         action="callback", callback=self.set_option,
                         metavar="SDDL")
-        self.add_option("--service-allowed-to-authenticate-from-device-silo",
-                        help="To authenticate, the service must authenticate on a device in SILO.",
-                        type=str, dest="allowed_to_authenticate_from_device_silo",
-                        action="callback", callback=self.set_option,
-                        metavar="SILO")
-        self.add_option("--service-allowed-to-authenticate-from-device-group",
-                        help="To authenticate, the service must authenticate on a device in GROUP.",
-                        type=str, dest="allowed_to_authenticate_from_device_group",
-                        action="callback", callback=self.set_option,
-                        metavar="GROUP")
         self.add_option("--service-allowed-to-authenticate-to",
                         help="The target service requires the connecting user to match SDDL",
                         type=str, dest="allowed_to_authenticate_to",
@@ -243,26 +222,8 @@ class cmd_domain_auth_policy_create(Command):
         if audit and enforce:
             raise CommandError("--audit and --enforce cannot be used together.")
 
-        # Check for repeated, similar arguments.
-        check_similar_args("--service-allowed-to-authenticate-from",
-                           [serviceopts.allowed_to_authenticate_from,
-                            serviceopts.allowed_to_authenticate_from_device_group,
-                            serviceopts.allowed_to_authenticate_from_device_silo])
-
         ldb = self.ldb_connect(hostopts, sambaopts, credopts)
 
-        # Generate SDDL for authenticating service accounts from a device in a group
-        if serviceopts.allowed_to_authenticate_from_device_group:
-            group = Group.get(
-                ldb, cn=serviceopts.allowed_to_authenticate_from_device_group)
-            serviceopts.allowed_to_authenticate_from = group.get_authentication_sddl()
-
-        # Generate SDDL for authenticating service accounts from a device in a silo
-        if serviceopts.allowed_to_authenticate_from_device_silo:
-            silo = AuthenticationSilo.get(
-                ldb, cn=serviceopts.allowed_to_authenticate_from_device_silo)
-            serviceopts.allowed_to_authenticate_from = silo.get_authentication_sddl()
-
         try:
             policy = AuthenticationPolicy.get(ldb, cn=name)
         except ModelError as e:
@@ -357,26 +318,8 @@ class cmd_domain_auth_policy_modify(Command):
         if audit and enforce:
             raise CommandError("--audit and --enforce cannot be used together.")
 
-        # Check for repeated, similar arguments.
-        check_similar_args("--service-allowed-to-authenticate-from",
-                           [serviceopts.allowed_to_authenticate_from,
-                            serviceopts.allowed_to_authenticate_from_device_group,
-                            serviceopts.allowed_to_authenticate_from_device_silo])
-
         ldb = self.ldb_connect(hostopts, sambaopts, credopts)
 
-        # Generate SDDL for authenticating users from a device a device in a group
-        if serviceopts.allowed_to_authenticate_from_device_group:
-            group = Group.get(
-                ldb, cn=serviceopts.allowed_to_authenticate_from_device_group)
-            serviceopts.allowed_to_authenticate_from = group.get_authentication_sddl()
-
-        # Generate SDDL for authenticating service accounts from a device in a silo
-        if serviceopts.allowed_to_authenticate_from_device_silo:
-            silo = AuthenticationSilo.get(
-                ldb, cn=serviceopts.allowed_to_authenticate_from_device_silo)
-            serviceopts.allowed_to_authenticate_from = silo.get_authentication_sddl()
-
         try:
             policy = AuthenticationPolicy.get(ldb, cn=name)
         except ModelError as e:

diff --git a/python/samba/tests/samba_tool/domain_auth_policy.py b/python/samba/tests/samba_tool/domain_auth_policy.py
index 7c07ab84613f11f9dad8077a997db30500a735b6..864979608eac1be8f137be25c084b947b546bbf7 100644 (file)
--- a/python/samba/tests/samba_tool/domain_auth_policy.py
+++ b/python/samba/tests/samba_tool/domain_auth_policy.py
@@ -187,50 +187,6 @@ class AuthPolicyCmdTestCase(SiloTest):
         self.assertIn("--service-tgt-lifetime-mins must be between 45 and 2147483647",
                       err)
 
-    def test_create__service_allowed_to_authenticate_from_device_group(self):
-        """Tests the --service-allowed-to-authenticate-from-device-group shortcut."""
-        name = self.unique_name()
-        expected = "O:SYG:SYD:(XA;OICI;CR;;;WD;(Member_of_any {SID(%s)}))" % (
-            self.device_group.object_sid)
-
-        self.addCleanup(self.delete_authentication_policy, name=name, force=True)
-        result, out, err = self.runcmd("domain", "auth", "policy", "create",
-                                       "--name", name,
-                                       "--service-allowed-to-authenticate-from-device-group",
-                                       self.device_group.name)
-        self.assertIsNone(result, msg=err)
-
-        # Check policy fields.
-        policy = self.get_authentication_policy(name)
-        self.assertEqual(str(policy["cn"]), name)
-
-        # Check generated SDDL.
-        desc = policy["msDS-ServiceAllowedToAuthenticateFrom"][0]
-        sddl = ndr_unpack(security.descriptor, desc).as_sddl()
-        self.assertEqual(sddl, expected)
-
-    def test_create__service_allowed_to_authenticate_from_device_silo(self):
-        """Tests the --service-allowed-to-authenticate-from-device-silo shortcut."""
-        name = self.unique_name()
-
-        self.addCleanup(self.delete_authentication_policy, name=name, force=True)
-        result, out, err = self.runcmd("domain", "auth", "policy", "create",
-                                       "--name", name,
-                                       "--service-allowed-to-authenticate-from-device-silo",
-                                       "Managers")
-        self.assertIsNone(result, msg=err)
-
-        # Check policy fields.
-        policy = self.get_authentication_policy(name)
-        self.assertEqual(str(policy["cn"]), name)
-        desc = policy["msDS-ServiceAllowedToAuthenticateFrom"][0]
-
-        # Check generated SDDL.
-        sddl = ndr_unpack(security.descriptor, desc).as_sddl()
-        self.assertEqual(
-            sddl,
-            'O:SYG:SYD:(XA;OICI;CR;;;WD;(@USER.ad://ext/AuthenticationSilo == "Managers"))')
-
     def test_create__computer_tgt_lifetime_mins(self):
         """Test create a new authentication policy with --computer-tgt-lifetime-mins.
 
@@ -547,24 +503,27 @@ class AuthPolicyCmdTestCase(SiloTest):
         self.assertEqual(result, -1)
         self.assertIn("Cannot have both --by-group and --by-silo options.", err)
 
-    def test_create__service_allowed_to_authenticate_from_repeated(self):
+    def test_service_allowed_to_authenticate_from__set_repeated(self):
         """Test repeating similar arguments doesn't make sense to use together.
 
-        --service-allowed-to-authenticate-from
-        --service-allowed-to-authenticate-from-device-silo
+        service-allowed-to-authenticate-from set --device-group
+        service-allowed-to-authenticate-from set --device-silo
         """
-        sddl = 'O:SYG:SYD:(XA;OICI;CR;;;WD;(@USER.ad://ext/AuthenticationSilo == "Managers"))'
         name = self.unique_name()
 
-        result, out, err = self.runcmd("domain", "auth", "policy", "create",
-                                       "--name", name,
-                                       "--service-allowed-to-authenticate-from",
-                                       sddl,
-                                       "--service-allowed-to-authenticate-from-device-silo",
+        self.runcmd("domain", "auth", "policy", "create", "--name", name)
+        self.addCleanup(self.delete_authentication_policy, name=name, force=True)
+
+        result, out, err = self.runcmd("domain", "auth", "policy",
+                                       "service-allowed-to-authenticate-from",
+                                       "set", "--name", name,
+                                       "--device-group",
+                                       self.device_group.name,
+                                       "--device-silo",
                                        "QA")
 
         self.assertEqual(result, -1)
-        self.assertIn("--service-allowed-to-authenticate-from argument repeated 2 times.", err)
+        self.assertIn("Cannot have both --device-group and --device-silo options.", err)
 
     def test_service_allowed_to_authenticate_to__set_repeated(self):
         """Test repeating similar arguments doesn't make sense to use together.
@@ -938,8 +897,8 @@ class AuthPolicyCmdTestCase(SiloTest):
         sddl = ndr_unpack(security.descriptor, desc).as_sddl()
         self.assertEqual(sddl, expected)
 
-    def test_modify__service_allowed_to_authenticate_from_device_group(self):
-        """Test the --service-allowed-to-authenticate-from-device-group shortcut."""
+    def test_service_allowed_to_authenticate_from__set_device_group(self):
+        """Tests the service-allowed-to-authenticate-from set --device-group shortcut."""
         name = self.unique_name()
         expected = "O:SYG:SYD:(XA;OICI;CR;;;WD;(Member_of_any {SID(%s)}))" % (
             self.device_group.object_sid)
@@ -949,10 +908,10 @@ class AuthPolicyCmdTestCase(SiloTest):
         self.runcmd("domain", "auth", "policy", "create", "--name", name)
 
         # Modify user allowed to authenticate from silo field
-        result, out, err = self.runcmd("domain", "auth", "policy", "modify",
-                                       "--name", name,
-                                       "--service-allowed-to-authenticate-from-device-group",
-                                       self.device_group.name)
+        result, out, err = self.runcmd("domain", "auth", "policy",
+                                       "service-allowed-to-authenticate-from",
+                                       "set", "--name", name,
+                                       "--device-group", self.device_group.name)
         self.assertIsNone(result, msg=err)
 
         # Check generated SDDL.
@@ -961,8 +920,8 @@ class AuthPolicyCmdTestCase(SiloTest):
         sddl = ndr_unpack(security.descriptor, desc).as_sddl()
         self.assertEqual(sddl, expected)
 
-    def test_modify__service_allowed_to_authenticate_from_device_silo(self):
-        """Test the --service-allowed-to-authenticate-from-device-silo shortcut."""
+    def test_service_allowed_to_authenticate_from__set_device_silo(self):
+        """Tests the service-allowed-to-authenticate-from set --device-silo shortcut."""
         name = self.unique_name()
 
         # Create a policy to modify for this test.
@@ -970,10 +929,10 @@ class AuthPolicyCmdTestCase(SiloTest):
         self.runcmd("domain", "auth", "policy", "create", "--name", name)
 
         # Modify user allowed to authenticate from silo field
-        result, out, err = self.runcmd("domain", "auth", "policy", "modify",
-                                       "--name", name,
-                                       "--service-allowed-to-authenticate-from-device-silo",
-                                       "Developers")
+        result, out, err = self.runcmd("domain", "auth", "policy",
+                                       "service-allowed-to-authenticate-from",
+                                       "set", "--name", name,
+                                       "--device-silo", "Developers")
         self.assertIsNone(result, msg=err)
 
         # Check generated SDDL.