[Freeipa-devel] [PATCH] Update of update
Kevin McCarthy
kmccarth at redhat.com
Mon Aug 20 17:49:23 UTC 2007
Kevin McCarthy wrote:
> Rob Crittenden wrote:
> > Pull some of the logic into the server-side. Change update_user to take a
> > User object and do the conversion to a dict internally instead.
> >
> > Don't allow the User object to use __setattr__ except for dn and data. We
> > want them to use setValue() or setValues() so the cidict is used.
> >
> > The ipa-usermod tool is updated to use the new API.
>
> Rob and I discussed this over IRC. This introduces a subtle change in
> how updates work - old unchanged values will end up overwriting a value
> someone else may have updated "in the middle".
>
> I'm going to take a different shot at this and post a patch for this.
This is almos the same as Rob's patch, but just changes it to cache the
original values when the user is instantiated.
If this looks good, I'd like the pull the utf-8 conversion inside the
user object and then change add_user() to take a user object parameter
too.
-Kevin
-------------- next part --------------
# HG changeset patch
# User Kevin McCarthy <kmccarth at redhat.com>
# Date 1187632211 25200
# Node ID a69ececded00cb296b06631bbbef47dfdfe61dd8
# Parent b92cec02be1299dfc9f36358820d221420be4352
Embed origiginal values inside user, and have update_user pass in a user
object. Based on rcrit's original patch.
Push scalar to list value conversion inside funcs.py.
diff -r b92cec02be12 -r a69ececded00 ipa-admintools/ipa-usermod
--- a/ipa-admintools/ipa-usermod Fri Aug 17 15:32:05 2007 -0700
+++ b/ipa-admintools/ipa-usermod Mon Aug 20 10:50:11 2007 -0700
@@ -48,27 +48,23 @@ def parse_options():
return options, args
def main():
- olduser={}
- newuser={}
options, args = parse_options()
if len(args) != 2:
usage()
client = ipaclient.IPAClient()
- u = client.get_user(args[1])
- olduser = u.toDict()
- newuser = u.toDict()
+ user = client.get_user(args[1])
if options.gecos:
- newuser['gecos'] = [options.gecos]
+ user.setValue('gecos', options.gecos)
if options.directory:
- newuser['homedirectory'] = [options.directory]
+ user.setValue('homedirectory', options.directory)
if options.shell:
- newuser['loginshell'] = [options.shell]
+ user.setValue('loginshell', options.shell)
try:
- client.update_user(olduser, newuser)
+ client.update_user(user)
print args[1] + " successfully modified"
except xmlrpclib.Fault, f:
print f.faultString
diff -r b92cec02be12 -r a69ececded00 ipa-python/ipaclient.py
--- a/ipa-python/ipaclient.py Fri Aug 17 15:32:05 2007 -0700
+++ b/ipa-python/ipaclient.py Mon Aug 20 10:50:11 2007 -0700
@@ -116,14 +116,12 @@ class IPAClient:
return users
- def update_user(self,olduser,newuser):
- """Update a user entry. olduser is a dict of attribute/value pairs
- of the original entry. newuser is a dict of attribute/value pairs
- of the new entry."""
+ def update_user(self,user):
+ """Update a user entry."""
realm = config.config.get_realm()
- result = self.transport.update_user(olduser,newuser)
+ result = self.transport.update_user(user.origDataDict(), user.toDict())
return result
def mark_user_deleted(self,uid):
diff -r b92cec02be12 -r a69ececded00 ipa-python/user.py
--- a/ipa-python/user.py Fri Aug 17 15:32:05 2007 -0700
+++ b/ipa-python/user.py Mon Aug 20 10:50:11 2007 -0700
@@ -11,7 +11,8 @@ class User:
In python-ldap, entries are returned as a list of 2-tuples.
Instance variables:
dn - string - the string DN of the entry
- data - cidict - case insensitive dict of the attributes and values"""
+ data - cidict - case insensitive dict of the attributes and values
+ orig_data - cidict - case insentiive dict of the original attributes and values"""
def __init__(self,entrydata):
"""data is the raw data returned from the python-ldap result method,
@@ -32,6 +33,8 @@ class User:
self.dn = ''
self.data = ldap.cidict.cidict()
+ self.orig_data = dict(self.data)
+
def __nonzero__(self):
"""This allows us to do tests like if entry: returns false if there is no data,
true otherwise"""
@@ -40,6 +43,14 @@ class User:
def hasAttr(self,name):
"""Return True if this entry has an attribute named name, False otherwise"""
return self.data and self.data.has_key(name)
+
+ def __setattr__(self,name,value):
+ """One should use setValue() or setValues() to set values except for
+ dn and data which are special."""
+ if name != 'dn' and name != 'data' and name != 'orig_data':
+ raise KeyError, 'use setValue() or setValues()'
+ else:
+ self.__dict__[name] = value
def __getattr__(self,name):
"""If name is the name of an LDAP attribute, return the first value for that
@@ -72,9 +83,9 @@ class User:
ent.setValue('name', ('value1', 'value2', ..., 'valueN'))
Since *value is a tuple, we may have to extract a list or tuple from that
tuple as in the last two examples above"""
- if (len(value[0]) < 1):
+ if (len(value) < 1):
return
- if isinstance(value[0],list) or isinstance(value[0],tuple):
+ if (len(value) == 1):
self.data[name] = value[0]
else:
self.data[name] = value
@@ -100,6 +111,14 @@ class User:
"""Return a list of all attributes in the entry"""
return self.data.keys()
+ def origDataDict(self):
+ """Returns a dict of the original values of the user. Used for updates."""
+ result = {}
+ for k in self.orig_data.keys():
+ result[k] = self.orig_data[k]
+ result['dn'] = self.dn
+ return result
+
# def __str__(self):
# """Convert the Entry to its LDIF representation"""
# return self.__repr__()
diff -r b92cec02be12 -r a69ececded00 ipa-server/ipa-gui/ipagui/controllers.py
--- a/ipa-server/ipa-gui/ipagui/controllers.py Fri Aug 17 15:32:05 2007 -0700
+++ b/ipa-server/ipa-gui/ipagui/controllers.py Mon Aug 20 10:50:11 2007 -0700
@@ -32,25 +32,10 @@ def restrict_post():
turbogears.flash("This method only accepts posts")
raise turbogears.redirect("/")
-def to_ldap_hash(orig):
- """LDAP hashes expect all values to be a list. This method converts single
- entries to a list."""
- new={}
- for (k,v) in orig.iteritems():
- if v == None:
- continue
- if not isinstance(v, list) and k != 'dn':
- v = [v]
- new[k] = v
-
- return new
-
-def set_ldap_value(hash, key, value):
- """Converts unicode strings to normal strings
- (because LDAP is choking on unicode strings"""
+def utf8_encode(value):
if value != None:
value = value.encode('utf-8')
- hash[key] = value
+ return value
class Root(controllers.RootController):
@@ -86,11 +71,11 @@ class Root(controllers.RootController):
try:
new_user = {}
- set_ldap_value(new_user, 'uid', kw.get('uid'))
- set_ldap_value(new_user, 'givenname', kw.get('givenname'))
- set_ldap_value(new_user, 'sn', kw.get('sn'))
- set_ldap_value(new_user, 'mail', kw.get('mail'))
- set_ldap_value(new_user, 'telephonenumber', kw.get('telephonenumber'))
+ new_user['uid'] = utf8_encode(kw.get('uid'))
+ new_user['givenname'] = utf8_encode(kw.get('givenname'))
+ new_user['sn'] = utf8_encode(kw.get('sn'))
+ new_user['mail'] = utf8_encode(kw.get('mail'))
+ new_user['telephonenumber'] = utf8_encode(kw.get('telephonenumber'))
rv = client.add_user(new_user)
turbogears.flash("%s added!" % kw['uid'])
@@ -107,11 +92,11 @@ class Root(controllers.RootController):
turbogears.flash("There was a problem with the form!")
user = client.get_user(uid)
- user_hash = user.toDict()
+ user_dict = user.toDict()
# store a copy of the original user for the update later
- user_data = b64encode(dumps(user_hash))
- user_hash['user_orig'] = user_data
- return dict(form=user_edit_form, user=user_hash)
+ user_data = b64encode(dumps(user_dict))
+ user_dict['user_orig'] = user_data
+ return dict(form=user_edit_form, user=user_dict)
@expose()
def userupdate(self, **kw):
@@ -127,23 +112,21 @@ class Root(controllers.RootController):
tg_template='ipagui.templates.useredit')
try:
- orig_user = loads(b64decode(kw.get('user_orig')))
+ orig_user_dict = loads(b64decode(kw.get('user_orig')))
- new_user = dict(orig_user)
- set_ldap_value(new_user, 'givenname', kw.get('givenname'))
- set_ldap_value(new_user, 'sn', kw.get('sn'))
- set_ldap_value(new_user, 'mail', kw.get('mail'))
- set_ldap_value(new_user, 'telephonenumber', kw.get('telephonenumber'))
+ new_user = ipa.user.User(orig_user_dict)
+ new_user.setValue('givenname', utf8_encode(kw.get('givenname')))
+ new_user.setValue('sn', utf8_encode(kw.get('sn')))
+ new_user.setValue('mail', utf8_encode(kw.get('mail')))
+ new_user.setValue('telephonenumber', utf8_encode(kw.get('telephonenumber')))
#
# this is a hack until we decide on the policy for names/cn/sn/givenName
#
- set_ldap_value(new_user, 'cn',
- "%s %s" % (kw.get('givenname'), kw.get('sn')))
+ new_user.setValue('cn',
+ "%s %s" % (new_user.getValue('givenname'),
+ new_user.getValue('sn')))
- orig_user = to_ldap_hash(orig_user)
- new_user = to_ldap_hash(new_user)
-
- rv = client.update_user(orig_user, new_user)
+ rv = client.update_user(new_user)
turbogears.flash("%s updated!" % kw['uid'])
raise turbogears.redirect('/usershow', uid=kw['uid'])
except xmlrpclib.Fault, f:
diff -r b92cec02be12 -r a69ececded00 ipa-server/xmlrpc-server/funcs.py
--- a/ipa-server/xmlrpc-server/funcs.py Fri Aug 17 15:32:05 2007 -0700
+++ b/ipa-server/xmlrpc-server/funcs.py Mon Aug 20 10:50:11 2007 -0700
@@ -363,6 +363,17 @@ class IPAServer:
return users
+ def convert_scalar_values(self, orig_dict):
+ """LDAP update dicts expect all values to be a list (except for dn).
+ This method converts single entries to a list."""
+ new_dict={}
+ for (k,v) in orig_dict.iteritems():
+ if not isinstance(v, list) and k != 'dn':
+ v = [v]
+ new_dict[k] = v
+
+ return new_dict
+
def update_user (self, args, newuser=None, opts=None):
"""Update a user in LDAP"""
global _LDAPPool
@@ -384,6 +395,9 @@ class IPAServer:
if (isinstance(newuser, tuple)):
newuser = newuser[0]
+ olduser = self.convert_scalar_values(olduser)
+ newuser = self.convert_scalar_values(newuser)
+
# Should be able to get this from either the old or new user
# but just in case someone has decided to try changing it, use the
# original
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 2228 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20070820/1fe9b69e/attachment.bin>
More information about the Freeipa-devel
mailing list