diff --git a/admin_site/viewsets.py b/admin_site/viewsets.py index 28199886..b2256104 100644 --- a/admin_site/viewsets.py +++ b/admin_site/viewsets.py @@ -9,6 +9,7 @@ from django.db.models import Model, QuerySet from django.forms import BaseModelForm from wagtail.admin.forms.models import WagtailAdminModelForm +from wagtail.admin.ui.tables import BulkActionsCheckboxColumn from wagtail.snippets.views.chooser import ChooseResultsView, ChooseView, SnippetChooserViewSet from wagtail.snippets.views.snippets import ( CopyView, @@ -113,7 +114,15 @@ def get_form_kwargs(self): class PathsIndexView(HideSnippetsFromBreadcrumbsMixin, IndexView[_ModelT, _QS]): - pass + def user_can_change_or_delete_model(self) -> bool: + return self.permission_policy.user_has_any_permission(self.request.user, ('delete', 'change')) + + @cached_property + def columns(self): + columns = super().columns + if self.user_can_change_or_delete_model(): + return columns + return [c for c in columns if not isinstance(c, BulkActionsCheckboxColumn)] class PathsUsageView(HideSnippetsFromBreadcrumbsMixin, UsageView): diff --git a/admin_site/wagtail_hooks.py b/admin_site/wagtail_hooks.py index f3176a13..a815d166 100644 --- a/admin_site/wagtail_hooks.py +++ b/admin_site/wagtail_hooks.py @@ -9,7 +9,7 @@ from wagtail import hooks from wagtail.admin.menu import Menu, MenuItem, SubmenuMenuItem -from nodes.models import InstanceConfig +from wagtail_localize.wagtail_hooks import TranslationsReportMenuItem if TYPE_CHECKING: from paths.types import PathsAdminRequest @@ -53,7 +53,7 @@ class InstanceItem(MenuItem): class InstanceChooserMenu(Menu): def menu_items_for_request(self, request: PathsAdminRequest): user = request.user - instances = InstanceConfig.permission_policy().instances_user_has_any_permission_for(user, ['change']) + instances = user.get_adminable_instances() if len(instances) < 2: return [] items = [] @@ -82,3 +82,13 @@ def register_instance_chooser(): @hooks.register('construct_main_menu') def hide_snippets_menu_item(request, menu_items): menu_items[:] = [item for item in menu_items if item.name != 'snippets'] + + +@hooks.register('construct_reports_menu') +def patch_translations_report_menu_item(request: PathsAdminRequest, menu_items: list): + # We don't want to show the Reports menu to people without rights to it. + # TranslationsReportMenuItem is always shown by default. + # Hide from non-superusers until we know who needs this menu item + if getattr(request.user, 'is_superuser', False): + return + menu_items[:] = [menu_item for menu_item in menu_items if not isinstance(menu_item, TranslationsReportMenuItem)] diff --git a/datasets/tests/test_dataset_authorization.py b/datasets/tests/test_dataset_authorization.py index 4a559f19..b03d9e1b 100644 --- a/datasets/tests/test_dataset_authorization.py +++ b/datasets/tests/test_dataset_authorization.py @@ -7,6 +7,7 @@ from kausal_common.datasets.models import Dataset, DatasetSchema from paths.admin_context import set_admin_instance +from paths.context import RealmContext, realm_context from admin_site.dataset_admin import DatasetSchemaViewSet from nodes.models import InstanceConfig @@ -24,7 +25,6 @@ def get(user: User, view, url: str, instance_config: InstanceConfig, kwargs: dic if not user.selected_instance: user.selected_instance = instance_config user.save() - from paths.context import RealmContext, realm_context set_admin_instance(instance_config, request=request) ctx = RealmContext(realm = instance_config, user=request.user) with realm_context.activate(ctx): @@ -142,7 +142,10 @@ def test_dataset_schema_index_view(self, client, setup_test_data, user_key, expe response = get_in_admin_context(user, view, url, data['instance1']) return - response.render() + ctx = RealmContext(realm = data['instance1'], user=user) + with realm_context.activate(ctx): + response.render() + assert response.status_code == 200 content = response.content.decode('utf-8') for schema_name in expected_schemas: @@ -235,8 +238,17 @@ def test_dataset_index_view(self, client, setup_test_data, user_key, expected_da return for instance_key, dataset_key in expected_datasets: - response = get_in_admin_context(user, view, url, data[instance_key]) - response.render() + try: + response = get_in_admin_context(user, view, url, data[instance_key]) + except PermissionDenied: + assert dataset_key is None + continue + else: + assert dataset_key is not None + ctx = RealmContext(realm = data[instance_key], user=user) + with realm_context.activate(ctx): + response.render() + assert response.status_code == 200 content = response.content.decode('utf-8') diff --git a/nodes/models.py b/nodes/models.py index 2b4a29ab..64ceb7d9 100644 --- a/nodes/models.py +++ b/nodes/models.py @@ -151,13 +151,16 @@ def is_framework_viewer(self, user: User, obj: InstanceConfig) -> bool: return False return user.has_instance_role(self.fw_viewer_role, obj.framework_config.framework) - def construct_perm_q(self, user: User, action: ObjectSpecificAction) -> models.Q | None: + def construct_perm_q(self, user: User, action: ObjectSpecificAction, include_implicit_public: bool = True) -> models.Q | None: is_admin = self.admin_role.role_q(user) is_viewer = self.viewer_role.role_q(user) is_fw_admin = self.fw_admin_role.role_q(user, prefix='framework_config__framework') is_fw_viewer = self.fw_viewer_role.role_q(user, prefix='framework_config__framework') if action == 'view': - return is_viewer | is_admin | is_fw_admin | is_fw_viewer | Q(framework_config__isnull=True) + q = is_viewer | is_admin | is_fw_admin | is_fw_viewer + if include_implicit_public: + q |= Q(framework_config__isnull=True) + return q return is_admin | is_fw_admin def construct_perm_q_anon(self, action: BaseObjectAction) -> Q | None: @@ -167,7 +170,16 @@ def construct_perm_q_anon(self, action: BaseObjectAction) -> Q | None: return None def adminable_instances(self, user: User) -> InstanceConfigQuerySet: - return self.instances_user_has_any_permission_for(user, ['change']) + """ + Return instances that the user has been explicitly granted access to. + + We can't simply use the 'view' permission on InstanceConfig, because + most instances will be public by default. + """ + qs = self.get_queryset() + if user.is_superuser: + return qs + return qs.filter(self.construct_perm_q(user, 'view', include_implicit_public=False)) def user_has_perm(self, user: User, action: ObjectSpecificAction, obj: InstanceConfig) -> bool: if action == 'delete': diff --git a/nodes/node_admin.py b/nodes/node_admin.py index afa0a804..fedd99f8 100644 --- a/nodes/node_admin.py +++ b/nodes/node_admin.py @@ -11,6 +11,7 @@ class NodeViewSet(PathsViewSet[NodeConfig, NodeConfigQuerySet]): model = NodeConfig + inspect_view_enabled = True icon = 'kausal-node' add_to_admin_menu = True search_fields = ['name', 'identifier'] diff --git a/nodes/roles.py b/nodes/roles.py index eadbc48e..e682f787 100644 --- a/nodes/roles.py +++ b/nodes/roles.py @@ -60,6 +60,7 @@ class InstanceViewerRole(InstanceGroupMembershipRole, InstanceSpecificRole['Inst instance_group_field_name = 'viewer_group' model_perms = [ + ('wagtailadmin', 'admin', ('access',)), ('nodes', ('instanceconfig', 'nodeconfig'), ('view',)), ('frameworks', ( 'framework', 'frameworkconfig', 'measure', 'measuredatapoint', diff --git a/pages/sitecontent.py b/pages/sitecontent.py index 96c8ae70..fd3b30f3 100644 --- a/pages/sitecontent.py +++ b/pages/sitecontent.py @@ -1,16 +1,23 @@ +from __future__ import annotations + from typing import Callable from django.urls import reverse from django.utils.translation import gettext_lazy as _ from wagtail.admin.menu import MenuItem from wagtail.admin.panels import FieldPanel -from wagtail.permission_policies.base import ModelPermissionPolicy -from admin_site.viewsets import PathsViewSet, PathsEditView +from kausal_common.models.permission_policy import ParentInheritedPolicy + +from admin_site.viewsets import PathsEditView, PathsViewSet from pages.models import InstanceSiteContent -class SiteContentPermissionPolicy(ModelPermissionPolicy): +class SiteContentPermissionPolicy(ParentInheritedPolicy): + def __init__(self): + from nodes.models import InstanceConfig + super().__init__(model=InstanceSiteContent, parent_model=InstanceConfig, parent_field='instance') + def user_has_permission(self, user, action): # Disable creating of site content instances if action == 'add': @@ -44,7 +51,7 @@ def render_component(self, request): def is_shown(self, request): user = request.user - if user.is_superuser or user.can_access_admin(): + if user.is_superuser: return True instance = request.admin_instance field = self.get_one_to_one_field(instance) @@ -88,7 +95,7 @@ class InstanceSiteContentViewSet(PathsViewSet): @property def permission_policy(self): - return SiteContentPermissionPolicy(self.model) + return SiteContentPermissionPolicy() def get_queryset(self, request): qs = super().get_queryset(request) diff --git a/paths/dataset_permission_policy.py b/paths/dataset_permission_policy.py index b4d6834a..a981d924 100644 --- a/paths/dataset_permission_policy.py +++ b/paths/dataset_permission_policy.py @@ -26,7 +26,12 @@ from nodes.roles import instance_admin_role, instance_viewer_role if TYPE_CHECKING: + from collections.abc import Sequence + + from django.contrib.auth.models import AnonymousUser + from kausal_common.models.permissions import PermissionedModel + from kausal_common.models.roles import InstanceSpecificRole from nodes.models import InstanceConfig from users.models import User @@ -35,7 +40,7 @@ _QS = TypeVar('_QS', bound=QuerySet, default=QuerySet[_M]) -class InstanceConfigScopedPermissionPolicy(ModelPermissionPolicy[_M, _QS]): +class InstanceConfigScopedPermissionPolicy(ModelPermissionPolicy[_M, 'InstanceConfig', _QS]): """Permission policy for models that have one or many InstanceConfig objects as scope.""" def __init__(self, model: type[_M]): @@ -83,6 +88,17 @@ def user_can_create(self, user: User, context: InstanceConfig) -> bool: def construct_perm_q_anon(self, action: BaseObjectAction) -> Q | None: return None + def user_has_any_role_in_active_instance(self, user: User, roles: Sequence[InstanceSpecificRole[InstanceConfig]]) -> bool: + if user.is_superuser: + return True + + active_instance = realm_context.get().realm + + if active_instance is None: + return False + + return any(user.has_instance_role(role, active_instance) for role in roles) + class DatasetSchemaPermissionPolicy(InstanceConfigScopedPermissionPolicy[DatasetSchema]): """Permission policy for DatasetSchema, based on its scope (InstanceConfig).""" @@ -119,6 +135,19 @@ def construct_perm_q(self, user: User, action: BaseObjectAction) -> Q | None: return admin_q + def user_has_permission(self, user: User | AnonymousUser, action: str) -> bool: + if not self.user_is_authenticated(user): + return False + + allowed_roles: list[InstanceSpecificRole[InstanceConfig]] = [self.instance_admin_role] + if action == 'view': + allowed_roles.append(self.instance_viewer_role) + + return self.user_has_any_role_in_active_instance(user, allowed_roles) + + def user_has_any_permission(self, user: User | AnonymousUser, actions: Sequence[str]) -> bool: + return any(self.user_has_permission(user, action) for action in actions) + class DatasetPermissionPolicy(ParentInheritedPolicy[Dataset, DatasetSchema, DatasetQuerySet]): """Permission policy for Dataset, inheriting from its schema.""" diff --git a/paths/middleware.py b/paths/middleware.py index ec4abbf8..61b580c8 100644 --- a/paths/middleware.py +++ b/paths/middleware.py @@ -67,8 +67,7 @@ async def get_admin_instance(self, request: PathsAdminRequest) -> None | Instanc instance_config = await InstanceConfig.objects.filter(id=admin_instance_id).afirst() adminable_instances = [ - ic async for ic in InstanceConfig.permission_policy().instances_user_has_any_permission_for(user, ['change']) - .filter(site__isnull=False) + ic async for ic in user.get_adminable_instances().filter(site__isnull=False) ] if instance_config is not None and instance_config not in adminable_instances: instance_config = None diff --git a/paths/settings.py b/paths/settings.py index e2c40a79..8bda581f 100644 --- a/paths/settings.py +++ b/paths/settings.py @@ -134,7 +134,6 @@ 'wagtail.search', 'wagtail.admin', 'wagtail', - 'wagtail.contrib.styleguide', 'wagtail_localize', 'wagtail_localize.locales', # replaces `wagtail.locales` 'wagtailfontawesomesvg', @@ -165,6 +164,9 @@ 'request_log', ] +if DEBUG: + INSTALLED_APPS.append('wagtail.contrib.styleguide') + MIDDLEWARE = [ 'django.contrib.sessions.middleware.SessionMiddleware', 'corsheaders.middleware.CorsMiddleware', diff --git a/requirements-kausal.txt b/requirements-kausal.txt index 96edd8a7..3b30df60 100644 --- a/requirements-kausal.txt +++ b/requirements-kausal.txt @@ -1,6 +1,6 @@ # This file was autogenerated by uv via the following command: # uv pip compile --generate-hashes --refresh --no-deps -o requirements-kausal.txt requirements-kausal.in -kausal-paths-extensions==0.6.34 \ - --hash=md5:e44de7ceed70e96227b08d4b446cea7c \ - --hash=md5:f55b1b30d555d947bdd41442e9cadf0a +kausal-paths-extensions==0.6.35 \ + --hash=md5:4c3afd195ea0175c4d171b25783a6b7f \ + --hash=md5:68ac0539d863e60842bf9879e6c9927e # via -r requirements-kausal.in diff --git a/users/models.py b/users/models.py index 30d5ae93..1dd116f0 100644 --- a/users/models.py +++ b/users/models.py @@ -72,7 +72,7 @@ def get_active_instance(self) -> InstanceConfig | None: def get_adminable_instances(self) -> InstanceConfigQuerySet: from nodes.models import InstanceConfig - return InstanceConfig.permission_policy().instances_user_has_permission_for(self, 'change') + return InstanceConfig.permission_policy().adminable_instances(self) @cached_property def cgroups(self) -> QS[Group]: