Merge lp:~rlane/nova/ldap-user-modify-only into lp:~hudson-openstack/nova/trunk

Proposed by Ryan Lane
Status: Merged
Approved by: Todd Willey
Approved revision: 390
Merged at revision: 447
Proposed branch: lp:~rlane/nova/ldap-user-modify-only
Merge into: lp:~hudson-openstack/nova/trunk
Diff against target: 237 lines (+104/-36)
1 file modified
nova/auth/ldapdriver.py (+104/-36)
To merge this branch: bzr merge lp:~rlane/nova/ldap-user-modify-only
Reviewer Review Type Date Requested Status
Todd Willey (community) Approve
Vish Ishaya (community) Approve
Review via email: mp+41971@code.launchpad.net

Description of the change

This change adds better support for LDAP integration with pre-existing LDAP infrastructures. A new configuration option has been added to specify the LDAP driver should only modify/add/delete attributes for user entries.

This change also fixes crashing issues for entries that have the novaUser objectclass, but do not have accessKey, secretKey, or isAdmin attributes. The code now only identifies a user as existing if all attributes and the objectclass exists.

A couple new functions were added to check for existence of users in LDAP, even if they do not have the novaUser objectclass.

The ldap_user_modify_only configuration option added assumes that users will be managed by external means, and will not attempt to add or delete user entries.

To post a comment you must log in.
Revision history for this message
Vish Ishaya (vishvananda) wrote :

lgtm

I would love something like this for projects (and possibly roles). We have paranoid deletion for everything in the db except for this. Permanently deleting objects is bad for audit trails.

review: Approve
Revision history for this message
Todd Willey (xtoddx) :
review: Approve
Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

The attempt to merge lp:~rlane/nova/ldap-user-modify-only into lp:nova failed. Below is the output from the failed tests.

nova/auth/ldapdriver.py:141:24: W601 .has_key() is deprecated, use 'in'
                if user.has_key('secretKey'):
                       ^
    The {}.has_key() method will be removed in the future version of
    Python. Use the 'in' operation instead, like:
    d = {"a": 1, "b": 2}
    if "b" in d:
        print d["b"]
nova/auth/ldapdriver.py:147:24: W601 .has_key() is deprecated, use 'in'
                if user.has_key('accessKey'):
                       ^
    The {}.has_key() method will be removed in the future version of
    Python. Use the 'in' operation instead, like:
    d = {"a": 1, "b": 2}
    if "b" in d:
        print d["b"]
nova/auth/ldapdriver.py:153:24: W601 .has_key() is deprecated, use 'in'
                if user.has_key('isAdmin'):
                       ^
    The {}.has_key() method will be removed in the future version of
    Python. Use the 'in' operation instead, like:
    d = {"a": 1, "b": 2}
    if "b" in d:
        print d["b"]
nova/auth/ldapdriver.py:162:80: E501 line too long (83 characters)
                raise exception.NotFound("LDAP object for %s doesn't exist" % name)
                                                                               ^
    Limit all lines to a maximum of 79 characters.

    There are still many devices around that are limited to 80 character
    lines; plus, limiting windows to 80 characters makes it possible to have
    several windows side-by-side. The default wrapping on such devices looks
    ugly. Therefore, please limit all lines to a maximum of 79 characters.
    For flowing long blocks of text (docstrings or comments), limiting the
    length to 72 characters is recommended.
nova/auth/ldapdriver.py:300:20: W601 .has_key() is deprecated, use 'in'
            if user.has_key('secretKey'):
                   ^
    The {}.has_key() method will be removed in the future version of
    Python. Use the 'in' operation instead, like:
    d = {"a": 1, "b": 2}
    if "b" in d:
        print d["b"]
nova/auth/ldapdriver.py:303:20: W601 .has_key() is deprecated, use 'in'
            if user.has_key('accessKey'):
                   ^
    The {}.has_key() method will be removed in the future version of
    Python. Use the 'in' operation instead, like:
    d = {"a": 1, "b": 2}
    if "b" in d:
        print d["b"]
nova/auth/ldapdriver.py:306:20: W601 .has_key() is deprecated, use 'in'
            if user.has_key('isAdmin'):
                   ^
    The {}.has_key() method will be removed in the future version of
    Python. Use the 'in' operation instead, like:
    d = {"a": 1, "b": 2}
    if "b" in d:
        print d["b"]
