[PATCH] samba-tool: Easily edit a users object in AD
Rowland Penny
rpenny at samba.org
Tue Jun 27 09:30:55 UTC 2017
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.
Rowland
More information about the samba-technical
mailing list