Summary

Add an API for managing OAuth applications

Review Request #9002 — Created June 8, 2017 and submitted July 5, 2017, 2:17 p.m.

Information

Owner

brennie

Repository

Review Board

Branch

release-3.0.x

Bugs

Depends On

~~8987~~

Blocks

9013

Reviewers

Groups

reviewboard

People

Description

OAuth applications can now be created and managed via the oauth-apps/
API endpoint. Application information is only available to (1) the user
who created the app, (2) site administrators, and (3) Local Site
administrators when the application is associated with a Local Site.

Testing Done

Ran unit tests.

Issues

Description	From	Last Updated
W293 blank line contains whitespace	reviewbot	June 8, 2017, 4:34 p.m.
W293 blank line contains whitespace	reviewbot	June 8, 2017, 4:34 p.m.
W293 blank line contains whitespace	reviewbot	June 8, 2017, 4:34 p.m.
W293 blank line contains whitespace	reviewbot	June 8, 2017, 4:34 p.m.
W293 blank line contains whitespace	reviewbot	June 8, 2017, 4:34 p.m.
W291 trailing whitespace	reviewbot	June 8, 2017, 4:34 p.m.
W293 blank line contains whitespace	reviewbot	June 8, 2017, 4:34 p.m.
W293 blank line contains whitespace	reviewbot	June 8, 2017, 4:34 p.m.
W293 blank line contains whitespace	reviewbot	June 8, 2017, 4:34 p.m.
W293 blank line contains whitespace	reviewbot	June 8, 2017, 4:34 p.m.
W293 blank line contains whitespace	reviewbot	June 8, 2017, 4:34 p.m.
W293 blank line contains whitespace	reviewbot	June 8, 2017, 4:33 p.m.
W293 blank line contains whitespace	reviewbot	June 8, 2017, 4:33 p.m.
W293 blank line contains whitespace	reviewbot	June 8, 2017, 4:33 p.m.
W293 blank line contains whitespace	reviewbot	June 8, 2017, 4:33 p.m.
W293 blank line contains whitespace	reviewbot	June 8, 2017, 4:33 p.m.
W293 blank line contains whitespace	reviewbot	June 8, 2017, 4:33 p.m.
W293 blank line contains whitespace	reviewbot	June 8, 2017, 4:33 p.m.
E303 too many blank lines (2)	reviewbot	June 8, 2017, 4:33 p.m.
W293 blank line contains whitespace	reviewbot	June 8, 2017, 4:33 p.m.
W293 blank line contains whitespace	reviewbot	June 8, 2017, 4:33 p.m.
W293 blank line contains whitespace	reviewbot	June 8, 2017, 4:33 p.m.
W293 blank line contains whitespace	reviewbot	June 8, 2017, 4:33 p.m.
W293 blank line contains whitespace	reviewbot	June 8, 2017, 4:33 p.m.
W293 blank line contains whitespace	reviewbot	June 8, 2017, 4:33 p.m.
W293 blank line contains whitespace	reviewbot	June 8, 2017, 4:33 p.m.
W293 blank line contains whitespace	reviewbot	June 8, 2017, 4:33 p.m.
W293 blank line contains whitespace	reviewbot	June 8, 2017, 4:33 p.m.
E702 multiple statements on one line (semicolon)	reviewbot	June 8, 2017, 4:33 p.m.
E305 expected 2 blank lines after class or function definition, found 1	reviewbot	June 8, 2017, 4:33 p.m.
W293 blank line contains whitespace	reviewbot	June 8, 2017, 4:38 p.m.
E303 too many blank lines (2)	reviewbot	June 8, 2017, 4:37 p.m.
E303 too many blank lines (5)	reviewbot	June 8, 2017, 4:37 p.m.
F811 redefinition of unused 'test_get_filtered' from line 77	reviewbot	June 8, 2017, 4:38 p.m.
E303 too many blank lines (2)	reviewbot	June 8, 2017, 4:38 p.m.
): should be on the previous line. Also, docstring?	david	June 15, 2017, 5:47 p.m.
Module docstring?	david	June 15, 2017, 5:50 p.m.
Swap these two lines.	david	June 15, 2017, 5:50 p.m.
Swap these two lines.	david	June 15, 2017, 5:50 p.m.
You can enforce the actual values by doing: 'type': (Application.GRANT_AUTHORIZATION_CODE, Application.GRANT_CLIENT_CREDENTIALS, Application.GRANT_IMPLICIT, Application.GRANT_PASSWORD)	david	June 15, 2017, 5:51 p.m.
This can also use 'type': (Application.CLIENT_CONFIDENTIAL, Application.CLIENT_PUBLIC)	david	June 15, 2017, 5:51 p.m.
Can this (and the other changes in this file) be rolled up into existing review requests?	chipx86	June 23, 2017, 1:57 p.m.
Needs a space before "stylesheet"	chipx86	June 23, 2017, 2:08 p.m.
, optional	chipx86	June 23, 2017, 2:09 p.m.
, optional	chipx86	June 23, 2017, 2:09 p.m.
, optional	chipx86	June 23, 2017, 2:09 p.m.
, optional	chipx86	June 23, 2017, 2:09 p.m.
Can you put GRANT_PASSWORD on its own line?	chipx86	June 23, 2017, 2:10 p.m.
I'd like to include more information on what this is, for those new to OAuth2.	chipx86	June 23, 2017, 2:10 p.m.
Can you break the string after the newlines, so it reads more like paragraphs?	chipx86	June 23, 2017, 2:10 p.m.
Missing a period. Should also include a bit more information on what this is. In general, the more explicit the …	chipx86	June 23, 2017, 2:11 p.m.
Can you break the strings at newlines?	chipx86	June 23, 2017, 2:11 p.m.
Can you break at the newlines?	chipx86	June 23, 2017, 2:12 p.m.
Local Site	chipx86	June 23, 2017, 2:13 p.m.
Local Site	chipx86	June 23, 2017, 2:13 p.m.
Rather than having all this here, let's add permission checks to Application. There should be an is_accessible_by for this and …	chipx86	June 23, 2017, 2:22 p.m.
Should use local_site.is_mutable_by(request.user).	chipx86	June 23, 2017, 2:17 p.m.
This can fit on one line.	chipx86	June 23, 2017, 2:21 p.m.
, optional	chipx86	June 23, 2017, 2:22 p.m.
, optional	chipx86	June 23, 2017, 2:22 p.m.
local_site.is_mutable_by(request.user)	chipx86	June 23, 2017, 2:22 p.m.
This is already provided by the base class.	chipx86	June 23, 2017, 2:22 p.m.
We need a @webapi_request_fields that documents username=.	chipx86	June 23, 2017, 2:25 p.m.
These are already provided by the base class.	chipx86	June 23, 2017, 2:26 p.m.
No blank line.	chipx86	June 23, 2017, 2:26 p.m.
I don't really love the complexity of building a temp function that does its own caching. The logic should probably …	chipx86	June 23, 2017, 2:56 p.m.
Can you put this in parens? (the superuser or ...).	chipx86	June 23, 2017, 2:34 p.m.
local_site.is_mutable_by(request.user).	chipx86	June 23, 2017, 2:34 p.m.
How about "You don't have permission to set this field."? We may very well want special permissions for modifying OAuth2 …	chipx86	June 23, 2017, 2:35 p.m.
values_list would go on the next line.	chipx86	June 23, 2017, 2:35 p.m.
We should default this to None up above, instead of setting it here and in the else: further down.	chipx86	June 23, 2017, 2:36 p.m.
Let's remove the "on this LocalSite." RBCommons users are just going to ask questions about it. From their point of …	chipx86	June 23, 2017, 2:36 p.m.
How about instead of this, we have a not_owner_status_code and not_owner_error, defaulting to 403 and PERMISSION_DENIED, respectively. Then it's a …	chipx86	June 23, 2017, 2:37 p.m.
resources should go before tests.	chipx86	June 23, 2017, 2:39 p.m.
Missing a docstring.	chipx86	June 23, 2017, 2:40 p.m.
This sort of thing is repeated an aweful lot. How about giving _make_applications a filter argument?	chipx86	June 23, 2017, 2:42 p.m.
Can we use keyword arguments for all these? It'll make it more self-documenting.	chipx86	June 23, 2017, 2:47 p.m.
Missing a docstring.	chipx86	June 23, 2017, 2:47 p.m.
No need for these blank lines. They're all setting common state for the tests.	chipx86	June 23, 2017, 2:47 p.m.
Can you put a blank line before/after the api_get? It's helpful to have a block of setup code, a block …	chipx86	June 23, 2017, 2:48 p.m.
W293 blank line contains whitespace	reviewbot	June 23, 2017, 2:59 p.m.
W293 blank line contains whitespace	reviewbot	June 23, 2017, 2:59 p.m.
F401 'djblets.webapi.errors.PERMISSION_DENIED' imported but unused	reviewbot	June 23, 2017, 2:59 p.m.
E131 continuation line unaligned for hanging indent	reviewbot	June 23, 2017, 2:59 p.m.
E131 continuation line unaligned for hanging indent	reviewbot	June 23, 2017, 2:59 p.m.
F401 'djblets.webapi.errors.DOES_NOT_EXIST' imported but unused	reviewbot	June 23, 2017, 2:59 p.m.
W293 blank line contains whitespace	reviewbot	June 23, 2017, 3 p.m.
Blank line between these two.	david	July 4, 2017, 4:48 p.m.
i before s	david	July 4, 2017, 4:48 p.m.