nova/auth/ldapdriver.py:515:17: W601 .has_key() is deprecated, use 'in'
        if (attr.has_key('accessKey') and attr.has_key('secretKey') \
                ^
    The {}.has_key() method will be removed in the future version of
    Python. Use the 'in' operation instead, like:
    d = {"a": 1, "b": 2}
    if "b" in d:
        print d["b"]

lp:~rlane/nova/ldap-user-modify-only updated
389. By Ryan Lane <laner@controller>

pep8 fix

390. By Ryan Lane <laner@controller>

More pep8 fixes to remove deprecated functions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'nova/auth/ldapdriver.py'
2--- nova/auth/ldapdriver.py 2010-11-26 17:01:50 +0000
3+++ nova/auth/ldapdriver.py 2010-12-08 00:36:26 +0000
4@@ -40,6 +40,8 @@
5 flags.DEFINE_string('ldap_user_unit', 'Users', 'OID for Users')
6 flags.DEFINE_string('ldap_user_subtree', 'ou=Users,dc=example,dc=com',
7 'OU for Users')
8+flags.DEFINE_boolean('ldap_user_modify_only', False,
9+ 'Modify attributes for users instead of creating/deleting')
10 flags.DEFINE_string('ldap_project_subtree', 'ou=Groups,dc=example,dc=com',
11 'OU for Projects')
12 flags.DEFINE_string('role_project_subtree', 'ou=Groups,dc=example,dc=com',
13@@ -89,8 +91,7 @@
14
15 def get_user(self, uid):
16 """Retrieve user by id"""
17- attr = self.__find_object(self.__uid_to_dn(uid),
18- '(objectclass=novaUser)')
19+ attr = self.__get_ldap_user(uid)
20 return self.__to_user(attr)
21
22 def get_user_from_access_key(self, access):
23@@ -110,7 +111,12 @@
24 """Retrieve list of users"""
25 attrs = self.__find_objects(FLAGS.ldap_user_subtree,
26 '(objectclass=novaUser)')
27- return [self.__to_user(attr) for attr in attrs]
28+ users = []
29+ for attr in attrs:
30+ user = self.__to_user(attr)
31+ if user is not None:
32+ users.append(user)
33+ return users
34
35 def get_projects(self, uid=None):
36 """Retrieve list of projects"""
37@@ -125,21 +131,52 @@
38 """Create a user"""
39 if self.__user_exists(name):
40 raise exception.Duplicate("LDAP user %s already exists" % name)
41- attr = [
42- ('objectclass', ['person',
43- 'organizationalPerson',
44- 'inetOrgPerson',
45- 'novaUser']),
46- ('ou', [FLAGS.ldap_user_unit]),
47- ('uid', [name]),
48- ('sn', [name]),
49- ('cn', [name]),
50- ('secretKey', [secret_key]),
51- ('accessKey', [access_key]),
52- ('isAdmin', [str(is_admin).upper()]),
53- ]
54- self.conn.add_s(self.__uid_to_dn(name), attr)
55- return self.__to_user(dict(attr))
56+ if FLAGS.ldap_user_modify_only:
57+ if self.__ldap_user_exists(name):
58+ # Retrieve user by name
59+ user = self.__get_ldap_user(name)
60+ # Entry could be malformed, test for missing attrs.
61+ # Malformed entries are useless, replace attributes found.
62+ attr = []
63+ if 'secretKey' in user.keys():
64+ attr.append((self.ldap.MOD_REPLACE, 'secretKey', \
65+ [secret_key]))
66+ else:
67+ attr.append((self.ldap.MOD_ADD, 'secretKey', \
68+ [secret_key]))
69+ if 'accessKey' in user.keys():
70+ attr.append((self.ldap.MOD_REPLACE, 'accessKey', \
71+ [access_key]))
72+ else:
73+ attr.append((self.ldap.MOD_ADD, 'accessKey', \
74+ [access_key]))
75+ if 'isAdmin' in user.keys():
76+ attr.append((self.ldap.MOD_REPLACE, 'isAdmin', \
77+ [str(is_admin).upper()]))
78+ else:
79+ attr.append((self.ldap.MOD_ADD, 'isAdmin', \
80+ [str(is_admin).upper()]))
81+ self.conn.modify_s(self.__uid_to_dn(name), attr)
82+ return self.get_user(name)
83+ else:
84+ raise exception.NotFound("LDAP object for %s doesn't exist"
85+ % name)
86+ else:
87+ attr = [
88+ ('objectclass', ['person',
89+ 'organizationalPerson',
90+ 'inetOrgPerson',
91+ 'novaUser']),
92+ ('ou', [FLAGS.ldap_user_unit]),
93+ ('uid', [name]),
94+ ('sn', [name]),
95+ ('cn', [name]),
96+ ('secretKey', [secret_key]),
97+ ('accessKey', [access_key]),
98+ ('isAdmin', [str(is_admin).upper()]),
99+ ]
100+ self.conn.add_s(self.__uid_to_dn(name), attr)
101+ return self.__to_user(dict(attr))
102
103 def create_project(self, name, manager_uid,
104 description=None, member_uids=None):
105@@ -155,7 +192,7 @@
106 if description is None:
107 description = name
108 members = []
109- if member_uids != None:
110+ if member_uids is not None:
111 for member_uid in member_uids:
112 if not self.__user_exists(member_uid):
113 raise exception.NotFound("Project can't be created "
114@@ -256,7 +293,24 @@
115 if not self.__user_exists(uid):
116 raise exception.NotFound("User %s doesn't exist" % uid)
117 self.__remove_from_all(uid)
118- self.conn.delete_s(self.__uid_to_dn(uid))
119+ if FLAGS.ldap_user_modify_only:
120+ # Delete attributes
121+ attr = []
122+ # Retrieve user by name
123+ user = self.__get_ldap_user(uid)
124+ if 'secretKey' in user.keys():
125+ attr.append((self.ldap.MOD_DELETE, 'secretKey', \
126+ user['secretKey']))
127+ if 'accessKey' in user.keys():
128+ attr.append((self.ldap.MOD_DELETE, 'accessKey', \
129+ user['accessKey']))
130+ if 'isAdmin' in user.keys():
131+ attr.append((self.ldap.MOD_DELETE, 'isAdmin', \
132+ user['isAdmin']))
133+ self.conn.modify_s(self.__uid_to_dn(uid), attr)
134+ else:
135+ # Delete entry
136+ self.conn.delete_s(self.__uid_to_dn(uid))
137
138 def delete_project(self, project_id):
139 """Delete a project"""
140@@ -265,7 +319,7 @@
141 self.__delete_group(project_dn)
142
143 def modify_user(self, uid, access_key=None, secret_key=None, admin=None):
144- """Modify an existing project"""
145+ """Modify an existing user"""
146 if not access_key and not secret_key and admin is None:
147 return
148 attr = []
149@@ -279,11 +333,21 @@
150
151 def __user_exists(self, uid):
152 """Check if user exists"""
153- return self.get_user(uid) != None
154+ return self.get_user(uid) is not None
155+
156+ def __ldap_user_exists(self, uid):
157+ """Check if the user exists in ldap"""
158+ return self.__get_ldap_user(uid) is not None
159
160 def __project_exists(self, project_id):
161 """Check if project exists"""
162- return self.get_project(project_id) != None
163+ return self.get_project(project_id) is not None
164+
165+ def __get_ldap_user(self, uid):
166+ """Retrieve LDAP user entry by id"""
167+ attr = self.__find_object(self.__uid_to_dn(uid),
168+ '(objectclass=novaUser)')
169+ return attr
170
171 def __find_object(self, dn, query=None, scope=None):
172 """Find an object by dn and query"""
173@@ -330,12 +394,12 @@
174
175 def __group_exists(self, dn):
176 """Check if group exists"""
177- return self.__find_object(dn, '(objectclass=groupOfNames)') != None
178+ return self.__find_object(dn, '(objectclass=groupOfNames)') is not None
179
180 @staticmethod
181 def __role_to_dn(role, project_id=None):
182 """Convert role to corresponding dn"""
183- if project_id == None:
184+ if project_id is None:
185 return FLAGS.__getitem__("ldap_%s" % role).value
186 else:
187 return 'cn=%s,cn=%s,%s' % (role,
188@@ -349,7 +413,7 @@
189 raise exception.Duplicate("Group can't be created because "
190 "group %s already exists" % name)
191 members = []
192- if member_uids != None:
193+ if member_uids is not None:
194 for member_uid in member_uids:
195 if not self.__user_exists(member_uid):
196 raise exception.NotFound("Group can't be created "
197@@ -375,7 +439,7 @@
198 res = self.__find_object(group_dn,
199 '(member=%s)' % self.__uid_to_dn(uid),
200 self.ldap.SCOPE_BASE)
201- return res != None
202+ return res is not None
203
204 def __add_to_group(self, uid, group_dn):
205 """Add user to group"""
206@@ -447,18 +511,22 @@
207 @staticmethod
208 def __to_user(attr):
209 """Convert ldap attributes to User object"""
210- if attr == None:
211- return None
212- return {
213- 'id': attr['uid'][0],
214- 'name': attr['cn'][0],
215- 'access': attr['accessKey'][0],
216- 'secret': attr['secretKey'][0],
217- 'admin': (attr['isAdmin'][0] == 'TRUE')}
218+ if attr is None:
219+ return None
220+ if ('accessKey' in attr.keys() and 'secretKey' in attr.keys() \
221+ and 'isAdmin' in attr.keys()):
222+ return {
223+ 'id': attr['uid'][0],
224+ 'name': attr['cn'][0],
225+ 'access': attr['accessKey'][0],
226+ 'secret': attr['secretKey'][0],
227+ 'admin': (attr['isAdmin'][0] == 'TRUE')}
228+ else:
229+ return None
230
231 def __to_project(self, attr):
232 """Convert ldap attributes to Project object"""
233- if attr == None:
234+ if attr is None:
235 return None
236 member_dns = attr.get('member', [])
237 return {