[PATCH] Allow editing groups from samba-tool

Alexander Bokovoy ab at samba.org
Mon Apr 30 05:32:14 UTC 2018


On ma, 30 huhti 2018, William Brown via samba-technical wrote:
> Hi there,
> 
> samba tool allows a command like "user edit" that opens the object as
> an ldif, and it can be edited and commited.
> 
> This modifies the location of that implementation to be a generic
> EditCommand that can be derived, and demonstrates how it could be
> implemented for group also.
> 
> One minor detail is how we pass in the "type". Should this be a
> "self.attribute" of the EditCommand rather than an argument to
> run_edit? filter makes sense to be an argument due to unknown string
> formatting for the filter, similar with the samaccount name being an
> argument for error messages. But the type could be a self attribute.
I think passing filter is more flexible here.

There is one comment I'd like to address but otherwise it looks good.
See inline.

> 
> Thanks for your review!
> 
> William

> From 9961efc5f599d043479d2376c6d96a378302f57f Mon Sep 17 00:00:00 2001
> From: William Brown <william at blackhats.net.au>
> Date: Mon, 30 Apr 2018 14:28:31 +1200
> Subject: [PATCH] python/samba/netcmd/{__init__.py,user.py,group.py} improve
>  edit command.
> 
> samba-tool allows editing an ldif of objects which can be then commited
> back to the directory server. This makes an EditCommand type which can
> be reused, and implemnets the correct values for group to allow:
> 
> samba-tool group edit ...
> samba-tool user edit ...
> 
> Signed-off-by: William Brown <william at blackhats.net.au>
> ---
>  python/samba/netcmd/__init__.py | 108 ++++++++++++++++++++++++++++++++++++++++
>  python/samba/netcmd/group.py    |  64 +++++++++++++++++++++++-
>  python/samba/netcmd/user.py     |  87 +++-----------------------------
>  3 files changed, 179 insertions(+), 80 deletions(-)
> 
> diff --git a/python/samba/netcmd/__init__.py b/python/samba/netcmd/__init__.py
> index 77976780150..65cfbcfc035 100644
> --- a/python/samba/netcmd/__init__.py
> +++ b/python/samba/netcmd/__init__.py
> @@ -22,6 +22,16 @@ from ldb import LdbError
>  import sys, traceback
>  import textwrap
>  
> +# For editCommand
> +import re
> +import os
> +import ldb
> +import difflib
> +import tempfile
> +from samba.samdb import SamDB
> +from samba.auth import system_session
> +from subprocess import Popen, PIPE, STDOUT, check_call, CalledProcessError
> +
>  class Option(optparse.Option):
>      pass
>  
> @@ -190,6 +200,104 @@ class Command(object):
>          return logger
>  
>  
> +class EditCommand(Command):
> +    """A specialise variant of Command for DS object editing."""
> +
> +    def _run_edit(self, filt, objectname, objecttype, credopts=None,
> +            sambaopts=None, versionopts=None, H=None, editor=None):
> +
> +        lp = sambaopts.get_loadparm()
> +        creds = credopts.get_credentials(lp, fallback_machine=True)
> +        samdb = SamDB(url=H, session_info=system_session(),
> +                      credentials=creds, lp=lp)
> +
> +        domaindn = samdb.domain_dn()
> +
> +        try:
> +            res = samdb.search(base=domaindn,
> +                               expression=filt,
> +                               scope=ldb.SCOPE_SUBTREE)
> +            object_dn = res[0].dn
> +        except IndexError:
> +            raise CommandError('Unable to find %s "%s"' %
> +                                  (objecttype.lower(), objectname))
> +
> +        for msg in res:
> +            r_ldif = samdb.write_ldif(msg, 1)
> +            # remove 'changetype' line
> +            result_ldif = re.sub('changetype: add\n', '', r_ldif)
> +            # Remove line wrapping where it exists.
> +            # This fixes an issue when you do the diff that doesn't know how
> +            # to handle a multi-line value - or add one ....
> +            # In some cases *true* multiline values may need b64 encoding to
> +            # preserve the newlines.
> +            result_ldif = re.sub('\n ', '', result_ldif)
> +
> +            if editor is None:
> +                editor = os.environ.get('EDITOR')
> +                if editor is None:
> +                    editor = 'vi'
> +
> +            # Assert the editor chosen actually exists!
> +            if os.path.exists(editor) is False:
> +                raise CommandError('Unable to access "%s". Does it exist?'
> +                                   % editor)
I know this was partially copied from the existing code but I'd rather
pull the editor checks out of the loop and do this first thing in the
+run_edit() so that we don't even try opening SamDB and search there for
an LDAP record if editor is missing. Original code didn't do 
'raise CommandError' because it assumed 'vi' always exists on the system
but if you are adding a terminal path here, better to move it outside of
the loop.

