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

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

Information

Review Board
master

Reviewers

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

Description From Last Updated

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

chipx86chipx86

'django_reset' imported but unused

reviewbotreviewbot

'from settings_local import *' used; unable to detect undefined names

reviewbotreviewbot

So we can't actually do this for a few reasons: 1) It's not safe to import things like API resource …

chipx86chipx86

Col: 37 W292 no newline at end of file

reviewbotreviewbot

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

chipx86chipx86

'AccessToken' imported but unused

reviewbotreviewbot

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

chipx86chipx86

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

reviewbotreviewbot

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

chipx86chipx86

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

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

'django_reset' imported but unused

reviewbotreviewbot

'from settings_local import *' used; unable to detect undefined names

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot
reviewbot
  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)
     
     
    Show all issues
     'django_reset' imported but unused
    
  3. reviewboard/settings.py (Diff revision 1)
     
     
    Show all issues
     'from settings_local import *' used; unable to detect undefined names
    
  4. reviewboard/settings.py (Diff revision 1)
     
     
    Show all issues
    Col: 37
     W292 no newline at end of file
    
  5. reviewboard/webapi/base.py (Diff revision 1)
     
     
    Show all issues
     'AccessToken' imported but unused
    
  6. reviewboard/webapi/base.py (Diff revision 1)
     
     
    Show all issues
    Col: 80
     E501 line too long (82 > 79 characters)
    
  7. 
      
chipx86
  1. 
      
  2. reviewboard/settings.py (Diff revision 1)
     
     
    Show all issues

    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)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
    Show all issues

    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)
     
     
     
     
     
    Show all issues

    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)
     
     
     
     
    Show all issues

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

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

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

  7. 
      
LE
reviewbot
  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. Show all issues
    Col: 80
     E501 line too long (80 > 79 characters)
    
  3. Show all issues
    Col: 80
     E501 line too long (80 > 79 characters)
    
  4. Show all issues
    Col: 80
     E501 line too long (80 > 79 characters)
    
  5. Show all issues
    Col: 80
     E501 line too long (80 > 79 characters)
    
  6. reviewboard/settings.py (Diff revision 2)
     
     
    Show all issues
     'django_reset' imported but unused
    
  7. reviewboard/settings.py (Diff revision 2)
     
     
    Show all issues
     'from settings_local import *' used; unable to detect undefined names
    
  8. reviewboard/settings.py (Diff revision 2)
     
     
    Show all issues
    Col: 80
     E501 line too long (80 > 79 characters)
    
  9. reviewboard/settings.py (Diff revision 2)
     
     
    Show all issues
    Col: 80
     E501 line too long (80 > 79 characters)
    
  10. 
      
brennie
Review request changed
Status:
Discarded