Skip to content

Commit a594d32

Browse files
authored
Merge pull request #188 from pneumaticapp/backend/permission/45414__fix_permission_for_role_template
45414 backend [ permission ] Fix permission for role template
2 parents 48e0f31 + d15d1fb commit a594d32

9 files changed

Lines changed: 104 additions & 227 deletions

File tree

backend/src/processes/permissions.py

Lines changed: 81 additions & 137 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,7 @@ class TemplateAdminOwnerPermission(BasePermission):
2929

3030
"""
3131
For template editing operations.
32-
Only admin users who are template owners can edit.
33-
Non-admin owners have viewer-level access only.
32+
Only users who are template owners can edit.
3433
"""
3534

3635
message = MSG_PT_0023
@@ -44,9 +43,6 @@ def has_permission(self, request, view):
4443
if request.user.is_account_owner:
4544
return True
4645

47-
if not request.user.is_admin:
48-
return False
49-
5046
template_owner_qst = (
5147
Template.objects
5248
.by_id(template_id)
@@ -106,6 +102,42 @@ def has_permission(self, request, view):
106102
)
107103

108104

105+
class TemplateFieldsPermission(BasePermission):
106+
107+
""" Allow access for template owners, viewers or workflow members """
108+
109+
message = MSG_PT_0070
110+
111+
def has_permission(self, request, view):
112+
try:
113+
template_id = int(view.kwargs.get('pk'))
114+
except (ValueError, TypeError):
115+
return False
116+
117+
user = request.user
118+
if user.is_account_owner:
119+
return True
120+
121+
return (
122+
Template.objects
123+
.by_id(template_id)
124+
.on_account(user.account_id)
125+
.filter(
126+
Q(
127+
owners__type=OwnerType.USER,
128+
owners__user_id=user.id,
129+
owners__is_deleted=False,
130+
owners__role__in=(OwnerRole.OWNER, OwnerRole.VIEWER),
131+
) | Q(
132+
owners__type=OwnerType.GROUP,
133+
owners__group__users__id=user.id,
134+
owners__is_deleted=False,
135+
owners__role__in=(OwnerRole.OWNER, OwnerRole.VIEWER),
136+
) | Q(workflows__members=user.id),
137+
).exists()
138+
)
139+
140+
109141
class UserCanAccessHighlightsPermission(BasePermission):
110142

111143
""" Allow admin, account owner, template owners or template viewers
@@ -203,15 +235,8 @@ class WorkflowMemberPermission(BasePermission):
203235
Allow access for:
204236
- Account owner
205237
- Guests (non-user type)
206-
- Workflow starter (user who launched the workflow)
207238
- Task performers (actual performers on tasks)
208239
- Workflow members (performers, mentioned users)
209-
- Admin template owners
210-
211-
Note: Template starters who are ONLY starters (not owners/viewers/members)
212-
should not have access to workflows they didn't start.
213-
This is enforced by checking that starters are not in workflow.members
214-
unless they are also performers or were mentioned.
215240
"""
216241

217242
def has_permission(self, request, view):
@@ -222,51 +247,18 @@ def has_permission(self, request, view):
222247

223248
user = request.user
224249

225-
if user.type != UserType.USER:
226-
return True
227-
228-
if user.is_account_owner:
250+
if user.type != UserType.USER or user.is_account_owner:
229251
return True
230252

231-
workflow = Workflow.objects.filter(
253+
return Workflow.objects.filter(
232254
pk=workflow_id,
233255
account_id=user.account_id,
234-
).first()
235-
236-
if not workflow:
237-
return False
238-
239-
if workflow.workflow_starter_id == user.id:
240-
return True
241-
242-
is_performer = TaskPerformer.objects.filter(
243-
task__workflow_id=workflow_id,
244-
task__account_id=user.account_id,
245256
).filter(
246-
Q(user_id=user.id) |
247-
Q(group__users__id=user.id),
257+
Q(tasks__taskperformer__user_id=user.id)
258+
| Q(tasks__taskperformer__group__users__id=user.id)
259+
| Q(members=user.id),
248260
).exists()
249261

250-
if is_performer:
251-
return True
252-
253-
is_member = workflow.members.filter(id=user.id).exists()
254-
if is_member:
255-
return True
256-
257-
template = workflow.template
258-
if template and user.is_admin:
259-
is_template_owner = template.owners.filter(
260-
Q(type=OwnerType.USER, user_id=user.id)
261-
| Q(type=OwnerType.GROUP, group__users__id=user.id),
262-
role=OwnerRole.OWNER,
263-
is_deleted=False,
264-
).exists()
265-
if is_template_owner:
266-
return True
267-
268-
return False
269-
270262

271263
class TaskRevertPermission(BasePermission):
272264

@@ -316,8 +308,6 @@ class TaskWorkflowOwnerPermission(BasePermission):
316308

317309
"""
318310
For task editing operations (performers, due_date).
319-
Only admin users who are workflow owners can edit.
320-
Non-admin workflow owners have viewer-level access only.
321311
"""
322312

323313
def has_permission(self, request, view):
@@ -329,9 +319,6 @@ def has_permission(self, request, view):
329319
if request.user.is_account_owner:
330320
return True
331321

332-
if not request.user.is_admin:
333-
return False
334-
335322
workflow_owner_qst = Task.objects.filter(
336323
workflow__owners=request.user,
337324
pk=task_id,
@@ -360,54 +347,25 @@ def has_permission(self, request, view):
360347

361348
user = request.user
362349

363-
if user.type != UserType.USER:
350+
if user.type != UserType.USER or user.is_account_owner:
364351
return True
365352

366-
if user.is_account_owner:
367-
return True
368-
369-
task = Task.objects.filter(
353+
has_access = Task.objects.filter(
370354
id=task_id,
371355
account_id=user.account_id,
372-
).select_related('workflow', 'workflow__template').first()
373-
374-
if not task:
375-
# If task doesn't exist, let Django handle 404 response
376-
# by allowing permission and letting get_object_or_404 fail
377-
return True
378-
379-
workflow = task.workflow
380-
381-
if workflow.workflow_starter_id == user.id:
382-
return True
383-
384-
is_performer = TaskPerformer.objects.filter(
385-
task__workflow_id=workflow.id,
386-
task__account_id=user.account_id,
387356
).filter(
388-
Q(user_id=user.id) |
389-
Q(group__users__id=user.id),
357+
Q(workflow__tasks__taskperformer__user_id=user.id)
358+
| Q(workflow__tasks__taskperformer__group__users__id=user.id)
359+
| Q(workflow__members=user.id),
390360
).exists()
391361

392-
if is_performer:
393-
return True
394-
395-
is_member = workflow.members.filter(id=user.id).exists()
396-
if is_member:
362+
if has_access:
397363
return True
398364

399-
template = workflow.template
400-
if template and user.is_admin:
401-
is_template_owner = template.owners.filter(
402-
Q(type=OwnerType.USER, user_id=user.id)
403-
| Q(type=OwnerType.GROUP, group__users__id=user.id),
404-
role=OwnerRole.OWNER,
405-
is_deleted=False,
406-
).exists()
407-
if is_template_owner:
408-
return True
409-
410-
return False
365+
return not Task.objects.filter(
366+
id=task_id,
367+
account_id=user.account_id,
368+
).exists()
411369

412370

413371
class TaskWorkflowMemberOrViewerPermission(BasePermission):
@@ -438,11 +396,11 @@ class TaskCommentPermission(BasePermission):
438396

439397
"""
440398
Allow task comments for:
441-
- Account owners and admins
442-
- Workflow members
399+
- Guests
400+
- Workflow members and owners
401+
- Task performers (direct and via group)
443402
- Template owners
444403
- Template viewers
445-
- Task performers
446404
"""
447405

448406
def has_permission(self, request, view):
@@ -451,51 +409,38 @@ def has_permission(self, request, view):
451409
except (ValueError, TypeError):
452410
return False
453411

454-
if request.user.is_account_owner or request.user.is_admin:
455-
return True
412+
user = request.user
456413

457-
if request.user.type == UserType.GUEST:
414+
if request.user.type != UserType.USER:
458415
return True
459416

460-
user_id = request.user.id
461-
account_id = request.user.account_id
462-
463-
# Check workflow member or owner (single query)
464-
is_workflow_member = Workflow.objects.filter(
417+
user_id = user.id
418+
account_id = user.account_id
419+
base_qst = Workflow.objects.filter(
465420
tasks__id=task_id,
466421
account_id=account_id,
467-
).filter(
468-
Q(members=user_id) | Q(owners=user_id),
422+
)
423+
424+
is_workflow_member = base_qst.filter(
425+
Q(members=user_id)
426+
| Q(owners=user_id)
427+
| Q(tasks__taskperformer__user_id=user_id)
428+
| Q(tasks__taskperformer__group__users__id=user_id),
469429
).exists()
470430
if is_workflow_member:
471431
return True
472432

473-
# Check task performer
474-
is_performer = (
475-
TaskPerformer.objects
476-
.by_task(task_id)
477-
.filter(task__account_id=account_id)
478-
.by_user_or_group(user_id)
479-
.exists()
480-
)
481-
if is_performer:
482-
return True
483-
484433
# Check template owner or viewer
485-
return (
486-
Workflow.objects
487-
.filter(tasks__id=task_id, account_id=account_id)
488-
.with_template_owner_or_viewer(user_id)
489-
.exists()
490-
)
434+
return base_qst.with_template_owner_or_viewer(user_id).exists()
491435

492436

493437
class WorkflowCommentPermission(BasePermission):
494438

495439
"""
496440
Allow workflow comments for:
497-
- Account owners and admins
498-
- Workflow members
441+
- Guests
442+
- Workflow members and owners
443+
- Group task performers
499444
- Template owners
500445
- Template viewers
501446
"""
@@ -506,19 +451,20 @@ def has_permission(self, request, view):
506451
except (ValueError, TypeError):
507452
return False
508453

509-
if request.user.is_account_owner or request.user.is_admin:
510-
return True
454+
user = request.user
511455

512-
if request.user.type == UserType.GUEST:
456+
if request.user.type != UserType.USER:
513457
return True
514458

515-
user_id = request.user.id
516-
account_id = request.user.account_id
459+
user_id = user.id
460+
account_id = user.account_id
517461
base_qst = Workflow.objects.by_id(workflow_id).on_account(account_id)
518462

519-
# Check workflow member or owner (single query)
520463
is_workflow_member = base_qst.filter(
521-
Q(members=user_id) | Q(owners=user_id),
464+
Q(members=user_id)
465+
| Q(owners=user_id)
466+
| Q(tasks__taskperformer__user_id=user.id)
467+
| Q(tasks__taskperformer__group__users__id=user_id),
522468
).exists()
523469
if is_workflow_member:
524470
return True
@@ -658,7 +604,7 @@ class CommentReactionPermission(BasePermission):
658604

659605
"""
660606
Allow comment reactions/watched for:
661-
- Account owners and admins
607+
- Account owners
662608
- Workflow members
663609
- Template owners
664610
- Template viewers
@@ -677,7 +623,7 @@ def has_permission(self, request, view):
677623
account_id=user.account_id,
678624
).type_comment()
679625

680-
if user.is_account_owner or user.is_admin:
626+
if user.is_account_owner:
681627
return qst.exists()
682628

683629
if user.type == UserType.GUEST:
@@ -738,8 +684,6 @@ def has_permission(self, request, view):
738684
if preset.author_id == user.id or user.is_account_owner:
739685
return True
740686
if preset.type == PresetType.ACCOUNT:
741-
if not user.is_admin:
742-
return False
743687
return preset.template.owners.filter(
744688
Q(
745689
type=OwnerType.USER,

backend/src/processes/tests/test_permissions/test_task_comment_permission.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ def test_has_permission__admin__ok(self):
7272
result = permission.has_permission(request, view)
7373

7474
# assert
75-
assert result is True
75+
assert result is False
7676

7777
def test_has_permission__workflow_member__ok(self):
7878
# arrange

0 commit comments

Comments
 (0)