netcmd: auth policy: add OptionGroup classes for user, service and computer options

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/getopt.py b/python/samba/getopt.py
index 210288f9a0d9fbeb99bc534a9677c59e33ee5747..fe475477ebf836b4f3e4bfb676b737548de942ad 100644 (file)
--- a/python/samba/getopt.py
+++ b/python/samba/getopt.py
@@ -171,6 +171,29 @@ class OptionParser(optparse.OptionParser):
         return super().check_values(values, args)
 
 
+class OptionGroup(optparse.OptionGroup):
+    """Samba OptionGroup base class.
+
+    Provides a generic set_option method to be used as Option callback,
+    so that one doesn't need to be created for every available Option.
+
+    Also overrides the add_option method, so it correctly initialises
+    the defaults on the OptionGroup.
+    """
+
+    def add_option(self, *args, **kwargs):
+        """Override add_option so it applies defaults during constructor."""
+        opt = super().add_option(*args, **kwargs)
+        default = None if opt.default == optparse.NO_DEFAULT else opt.default
+        self.set_option(opt, opt.get_opt_string(), default, self.parser)
+        return opt
+
+    def set_option(self, option, opt_str, arg, parser):
+        """Callback to set the attribute based on the Option dest name."""
+        dest = option.dest or option._long_opts[0][2:].replace("-", "_")
+        setattr(self, dest, arg)
+
+
 class SambaOptions(optparse.OptionGroup):
     """General Samba-related command line options."""
 

diff --git a/python/samba/netcmd/__init__.py b/python/samba/netcmd/__init__.py
index ca7727b42b62ddab856cd5b02040ee2b041bc9d3..31541a983474bd8149323799eaa3a06a5480dc44 100644 (file)
--- a/python/samba/netcmd/__init__.py
+++ b/python/samba/netcmd/__init__.py
@@ -224,7 +224,7 @@ class Command(object):
         kwargs = dict(opts.__dict__)
         for option_group in parser.option_groups:
             for option in option_group.option_list:
-                if option.dest is not None:
+                if option.dest is not None and option.dest in kwargs:
                     del kwargs[option.dest]
         kwargs.update(optiongroups)
 

diff --git a/python/samba/netcmd/domain/auth/policy.py b/python/samba/netcmd/domain/auth/policy.py
index 7d7dea7ef03870b6d3ab0b228c21268e2cb84374..6ee85602907e90562915a9afbf25a864831a7ed6 100644 (file)
--- a/python/samba/netcmd/domain/auth/policy.py
+++ b/python/samba/netcmd/domain/auth/policy.py
@@ -29,6 +29,75 @@ from samba.netcmd.domain.models.exceptions import ModelError
 from samba.netcmd.validators import Range
 
 
+class UserOptions(options.OptionGroup):
+    """User options used by policy create and policy modify commands."""
+
+    def __init__(self, parser):
+        super().__init__(parser, "User Options")
+
+        self.add_option("--user-tgt-lifetime",
+                        help="Ticket-Granting-Ticket lifetime for user accounts.",
+                        dest="tgt_lifetime", type=int, action="callback",
+                        callback=self.set_option,
+                        validators=[Range(min=MIN_TGT_LIFETIME, max=MAX_TGT_LIFETIME)])
+        self.add_option("--user-allow-ntlm-auth",
+                        help="Allow NTLM network authentication when user "
+                             "is restricted to selected devices.",
+                        dest="allow_ntlm_auth", default=False,
+                        action="callback", callback=self.set_option)
+        self.add_option("--user-allowed-to-authenticate-from",
+                        help="Conditions user is allowed to authenticate from.",
+                        type=str, dest="allowed_to_authenticate_from",
+                        action="callback", callback=self.set_option)
+        self.add_option("--user-allowed-to-authenticate-to",
+                        help="Conditions user is allowed to authenticate to.",
+                        type=str, dest="allowed_to_authenticate_to",
+                        action="callback", callback=self.set_option)
+
+
+class ServiceOptions(options.OptionGroup):
+    """Service options used by policy create and policy modify commands."""
+
+    def __init__(self, parser):
+        super().__init__(parser, "Service Options")
+
+        self.add_option("--service-tgt-lifetime",
+                        help="Ticket-Granting-Ticket lifetime for service accounts.",
+                        dest="tgt_lifetime", type=int, action="callback",
+                        callback=self.set_option,
+                        validators=[Range(min=MIN_TGT_LIFETIME, max=MAX_TGT_LIFETIME)])
+        self.add_option("--service-allow-ntlm-auth",
+                        help="Allow NTLM network authentication when service "
+                             "is restricted to selected devices.",
+                        dest="allow_ntlm_auth", default=False,
+                        action="callback", callback=self.set_option)
+        self.add_option("--service-allowed-to-authenticate-from",
+                        help="Conditions service is allowed to authenticate from.",
+                        type=str, dest="allowed_to_authenticate_from",
+                        action="callback", callback=self.set_option)
+        self.add_option("--service-allowed-to-authenticate-to",
+                        help="Conditions service is allowed to authenticate to.",
+                        type=str, dest="allowed_to_authenticate_to",
+                        action="callback", callback=self.set_option)
+
+
+class ComputerOptions(options.OptionGroup):
+    """Computer options used by policy create and policy modify commands."""
+
+    def __init__(self, parser):
+        super().__init__(parser, "Computer Options")
+
+        self.add_option("--computer-tgt-lifetime",
+                        help="Ticket-Granting-Ticket lifetime for computer accounts.",
+                        dest="tgt_lifetime", type=int, action="callback",
+                        callback=self.set_option,
+                        validators=[Range(min=MIN_TGT_LIFETIME, max=MAX_TGT_LIFETIME)])
+        self.add_option("--computer-allowed-to-authenticate-to",
+                        help="Conditions computer is allowed to authenticate to.",
+                        type=str, dest="allowed_to_authenticate_to",
+                        action="callback", callback=self.set_option)
+
+
 class cmd_domain_auth_policy_list(Command):
     """List authentication policies on the domain."""
 