flake8 failed.

JSHint passed.

flake8

```
Warning: Showing 30 of 42 failures.
```
reviewboard/oauth/forms.py (Diff revision 1)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 1)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 1)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 1)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 1)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 1)
The issue has been resolved. Show all issues
```
W291 trailing whitespace
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 1)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 1)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 1)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 1)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 1)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 1)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 1)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 1)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 1)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 1)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 1)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 1)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 1)
The issue has been resolved. Show all issues
```
E303 too many blank lines (2)
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 1)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 1)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 1)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 1)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 1)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 1)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 1)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 1)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 1)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 1)
The issue has been resolved. Show all issues
```
E702 multiple statements on one line (semicolon)
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 1)
The issue has been resolved. Show all issues
```
E305 expected 2 blank lines after class or function definition, found 1
```

Change Summary:

whitespace

Diff:

Revision 2 (+1012 -10)

Show changes

	reviewboard/oauth/forms.py
	reviewboard/templates/admin/change_form.html
	reviewboard/testing/testcase.py
	reviewboard/webapi/resources/__init__.py
	reviewboard/webapi/resources/oauth_app.py
	reviewboard/webapi/resources/root.py
	reviewboard/webapi/tests/mimetypes.py
	reviewboard/webapi/tests/mixins.py
	3 more

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

reviewboard/oauth/forms.py (Diff revision 2)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 2)
The issue has been resolved. Show all issues
```
E303 too many blank lines (2)
```
reviewboard/webapi/tests/test_oauth_app.py (Diff revision 2)
The issue has been resolved. Show all issues
```
E303 too many blank lines (5)
```
reviewboard/webapi/tests/test_oauth_app.py (Diff revision 2)
The issue has been resolved. Show all issues
```
F811 redefinition of unused 'test_get_filtered' from line 77
```
reviewboard/webapi/tests/test_oauth_app.py (Diff revision 2)
The issue has been resolved. Show all issues
```
E303 too many blank lines (2)
```

Change Summary:

flake8

Diff:

Revision 3 (+1007 -10)

Show changes

	reviewboard/oauth/forms.py
	reviewboard/templates/admin/change_form.html
	reviewboard/testing/testcase.py
	reviewboard/webapi/resources/__init__.py
	reviewboard/webapi/resources/oauth_app.py
	reviewboard/webapi/resources/root.py
	reviewboard/webapi/tests/mimetypes.py
	reviewboard/webapi/tests/mixins.py
	3 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/testing/testcase.py (Diff revision 3)
The issue has been resolved. Show all issues
```
): should be on the previous line. Also, docstring?
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Module docstring?
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Swap these two lines.
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Swap these two lines.
```

reviewboard/webapi/resources/oauth_app.py (Diff revision 3)

The issue has been resolved. Show all issues

You can enforce the actual values by doing:

'type': (Application.GRANT_AUTHORIZATION_CODE,
         Application.GRANT_CLIENT_CREDENTIALS,
         Application.GRANT_IMPLICIT,
         Application.GRANT_PASSWORD)

reviewboard/webapi/resources/oauth_app.py (Diff revision 3)

The issue has been resolved. Show all issues

This can also use 'type': (Application.CLIENT_CONFIDENTIAL, Application.CLIENT_PUBLIC)

Diff:

Revision 4 (+1038 -10)

Show changes

	reviewboard/oauth/forms.py
	reviewboard/templates/admin/change_form.html
	reviewboard/testing/testcase.py
	reviewboard/webapi/resources/__init__.py
	reviewboard/webapi/resources/oauth_app.py
	reviewboard/webapi/resources/root.py
	reviewboard/webapi/tests/mimetypes.py
	reviewboard/webapi/tests/mixins.py
	3 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:

Lock the skip_authorization field to admins. Cleanups. More tests.

Diff:

Revision 5 (+1293 -10)

Show changes

	reviewboard/oauth/forms.py
	reviewboard/templates/admin/change_form.html
	reviewboard/testing/testcase.py
	reviewboard/webapi/resources/__init__.py
	reviewboard/webapi/resources/oauth_app.py
	reviewboard/webapi/resources/root.py
	reviewboard/webapi/tests/mimetypes.py
	reviewboard/webapi/tests/mixins.py
	3 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Diff:

Revision 6 (+1294 -10)

Show changes

	reviewboard/oauth/forms.py
	reviewboard/templates/admin/change_form.html
	reviewboard/testing/testcase.py
	reviewboard/webapi/resources/__init__.py
	reviewboard/webapi/resources/oauth_app.py
	reviewboard/webapi/resources/root.py
	reviewboard/webapi/tests/mimetypes.py
	reviewboard/webapi/tests/mixins.py
	3 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Diff:

Revision 7 (+1293 -9)

Show changes

	reviewboard/oauth/forms.py
	reviewboard/templates/admin/change_form.html
	reviewboard/testing/testcase.py
	reviewboard/webapi/resources/__init__.py
	reviewboard/webapi/resources/oauth_app.py
	reviewboard/webapi/resources/root.py
	reviewboard/webapi/tests/mimetypes.py
	reviewboard/webapi/tests/mixins.py
	2 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/oauth/forms.py (Diff revision 7)

The issue has been resolved. Show all issues

Can this (and the other changes in this file) be rolled up into existing review requests?

reviewboard/templates/admin/change_form.html (Diff revision 7)
The issue has been resolved. Show all issues
```
Needs a space before "stylesheet"
```
reviewboard/testing/testcase.py (Diff revision 7)
The issue has been resolved. Show all issues
```
, optional
```
reviewboard/testing/testcase.py (Diff revision 7)
The issue has been resolved. Show all issues
```
, optional
```
reviewboard/testing/testcase.py (Diff revision 7)
The issue has been resolved. Show all issues
```
, optional
```
reviewboard/testing/testcase.py (Diff revision 7)
The issue has been resolved. Show all issues
```
, optional
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 7)
The issue has been resolved. Show all issues
```
Can you put GRANT_PASSWORD on its own line?
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 7)
The issue has been resolved. Show all issues
```
I'd like to include more information on what this is, for those new to OAuth2.
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 7)
The issue has been resolved. Show all issues
```
Can you break the string after the newlines, so it reads more like paragraphs?
```

