netcmd: silo command remove combined --policy which set all 3

doesn't make much sense to set all 3 to the same policy, user authentication policy, service authentication policy, computer authentication policy

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>

4 files changed, 42 insertions, 72 deletions

diff --git a/python/samba/netcmd/domain/auth/silo.py b/python/samba/netcmd/domain/auth/silo.py
index 0c486aeeaff..945fef7785f 100644
--- a/python/samba/netcmd/domain/auth/silo.py
+++ b/python/samba/netcmd/domain/auth/silo.py
@@ -115,9 +115,6 @@ class cmd_domain_auth_silo_create(Command):
         Option("--description",
                help="Optional description for authentication silo.",
                dest="description", action="store", type=str),
-        Option("--policy",
-               help="Use single policy for all principals in this silo.",
-               dest="policy", action="store", type=str),
         Option("--user-policy",
                help="User account policy.",
                dest="user_policy", action="store", type=str),
@@ -154,22 +151,15 @@ class cmd_domain_auth_silo_create(Command):
             raise CommandError(e)
 
     def run(self, hostopts=None, sambaopts=None, credopts=None, name=None,
-            description=None, policy=None, user_policy=None,
-            service_policy=None, computer_policy=None, protect=None,
-            unprotect=None, audit=None, enforce=None):
+            description=None, user_policy=None, service_policy=None,
+            computer_policy=None, protect=None, unprotect=None, audit=None,
+            enforce=None):
 
         if protect and unprotect:
             raise CommandError("--protect and --unprotect cannot be used together.")
         if audit and enforce:
             raise CommandError("--audit and --enforce cannot be used together.")
 
-        # If --policy is present start with that as the base. Then optionally
-        # --user-policy, --service-policy, --computer-policy can override this.
-        if policy is not None:
-            user_policy = user_policy or policy
-            service_policy = service_policy or policy
-            computer_policy = computer_policy or policy
-
         ldb = self.ldb_connect(hostopts, sambaopts, credopts)
 
         try:
@@ -233,9 +223,6 @@ class cmd_domain_auth_silo_modify(Command):
         Option("--description",
                help="Optional description for authentication silo.",
                dest="description", action="store", type=str),
-        Option("--policy",
-               help="Set single policy for all principals in this silo.",
-               dest="policy", action="store", type=str),
         Option("--user-policy",
                help="Set User account policy.",
                dest="user_policy", action="store", type=str),
@@ -272,22 +259,15 @@ class cmd_domain_auth_silo_modify(Command):
             raise CommandError(e)
 
     def run(self, hostopts=None, sambaopts=None, credopts=None, name=None,
-            description=None, policy=None, user_policy=None,
-            service_policy=None, computer_policy=None, protect=None,
-            unprotect=None, audit=None, enforce=None):
+            description=None, user_policy=None, service_policy=None,
+            computer_policy=None, protect=None, unprotect=None, audit=None,
+            enforce=None):
 
         if audit and enforce:
             raise CommandError("--audit and --enforce cannot be used together.")
         if protect and unprotect:
             raise CommandError("--protect and --unprotect cannot be used together.")
 
-        # If --policy is set then start with that for all policies.
-        # They can be individually overridden as well after that.
-        if policy is not None:
-            user_policy = user_policy or policy
-            service_policy = service_policy or policy
-            computer_policy = computer_policy or policy
-
         ldb = self.ldb_connect(hostopts, sambaopts, credopts)
 
         try:
diff --git a/python/samba/tests/samba_tool/domain_auth_base.py b/python/samba/tests/samba_tool/domain_auth_base.py
index a0f423767c6..505674d7d91 100644
--- a/python/samba/tests/samba_tool/domain_auth_base.py
+++ b/python/samba/tests/samba_tool/domain_auth_base.py
@@ -40,17 +40,16 @@ class BaseAuthCmdTest(SambaToolCmdTest):
 
     @classmethod
     def setUpTestData(cls):
