MAAS

Merge lp://qastaging/~rvb/maas/maas-security-model-api2 into lp://qastaging/~maas-committers/maas/trunk

maas-security-model-api2
Merge into trunk

Proposed by Raphaël Badin on 2012-01-24

Status:	Merged
Approved by:	Raphaël Badin on 2012-01-24
Approved revision:	no longer in the source branch.
Merged at revision:	46
Proposed branch:	lp://qastaging/~rvb/maas/maas-security-model-api2
Merge into:	lp://qastaging/~maas-committers/maas/trunk
Prerequisite:	lp://qastaging/~rvb/maas/maas-templates-fixes
Diff against target:	594 lines (+213/-52) 8 files modified docs/models.rst (+2/-0) src/maas/settings.py (+4/-0) src/maasserver/api.py (+40/-9) src/maasserver/models.py (+36/-7) src/maasserver/testing/__init__.py (+24/-0) src/maasserver/tests/test_api.py (+100/-27) src/maasserver/tests/test_auth.py (+6/-8) src/maasserver/views.py (+1/-1)
To merge this branch:	bzr merge lp://qastaging/~rvb/maas/maas-security-model-api2
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Gavin Panella (community)		2012-01-24	Approve on 2012-01-24
Review via email: mp+89949@code.qastaging.launchpad.net

Commit message

Apply security to the API.

Description of the change

This branch is the second (and final) branch to use the security mechanisms introduced in ~rvb/maas/maas-security-model in the API.

= Notes =

I've removed the admin special case in the backend and in the tests because django already enforces the fact that an admin has all the permissions.

Revision history for this message

Gavin Panella (allenap) wrote on 2012-01-24:

Looks good :)

[1]

+ try:
...
+ except PermissionDenied:
+ return rc.FORBIDDEN

This appears in many places. Is it worth creating a decorator that
changes uncaught PermissionDenied exceptions into rc.FORBIDDEN?

[2]

There's get_visible_node_or_404 and visible_nodes. I think the latter
ought to be renamed get_visible_nodes to match, and so that it doesn't
resemble a property.

[3]

+ :type user: django.contrib.auth.models.User

s/[.]$//

This is shown withing braces in the generated docs and looks funny
with a full-stop at the end.

review: Approve

Revision history for this message

Raphaël Badin (rvb) wrote on 2012-01-24:

> Looks good :)
>
>
> [1]
>
> + try:
> ...
> + except PermissionDenied:
> + return rc.FORBIDDEN
>
> This appears in many places. Is it worth creating a decorator that
> changes uncaught PermissionDenied exceptions into rc.FORBIDDEN?

Good idea, I've done it.

> [2]
>
> There's get_visible_node_or_404 and visible_nodes. I think the latter
> ought to be renamed get_visible_nodes to match, and so that it doesn't
> resemble a property.

Done.

> [3]
>
> + :type user: django.contrib.auth.models.User
>
> s/[.]$//

Okay.
> This is shown withing braces in the generated docs and looks funny
> with a full-stop at the end.