reviewboard/webapi/resources/oauth_app.py (Diff revision 7)

The issue has been resolved. Show all issues

Missing a period.

Should also include a bit more information on what this is. In general, the more explicit the descriptions/help text, the more time we save others and ourselves.

reviewboard/webapi/resources/oauth_app.py (Diff revision 7)
The issue has been resolved. Show all issues
```
Can you break the strings at newlines?
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 7)
The issue has been resolved. Show all issues
```
Can you break at the newlines?
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 7)
The issue has been resolved. Show all issues
```
Local Site
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 7)
The issue has been resolved. Show all issues
```
Local Site
```

reviewboard/webapi/resources/oauth_app.py (Diff revision 7)

The issue has been resolved. Show all issues

Rather than having all this here, let's add permission checks to Application. There should be an is_accessible_by for this and is_mutable_by for the modify/delete checks (which are free to check is_accessible_by).

reviewboard/webapi/resources/oauth_app.py (Diff revision 7)
The issue has been resolved. Show all issues
```
Should use local_site.is_mutable_by(request.user).
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 7)
The issue has been resolved. Show all issues
```
This can fit on one line.
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 7)
The issue has been resolved. Show all issues
```
, optional
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 7)
The issue has been resolved. Show all issues
```
, optional
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 7)
The issue has been resolved. Show all issues
```
local_site.is_mutable_by(request.user)
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 7)
The issue has been resolved. Show all issues
```
This is already provided by the base class.
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 7)
The issue has been resolved. Show all issues
```
We need a @webapi_request_fields that documents username=.
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 7)
The issue has been resolved. Show all issues
```
These are already provided by the base class.
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 7)
The issue has been resolved. Show all issues
```
No blank line.
```

reviewboard/webapi/resources/oauth_app.py (Diff revision 7)

The issue has been resolved. Show all issues

I don't really love the complexity of building a temp function that does its own caching. The logic should probably change instead.
How about:

skip_authorization = parsed_request_fields.get('skip_authorization', False)
is_owner = (username is not None and
            username != request.user.username)

if skip_authorization or is_owner:
    # The user wants to set a fields only available to administrators.
    # Check for permission first.
    if not (request.user.is_authenticated() and
            (request.user.is_superuser or
             (...))):
        # The user doesn't have permission to modify these fields, so
        # return error messages for each of the fields they attempted to
        # modify.
        if skip_authorization:
            errors['skip_authorization'].append(...)

        if is_owner:
            errors['user'].append(...)
    else:
        # Start handling those fields.
        if is_owner:
            ...


This gets rid of the need to build a temp function that caches on itself, and it separates out the permission checking from the rest of the logic.
The User lookups would go in the else:. If the user doesn't have permission to set these fields, we shouldn't be giving them any information on whether the user they've set is valid. They should just get denied up-front.

reviewboard/webapi/resources/oauth_app.py (Diff revision 7)
The issue has been resolved. Show all issues
```
Can you put this in parens? (the superuser or ...).
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 7)
The issue has been resolved. Show all issues
```
local_site.is_mutable_by(request.user).
```

reviewboard/webapi/resources/oauth_app.py (Diff revision 7)

The issue has been resolved. Show all issues

How about "You don't have permission to set this field."? We may very well want special permissions for modifying OAuth2 entries for organizations without granting full admin privileges down the road.

reviewboard/webapi/resources/oauth_app.py (Diff revision 7)
The issue has been resolved. Show all issues
```
values_list would go on the next line.
```

reviewboard/webapi/resources/oauth_app.py (Diff revision 7)

The issue has been resolved. Show all issues

We should default this to None up above, instead of setting it here and in the else: further down.

reviewboard/webapi/resources/oauth_app.py (Diff revision 7)

The issue has been resolved. Show all issues

Let's remove the "on this LocalSite." RBCommons users are just going to ask questions about it. From their point of view, their team's users are the entire set of users.

reviewboard/webapi/tests/mixins.py (Diff revision 7)

The issue has been resolved. Show all issues

How about instead of this, we have a not_owner_status_code and not_owner_error, defaulting to 403 and PERMISSION_DENIED, respectively. Then it's a little more immediately clear what the purpose is, and it's not hard-coded for one specific use case.

These should also have doc comments.

reviewboard/webapi/tests/test_oauth_app.py (Diff revision 7)
The issue has been resolved. Show all issues
```
resources should go before tests.
```
reviewboard/webapi/tests/test_oauth_app.py (Diff revision 7)
The issue has been resolved. Show all issues
```
Missing a docstring.
```

reviewboard/webapi/tests/test_oauth_app.py (Diff revision 7)

The issue has been resolved. Show all issues

This sort of thing is repeated an aweful lot. How about giving _make_applications a filter argument?

reviewboard/webapi/tests/test_oauth_app.py (Diff revision 7)
The issue has been resolved. Show all issues
```
Can we use keyword arguments for all these? It'll make it more self-documenting.
```
reviewboard/webapi/tests/test_oauth_app.py (Diff revision 7)
The issue has been resolved. Show all issues
```
Missing a docstring.
```

reviewboard/webapi/tests/test_oauth_app.py (Diff revision 7)

The issue has been resolved. Show all issues

No need for these blank lines. They're all setting common state for the tests.

reviewboard/webapi/tests/test_oauth_app.py (Diff revision 7)

The issue has been resolved. Show all issues

Can you put a blank line before/after the api_get?

It's helpful to have a block of setup code, a block of test execution, and a block of assertions.

Here and below.

Change Summary:

Addressed Christian's issues.

Diff:

Revision 8 (+1361 -6)

Show changes

	reviewboard/oauth/models.py
	reviewboard/templates/admin/change_form.html
	reviewboard/testing/testcase.py
	reviewboard/webapi/resources/__init__.py
	reviewboard/webapi/resources/oauth_app.py
	reviewboard/webapi/resources/root.py
	reviewboard/webapi/tests/mimetypes.py
	reviewboard/webapi/tests/mixins.py
	2 more

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

reviewboard/oauth/models.py (Diff revision 8)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/oauth/models.py (Diff revision 8)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 8)
The issue has been resolved. Show all issues
```
F401 'djblets.webapi.errors.PERMISSION_DENIED' imported but unused
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 8)
The issue has been resolved. Show all issues
```
E131 continuation line unaligned for hanging indent
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 8)
The issue has been resolved. Show all issues
```
E131 continuation line unaligned for hanging indent
```
reviewboard/webapi/tests/mixins.py (Diff revision 8)
The issue has been resolved. Show all issues
```
F401 'djblets.webapi.errors.DOES_NOT_EXIST' imported but unused
```
reviewboard/webapi/tests/test_oauth_app.py (Diff revision 8)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```

Diff:

Revision 9 (+1360 -5)

Show changes

	reviewboard/oauth/models.py
	reviewboard/templates/admin/change_form.html
	reviewboard/testing/testcase.py
	reviewboard/webapi/resources/__init__.py
	reviewboard/webapi/resources/oauth_app.py
	reviewboard/webapi/resources/root.py
	reviewboard/webapi/tests/mimetypes.py
	reviewboard/webapi/tests/mixins.py
	2 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Depends On:: ~~8987 - Revamp the OAuth2 application admin form~~
~~9013 - Add a user-editable OAuth2 Application form~~

Depends On:: ~~9013 - Add a user-editable OAuth2 Application form~~
~~8987 - Revamp the OAuth2 application admin form~~

reviewboard/webapi/resources/oauth_app.py (Diff revision 9)
The issue has been resolved. Show all issues
```
Blank line between these two.
```
reviewboard/webapi/resources/oauth_app.py (Diff revision 9)
The issue has been resolved. Show all issues
```
i before s
```

Change Summary:

Light refactoring, address david's issues, allow regenerating client secrets.

Description:

		OAuth applications can now be created and managed via the `oauth-apps/`
		API endpoint. Application information is only available to (1) the user
~		who created the app, (2) site administrators, and (3) LocalSite
~		administrators when the application is associated with a LocalSite.
	~	who created the app, (2) site administrators, and (3) Local Site
	~	administrators when the application is associated with a Local Site.

Diff:

Revision 10 (+1405 -4)

Show changes

	reviewboard/oauth/models.py
	reviewboard/webapi/resources/__init__.py
	reviewboard/webapi/resources/oauth_app.py
	reviewboard/webapi/resources/root.py
	reviewboard/webapi/tests/mimetypes.py
	reviewboard/webapi/tests/mixins.py
	reviewboard/webapi/tests/test_oauth_app.py
	reviewboard/webapi/tests/urls.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-3.0.x (2993ce0)