Merge lp:~benoit.pierre/sloecode/incubation into lp:sloecode
- incubation
- Merge into trunk
Status: | Needs review |
---|---|
Proposed branch: | lp:~benoit.pierre/sloecode/incubation |
Merge into: | lp:sloecode |
Diff against target: |
885 lines (+261/-74) (has conflicts) 25 files modified
development.ini (+4/-1) sloecode/bzr/display.py (+13/-4) sloecode/codehosting/server.py (+10/-0) sloecode/config/middleware.py (+1/-1) sloecode/controllers/admin/person.py (+18/-8) sloecode/controllers/admin/project.py (+2/-0) sloecode/controllers/person.py (+31/-0) sloecode/controllers/project.py (+6/-2) sloecode/controllers/xmlrpc.py (+7/-0) sloecode/lib/auth.py (+16/-9) sloecode/lib/helpers.py (+13/-2) sloecode/lib/predicates.py (+40/-19) sloecode/lib/security.py (+43/-10) sloecode/model/person.py (+2/-2) sloecode/model/project.py (+15/-3) sloecode/sshserver/launcher.py (+2/-0) sloecode/templates/admin/person-create.html (+2/-0) sloecode/templates/admin/person-update.html (+6/-0) sloecode/templates/admin/project-create.html (+6/-0) sloecode/templates/admin/project-list.html (+2/-0) sloecode/templates/admin/project-update.html (+4/-0) sloecode/templates/login.html (+1/-1) sloecode/templates/macros/nav.html (+7/-4) sloecode/templates/person-details.html (+4/-4) sloecode/templates/project-details.html (+6/-4) Text conflict in sloecode/codehosting/server.py |
To merge this branch: | bzr merge lp:~benoit.pierre/sloecode/incubation |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Thomi Richards (community) | Needs Fixing | ||
Review via email:
|
Commit message
Description of the change
Various changes. I'm creating the merge proposal since I need somewhere to make notes about the changes as I'm scanning through them.
These changes look great - I was thinking of adding similar functionality myself. There's a few places where things need to be tweaked. I'll add comments below, and we can discuss them here, or on IRC. It seems like we're not in similar timezones (I'm currently +1300), so another option may be on the sloecode-developers mailing list.

Thomi Richards (thomir-deactivatedaccount) wrote : | # |

Benoit Pierre (benoit.pierre) wrote : | # |
On Thu, 29 Sep 2011 00:45:28 -0000, Thomi Richards <email address hidden> wrote:
> Review: Needs Fixing
>
> Revision 129 & 130 are fine.
OK, I'll create a branch/merge proposal for this.
>
> Revision 131 looks fine, but I'm a bit concerned about revision 132. I
> understand that you need to add new users to the sloecode database
> once they're authenticated, but surely there's a hook in the repoze
> PAM authentication plugin that can be used for this? In fact, I think
> it can be easily done by specialising the PAMAuthenticato
> overriding the authenticate(...) method such that successful
> authentications are then inserted into the database, if they don't
> already exist. See:
>
> http://
>
> for more information.
OK.
> Also, while you're doing this, I think it makes sense to set the
> password in the database correctly - that way one can switch away from
> PAM authentication in the future and existing accounts will continue
> to work. Once you're making the change listed above, this should be
> trivial to implement.
I'm really not sure mirroring the password should be done, or if it's
even possible at all. And how do you handle password changes, where
there are not done through the sloecode web interface?
AFAIK, redmine or reviewboard don't do it when using an external
authentication mechanism (like PAM, LDAP, or ActiveDirectory).
I used PAM because it was easier to setup, but I started by looking at
the LDAP plugin, I think some of the authentication configuration could
be moved to the configuration file.
> Finally, if you're using PAM authentication, how do you change the
> admin password from it's initial, insecure setting?
This is a real problem :) Ideally, the admin account should be special:
always using the internal authentication mechanism, especially if at
some point, LDAP/PAM configuration is available through the web
interface, so you can always login to fix an invalid configuration.
> Revisions 133, 134, 135 look fine.
>
> Revision 136: the public_projects() function in
> sloecode/
> all the public projects, rather than getting all projects and
> iterating over them. You should be able to do something similar to:
>
> def public_projects():
> return Project.
>
> Unfortunately, whenever we make a database schema change we need to
> also write a database patch file that is able to alter a live database
> without deleting the current data. These are simple SQL scripts that
> make the appropriate changes, and are run as part of the debian
> package's upgrade scripts. See the existing ones in
> 'dbpatches/
> SQLite. SQLite is a bit more tricky because it doesn't let you alter
> tables much, but you should be able to adapt an existing patch file
> for your needs. I'd suggest adding the 'public' column, and setting
> all projects to private by default.
>
> Also, it would be nice to have some text explaining what a public
> project is in the template - either as a short string next to the
> 'Public' checkbox, or as...