@@ -108,6 +177,9 @@ class cmd_domain_auth_policy_create(Command):
         "sambaopts": options.SambaOptions,
         "credopts": options.CredentialsOptions,
         "hostopts": options.HostOptions,
+        "useropts": UserOptions,
+        "serviceopts": ServiceOptions,
+        "computeropts": ComputerOptions,
     }
 
     takes_options = [
@@ -133,53 +205,12 @@ class cmd_domain_auth_policy_create(Command):
                dest="strong_ntlm_policy", type="choice", action="store",
                choices=StrongNTLMPolicy.get_choices(),
                default="Disabled"),
-        Option("--user-tgt-lifetime",
-               help="Ticket-Granting-Ticket lifetime for user accounts.",
-               dest="user_tgt_lifetime", type=int, action="store",
-               validators=[Range(min=MIN_TGT_LIFETIME, max=MAX_TGT_LIFETIME)]),
-        Option("--user-allow-ntlm-auth",
-               help="Allow NTLM network authentication when user "
-                    "is restricted to selected devices.",
-               dest="user_allow_ntlm_auth", action="store_true",
-               default=False),
-        Option("--user-allowed-to-authenticate-from",
-               help="Conditions user is allowed to authenticate from.",
-               dest="user_allowed_to_authenticate_from", type=str, action="store"),
-        Option("--user-allowed-to-authenticate-to",
-               help="Conditions user is allowed to authenticate to.",
-               dest="user_allowed_to_authenticate_to", type=str, action="store"),
-        Option("--service-tgt-lifetime",
-               help="Ticket-Granting-Ticket lifetime for service accounts.",
-               dest="service_tgt_lifetime", type=int, action="store",
-               validators=[Range(min=MIN_TGT_LIFETIME, max=MAX_TGT_LIFETIME)]),
-        Option("--service-allow-ntlm-auth",
-               help="Allow NTLM network authentication when service "
-                    "is restricted to selected devices.",
-               dest="service_allow_ntlm_auth", action="store_true",
-               default=False),
-        Option("--service-allowed-to-authenticate-from",
-               help="Conditions service is allowed to authenticate from.",
-               dest="service_allowed_to_authenticate_from", type=str, action="store"),
-        Option("--service-allowed-to-authenticate-to",
-               help="Conditions service is allowed to authenticate to.",
-               dest="service_allowed_to_authenticate_to", type=str, action="store"),
-        Option("--computer-tgt-lifetime",
-               help="Ticket-Granting-Ticket lifetime for computer accounts.",
-               dest="computer_tgt_lifetime", type=int, action="store",
-               validators=[Range(min=MIN_TGT_LIFETIME, max=MAX_TGT_LIFETIME)]),
-        Option("--computer-allowed-to-authenticate-to",
-               help="Conditions computer is allowed to authenticate to.",
-               dest="computer_allowed_to_authenticate_to", type=str, action="store"),
     ]
 