-        cls.create_authentication_policy(name="Single Policy")
         cls.create_authentication_policy(name="User Policy")
         cls.create_authentication_policy(name="Service Policy")
         cls.create_authentication_policy(name="Computer Policy")
 
         cls.create_authentication_silo(name="Developers",
                                        description="Developers, Developers",
-                                       policy="Single Policy")
+                                       user_policy="User Policy")
         cls.create_authentication_silo(name="Managers",
                                        description="Managers",
-                                       policy="Single Policy")
+                                       user_policy="User Policy")
         cls.create_authentication_silo(name="QA",
                                        description="Quality Assurance",
                                        user_policy="User Policy",
@@ -147,7 +146,7 @@ class BaseAuthCmdTest(SambaToolCmdTest):
         assert "Deleted authentication policy" in out
 
     @classmethod
-    def create_authentication_silo(cls, name, description=None, policy=None,
+    def create_authentication_silo(cls, name, description=None,
                                    user_policy=None, service_policy=None,
                                    computer_policy=None, audit=False,
                                    protect=False):
@@ -156,14 +155,13 @@ class BaseAuthCmdTest(SambaToolCmdTest):
         # Base command for create authentication policy.
         cmd = ["domain", "auth", "silo", "create", "--name", name]
 
-        # If --policy is present, use a singular authentication policy.
-        # otherwise use --user-policy, --service-policy, --computer-policy
-        if policy is not None:
-            cmd += ["--policy", policy]
-        else:
-            cmd += ["--user-policy", user_policy,
-                    "--service-policy", service_policy,
-                    "--computer-policy", computer_policy]
+        # Authentication policies.
+        if user_policy:
+            cmd += ["--user-policy", user_policy]
+        if service_policy:
+            cmd += ["--service-policy", service_policy]
+        if computer_policy:
+            cmd += ["--computer-policy", computer_policy]
 
         # Other optional attributes.
         if description is not None:
diff --git a/python/samba/tests/samba_tool/domain_auth_policy.py b/python/samba/tests/samba_tool/domain_auth_policy.py
index 0cf5d15ca8b..9aa94becfd3 100644
--- a/python/samba/tests/samba_tool/domain_auth_policy.py
+++ b/python/samba/tests/samba_tool/domain_auth_policy.py
@@ -40,8 +40,7 @@ class AuthPolicyCmdTestCase(BaseAuthCmdTest):
         result, out, err = self.runcmd("domain", "auth", "policy", "list")
         self.assertIsNone(result, msg=err)
 
-        expected_policies = [
-            "Single Policy", "User Policy", "Service Policy", "Computer Policy"]
+        expected_policies = ["User Policy", "Service Policy", "Computer Policy"]
 
         for policy in expected_policies:
             self.assertIn(policy, out)
@@ -55,8 +54,7 @@ class AuthPolicyCmdTestCase(BaseAuthCmdTest):
         # we should get valid json
         policies = json.loads(out)
 
-        expected_policies = [
-            "Single Policy", "User Policy", "Service Policy", "Computer Policy"]
+        expected_policies = ["User Policy", "Service Policy", "Computer Policy"]
 
         for name in expected_policies:
             policy = policies[name]
@@ -69,14 +67,14 @@ class AuthPolicyCmdTestCase(BaseAuthCmdTest):
     def test_authentication_policy_view(self):
         """Test viewing a single authentication policy."""
         result, out, err = self.runcmd("domain", "auth", "policy", "view",
-                                       "--name", "Single Policy")
+                                       "--name", "User Policy")
         self.assertIsNone(result, msg=err)
 
         # we should get valid json
         policy = json.loads(out)
 
         # check a few fields only
-        self.assertEqual(policy["cn"], "Single Policy")
+        self.assertEqual(policy["cn"], "User Policy")
         self.assertEqual(policy["msDS-AuthNPolicyEnforced"], True)
 
     def test_authentication_policy_view_notfound(self):
@@ -256,9 +254,9 @@ class AuthPolicyCmdTestCase(BaseAuthCmdTest):
     def test_authentication_policy_create_already_exists(self):
         """Test creating a new authentication policy that already exists."""
         result, out, err = self.runcmd("domain", "auth", "policy", "create",
-                                       "--name", "Single Policy")
+                                       "--name", "User Policy")
         self.assertEqual(result, -1)
-        self.assertIn("Authentication policy Single Policy already exists", err)
+        self.assertIn("Authentication policy User Policy already exists", err)
 
     def test_authentication_policy_create_name_missing(self):
         """Test create authentication policy without --name argument."""
@@ -541,7 +539,7 @@ class AuthPolicyCmdTestCase(BaseAuthCmdTest):
     def test_authentication_policy_modify_audit_enforce_together(self):
         """Test modify auth policy using both --audit and --enforce."""
         result, out, err = self.runcmd("domain", "auth", "policy", "modify",
-                                       "--name", "Single Policy",
+                                       "--name", "User Policy",
                                        "--audit", "--enforce")
         self.assertEqual(result, -1)
         self.assertIn("--audit and --enforce cannot be used together.", err)
@@ -549,7 +547,7 @@ class AuthPolicyCmdTestCase(BaseAuthCmdTest):
     def test_authentication_policy_modify_protect_unprotect_together(self):
         """Test modify authentication policy using --protect and --unprotect."""
         result, out, err = self.runcmd("domain", "auth", "policy", "modify",
-                                       "--name", "Single Policy",
+                                       "--name", "User Policy",
                                        "--protect", "--unprotect")
         self.assertEqual(result, -1)
         self.assertIn("--protect and --unprotect cannot be used together.", err)
