Skip to content

Commit c04c5a4

Browse files
committed
Fix viewer role login
Pre-existing viewer role could not log in. Fix login by modifying what is considered an "adminable instance" for a user and by adding access to wagtail admin as permission for the viewer role.
1 parent d73cb5a commit c04c5a4

File tree

5 files changed

+19
-7
lines changed

5 files changed

+19
-7
lines changed

admin_site/wagtail_hooks.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ class InstanceItem(MenuItem):
5353
class InstanceChooserMenu(Menu):
5454
def menu_items_for_request(self, request: PathsAdminRequest):
5555
user = request.user
56-
instances = InstanceConfig.permission_policy().instances_user_has_any_permission_for(user, ['change'])
56+
instances = user.get_adminable_instances()
5757
if len(instances) < 2:
5858
return []
5959
items = []

nodes/models.py

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -151,13 +151,16 @@ def is_framework_viewer(self, user: User, obj: InstanceConfig) -> bool:
151151
return False
152152
return user.has_instance_role(self.fw_viewer_role, obj.framework_config.framework)
153153

154-
def construct_perm_q(self, user: User, action: ObjectSpecificAction) -> models.Q | None:
154+
def construct_perm_q(self, user: User, action: ObjectSpecificAction, include_implicit_public: bool = True) -> models.Q | None:
155155
is_admin = self.admin_role.role_q(user)
156156
is_viewer = self.viewer_role.role_q(user)
157157
is_fw_admin = self.fw_admin_role.role_q(user, prefix='framework_config__framework')
158158
is_fw_viewer = self.fw_viewer_role.role_q(user, prefix='framework_config__framework')
159159
if action == 'view':
160-
return is_viewer | is_admin | is_fw_admin | is_fw_viewer | Q(framework_config__isnull=True)
160+
q = is_viewer | is_admin | is_fw_admin | is_fw_viewer
161+
if include_implicit_public:
162+
q |= Q(framework_config__isnull=True)
163+
return q
161164
return is_admin | is_fw_admin
162165

163166
def construct_perm_q_anon(self, action: BaseObjectAction) -> Q | None:
@@ -167,7 +170,16 @@ def construct_perm_q_anon(self, action: BaseObjectAction) -> Q | None:
167170
return None
168171

169172
def adminable_instances(self, user: User) -> InstanceConfigQuerySet:
170-
return self.instances_user_has_any_permission_for(user, ['change'])
173+
"""
174+
Return instances that the user has been explicitly granted access to.
175+
176+
We can't simply use the 'view' permission on InstanceConfig, because
177+
most instances will be public by default.
178+
"""
179+
qs = self.get_queryset()
180+
if user.is_superuser:
181+
return qs
182+
return qs.filter(self.construct_perm_q(user, 'view', include_implicit_public=False))
171183

172184
def user_has_perm(self, user: User, action: ObjectSpecificAction, obj: InstanceConfig) -> bool:
173185
if action == 'delete':

nodes/roles.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ class InstanceViewerRole(InstanceGroupMembershipRole, InstanceSpecificRole['Inst
6060
instance_group_field_name = 'viewer_group'
6161

6262
model_perms = [
63+
('wagtailadmin', 'admin', ('access',)),
6364
('nodes', ('instanceconfig', 'nodeconfig'), ('view',)),
6465
('frameworks', (
6566
'framework', 'frameworkconfig', 'measure', 'measuredatapoint',

paths/middleware.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,7 @@ async def get_admin_instance(self, request: PathsAdminRequest) -> None | Instanc
6767
instance_config = await InstanceConfig.objects.filter(id=admin_instance_id).afirst()
6868

6969
adminable_instances = [
70-
ic async for ic in InstanceConfig.permission_policy().instances_user_has_any_permission_for(user, ['change'])
71-
.filter(site__isnull=False)
70+
ic async for ic in user.get_adminable_instances().filter(site__isnull=False)
7271
]
7372
if instance_config is not None and instance_config not in adminable_instances:
7473
instance_config = None

users/models.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ def get_active_instance(self) -> InstanceConfig | None:
7272

7373
def get_adminable_instances(self) -> InstanceConfigQuerySet:
7474
from nodes.models import InstanceConfig
75-
return InstanceConfig.permission_policy().instances_user_has_permission_for(self, 'change')
75+
return InstanceConfig.permission_policy().adminable_instances(self)
7676

7777
@cached_property
7878
def cgroups(self) -> QS[Group]:

0 commit comments

Comments
 (0)