flake8
passed.
JSHint
passed.
Allow API tokens to be flagged as deprecated.
Review Request #12627 — Created Sept. 26, 2022 and submitted
| Information | |
|---|---|
| maubin | |
| Djblets | |
| release-3.x | |
| 12631 | |
| Reviewers | |
| djblets | |
This change introduces a concept of deprecated token generators and allows API
tokens to be considered as deprecated if they were generated by a deprecated
token generator. The deprecated token generators can be set via the
DJBLETS_DEPRECATED_TOKEN_GENERATORSsetting.Now when a deprecated token is used for authentication, a deprecation notice
will be sent via a response header. Previously, the baseWebAPIResource
class would include headers set by authentication backends in the response
only when the authentication failed. For the purposes of including
deprecation notices in successful responses, we modified theWebAPIResource
to always include any headers set by authentication backends.
- Ran all unit tests
- Made requests using deprecated and non deprecated tokens to some
Reviewboard API endpoints (specifically GET and POST requests to
/review-requests/id/reviews/) - Ran all webapi tests in the Reviewboard test suite.
| Summary |
|---|
| Description | From | Last Updated |
|---|---|---|
|
For type comparisons, it's best to use is instead of ==. Though in this case, an assertion isn't going to … |
 chipx86
|
|
|
Blank line between statements and comments. |
chipx86
|
|
|
This is best as "Returns whether ...", since it's a function and not a property. |
chipx86
|
|
|
Do we need both this and setting headers= above in some classes? |
chipx86
|
|
|
Let's alphabetize the keys. |
chipx86
|
|
|
Returns -> Return |
 david
|
|
|
Might as well update this to "Return" while we're here too. |
david
|
|
|
The final .0 isn't needed here. |
chipx86
|
|
|
Same here. This can be 3.1. |
chipx86
|
Summary: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Description: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Testing Done: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Commits: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Diff: |
Revision 2 (+376 -38) |
Checks run (2 succeeded)
flake8
passed.
JSHint
passed.
-
-
djblets/secrets/token_generators/registry.py (Diff revision 2) For
typecomparisons, it's best to useisinstead of==.Though in this case, an assertion isn't going to tell a consumer much. Assertions are best done when we control the input and output.
Instead, if we want to check this, we should do a conditional that raises an
ImproperlyConfigured, which is a Django exception that means "this setting (or something similar) is wrong." -
djblets/webapi/auth/backends/api_tokens.py (Diff revision 2) Blank line between statements and comments.
-
djblets/webapi/models.py (Diff revision 2) This is best as "Returns whether ...", since it's a function and not a property.
-
djblets/webapi/resources/base.py (Diff revision 2) Do we need both this and setting
headers=above in some classes? -
Commits: |
|
||||||
|---|---|---|---|---|---|---|---|
Diff: |
Revision 3 (+378 -38) |
Checks run (2 succeeded)
flake8
passed.
JSHint
passed.
Change Summary:
Changed the deprecation notice message.
Commits: |
|
||||||
|---|---|---|---|---|---|---|---|
Diff: |
Revision 4 (+382 -38) |
Checks run (2 succeeded)
flake8
passed.
JSHint
passed.
-
-
-
djblets/webapi/models.py (Diff revision 4) Might as well update this to "Return" while we're here too.
Change Summary:
Changed "Returns" to "Return" in some docstrings.
Commits: |
|
||||||
|---|---|---|---|---|---|---|---|
Diff: |
Revision 5 (+384 -40) |
Checks run (2 succeeded)
flake8
passed.
JSHint
passed.
Change Summary:
Changed
Version Added:'s to 3.1.0 and added type hints to some functions.
Commits: |
|
||||||
|---|---|---|---|---|---|---|---|
Diff: |
Revision 6 (+394 -42) |
Checks run (2 succeeded)
flake8
passed.
JSHint
passed.
-
Looks great! Two tiny nits that are just for keeping consistency in docs, but all the code looks great! Feel free to make these changes and land it.
-
-
Commits: |
|
||||||
|---|---|---|---|---|---|---|---|
Diff: |
Revision 7 (+394 -42) |
Checks run (2 succeeded)
flake8
passed.
JSHint
passed.