diff --git a/allianceauth/authentication/decorators.py b/allianceauth/authentication/decorators.py index 88742fd7..6a5d0b91 100644 --- a/allianceauth/authentication/decorators.py +++ b/allianceauth/authentication/decorators.py @@ -1,18 +1,28 @@ -from django.conf.urls import include -from django.contrib.auth.decorators import user_passes_test -from django.core.exceptions import PermissionDenied from functools import wraps -from django.shortcuts import redirect +from typing import Callable, Iterable, Optional + +from django.conf.urls import include from django.contrib import messages +from django.contrib.auth.decorators import login_required, user_passes_test +from django.core.exceptions import PermissionDenied +from django.shortcuts import redirect from django.utils.translation import gettext_lazy as _ -from django.contrib.auth.decorators import login_required def user_has_main_character(user): return bool(user.profile.main_character) -def decorate_url_patterns(urls, decorator): +def decorate_url_patterns( + urls, decorator: Callable, excluded_views: Optional[Iterable] = None +): + """Decorate views given in url patterns except when they are explicitly excluded. + + Args: + - urls: Django URL patterns + - decorator: Decorator to be added to each view + - exclude_views: Optional iterable of view names to be excluded + """ url_list, app_name, namespace = include(urls) def process_patterns(url_patterns): @@ -22,6 +32,8 @@ def decorate_url_patterns(urls, decorator): process_patterns(pattern.url_patterns) else: # this is a pattern + if excluded_views and pattern.lookup_str in excluded_views: + return pattern.callback = decorator(pattern.callback) process_patterns(url_list) diff --git a/allianceauth/authentication/tests/test_decorators.py b/allianceauth/authentication/tests/test_decorators.py index 69c3949e..5b729d01 100644 --- a/allianceauth/authentication/tests/test_decorators.py +++ b/allianceauth/authentication/tests/test_decorators.py @@ -4,16 +4,16 @@ from urllib import parse from django.conf import settings from django.contrib.auth.models import AnonymousUser from django.http.response import HttpResponse -from django.shortcuts import reverse from django.test import TestCase from django.test.client import RequestFactory +from django.urls import reverse, URLPattern from allianceauth.eveonline.models import EveCharacter from allianceauth.tests.auth_utils import AuthUtils -from ..decorators import main_character_required -from ..models import CharacterOwnership +from ..decorators import decorate_url_patterns, main_character_required +from ..models import CharacterOwnership MODULE_PATH = 'allianceauth.authentication' @@ -66,3 +66,33 @@ class DecoratorTestCase(TestCase): setattr(self.request, 'user', self.main_user) response = self.dummy_view(self.request) self.assertEqual(response.status_code, 200) + + +class TestDecorateUrlPatterns(TestCase): + def test_should_add_decorator_by_default(self): + # given + decorator = mock.MagicMock(name="decorator") + view = mock.MagicMock(name="view") + path = mock.MagicMock(spec=URLPattern, name="path") + path.callback = view + path.lookup_str = "my_lookup_str" + urls = [path] + urlconf_module = urls + # when + decorate_url_patterns(urlconf_module, decorator) + # then + self.assertEqual(path.callback, decorator(view)) + + def test_should_not_add_decorator_when_excluded(self): + # given + decorator = mock.MagicMock(name="decorator") + view = mock.MagicMock(name="view") + path = mock.MagicMock(spec=URLPattern, name="path") + path.callback = view + path.lookup_str = "my_lookup_str" + urls = [path] + urlconf_module = urls + # when + decorate_url_patterns(urlconf_module, decorator, excluded_views=["my_lookup_str"]) + # then + self.assertEqual(path.callback, view) diff --git a/allianceauth/project_template/project_name/settings/local.py b/allianceauth/project_template/project_name/settings/local.py index 296bee02..1405fbd4 100644 --- a/allianceauth/project_template/project_name/settings/local.py +++ b/allianceauth/project_template/project_name/settings/local.py @@ -32,6 +32,10 @@ INSTALLED_APPS += [ # To change the logging level for extensions, uncomment the following line. # LOGGING['handlers']['extension_file']['level'] = 'DEBUG' +# By default apps are prevented from having public views for security reasons. +# If you want to allow specific apps to have public views +# you can put there names here (same name as in INSTALLED_APPS): +APPS_WITH_PUBLIC_VIEWS = [] # Enter credentials to use MySQL/MariaDB. Comment out to use sqlite3 DATABASES['default'] = { diff --git a/allianceauth/services/hooks.py b/allianceauth/services/hooks.py index 59595a84..986dc6f5 100644 --- a/allianceauth/services/hooks.py +++ b/allianceauth/services/hooks.py @@ -1,15 +1,18 @@ +from string import Formatter +from typing import Iterable, Optional + +from django.conf import settings from django.conf.urls import include -from django.urls import re_path from django.core.exceptions import ObjectDoesNotExist from django.template.loader import render_to_string +from django.urls import re_path from django.utils.functional import cached_property -from django.conf import settings -from string import Formatter from allianceauth.hooks import get_hooks from .models import NameFormatConfig + def get_extension_logger(name): """ Takes the name of a plugin/extension and generates a child logger of the extensions logger @@ -156,8 +159,32 @@ class MenuItemHook: class UrlHook: - def __init__(self, urls, namespace, base_url): + """A hook for registering the URLs of a Django app. + + Args: + - urls: The urls module to include + - namespace: The URL namespace to apply. This is usually just the app name. + - base_url: The URL prefix to match against in regex form. + Example ``r'^app_name/'``. + This prefix will be applied in front of all URL patterns included. + It is possible to use the same prefix as existing apps (or no prefix at all), + but standard URL resolution ordering applies + (hook URLs are the last ones registered). + - excluded_views: Optional list of views to be excluded + from auto-decorating them with the + default ``main_character_required`` decorator, e.g. to make them public. + Views must be specified by their qualified name, + e.g. ``["example.views.my_public_view"]`` + """ + def __init__( + self, + urls, + namespace: str, + base_url: str, + excluded_views : Optional[Iterable[str]] = None + ): self.include_pattern = re_path(base_url, include(urls, namespace=namespace)) + self.excluded_views = set(excluded_views or []) class NameFormatter: diff --git a/allianceauth/services/tests/test_hooks.py b/allianceauth/services/tests/test_hooks.py new file mode 100644 index 00000000..dd3d787b --- /dev/null +++ b/allianceauth/services/tests/test_hooks.py @@ -0,0 +1,30 @@ +from unittest import TestCase + +from allianceauth.services.hooks import UrlHook +from allianceauth.groupmanagement import urls + + +class TestUrlHook(TestCase): + def test_can_create_simple_hook(self): + # when + obj = UrlHook(urls, "groupmanagement", r"^groupmanagement/") + # then + self.assertEqual(obj.include_pattern.app_name, "groupmanagement") + self.assertFalse(obj.excluded_views) + + def test_can_create_hook_with_excluded_views(self): + # when + obj = UrlHook( + urls, + "groupmanagement", + r"^groupmanagement/", + ["groupmanagement.views.group_management"], + ) + # then + self.assertEqual(obj.include_pattern.app_name, "groupmanagement") + self.assertIn("groupmanagement.views.group_management", obj.excluded_views) + + def test_should_raise_error_when_called_with_invalid_excluded_views(self): + # when/then + with self.assertRaises(TypeError): + UrlHook(urls, "groupmanagement", r"^groupmanagement/", 99) diff --git a/allianceauth/tests/test_urls.py b/allianceauth/tests/test_urls.py new file mode 100644 index 00000000..95315b8d --- /dev/null +++ b/allianceauth/tests/test_urls.py @@ -0,0 +1,72 @@ +from unittest import TestCase +from unittest.mock import patch, MagicMock +from django.urls import URLPattern + +from allianceauth.services.hooks import UrlHook +from allianceauth.urls import urls_from_apps + +MODULE_PATH = "allianceauth.urls" + + +@patch(MODULE_PATH + ".main_character_required") +@patch(MODULE_PATH + ".decorate_url_patterns") +class TestUrlsFromApps(TestCase): + def test_should_decorate_url_by_default(self, mock_decorate_url_patterns, mock_main_character_required): + # given + def hook_function(): + return UrlHook(urlconf_module, "my_namespace", r"^my_app/") + + view = MagicMock(name="view") + path = MagicMock(spec=URLPattern, name="path") + path.callback = view + urlconf_module = [patch], "my_app" + # when + result = urls_from_apps([hook_function], []) + # then + self.assertIsInstance(result[0], URLPattern) + self.assertTrue(mock_decorate_url_patterns.called) + args, _ = mock_decorate_url_patterns.call_args + decorator = args[1] + self.assertEqual(decorator, mock_main_character_required) + excluded_views = args[2] + self.assertIsNone(excluded_views) + + def test_should_not_decorate_when_excluded(self, mock_decorate_url_patterns, mock_main_character_required): + # given + def hook_function(): + return UrlHook(urlconf_module, "my_namespace", r"^my_app/", ["excluded_view"]) + + view = MagicMock(name="view") + path = MagicMock(spec=URLPattern, name="path") + path.callback = view + urlconf_module = [patch], "my_app" + # when + result = urls_from_apps([hook_function], ["my_app"]) + # then + self.assertIsInstance(result[0], URLPattern) + self.assertTrue(mock_decorate_url_patterns.called) + args, _ = mock_decorate_url_patterns.call_args + decorator = args[1] + self.assertEqual(decorator, mock_main_character_required) + excluded_views = args[2] + self.assertSetEqual(excluded_views, {"excluded_view"}) + + def test_should_decorate_when_app_has_no_permission(self, mock_decorate_url_patterns, mock_main_character_required): + # given + def hook_function(): + return UrlHook(urlconf_module, "my_namespace", r"^my_app/", ["excluded_view"]) + + view = MagicMock(name="view") + path = MagicMock(spec=URLPattern, name="path") + path.callback = view + urlconf_module = [patch], "my_app" + # when + result = urls_from_apps([hook_function], ["other_app"]) + # then + self.assertIsInstance(result[0], URLPattern) + self.assertTrue(mock_decorate_url_patterns.called) + args, _ = mock_decorate_url_patterns.call_args + decorator = args[1] + self.assertEqual(decorator, mock_main_character_required) + excluded_views = args[2] + self.assertIsNone(excluded_views) diff --git a/allianceauth/urls.py b/allianceauth/urls.py index 3a8ea22c..995b94ca 100755 --- a/allianceauth/urls.py +++ b/allianceauth/urls.py @@ -1,24 +1,54 @@ -from django.urls import path -import esi.urls +from typing import List, Iterable, Callable -from django.conf.urls import include +import esi.urls +from django.conf import settings from django.contrib import admin +from django.urls import URLPattern, include, path from django.views.generic.base import TemplateView -import allianceauth.authentication.views import allianceauth.authentication.urls -import allianceauth.notifications.urls +import allianceauth.authentication.views import allianceauth.groupmanagement.urls +import allianceauth.notifications.urls import allianceauth.services.urls -from allianceauth.authentication.decorators import main_character_required, decorate_url_patterns -from allianceauth import NAME -from allianceauth import views +from allianceauth import NAME, views from allianceauth.authentication import hmac_urls +from allianceauth.authentication.decorators import ( + decorate_url_patterns, + main_character_required +) from allianceauth.hooks import get_hooks admin.site.site_header = NAME +def urls_from_apps( + apps_hook_functions: Iterable[Callable], public_views_allowed: List[str] +) -> List[URLPattern]: + """Return urls from apps and add default decorators.""" + + url_patterns = [] + allowed_apps = set(public_views_allowed) + for app_hook_function in apps_hook_functions: + url_hook = app_hook_function() + app_pattern = url_hook.include_pattern + excluded_views = ( + url_hook.excluded_views + if app_pattern.app_name in allowed_apps + else None + ) + url_patterns += [ + path( + "", + decorate_url_patterns( + [app_pattern], main_character_required, excluded_views + ) + ) + ] + + return url_patterns + + # Functional/Untranslated URL's urlpatterns = [ # Locale @@ -49,8 +79,6 @@ urlpatterns = [ path('night/', views.NightModeRedirectView.as_view(), name='nightmode') ] - -# Append app urls -app_urls = get_hooks('url_hook') -for app in app_urls: - urlpatterns += [path('', decorate_url_patterns([app().include_pattern], main_character_required))] +url_hooks = get_hooks("url_hook") +public_views_allows = getattr(settings, "APPS_WITH_PUBLIC_VIEWS", []) +urlpatterns += urls_from_apps(url_hooks, public_views_allows) diff --git a/docs/development/custom/url-hooks.md b/docs/development/custom/url-hooks.md index f9d76d56..1c43544a 100644 --- a/docs/development/custom/url-hooks.md +++ b/docs/development/custom/url-hooks.md @@ -1,54 +1,65 @@ # URL Hooks -```eval_rst -.. note:: - URLs added through URL Hooks are protected by a decorator which ensures the requesting user is logged in and has a main character set. -``` +## Base functionality The URL hooks allow you to dynamically specify URL patterns from your plugin app or service. To achieve this you should subclass or instantiate the `services.hooks.UrlHook` class and then register the URL patterns with the hook. To register a UrlHook class you would do the following: - @hooks.register('url_hook') - def register_urls(): - return UrlHook(app_name.urls, 'app_name', r^'app_name/') +```python +@hooks.register('url_hook') +def register_urls(): + return UrlHook(app_name.urls, 'app_name', r^'app_name/') +``` +### Public views -The `UrlHook` class specifies some parameters/instance variables required for URL pattern inclusion. +In addition is it possible to make views public. Normally, all views are automatically decorated with the `main_character_required` decorator. That decorator ensures a user needs to be logged in and have a main before he can access that view. This feature protects against a community app sneaking in a public view without the administrator knowing about it. -`UrlHook(urls, app_name, base_url)` +An app can opt-out of this feature by adding a list of views to be excluded when registering the URLs. See the `excluded_views` parameter for details. -#### urls -The urls module to include. See [the Django docs](https://docs.djangoproject.com/en/dev/topics/http/urls/#example) for designing urlpatterns. -#### namespace -The URL namespace to apply. This is usually just the app name. -#### base_url -The URL prefix to match against in regex form. Example `r'^app_name/'`. This prefix will be applied in front of all URL patterns included. It is possible to use the same prefix as existing apps (or no prefix at all) but [standard URL resolution](https://docs.djangoproject.com/en/dev/topics/http/urls/#how-django-processes-a-request) ordering applies (hook URLs are the last ones registered). +```eval_rst +.. note:: + Note that for a public view to work, administrators need to also explicitly allow apps to have public views in their AA installation, by adding the apps label to ``APPS_WITH_PUBLIC_VIEWS`` setting. +``` -### Example +## Examples An app called `plugin` provides a single view: - def index(request): - return render(request, 'plugin/index.html') +```python +def index(request): + return render(request, 'plugin/index.html') +``` The app's `urls.py` would look like so: - from django.urls import path - import plugin.views +```python +from django.urls import path +import plugin.views - urlpatterns = [ - path('index/', plugins.views.index, name='index'), - ] +urlpatterns = [ + path('index/', plugins.views.index, name='index'), +] +``` Subsequently it would implement the UrlHook in a dedicated `auth_hooks.py` file like so: - from alliance_auth import hooks - from services.hooks import UrlHook - import plugin.urls +```python +from alliance_auth import hooks +from services.hooks import UrlHook +import plugin.urls - @hooks.register('url_hook') - def register_urls(): - return UrlHook(plugin.urls, 'plugin', r^'plugin/') +@hooks.register('url_hook') +def register_urls(): + return UrlHook(plugin.urls, 'plugin', r^'plugin/') +``` When this app is included in the project's `settings.INSTALLED_APPS` users would access the index view by navigating to `https://example.com/plugin/index`. + +## API + +```eval_rst +.. autoclass:: allianceauth.services.hooks.UrlHook + :members: +```