Thomi Richards (thomir-deactivatedaccount) wrote : | # |
> On Thu, 29 Sep 2011 00:45:28 -0000, Thomi Richards <email address hidden> wrote:
> > Also, while you're doing this, I think it makes sense to set the
> > password in the database correctly - that way one can switch away from
> > PAM authentication in the future and existing accounts will continue
> > to work. Once you're making the change listed above, this should be
> > trivial to implement.
>
> I'm really not sure mirroring the password should be done, or if it's
> even possible at all. And how do you handle password changes, where
> there are not done through the sloecode web interface?
>
> AFAIK, redmine or reviewboard don't do it when using an external
> authentication mechanism (like PAM, LDAP, or ActiveDirectory).
>
> I used PAM because it was easier to setup, but I started by looking at
> the LDAP plugin, I think some of the authentication configuration could
> be moved to the configuration file.
>
Good point.
Unmerged revisions
- 143. By Benoit Pierre
-
More fine-grained control for projects.
- 142. By Benoit Pierre
-
Quick hack: change branches display name.
- 141. By Benoit Pierre
-
More fine-grained control for projects.
Read-only access for:
* everybody when project is public
* all project members if project is privateRead/write access for:
* managers and developers of the project - 140. By Benoit Pierre
-
Quick hack: change branches display name.
Use path relative to containing repository, instead of nickname.
- 139. By Benoit Pierre
-
Change logfile name to include username when launching 'bzr sc-serve'.
- 138. By Benoit Pierre
-
Quick hack to allow read-only access to all user repositories.
- 137. By Benoit Pierre
-
Fix "permission denied" trace when accessing another user repository.
- 136. By Benoit Pierre
-
Add support for public projects.
A public project will automatically be read-only accessible by all users,
without the need to manually add them as observers. - 135. By Benoit Pierre
-
Remove left-over debug trace in helpers.
- 134. By Benoit Pierre
-
Fix user creation to work when password modification is not allowed.
Even when using PAM for authentication, it can still be useful to create users:
- an administrator can manage the details of user creation.
- even if the user don't exist on the PAM side, and won't be allowed to connect
to the web interface, he may still interact with the code hosting server.
Preview Diff
1 | === modified file 'development.ini' |
2 | --- development.ini 2011-06-04 07:57:38 +0000 |
3 | +++ development.ini 2011-09-28 23:56:23 +0000 |
4 | @@ -6,7 +6,8 @@ |
5 | [DEFAULT] |
6 | debug = true |
7 | # Uncomment and replace with the address which should receive any error reports |
8 | -#email_to = you@yourdomain.com |
9 | +#email_domain = yourdomain.com |
10 | +#email_to = you@%(email_domain) |
11 | smtp_server = localhost |
12 | error_email_from = paste@localhost |
13 | |
14 | @@ -39,6 +40,8 @@ |
15 | use = egg:sloecode |
16 | full_stack = true |
17 | static_files = true |
18 | +# Uncomment to switch to PAM for user authentication. |
19 | +#use_pam = true |
20 | |
21 | cache_dir = %(here)s/data |
22 | beaker.session.key = sloecode |
23 | |
24 | === modified file 'sloecode/bzr/display.py' |
25 | --- sloecode/bzr/display.py 2011-08-15 00:56:40 +0000 |
26 | +++ sloecode/bzr/display.py 2011-09-28 23:56:23 +0000 |
27 | @@ -9,7 +9,7 @@ |
28 | from bzrlib.branch import Branch |
29 | from bzrlib.log import LogFormatter |
30 | from bzrlib.osutils import format_date |
31 | -from bzrlib.urlutils import parse_url |
32 | +from bzrlib.urlutils import parse_url, relative_url |
33 | |
34 | class BazaarRepositoryAdaptor(object): |
35 | """Wrap a bzrlib repository object and expose useful information |
36 | @@ -36,9 +36,14 @@ |
37 | """Get a branch adaptor object for a single, named branch. |
38 | """ |
39 | for b in self.branches: |
40 | - if b.get_name() == branch_name: |
41 | + if b.get_name(self.get_path()) == branch_name: |
42 | return b |
43 | |
44 | + def get_path(self): |
45 | + """Return repository path. |
46 | + """ |
47 | + return self.repo.user_url |
48 | + |
49 | def delete_branch(self, branch_name): |
50 | """ |
51 | Delete a branch from disk. This method WILL NOT BACKUP THE BRANCH, so |
52 | @@ -74,13 +79,17 @@ |
53 | """)) |
54 | self.branch = branch |
55 | |
56 | - def get_name(self): |
57 | + def get_name(self, basepath=None): |
58 | """Get the branch name. |
59 | |
60 | In fact this returns the branch nickname, since the branch name |
61 | seems to always be set to None |
62 | """ |
63 | - return html_escape(self.branch.get_config().get_nickname()) |
64 | + if basepath is None: |
65 | + name = self.branch.get_config().get_nickname() |
66 | + else: |
67 | + name = relative_url(basepath, self.branch.base) |
68 | + return html_escape(name) |
69 | |
70 | def get_log(self): |
71 | """Get a log of this branch. |
72 | |
73 | === modified file 'sloecode/codehosting/server.py' |
74 | --- sloecode/codehosting/server.py 2011-09-27 21:11:43 +0000 |
75 | +++ sloecode/codehosting/server.py 2011-09-28 23:56:23 +0000 |
76 | @@ -120,11 +120,21 @@ |
77 | unregister_transport(self.scheme, self._transportFactory) |
78 | |
79 | def translateUserPath(self, username): |
80 | + if False: |
81 | + if username != self.username and not self.is_admin: |
82 | + raise PermissionDenied( |
83 | + "You (%s) do not have access to user %s's branches." % |
84 | + (self.username, username)) |
85 | + transport = self.user_transport.clone(username) |
86 | if username != self.username and not self.is_admin: |
87 | +<<<<<<< TREE |
88 | raise PermissionDenied( |
89 | "You (%s) do not have access to user %s's branches." % |
90 | (self.username, username)) |
91 | transport = self.user_transport.clone(username) |
92 | +======= |
93 | + transport = get_readonly_transport(transport) |
94 | +>>>>>>> MERGE-SOURCE |
95 | return transport |
96 | |
97 | def translateProjectPath(self, project_name): |
98 | |
99 | === modified file 'sloecode/config/middleware.py' |
100 | --- sloecode/config/middleware.py 2010-12-20 22:16:03 +0000 |
101 | +++ sloecode/config/middleware.py 2011-09-28 23:56:23 +0000 |
102 | @@ -46,7 +46,7 @@ |
103 | app = SessionMiddleware(app, config) |
104 | |
105 | # Add out authorisation & authentication framework: |
106 | - app = add_auth(app) |
107 | + app = add_auth(app, config) |
108 | |
109 | if asbool(full_stack): |
110 | # Handle Python exceptions |
111 | |
112 | === modified file 'sloecode/controllers/admin/person.py' |
113 | --- sloecode/controllers/admin/person.py 2011-07-02 22:51:24 +0000 |
114 | +++ sloecode/controllers/admin/person.py 2011-09-28 23:56:23 +0000 |
115 | @@ -46,6 +46,9 @@ |
116 | # we use FormEncode to validate the incoming data. First, create the |
117 | # schema for the model object we are trying to validate against: |
118 | schema = PersonSchema() |
119 | + if not h.allow_password_modification(): |
120 | + del schema.fields['password'] |
121 | + del schema.fields['password_verify'] |
122 | try: |
123 | # try and validate it. The formencode.Invalid exception will be |
124 | # thrown if anything is wrong (e.g.- missing fields) |
125 | @@ -63,7 +66,8 @@ |
126 | # has given us python values, rather than raw strings: |
127 | new_person = Person() |
128 | new_person.login = form_result['login'] |
129 | - new_person.password = form_result['password'] |
130 | + if h.allow_password_modification(): |
131 | + new_person.password = form_result['password'] |
132 | new_person.name = form_result['name'] |
133 | new_person.email = form_result['email'] |
134 | for role in form_result['site_roles']: |
135 | @@ -111,9 +115,14 @@ |
136 | schema = PersonSchema() |
137 | # we don't care about the login any more: |
138 | del schema.fields['login'] |
139 | - # we need to set the password & password-verify fields to be optional: |
140 | - schema.fields['password'].not_empty = False |
141 | - schema.fields['password_verify'].not_empty = False |
142 | + |
143 | + if h.allow_password_modification(): |
144 | + # we need to set the password & password-verify fields to be optional: |
145 | + schema.fields['password'].not_empty = False |
146 | + schema.fields['password_verify'].not_empty = False |
147 | + else: |
148 | + del schema.fields['password'] |
149 | + del schema.fields['password_verify'] |
150 | |
151 | try: |
152 | form_result = schema.to_python(request.params) |
153 | @@ -125,10 +134,11 @@ |
154 | person_id = id |
155 | person = Person.get(id=person_id)[0] |
156 | |
157 | - # only update password if the user filled it in. |
158 | - pw = form_result['password'] |
159 | - if pw.strip() != "": |
160 | - person.password = pw |
161 | + if h.allow_password_modification(): |
162 | + # only update password if the user filled it in. |
163 | + pw = form_result['password'] |
164 | + if pw.strip() != "": |
165 | + person.password = pw |
166 | |
167 | person.name = form_result['name'] |
168 | person.email = form_result['email'] |
169 | |
170 | === modified file 'sloecode/controllers/admin/project.py' |
171 | --- sloecode/controllers/admin/project.py 2011-06-22 06:54:54 +0000 |
172 | +++ sloecode/controllers/admin/project.py 2011-09-28 23:56:23 +0000 |
173 | @@ -53,6 +53,7 @@ |
174 | new_project.name = form_result['name'] |
175 | new_project.displayname = form_result['displayname'] |
176 | new_project.description = form_result['description'] |
177 | + new_project.public = form_result['public'] |
178 | |
179 | Session.add(new_project) |
180 | Session.commit() |
181 | @@ -88,6 +89,7 @@ |
182 | project = Project.get(id=id)[0] |
183 | project.displayname = form_result['displayname'] |
184 | project.description = form_result['description'] |
185 | + project.public = form_result['public'] |
186 | |
187 | Session.merge(project) |
188 | Session.commit() |
189 | |
190 | === modified file 'sloecode/controllers/person.py' |
191 | --- sloecode/controllers/person.py 2011-08-15 01:43:23 +0000 |
192 | +++ sloecode/controllers/person.py 2011-09-28 23:56:23 +0000 |
193 | @@ -79,6 +79,37 @@ |
194 | return redirect(h.url(controller='person', action='index', |
195 | person_name=user.login)) |
196 | |
197 | + @ActionProtector(Any(has_site_role(role=USER_ADMIN), not_anonymous())) |
198 | + def me_or_new(self): |
199 | + """Redirect to the main user page or to account settings if new user. |
200 | + """ |
201 | + identity = request.environ['repoze.who.identity'] |
202 | + user = identity.get('user') |
203 | + if user is None: |
204 | + # If we're here, user was authenticated, so password is valid, but |
205 | + # does not exists in the DB, which can only happen when using an |
206 | + # external authentication scheme, so create it automatically. |
207 | + login = identity.get('repoze.who.userid') |
208 | + new_person = Person() |
209 | + new_person.login = login |
210 | + new_person.password = '' |
211 | + new_person.name = login |
212 | + email_domain = config.get('email_domain', None) |
213 | + if email_domain is None: |
214 | + new_person.email = '' |
215 | + else: |
216 | + new_person.email = '%s@%s' % (login, email_domain) |
217 | + Session.add(new_person) |
218 | + Session.commit() |
219 | + config['bzr.factory'].create_shared_repository_for_person(new_person.login) |
220 | + return self.render('/admin/person-update.html', |
221 | + {'person': new_person, |
222 | + 'personal_modification': True, |
223 | + 'password_modification': False}) |
224 | + else: |
225 | + # Existing user. |
226 | + return redirect(h.url(controller='person', action='me')) |
227 | + |
228 | @ActionProtector(Any(has_site_role(role=USER_ADMIN), owns_page())) |
229 | def manage_ssh_keys(self, person_name): |
230 | """Render page that displays stored SSH keys, and allows user to add |
231 | |
232 | === modified file 'sloecode/controllers/project.py' |
233 | --- sloecode/controllers/project.py 2011-08-15 01:43:23 +0000 |
234 | +++ sloecode/controllers/project.py 2011-09-28 23:56:23 +0000 |
235 | @@ -11,12 +11,11 @@ |
236 | from sloecode.model.membership import Membership |
237 | from sloecode.model.site_role import PROJECT_ADMIN |
238 | from sloecode.model.meta import Session |
239 | -from sloecode.lib.predicates import has_site_role, is_project_member |
240 | +from sloecode.lib.predicates import has_site_role, has_project_read_access, has_project_write_access |
241 | import sloecode.lib.helpers as h |
242 | |
243 | log = logging.getLogger(__name__) |
244 | |
245 | -@ControllerProtector(Any(has_site_role(PROJECT_ADMIN), is_project_member())) |
246 | class ProjectController(BaseController): |
247 | """ProjectController contains basic admin functionality for the Project |
248 | model. |
249 | @@ -24,6 +23,7 @@ |
250 | This includes basic CRUD information. |
251 | """ |
252 | |
253 | + @ControllerProtector(Any(has_site_role(PROJECT_ADMIN), has_project_read_access())) |
254 | def index(self, project_name): |
255 | """Render project details page. |
256 | """ |
257 | @@ -46,6 +46,7 @@ |
258 | 'user_role': user_role, |
259 | 'repo': repo}) |
260 | |
261 | + @ControllerProtector(Any(has_site_role(PROJECT_ADMIN), has_project_read_access())) |
262 | def branch_log(self, project_name): |
263 | projects = Project.get(name=project_name) |
264 | existing_project = projects[0] if projects else None |
265 | @@ -60,6 +61,7 @@ |
266 | {'project': existing_project, |
267 | 'branch': branch}) |
268 | |
269 | + @ControllerProtector(Any(has_site_role(PROJECT_ADMIN), has_project_write_access())) |
270 | def delete_branch(self, project_name): |
271 | """Delete a branch on disk. |
272 | """ |
273 | @@ -73,6 +75,7 @@ |
274 | return redirect(h.url(controller='project', action='index', |
275 | project_name=project_name)) |
276 | |
277 | + @ControllerProtector(Any(has_site_role(PROJECT_ADMIN), has_project_write_access())) |
278 | def manage_users(self, project_name): |
279 | """ |
280 | Add/Remove users from this project. |
281 | @@ -97,6 +100,7 @@ |
282 | {'all_people': all_people, |
283 | 'project_name': project_name}) |
284 | |
285 | + @ControllerProtector(Any(has_site_role(PROJECT_ADMIN), has_project_write_access())) |
286 | def process_manage_users(self, project_name): |
287 | """ |
288 | Process the change in the users-in-projects |
289 | |
290 | === modified file 'sloecode/controllers/xmlrpc.py' |
291 | --- sloecode/controllers/xmlrpc.py 2011-03-25 08:45:06 +0000 |
292 | +++ sloecode/controllers/xmlrpc.py 2011-09-28 23:56:23 +0000 |
293 | @@ -5,6 +5,7 @@ |
294 | |
295 | from pylons.controllers.xmlrpc import XMLRPCController |
296 | from sloecode.model import Person, Project |
297 | +from sloecode.model.project import public_projects |
298 | from sloecode.model.site_role import PROJECT_ADMIN |
299 | from sloecode.model.membership import PR_DEVELOPER, PR_MANAGER |
300 | |
301 | @@ -79,6 +80,12 @@ |
302 | writable_projects.append(mem.project.name) |
303 | else: |
304 | readable_projects.append(mem.project.name) |
305 | + for project in public_projects(): |
306 | + if project.name in writable_projects: |
307 | + continue |
308 | + if project.name in readable_projects: |
309 | + continue |
310 | + readable_projects.append(project.name) |
311 | return [isAdmin, readable_projects, writable_projects] |
312 | |
313 | getAccessPermissionsForUser.signature = [['array', 'string']] |
314 | |
315 | === modified file 'sloecode/lib/auth.py' |
316 | --- sloecode/lib/auth.py 2011-03-25 04:58:16 +0000 |
317 | +++ sloecode/lib/auth.py 2011-09-28 23:56:23 +0000 |
318 | @@ -2,7 +2,6 @@ |
319 | """ |
320 | |
321 | from repoze.what.middleware import setup_auth |
322 | -from repoze.who.plugins.sa import SQLAlchemyAuthenticatorPlugin |
323 | from repoze.who.plugins.sa import SQLAlchemyUserMDPlugin |
324 | from repoze.who.plugins.friendlyform import FriendlyFormPlugin |
325 | from repoze.who.plugins.auth_tkt import AuthTktCookiePlugin |
326 | @@ -11,7 +10,7 @@ |
327 | from sloecode.model import Person |
328 | |
329 | |
330 | -def add_auth(app): |
331 | +def add_auth(app, config): |
332 | """ |
333 | Add authentication and authorization middleware to the ``app``. |
334 | |
335 | @@ -26,12 +25,20 @@ |
336 | |
337 | """ |
338 | |
339 | - # we use the SQLAlchemy repoze.who plugin for authentication: |
340 | - authenticator = SQLAlchemyAuthenticatorPlugin(Person, Session) |
341 | - # override the default names for the login column, and the verification |
342 | - # method: |
343 | - authenticator.translations['user_name'] = 'login' |
344 | - authenticator.translations['validate_password'] = 'verify_password' |
345 | + use_pam = config.get('use_pam', False) |
346 | + if use_pam: |
347 | + from repoze.who.plugins.pam import PamAuthenticatorPlugin |
348 | + authenticator = PamAuthenticatorPlugin() |
349 | + authenticator_name = 'pam_auth' |
350 | + else: |
351 | + # we use the SQLAlchemy repoze.who plugin for authentication: |
352 | + from repoze.who.plugins.sa import SQLAlchemyAuthenticatorPlugin |
353 | + authenticator = SQLAlchemyAuthenticatorPlugin(Person, Session) |
354 | + authenticator_name = 'sa_auth' |
355 | + # override the default names for the login column, and the verification |
356 | + # method: |
357 | + authenticator.translations['user_name'] = 'login' |
358 | + authenticator.translations['validate_password'] = 'verify_password' |
359 | |
360 | # We also want to use the SA metadata providor: |
361 | mdprovidor = SQLAlchemyUserMDPlugin(Person, Session) |
362 | @@ -50,7 +57,7 @@ |
363 | auth_tkt = AuthTktCookiePlugin('S3cr3tStr1ng') |
364 | |
365 | identifiers = [('form', id_form),('auth_tkt', auth_tkt)] |
366 | - authenticators = [('sa_auth', authenticator)] |
367 | + authenticators = [(authenticator_name, authenticator)] |
368 | challengers = [('form', id_form)] |
369 | mdproviders = [('sa_md', mdprovidor)] |
370 | |
371 | |
372 | === modified file 'sloecode/lib/helpers.py' |
373 | --- sloecode/lib/helpers.py 2011-08-11 08:10:00 +0000 |
374 | +++ sloecode/lib/helpers.py 2011-09-28 23:56:23 +0000 |
375 | @@ -18,11 +18,12 @@ |
376 | # to the generated URL. |
377 | # |
378 | # Bottom line: ALL generated URLs MUST be wrapped in a call to url(): |
379 | -from pylons import url |
380 | +from pylons import url, config |
381 | |
382 | # Roles information |
383 | from sloecode.model.membership import PROJECT_ROLES |
384 | from sloecode.model.site_role import SITE_ROLES |
385 | +from sloecode.model.project import public_projects |
386 | |
387 | # a simple link_to implementation that handles a confirm kwarg that prints |
388 | # very simple javascript asking the user if they really want to continue. |
389 | @@ -56,7 +57,7 @@ |
390 | |
391 | # import helper functions that let templates determine if the user has |
392 | # certain permissions: |
393 | -from sloecode.lib.security import has_site_role, has_project_role |
394 | +from sloecode.lib.security import has_site_role, has_project_role, has_project_access |
395 | # import site role names as well: |
396 | from sloecode.model.site_role import USER_ADMIN, PROJECT_ADMIN |
397 | |
398 | @@ -76,3 +77,13 @@ |
399 | |
400 | link = url('/help/' + target) |
401 | return link_to(label, link, class_='help_link') |
402 | + |
403 | +def allow_password_modification(): |
404 | + # No password modification if PAM is used for authentication. |
405 | + use_pam = config.get('use_pam', False) |
406 | + return not use_pam |
407 | + |
408 | +def allow_user_creation(): |
409 | + # Don't allow user creation when using PAM for authentication. |
410 | + use_pam = config.get('use_pam', False) |
411 | + return not use_pam |
412 | |
413 | === modified file 'sloecode/lib/predicates.py' |
414 | --- sloecode/lib/predicates.py 2011-02-12 08:06:36 +0000 |
415 | +++ sloecode/lib/predicates.py 2011-09-28 23:56:23 +0000 |
416 | @@ -4,6 +4,7 @@ |
417 | from repoze.what.predicates import Predicate |
418 | |
419 | import sloecode.lib.security as security |
420 | +from sloecode.model.project import Project |
421 | |
422 | class has_site_role(Predicate): |
423 | """Checks whether the logged in user has the desired site role. |
424 | @@ -57,22 +58,42 @@ |
425 | except KeyError: |
426 | self.unmet() |
427 | |
428 | -class is_project_member(Predicate): |
429 | - """Checks whether the logged in user is a member of the project resource |
430 | - being requested. |
431 | - """ |
432 | - |
433 | - message = "You must be a member of a project in order to view it's page." |
434 | - |
435 | - def evaluate(self, environ, credentials): |
436 | - try: |
437 | - user = environ['repoze.who.identity'].get('user') |
438 | - args = self.parse_variables(environ) |
439 | - named_args = args['named_args'] |
440 | - project_name = named_args['project_name'] |
441 | - for membership in user.projects: |
442 | - if membership.project.name == project_name: |
443 | - return |
444 | - self.unmet() |
445 | - except KeyError: |
446 | - self.unmet() |
447 | +class has_project_read_access(Predicate): |
448 | + """Checks whether the logged in user as read-only access to the project resource |
449 | + being requested. |
450 | + """ |
451 | + |
452 | + message = "You must be a member of a project or it must be public in order to view it's page." |
453 | + |
454 | + def evaluate(self, environ, credentials): |
455 | + try: |
456 | + user = environ['repoze.who.identity'].get('user') |
457 | + args = self.parse_variables(environ) |
458 | + named_args = args['named_args'] |
459 | + project_name = named_args['project_name'] |
460 | + if security.has_project_access(project_name, user, read_only=True): |
461 | + return |
462 | + self.unmet() |
463 | + except KeyError: |
464 | + self.unmet() |
465 | + |
466 | + |
467 | +class has_project_write_access(Predicate): |
468 | + """Checks whether the logged in user as read/write access to the project resource |
469 | + being requested. |
470 | + """ |
471 | + |
472 | + message = "You must be a developper/manager of a project in order to have write access." |
473 | + |
474 | + def evaluate(self, environ, credentials): |
475 | + try: |
476 | + user = environ['repoze.who.identity'].get('user') |
477 | + args = self.parse_variables(environ) |
478 | + named_args = args['named_args'] |
479 | + project_name = named_args['project_name'] |
480 | + if security.has_project_access(project_name, user, read_only=False): |
481 | + return |
482 | + self.unmet() |
483 | + except KeyError: |
484 | + self.unmet() |
485 | + |
486 | |
487 | === modified file 'sloecode/lib/security.py' |
488 | --- sloecode/lib/security.py 2011-01-22 23:30:57 +0000 |
489 | +++ sloecode/lib/security.py 2011-09-28 23:56:23 +0000 |
490 | @@ -5,6 +5,7 @@ |
491 | from pylons import request |
492 | from sloecode.model import Person, Membership, Project |
493 | from sloecode.model.meta import Session |
494 | +from sloecode.model.membership import PR_MANAGER, PR_DEVELOPER |
495 | |
496 | |
497 | def has_site_role(role_name, user=None): |
498 | @@ -27,11 +28,7 @@ |
499 | return False |
500 | |
501 | |
502 | -def has_project_role(project, role_name, user=None): |
503 | - """Determines whether the currently logged in user has the specified role |
504 | - for a given project. The project can either be the project name (as a |
505 | - string or unicode string), a Project object, or the project id (int). |
506 | - """ |
507 | +def _get_project_object(project): |
508 | prj_obj = None |
509 | if isinstance(project, Project): |
510 | prj_obj = project |
511 | @@ -40,17 +37,27 @@ |
512 | if len(projects): |
513 | prj_obj = projects[0] |
514 | else: |
515 | - # that ID does not exist. return false: |
516 | - return False |
517 | + # that ID does not exist. |
518 | + return None |
519 | elif isinstance(project, basestring): |
520 | projects = Project.get(name=project) |
521 | if len(projects): |
522 | prj_obj = projects[0] |
523 | else: |
524 | - # that name does not exist. return false: |
525 | - return False |
526 | + # that name does not exist. |
527 | + return None |
528 | else: |
529 | raise TypeError("project must be Project,str,unicode, or int type.") |
530 | + return prj_obj |
531 | + |
532 | +def has_project_role(project, role_name, user=None): |
533 | + """Determines whether the currently logged in user has the specified role |
534 | + for a given project. The project can either be the project name (as a |
535 | + string or unicode string), a Project object, or the project id (int). |
536 | + """ |
537 | + prj_obj = _get_project_object(project) |
538 | + if prj_obj is None: |
539 | + return False |
540 | try: |
541 | if not user: |
542 | e = request.environ |
543 | @@ -61,4 +68,30 @@ |
544 | # any results? |
545 | return q.count() > 0 |
546 | except KeyError: |
547 | - return False |
548 | \ No newline at end of file |
549 | + return False |
550 | + |
551 | +def has_project_access(project, user=None, read_only=True): |
552 | + """Checks whether user as read access to the project resource.""" |
553 | + prj_obj = _get_project_object(project) |
554 | + if prj_obj is None: |
555 | + return False |
556 | + if read_only and prj_obj.public: |
557 | + # Read only access allowed for everybody if project is public. |
558 | + return True |
559 | + try: |
560 | + if not user: |
561 | + e = request.environ |
562 | + user = e['repoze.who.identity'].get('user') |
563 | + except KeyError: |
564 | + return False |
565 | + for membership in user.projects: |
566 | + if membership.project.name == project.name: |
567 | + if read_only: |
568 | + # Read only access allowed for all project roles. |
569 | + return True |
570 | + if membership.role in [PR_MANAGER, PR_DEVELOPER]: |
571 | + # Only allow write access for managers and developpers. |
572 | + return True |
573 | + return False |
574 | + return False |
575 | + |
576 | |
577 | === modified file 'sloecode/model/person.py' |
578 | --- sloecode/model/person.py 2011-07-22 04:20:11 +0000 |
579 | +++ sloecode/model/person.py 2011-09-28 23:56:23 +0000 |
580 | @@ -78,7 +78,7 @@ |
581 | """A simple formencode validator that checks that Person logins are unique. |
582 | """ |
583 | |
584 | - login_regex = re.compile(r'^[a-z0-9]{3,24}$') |
585 | + login_regex = re.compile(r'^[a-z0-9][a-z0-9_.-]{2,23}$') |
586 | |
587 | def _to_python(self, value, state): |
588 | """Try to validate form data. |
589 | @@ -88,7 +88,7 @@ |
590 | 'That login already exists.', value, state) |
591 | |
592 | if not self.login_regex.match(value): |
593 | - raise formencode.Invalid('Logins must be lower-case letters or numbers only.', value, state) |
594 | + raise formencode.Invalid('Logins must be lower-case letters, numbers, or _-. only.', value, state) |
595 | |
596 | return value |
597 | |
598 | |
599 | === modified file 'sloecode/model/project.py' |
600 | --- sloecode/model/project.py 2011-07-02 07:13:32 +0000 |
601 | +++ sloecode/model/project.py 2011-09-28 23:56:23 +0000 |
602 | @@ -6,7 +6,7 @@ |
603 | |
604 | from sqlalchemy import Column |
605 | from sqlalchemy.orm import relationship |
606 | -from sqlalchemy.types import Integer, String, Text |
607 | +from sqlalchemy.types import Boolean, Integer, String, Text |
608 | |
609 | |
610 | from sloecode.model.meta import Base, QueryMixin |
611 | @@ -22,6 +22,7 @@ |
612 | |
613 | id = Column(Integer, primary_key=True) |
614 | name = Column(String(24), unique=True) |
615 | + public = Column(Boolean) |
616 | displayname = Column(String(32)) |
617 | description = Column(Text()) |
618 | |
619 | @@ -32,7 +33,7 @@ |
620 | class UniqueProject(formencode.FancyValidator): |
621 | "A simple formencode validator that checks Project names are unique." |
622 | |
623 | - project_regex = re.compile(r'^[a-z0-9]{3,24}$') |
624 | + project_regex = re.compile(r'^[a-z0-9][a-z0-9_.-]{2,23}$') |
625 | |
626 | def _to_python(self, value, state): |
627 | """Validate form-data. |
628 | @@ -44,7 +45,7 @@ |
629 | |
630 | if not self.project_regex.match(value): |
631 | raise formencode.Invalid( |
632 | - 'A project name must be lower case letters and numbers, and be 3-24 characters long.', |
633 | + 'A project name must be lower case letters, numbers, or _-., and be 3-24 characters long.', |
634 | value, state) |
635 | |
636 | return value |
637 | @@ -57,5 +58,16 @@ |
638 | filter_extra_fields = True |
639 | |
640 | name = UniqueProject(not_empty=True) |
641 | + public = formencode.validators.Bool() |
642 | displayname = formencode.validators.String(not_empty=True, max=32) |
643 | description = formencode.validators.String(not_empty=False) |
644 | + |
645 | + |
646 | +def public_projects(): |
647 | + """ Return the list of public projects. """ |
648 | + projects = [] |
649 | + for project in Project.get(): |
650 | + if project.public: |
651 | + projects.append(project) |
652 | + return projects |
653 | + |
654 | |
655 | === modified file 'sloecode/sshserver/launcher.py' |
656 | --- sloecode/sshserver/launcher.py 2011-05-07 00:10:27 +0000 |
657 | +++ sloecode/sshserver/launcher.py 2011-09-28 23:56:23 +0000 |
658 | @@ -204,6 +204,8 @@ |
659 | environment = dict(os.environ) |
660 | environment['BZR_EMAIL'] = '%s@%s' % (avatar.username, self.hostname) |
661 | environment['BZR_PLUGIN_PATH'] = get_BZR_PLUGIN_PATH_for_subprocess() |
662 | + environment['BZR_LOG'] = '%s/.bzr.log-%s' % (environment['HOME'], |
663 | + avatar.username) |
664 | |
665 | return RestrictedExecOnlySession( |
666 | avatar, reactor, allowed_command, command, environment=environment) |
667 | |
668 | === modified file 'sloecode/templates/admin/person-create.html' |
669 | --- sloecode/templates/admin/person-create.html 2011-06-22 06:54:54 +0000 |
670 | +++ sloecode/templates/admin/person-create.html 2011-09-28 23:56:23 +0000 |
671 | @@ -23,6 +23,7 @@ |
672 | <td>Login:</td> |
673 | <td>{{ mt.text('login') }}</td> |
674 | </tr> |
675 | + {% if h.allow_password_modification() %} |
676 | <tr> |
677 | <td>Password:</td> |
678 | <td>{{ mt.password('password') }}</td> |
679 | @@ -31,6 +32,7 @@ |
680 | <td>Verify Password:</td> |
681 | <td>{{ mt.password('password_verify') }}</td> |
682 | </tr> |
683 | + {% endif %} |
684 | <tr> |
685 | <td>Name:</td> |
686 | <td>{{ mt.text('name') }}</td> |
687 | |
688 | === modified file 'sloecode/templates/admin/person-update.html' |
689 | --- sloecode/templates/admin/person-update.html 2011-07-02 22:51:24 +0000 |
690 | +++ sloecode/templates/admin/person-update.html 2011-09-28 23:56:23 +0000 |
691 | @@ -25,6 +25,7 @@ |
692 | <td>Username:</td> |
693 | <td>{{ person.login }}</td> |
694 | </tr> |
695 | + {% if h.allow_password_modification() %} |
696 | <tr> |
697 | <td>Password:</td> |
698 | <td>{{ h.password('password') }}</td> |
699 | @@ -33,6 +34,7 @@ |
700 | <td>Confirm Password:</td> |
701 | <td>{{ h.password('password_verify') }}</td> |
702 | </tr> |
703 | + {% endif %} |
704 | <tr> |
705 | <td>Name:</td> |
706 | <td>{{ mt.text('name') }}</td> |
707 | @@ -75,6 +77,10 @@ |
708 | </div> |
709 | </div> |
710 | <script type="text/javascript" language="JavaScript"> |
711 | +{% if h.allow_password_modification() %} |
712 | document.forms['person_update_form'].elements['password'].focus(); |
713 | +{% else %} |
714 | + document.forms['person_update_form'].elements['name'].focus(); |
715 | +{% endif %} |
716 | </script> |
717 | {% endblock %} |
718 | |
719 | === modified file 'sloecode/templates/admin/project-create.html' |
720 | --- sloecode/templates/admin/project-create.html 2011-06-22 06:54:54 +0000 |
721 | +++ sloecode/templates/admin/project-create.html 2011-09-28 23:56:23 +0000 |
722 | @@ -37,6 +37,12 @@ |
723 | <td>{{ mt.textarea('description', cols=45, rows=5) }}</td> |
724 | </tr> |
725 | <tr> |
726 | + <td>Public</td> |
727 | + <td> |
728 | + {{mt.checkbox('public')}} |
729 | + </td> |
730 | + </tr> |
731 | + <tr> |
732 | <td colspan="2"> |
733 | {{h.submit('update', 'Create Project')}} |
734 | </td> |
735 | |
736 | === modified file 'sloecode/templates/admin/project-list.html' |
737 | --- sloecode/templates/admin/project-list.html 2011-06-22 06:54:54 +0000 |
738 | +++ sloecode/templates/admin/project-list.html 2011-09-28 23:56:23 +0000 |
739 | @@ -21,6 +21,7 @@ |
740 | <th>Name</th> |
741 | <th>Display Name</th> |
742 | <th>Description</th> |
743 | + <th>Public</th> |
744 | <th>Actions</th> |
745 | </tr> |
746 | {% set row_class = cycler('odd', 'even') %} |
747 | @@ -31,6 +32,7 @@ |
748 | class='project') }}</td> |
749 | <td>{{project.displayname|e}}</td> |
750 | <td>{{project.description|e}}</td> |
751 | + <td>{% if project.public %}Yes{% else %}No{% endif %}</td> |
752 | <td>{{h.link_to('Edit', |
753 | h.url(controller='admin/project', |
754 | action='update', |
755 | |
756 | === modified file 'sloecode/templates/admin/project-update.html' |
757 | --- sloecode/templates/admin/project-update.html 2011-06-22 06:54:54 +0000 |
758 | +++ sloecode/templates/admin/project-update.html 2011-09-28 23:56:23 +0000 |
759 | @@ -18,6 +18,10 @@ |
760 | <td>{{ mt.textarea('description') }}</td> |
761 | </tr> |
762 | <tr> |
763 | + <td>Public</td> |
764 | + <td>{{ mt.checkbox('public') }}</td> |
765 | + </tr> |
766 | + <tr> |
767 | <td colspan="2"> |
768 | {{h.submit('update', 'Update Project')}} |
769 | </td> |
770 | |
771 | === modified file 'sloecode/templates/login.html' |
772 | --- sloecode/templates/login.html 2011-06-22 06:54:54 +0000 |
773 | +++ sloecode/templates/login.html 2011-09-28 23:56:23 +0000 |
774 | @@ -29,7 +29,7 @@ |
775 | <td colspan="2"> {{ h.submit('login_btn', 'Login') }} </td> |
776 | </tr> |
777 | </table> |
778 | - {{ h.hidden('came_from', h.url(controller='person', action='me')) }} |
779 | + {{ h.hidden('came_from', h.url(controller='person', action='me_or_new')) }} |
780 | {{ h.end_form() }} |
781 | </div> |
782 | </div> |
783 | |
784 | === modified file 'sloecode/templates/macros/nav.html' |
785 | --- sloecode/templates/macros/nav.html 2011-07-02 22:51:24 +0000 |
786 | +++ sloecode/templates/macros/nav.html 2011-09-28 23:56:23 +0000 |
787 | @@ -31,7 +31,8 @@ |
788 | class='yui3-menuitem-content') }} |
789 | </li> |
790 | |
791 | - {% if user_details.projects|length %} |
792 | + {% set projects = h.public_projects() + user_details.projects %} |
793 | + {% if projects|length %} |
794 | <li> |
795 | <a class="yui3-menu-label" href="#proj_list"> |
796 | <em>Projects</em> |
797 | @@ -41,11 +42,11 @@ |
798 | <div class="yui3-menu-content"> |
799 | <ul> |
800 | {# List of projects that this user is in #} |
801 | - {% for membership in user_details.projects %} |
802 | + {% for project in projects %} |
803 | <li class="yui3-menuitem"> |
804 | - {{ h.link_to(membership.project.name, |
805 | + {{ h.link_to(project.name, |
806 | h.url(controller='project', |
807 | - project_name=membership.project.name), |
808 | + project_name=project.name), |
809 | class='yui3-menuitem-content' |
810 | ) }} |
811 | </li> |
812 | @@ -81,12 +82,14 @@ |
813 | class='yui3-menuitem-content') |
814 | }} |
815 | </li> |
816 | + {% if h.allow_user_creation() %} |
817 | <li class="yui3-menuitem"> |
818 | {{ h.link_to('Create New', |
819 | h.url(controller='admin/person', action='create'), |
820 | class='yui3-menuitem-content') |
821 | }} |
822 | </li> |
823 | + {% endif %} |
824 | </ul> |
825 | </div> |
826 | </div> |
827 | |
828 | === modified file 'sloecode/templates/person-details.html' |
829 | --- sloecode/templates/person-details.html 2011-08-15 00:56:40 +0000 |
830 | +++ sloecode/templates/person-details.html 2011-09-28 23:56:23 +0000 |
831 | @@ -65,17 +65,17 @@ |
832 | {% for branch in branches %} |
833 | <tr> |
834 | <td>{{h.image(h.url('/bzr_icon.png'), 'Bazaar Branch')}}</td> |
835 | - <td>{{branch.get_name()}}</td> |
836 | + <td>{{branch.get_name(repo.get_path())}}</td> |
837 | <td>{{branch.get_last_revision_number()}} revisions</td> |
838 | <td>{{h.link_to(h.image(h.url('/application-list.png'), 'Branch Log'), |
839 | h.url(controller='person', person_name=person.login, |
840 | - action='branch_log', branch_name=branch.get_name()), |
841 | + action='branch_log', branch_name=branch.get_name(repo.get_path())), |
842 | title="Branch Log")}} |
843 | {{h.link_to(h.image(h.url('/bin-exclamation.png'), 'Delete Branch'), |
844 | h.url(controller='person', person_name=person.login, |
845 | - action='delete_branch', branch_name=branch.get_name()), |
846 | + action='delete_branch', branch_name=branch.get_name(repo.get_path())), |
847 | title="Delete Branch", |
848 | - confirm="Are you sure you want to delete the '" + branch.get_name() |
849 | + confirm="Are you sure you want to delete the '" + branch.get_name(repo.get_path()) |
850 | + "' branch? Branch deletion is permanent!")}} |
851 | </td> |
852 | </tr> |
853 | |
854 | === modified file 'sloecode/templates/project-details.html' |
855 | --- sloecode/templates/project-details.html 2011-08-15 01:43:23 +0000 |
856 | +++ sloecode/templates/project-details.html 2011-09-28 23:56:23 +0000 |
857 | @@ -40,7 +40,7 @@ |
858 | {{h.image(h.url('/bzr_icon.png'), 'Bazaar Branch')}} |
859 | </td> |
860 | <td> |
861 | - {{branch.get_name()}} |
862 | + {{branch.get_name(repo.get_path())}} |
863 | </td> |
864 | <td> |
865 | {{branch.get_last_revision_number()}} |
866 | @@ -48,14 +48,16 @@ |
867 | <td> |
868 | {{h.link_to(h.image(h.url('/application-list.png'), 'Branch Log'), |
869 | h.url(controller='project', project_name=project.name, |
870 | - action='branch_log', branch_name=branch.get_name()), |
871 | + action='branch_log', branch_name=branch.get_name(repo.get_path())), |
872 | title="Branch Log")}} |
873 | + {% if h.has_project_access(project, read_only=False) %} |
874 | {{h.link_to(h.image(h.url('/bin-exclamation.png'), 'Delete Branch'), |
875 | h.url(controller='project', project_name=project.name, |
876 | - action='delete_branch', branch_name=branch.get_name()), |
877 | + action='delete_branch', branch_name=branch.get_name(repo.get_path())), |
878 | title="Delete Branch", |
879 | - confirm="Are you sure you want to delete the '" + branch.get_name() |
880 | + confirm="Are you sure you want to delete the '" + branch.get_name(repo.get_path()) |
881 | + "' branch? Branch deletion is permanent!")}} |
882 | + {% endif %} |
883 | </td> |
884 | </tr> |
885 | {% endfor %} |
Revision 129 & 130 are fine.
Revision 131 looks fine, but I'm a bit concerned about revision 132. I understand that you need to add new users to the sloecode database once they're authenticated, but surely there's a hook in the repoze PAM authentication plugin that can be used for this? In fact, I think it can be easily done by specialising the PAMAuthenticato rPlugin and overriding the authenticate(...) method such that successful authentications are then inserted into the database, if they don't already exist. See:
http:// docs.repoze. org/who/ 1.0/narr. html#writing- an-authenticato r-plugin
for more information. Also, while you're doing this, I think it makes sense to set the password in the database correctly - that way one can switch away from PAM authentication in the future and existing accounts will continue to work. Once you're making the change listed above, this should be trivial to implement.
Revisions 133, 134, 135 look fine.
Revision 136: the public_projects() function in sloecode/ model/project. py should really use a SQLAlchemy query to get all the public projects, rather than getting all projects and iterating over them. You should be able to do something similar to:
def public_projects(): get(public= True)
return Project.
Unfortunately, whenever we make a database schema change we need to also write a database patch file that is able to alter a live database without deleting the current data. These are simple SQL scripts that make the appropriate changes, and are run as part of the debian package's upgrade scripts. See the existing ones in 'dbpatches/ patches/ */' - you need to write one for mysql and one for SQLite. SQLite is a bit more tricky because it doesn't let you alter tables much, but you should be able to adapt an existing patch file for your needs. I'd suggest adding the 'public' column, and setting all projects to private by default.
Also, it would be nice to have some text explaining what a public project is in the template - either as a short string next to the 'Public' checkbox, or as a help page. Something that explains that public projects are readable by anyone with access to the sloecode server, but write access is still restricted to developers and managers. I also wonder whether we should not allow the addition of 'Observers' to a public project - then again, that UI needs to be re-done anyway, so maybe we'll deal with that later.
Finally, this isn't a bug that you've introduced, but in the nav.html file, projects should be listed alphabetically, in a case-insensitive fashion. If you're able to make that change without too much trouble that would be superb!
Rev 137 is great, and has already been merged with trunk.
Rev 138 needs more thought. I'd like users to have some control over whether their branches are public or private.... or maybe this is a config setting?
Revision 139 looks OK, but maybe thumper can comment on this?
Revision 140 - I'd like some more information on the change - it looks potentially good, but I can't see any change in output on my test system... In any case we'd need to get rid of the 'if False' thing ;)
Revision 141 looks good.
Revision 142: Same comment as rev 140 - I'd l...