[PATCH] Allow editing groups from samba-tool
Rowland Penny
rpenny at samba.org
Mon Apr 30 06:55:25 UTC 2018
On Mon, 30 Apr 2018 08:32:14 +0300
Alexander Bokovoy via samba-technical <samba-technical at lists.samba.org>
wrote:
See inline comments.
> 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.
Then why didn't you raise this in the first place ? I would have fixed
it then.
>
> > +
> > + 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.
Please don't add even more greengrocers apostrophes to Samba, it is
<users> not <users's>
> >
> > 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))" %
Why change 'filter' to 'filt' ? is it so that the line below is less
than 80 characters ?
> > (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
> >
>
>
Rowland
More information about the samba-technical
mailing list