> +
> +            with tempfile.NamedTemporaryFile(suffix=".tmp") as t_file:
> +                t_file.write(result_ldif)
> +                t_file.flush()
> +                try:
> +                    check_call([editor, t_file.name])
> +                except CalledProcessError as e:
> +                    raise CalledProcessError("ERROR: ", e)
> +                with open(t_file.name) as edited_file:
> +                    edited_message = edited_file.read()
> +
> +        if result_ldif != edited_message:
> +            diff = difflib.ndiff(result_ldif.splitlines(),
> +                                 edited_message.splitlines())
> +            minus_lines = []
> +            plus_lines = []
> +            for line in diff:
> +                if line.startswith('-'):
> +                    line = line[2:]
> +                    minus_lines.append(line)
> +                elif line.startswith('+'):
> +                    line = line[2:]
> +                    plus_lines.append(line)
> +
> +            object_ldif="dn: %s\n" % object_dn
> +            object_ldif += "changetype: modify\n"
> +
> +            for line in minus_lines:
> +                attr, val = line.split(':', 1)
> +                search_attr="%s:" % attr
> +                if not re.search(r'^' + search_attr, str(plus_lines)):
> +                    object_ldif += "delete: %s\n" % attr
> +                    object_ldif += "%s: %s\n" % (attr, val)
> +
> +            for line in plus_lines:
> +                attr, val = line.split(':', 1)
> +                search_attr="%s:" % attr
> +                if re.search(r'^' + search_attr, str(minus_lines)):
> +                    object_ldif += "replace: %s\n" % attr
> +                    object_ldif += "%s: %s\n" % (attr, val)
> +                if not re.search(r'^' + search_attr, str(minus_lines)):
> +                    object_ldif += "add: %s\n" % attr
> +                    object_ldif += "%s: %s\n" % (attr, val)
> +
> +            try:
> +                samdb.modify_ldif(object_ldif)
> +            except Exception as e:
> +                raise CommandError("Failed to modify %s '%s': " %
> +                                   (objecttype.lower(), objectname), e)
> +
> +            self.outf.write("Modified %s '%s' successfully\n" %
> +                               (objecttype, objectname))
> +
> +
> +
> +
>  class SuperCommand(Command):
>      """A samba-tool command with subcommands."""
>  
> diff --git a/python/samba/netcmd/group.py b/python/samba/netcmd/group.py
> index a4969cc6ba9..c2713e1879a 100644
> --- a/python/samba/netcmd/group.py
> +++ b/python/samba/netcmd/group.py
> @@ -17,7 +17,7 @@
>  # along with this program.  If not, see <http://www.gnu.org/licenses/>.
>  
>  import samba.getopt as options
> -from samba.netcmd import Command, SuperCommand, CommandError, Option
> +from samba.netcmd import Command, EditCommand, SuperCommand, CommandError, Option
>  import ldb
>  from samba.ndr import ndr_unpack
>  from samba.dcerpc import security
> @@ -500,6 +500,67 @@ class cmd_group_move(Command):
>          self.outf.write('Moved group "%s" into "%s"\n' %
>                          (groupname, full_new_parent_dn))
>  
> +class cmd_group_edit(EditCommand):
> +    """Modify Group AD object.
> +
> +This command will allow editing of a group object in the Active Directory
> +domain. You will then be able to add or change attributes and their values.
> +
> +The groupname specified on the command is the sAMAccountName.
> +
> +The command may be run from the root userid or another authorized userid.
> +
> +The -H or --URL= option can be used to execute the command against a remote
> +server.
> +
> +Example1:
> +samba-tool user edit Group1 -H ldap://samba.samdom.example.com \
> +-U administrator --password=passw1rd
> +
> +Example1 shows how to edit a group's attributes in the domain against a remote
> +LDAP server.
> +
> +The -H parameter is used to specify the remote target server.
> +
> +Example2:
> +samba-tool user edit Group2
> +
> +Example2 shows how to edit a group's attributes in the domain against a local
> +LDAP server.
> +
> +Example3:
> +samba-tool user edit Group3 --editor=nano
> +
> +Example3 shows how to edit a group's attributes in the domain against a local
> +LDAP server using the 'nano' editor.
> +
> +"""
> +    synopsis = "%prog <groupname> [options]"
> +
> +    takes_options = [
> +        Option("-H", "--URL", help="LDB URL for database or target server",
> +               type=str, metavar="URL", dest="H"),
> +        Option("--editor", help="Editor to use instead of the system default,"
> +               " or 'vi' if no system default is set.", type=str),
> +    ]
> +
> +    takes_args = ["groupname"]
> +    takes_optiongroups = {
> +        "sambaopts": options.SambaOptions,
> +        "credopts": options.CredentialsOptions,
> +        "versionopts": options.VersionOptions,
> +        }
> +
> +    def run(self, groupname, credopts=None, sambaopts=None, versionopts=None,
> +            H=None, editor=None):
> +
> +        filt = ("(&(sAMAccountName=%s)(objectClass=group))" %
> +                  ldb.binary_encode(groupname))
> +
> +        self._run_edit(filt, groupname, 'Group', credopts, sambaopts, versionopts,
> +                       H, editor)
> +
> +
>  class cmd_group(SuperCommand):
>      """Group management."""
>  
> @@ -511,3 +572,4 @@ class cmd_group(SuperCommand):
>      subcommands["list"] = cmd_group_list()
>      subcommands["listmembers"] = cmd_group_list_members()
>      subcommands["move"] = cmd_group_move()
> +    subcommands["edit"] = cmd_group_edit()
> diff --git a/python/samba/netcmd/user.py b/python/samba/netcmd/user.py
> index dfe167d8c7d..89bae061c2b 100644
> --- a/python/samba/netcmd/user.py
> +++ b/python/samba/netcmd/user.py
> @@ -51,6 +51,7 @@ from samba.net import Net
>  from samba.netcmd import (
>      Command,
>      CommandError,
> +    EditCommand,
>      SuperCommand,
>      Option,
>      )
> @@ -2294,7 +2295,7 @@ samba-tool user syncpasswords --terminate \\
>          update_pid(None)
>          return
>  
> -class cmd_user_edit(Command):
> +class cmd_user_edit(EditCommand):
>      """Modify User AD object.
>  
>  This command will allow editing of a user account in the Active Directory
> @@ -2311,7 +2312,7 @@ Example1:
>  samba-tool user edit User1 -H ldap://samba.samdom.example.com \
>  -U administrator --password=passw1rd
>  
> -Example1 shows how to edit a users attributes in the domain against a remote
> +Example1 shows how to edit a user's attributes in the domain against a remote
>  LDAP server.
>  
>  The -H parameter is used to specify the remote target server.
> @@ -2319,13 +2320,13 @@ The -H parameter is used to specify the remote target server.
>  Example2:
>  samba-tool user edit User2
>  
> -Example2 shows how to edit a users attributes in the domain against a local
> +Example2 shows how to edit a user's attributes in the domain against a local
>  LDAP server.
>  
>  Example3:
>  samba-tool user edit User3 --editor=nano
>  
> -Example3 shows how to edit a users attributes in the domain against a local
> +Example3 shows how to edit a user's attributes in the domain against a local
>  LDAP server using the 'nano' editor.
>  
>  """
> @@ -2348,84 +2349,12 @@ LDAP server using the 'nano' editor.
>      def run(self, username, credopts=None, sambaopts=None, versionopts=None,
>              H=None, editor=None):
>  
> -        lp = sambaopts.get_loadparm()
> -        creds = credopts.get_credentials(lp, fallback_machine=True)
> -        samdb = SamDB(url=H, session_info=system_session(),
> -                      credentials=creds, lp=lp)
> -
> -        filter = ("(&(sAMAccountType=%d)(sAMAccountName=%s))" %
> +        filt = ("(&(sAMAccountType=%d)(sAMAccountName=%s))" %
>                    (dsdb.ATYPE_NORMAL_ACCOUNT, ldb.binary_encode(username)))
>  
> -        domaindn = samdb.domain_dn()
> -
> -        try:
> -            res = samdb.search(base=domaindn,
> -                               expression=filter,
> -                               scope=ldb.SCOPE_SUBTREE)
> -            user_dn = res[0].dn
> -        except IndexError:
> -            raise CommandError('Unable to find user "%s"' % (username))
> -
> -        for msg in res:
> -            r_ldif = samdb.write_ldif(msg, 1)
> -            # remove 'changetype' line
> -            result_ldif = re.sub('changetype: add\n', '', r_ldif)
> -
> -            if editor is None:
> -                editor = os.environ.get('EDITOR')
> -                if editor is None:
> -                    editor = 'vi'
> -
> -            with tempfile.NamedTemporaryFile(suffix=".tmp") as t_file:
> -                t_file.write(result_ldif)
> -                t_file.flush()
> -                try:
> -                    check_call([editor, t_file.name])
> -                except CalledProcessError as e:
> -                    raise CalledProcessError("ERROR: ", e)
> -                with open(t_file.name) as edited_file:
> -                    edited_message = edited_file.read()
> -
> -        if result_ldif != edited_message:
> -            diff = difflib.ndiff(result_ldif.splitlines(),
> -                                 edited_message.splitlines())
> -            minus_lines = []
> -            plus_lines = []
> -            for line in diff:
> -                if line.startswith('-'):
> -                    line = line[2:]
> -                    minus_lines.append(line)
> -                elif line.startswith('+'):
> -                    line = line[2:]
> -                    plus_lines.append(line)
> -
> -            user_ldif="dn: %s\n" % user_dn
> -            user_ldif += "changetype: modify\n"
> -
> -            for line in minus_lines:
> -                attr, val = line.split(':', 1)
> -                search_attr="%s:" % attr
> -                if not re.search(r'^' + search_attr, str(plus_lines)):
> -                    user_ldif += "delete: %s\n" % attr
> -                    user_ldif += "%s: %s\n" % (attr, val)
> -
> -            for line in plus_lines:
> -                attr, val = line.split(':', 1)
> -                search_attr="%s:" % attr
> -                if re.search(r'^' + search_attr, str(minus_lines)):
> -                    user_ldif += "replace: %s\n" % attr
> -                    user_ldif += "%s: %s\n" % (attr, val)
> -                if not re.search(r'^' + search_attr, str(minus_lines)):
> -                    user_ldif += "add: %s\n" % attr
> -                    user_ldif += "%s: %s\n" % (attr, val)
> -
> -            try:
> -                samdb.modify_ldif(user_ldif)
> -            except Exception as e:
> -                raise CommandError("Failed to modify user '%s': " %
> -                                   username, e)
> +        self._run_edit(filt, username, 'User', credopts, sambaopts, versionopts,
> +                       H, editor)
>  
> -            self.outf.write("Modified User '%s' successfully\n" % username)
>  
>  class cmd_user_show(Command):
>      """Display a user AD object.
> -- 
> 2.14.3
> 


-- 
/ Alexander Bokovoy



More information about the samba-technical mailing list