[PATCH] samba-tool: Easily edit a users object in AD
Douglas Bagnall
douglas.bagnall at catalyst.net.nz
Tue Jun 27 02:35:42 UTC 2017
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
More information about the samba-technical
mailing list