@@ -560,7 +558,7 @@ class AuthPolicyCmdTestCase(BaseAuthCmdTest):
         with patch.object(SamDB, "modify") as modify_mock:
             modify_mock.side_effect = ModelError("Custom error message")
             result, out, err = self.runcmd("domain", "auth", "policy", "modify",
-                                           "--name", "Single Policy",
+                                           "--name", "User Policy",
                                            "--description", "New description")
             self.assertEqual(result, -1)
             self.assertIn("Custom error message", err)
diff --git a/python/samba/tests/samba_tool/domain_auth_silo.py b/python/samba/tests/samba_tool/domain_auth_silo.py
index c1d73058b43..632609fcd3f 100644
--- a/python/samba/tests/samba_tool/domain_auth_silo.py
+++ b/python/samba/tests/samba_tool/domain_auth_silo.py
@@ -58,8 +58,6 @@ class AuthSiloCmdTestCase(BaseAuthCmdTest):
             silo = silos[name]
             self.assertIn("msDS-AuthNPolicySilo", list(silo["objectClass"]))
             self.assertIn("description", silo)
-            self.assertIn("msDS-ComputerAuthNPolicy", silo)
-            self.assertIn("msDS-ServiceAuthNPolicy", silo)
             self.assertIn("msDS-UserAuthNPolicy", silo)
             self.assertIn("objectGUID", silo)
 
@@ -96,15 +94,13 @@ class AuthSiloCmdTestCase(BaseAuthCmdTest):
 
         result, out, err = self.runcmd("domain", "auth", "silo", "create",
                                        "--name", "singlePolicy",
-                                       "--policy", "Single Policy")
+                                       "--user-policy", "User Policy")
         self.assertIsNone(result, msg=err)
 
         # Check silo that was created
         silo = self.get_authentication_silo("singlePolicy")
         self.assertEqual(str(silo["cn"]), "singlePolicy")
-        self.assertIn("Single Policy", str(silo["msDS-UserAuthNPolicy"]))
-        self.assertIn("Single Policy", str(silo["msDS-ServiceAuthNPolicy"]))
-        self.assertIn("Single Policy", str(silo["msDS-ComputerAuthNPolicy"]))
+        self.assertIn("User Policy", str(silo["msDS-UserAuthNPolicy"]))
         self.assertEqual(str(silo["msDS-AuthNPolicySiloEnforced"]), "TRUE")
 
     def test_authentication_silo_create_multiple_policies(self):
@@ -129,36 +125,34 @@ class AuthSiloCmdTestCase(BaseAuthCmdTest):
 
     def test_authentication_silo_create_policy_dn(self):
         """Test creating a new authentication silo when policy is a dn."""
-        policy = self.get_authentication_policy("Single Policy")
+        policy = self.get_authentication_policy("User Policy")
 
         self.addCleanup(self.delete_authentication_silo,
                         name="singlePolicyDN", force=True)
 
         result, out, err = self.runcmd("domain", "auth", "silo", "create",
                                        "--name", "singlePolicyDN",
-                                       "--policy", policy["dn"])
+                                       "--user-policy", policy["dn"])
         self.assertIsNone(result, msg=err)
 
         # Check silo that was created
         silo = self.get_authentication_silo("singlePolicyDN")
         self.assertEqual(str(silo["cn"]), "singlePolicyDN")
         self.assertIn(str(policy["name"]), str(silo["msDS-UserAuthNPolicy"]))
-        self.assertIn(str(policy["name"]), str(silo["msDS-ServiceAuthNPolicy"]))
-        self.assertIn(str(policy["name"]), str(silo["msDS-ComputerAuthNPolicy"]))
         self.assertEqual(str(silo["msDS-AuthNPolicySiloEnforced"]), "TRUE")
 
     def test_authentication_silo_create_already_exists(self):
         """Test creating a new authentication silo that already exists."""
         result, out, err = self.runcmd("domain", "auth", "silo", "create",
                                        "--name", "Developers",
-                                       "--policy", "Single Policy")
+                                       "--user-policy", "User Policy")
         self.assertEqual(result, -1)
         self.assertIn("Authentication silo Developers already exists.", err)
 
     def test_authentication_silo_create_name_missing(self):
         """Test create authentication silo without --name argument."""
         result, out, err = self.runcmd("domain", "auth", "silo", "create",
-                                       "--policy", "Single Policy")
+                                       "--user-policy", "User Policy")
         self.assertEqual(result, -1)
         self.assertIn("Argument --name is required.", err)
 
@@ -169,7 +163,7 @@ class AuthSiloCmdTestCase(BaseAuthCmdTest):
 
         result, out, err = self.runcmd("domain", "auth", "silo", "create",
                                        "--name", "auditPolicies",
-                                       "--policy", "Single Policy",
+                                       "--user-policy", "User Policy",
                                        "--audit")
         self.assertIsNone(result, msg=err)
 
