[OAuth2Provider][WIP] Enabling OAuth2 authorization on Review Board Web API

Review Request #7996 - Created Feb. 25, 2016 and discarded

Minh Le Hoang
Review Board
master
7941, 7997
7997, 8095
reviewboard
brennie, chipx86

Prior to this commit. A user can manage list of all OAuth2 client applications
and OAuth2 tokens but there is no mechanism to authorize resources using OAuth2
protocol. In this commit, OAuth2 authorization mechanism is added to
the current authorization mechanism for Review Board web API resources.
Services now can follow OAuth2 protocol, send a request with appropriate
authorization header to work with Review Board's resources. All the scopes
are loaded at RB start time, and external services can request a token with
specify scope through OAuth2 protocol. When using Review Board's API resource,
the scope of a token is checked to make sure that the external service has
the right permission the resource.

Since Django OAuth toolkit does not support dynamic scope, modifications
are needed. OAuth2Validator.validate_scopes now always use the dynamic
dict of scopes for scope validation. OAUTH2_PROVIDER['SCOPES'] is a
dynamic dict of scopes. OAuth2ProviderSettings does not cache any scope related
information and always provive most up to date scope information.

Manual test. Missing unit tests. I will be added latter

  • 6
  • 0
  • 8
  • 4
  • 18
Description From Last Updated
Col: 80 E501 line too long (80 > 79 characters) Review Bot Review Bot
Col: 80 E501 line too long (80 > 79 characters) Review Bot Review Bot
Col: 80 E501 line too long (80 > 79 characters) Review Bot Review Bot
Col: 80 E501 line too long (80 > 79 characters) Review Bot Review Bot
Col: 80 E501 line too long (80 > 79 characters) Review Bot Review Bot
Col: 80 E501 line too long (80 > 79 characters) Review Bot Review Bot
Review Bot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/admin/siteconfig.py
        reviewboard/webapi/base.py
        reviewboard/webapi/auth_backends.py
        reviewboard/webapi/resources/oauth2_token.py
        reviewboard/settings.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/admin/siteconfig.py
        reviewboard/webapi/base.py
        reviewboard/webapi/auth_backends.py
        reviewboard/webapi/resources/oauth2_token.py
        reviewboard/settings.py
    
    
  2. reviewboard/settings.py (Diff revision 1)
     
     
     'django_reset' imported but unused
    
  3. reviewboard/settings.py (Diff revision 1)
     
     
     'from settings_local import *' used; unable to detect undefined names
    
  4. reviewboard/settings.py (Diff revision 1)
     
     
    Col: 37
     W292 no newline at end of file
    
  5. reviewboard/webapi/base.py (Diff revision 1)
     
     
     'AccessToken' imported but unused
    
  6. reviewboard/webapi/base.py (Diff revision 1)
     
     
    Col: 80
     E501 line too long (82 > 79 characters)
    
  7. 
      
Christian Hammond
  1. 
      
  2. reviewboard/settings.py (Diff revision 1)
     
     

    We can't actually import this here, or certain things will break due to circular references/initialization orders. I'll go into this more in the next comment.

  3. reviewboard/settings.py (Diff revision 1)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     

    So we can't actually do this for a few reasons:

    1) It's not safe to import things like API resource code within settings.py.
    2) Iterating here will lead to slowdown at startup.
    3) API is dynamic. If an extension is enabled, and it provides API, we'll have new possible scopes. That needs to be taken into consideration.

    What we need to do instead is to not have a hard-coded list of scopes, and instead be able to check scopes dynamically as needed.

    I don't recall the details exactly, but when we first investigated OAuth2 support, we determined this was doable, but it would require additional work. It just depends on the parts of oauth2_provider that we access.

    A lot of what oauth2_provider provides (mixins for access control on views, decorators) checks against SCOPES, but if we don't use them, or we provide our own versions, we can do a better job of this. Since we're really just trying to protect the API (and disallow access to non-API), we only care about access control at the API level. So, any code that we'd have to execute that would check against SCOPES can instead check against the policy_id each resource has.

    We can talk about this more on Slack.

  4. reviewboard/webapi/auth_backends.py (Diff revision 1)
     
     
     
     
     

    This should be part of the same import group, in alphabetical order, since these are all outside modules.

  5. reviewboard/webapi/base.py (Diff revision 1)
     
     
     
     

    These should all be part of the same improt group, in alphabetical order.

  6. reviewboard/webapi/base.py (Diff revision 1)
     
     

    Is WebAPIToken what we want? I thought we'd be using something different.

  7. 
      
Minh Le Hoang
Review Bot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/admin/siteconfig.py
        reviewboard/webapi/base.py
        reviewboard/settings.py
        reviewboard/oauth2/dynamic_scope_dictionary.py
        reviewboard/webapi/auth_backends.py
        reviewboard/oauth2/dynamic_scope_patcher.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/admin/siteconfig.py
        reviewboard/webapi/base.py
        reviewboard/settings.py
        reviewboard/oauth2/dynamic_scope_dictionary.py
        reviewboard/webapi/auth_backends.py
        reviewboard/oauth2/dynamic_scope_patcher.py
    
    
  2. Col: 80
     E501 line too long (80 > 79 characters)
    
  3. Col: 80
     E501 line too long (80 > 79 characters)
    
  4. Col: 80
     E501 line too long (80 > 79 characters)
    
  5. Col: 80
     E501 line too long (80 > 79 characters)
    
  6. reviewboard/settings.py (Diff revision 2)
     
     
     'django_reset' imported but unused
    
  7. reviewboard/settings.py (Diff revision 2)
     
     
     'from settings_local import *' used; unable to detect undefined names
    
  8. reviewboard/settings.py (Diff revision 2)
     
     
    Col: 80
     E501 line too long (80 > 79 characters)
    
  9. reviewboard/settings.py (Diff revision 2)
     
     
    Col: 80
     E501 line too long (80 > 79 characters)
    
  10. 
      
Barret Rennie
Review request changed

Status: Discarded

Loading...