Merge lp:~milo/linaro-patchmetrics/crowd-teams into lp:linaro-patchmetrics
- crowd-teams
- Merge into trunk
Status: | Merged |
---|---|
Merged at revision: | 379 |
Proposed branch: | lp:~milo/linaro-patchmetrics/crowd-teams |
Merge into: | lp:linaro-patchmetrics |
Diff against target: |
524 lines (+395/-19) 4 files modified
apps/patchmetrics/bin/get-linaro-membership.py (+25/-7) apps/patchmetrics/crowd.py (+238/-0) apps/patchmetrics/utils.py (+122/-7) apps/settings.py (+10/-5) |
To merge this branch: | bzr merge lp:~milo/linaro-patchmetrics/crowd-teams |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Paul Sokolovsky | Approve | ||
Linaro Automation & Validation | Pending | ||
Review via email: mp+185984@code.launchpad.net |
Commit message
Description of the change
The following MP is the starting work to move away from Launchpad and use Crowd in patchwork/
Here the Launchpad teams extraction has been reworked in order to interact with Crowd API. Launchpad integration has not been removed, and Crowd integration at the moment is commented out. When we roll out the final changes to patchwork/
What has been done:
- Introduced a Crowd class, that handles the connection to the Crowd API and that perform basic queries for the scope needed (users).
- Introduced a CrowdUser class that returns a handy Python object for a Crowd user as extracted from the Crowd API.
Two new variables in settings.py have been introduced: one is necessary to configure the Crowd integration, and it needs a user, password, url, and optional port to connect to the Crowd API. The file is a INI-style file, with a [crowd] section.
The second variable, optional, is to define a text file with whitelisted groups. Since when we retrieve a user information we get all the groups she is a member of, we might end up creating too many groups in patchwork/
There are two things still missing:
- Extracting or constructing a team display_name attribute: this is needed when creating new teams to give them a better visible name. Unfortunately Crowd does not expose that information, and we might need to use LDAP to achieve that.
- Modifying the 'sync_openid_urls' function: this function is called when syncing teams and users, and patchmetrics creates new users. The function patch the DB with a openid login URL from Launchpad. That needs to be modified to match the Linaro Login URL, but it should be done when we switch the authentication to Crowd too.
- 379. By Milo Casagrande
-
Added Crowd settings.
- 380. By Milo Casagrande
-
Merged from trunk.
Paul Sokolovsky (pfalcon) wrote : | # |
"(even rough example for Crowd API access addressed that providing layered functions for API calls)." - I mean example I provided some time ago.
Milo Casagrande (milo) wrote : | # |
On Sun, Sep 22, 2013 at 11:12 PM, Paul Sokolovsky
<email address hidden> wrote:
> Review: Approve
>
> I have few concerns with crowd.py:
>
> 1. it doesn't follow DRY, for example has "/crowd/
Addressed this, even if I think more refactoring might be needed.
> 2. I find "login_with" to be pretty confusing name for a factory method. It rather would be called something like "connect", though I don't understand why that work can't be done in old good constructor.
>
> 3.
> 264 + elif resp.status == 401 or resp.status == 403:
> 265 + raise CrowdException(
>
> 403 is rather important HTTP result code to shove it together with any other code, especially not mentioning "forbidden" explicitly in a result.
Treated 401 and 403 separately: CrowdUnauthoriz
CrowdForbiddenE
> 4. Crowd credentials storage scheme is not compatible with django-
It is not, and I hope we can reuse it in some way. I didn't want to
jump into it right away, because I would have had to go through the
same studies and tests you have done, spending more time on it.
--
Milo Casagrande | Automation Engineer
Linaro.org <www.linaro.org> │ Open source software for ARM SoCs
- 381. By Milo Casagrande
-
Addressed comments from merge proposal.
Preview Diff
1 | === modified file 'apps/patchmetrics/bin/get-linaro-membership.py' | |||
2 | --- apps/patchmetrics/bin/get-linaro-membership.py 2012-03-29 16:25:26 +0000 | |||
3 | +++ apps/patchmetrics/bin/get-linaro-membership.py 2013-09-23 07:09:44 +0000 | |||
4 | @@ -2,24 +2,27 @@ | |||
5 | 2 | 2 | ||
6 | 3 | import _pythonpath | 3 | import _pythonpath |
7 | 4 | import sys | 4 | import sys |
8 | 5 | |||
9 | 6 | from patchmetrics.crowd import Crowd | ||
10 | 5 | from django.conf import settings | 7 | from django.conf import settings |
11 | 6 | from launchpadlib.launchpad import Launchpad | 8 | from launchpadlib.launchpad import Launchpad |
12 | 7 | from launchpadlib.uris import LPNET_SERVICE_ROOT | 9 | from launchpadlib.uris import LPNET_SERVICE_ROOT |
13 | 8 | from patchwork.models import Person | ||
14 | 9 | from patchmetrics.utils import ( | 10 | from patchmetrics.utils import ( |
15 | 10 | sync_launchpad_memberships_for, | 11 | sync_launchpad_memberships_for, |
17 | 11 | ) | 12 | sync_user_memberships, |
18 | 13 | ) | ||
19 | 14 | from patchwork.models import Person | ||
20 | 12 | 15 | ||
21 | 13 | 16 | ||
22 | 14 | if __name__ == '__main__': | 17 | if __name__ == '__main__': |
24 | 15 | """Maps email addresses to Launchpad users to track Linaro's patches | 18 | """Maps email addresses to Linaro personnel to track Linaro's patches |
25 | 16 | 19 | ||
26 | 17 | Pulls all email addresses out of the database, checks to see if each | 20 | Pulls all email addresses out of the database, checks to see if each |
28 | 18 | email address is associated with a Launchpad user who is a member of | 21 | email address is associated with a Linaro Login user who is a member of |
29 | 19 | a Linaro sub-team, and if they are, add a mapping between that Person | 22 | a Linaro sub-team, and if they are, add a mapping between that Person |
30 | 20 | object and a User object (many people (email addresses) can be mapped | 23 | object and a User object (many people (email addresses) can be mapped |
33 | 21 | onto a single User (Launchpad identity)). This allows us to map patches | 24 | onto a single User (Linaro identity)). This allows us to map patches |
34 | 22 | by email address on to Launchpad users, and thus Linaro teams. | 25 | by email address on to Linaro users, and thus Linaro teams. |
35 | 23 | 26 | ||
36 | 24 | This script has been significantly re-written. Previously we pulled all | 27 | This script has been significantly re-written. Previously we pulled all |
37 | 25 | users who were members of a linaro sub-team out of Launchpad and saved | 28 | users who were members of a linaro sub-team out of Launchpad and saved |
38 | @@ -28,7 +31,6 @@ | |||
39 | 28 | because someone has emailed a patch to patches@linaro.org. For this reason | 31 | because someone has emailed a patch to patches@linaro.org. For this reason |
40 | 29 | we don't automatically pick up new Linaro engineers until they have | 32 | we don't automatically pick up new Linaro engineers until they have |
41 | 30 | submitted a patch in this way, which is a change vs the old behaviour. | 33 | submitted a patch in this way, which is a change vs the old behaviour. |
42 | 31 | |||
43 | 32 | """ | 34 | """ |
44 | 33 | if settings.LPLIB_CREDENTIALS_FILE is None: | 35 | if settings.LPLIB_CREDENTIALS_FILE is None: |
45 | 34 | print 'No LPLIB_CREDENTIALS_FILE defined in settings.py' | 36 | print 'No LPLIB_CREDENTIALS_FILE defined in settings.py' |
46 | @@ -45,4 +47,20 @@ | |||
47 | 45 | 47 | ||
48 | 46 | linaro = lp.people['linaro'] | 48 | linaro = lp.people['linaro'] |
49 | 47 | 49 | ||
50 | 50 | # TODO: crowd stuff, enable when ready. | ||
51 | 51 | # if settings.CROWD_CREDENTIALS_FILE is None: | ||
52 | 52 | # sys.stderr.write("CROWD_CREDENTIALS_FILE not defined in " | ||
53 | 53 | # "settings.py.\n") | ||
54 | 54 | # sys.exit(1) | ||
55 | 55 | |||
56 | 56 | # whitelisted_groups = [] | ||
57 | 57 | # if settings.CROWD_GROUPS_WHITELIST is None: | ||
58 | 58 | # print "No CROWD_GROUPS_WHITELIST defined, all groups will be used." | ||
59 | 59 | # else: | ||
60 | 60 | # with open(settings.CROWD_GROUPS_WHITELIST) as read_file: | ||
61 | 61 | # whitelisted_groups = [x.strip() for x in read_file if x.strip()] | ||
62 | 62 | |||
63 | 63 | # crwd = Crowd.login_with(settings.CROWD_CREDENTIALS_FILE) | ||
64 | 64 | # sync_user_memberships(input_email_addresses, crwd, whitelisted_groups) | ||
65 | 65 | |||
66 | 48 | sync_launchpad_memberships_for(input_email_addresses, linaro, lp) | 66 | sync_launchpad_memberships_for(input_email_addresses, linaro, lp) |
67 | 49 | 67 | ||
68 | === added file 'apps/patchmetrics/crowd.py' | |||
69 | --- apps/patchmetrics/crowd.py 1970-01-01 00:00:00 +0000 | |||
70 | +++ apps/patchmetrics/crowd.py 2013-09-23 07:09:44 +0000 | |||
71 | @@ -0,0 +1,238 @@ | |||
72 | 1 | # Copyright (C) 2013 Linaro | ||
73 | 2 | # | ||
74 | 3 | # Author: Milo Casagrande <milo.casagrande@linaro.org> | ||
75 | 4 | # This file is part of the Patchmetrics package. | ||
76 | 5 | # | ||
77 | 6 | # Patchmetrics is free software; you can redistribute it and/or modify | ||
78 | 7 | # it under the terms of the GNU General Public License as published by | ||
79 | 8 | # the Free Software Foundation; either version 2 of the License, or | ||
80 | 9 | # (at your option) any later version. | ||
81 | 10 | # | ||
82 | 11 | # Patchmetrics is distributed in the hope that it will be useful, | ||
83 | 12 | # but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
84 | 13 | # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
85 | 14 | # GNU General Public License for more details. | ||
86 | 15 | # | ||
87 | 16 | # You should have received a copy of the GNU General Public License | ||
88 | 17 | # along with Patchwork; if not, write to the Free Software | ||
89 | 18 | # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA | ||
90 | 19 | |||
91 | 20 | import base64 | ||
92 | 21 | import httplib | ||
93 | 22 | import urllib | ||
94 | 23 | import ConfigParser | ||
95 | 24 | import json | ||
96 | 25 | import types | ||
97 | 26 | |||
98 | 27 | |||
99 | 28 | class CrowdException(Exception): | ||
100 | 29 | """Base class for Crowd exceptions.""" | ||
101 | 30 | |||
102 | 31 | |||
103 | 32 | class CrowdNotFoundException(CrowdException): | ||
104 | 33 | """An exception for 404 status.""" | ||
105 | 34 | |||
106 | 35 | |||
107 | 36 | class CrowdForbiddenException(CrowdException): | ||
108 | 37 | """An exception for 403 status.""" | ||
109 | 38 | |||
110 | 39 | |||
111 | 40 | class CrowdUnauthorizedException(CrowdException): | ||
112 | 41 | """ An exception for 401 status.""" | ||
113 | 42 | |||
114 | 43 | |||
115 | 44 | class CrowdUser(object): | ||
116 | 45 | """An object that depicts a user from the Crowd system. | ||
117 | 46 | |||
118 | 47 | It has the following properties: | ||
119 | 48 | name (str) | ||
120 | 49 | display_name (str) | ||
121 | 50 | teams (list) | ||
122 | 51 | emails (list) | ||
123 | 52 | """ | ||
124 | 53 | |||
125 | 54 | def __init__(self): | ||
126 | 55 | self._display_name = None | ||
127 | 56 | self._emails = None | ||
128 | 57 | self._teams = None | ||
129 | 58 | self._name = None | ||
130 | 59 | |||
131 | 60 | @property | ||
132 | 61 | def emails(self): | ||
133 | 62 | return self._emails | ||
134 | 63 | |||
135 | 64 | @emails.setter | ||
136 | 65 | def emails(self, value): | ||
137 | 66 | if isinstance(value, types.StringTypes): | ||
138 | 67 | self._emails = [value] | ||
139 | 68 | else: | ||
140 | 69 | self._emails = list(value) | ||
141 | 70 | |||
142 | 71 | @property | ||
143 | 72 | def teams(self): | ||
144 | 73 | return self._teams | ||
145 | 74 | |||
146 | 75 | @teams.setter | ||
147 | 76 | def teams(self, value): | ||
148 | 77 | if isinstance(value, types.StringTypes): | ||
149 | 78 | self._teams = [value] | ||
150 | 79 | else: | ||
151 | 80 | self._teams = list(value) | ||
152 | 81 | |||
153 | 82 | @property | ||
154 | 83 | def display_name(self): | ||
155 | 84 | return self._display_name | ||
156 | 85 | |||
157 | 86 | @display_name.setter | ||
158 | 87 | def display_name(self, value): | ||
159 | 88 | self._display_name = value | ||
160 | 89 | |||
161 | 90 | @property | ||
162 | 91 | def name(self): | ||
163 | 92 | return self._name | ||
164 | 93 | |||
165 | 94 | @name.setter | ||
166 | 95 | def name(self, value): | ||
167 | 96 | self._name = value | ||
168 | 97 | |||
169 | 98 | @staticmethod | ||
170 | 99 | def _user_from_json(data): | ||
171 | 100 | user = CrowdUser() | ||
172 | 101 | user.display_name = data['display-name'] | ||
173 | 102 | user.name = data['name'] | ||
174 | 103 | user.emails = data['email'] | ||
175 | 104 | return user | ||
176 | 105 | |||
177 | 106 | @staticmethod | ||
178 | 107 | def from_json(data): | ||
179 | 108 | """Creates a CrowdUser from JSON data. | ||
180 | 109 | |||
181 | 110 | The JSON data has to contain these fields: | ||
182 | 111 | display-name | ||
183 | 112 | name | ||
184 | 113 | |||
185 | 114 | |||
186 | 115 | :param data: The JSON file to load. | ||
187 | 116 | """ | ||
188 | 117 | json_data = json.load(data) | ||
189 | 118 | return CrowdUser._user_from_json(json_data) | ||
190 | 119 | |||
191 | 120 | @staticmethod | ||
192 | 121 | def from_json_s(string): | ||
193 | 122 | """Creates a CrowdUser from a JSON string. | ||
194 | 123 | |||
195 | 124 | The JSON data has to contain these fields: | ||
196 | 125 | display-name | ||
197 | 126 | name | ||
198 | 127 | |||
199 | 128 | |||
200 | 129 | :param string: The JSON string. | ||
201 | 130 | :type str | ||
202 | 131 | """ | ||
203 | 132 | json_data = json.loads(string) | ||
204 | 133 | return CrowdUser._user_from_json(json_data) | ||
205 | 134 | |||
206 | 135 | |||
207 | 136 | class Crowd(object): | ||
208 | 137 | """A Crowd object used to perform query operations.""" | ||
209 | 138 | |||
210 | 139 | def __init__(self, usr, pwd, url, port=8443): | ||
211 | 140 | self._usr = usr | ||
212 | 141 | self._pwd = pwd | ||
213 | 142 | self._url = url | ||
214 | 143 | self._port = port | ||
215 | 144 | |||
216 | 145 | self._auth = base64.encodestring('{0}:{1}'.format(self._usr, | ||
217 | 146 | self._pwd)) | ||
218 | 147 | self._headers = { | ||
219 | 148 | "Authorization": "Basic {0}".format(self._auth), | ||
220 | 149 | "Accept": "application/json" | ||
221 | 150 | } | ||
222 | 151 | |||
223 | 152 | def get_user(self, email): | ||
224 | 153 | params = {"username": email} | ||
225 | 154 | |||
226 | 155 | resource = "/user?{0}".format(urllib.urlencode(params)) | ||
227 | 156 | return CrowdUser.from_json_s(self._get_rest_usermanagement(resource)) | ||
228 | 157 | |||
229 | 158 | def get_user_with_groups(self, email): | ||
230 | 159 | """Gets all the groups a user is member of. | ||
231 | 160 | |||
232 | 161 | :param email: The user email. | ||
233 | 162 | :return A CrowdUser object. | ||
234 | 163 | """ | ||
235 | 164 | # First get the user, if it does not exist, we skip all the operations | ||
236 | 165 | # here. | ||
237 | 166 | user = self.get_user(email) | ||
238 | 167 | |||
239 | 168 | params = {"username": email} | ||
240 | 169 | |||
241 | 170 | resource = "/user/group/nested?{0}".format( | ||
242 | 171 | urllib.urlencode(params)) | ||
243 | 172 | data = json.loads(self._get_rest_usermanagement(resource)) | ||
244 | 173 | |||
245 | 174 | teams = [] | ||
246 | 175 | if data["groups"]: | ||
247 | 176 | teams = [x["name"] for x in data["groups"]] | ||
248 | 177 | user.teams = teams | ||
249 | 178 | |||
250 | 179 | return user | ||
251 | 180 | |||
252 | 181 | def _get_rest_usermanagement(self, resource): | ||
253 | 182 | api_url = "/crowd/rest/usermanagement/1{0}".format(resource) | ||
254 | 183 | return self._get(api_url) | ||
255 | 184 | |||
256 | 185 | def _get(self, api_url): | ||
257 | 186 | """Performs a GET operation on the API URL. | ||
258 | 187 | |||
259 | 188 | :param api_url: The URL of the API to use. | ||
260 | 189 | :return The response data. | ||
261 | 190 | :raise CrowdNotFoundException if the response status is 404, | ||
262 | 191 | CrowdForbiddenException if status is 403, | ||
263 | 192 | CrowdUnauthorizedException if status is 401, and CrowdException | ||
264 | 193 | in other cases. | ||
265 | 194 | """ | ||
266 | 195 | connection = httplib.HTTPSConnection(self._url, self._port) | ||
267 | 196 | connection.request("GET", api_url, headers=self._headers) | ||
268 | 197 | resp = connection.getresponse() | ||
269 | 198 | |||
270 | 199 | if resp.status == 200: | ||
271 | 200 | return resp.read() | ||
272 | 201 | elif resp.status == 404: | ||
273 | 202 | raise CrowdNotFoundException("Resource not found") | ||
274 | 203 | elif resp.status == 401: | ||
275 | 204 | raise CrowdUnauthorizedException( | ||
276 | 205 | "Authorization not granted to fulfill the request") | ||
277 | 206 | elif resp.status == 403: | ||
278 | 207 | raise CrowdForbiddenException( | ||
279 | 208 | "Access forbidden to fulfill the request") | ||
280 | 209 | else: | ||
281 | 210 | raise CrowdException("Unknow Crowd status {0}".format(resp.status)) | ||
282 | 211 | |||
283 | 212 | @staticmethod | ||
284 | 213 | def login_with(credentials_file): | ||
285 | 214 | """Create a Crowd object with the provided credentials file.""" | ||
286 | 215 | config = ConfigParser.ConfigParser() | ||
287 | 216 | config.read(credentials_file) | ||
288 | 217 | |||
289 | 218 | crowd = None | ||
290 | 219 | if config: | ||
291 | 220 | try: | ||
292 | 221 | usr = config.get("crowd", "user") | ||
293 | 222 | pwd = config.get("crowd", "password") | ||
294 | 223 | url = config.get("crowd", "url") | ||
295 | 224 | except ConfigParser.NoOptionError: | ||
296 | 225 | raise CrowdException("Missing mandatory config parameter.") | ||
297 | 226 | |||
298 | 227 | try: | ||
299 | 228 | port = config.get("crowd", "port") | ||
300 | 229 | crowd = Crowd(usr, pwd, url, port=port) | ||
301 | 230 | except ConfigParser.NoOptionError: | ||
302 | 231 | pass | ||
303 | 232 | |||
304 | 233 | if crowd is None: | ||
305 | 234 | crowd = Crowd(usr, pwd, url) | ||
306 | 235 | |||
307 | 236 | return crowd | ||
308 | 237 | else: | ||
309 | 238 | raise CrowdException("Missing Crowd configuration values.") | ||
310 | 0 | 239 | ||
311 | === modified file 'apps/patchmetrics/utils.py' | |||
312 | --- apps/patchmetrics/utils.py 2013-04-30 09:01:32 +0000 | |||
313 | +++ apps/patchmetrics/utils.py 2013-09-23 07:09:44 +0000 | |||
314 | @@ -1,15 +1,18 @@ | |||
315 | 1 | import logging | 1 | import logging |
316 | 2 | import time | ||
317 | 3 | import urllib2 | ||
318 | 2 | 4 | ||
319 | 5 | from patchmetrics.crowd import ( | ||
320 | 6 | CrowdException, | ||
321 | 7 | CrowdNotFoundException, | ||
322 | 8 | ) | ||
323 | 3 | from django.contrib.auth.models import User | 9 | from django.contrib.auth.models import User |
324 | 4 | from django.db.models import Q | 10 | from django.db.models import Q |
325 | 5 | from django_openid_auth.models import UserOpenID | 11 | from django_openid_auth.models import UserOpenID |
326 | 12 | from lazr.restfulclient.errors import HTTPError | ||
327 | 13 | from patchmetrics.models import Team | ||
328 | 6 | from patchwork.models import Person | 14 | from patchwork.models import Person |
329 | 7 | from patchmetrics.models import Team | ||
330 | 8 | import urllib2 | ||
331 | 9 | import time | ||
332 | 10 | from xml.dom.minidom import parseString | 15 | from xml.dom.minidom import parseString |
333 | 11 | from lazr.restfulclient.errors import HTTPError | ||
334 | 12 | |||
335 | 13 | 16 | ||
336 | 14 | logging.basicConfig() | 17 | logging.basicConfig() |
337 | 15 | logger = logging.getLogger() | 18 | logger = logging.getLogger() |
338 | @@ -20,6 +23,61 @@ | |||
339 | 20 | MAX_RETRIES = 3 | 23 | MAX_RETRIES = 3 |
340 | 21 | 24 | ||
341 | 22 | 25 | ||
342 | 26 | def sync_crowd_memberships(memberships): | ||
343 | 27 | """Make sure the given memberships are represented in the database. | ||
344 | 28 | |||
345 | 29 | :param memberships: A dict mapping each team to a list of members. Each | ||
346 | 30 | member is in turn represented by a CrowdUser object. | ||
347 | 31 | For example: {Team: [CrowdUser1, CrowdUser2, ...]} | ||
348 | 32 | """ | ||
349 | 33 | created_memberships = [] | ||
350 | 34 | for team, members in memberships.iteritems(): | ||
351 | 35 | try: | ||
352 | 36 | db_team = Team.objects.get(name=team) | ||
353 | 37 | except Team.DoesNotExist: | ||
354 | 38 | # TODO: need to extract the display_name for a team. | ||
355 | 39 | logger.info( | ||
356 | 40 | 'Creating new team: {0} ({1})'.format(team, team.display_name)) | ||
357 | 41 | db_team = Team(name=team, display_name=team.display_name) | ||
358 | 42 | db_team.save() | ||
359 | 43 | |||
360 | 44 | for member in members: | ||
361 | 45 | people = [] | ||
362 | 46 | for email in member.emails: | ||
363 | 47 | try: | ||
364 | 48 | person = Person.objects.get(email=email) | ||
365 | 49 | except Person.DoesNotExist: | ||
366 | 50 | logger.info("Creating new person: {0} " | ||
367 | 51 | "({1})".format(member.name, email)) | ||
368 | 52 | person = Person(name=member.display_name, email=email) | ||
369 | 53 | person.save() | ||
370 | 54 | people.append(person) | ||
371 | 55 | |||
372 | 56 | # Get all Person entries that might represent other email | ||
373 | 57 | # addresses of this same user. | ||
374 | 58 | people.extend(Person.objects.filter(name=member.display_name)) | ||
375 | 59 | |||
376 | 60 | user = get_user(member.name, member.emails) | ||
377 | 61 | if user is None: | ||
378 | 62 | user = User.objects.create_user( | ||
379 | 63 | member.name, member.emails[0], password=None) | ||
380 | 64 | |||
381 | 65 | # Now link all the Person entries to the user account. | ||
382 | 66 | for person in people: | ||
383 | 67 | person.user = user | ||
384 | 68 | person.save() | ||
385 | 69 | |||
386 | 70 | # And finally, make sure the user is a member of the team. | ||
387 | 71 | if user not in db_team.members: | ||
388 | 72 | logger.info("Adding {0} as a member " | ||
389 | 73 | "of {%1}".format(member.name, db_team.name)) | ||
390 | 74 | membership = db_team.add_member(user) | ||
391 | 75 | membership.save() | ||
392 | 76 | created_memberships.append(membership) | ||
393 | 77 | |||
394 | 78 | return created_memberships | ||
395 | 79 | |||
396 | 80 | |||
397 | 23 | def sync_memberships(memberships): | 81 | def sync_memberships(memberships): |
398 | 24 | """Make sure the given memberships are represented in our database. | 82 | """Make sure the given memberships are represented in our database. |
399 | 25 | 83 | ||
400 | @@ -133,12 +191,66 @@ | |||
401 | 133 | if users.count() == 1: | 191 | if users.count() == 1: |
402 | 134 | return users[0] | 192 | return users[0] |
403 | 135 | elif users.count() > 1: | 193 | elif users.count() > 1: |
405 | 136 | print "Found more than one user for %s; using the first one" % emails | 194 | logger.info("Found more than one user for {0}; " |
406 | 195 | "using the first one".format(emails)) | ||
407 | 137 | return users[0] | 196 | return users[0] |
408 | 138 | else: | 197 | else: |
409 | 139 | return None | 198 | return None |
410 | 140 | 199 | ||
411 | 141 | 200 | ||
412 | 201 | def sync_user_memberships(email_addresses, crowd, whitelisted_groups=[]): | ||
413 | 202 | """If an input email matches one from Linaro Login, add them to the db. | ||
414 | 203 | |||
415 | 204 | If an email address matches a user in Linaro Login, takes the CrowdUser | ||
416 | 205 | object and passes it to sync_crowd_memberships, where CrowdUser are | ||
417 | 206 | matched with User objects (many CrowdUser can map to a single User). | ||
418 | 207 | The tail call to `sync_openid_urls` then adds OpenID URLs to each User | ||
419 | 208 | object. | ||
420 | 209 | |||
421 | 210 | :param: email_addresses: List of email addresses to look in Linaro Login. | ||
422 | 211 | :type list | ||
423 | 212 | :param crowd: The Crowd object instance to perform query to Linaro Login. | ||
424 | 213 | :type Crowd | ||
425 | 214 | :param whitelisted_groups: A list of valid groups/teams to create. | ||
426 | 215 | :type list | ||
427 | 216 | """ | ||
428 | 217 | memberships = {} | ||
429 | 218 | for email in email_addresses: | ||
430 | 219 | email = email.lower() | ||
431 | 220 | |||
432 | 221 | user = None | ||
433 | 222 | try: | ||
434 | 223 | user = crowd.get_user_with_groups(email) | ||
435 | 224 | except CrowdNotFoundException: | ||
436 | 225 | # If there is not a user matching that email addess in Linaro, | ||
437 | 226 | # move on. | ||
438 | 227 | pass | ||
439 | 228 | except CrowdException: | ||
440 | 229 | # If something else went wrong, report it. | ||
441 | 230 | logger.warning("Error while searching email address " | ||
442 | 231 | "'{0}'.".format(email)) | ||
443 | 232 | |||
444 | 233 | # No user with that email, or user is not part of any team. | ||
445 | 234 | if user is None or not user.teams: | ||
446 | 235 | continue | ||
447 | 236 | else: | ||
448 | 237 | for team in user.teams: | ||
449 | 238 | # If we have a list of valid groups, obey it. Otherwise, all | ||
450 | 239 | # groups are valid. | ||
451 | 240 | if whitelisted_groups: | ||
452 | 241 | if team in whitelisted_groups: | ||
453 | 242 | if not team in memberships: | ||
454 | 243 | memberships[team] = [] | ||
455 | 244 | memberships[team].append(user) | ||
456 | 245 | else: | ||
457 | 246 | if not team in memberships: | ||
458 | 247 | memberships[team] = [] | ||
459 | 248 | memberships[team].append(user) | ||
460 | 249 | |||
461 | 250 | sync_crowd_memberships(memberships) | ||
462 | 251 | sync_openid_urls() | ||
463 | 252 | |||
464 | 253 | |||
465 | 142 | def sync_launchpad_memberships_for(input_email_addresses, team, lp): | 254 | def sync_launchpad_memberships_for(input_email_addresses, team, lp): |
466 | 143 | """If an input email matches a LP user, add them to the database. | 255 | """If an input email matches a LP user, add them to the database. |
467 | 144 | 256 | ||
468 | @@ -161,7 +273,7 @@ | |||
469 | 161 | # https://bugs.launchpad.net/linaro-patchmetrics/+bug/967290 | 273 | # https://bugs.launchpad.net/linaro-patchmetrics/+bug/967290 |
470 | 162 | person = None # Launchpad lookup failed. | 274 | person = None # Launchpad lookup failed. |
471 | 163 | logger.warning("Launchpad rejected email lookup and returned " | 275 | logger.warning("Launchpad rejected email lookup and returned " |
473 | 164 | "HTTPError. Offending email address: %s" % email) | 276 | "HTTPError. Offending email address: %s" % email) |
474 | 165 | 277 | ||
475 | 166 | if not person: | 278 | if not person: |
476 | 167 | # No Launchpad entry for this person. | 279 | # No Launchpad entry for this person. |
477 | @@ -198,6 +310,9 @@ | |||
478 | 198 | When sync_memberships creates a User it doesn't do this, so this function | 310 | When sync_memberships creates a User it doesn't do this, so this function |
479 | 199 | just patches up the database with that extra information. | 311 | just patches up the database with that extra information. |
480 | 200 | """ | 312 | """ |
481 | 313 | # TODO | ||
482 | 314 | # Crowd openid urls are like this: | ||
483 | 315 | # https://login.linaro.org:8443/openidserver/users/email@linaro.org | ||
484 | 201 | for user in User.objects.all(): | 316 | for user in User.objects.all(): |
485 | 202 | if UserOpenID.objects.filter(user=user).count() == 0: | 317 | if UserOpenID.objects.filter(user=user).count() == 0: |
486 | 203 | try: | 318 | try: |
487 | 204 | 319 | ||
488 | === modified file 'apps/settings.py' | |||
489 | --- apps/settings.py 2012-10-18 09:55:07 +0000 | |||
490 | +++ apps/settings.py 2013-09-23 07:09:44 +0000 | |||
491 | @@ -100,7 +100,7 @@ | |||
492 | 100 | "django.core.context_processors.media", | 100 | "django.core.context_processors.media", |
493 | 101 | "patchwork.context_processors.login_url", | 101 | "patchwork.context_processors.login_url", |
494 | 102 | "patchwork.context_processors.register_url", | 102 | "patchwork.context_processors.register_url", |
496 | 103 | ) | 103 | ) |
497 | 104 | 104 | ||
498 | 105 | AUTH_PROFILE_MODULE = "patchwork.userprofile" | 105 | AUTH_PROFILE_MODULE = "patchwork.userprofile" |
499 | 106 | 106 | ||
500 | @@ -127,6 +127,12 @@ | |||
501 | 127 | # means it will use the default, which is ~/.launchpadlib. | 127 | # means it will use the default, which is ~/.launchpadlib. |
502 | 128 | LPLIB_DIR = None | 128 | LPLIB_DIR = None |
503 | 129 | 129 | ||
504 | 130 | # The file containing Crowd credentials to access Linaro Login Crowd API. | ||
505 | 131 | CROWD_CREDENTIALS_FILE = None | ||
506 | 132 | # File with a list of groups that should be created in the DB. If other groups | ||
507 | 133 | # are taken out from Crowd, and they are not here, they will be skipped. | ||
508 | 134 | CROWD_GROUPS_WHITELIST = None | ||
509 | 135 | |||
510 | 130 | # Where to store the git repos that we scan to identify which patches have | 136 | # Where to store the git repos that we scan to identify which patches have |
511 | 131 | # been committed. | 137 | # been committed. |
512 | 132 | PATCHWORK_GIT_REPOS_DIR = '/tmp' | 138 | PATCHWORK_GIT_REPOS_DIR = '/tmp' |
513 | @@ -143,8 +149,7 @@ | |||
514 | 143 | from local_settings import * | 149 | from local_settings import * |
515 | 144 | except ImportError, ex: | 150 | except ImportError, ex: |
516 | 145 | import sys | 151 | import sys |
521 | 146 | sys.stderr.write(\ | 152 | sys.stderr.write("settings.py: error importing local settings file:\n" |
522 | 147 | ("settings.py: error importing local settings file:\n" + \ | 153 | "\t%s\n" |
523 | 148 | "\t%s\n" + \ | 154 | "Do you have a local_settings.py module?\n" % str(ex)) |
520 | 149 | "Do you have a local_settings.py module?\n") % str(ex)) | ||
524 | 150 | raise | 155 | raise |
I have few concerns with crowd.py:
1. it doesn't follow DRY, for example has "/crowd/ rest/usermanage ment/1" twice (even rough example for Crowd API access addressed that providing layered functions for API calls).
2. I find "login_with" to be pretty confusing name for a factory method. It rather would be called something like "connect", though I don't understand why that work can't be done in old good constructor.
3. "Impossible to fulfill the request")
264 + elif resp.status == 401 or resp.status == 403:
265 + raise CrowdException(
403 is rather important HTTP result code to shove it together with any other code, especially not mentioning "forbidden" explicitly in a result.
4. Crowd credentials storage scheme is not compatible with django- crowd-backend which would be used for user auth support.
I'd suggest to do something about 3. before merging. Otherwise, based on the discussion in the email (i.e. confirming it was tested), this can go in, so we'll definitely need to address 4 later.