Skip to content

Commit

Permalink
Make OIDC support optional
Browse files Browse the repository at this point in the history
Make OIDC support optional by not requiring OIDC_RSA_PRIVATE_KEY to be
set in the settings, and using the standard oauthlib.oauth2.Server class
when an OIDC private key is not configured.

Add a test fixture wrapping oauth2_settings. This allows individual
tests / test suites to override oauth2 settings and have them reset at
the end of the test. This avoids configuration leaking from one test to
another, and allows us to test multiple different configurations in one
test run.

When using the oauth2_settings fixture, allow configuration for the test
case to be loaded from a pytest marker called oauth2_settings.

Split out OIDC specific tests requiring specific OIDC configuration into
separate TestCase.

Adjust the OAuthLibMixin to fallback to using the server, validator and
core classes specified in oauth2_settings when not hardcoded in to the
class. These classes can still be specified as hard-coded attributes in
sub-classes, but it's no longer required if you just want what is
configured in oauth2_settings, so remove all attributes that are just
pointing at the configuration anyway.

Add a setting ALWAYS_RELOAD_OAUTHLIB_CORE, which causes OAuthLibMixin to
reload the OAuthLibCore object on each request. This is only intended to
be used during testing, to allow the views to recognise changes in
configuration.

Show missing coverage lines in the coverage report.

Fixes: jazzband#873
  • Loading branch information
tevansuk committed Jan 25, 2021
1 parent b05e0c2 commit 183b02a
Show file tree
Hide file tree
Showing 27 changed files with 478 additions and 422 deletions.
19 changes: 15 additions & 4 deletions oauth2_provider/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@
"ACCESS_TOKEN_GENERATOR": None,
"REFRESH_TOKEN_GENERATOR": None,
"EXTRA_SERVER_KWARGS": {},
"OAUTH2_SERVER_CLASS": "oauthlib.openid.connect.core.endpoints.pre_configured.Server",
"OAUTH2_SERVER_CLASS": "oauthlib.oauth2.Server",
"OIDC_SERVER_CLASS": "oauthlib.openid.connect.core.endpoints.pre_configured.Server",
"OAUTH2_VALIDATOR_CLASS": "oauth2_provider.oauth2_validators.OAuth2Validator",
"OAUTH2_BACKEND_CLASS": "oauth2_provider.oauth2_backends.OAuthLibCore",
"SCOPES": {"read": "Reading scope", "write": "Writing scope"},
Expand Down Expand Up @@ -92,6 +93,9 @@
"RESOURCE_SERVER_TOKEN_CACHING_SECONDS": 36000,
# Whether or not PKCE is required
"PKCE_REQUIRED": False,
# Whether to re-create OAuthlibCore on every request.
# Should only be required in testing.
"ALWAYS_RELOAD_OAUTHLIB_CORE": False,
}

# List of settings that cannot be empty
Expand All @@ -103,7 +107,6 @@
"OAUTH2_BACKEND_CLASS",
"SCOPES",
"ALLOWED_REDIRECT_URI_SCHEMES",
"OIDC_RSA_PRIVATE_KEY",
"OIDC_RESPONSE_TYPES_SUPPORTED",
"OIDC_SUBJECT_TYPES_SUPPORTED",
"OIDC_ID_TOKEN_SIGNING_ALG_VALUES_SUPPORTED",
Expand Down Expand Up @@ -182,13 +185,17 @@ def user_settings(self):
def __getattr__(self, attr):
if attr not in self.defaults:
raise AttributeError("Invalid OAuth2Provider setting: %s" % attr)

try:
# Check if present in user settings
val = self.user_settings[attr]
except KeyError:
# Fall back to defaults
val = self.defaults[attr]
# Special case OAUTH2_SERVER_CLASS - if not specified, and OIDC is
# enabled, use the OIDC_SERVER_CLASS setting instead
if attr == "OAUTH2_SERVER_CLASS" and self.is_oidc_enabled:
val = self.defaults["OIDC_SERVER_CLASS"]
else:
val = self.defaults[attr]

# Coerce import strings into classes
if val and attr in self.import_strings:
Expand Down Expand Up @@ -254,6 +261,10 @@ def reload(self):
if hasattr(self, "_user_settings"):
delattr(self, "_user_settings")

@property
def is_oidc_enabled(self):
return bool(self.OIDC_RSA_PRIVATE_KEY)


oauth2_settings = OAuth2ProviderSettings(USER_SETTINGS, DEFAULTS, IMPORT_STRINGS, MANDATORY)

Expand Down
12 changes: 0 additions & 12 deletions oauth2_provider/views/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,6 @@ class AuthorizationView(BaseAuthorizationView, FormView):
template_name = "oauth2_provider/authorize.html"
form_class = AllowForm

server_class = oauth2_settings.OAUTH2_SERVER_CLASS
validator_class = oauth2_settings.OAUTH2_VALIDATOR_CLASS
oauthlib_backend_class = oauth2_settings.OAUTH2_BACKEND_CLASS

skip_authorization_completely = False

def get_initial(self):
Expand Down Expand Up @@ -267,10 +263,6 @@ class TokenView(OAuthLibMixin, View):
* Client credentials
"""

server_class = oauth2_settings.OAUTH2_SERVER_CLASS
validator_class = oauth2_settings.OAUTH2_VALIDATOR_CLASS
oauthlib_backend_class = oauth2_settings.OAUTH2_BACKEND_CLASS

@method_decorator(sensitive_post_parameters("password"))
def post(self, request, *args, **kwargs):
url, headers, body, status = self.create_token_response(request)
Expand All @@ -292,10 +284,6 @@ class RevokeTokenView(OAuthLibMixin, View):
Implements an endpoint to revoke access or refresh tokens
"""

server_class = oauth2_settings.OAUTH2_SERVER_CLASS
validator_class = oauth2_settings.OAUTH2_VALIDATOR_CLASS
oauthlib_backend_class = oauth2_settings.OAUTH2_BACKEND_CLASS

def post(self, request, *args, **kwargs):
url, headers, body, status = self.create_revocation_response(request)
response = HttpResponse(content=body or "", status=status)
Expand Down
14 changes: 2 additions & 12 deletions oauth2_provider/views/generic.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
from django.views.generic import View

from ..settings import oauth2_settings
from .mixins import (
ClientProtectedResourceMixin,
OAuthLibMixin,
Expand All @@ -10,16 +9,7 @@
)


class InitializationMixin(OAuthLibMixin):

"""Initializer for OauthLibMixin"""

server_class = oauth2_settings.OAUTH2_SERVER_CLASS
validator_class = oauth2_settings.OAUTH2_VALIDATOR_CLASS
oauthlib_backend_class = oauth2_settings.OAUTH2_BACKEND_CLASS


