Skip to content

POST /v1/groups is authorized for any GROUP_MANAGER, because a conditional 'manage' rule passes the route guard #1468

Description

@joshunrau

Summary

POST /v1/groups is authorized for any GROUP_MANAGER, so a group manager can create arbitrary new groups.

Why

apps/api/src/auth/ability.factory.ts grants a group manager:

ability.can('manage', 'Group', { id: { in: groupIds } });

JwtAuthGuard evaluates @RouteAccess against the subject type only (ability.can(action, subject) with a subject name, not a row), and CASL ignores a rule's conditions for a type-level check. So the condition is invisible to the guard:

can('create', 'Group') => true      can('create', 'User') => false
can('update', 'Group') => true      can('manage', 'all')  => false
can('delete', 'Group') => true

(Confirmed by building that exact rule set with @casl/prisma and calling can directly.)

Every sibling route survives this because the service re-checks against real rows — accessibleQuery(ability, ...) in the Prisma where, i.e. Layer 2 in .agents/docs/architecture/auth-and-permissions.md. GroupsService.create is the exception: it takes no ability parameter at all, and a create has no existing row to scope against, so nothing narrows it.

Repro

POST /v1/groups with a group manager's access token → 201.

Also reachable through the UI: /admin/groups/create renders for a group manager (no route-level role check exists — see the companion issue), so the form submits successfully and redirects to the group list.

Impact

Bounded, but real. The created group is instance-wide, is connected to every non-repo instrument, occupies the unique-name space, and appears in every admin's group list.

It is not a data-access escalation: permissions are frozen into the JWT at login, so the creator is not a member of the group they just created and cannot read anything through it.

Suggested fix

Either:

  1. Narrow @RouteAccess on GroupsController.create to { action: 'manage', subject: 'all' }, matching who the UI intends to expose it to and the sibling PATCH /v1/setup; or
  2. Give GroupsService.create an { ability } parameter and check the payload with forcedAppSubject('Group', ...), the way files.service.ts does.

(1) is smaller and matches existing convention. Worth a unit test in src/auth/__tests__/ability.factory.test.ts asserting the deny case either way.

Notes

Found while writing the Playwright suite (branch e2e-tests). testing/src/specs/authorization.spec.ts now has a server-side authorization block asserting the whole family of privileged requests is refused for non-admin roles; POST /v1/groups is deliberately excluded from that table so the suite does not lock in this behaviour. Add it back when this is fixed.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions