Merge lp:~adeuring/launchpad/specification-auth-check into lp:launchpad
| Status: | Merged |
|---|---|
| Approved by: | Abel Deuring on 2012-08-21 |
| Approved revision: | no longer in the source branch. |
| Merged at revision: | 15844 |
| Proposed branch: | lp:~adeuring/launchpad/specification-auth-check |
| Merge into: | lp:launchpad |
| Diff against target: |
685 lines (+379/-34) 8 files modified
lib/lp/blueprints/browser/tests/test_specificationdependency.py (+13/-3) lib/lp/blueprints/configure.zcml (+17/-5) lib/lp/blueprints/interfaces/specification.py (+29/-9) lib/lp/blueprints/model/specification.py (+27/-0) lib/lp/blueprints/tests/test_specification.py (+235/-1) lib/lp/blueprints/tests/test_webservice.py (+31/-16) lib/lp/permissions.zcml (+5/-0) lib/lp/security.py (+22/-0) |
| To merge this branch: | bzr merge lp:~adeuring/launchpad/specification-auth-check |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Francesco Banconi (community) | 2012-08-20 | Approve on 2012-08-21 | |
|
Review via email:
|
|||
Commit Message
Prepare permission checks for ISPecification for access grants.
Description of the Change
This branch changes the security adapters for blueprints so that
the visibility of a blueprint can be limited to certain users as
described in https:/
Real sharing, as already implemented for bugs and branches, is not
yet possible.
The core changes are:
- the new class ISpecificationView. This class contains most
properties which were before defined in ISpecificationP
- ISpecificationP
database ID, information_type and a new method userCanView().
userCanView() is used to check if a user may access the
specifcation.
- new security adapters ViewSpecification and EditWhiteboardS
- a new permission "launchpad.
The current access rules are:
- Everybody, including anonymous users, can view every specification.
- Each logged in user can change the whiteboard.
- changes of some properties required the permission launchpad.Admin.
- changes of most properties require the permission launchpad.Edit.
This branch does not change the security adapters for launchpad.Admin
and launchpad.Edit.
The permissions for read access of ISpecificationView and for changes
of the whiteboard attribute of course need future changes. The final
rule will be: If a project is not publicly visible, only persons which
have access grants to the specification may view the data and change
the whiteboard.
Since the access grant related tables cannot yet store grants for
specifications, the current implementation of userCanView() simply
checks if the specification is publicly visible; if not, only those
people who can also edit most properties are granted view access.
Note that the default value of Specification.
1 (or InformationType
at present, so the "practical access rules" did not change.
Some notes about implementation details:
- Using the permission launchpad.View instead of launchpad.
in
<require
does not work: lp/secruity.py defined another Adapter for this
permission:
class AnonymousAccess
"""Anonymous users have launchpad.View on ISpecificationP
This is only needed because lazr.restful is hard-coded to check that
permission before returning things in a collection.
"""
permission = 'launchpad.View'
usedfor = ISpecificationP
- Changes to ISpecification.
launchpad.
is a special "short-cut logic" for this permission in
lp.services.
if (permission == 'launchpad.
return True
In other words: No security adapters are used for this permission.
So I added a new permission "launchpad.
not the best name -- I'd be grateful for a better suggestion.
- The permission definitions for ISpecificationn are quite complex,
To get an overview about all permissions and their adapters,
I added the tests test_get_
test_
Only when I wrote these tests, I noticed this config directive:
<require
That is of course easy to change -- but I missed it at first...
- The tests test_special_
should not only test that the owner of a specification has "special
right", but that other people certain roles have these right too.
I will add such tests in a later branch -- the diff for this one is
already long enough, and there are already some tests, for example in
./stories/
- I had to change some existing tests:
lib/lp/
lib/lp/
browser requests and want to access some attributes of a specification
when the request finished. During the end of a request, the function
endInteraction() is called which deletes thread_
but the new security adapters need access to the interaction.
The fixes are trivel: Aadding "with_person_
in some cases it is possible to access a specification attribute before
the request is issued.
tests:
./bin/test -vvt lp.blueprints.
no lint.

The code looks good Abel: this is a nice and well tested incremental change. Unfortunately I don't have a brilliant suggestion for the name of the new permission.
The tests pass, the new ones and the changed ones. Some comments follow.
286 + def check_permissio ns(self, expected_ permissions, used_permissions,
287 + type_):
It seems that indentation should be fixed here.
343 + def test_set_ permissions( self): get_permissions = {
344 + expected_
Maybe you intended expected_ set_permissions ?