class ProtectedResourceView(ProtectedResourceMixin, InitializationMixin, View):
class ProtectedResourceView(ProtectedResourceMixin, OAuthLibMixin, View):
"""
Generic view protecting resources by providing OAuth2 authentication out of the box
"""
Expand All @@ -45,7 +35,7 @@ class ReadWriteScopedResourceView(ReadWriteScopedResourceMixin, ProtectedResourc
pass


class ClientProtectedResourceView(ClientProtectedResourceMixin, InitializationMixin, View):
class ClientProtectedResourceView(ClientProtectedResourceMixin, OAuthLibMixin, View):

"""View for protecting a resource with client-credentials method.
This involves allowing access tokens, Basic Auth and plain credentials in request body.
Expand Down
21 changes: 8 additions & 13 deletions oauth2_provider/views/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ class OAuthLibMixin:
* validator_class
* oauthlib_backend_class
If these class variables are not set, it will fall back to using the classes
specified in oauth2_settings (OAUTH2_SERVER_CLASS, OAUTH2_VALIDATOR_CLASS
and OAUTH2_BACKEND_CLASS).
"""

server_class = None
Expand All @@ -37,10 +40,7 @@ def get_server_class(cls):
Return the OAuthlib server class to use
"""
if cls.server_class is None:
raise ImproperlyConfigured(
"OAuthLibMixin requires either a definition of 'server_class'"
" or an implementation of 'get_server_class()'"
)
return oauth2_settings.OAUTH2_SERVER_CLASS
else:
return cls.server_class

Expand All @@ -50,10 +50,7 @@ def get_validator_class(cls):
Return the RequestValidator implementation class to use
"""
if cls.validator_class is None:
raise ImproperlyConfigured(
"OAuthLibMixin requires either a definition of 'validator_class'"
" or an implementation of 'get_validator_class()'"
)
return oauth2_settings.OAUTH2_VALIDATOR_CLASS
else:
return cls.validator_class

Expand All @@ -63,10 +60,7 @@ def get_oauthlib_backend_class(cls):
Return the OAuthLibCore implementation class to use
"""
if cls.oauthlib_backend_class is None:
raise ImproperlyConfigured(
"OAuthLibMixin requires either a definition of 'oauthlib_backend_class'"
" or an implementation of 'get_oauthlib_backend_class()'"
)
return oauth2_settings.OAUTH2_BACKEND_CLASS
else:
return cls.oauthlib_backend_class

Expand All @@ -85,8 +79,9 @@ def get_server(cls):
def get_oauthlib_core(cls):
"""
Cache and return `OAuthlibCore` instance so it will be created only on first request
unless ALWAYS_RELOAD_OAUTHLIB_CORE is True.
"""
if not hasattr(cls, "_oauthlib_core"):
if not hasattr(cls, "_oauthlib_core") or oauth2_settings.ALWAYS_RELOAD_OAUTHLIB_CORE:
server = cls.get_server()
core_class = cls.get_oauthlib_backend_class()
cls._oauthlib_core = core_class(server)
Expand Down
4 changes: 0 additions & 4 deletions oauth2_provider/views/oidc.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,6 @@ class UserInfoView(OAuthLibMixin, View):
View used to show Claims about the authenticated End-User
"""

server_class = oauth2_settings.OAUTH2_SERVER_CLASS
validator_class = oauth2_settings.OAUTH2_VALIDATOR_CLASS
oauthlib_backend_class = oauth2_settings.OAUTH2_BACKEND_CLASS

def get(self, request, *args, **kwargs):
url, headers, body, status = self.create_userinfo_response(request)
response = HttpResponse(content=body or "", status=status)
Expand Down
67 changes: 67 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import pytest
from django.conf import settings as test_settings
from jwcrypto import jwk

from oauth2_provider.settings import oauth2_settings as _oauth2_settings


class OAuthSettingsWrapper:
"""
A wrapper around oauth2_settings to ensure that when an overridden value is
set, it also records it in _cached_attrs, so that the settings can be reset.
"""

def __init__(self, settings, user_settings):
if user_settings:
settings.OAUTH2_PROVIDER = user_settings
_oauth2_settings.reload()
self.settings = settings
# Reload OAuthlibCore for every view request during tests
self.ALWAYS_RELOAD_OAUTHLIB_CORE = True

def __setattr__(self, attr, value):
setattr(_oauth2_settings, attr, value)
_oauth2_settings._cached_attrs.add(attr)

def __delattr__(self, attr):
delattr(_oauth2_settings, attr)
if attr in _oauth2_settings._cached_attrs:
_oauth2_settings._cached_attrs.remove(attr)

def __getattr__(self, attr):
return getattr(_oauth2_settings, attr)

def finalize(self):
self.settings.finalize()
_oauth2_settings.reload()


@pytest.fixture
def oauth2_settings(request, settings):
"""
A fixture that provides a simple way to override OAUTH2_PROVIDER settings.
It can be used two ways - either setting things on the fly, or by reading
configuration data from the pytest marker oauth2_settings.
If used on a standard pytest function, you can use argument dependency
injection to get the wrapper. If used on a unittest.TestCase, the wrapper
is made available on the class instance, as `oauth2_settings`.
Anything overridden will be restored at the end of the test case, ensuring
that there is no configuration leakage between test cases.
"""
marker = request.node.get_closest_marker("oauth2_settings")
user_settings = {}
if marker is not None:
user_settings = marker.args[0]
wrapper = OAuthSettingsWrapper(settings, user_settings)
if request.instance is not None:
request.instance.oauth2_settings = wrapper
yield wrapper
wrapper.finalize()


@pytest.fixture(scope="class")
def oidc_key(request):
request.cls.key = jwk.JWK.from_pem(test_settings.OIDC_RSA_PRIVATE_KEY.encode("utf8"))
42 changes: 42 additions & 0 deletions tests/presets.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
from copy import deepcopy

from django.conf import settings


# A set of OAUTH2_PROVIDER settings dicts that can be used in tests

DEFAULT_SCOPES_RW = {"DEFAULT_SCOPES": ["read", "write"]}
DEFAULT_SCOPES_RO = {"DEFAULT_SCOPES": ["read"]}
OIDC_SETTINGS_RW = {
"OIDC_ISS_ENDPOINT": "http://localhost",
"OIDC_USERINFO_ENDPOINT": "http://localhost/userinfo/",
"OIDC_RSA_PRIVATE_KEY": settings.OIDC_RSA_PRIVATE_KEY,
"SCOPES": {
"read": "Reading scope",
"write": "Writing scope",
"openid": "OpenID connect",
},
"DEFAULT_SCOPES": ["read", "write"],
}
OIDC_SETTINGS_RO = deepcopy(OIDC_SETTINGS_RW)
OIDC_SETTINGS_RO["DEFAULT_SCOPES"] = ["read"]
REST_FRAMEWORK_SCOPES = {
"SCOPES": {
"read": "Read scope",
"write": "Write scope",
"scope1": "Scope 1",
"scope2": "Scope 2",
"resource1": "Resource 1",
},
}
INTROSPECTION_SETTINGS = {
"SCOPES": {
"read": "Read scope",
"write": "Write scope",
"introspection": "Introspection scope",
"dolphin": "eek eek eek scope",
},
"RESOURCE_SERVER_INTROSPECTION_URL": "http://example.org/introspection",
"READ_SCOPE": "read",
"WRITE_SCOPE": "write",
}
6 changes: 0 additions & 6 deletions tests/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,12 +134,6 @@
dTnvCVtA59ne4LEVie/PMH/odQWY0SxVm/76uBZv/1vY
-----END RSA PRIVATE KEY-----"""

OAUTH2_PROVIDER = {
"OIDC_ISS_ENDPOINT": "http://localhost",
"OIDC_USERINFO_ENDPOINT": "http://localhost/userinfo/",
"OIDC_RSA_PRIVATE_KEY": OIDC_RSA_PRIVATE_KEY,
}

OAUTH2_PROVIDER_ACCESS_TOKEN_MODEL = "oauth2_provider.AccessToken"
OAUTH2_PROVIDER_APPLICATION_MODEL = "oauth2_provider.Application"
OAUTH2_PROVIDER_REFRESH_TOKEN_MODEL = "oauth2_provider.RefreshToken"
Expand Down
8 changes: 3 additions & 5 deletions tests/test_application_views.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import pytest
from django.contrib.auth import get_user_model
from django.test import TestCase
from django.urls import reverse

from oauth2_provider.models import get_application_model
from oauth2_provider.settings import oauth2_settings
from oauth2_provider.views.application import ApplicationRegistration

from .models import SampleApplication
Expand All @@ -23,21 +23,19 @@ def tearDown(self):
self.bar_user.delete()


@pytest.mark.usefixtures("oauth2_settings")
class TestApplicationRegistrationView(BaseTest):
@pytest.mark.oauth2_settings({"APPLICATION_MODEL": "tests.SampleApplication"})
def test_get_form_class(self):
"""
Tests that the form class returned by the "get_form_class" method is
bound to custom application model defined in the
"OAUTH2_PROVIDER_APPLICATION_MODEL" setting.
"""
# Patch oauth2 settings to use a custom Application model
oauth2_settings.APPLICATION_MODEL = "tests.SampleApplication"
# Create a registration view and tests that the model form is bound
# to the custom Application model
application_form_class = ApplicationRegistration().get_form_class()
self.assertEqual(SampleApplication, application_form_class._meta.model)
# Revert oauth2 settings
oauth2_settings.APPLICATION_MODEL = "oauth2_provider.Application"

def test_application_registration_user(self):
self.client.login(username="foo_user", password="123456")
Expand Down
Loading

0 comments on commit 183b02a

Please sign in to comment.