• 
      

    Add API token support to the Djblets web API infrastructure.

    Review Request #7475 — Created July 2, 2015 and submitted

    Information

    Djblets
    release-0.9.x
    9a80f73...

    Reviewers

    Review Board 2.5 introduced support for using API tokens for
    authentication, but this was limited to Review Board. This change ports
    over that logic into Djblets, so that other applications can make use of
    the same API token capabilities.

    This consists of backends for authentication, mixins for WebAPIResource
    subclasses, an abstract model that projects can subclass for storing API
    token data, policy validation and enforcement, and a model manager.

    There are also unit tests for API token policy validation, plus a new
    assertion function for helping test ValidationError messages.

    Unit tests for both Djblets and Review Board pass (with the API token code
    ripped out of Review Board).

    Description From Last Updated

    undefined name 'WebAPIToken'

    reviewbot reviewbot

    Col: 1 W391 blank line at end of file

    reviewbot reviewbot

    Col: 80 E501 line too long (82 > 79 characters)

    reviewbot reviewbot

    Col: 80 E501 line too long (82 > 79 characters)

    reviewbot reviewbot

    Col: 80 E501 line too long (80 > 79 characters)

    reviewbot reviewbot

    Col: 80 E501 line too long (80 > 79 characters)

    reviewbot reviewbot

    The key that gets passed in to sha1 needs to be bytes, not unicode. I'd suggest assembling the key and …

    david david

    Add a period at the end?

    david david

    Can we swap the single and double quotes in this string?

    david david

    Same here?

    david david

    Same here?

    david david

    And here?

    david david

    And here?

    david david

    And here? I see more below but I'm not going to add comments for them.

    david david

    Docstring?

    david david

    This should probably be self.api_token_model.DoesNotExist.

    brennie brennie

    You don't need pass with a docstring.

    brennie brennie

    if not policy ?

    brennie brennie

    I think this should use six.string_types instead of basestring.

    david david

    'ObjectDoesNotExist' imported but unused

    reviewbot reviewbot
    reviewbot
    1. Tool: Pyflakes
      Processed Files:
          djblets/webapi/auth/backends/api_tokens.py
          djblets/webapi/resources/mixins/api_tokens.py
          djblets/webapi/errors.py
          djblets/webapi/models.py
          djblets/webapi/tests/test_api_policy.py
          djblets/webapi/managers.py
      
      Ignored Files:
          djblets/webapi/resources/mixins/__init__.py
          docs/djblets/coderef/index.rst
      
      
      
      Tool: PEP8 Style Checker
      Processed Files:
          djblets/webapi/auth/backends/api_tokens.py
          djblets/webapi/resources/mixins/api_tokens.py
          djblets/webapi/errors.py
          djblets/webapi/models.py
          djblets/webapi/tests/test_api_policy.py
          djblets/webapi/managers.py
      
      Ignored Files:
          djblets/webapi/resources/mixins/__init__.py
          docs/djblets/coderef/index.rst
      
      
    2. Show all issues
       undefined name 'WebAPIToken'
      
    3. Show all issues
      Col: 1
       W391 blank line at end of file
      
    4. djblets/webapi/tests/test_api_policy.py (Diff revision 1)
       
       
      Show all issues
      Col: 80
       E501 line too long (82 > 79 characters)
      
    5. djblets/webapi/tests/test_api_policy.py (Diff revision 1)
       
       
      Show all issues
      Col: 80
       E501 line too long (82 > 79 characters)
      
    6. djblets/webapi/tests/test_api_policy.py (Diff revision 1)
       
       
      Show all issues
      Col: 80
       E501 line too long (80 > 79 characters)
      
    7. djblets/webapi/tests/test_api_policy.py (Diff revision 1)
       
       
      Show all issues
      Col: 80
       E501 line too long (80 > 79 characters)
      
    8. 
        
    chipx86
    reviewbot
    1. Tool: PEP8 Style Checker
      Processed Files:
          djblets/webapi/auth/backends/api_tokens.py
          djblets/webapi/resources/mixins/api_tokens.py
          djblets/webapi/errors.py
          djblets/webapi/models.py
          djblets/webapi/tests/test_api_policy.py
          djblets/webapi/managers.py
      
      Ignored Files:
          djblets/webapi/resources/mixins/__init__.py
          docs/djblets/coderef/index.rst
      
      
      
      Tool: Pyflakes
      Processed Files:
          djblets/webapi/auth/backends/api_tokens.py
          djblets/webapi/resources/mixins/api_tokens.py
          djblets/webapi/errors.py
          djblets/webapi/models.py
          djblets/webapi/tests/test_api_policy.py
          djblets/webapi/managers.py
      
      Ignored Files:
          djblets/webapi/resources/mixins/__init__.py
          docs/djblets/coderef/index.rst
      
      
    2. 
        
    david
    1. 
        
    2. djblets/webapi/managers.py (Diff revision 2)
       
       
       
      Show all issues

      The key that gets passed in to sha1 needs to be bytes, not unicode. I'd suggest assembling the key and then calling .encode('utf-8').

      1. This code is all directly copied from Review Board, and seems to be working?

      2. >>> hashlib.sha1(u'abc').hexdigest()
        'a9993e364706816aba3e25717850c26c9cd0d89d'
        
        >>> hashlib.sha1('abc').hexdigest()
        'a9993e364706816aba3e25717850c26c9cd0d89d'
        
      3. I think that's because Python 2 will coerce it. Python 3 won't.

    3. djblets/webapi/managers.py (Diff revision 2)
       
       
      Show all issues

      Add a period at the end?

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

      Can we swap the single and double quotes in this string?

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

      Same here?

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

      Same here?

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

      And here?

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

      And here?

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

      And here?

      I see more below but I'm not going to add comments for them.

    10. Show all issues

      Docstring?

    11. 
        
    chipx86
    reviewbot
    1. Tool: Pyflakes
      Processed Files:
          djblets/webapi/auth/backends/api_tokens.py
          djblets/webapi/resources/mixins/api_tokens.py
          djblets/webapi/tests/test_api_policy.py
          djblets/testing/testcases.py
          djblets/webapi/models.py
          djblets/webapi/errors.py
          djblets/webapi/managers.py
      
      Ignored Files:
          djblets/webapi/resources/mixins/__init__.py
          docs/djblets/coderef/index.rst
      
      
      
      Tool: PEP8 Style Checker
      Processed Files:
          djblets/webapi/auth/backends/api_tokens.py
          djblets/webapi/resources/mixins/api_tokens.py
          djblets/webapi/tests/test_api_policy.py
          djblets/testing/testcases.py
          djblets/webapi/models.py
          djblets/webapi/errors.py
          djblets/webapi/managers.py
      
      Ignored Files:
          djblets/webapi/resources/mixins/__init__.py
          docs/djblets/coderef/index.rst
      
      
    2. 
        
    brennie
    1. 
        
    2. Show all issues

      This should probably be self.api_token_model.DoesNotExist.

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

      You don't need pass with a docstring.

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

      if not policy ?

    5. 
        
    chipx86
    reviewbot
    1. Tool: Pyflakes
      Processed Files:
          djblets/webapi/auth/backends/api_tokens.py
          djblets/webapi/resources/mixins/api_tokens.py
          djblets/webapi/tests/test_api_policy.py
          djblets/testing/testcases.py
          djblets/webapi/models.py
          djblets/webapi/errors.py
          djblets/webapi/managers.py
      
      Ignored Files:
          djblets/webapi/resources/mixins/__init__.py
          docs/djblets/coderef/index.rst
      
      
      
      Tool: PEP8 Style Checker
      Processed Files:
          djblets/webapi/auth/backends/api_tokens.py
          djblets/webapi/resources/mixins/api_tokens.py
          djblets/webapi/tests/test_api_policy.py
          djblets/testing/testcases.py
          djblets/webapi/models.py
          djblets/webapi/errors.py
          djblets/webapi/managers.py
      
      Ignored Files:
          djblets/webapi/resources/mixins/__init__.py
          docs/djblets/coderef/index.rst
      
      
    2. Show all issues
       'ObjectDoesNotExist' imported but unused
      
    3. 
        
    david
    1. 
        
    2. djblets/testing/testcases.py (Diff revision 4)
       
       
      Show all issues

      I think this should use six.string_types instead of basestring.

    3. 
        
    chipx86
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to release-0.9.x (8cc58e8)