Skip to content

fix: resolve collaborator role server-side instead of trusting the request body - #1378

Merged
SheyeJDev merged 4 commits into
Split-Naira:mainfrom
hi-bit-ke:fix/1320-enforce-project-permissions
Sep 27, 2026
Merged

SheyeJDev merged 4 commits into
Split-Naira:mainfrom
hi-bit-ke:fix/1320-enforce-project-permissions

Conversation

@hi-bit-ke

Copy link
Copy Markdown
Contributor

Overview

assertPermission enforces the role it is given, but nothing resolved which
role a requester actually holds: POST /projects/:projectId/delete read the
role straight out of the request body and defaulted it to owner:

const role: CollaboratorRole = body.role ?? "owner";
assertPermission(role, "project:delete");

Any caller could therefore send {"role":"owner"} and delete a project. The
matrix was defined and tested; it was not enforced server-side, which is what
the issue asks for.

This PR resolves the role from the authenticated requester address against the
project's stored collaboration record, and denies by default when that record
cannot be established.

Related Issue

Closes #1320

Changes

  • backend/src/services/collaboration/permissions.ts — adds
    ProjectRoleContext (a project's owner plus collaborators with optional
    roles) and two functions built on the existing matrix:
    resolveRequesterRole() maps an address to its role (owner wins, unlisted →
    null, missing role → viewer), and assertProjectPermission() resolves
    first and then asserts, so a role can never be supplied by the caller. It
    fails closed: an unknown requester or an unavailable record throws
    permission_denied with role: null and reason: "project_role_unknown".
  • backend/src/middleware/project-permission.ts (new) —
    createProjectPermissionMiddleware(permission, resolveContext) composes after
    the existing requireStellarAddress (which verifies the
    X-Stellar-Address header), resolves the role, publishes it on
    res.locals.collaboratorRole, and answers 401 when unauthenticated, 400
    without a project id, and 403 when the role is missing or insufficient.
    A resolver that throws is treated as "no record" rather than as access.
  • backend/src/routes/collaboration.ts — the delete route now runs
    requireStellarAddress + the permission middleware, and the role field is
    removed from its body schema. The response echoes the resolved role for audit.
    A setProjectRoleContextResolver() registration point supplies the
    authoritative record at startup; while unregistered, the route fails closed
    instead of trusting the caller.

Verification Results

$ npx vitest run src/services/collaboration
 ✓ src/services/collaboration/__tests__/permissions-matrix.test.ts (8 tests)
 ✓ src/services/collaboration/__tests__/permissions-enforcement.test.ts (17 tests)
 Test Files  2 passed (2)
      Tests  25 passed (25)

The new suite covers role resolution (owner precedence, stored roles, unlisted
addresses, unusable input), fail-closed behaviour (unknown requester, unavailable
record, resolver throwing), and — the regression this fixes — that the
middleware ignores a role: "owner" in the request body.

Not run in this environment: the full backend suite and `npm run build`
(the change was applied through the GitHub Contents API with no local clone).
Acceptance Criteria Status
Collaborator roles defined with a permissions matrix ✅ already present (ROLE_PERMISSIONS, getPermissionsMatrix); unchanged
Permissions enforced server-side ✅ assertProjectPermission() + createProjectPermissionMiddleware(), applied to the delete route
A caller cannot grant itself a role ✅ the body role field is removed and a claimed role: "owner" is asserted to be ignored
Denied by default when the role cannot be established ✅ unknown requester / unavailable record / resolver error all return 403 permission_denied

@drips-wave

drips-wave Bot commented Sep 27, 2026

Copy link
Copy Markdown

@hi-bit-ke Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@SheyeJDev

Copy link
Copy Markdown
Contributor

LGTM

@SheyeJDev
SheyeJDev merged commit 4a4bec2 into Split-Naira:main Sep 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Product] Add collaborator role permissions

2 participants