[PATCH] samba-tool: Easily edit a users object in AD
Alexander Bokovoy
ab at samba.org
Tue Jun 27 09:44:14 UTC 2017
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.
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.
--
/ Alexander Bokovoy
More information about the samba-technical
mailing list