Allow API tokens to be flagged as deprecated.

Review Request #12627 — Created Sept. 26, 2022 and submitted

Information

Djblets
release-3.x

Reviewers

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_GENERATORS setting.

Now when a deprecated token is used for authentication, a deprecation notice
will be sent via a response header. Previously, the base WebAPIResource
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 the WebAPIResource
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 ID
Allow API tokens to be flagged as deprecated.
d05fdaa1ac8507c72f043fbfbcba13c798c16449
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 …

chipx86chipx86

Blank line between statements and comments.

chipx86chipx86

This is best as "Returns whether ...", since it's a function and not a property.

chipx86chipx86

Do we need both this and setting headers= above in some classes?

chipx86chipx86

Let's alphabetize the keys.

chipx86chipx86

Returns -> Return

daviddavid

Might as well update this to "Return" while we're here too.

daviddavid

The final .0 isn't needed here.

chipx86chipx86

Same here. This can be 3.1.

chipx86chipx86
maubin
chipx86
  1. 
      
  2. Show all issues

    For type comparisons, it's best to use is instead 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."

  3. djblets/webapi/auth/backends/api_tokens.py (Diff revision 2)
     
     
     
    Show all issues

    Blank line between statements and comments.

  4. djblets/webapi/models.py (Diff revision 2)
     
     
    Show all issues

    This is best as "Returns whether ...", since it's a function and not a property.

  5. djblets/webapi/resources/base.py (Diff revision 2)
     
     
     
     
     
    Show all issues

    Do we need both this and setting headers= above in some classes?

  6. djblets/webapi/tests/test_api_token.py (Diff revision 2)
     
     
     
     
    Show all issues

    Let's alphabetize the keys.

  7. 
      
maubin
maubin
david
  1. 
      
  2. djblets/webapi/models.py (Diff revision 4)
     
     
    Show all issues

    Returns -> Return

  3. djblets/webapi/models.py (Diff revision 4)
     
     
    Show all issues

    Might as well update this to "Return" while we're here too.

  4. 
      
maubin
maubin
chipx86
  1. 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.

  2. Show all issues

    The final .0 isn't needed here.

  3. djblets/webapi/models.py (Diff revision 6)
     
     
    Show all issues

    Same here. This can be 3.1.

  4. 
      
maubin
maubin
Review request changed
Status:
Completed
Change Summary:
Pushed to release-3.x (c96db6a)