-    def run(self, hostopts=None, sambaopts=None, credopts=None, name=None,
-            description=None, protect=None, unprotect=None, audit=None,
-            enforce=None, strong_ntlm_policy=None, user_tgt_lifetime=None,
-            user_allow_ntlm_auth=None, user_allowed_to_authenticate_from=None,
-            user_allowed_to_authenticate_to=None, service_tgt_lifetime=None,
-            service_allow_ntlm_auth=None, service_allowed_to_authenticate_from=None,
-            service_allowed_to_authenticate_to=None, computer_tgt_lifetime=None,
-            computer_allowed_to_authenticate_to=None):
+    def run(self, hostopts=None, sambaopts=None, credopts=None, useropts=None,
+            serviceopts=None, computeropts=None, name=None, description=None,
+            protect=None, unprotect=None, audit=None, enforce=None,
+            strong_ntlm_policy=None):
 
         if protect and unprotect:
             raise CommandError("--protect and --unprotect cannot be used together.")
@@ -202,16 +233,16 @@ class cmd_domain_auth_policy_create(Command):
             cn=name,
             description=description,
             strong_ntlm_policy=StrongNTLMPolicy[strong_ntlm_policy.upper()],
-            user_allow_ntlm_auth=user_allow_ntlm_auth,
-            user_tgt_lifetime=user_tgt_lifetime,
-            user_allowed_to_authenticate_from=user_allowed_to_authenticate_from,
-            user_allowed_to_authenticate_to=user_allowed_to_authenticate_to,
-            service_allow_ntlm_auth=service_allow_ntlm_auth,
-            service_tgt_lifetime=service_tgt_lifetime,
-            service_allowed_to_authenticate_from=service_allowed_to_authenticate_from,
-            service_allowed_to_authenticate_to=service_allowed_to_authenticate_to,
-            computer_tgt_lifetime=computer_tgt_lifetime,
-            computer_allowed_to_authenticate_to=computer_allowed_to_authenticate_to,
+            user_allow_ntlm_auth=useropts.allow_ntlm_auth,
+            user_tgt_lifetime=useropts.tgt_lifetime,
+            user_allowed_to_authenticate_from=useropts.allowed_to_authenticate_from,
+            user_allowed_to_authenticate_to=useropts.allowed_to_authenticate_to,
+            service_allow_ntlm_auth=serviceopts.allow_ntlm_auth,
+            service_tgt_lifetime=serviceopts.tgt_lifetime,
+            service_allowed_to_authenticate_from=serviceopts.allowed_to_authenticate_from,
+            service_allowed_to_authenticate_to=serviceopts.allowed_to_authenticate_to,
+            computer_tgt_lifetime=computeropts.tgt_lifetime,
+            computer_allowed_to_authenticate_to=computeropts.allowed_to_authenticate_to,
         )
 
         # Either --enforce will be set or --audit but never both.
@@ -243,6 +274,9 @@ class cmd_domain_auth_policy_modify(Command):
         "sambaopts": options.SambaOptions,
         "credopts": options.CredentialsOptions,
         "hostopts": options.HostOptions,
+        "useropts": UserOptions,
+        "serviceopts": ServiceOptions,
+        "computeropts": ComputerOptions,
     }
 
     takes_options = [
@@ -267,53 +301,12 @@ class cmd_domain_auth_policy_modify(Command):
                help=f"Strong NTLM Policy ({StrongNTLMPolicy.choices_str()}).",
                dest="strong_ntlm_policy", type="choice", action="store",
                choices=StrongNTLMPolicy.get_choices()),
