From 1c3f81852cb8337e2305f968be5bd8165997d27e Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Tue, 14 Oct 2008 17:46:36 -0400 Subject: [PATCH 1/5] Move some functionality from user-add to the backend ldap create function --- ipa_server/plugins/b_ldap.py | 18 ++++++++- ipa_server/servercore.py | 13 ++----- ipa_server/test_client | 12 +++--- ipalib/plugins/f_user.py | 73 ++++++++++++------------------------ test_server | 18 ++++++--- 5 files changed, 63 insertions(+), 71 deletions(-) diff --git a/ipa_server/plugins/b_ldap.py b/ipa_server/plugins/b_ldap.py index 69c2aeb58..600f1c86f 100644 --- a/ipa_server/plugins/b_ldap.py +++ b/ipa_server/plugins/b_ldap.py @@ -25,7 +25,11 @@ This wraps the python-ldap bindings. import ldap as _ldap from ipalib import api +from ipalib import errors from ipalib.crud import CrudBackend +from ipa_server import servercore +from ipa_server import ipaldap +import ldap class ldap(CrudBackend): @@ -46,6 +50,18 @@ class ldap(CrudBackend): ) def create(self, **kw): - return kw + if servercore.entry_exists(kw['dn']): + raise errors.DuplicateEntry("entry already exists") + + entry = ipaldap.Entry(kw['dn']) + + # dn isn't allowed to be in the entry itself + del kw['dn'] + + # Fill in our new entry + for k in kw: + entry.setValues(k, kw[k]) + + return servercore.add_entry(entry) api.register(ldap) diff --git a/ipa_server/servercore.py b/ipa_server/servercore.py index 3e98e6f61..7310104df 100644 --- a/ipa_server/servercore.py +++ b/ipa_server/servercore.py @@ -184,18 +184,13 @@ def get_user_by_uid(uid, sattrs): # User support -def user_exists(uid): - """Return True if the exists, False otherwise.""" - # FIXME: fix the filter - # FIXME: should accept a container to look in -# uid = self.__safe_filter(uid) - searchfilter = "(&(uid=%s)(objectclass=posixAccount))" % uid - +def entry_exists(dn): + """Return True if the entry exists, False otherwise.""" try: - get_sub_entry("cn=accounts," + basedn, searchfilter, ['dn','uid']) + get_base_entry(dn, "objectclass=*", ['dn','objectclass']) return True except errors.NotFound: - return True + return False def get_user_by_uid (uid, sattrs): """Get a specific user's entry. Return as a dict of values. diff --git a/ipa_server/test_client b/ipa_server/test_client index 364fd3b81..3b4794d95 100755 --- a/ipa_server/test_client +++ b/ipa_server/test_client @@ -13,16 +13,16 @@ def user_find(uid): # main server = xmlrpclib.ServerProxy("http://localhost:8888/") -print server.system.listMethods() -print server.system.methodHelp("user_add") +#print server.system.listMethods() +#print server.system.methodHelp("user_add") try: - args="admin" + args="jsmith1" kw = {'givenname':'Joe', 'sn':'Smith'} - result = server.user_add(args, kw) + result = server.user_add(kw, args) print "returned %s" % result except xmlrpclib.Fault, e: print e.faultString -user_find("admin") -user_find("notfound") +#user_find("admin") +#user_find("notfound") diff --git a/ipalib/plugins/f_user.py b/ipalib/plugins/f_user.py index b2c191fbe..e3ecd2234 100644 --- a/ipalib/plugins/f_user.py +++ b/ipalib/plugins/f_user.py @@ -70,16 +70,17 @@ class user(frontend.Object): default_from=lambda givenname, sn: givenname[0] + sn, normalize=lambda value: value.lower(), ), - Param('gecos', + Param('gecos?', doc='GECOS field', default_from=lambda uid: uid, ), - Param('homedirectory', + Param('homedirectory?', cli_name='home', doc='Path of user home directory', default_from=lambda uid: '/home/%s' % uid, ), - Param('shell', + Param('loginshell?', + cli_name='shell', default=u'/bin/sh', doc='Login shell', ), @@ -110,44 +111,22 @@ class user_add(crud.Add): ldap = self.api.Backend.ldap kw['uid'] = uid kw['dn'] = ldap.get_user_dn(uid) - return ldap.create(**kw) - - if servercore.user_exists(user['uid']): - raise errors.Duplicate("user already exists") - if servercore.uid_too_long(user['uid']): + if servercore.uid_too_long(kw['uid']): raise errors.UsernameTooLong - # dn is set here, not by the user - try: - del user['dn'] - except KeyError: - pass - - # No need to set empty fields, and they can cause issues when they - # get to LDAP, like: - # TypeError: ('expected a string in the list', None) - for k in user.keys(): - if not user[k] or len(user[k]) == 0 or (isinstance(user[k],list) and len(user[k]) == 1 and '' in user[k]): - del user[k] - - dn="uid=%s,%s,%s" % (ldap.dn.escape_dn_chars(user['uid']), - user_container,servercore.basedn) - - entry = ipaldap.Entry(dn) - # Get our configuration config = servercore.get_ipa_config() # Let us add in some missing attributes - if user.get('homedirectory') is None: - user['homedirectory'] = '%s/%s' % (config.get('ipahomesrootdir'), user.get('uid')) - user['homedirectory'] = user['homedirectory'].replace('//', '/') - user['homedirectory'] = user['homedirectory'].rstrip('/') - if user.get('loginshell') is None: - user['loginshell'] = config.get('ipadefaultloginshell') - if user.get('gecos') is None: - user['gecos'] = user['uid'] + if kw.get('homedirectory') is None: + kw['homedirectory'] = '%s/%s' % (config.get('ipahomesrootdir'), kw.get('uid')) + kw['homedirectory'] = kw['homedirectory'].replace('//', '/') + kw['homedirectory'] = kw['homedirectory'].rstrip('/') + if kw.get('loginshell') is None: + kw['loginshell'] = config.get('ipadefaultloginshell') + if kw.get('gecos') is None: + kw['gecos'] = kw['uid'] # If uidnumber is blank the the FDS dna_plugin will automatically # assign the next value. So we don't have to do anything with it. @@ -156,33 +135,27 @@ class user_add(crud.Add): try: default_group = servercore.get_entry_by_dn(group_dn, ['dn','gidNumber']) if default_group: - user['gidnumber'] = default_group.get('gidnumber') + kw['gidnumber'] = default_group.get('gidnumber') except errors.NotFound: - # Fake an LDAP error so we can return something useful to the user - raise errors.NotFound, "The default group for new users, '%s', cannot be found." % config.get('ipadefaultprimarygroup') + # Fake an LDAP error so we can return something useful to the kw + raise errors.NotFound, "The default group for new kws, '%s', cannot be found." % config.get('ipadefaultprimarygroup') except Exception, e: # catch everything else raise e - if user.get('krbprincipalname') is None: - user['krbprincipalname'] = "%s@%s" % (user.get('uid'), servercore.realm) + if kw.get('krbprincipalname') is None: + kw['krbprincipalname'] = "%s@%s" % (kw.get('uid'), servercore.realm) # FIXME. This is a hack so we can request separate First and Last # name in the GUI. - if user.get('cn') is None: - user['cn'] = "%s %s" % (user.get('givenname'), - user.get('sn')) + if kw.get('cn') is None: + kw['cn'] = "%s %s" % (kw.get('givenname'), + kw.get('sn')) # some required objectclasses - entry.setValues('objectClass', (config.get('ipauserobjectclasses'))) - # entry.setValues('objectClass', ['top', 'person', 'organizationalPerson', 'inetOrgPerson', 'inetUser', 'posixAccount', 'krbPrincipalAux']) + kw['objectClass'] = config.get('ipauserobjectclasses') - # fill in our new entry with everything sent by the user - for u in user: - entry.setValues(u, user[u]) - - result = servercore.add_entry(entry) - return result + return ldap.create(**kw) api.register(user_add) diff --git a/test_server b/test_server index d26fd5962..d58d00af7 100755 --- a/test_server +++ b/test_server @@ -8,6 +8,7 @@ import re import threading import commands from ipalib import api +from ipalib import config from ipa_server import conn from ipa_server.servercore import context import ipalib.load_plugins @@ -46,19 +47,21 @@ class LoggingSimpleXMLRPCRequestHandler(SimpleXMLRPCServer.SimpleXMLRPCRequestHa """ # this is fine for our test server - uid = commands.getoutput('/usr/bin/id -u') + # uid = commands.getoutput('/usr/bin/id -u') + uid = "500" krbccache = "FILE:/tmp/krb5cc_" + uid func = None try: - # FIXME: don't hardcode host and port - context.conn = conn.IPAConn("localhost", 389, krbccache) try: # check to see if a matching function has been registered func = funcs[method] except KeyError: raise Exception('method "%s" is not supported' % method) (args, kw) = xmlrpc_unmarshal(*params) + # FIXME: don't hardcode host and port + context.conn = conn.IPAConn("localhost", 389, krbccache) + logger.info("calling %s" % method) return func(*args, **kw) finally: # Clean up any per-request data and connections @@ -140,9 +143,14 @@ XMLRPCServer = StoppableXMLRPCServer(("",PORT), LoggingSimpleXMLRPCRequestHandle XMLRPCServer.register_introspection_functions() -# Get and register all the methods -api.env.server_context = True api.finalize() + +# Initialize our environment +env_dict = config.read_config() +env_dict['server_context'] = True +api.env.update(config.generate_env(env_dict)) + +# Get and register all the methods for cmd in api.Command: logger.info("registering %s" % cmd) XMLRPCServer.register_function(api.Command[cmd], cmd) From cfc8450efd92dc0fb6648e97b27416c67625adfb Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Tue, 14 Oct 2008 22:22:01 -0400 Subject: [PATCH 2/5] Port user-show to new CrudBackend framework --- ipa_server/plugins/b_ldap.py | 38 ++++++++++++++++++++++++++++++++++-- ipalib/config.py | 3 +++ ipalib/crud.py | 8 +++++--- ipalib/plugins/b_xmlrpc.py | 2 +- ipalib/plugins/f_user.py | 31 +++++++++++++++++------------ 5 files changed, 63 insertions(+), 19 deletions(-) diff --git a/ipa_server/plugins/b_ldap.py b/ipa_server/plugins/b_ldap.py index 600f1c86f..84f25245b 100644 --- a/ipa_server/plugins/b_ldap.py +++ b/ipa_server/plugins/b_ldap.py @@ -29,7 +29,6 @@ from ipalib import errors from ipalib.crud import CrudBackend from ipa_server import servercore from ipa_server import ipaldap -import ldap class ldap(CrudBackend): @@ -39,7 +38,7 @@ class ldap(CrudBackend): dn = _ldap.dn - def get_user_dn(self, uid): + def make_user_dn(self, uid): """ Construct user dn from uid. """ @@ -49,6 +48,35 @@ class ldap(CrudBackend): self.api.env.basedn, ) + def find_entry_dn(self, key_attribute, primary_key, attributes=None, + object_type=None): + """ + Find an existing entry's dn from an attribute + """ + key_attribute = key_attribute.lower() + if not object_type: + if key_attribute == "uid": # User + filter = "posixAccount" + elif key_attribute == "cn": # Group + object_type = "posixGroup" + elif key_attribute == "krbprincipal": # Service + object_type = "krbPrincipal" + + if not object_type: + return None + + filter = "(&(%s=%s)(objectclass=%s))" % ( + key_attribute, + self.dn.escape_dn_chars(primary_key), + object_type + ) + + search_base = "%s, %s" % (self.api.env.container_accounts, self.api.env.basedn) + + entry = servercore.get_sub_entry(search_base, filter, attributes) + + return entry['dn'] + def create(self, **kw): if servercore.entry_exists(kw['dn']): raise errors.DuplicateEntry("entry already exists") @@ -64,4 +92,10 @@ class ldap(CrudBackend): return servercore.add_entry(entry) + def retrieve(self, dn, attributes=None): + return servercore.get_entry_by_dn(dn, attributes) + + def delete(self, dn): + return servercore.delete_entry(dn) + api.register(ldap) diff --git a/ipalib/config.py b/ipalib/config.py index a606a40b1..42bf7787b 100644 --- a/ipalib/config.py +++ b/ipalib/config.py @@ -25,8 +25,11 @@ DEFAULT_CONF='/etc/ipa/ipa.conf' def generate_env(d={}): default = dict( + container_accounts = 'cn=accounts', basedn = 'dc=example,dc=com', container_user = 'cn=users,cn=accounts', + container_group = 'cn=groups,cn=accounts', + container_service = 'cn=services,cn=accounts', domain = LazyProp(get_domain), interactive = True, query_dns = True, diff --git a/ipalib/crud.py b/ipalib/crud.py index 5a60ac8c6..5cd7b0a41 100644 --- a/ipalib/crud.py +++ b/ipalib/crud.py @@ -87,12 +87,14 @@ class CrudBackend(backend.Backend): """ raise NotImplementedError('%s.create()' % self.name) - def retrieve(self, primary_key): + def retrieve(self, primary_key, attributes): """ Retrieve an existing entry. - This method should take a single argument, the primary_key of the - entry in question. + This method should take a two arguments: the primary_key of the + entry in question and a list of the attributes to be retrieved. + If the list of attributes is None then all non-operational + attributes will be returned. If such an entry exists, this method should return a dict representing that entry. If no such entry exists, this method diff --git a/ipalib/plugins/b_xmlrpc.py b/ipalib/plugins/b_xmlrpc.py index 618f8385d..db2af1abd 100644 --- a/ipalib/plugins/b_xmlrpc.py +++ b/ipalib/plugins/b_xmlrpc.py @@ -58,6 +58,6 @@ class xmlrpc(Backend): print "%s: %s" % (code, getattr(err,'__doc__','')) else: raise err - return False + return {} api.register(xmlrpc) diff --git a/ipalib/plugins/f_user.py b/ipalib/plugins/f_user.py index e3ecd2234..1e79c4b8d 100644 --- a/ipalib/plugins/f_user.py +++ b/ipalib/plugins/f_user.py @@ -110,7 +110,7 @@ class user_add(crud.Add): assert 'dn' not in kw ldap = self.api.Backend.ldap kw['uid'] = uid - kw['dn'] = ldap.get_user_dn(uid) + kw['dn'] = ldap.make_user_dn(uid) if servercore.uid_too_long(kw['uid']): raise errors.UsernameTooLong @@ -244,18 +244,23 @@ api.register(user_find) class user_show(crud.Get): 'Examine an existing user.' - def execute(self, *args, **kw): - uid=args[0] - result = servercore.get_user_by_uid(uid, ["*"]) - return result - def forward(self, *args, **kw): - try: - result = super(crud.Get, self).forward(*args, **kw) - if not result: return - for a in result: - print a, ": ", result[a] - except errors.NotFound: - print "User %s not found" % args[0] + def execute(self, uid, **kw): + """ + Execute the user-show operation. + + The dn should not be passed as a keyword argument as it is constructed + by this method. + + Returns the entry + + :param uid: The login name of the user to retrieve. + :param kw: Not used. + """ + ldap = self.api.Backend.ldap + dn = ldap.find_entry_dn("uid", uid, ["*"], "posixAccount") + # FIXME: should kw contain the list of attributes? + return ldap.retrieve(dn) + api.register(user_show) class user_lock(frontend.Command): From f7c044495ae22d372fb064dbacfe0ff027c437a7 Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Tue, 14 Oct 2008 22:48:57 -0400 Subject: [PATCH 3/5] Port user_del to CrudBackend Override output_for_cli() to generate nicer output --- ipalib/plugins/f_user.py | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/ipalib/plugins/f_user.py b/ipalib/plugins/f_user.py index 1e79c4b8d..79c457353 100644 --- a/ipalib/plugins/f_user.py +++ b/ipalib/plugins/f_user.py @@ -157,23 +157,30 @@ class user_add(crud.Add): return ldap.create(**kw) + def output_for_cli(self, ret): + """ + Output result of this command to command line interface. + """ + if ret: + print "User added" + api.register(user_add) class user_del(crud.Del): 'Delete an existing user.' - def execute(self, *args, **kw): - """args[0] = uid of the user to remove - - Delete a user. Not to be confused with inactivate_user. This + def execute(self, uid, **kw): + """Delete a user. Not to be confused with inactivate_user. This makes the entry go away completely. uid is the uid of the user to delete The memberOf plugin handles removing the user from any other groups. + + :param uid: The login name of the user being added. + :param kw: Not used. """ - uid = args[0] if uid == "admin": # FIXME: do we still want a "special" user? raise SyntaxError("admin required") @@ -183,11 +190,16 @@ class user_del(crud.Del): if not user: raise errors.NotFound - return servercore.delete_entry(user['dn']) - def forward(self, *args, **kw): - result = super(crud.Del, self).forward(*args, **kw) - if result: - print "User %s removed" % args[0] + ldap = self.api.Backend.ldap + dn = ldap.find_entry_dn("uid", uid, ["*"], "posixAccount") + return ldap.delete(dn) + def output_for_cli(self, ret): + """ + Output result of this command to command line interface. + """ + if ret: + print "User deleted" + api.register(user_del) From e7937f294445d53396f7fb87d52eb4d4c9b97110 Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Wed, 15 Oct 2008 09:57:49 -0400 Subject: [PATCH 4/5] Add missing * to *kw to make it pass named arguments instead of positional --- ipalib/crud.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ipalib/crud.py b/ipalib/crud.py index 5cd7b0a41..ba4d57187 100644 --- a/ipalib/crud.py +++ b/ipalib/crud.py @@ -70,7 +70,7 @@ class CrudBackend(backend.Backend): Base class defining generic CRUD backend API. """ - def create(self, *kw): + def create(self, **kw): """ Create a new entry. @@ -102,7 +102,7 @@ class CrudBackend(backend.Backend): """ raise NotImplementedError('%s.retrieve()' % self.name) - def update(self, primary_key, *kw): + def update(self, primary_key, **kw): """ Update an existing entry. From 789a248daa71d5d1377e0dc9f0cd3afe107d4f2a Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Wed, 15 Oct 2008 09:58:29 -0400 Subject: [PATCH 5/5] Port user-mod to use ldap update() method --- ipa_server/plugins/b_ldap.py | 15 +++++++++++--- ipalib/plugins/f_user.py | 38 ++++++++++++++++++++---------------- 2 files changed, 33 insertions(+), 20 deletions(-) diff --git a/ipa_server/plugins/b_ldap.py b/ipa_server/plugins/b_ldap.py index 84f25245b..26a3495c3 100644 --- a/ipa_server/plugins/b_ldap.py +++ b/ipa_server/plugins/b_ldap.py @@ -48,8 +48,7 @@ class ldap(CrudBackend): self.api.env.basedn, ) - def find_entry_dn(self, key_attribute, primary_key, attributes=None, - object_type=None): + def find_entry_dn(self, key_attribute, primary_key, object_type=None): """ Find an existing entry's dn from an attribute """ @@ -73,7 +72,7 @@ class ldap(CrudBackend): search_base = "%s, %s" % (self.api.env.container_accounts, self.api.env.basedn) - entry = servercore.get_sub_entry(search_base, filter, attributes) + entry = servercore.get_sub_entry(search_base, filter, ['dn', 'objectclass']) return entry['dn'] @@ -95,6 +94,16 @@ class ldap(CrudBackend): def retrieve(self, dn, attributes=None): return servercore.get_entry_by_dn(dn, attributes) + def update(self, dn, **kw): + result = self.retrieve(dn, ["*"]) + + entry = ipaldap.Entry((dn, servercore.convert_scalar_values(result))) + + for k in kw: + entry.setValues(k, kw[k]) + + return servercore.update_entry(entry.toDict()) + def delete(self, dn): return servercore.delete_entry(dn) diff --git a/ipalib/plugins/f_user.py b/ipalib/plugins/f_user.py index 79c457353..e95ee3b2f 100644 --- a/ipalib/plugins/f_user.py +++ b/ipalib/plugins/f_user.py @@ -205,27 +205,31 @@ api.register(user_del) class user_mod(crud.Mod): 'Edit an existing user.' - def execute(self, *args, **kw): - uid=args[0] + def execute(self, uid, **kw): + """ + Execute the user-mod operation. - # Get the existing user entry - result = servercore.get_sub_entry("cn=accounts," + servercore.basedn, "uid=%s" % uid, ["*"]) + The dn should not be passed as a keyword argument as it is constructed + by this method. - user = kw - dn = result.get('dn') - del result['dn'] - entry = ipaldap.Entry((dn, servercore.convert_scalar_values(result))) + Returns the entry - for u in user: - entry.setValues(u, user[u]) + :param uid: The login name of the user to retrieve. + :param kw: Keyword arguments for the other LDAP attributes. + """ + assert 'uid' not in kw + assert 'dn' not in kw + ldap = self.api.Backend.ldap + dn = ldap.find_entry_dn("uid", uid, "posixAccount") + return ldap.update(dn, **kw) - result = servercore.update_entry(entry.toDict()) + def output_for_cli(self, ret): + """ + Output result of this command to command line interface. + """ + if ret: + print "User updated" - return result - def forward(self, *args, **kw): - result = super(crud.Mod, self).forward(*args, **kw) - if result: - print "User %s modified" % args[0] api.register(user_mod) @@ -269,7 +273,7 @@ class user_show(crud.Get): :param kw: Not used. """ ldap = self.api.Backend.ldap - dn = ldap.find_entry_dn("uid", uid, ["*"], "posixAccount") + dn = ldap.find_entry_dn("uid", uid, "posixAccount") # FIXME: should kw contain the list of attributes? return ldap.retrieve(dn)