[PATCH] samba-tool: Easily edit a users object in AD
Rowland Penny
rpenny at samba.org
Tue Jun 27 10:12:50 UTC 2017
On Tue, 27 Jun 2017 12:44:14 +0300
Alexander Bokovoy <ab at samba.org> wrote:
> On ti, 27 kesä 2017, Rowland Penny via samba-technical wrote:
> > On Tue, 27 Jun 2017 14:35:42 +1200
> > Douglas Bagnall <douglas.bagnall at catalyst.net.nz> wrote:
> >
> > > hi Rowland,
> > >
> > > On 24/06/17 02:27, Rowland Penny via samba-technical wrote:
> > > > Hi, the attached patch is something I created to use instead of
> > > > running ldbedit when I just wanted to alter one of a users
> > > > attributes.
> > >
> > > Nice documentation, and it seems to avoid some ldbedit pitfalls
> > > (e.g. --show-deleted), but I see a few issues.
> > >
> > > 1: tests.
> > >
> > > 2..n: interspersed below.
> > > >
> > > > Rowland
> > > >
> > > >
> > > > samba-tool-Easily-edit-a-users-object-in-AD.patch
> > > >
> > > >
> > > > From 540f2bced8c8a6172fdef6aea6d74ef6584c4455 Mon Sep 17
> > > > 00:00:00 2001 From: Rowland Penny <rpenny at samba.org>
> > > > Date: Fri, 23 Jun 2017 14:27:54 +0100
> > > > Subject: [PATCH] samba-tool: Easily edit a users object in AD,
> > > > as if using ldbedit.
> > > >
> > > > Signed-off-by: Rowland Penny <rpenny at samba.org>
> > > > ---
> > > > python/samba/netcmd/user.py | 137
> > > > +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 136
> > > > insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/python/samba/netcmd/user.py
> > > > b/python/samba/netcmd/user.py index 53ac39f..6b12dde 100644
> > > > --- a/python/samba/netcmd/user.py
> > > > +++ b/python/samba/netcmd/user.py
> > > > @@ -21,6 +21,9 @@ import samba.getopt as options
> > > > import ldb
> > > > import pwd
> > > > import os
> > > > +import re
> > > > +import tempfile
> > > > +import difflib
> > > > import sys
> > > > import fcntl
> > > > import signal
> > > > @@ -28,7 +31,7 @@ import errno
> > > > import time
> > > > import base64
> > > > import binascii
> > > > -from subprocess import Popen, PIPE, STDOUT
> > > > +from subprocess import Popen, PIPE, STDOUT, call,
> > > > CalledProcessError
> > >
> > > You want check_call, not call if you're expecting a
> > > CalledProcessError. call returns the status code.
> > >
> > > > from getpass import getpass
> > > > from samba.auth import system_session
> > > > from samba.samdb import SamDB
> > > > @@ -2283,6 +2286,137 @@ samba-tool user syncpasswords
> > > > --terminate \\ update_pid(None)
> > > > return
> > > >
> > > > +class cmd_user_edit(Command):
> > > > + """Modify User AD object.
> > > > +
> > > > +This command will allow editing of a user account in the Active
> > > > Directory +domain. You will then be able to add or change
> > > > attributes and their values. +
> > > > +The username 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 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 +LDAP server.
> > > > +
> > > > +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 +LDAP server.
> > > > +
> > > > +Example3:
> > > > +samba-tool user edit User3 --editor=nano
> > > > +
> > > > +Example3 shows how to edit a users attributes in the domain
> > > > against a local +LDAP server using the 'nano' editor.
> > > > +
> > > > +"""
> > > > + synopsis = "%prog <username> [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 = ["username"]
> > > > + takes_optiongroups = {
> > > > + "sambaopts": options.SambaOptions,
> > > > + "credopts": options.CredentialsOptions,
> > > > + "versionopts": options.VersionOptions,
> > > > + }
> > > > +
> > > > + 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=805306368)(sAMAccountName=%s))" %
> > > ^^^
> > > It would be easier to read with
> > >
> > > "sAMAccountType=%d"... % (dsdb.UF_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:
> > > > + call([editor, t_file.name])
> > > > + except CalledProcessError, e:
> > > > + raise IOError("ERROR: ", e)
> > >
> > > Is there are reason to prefer IOError over CalledProcessError?
> > >
> > > > + 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(':')
> > >
> > > Here you want line.split(':', 1), or you'll get an error when a
> > > value contains a colon (like "url: http://example.com").
> > >
> > > > + if attr not in str(plus_lines):
> > > ^^^^^^^^^^^^^^^^^^^^^^^^^^
> > >
> > > This is NOT going to work well, because it matches whenever a
> > > value happens to contain the attribute's name. Contriving an
> > > example:
> > >
> > > cn: foo
> > > - street: 17 Picnic street.
> > > + streetAddress: 17 Picnic Street
> > >
> > > `str(plus_lines)` contains "street" (and incidentally "cn"),
> > > resulting in weird changes.
> > >
> > > What you need to do is something more like:
> > >
> > > minus_attrs = set()
> > > for line in diff:
> > > op = line[:2]
> > > attr, value = line[2:].split(':', 1)
> > > if change == '- ':
> > > minus_attrs.add(attr)
> > > ...
> > >
> > > if attr not in minus_attrs:
> > >
> > >
> > > > + user_ldif += "delete: %s\n" % attr
> > > > + user_ldif += "%s: %s\n" % (attr, val)
> > > > +
> > > > + for line in plus_lines:
> > > > + attr, val = line.split(':')
> > > > + if attr in str(minus_lines):
> > > > + user_ldif += "replace: %s\n" % attr
> > > > + user_ldif += "%s: %s\n" % (attr, val)
> > > > + if attr not in str(minus_lines):
> > > > + user_ldif += "add: %s\n" % attr
> > > > + user_ldif += "%s: %s\n" % (attr, val)
> > > > +
> > > > + try:
> > > > + samdb.modify_ldif(user_ldif)
> > > > + except Exception, e:
> > > > + raise CommandError("Failed to modify user
> > > > '%s': " %
> > > > + username, e)
> > > > +
> > > ^^^^^^^^
> > > Git tells me there's loose whitespace here.
> > >
> > > > + self.outf.write("Modified User '%s'
> > > > successfully\n" % username) +
> > > > class cmd_user(SuperCommand):
> > > > """User management."""
> > > >
> > > > @@ -2298,3 +2432,4 @@ class cmd_user(SuperCommand):
> > > > subcommands["setpassword"] = cmd_user_setpassword()
> > > > subcommands["getpassword"] = cmd_user_getpassword()
> > > > subcommands["syncpasswords"] = cmd_user_syncpasswords()
> > > > + subcommands["edit"] = cmd_user_edit()
> > > > -- 2.1.4
> > > >
> > >
> > > cheers,
> > > Douglas
> >
> > Hi Douglas, thanks for looking at my patch and I take on board most
> > of your comments and will work on the patch. The only comment I
> > have a problem with is this one:
> >
> > It would be easier to read with
> >
> > "sAMAccountType=%d"... % (dsdb.UF_NORMAL_ACCOUNT,...
> >
> > 'pydoc samba.dsdb' shows this:
> >
> > UF_NORMAL_ACCOUNT = 512
> >
> > This means that the filter will become:
> >
> > sAMAccountType=512
> >
> > Don't think that is a valid number for sAMAccountType, it is for
> > userAccountControl though.
> >
> > My way seems to be the accepted filter to just get a user.
> It is samba.dsdb.ATYPE_NORMAL_ACCOUNT, not UF_NORMAL_ACCOUNT. Please
> use this one.
I knew it wasn't correct, but for the ignorant (i.e. me), why use
'ATYPE_NORMAL_ACCOUNT' instead of '805306368' ?
>
> Also please fix 'except FooBar:' to be 'except Foobar as e:'. The
> former is not compatible with Python 3. See
> http://portingguide.readthedocs.io/en/latest/index.html for more
> details on Python 3 compatibility. I suspect, though, we'll have some
> issues with string processing too but it would be nice to reduce
> amount of Python 3 incompatibilities going forward.
>
Will do, this is the least of my worries ;-)
Rowland
More information about the samba-technical
mailing list