Skip to content

SlapdObject updates #463

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

SlapdObject updates #463

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
57493b1
Look for slapadd in /sbin as well
mdavidsaver Mar 8, 2022
d32007e
slapdtest: changes to CLI argument handling
mdavidsaver Apr 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Doc/spelling_wordlist.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ ldapControls
ldapControlTuples
ldapdelete
ldapi
ldapmodify
LDAPObject
ldaps
ldapurl
Expand Down
94 changes: 59 additions & 35 deletions Lib/slapdtest/_slapdtest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,6 @@ def _find_commands(self):
self.PATH_LDAPDELETE = self._find_command('ldapdelete')
self.PATH_LDAPMODIFY = self._find_command('ldapmodify')
self.PATH_LDAPWHOAMI = self._find_command('ldapwhoami')
self.PATH_SLAPADD = self._find_command('slapadd')

self.PATH_SLAPD = os.environ.get('SLAPD', None)
if not self.PATH_SLAPD:
Expand Down Expand Up @@ -483,34 +482,57 @@ def _stopped(self):
self._log.info('slapd[%d] terminated', self._proc.pid)
self._proc = None

def _cli_auth_args(self):
if self.cli_sasl_external:
authc_args = [
'-Y', 'EXTERNAL',
]
if not self._log.isEnabledFor(logging.DEBUG):
authc_args.append('-Q')
else:
authc_args = [
'-x',
'-D', self.root_dn,
'-w', self.root_pw,
]
return authc_args
def _cli_slap(self, cmd=None, extra_args=None, stdin_data=None): # pragma: no cover
"""Entry point for slap* commands directly modifying DB files."""
if self._proc is not None:
self._log.warning('Directly modifying DB files while slapd is running')

args = [self.PATH_SLAPD]

if cmd:
args.append('-T'+cmd)

args += ['-F', self._slapd_conf] + (extra_args or [])

return self._cli_exec(args, stdin_data=stdin_data)

def _cli_ldap(self, ldapcommand, extra_args=None, ldap_uri=None,
bind_dn=None, bind_pw=None,
stdin_data=None): # pragma: no cover
"""
Entry point for ldap* commands, interacting with running slapd
"""
if self._proc is None:
raise RuntimeError('Must start daemon before LDAP access is possible')

# no cover to avoid spurious coverage changes
def _cli_popen(self, ldapcommand, extra_args=None, ldap_uri=None,
stdin_data=None): # pragma: no cover
if ldap_uri is None:
ldap_uri = self.default_ldap_uri

if ldapcommand.split("/")[-1].startswith("ldap"):
args = [ldapcommand, '-H', ldap_uri] + self._cli_auth_args()
args = [ldapcommand, '-H', ldap_uri or self.default_ldap_uri]

if ldap_uri.startswith('ldapi://'):
args += ['-Y', 'EXTERNAL']

if not self._log.isEnabledFor(logging.DEBUG):
args.append('-Q')

if bind_dn or bind_pw:
raise RuntimeError('-Y EXTERNAL ignores -D and -w')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about treating those being passed in as a signal to use a simple bind? Also the self.cli_sasl_external check is gone?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about treating those being passed in as a signal to use a simple bind?

Maybe showing my ignorance here. Is a simple bind meaningful with ldapi://?

So the condition for choosing -Y EXTERNAL would be ldap_uri.startswith('ldapi://') and bind_dn is None?

Also the self.cli_sasl_external check is gone?

Yes. I don't think this was the correct condition. Knowing that ldapi:// is supported doesn't imply that it will be used for a particular operation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SASL EXTERNAL mechanism is independent of the ldapi:// transport. cli_sasl_external tells you whether that mechanism is available/should be used, ldapi:// tells you how to contact the server (over a UNIX socket vs. as opposed to a TCP socket), the LDAP protocol still behaves the same over both.

Hope that clears things up a bit, let me know if not.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example if we set up a TLS layer and used a client certificate while doing so, some servers could also accept the EXTERNAL mechanism, using the client certificate to generate the identity the session should bind as (OpenLDAP does this).

BTW not asking that you take it into account in this pull request, just giving some background.


else:
args = [ldapcommand, '-F', self._slapd_conf]
args += [
'-x',
'-D', bind_dn or self.root_dn,
'-w', bind_pw or self.root_pw,
]

args += (extra_args or [])
if extra_args:
args += extra_args

return self._cli_exec(args, stdin_data=stdin_data)

# no cover to avoid spurious coverage changes
def _cli_exec(self, args, stdin_data=None): # pragma: no cover
self._log.debug('Run command: %r', ' '.join(args))
proc = subprocess.Popen(
args, stdin=subprocess.PIPE, stdout=subprocess.PIPE,
Expand All @@ -530,27 +552,28 @@ def _cli_popen(self, ldapcommand, extra_args=None, ldap_uri=None,
)
return stdout_data, stderr_data

def ldapwhoami(self, extra_args=None):
def ldapwhoami(self, extra_args=None, **kws):
"""
Runs ldapwhoami on this slapd instance
"""
self._cli_popen(self.PATH_LDAPWHOAMI, extra_args=extra_args)
return self._cli_ldap(self.PATH_LDAPWHOAMI, extra_args=extra_args,
**kws)

def ldapadd(self, ldif, extra_args=None):
def ldapadd(self, ldif, extra_args=None, **kws):
"""
Runs ldapadd on this slapd instance, passing it the ldif content
"""
self._cli_popen(self.PATH_LDAPADD, extra_args=extra_args,
stdin_data=ldif.encode('utf-8'))
return self._cli_ldap(self.PATH_LDAPADD, extra_args=extra_args,
stdin_data=ldif.encode('utf-8'), **kws)

def ldapmodify(self, ldif, extra_args=None):
def ldapmodify(self, ldif, extra_args=None, **kws):
"""
Runs ldapadd on this slapd instance, passing it the ldif content
Runs ldapmodify on this slapd instance, passing it the ldif content
"""
self._cli_popen(self.PATH_LDAPMODIFY, extra_args=extra_args,
stdin_data=ldif.encode('utf-8'))
return self._cli_ldap(self.PATH_LDAPMODIFY, extra_args=extra_args,
stdin_data=ldif.encode('utf-8'), **kws)

def ldapdelete(self, dn, recursive=False, extra_args=None):
def ldapdelete(self, dn, recursive=False, extra_args=None, **kws):
"""
Runs ldapdelete on this slapd instance, deleting 'dn'
"""
Expand All @@ -559,14 +582,15 @@ def ldapdelete(self, dn, recursive=False, extra_args=None):
if recursive:
extra_args.append('-r')
extra_args.append(dn)
self._cli_popen(self.PATH_LDAPDELETE, extra_args=extra_args)
return self._cli_ldap(self.PATH_LDAPDELETE, extra_args=extra_args,
**kws)

def slapadd(self, ldif, extra_args=None):
"""
Runs slapadd on this slapd instance, passing it the ldif content
"""
self._cli_popen(
self.PATH_SLAPADD,
return self._cli_slap(
'add',
stdin_data=ldif.encode("utf-8") if ldif else None,
extra_args=extra_args,
)
Expand Down