-        Option("--user-tgt-lifetime",
-               help="Ticket-Granting-Ticket lifetime for user accounts.",
-               dest="user_tgt_lifetime", type=int, action="store",
-               validators=[Range(min=MIN_TGT_LIFETIME, max=MAX_TGT_LIFETIME)]),
-        Option("--user-allow-ntlm-auth",
-               help="Allow NTLM network authentication when user "
-                    "is restricted to selected devices.",
-               dest="user_allow_ntlm_auth", action="store_true",
-               default=False),
-        Option("--user-allowed-to-authenticate-from",
-               help="Conditions user is allowed to authenticate from.",
-               dest="user_allowed_to_authenticate_from", type=str, action="store"),
-        Option("--user-allowed-to-authenticate-to",
-               help="Conditions user is allowed to authenticate to.",
-               dest="user_allowed_to_authenticate_to", type=str, action="store"),
-        Option("--service-tgt-lifetime",
-               help="Ticket-Granting-Ticket lifetime for service accounts.",
-               dest="service_tgt_lifetime", type=int, action="store",
-               validators=[Range(min=MIN_TGT_LIFETIME, max=MAX_TGT_LIFETIME)]),
-        Option("--service-allow-ntlm-auth",
-               help="Allow NTLM network authentication when service "
-                    "is restricted to selected devices.",
-               dest="service_allow_ntlm_auth", action="store_true",
-               default=False),
-        Option("--service-allowed-to-authenticate-from",
-               help="Conditions service is allowed to authenticate from.",
-               dest="service_allowed_to_authenticate_from", type=str, action="store"),
-        Option("--service-allowed-to-authenticate-to",
-               help="Conditions service is allowed to authenticate to.",
-               dest="service_allowed_to_authenticate_to", type=str, action="store"),
-        Option("--computer-tgt-lifetime",
-               help="Ticket-Granting-Ticket lifetime for computer accounts.",
-               dest="computer_tgt_lifetime", type=int, action="store",
-               validators=[Range(min=MIN_TGT_LIFETIME, max=MAX_TGT_LIFETIME)]),
-        Option("--computer-allowed-to-authenticate-to",
-               help="Conditions computer is allowed to authenticate to.",
-               dest="computer_allowed_to_authenticate_to", type=str, action="store"),
     ]
 
-    def run(self, hostopts=None, sambaopts=None, credopts=None, name=None,
-            description=None, protect=None, unprotect=None, audit=None,
-            enforce=None, strong_ntlm_policy=None, user_tgt_lifetime=None,
-            user_allow_ntlm_auth=None, user_allowed_to_authenticate_from=None,
-            user_allowed_to_authenticate_to=None, service_tgt_lifetime=None,
-            service_allow_ntlm_auth=None, service_allowed_to_authenticate_from=None,
-            service_allowed_to_authenticate_to=None, computer_tgt_lifetime=None,
-            computer_allowed_to_authenticate_to=None):
+    def run(self, hostopts=None, sambaopts=None, credopts=None, useropts=None,
+            serviceopts=None, computeropts=None, name=None, description=None,
+            protect=None, unprotect=None, audit=None, enforce=None,
+            strong_ntlm_policy=None):
 
         if protect and unprotect:
             raise CommandError("--protect and --unprotect cannot be used together.")
@@ -348,40 +341,40 @@ class cmd_domain_auth_policy_modify(Command):
             policy.strong_ntlm_policy = \
                 StrongNTLMPolicy[strong_ntlm_policy.upper()]
 
-        if user_tgt_lifetime is not None:
-            policy.user_tgt_lifetime = user_tgt_lifetime
+        if useropts.tgt_lifetime is not None:
+            policy.user_tgt_lifetime = useropts.tgt_lifetime
 
-        if user_allowed_to_authenticate_from is not None:
+        if useropts.allowed_to_authenticate_from is not None:
             policy.user_allowed_to_authenticate_from = \
-                user_allowed_to_authenticate_from
+                useropts.allowed_to_authenticate_from
 
-        if user_allowed_to_authenticate_to is not None:
+        if useropts.allowed_to_authenticate_to is not None:
             policy.user_allowed_to_authenticate_to = \
-                user_allowed_to_authenticate_to
+                useropts.allowed_to_authenticate_to
 
         # Service sign on
         ##################
 
-        if service_tgt_lifetime is not None:
-            policy.service_tgt_lifetime = service_tgt_lifetime
+        if serviceopts.tgt_lifetime is not None:
+            policy.service_tgt_lifetime = serviceopts.tgt_lifetime
 
-        if service_allowed_to_authenticate_from is not None:
+        if serviceopts.allowed_to_authenticate_from is not None:
             policy.service_allowed_to_authenticate_from = \
-                service_allowed_to_authenticate_from
+                serviceopts.allowed_to_authenticate_from
 
-        if service_allowed_to_authenticate_to is not None:
+        if serviceopts.allowed_to_authenticate_to is not None:
             policy.service_allowed_to_authenticate_to = \
-                service_allowed_to_authenticate_to
+                serviceopts.allowed_to_authenticate_to
 
         # Computer
         ###########
 
-        if computer_tgt_lifetime is not None:
-            policy.computer_tgt_lifetime = computer_tgt_lifetime
+        if computeropts.tgt_lifetime is not None:
+            policy.computer_tgt_lifetime = computeropts.tgt_lifetime
 
-        if computer_allowed_to_authenticate_to is not None:
+        if computeropts.allowed_to_authenticate_to is not None:
             policy.computer_allowed_to_authenticate_to = \
-                computer_allowed_to_authenticate_to
+                computeropts.allowed_to_authenticate_to
 
         # Update policy.
         try: