-
-
-
djblets/webapi/resources/mixins/oauth2_tokens.py (Diff revision 1) Col: 24 W292 no newline at end of file
lehoangm got a fish trophy!
[OAuth2Provider][WIP] Enabling OAuth2 authorization on Web API
Review Request #7997 — Created Feb. 25, 2016 and discarded
Prior to this commit, there is no mechanism to authorize resources using OAuth2
protocol. In this commit, OAuth2 authorization mechanism is added to
the current authorization mechanism for web API resources.
Services now can follow OAuth2 protocol, send a request with appropriate
authorization header to work with Djblet's resources. External services can
request a token with specify scope through OAuth2 protocol. When using Djblet's
API resource, the scope of a token is checked to make sure that the
external service has the right permission the resource.This commit also adding unit tests for all current backend authentication:
the base authentication class, the basic authentication via username and
password, the api_token authentication, and the newly added oauth2
authentication backend.
Add test_base_auth_backend to test WebAPIAuthBackend
Add test_basic_auth_backend to test WebAPIBasicAuthBackend
Add test_api_auth_backend to test WebAPITokenAuthBackend
Add test_oauth2_auth_backend to test OAuth2TokenAuthBackendThere should be one more for ResourceOAuth2TokenMixin
| Description | From | Last Updated |
|---|---|---|
|
undefined name 'webapi_token' |
 reviewbot
|
|
|
Col: 24 W292 no newline at end of file |
reviewbot
|
|
|
Col: 1 W391 blank line at end of file |
reviewbot
|
|
|
Col: 1 W293 blank line contains whitespace |
reviewbot
|
|
|
Col: 80 E501 line too long (81 > 79 characters) |
reviewbot
|
|
|
This should further elaborate (in a second paragraph) which fields (and types) would be required by this backend (looks like … |
 chipx86
|
|
|
Description must be on the next line. The type should be django.http.HttpRequest. (Some of our older classes don't provide the … |
chipx86
|
|
|
What if it's 3? That'd be a different error. |
chipx86
|
|
|
The type should be django.http.HttpRequest. |
chipx86
|
|
|
Must just be tuple. You can document the None in the description. |
chipx86
|
|
|
Must be aligned, or it'll result in syntax errors in the docs. |
chipx86
|
|
|
Should be: result = super(OAuth2TokenAuthBackend, self).login_with_credentials( request, **credentials) |
chipx86
|
|
|
call_method_view should go on the previous line, with arguments on the second. |
chipx86
|
|
|
Like with the backend, this should document the fields and types required. |
chipx86
|
|
|
This can be one statement. |
chipx86
|
|
|
Try: return super(ResourceOAuth2TokenMixin, self).call_method_view( request, method, view, *args, **kwargs) |
chipx86
|
|
|
This needs to have Args and Returns sections. |
chipx86
|
|
|
I don't see how this code works. resources_policy should be a list of strings, which shouldn't have .get(). |
chipx86
|
|
|
Col: 1 W293 blank line contains whitespace |
reviewbot
|
|
|
Col: 1 W391 blank line at end of file |
reviewbot
|
|
|
I don't know about this. The unit tests are supposed to be testing the new auth backend, but they're really … |
chipx86
|
|
|
Missing period, and """ on the next line. Plus a blank line below. Same below. |
chipx86
|
|
|
"Return". Missing period. Same below. |
chipx86
|
|
|
Make sure to use the same format as other classes. Same below. |
chipx86
|
|
|
Missing type. Same below. |
chipx86
|
|
|
"Object" isn't a valid Python type. |
chipx86
|
|
|
Must inherit from object. However, we should use a real User, not a mock. |
chipx86
|
|
|
Can't align this way, or it'll break doc generation. Make sure to use the same format used by other classes. |
chipx86
|
|
|
Make sure to follow the same exact format other tests use for docstrings. |
chipx86
|
|
|
Col: 80 E501 line too long (81 > 79 characters) |
reviewbot
|
|
|
Make sure to follow the correct format for these. |
chipx86
|
|
|
Should have a trailing comma. |
chipx86
|
|
|
validate_credentials should be on the previous line. |
chipx86
|
|
|
Col: 80 E501 line too long (83 > 79 characters) |
reviewbot
|
|
|
'BaseWebAPIToken' imported but unused |
reviewbot
|
|
|
Col: 19 E251 unexpected spaces around keyword / parameter equals |
reviewbot
|
|
|
Col: 17 E251 unexpected spaces around keyword / parameter equals |
reviewbot
|
|
|
Col: 18 E251 unexpected spaces around keyword / parameter equals |
reviewbot
|
|
|
Col: 20 E251 unexpected spaces around keyword / parameter equals |
reviewbot
|
|
|
Col: 17 E251 unexpected spaces around keyword / parameter equals |
reviewbot
|
|
|
Col: 19 E251 unexpected spaces around keyword / parameter equals |
reviewbot
|
|
|
Col: 18 E251 unexpected spaces around keyword / parameter equals |
reviewbot
|
|
|
Col: 20 E251 unexpected spaces around keyword / parameter equals |
reviewbot
|
|
|
Col: 63 E231 missing whitespace after ',' |
reviewbot
|
|
|
Col: 80 E501 line too long (80 > 79 characters) |
reviewbot
|
|
|
Col: 80 E501 line too long (81 > 79 characters) |
reviewbot
|
|
|
'BaseWebAPIToken' imported but unused |
reviewbot
|
|
|
Col: 17 E251 unexpected spaces around keyword / parameter equals |
reviewbot
|
|
|
Col: 19 E251 unexpected spaces around keyword / parameter equals |
reviewbot
|
|
|
Col: 18 E251 unexpected spaces around keyword / parameter equals |
reviewbot
|
|
|
Col: 20 E251 unexpected spaces around keyword / parameter equals |
reviewbot
|
|
|
Col: 17 E251 unexpected spaces around keyword / parameter equals |
reviewbot
|
|
|
Col: 19 E251 unexpected spaces around keyword / parameter equals |
reviewbot
|
|
|
Col: 18 E251 unexpected spaces around keyword / parameter equals |
reviewbot
|
|
|
Col: 20 E251 unexpected spaces around keyword / parameter equals |
reviewbot
|
LE
Change Summary:
Change commit wording to fit in Djblet's context
Summary: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Description: |
|
LE
Change Summary:
Add WebAPIAuthBackend tests
Testing Done: |
|
||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 2 (+522 -83) |
-
Tool: Pyflakes Processed Files: djblets/markdown/tests.py djblets/webapi/resources/mixins/api_tokens.py djblets/webapi/auth/backends/oauth2_tokens.py djblets/webapi/resources/mixins/oauth2_tokens.py djblets/avatars/services/base.py djblets/avatars/tests.py djblets/avatars/registry.py djblets/webapi/tests/test_base_auth_backend.py Ignored Files: djblets/static/djblets/css/defs.less djblets/static/djblets/css/mixins/markdown.less djblets/static/djblets/css/config-forms.less .gitignore Tool: PEP8 Style Checker Processed Files: djblets/markdown/tests.py djblets/webapi/resources/mixins/api_tokens.py djblets/webapi/auth/backends/oauth2_tokens.py djblets/webapi/resources/mixins/oauth2_tokens.py djblets/avatars/services/base.py djblets/avatars/tests.py djblets/avatars/registry.py djblets/webapi/tests/test_base_auth_backend.py Ignored Files: djblets/static/djblets/css/defs.less djblets/static/djblets/css/mixins/markdown.less djblets/static/djblets/css/config-forms.less .gitignore -
djblets/webapi/resources/mixins/oauth2_tokens.py (Diff revision 2) Col: 1 W391 blank line at end of file
-
djblets/webapi/resources/mixins/oauth2_tokens.py (Diff revision 2) Col: 1 W293 blank line contains whitespace
-
djblets/webapi/tests/test_base_auth_backend.py (Diff revision 2) Col: 80 E501 line too long (81 > 79 characters)
-
Tool: Pyflakes Processed Files: djblets/webapi/resources/mixins/oauth2_tokens.py djblets/webapi/tests/test_base_auth_backend.py djblets/webapi/resources/mixins/api_tokens.py djblets/webapi/auth/backends/oauth2_tokens.py Tool: PEP8 Style Checker Processed Files: djblets/webapi/resources/mixins/oauth2_tokens.py djblets/webapi/tests/test_base_auth_backend.py djblets/webapi/resources/mixins/api_tokens.py djblets/webapi/auth/backends/oauth2_tokens.py -
djblets/webapi/resources/mixins/oauth2_tokens.py (Diff revision 3) Col: 1 W293 blank line contains whitespace
-
djblets/webapi/resources/mixins/oauth2_tokens.py (Diff revision 3) Col: 1 W391 blank line at end of file
-
djblets/webapi/tests/test_base_auth_backend.py (Diff revision 3) Col: 80 E501 line too long (81 > 79 characters)
-
-
djblets/webapi/auth/backends/oauth2_tokens.py (Diff revision 3) This should further elaborate (in a second paragraph) which fields (and types) would be required by this backend (looks like it's
tokenanduser, basically?). -
djblets/webapi/auth/backends/oauth2_tokens.py (Diff revision 3) Description must be on the next line.
The type should be
django.http.HttpRequest. (Some of our older classes don't provide the full path, but new ones should.) -
djblets/webapi/auth/backends/oauth2_tokens.py (Diff revision 3) What if it's 3? That'd be a different error.
-
djblets/webapi/auth/backends/oauth2_tokens.py (Diff revision 3) The type should be
django.http.HttpRequest. -
djblets/webapi/auth/backends/oauth2_tokens.py (Diff revision 3) Must just be
tuple. You can document theNonein the description. -
djblets/webapi/auth/backends/oauth2_tokens.py (Diff revision 3) Must be aligned, or it'll result in syntax errors in the docs.
-
djblets/webapi/auth/backends/oauth2_tokens.py (Diff revision 3) Should be:
result = super(OAuth2TokenAuthBackend, self).login_with_credentials( request, **credentials) -
djblets/webapi/resources/mixins/api_tokens.py (Diff revision 3) call_method_viewshould go on the previous line, with arguments on the second. -
djblets/webapi/resources/mixins/oauth2_tokens.py (Diff revision 3) Like with the backend, this should document the fields and types required.
-
-
djblets/webapi/resources/mixins/oauth2_tokens.py (Diff revision 3) Try:
return super(ResourceOAuth2TokenMixin, self).call_method_view( request, method, view, *args, **kwargs) -
djblets/webapi/resources/mixins/oauth2_tokens.py (Diff revision 3) This needs to have
ArgsandReturnssections. -
djblets/webapi/resources/mixins/oauth2_tokens.py (Diff revision 3) I don't see how this code works.
resources_policyshould be a list of strings, which shouldn't have.get(). -
djblets/webapi/tests/test_base_auth_backend.py (Diff revision 3) I don't know about this. The unit tests are supposed to be testing the new auth backend, but they're really just testing this fake backend, right? If so, the new test backend isn't getting any real coverage, making the tests useless.
-
djblets/webapi/tests/test_base_auth_backend.py (Diff revision 3) Missing period, and
"""on the next line.Plus a blank line below.
Same below.
-
djblets/webapi/tests/test_base_auth_backend.py (Diff revision 3) "Return".
Missing period.
Same below.
-
djblets/webapi/tests/test_base_auth_backend.py (Diff revision 3) Make sure to use the same format as other classes.
Same below.
-
-
djblets/webapi/tests/test_base_auth_backend.py (Diff revision 3) "Object" isn't a valid Python type.
-
djblets/webapi/tests/test_base_auth_backend.py (Diff revision 3) Must inherit from
object.However, we should use a real
User, not a mock. -
djblets/webapi/tests/test_base_auth_backend.py (Diff revision 3) Can't align this way, or it'll break doc generation.
Make sure to use the same format used by other classes.
-
djblets/webapi/tests/test_base_auth_backend.py (Diff revision 3) Make sure to follow the same exact format other tests use for docstrings.
-
djblets/webapi/tests/test_base_auth_backend.py (Diff revision 3) Make sure to follow the correct format for these.
-
-
djblets/webapi/tests/test_base_auth_backend.py (Diff revision 3) validate_credentialsshould be on the previous line.
LE
Change Summary:
Update CR; change description; adding testes
Description: |
|
||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Testing Done: |
|
||||||||||||||||||||||||||||||||||||||||||||||||
Diff: |
Revision 4 (+820 -2) |
-
Tool: Pyflakes Processed Files: djblets/webapi/resources/mixins/api_tokens.py djblets/webapi/auth/backends/oauth2_tokens.py djblets/webapi/tests/test_oauth2_auth_backend.py djblets/webapi/resources/mixins/oauth2_tokens.py djblets/webapi/tests/test_basic_auth_backend.py djblets/webapi/tests/test_api_auth_backend.py djblets/webapi/tests/test_base_auth_backend.py djblets/webapi/auth/backends/api_tokens.py Tool: PEP8 Style Checker Processed Files: djblets/webapi/resources/mixins/api_tokens.py djblets/webapi/auth/backends/oauth2_tokens.py djblets/webapi/tests/test_oauth2_auth_backend.py djblets/webapi/resources/mixins/oauth2_tokens.py djblets/webapi/tests/test_basic_auth_backend.py djblets/webapi/tests/test_api_auth_backend.py djblets/webapi/tests/test_base_auth_backend.py djblets/webapi/auth/backends/api_tokens.py -
djblets/webapi/auth/backends/oauth2_tokens.py (Diff revision 4) Col: 80 E501 line too long (83 > 79 characters)
-
djblets/webapi/tests/test_api_auth_backend.py (Diff revision 4) 'BaseWebAPIToken' imported but unused
-
djblets/webapi/tests/test_api_auth_backend.py (Diff revision 4) Col: 19 E251 unexpected spaces around keyword / parameter equals
-
djblets/webapi/tests/test_api_auth_backend.py (Diff revision 4) Col: 17 E251 unexpected spaces around keyword / parameter equals
-
djblets/webapi/tests/test_api_auth_backend.py (Diff revision 4) Col: 18 E251 unexpected spaces around keyword / parameter equals
-
djblets/webapi/tests/test_api_auth_backend.py (Diff revision 4) Col: 20 E251 unexpected spaces around keyword / parameter equals
-
djblets/webapi/tests/test_api_auth_backend.py (Diff revision 4) Col: 17 E251 unexpected spaces around keyword / parameter equals
-
djblets/webapi/tests/test_api_auth_backend.py (Diff revision 4) Col: 19 E251 unexpected spaces around keyword / parameter equals
-
djblets/webapi/tests/test_api_auth_backend.py (Diff revision 4) Col: 18 E251 unexpected spaces around keyword / parameter equals
-
djblets/webapi/tests/test_api_auth_backend.py (Diff revision 4) Col: 20 E251 unexpected spaces around keyword / parameter equals
-
djblets/webapi/tests/test_base_auth_backend.py (Diff revision 4) Col: 63 E231 missing whitespace after ','
-
djblets/webapi/tests/test_basic_auth_backend.py (Diff revision 4) Col: 80 E501 line too long (80 > 79 characters)
-
djblets/webapi/tests/test_basic_auth_backend.py (Diff revision 4) Col: 80 E501 line too long (81 > 79 characters)
-
djblets/webapi/tests/test_oauth2_auth_backend.py (Diff revision 4) 'BaseWebAPIToken' imported but unused
-
djblets/webapi/tests/test_oauth2_auth_backend.py (Diff revision 4) Col: 17 E251 unexpected spaces around keyword / parameter equals
-
djblets/webapi/tests/test_oauth2_auth_backend.py (Diff revision 4) Col: 19 E251 unexpected spaces around keyword / parameter equals
-
djblets/webapi/tests/test_oauth2_auth_backend.py (Diff revision 4) Col: 18 E251 unexpected spaces around keyword / parameter equals
-
djblets/webapi/tests/test_oauth2_auth_backend.py (Diff revision 4) Col: 20 E251 unexpected spaces around keyword / parameter equals
-
djblets/webapi/tests/test_oauth2_auth_backend.py (Diff revision 4) Col: 17 E251 unexpected spaces around keyword / parameter equals
-
djblets/webapi/tests/test_oauth2_auth_backend.py (Diff revision 4) Col: 19 E251 unexpected spaces around keyword / parameter equals
-
djblets/webapi/tests/test_oauth2_auth_backend.py (Diff revision 4) Col: 18 E251 unexpected spaces around keyword / parameter equals
-
djblets/webapi/tests/test_oauth2_auth_backend.py (Diff revision 4) Col: 20 E251 unexpected spaces around keyword / parameter equals
Tool: PEP8 Style Checker Processed Files: djblets/webapi/resources/mixins/oauth2_tokens.py djblets/webapi/resources/mixins/api_tokens.py djblets/webapi/auth/backends/oauth2_tokens.py Tool: Pyflakes Processed Files: djblets/webapi/resources/mixins/oauth2_tokens.py djblets/webapi/resources/mixins/api_tokens.py djblets/webapi/auth/backends/oauth2_tokens.py