Check permissions on incoming requests #291

almet · 2015-06-05T09:48:34Z

Adds permissions check to cliquet applications:

Create a context factory to provide information about the requested objects;
Update the authorization policy to look at what's contained in the context;
Change the default permission to be dynamic;
Add unit tests for context factory;
Add unit tests for authorization policy;
Update the functional tests;
Add functional tests without mocking authorization.

To review:

Naming and conventions;
Tests;
I've updated the tests to use another fake authz backend, I think that would make everything easier to test, but that can be discussed.

Otherwise, it's required to extend the service argument, which adds complexity to the API.

Now it will use the dynamic permission system by default.

Natim · 2015-06-09T15:19:09Z

cliquet/permission/__init__.py

+        """
+        raise NotImplementedError
+
+    def object_permission_principals(self, object_id, permission):


rename to get_ace_principals

leplatrem · 2015-06-09T20:33:23Z

cliquet/__init__.py

 from cliquet import utils
 from cliquet.initialization import (  # NOQA
    initialize, initialize_cliquet, install_middlewares)

+# Main Cliquet logger.
+logger = structlog.get_logger()


Is that #102 ?

That's just to make flake8 happy (otherwise imports are after an instruction)

@leplatrem

This avoids passing around information to the authorization policy when it's not necessary. Here, at the definition time, the function is curried with the known arguments. From a review by @leplatrem.

@leplatrem

The former could be a boolean flag. Here it's explicit it is doing an operation. From a review by @leplatrem

Rather than using a string, which is error prone, having a constant allows to catch errors earlier in the dev process.

…tory Check permissions on incoming requests

almet added the to review label Jun 5, 2015

almet force-pushed the permissions-context-factory branch from ee9b091 to a294690 Compare June 8, 2015 15:30

almet changed the title ~~[DO NOT MERGE] Permissions context factory~~ Permissions context factory Jun 9, 2015

Rémy HUBSCHER and others added 27 commits June 9, 2015 17:03

Add the redis permission backend.

e898dd2

Add Permission backend tests.

865b7e1

Test permission inheritance.

d08e83d

Add Permission Memory Backend.

cd30e81

Use a list of patcher for client_error_patcher.

12114bf

Add documentation.

d431997

Add some spec

adf4b00

@leplatrem review.

ef0d71f

Add permission initialization.

09d03b5

Refactor permission backend to handle principals instead of user_id.

5e22ac7

Fix docstrings.

fac1da1

Tests should pass until full integration is done

591b9c2

Wording and docstrings

e848589

Rename get_perm_keys in get_bound_permissions

49f9915

Get rid of print

019c3de

Decode results in Redis permission backend

c2d64d0

@ametaireau review.

34462ae

Add context factory for permissions

945575d

Update context factory tests

23d19af

Test acls set by the RouteFactory are valid

5219803

Add the PATCH http method

3a635ca

Add a viewset ability to the viewset.

78c33f3

Otherwise, it's required to extend the service argument, which adds complexity to the API.

Add an current_view utils method to get the current accessed view

b091bd2

update route factory and update authz policy

3cc4346

Add a missing import

c74da6a

Remove permissions support from ViewSets.

97a512f

Now it will use the dynamic permission system by default.

Update the functionnal wrt new authz policy.

4274347

Natim reviewed Jun 9, 2015
View reviewed changes

almet changed the title ~~Permissions context factory~~ Check permissions on incoming requests Jun 9, 2015

leplatrem reviewed Jun 9, 2015
View reviewed changes

almet added 7 commits June 10, 2015 11:54

Use a partial for context.has_permission.

99e8a2f

This avoids passing around information to the authorization policy when it's not necessary. Here, at the definition time, the function is curried with the known arguments. From a review by @leplatrem.

Rename has_permission in check_permission.

9a0cfc8

The former could be a boolean flag. Here it's explicit it is doing an operation. From a review by @leplatrem

Rename ctx.object_type into ctx.resource_name

49ab1ea

Use a constant for authorization.DYNAMIC.

4fb0530

Rather than using a string, which is error prone, having a constant allows to catch errors earlier in the dev process.

Split tests in finer grained test cases

b2dc7b7

Use a constant for USER_PRINCIPAL

9e523b2

Fix Flake8 style

fb27a2b

almet added a commit that referenced this pull request Jun 10, 2015

Merge pull request #291 from mozilla-services/permissions-context-fac…

36e5d76

…tory Check permissions on incoming requests

almet merged commit 36e5d76 into master Jun 10, 2015

almet removed the to review label Jun 10, 2015

almet deleted the permissions-context-factory branch June 10, 2015 13:23

leplatrem modified the milestone: 2.0 Jun 11, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Check permissions on incoming requests #291

Check permissions on incoming requests #291

almet commented Jun 5, 2015

Natim Jun 9, 2015

leplatrem Jun 9, 2015

almet Jun 10, 2015

Check permissions on incoming requests #291

Check permissions on incoming requests #291

Conversation

almet commented Jun 5, 2015

Natim Jun 9, 2015

Choose a reason for hiding this comment

leplatrem Jun 9, 2015

Choose a reason for hiding this comment

almet Jun 10, 2015

Choose a reason for hiding this comment