From df07ba713bbfde8990a90589444c0d6f34dde266 Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Sat, 25 May 2024 20:41:12 +0200 Subject: [PATCH 1/3] :sparkles: [#102] Add system check for OIDC_AUTHENTICATE_CLASS setting --- mozilla_django_oidc_db/apps.py | 3 ++ mozilla_django_oidc_db/checks.py | 48 ++++++++++++++++++++++++++++ setup.cfg | 3 ++ tests/test_checks.py | 54 ++++++++++++++++++++++++++++++++ 4 files changed, 108 insertions(+) create mode 100644 mozilla_django_oidc_db/checks.py create mode 100644 tests/test_checks.py diff --git a/mozilla_django_oidc_db/apps.py b/mozilla_django_oidc_db/apps.py index 1df4aa1..70bbb3c 100644 --- a/mozilla_django_oidc_db/apps.py +++ b/mozilla_django_oidc_db/apps.py @@ -4,3 +4,6 @@ class MozillaDjangoOidcDbConfig(AppConfig): name = "mozilla_django_oidc_db" default_auto_field = "django.db.models.AutoField" + + def ready(self) -> None: + from . import checks # noqa diff --git a/mozilla_django_oidc_db/checks.py b/mozilla_django_oidc_db/checks.py new file mode 100644 index 0000000..a11c19c --- /dev/null +++ b/mozilla_django_oidc_db/checks.py @@ -0,0 +1,48 @@ +import inspect +from collections.abc import Sequence + +from django.apps import AppConfig +from django.conf import settings +from django.core.checks import CheckMessage, Error, Warning, register +from django.utils.module_loading import import_string + +from .views import OIDCInit + + +@register() +def check_authenticate_class( + *, app_configs: Sequence[AppConfig] | None, **kwargs +) -> list[CheckMessage]: + if not ( + app_configs is None + or any(config.label == "mozilla_django_oidc_db" for config in app_configs) + ): + return [] + + dotted_path = settings.OIDC_AUTHENTICATE_CLASS + if not isinstance(dotted_path, str): + return [ + Error( + "'settings.OIDC_AUTHENTICATE_CLASS' must be a string that can be imported.", + hint=( + "Use 'mozilla_django_oidc_db.views.OIDCAuthenticationRequestView' or a " + "subclass of 'mozilla_django_oidc_db.views.OIDCInit'." + ), + id="mozilla_django_oidc_db.E001", + ) + ] + + view_cls = import_string(dotted_path) + if not inspect.isclass(view_cls) or not issubclass(view_cls, OIDCInit): + return [ + Warning( + "'settings.OIDC_AUTHENTICATE_CLASS' should be a subclass of 'OIDCInit'.", + hint=( + "Use 'mozilla_django_oidc_db.views.OIDCAuthenticationRequestView' or a " + "subclass of 'mozilla_django_oidc_db.views.OIDCInit'." + ), + id="mozilla_django_oidc_db.W001", + ) + ] + + return [] diff --git a/setup.cfg b/setup.cfg index 4710b63..42509ca 100644 --- a/setup.cfg +++ b/setup.cfg @@ -53,6 +53,9 @@ tests_require = isort black +[options.packages.find] +include = mozilla_django_oidc_db* + [options.extras_require] tests = psycopg2 diff --git a/tests/test_checks.py b/tests/test_checks.py new file mode 100644 index 0000000..da0a249 --- /dev/null +++ b/tests/test_checks.py @@ -0,0 +1,54 @@ +from django.apps import apps +from django.core.checks import Error, Warning + +import pytest + +from mozilla_django_oidc_db.checks import check_authenticate_class + + +@pytest.fixture(scope="session") +def app_configs(): + app_config = apps.get_app_config(app_label="mozilla_django_oidc_db") + return [app_config] + + +def test_check_authenticate_class_ok(app_configs, settings): + settings.OIDC_AUTHENTICATE_CLASS = ( + "mozilla_django_oidc_db.views.OIDCAuthenticationRequestView" + ) + + messages = check_authenticate_class(app_configs=app_configs) + + assert len(messages) == 0 + + +def test_check_authenticate_class_not_a_string(app_configs, settings): + settings.OIDC_AUTHENTICATE_CLASS = object() + + messages = check_authenticate_class(app_configs=app_configs) + + assert len(messages) == 1 + assert messages[0] == Error( + "'settings.OIDC_AUTHENTICATE_CLASS' must be a string that can be imported.", + hint=( + "Use 'mozilla_django_oidc_db.views.OIDCAuthenticationRequestView' or a " + "subclass of 'mozilla_django_oidc_db.views.OIDCInit'." + ), + id="mozilla_django_oidc_db.E001", + ) + + +def test_check_authenticate_class_invalid_view(app_configs, settings): + settings.OIDC_AUTHENTICATE_CLASS = "django.views.View" + + messages = check_authenticate_class(app_configs=app_configs) + + assert len(messages) == 1 + assert messages[0] == Warning( + "'settings.OIDC_AUTHENTICATE_CLASS' should be a subclass of 'OIDCInit'.", + hint=( + "Use 'mozilla_django_oidc_db.views.OIDCAuthenticationRequestView' or a " + "subclass of 'mozilla_django_oidc_db.views.OIDCInit'." + ), + id="mozilla_django_oidc_db.W001", + ) From 51831d7f08711de6a500cb5a0cd5c57d6011fa1d Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Sat, 25 May 2024 20:48:47 +0200 Subject: [PATCH 2/3] :sparkles: [#102] Add system check for OIDC_CALLBACK_CLASS setting --- mozilla_django_oidc_db/checks.py | 43 +++++++++++++++++++++++++-- tests/test_checks.py | 51 +++++++++++++++++++++++++++++++- 2 files changed, 91 insertions(+), 3 deletions(-) diff --git a/mozilla_django_oidc_db/checks.py b/mozilla_django_oidc_db/checks.py index a11c19c..6223933 100644 --- a/mozilla_django_oidc_db/checks.py +++ b/mozilla_django_oidc_db/checks.py @@ -6,7 +6,7 @@ from django.core.checks import CheckMessage, Error, Warning, register from django.utils.module_loading import import_string -from .views import OIDCInit +from .views import OIDCCallbackView, OIDCInit @register() @@ -15,7 +15,7 @@ def check_authenticate_class( ) -> list[CheckMessage]: if not ( app_configs is None - or any(config.label == "mozilla_django_oidc_db" for config in app_configs) + or any(config.name == "mozilla_django_oidc_db" for config in app_configs) ): return [] @@ -46,3 +46,42 @@ def check_authenticate_class( ] return [] + + +@register() +def check_callback_class( + *, app_configs: Sequence[AppConfig] | None, **kwargs +) -> list[CheckMessage]: + if not ( + app_configs is None + or any(config.name == "mozilla_django_oidc_db" for config in app_configs) + ): + return [] + + dotted_path = settings.OIDC_CALLBACK_CLASS + if not isinstance(dotted_path, str): + return [ + Error( + "'settings.OIDC_CALLBACK_CLASS' must be a string that can be imported.", + hint=( + "Use 'mozilla_django_oidc_db.views.OIDCCallbackView' or a " + "subclass of it." + ), + id="mozilla_django_oidc_db.E002", + ) + ] + + view_cls = import_string(dotted_path) + if not inspect.isclass(view_cls) or not issubclass(view_cls, OIDCCallbackView): + return [ + Warning( + "'settings.OIDC_CALLBACK_CLASS' should be a subclass of 'OIDCInit'.", + hint=( + "Use 'mozilla_django_oidc_db.views.OIDCCallbackView' or a " + "subclass of it." + ), + id="mozilla_django_oidc_db.W002", + ) + ] + + return [] diff --git a/tests/test_checks.py b/tests/test_checks.py index da0a249..d2f3493 100644 --- a/tests/test_checks.py +++ b/tests/test_checks.py @@ -3,7 +3,7 @@ import pytest -from mozilla_django_oidc_db.checks import check_authenticate_class +from mozilla_django_oidc_db.checks import check_authenticate_class, check_callback_class @pytest.fixture(scope="session") @@ -12,6 +12,15 @@ def app_configs(): return [app_config] +def test_nothing_reported_when_checking_other_app_label(settings): + settings.OIDC_AUTHENTICATE_CLASS = object() # this is invalid + app_config = apps.get_app_config(app_label="admin") + + messages = check_authenticate_class(app_configs=[app_config]) + + assert len(messages) == 0 + + def test_check_authenticate_class_ok(app_configs, settings): settings.OIDC_AUTHENTICATE_CLASS = ( "mozilla_django_oidc_db.views.OIDCAuthenticationRequestView" @@ -52,3 +61,43 @@ def test_check_authenticate_class_invalid_view(app_configs, settings): ), id="mozilla_django_oidc_db.W001", ) + + +def test_check_callback_class_ok(app_configs, settings): + settings.OIDC_CALLBACK_CLASS = "mozilla_django_oidc_db.views.OIDCCallbackView" + + messages = check_callback_class(app_configs=app_configs) + + assert len(messages) == 0 + + +def test_check_callback_class_not_a_string(app_configs, settings): + settings.OIDC_CALLBACK_CLASS = object() + + messages = check_callback_class(app_configs=app_configs) + + assert len(messages) == 1 + assert messages[0] == Error( + "'settings.OIDC_CALLBACK_CLASS' must be a string that can be imported.", + hint=( + "Use 'mozilla_django_oidc_db.views.OIDCCallbackView' or a " + "subclass of it." + ), + id="mozilla_django_oidc_db.E002", + ) + + +def test_check_callback_class_invalid_view(app_configs, settings): + settings.OIDC_CALLBACK_CLASS = "django.views.View" + + messages = check_callback_class(app_configs=app_configs) + + assert len(messages) == 1 + assert messages[0] == Warning( + "'settings.OIDC_CALLBACK_CLASS' should be a subclass of 'OIDCInit'.", + hint=( + "Use 'mozilla_django_oidc_db.views.OIDCCallbackView' or a " + "subclass of it." + ), + id="mozilla_django_oidc_db.W002", + ) From 9934fc8a3101c656996c8d628b124d1fa043da8f Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Sat, 25 May 2024 20:59:49 +0200 Subject: [PATCH 3/3] :art: [#99] Clean up check code --- mozilla_django_oidc_db/checks.py | 118 ++++++++++++++++--------------- testapp/models.py | 1 - 2 files changed, 62 insertions(+), 57 deletions(-) diff --git a/mozilla_django_oidc_db/checks.py b/mozilla_django_oidc_db/checks.py index 6223933..6967ab8 100644 --- a/mozilla_django_oidc_db/checks.py +++ b/mozilla_django_oidc_db/checks.py @@ -1,5 +1,6 @@ import inspect from collections.abc import Sequence +from typing import Any from django.apps import AppConfig from django.conf import settings @@ -9,9 +10,12 @@ from .views import OIDCCallbackView, OIDCInit -@register() -def check_authenticate_class( - *, app_configs: Sequence[AppConfig] | None, **kwargs +def _do_check( + app_configs: Sequence[AppConfig] | None, + dotted_path: Any, + type_error: Error, + subclass_reference: type, + subclass_warning: Warning, ) -> list[CheckMessage]: if not ( app_configs is None @@ -19,69 +23,71 @@ def check_authenticate_class( ): return [] - dotted_path = settings.OIDC_AUTHENTICATE_CLASS if not isinstance(dotted_path, str): - return [ - Error( - "'settings.OIDC_AUTHENTICATE_CLASS' must be a string that can be imported.", - hint=( - "Use 'mozilla_django_oidc_db.views.OIDCAuthenticationRequestView' or a " - "subclass of 'mozilla_django_oidc_db.views.OIDCInit'." - ), - id="mozilla_django_oidc_db.E001", - ) - ] + return [type_error] view_cls = import_string(dotted_path) - if not inspect.isclass(view_cls) or not issubclass(view_cls, OIDCInit): - return [ - Warning( - "'settings.OIDC_AUTHENTICATE_CLASS' should be a subclass of 'OIDCInit'.", - hint=( - "Use 'mozilla_django_oidc_db.views.OIDCAuthenticationRequestView' or a " - "subclass of 'mozilla_django_oidc_db.views.OIDCInit'." - ), - id="mozilla_django_oidc_db.W001", - ) - ] + if not inspect.isclass(view_cls) or not issubclass(view_cls, subclass_reference): + return [subclass_warning] return [] @register() -def check_callback_class( +def check_authenticate_class( *, app_configs: Sequence[AppConfig] | None, **kwargs ) -> list[CheckMessage]: - if not ( - app_configs is None - or any(config.name == "mozilla_django_oidc_db" for config in app_configs) - ): - return [] + type_error = Error( + "'settings.OIDC_AUTHENTICATE_CLASS' must be a string that can be imported.", + hint=( + "Use 'mozilla_django_oidc_db.views.OIDCAuthenticationRequestView' or a " + "subclass of 'mozilla_django_oidc_db.views.OIDCInit'." + ), + id="mozilla_django_oidc_db.E001", + ) + subclass_warning = Warning( + "'settings.OIDC_AUTHENTICATE_CLASS' should be a subclass of 'OIDCInit'.", + hint=( + "Use 'mozilla_django_oidc_db.views.OIDCAuthenticationRequestView' or a " + "subclass of 'mozilla_django_oidc_db.views.OIDCInit'." + ), + id="mozilla_django_oidc_db.W001", + ) - dotted_path = settings.OIDC_CALLBACK_CLASS - if not isinstance(dotted_path, str): - return [ - Error( - "'settings.OIDC_CALLBACK_CLASS' must be a string that can be imported.", - hint=( - "Use 'mozilla_django_oidc_db.views.OIDCCallbackView' or a " - "subclass of it." - ), - id="mozilla_django_oidc_db.E002", - ) - ] + return _do_check( + app_configs, + settings.OIDC_AUTHENTICATE_CLASS, + type_error=type_error, + subclass_reference=OIDCInit, + subclass_warning=subclass_warning, + ) - view_cls = import_string(dotted_path) - if not inspect.isclass(view_cls) or not issubclass(view_cls, OIDCCallbackView): - return [ - Warning( - "'settings.OIDC_CALLBACK_CLASS' should be a subclass of 'OIDCInit'.", - hint=( - "Use 'mozilla_django_oidc_db.views.OIDCCallbackView' or a " - "subclass of it." - ), - id="mozilla_django_oidc_db.W002", - ) - ] - return [] +@register() +def check_callback_class( + *, app_configs: Sequence[AppConfig] | None, **kwargs +) -> list[CheckMessage]: + type_error = Error( + "'settings.OIDC_CALLBACK_CLASS' must be a string that can be imported.", + hint=( + "Use 'mozilla_django_oidc_db.views.OIDCCallbackView' or a " + "subclass of it." + ), + id="mozilla_django_oidc_db.E002", + ) + subclass_warning = Warning( + "'settings.OIDC_CALLBACK_CLASS' should be a subclass of 'OIDCInit'.", + hint=( + "Use 'mozilla_django_oidc_db.views.OIDCCallbackView' or a " + "subclass of it." + ), + id="mozilla_django_oidc_db.W002", + ) + + return _do_check( + app_configs, + settings.OIDC_CALLBACK_CLASS, + type_error=type_error, + subclass_reference=OIDCCallbackView, + subclass_warning=subclass_warning, + ) diff --git a/testapp/models.py b/testapp/models.py index 5604179..a224020 100644 --- a/testapp/models.py +++ b/testapp/models.py @@ -9,7 +9,6 @@ class EmptyConfig(OpenIDConnectConfigBase): class WrongConfigModel(SingletonModel): - @property def oidc_op_authorization_endpoint(self): return "bad"