@@ -184,7 +178,7 @@ class AuthSiloCmdTestCase(BaseAuthCmdTest):
 
         result, out, err = self.runcmd("domain", "auth", "silo", "create",
                                        "--name", "enforcePolicies",
-                                       "--policy", "Single Policy",
+                                       "--user-policy", "User Policy",
                                        "--enforce")
         self.assertIsNone(result, msg=err)
 
@@ -196,7 +190,7 @@ class AuthSiloCmdTestCase(BaseAuthCmdTest):
         """Test create authentication silo using both --audit and --enforce."""
         result, out, err = self.runcmd("domain", "auth", "silo", "create",
                                        "--name", "enforceTogether",
-                                       "--policy", "Single Policy",
+                                       "--user-policy", "User Policy",
                                        "--audit", "--enforce")
         self.assertEqual(result, -1)
         self.assertIn("--audit and --enforce cannot be used together.", err)
@@ -205,7 +199,7 @@ class AuthSiloCmdTestCase(BaseAuthCmdTest):
         """Test create authentication silo using --protect and --unprotect."""
         result, out, err = self.runcmd("domain", "auth", "silo",
                                        "create", "--name", "protectTogether",
-                                       "--policy", "Single Policy",
+                                       "--user-policy", "User Policy",
                                        "--protect", "--unprotect")
         self.assertEqual(result, -1)
         self.assertIn("--protect and --unprotect cannot be used together.", err)
@@ -214,7 +208,7 @@ class AuthSiloCmdTestCase(BaseAuthCmdTest):
         """Test create authentication silo with a policy that doesn't exist."""
         result, out, err = self.runcmd("domain", "auth", "silo", "create",
                                        "--name", "policyNotFound",
-                                       "--policy", "Invalid Policy")
+                                       "--user-policy", "Invalid Policy")
         self.assertEqual(result, -1)
         self.assertIn("Authentication policy Invalid Policy not found.", err)
 
@@ -225,7 +219,7 @@ class AuthSiloCmdTestCase(BaseAuthCmdTest):
             add_mock.side_effect = ModelError("Custom error message")
             result, out, err = self.runcmd("domain", "auth", "silo", "create",
                                            "--name", "createFails",
-                                           "--policy", "Single Policy")
+                                           "--user-policy", "User Policy")
             self.assertEqual(result, -1)
             self.assertIn("Custom error message", err)
 
@@ -347,7 +341,7 @@ class AuthSiloCmdTestCase(BaseAuthCmdTest):
         # Create non-protected authentication silo.
         result, out, err = self.runcmd("domain", "auth", "silo", "create",
                                        "--name=deleteTest",
-                                       "--policy", "User Policy")
+                                       "--user-policy", "User Policy")
         self.assertIsNone(result, msg=err)
         silo = self.get_authentication_silo("deleteTest")
         self.assertIsNotNone(silo)
@@ -366,7 +360,7 @@ class AuthSiloCmdTestCase(BaseAuthCmdTest):
         # Create protected authentication silo.
         result, out, err = self.runcmd("domain", "auth", "silo", "create",
                                        "--name=deleteProtected",
-                                       "--policy", "User Policy",
+                                       "--user-policy", "User Policy",
                                        "--protect")
         self.assertIsNone(result, msg=err)
         silo = self.get_authentication_silo("deleteProtected")
@@ -408,7 +402,7 @@ class AuthSiloCmdTestCase(BaseAuthCmdTest):
         # Create protected authentication silo.
         result, out, err = self.runcmd("domain", "auth", "silo", "create",
                                        "--name=deleteForceFail",
-                                       "--policy", "User Policy",
+                                       "--user-policy", "User Policy",
                                        "--protect")
         self.assertIsNone(result, msg=err)
         silo = self.get_authentication_silo("deleteForceFail")
@@ -429,7 +423,7 @@ class AuthSiloCmdTestCase(BaseAuthCmdTest):
         # Create regular authentication silo.
         result, out, err = self.runcmd("domain", "auth", "silo", "create",
                                        "--name=regularSilo",
-                                       "--policy", "User Policy")
+                                       "--user-policy", "User Policy")
         self.assertIsNone(result, msg=err)
         silo = self.get_authentication_silo("regularSilo")
         self.assertIsNotNone(silo)
@@ -450,7 +444,7 @@ class AuthSiloCmdTestCase(BaseAuthCmdTest):
         # Create protected authentication silo.
         result, out, err = self.runcmd("domain", "auth", "silo", "create",
                                        "--name=protectedSilo",
-                                       "--policy", "User Policy",
+                                       "--user-policy", "User Policy",
                                        "--protect")
         self.assertIsNone(result, msg=err)
         silo = self.get_authentication_silo("protectedSilo")