Summary

Add policy enforcement through API tokens.

Review Request #6091 — Created July 10, 2014 and submitted July 23, 2014, 12:50 a.m.

Information

Owner

chipx86

Repository

Review Board

Branch

master

Bugs

Depends On

Commit

66d8888...

Reviewers

Groups

reviewboard

People

Description

This allows API tokens to dictate what resources can be accessed and

with what HTTP methods.
WebAPIToken.policy can now contain a set of rules that apply globally

and/or to individual resources, in order to restrict what a client can

access and what HTTP methods they can perform.
Policies are in the following format:
{
    "resources": {
        "*": {
            "allow": [<list of methods or "*">],
            "block": [<list of methods or "*">]
        },
        "<resource_policy_id>": {
            "*": {
                "allow": [<list of methods or "*">],
                "block": [<list of methods or "*">]
            },
            "<resource_id>": {
                "allow": [<list of methods or "*">],
                "block": [<list of methods or "*">]
            },
            ...
        },
        ...
    }
}

Specific resource policy IDs take precedence over the global *.
The * rule belonging to a resource covers all access across the list

and child resource IDs. The ID-specific entries take precedence over the

*.
Each sub-policy in a resource can have allow and/or block. Each

contains a list of HTTP method names ("POST", "GET", etc.) or a

single item of *, which means "all methods."
Any attempt to access a resource with an HTTP method that is denied by a

token's policy will result in a PERMISSION_DENIED error.
If a policy is completely empty, the default is to be permissive.

Testing Done

All new and existing unit tests pass.

I created a couple tokens to play with. I tested global blocking of the API, per-resource blocking, per-ID blocking. I also tested blocking with per-resource and per-ID allow.

Issues

Description	From	Last Updated
Can't this just be resource_id = kwargs.get(self.uri_object_key) ?	david	July 22, 2014, 6:29 p.m.
I think 'permission' is a better name than 'allowance'	david	July 22, 2014, 6:29 p.m.
Maybe include an explicit return None at the end?	david	July 22, 2014, 6:30 p.m.

Tool: Pyflakes
Processed Files:
    reviewboard/webapi/base.py
    reviewboard/webapi/resources/review_screenshot_comment.py
    reviewboard/webapi/resources/draft_filediff.py
    reviewboard/webapi/resources/repository_info.py
    reviewboard/webapi/resources/filediff.py
    reviewboard/webapi/resources/root.py
    reviewboard/webapi/tests/test_api_policy.py
    reviewboard/webapi/resources/review_request_draft.py
    reviewboard/webapi/resources/review_request_last_update.py
    reviewboard/webapi/resources/review_reply_screenshot_comment.py
    reviewboard/webapi/resources/server_info.py
    reviewboard/webapi/resources/review_file_attachment_comment.py
    reviewboard/webapi/resources/review_group_user.py
    reviewboard/webapi/resources/review_reply_diff_comment.py
    reviewboard/webapi/resources/repository_commits.py
    reviewboard/webapi/resources/repository_branches.py
    reviewboard/webapi/resources/review_reply_draft.py
    reviewboard/webapi/resources/filediff_comment.py
    reviewboard/webapi/resources/review_diff_comment.py
    reviewboard/webapi/resources/review_reply.py
    reviewboard/webapi/resources/review_reply_file_attachment_comment.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/webapi/base.py
    reviewboard/webapi/resources/review_screenshot_comment.py
    reviewboard/webapi/resources/draft_filediff.py
    reviewboard/webapi/resources/repository_info.py
    reviewboard/webapi/resources/filediff.py
    reviewboard/webapi/resources/root.py
    reviewboard/webapi/tests/test_api_policy.py
    reviewboard/webapi/resources/review_request_draft.py
    reviewboard/webapi/resources/review_request_last_update.py
    reviewboard/webapi/resources/review_reply_screenshot_comment.py
    reviewboard/webapi/resources/server_info.py
    reviewboard/webapi/resources/review_file_attachment_comment.py
    reviewboard/webapi/resources/review_group_user.py
    reviewboard/webapi/resources/review_reply_diff_comment.py
    reviewboard/webapi/resources/repository_commits.py
    reviewboard/webapi/resources/repository_branches.py
    reviewboard/webapi/resources/review_reply_draft.py
    reviewboard/webapi/resources/filediff_comment.py
    reviewboard/webapi/resources/review_diff_comment.py
    reviewboard/webapi/resources/review_reply.py
    reviewboard/webapi/resources/review_reply_file_attachment_comment.py

Description:

		This allows API tokens to dictate what resources can be accessed and
		with what HTTP methods.

		`WebAPIToken.policy` can now contain a set of rules that apply globally
		and/or to individual resources, in order to restrict what a client can
		access and what HTTP methods they can perform.

		Policies are in the following format:

		`{`
		"resources": {
		"*": {
		"allow": [<list of methods or "*">],
		"block": [<list of methods or "*">]
		},
		"<resource_policy_id>": {
		"*": {
		"allow": [<list of methods or "*">],
		"block": [<list of methods or "*">]
		},
		"<resource_id>": {
		"allow": [<list of methods or "*">],
		"block": [<list of methods or "*">]
		},
		...
		},
		...
		}
		}


~		Specific resource policy IDs take precedence over the global "*".
	~	Specific resource policy IDs take precedence over the global `*`.

~		The "" rule belonging to a resource covers all access across the list
	~	The `*` rule belonging to a resource covers all access across the list
		and child resource IDs. The ID-specific entries take precedence over the
~		"".
	~	`*`.

~		Each sub-policy in a resource can have "allow" and/or "block". Each
	~	Each sub-policy in a resource can have `allow` and/or `block`. Each
		contains a list of HTTP method names ("POST", "GET", etc.) or a
~		single item of "*", which means "all methods."
	~	single item of `*`, which means "all methods."

		Any attempt to access a resource with an HTTP method that is denied by a
		token's policy will result in a `PERMISSION_DENIED` error.

		If a policy is completely empty, the default is to be permissive.

reviewboard/webapi/base.py (Diff revision 1)

The issue has been resolved. Show all issues

Can't this just be resource_id = kwargs.get(self.uri_object_key) ?

reviewboard/webapi/base.py (Diff revision 1)
The issue has been resolved. Show all issues
```
I think 'permission' is a better name than 'allowance'
```
reviewboard/webapi/base.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Maybe include an explicit return None at the end?
```

Change Summary:


Fixed a bone-headed bit of logic that could easily be condensed.
Changed allowance to permission.
Added an explicit return statement in _check_resource_policy.

Commit:

-	0ce741bb302e40d3b268e661c134f7fc97c20981
+	66d888836546b3df595476d75d87895c27a62fa5

Diff:

Revision 2 (+458 -3)

Show changes

	reviewboard/webapi/base.py
	reviewboard/webapi/resources/draft_filediff.py
	reviewboard/webapi/resources/filediff.py
	reviewboard/webapi/resources/filediff_comment.py
	reviewboard/webapi/resources/repository_branches.py
	reviewboard/webapi/resources/repository_commits.py
	reviewboard/webapi/resources/repository_info.py
	reviewboard/webapi/resources/review_diff_comment.py
	13 more

Tool: Pyflakes
Processed Files:
    reviewboard/webapi/base.py
    reviewboard/webapi/resources/review_screenshot_comment.py
    reviewboard/webapi/resources/draft_filediff.py
    reviewboard/webapi/resources/repository_info.py
    reviewboard/webapi/resources/filediff.py
    reviewboard/webapi/resources/root.py
    reviewboard/webapi/tests/test_api_policy.py
    reviewboard/webapi/resources/review_request_draft.py
    reviewboard/webapi/resources/review_request_last_update.py
    reviewboard/webapi/resources/review_reply_screenshot_comment.py
    reviewboard/webapi/resources/server_info.py
    reviewboard/webapi/resources/review_file_attachment_comment.py
    reviewboard/webapi/resources/review_group_user.py
    reviewboard/webapi/resources/review_reply_diff_comment.py
    reviewboard/webapi/resources/repository_commits.py
    reviewboard/webapi/resources/repository_branches.py
    reviewboard/webapi/resources/review_reply_draft.py
    reviewboard/webapi/resources/filediff_comment.py
    reviewboard/webapi/resources/review_diff_comment.py
    reviewboard/webapi/resources/review_reply.py
    reviewboard/webapi/resources/review_reply_file_attachment_comment.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/webapi/base.py
    reviewboard/webapi/resources/review_screenshot_comment.py
    reviewboard/webapi/resources/draft_filediff.py
    reviewboard/webapi/resources/repository_info.py
    reviewboard/webapi/resources/filediff.py
    reviewboard/webapi/resources/root.py
    reviewboard/webapi/tests/test_api_policy.py
    reviewboard/webapi/resources/review_request_draft.py
    reviewboard/webapi/resources/review_request_last_update.py
    reviewboard/webapi/resources/review_reply_screenshot_comment.py
    reviewboard/webapi/resources/server_info.py
    reviewboard/webapi/resources/review_file_attachment_comment.py
    reviewboard/webapi/resources/review_group_user.py
    reviewboard/webapi/resources/review_reply_diff_comment.py
    reviewboard/webapi/resources/repository_commits.py
    reviewboard/webapi/resources/repository_branches.py
    reviewboard/webapi/resources/review_reply_draft.py
    reviewboard/webapi/resources/filediff_comment.py
    reviewboard/webapi/resources/review_diff_comment.py
    reviewboard/webapi/resources/review_reply.py
    reviewboard/webapi/resources/review_reply_file_attachment_comment.py

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to master (058eb89)