Skip to content

fix(security): guard sponsors/reputation routes, drop unreachable add… - #216

Merged
chonilius merged 1 commit into
MergeFi:mainfrom
Chidubemkingsley:fix/authz-guards-and-role-assignment
Aug 26, 2026
Merged

fix(security): guard sponsors/reputation routes, drop unreachable add…#216
chonilius merged 1 commit into
MergeFi:mainfrom
Chidubemkingsley:fix/authz-guards-and-role-assignment

Conversation

@Chidubemkingsley

@Chidubemkingsley Chidubemkingsley commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

closes #138
closes #137
closes #136
closes #135

Done. Pushed branch fix/authz-guards-and-role-assignment to the backend repo.
All four findings addressed:

  1. Misleading 401/403 test — renamed both users.e2e-spec.ts tests to "should return 403 when the guard denies the request" so titles match the expect(403) assertion against the mocked guard.
  2. Unreachable addRole — removed UsersService.addRole and its unit tests; documented (in users.service.ts) that role assignment is intentionally out of scope this version.
  3. Unguarded SponsorsController — added @UseGuards(JwtAuthGuard) + an ownership check (req.user.userId === sponsorId) to both /sponsors/:id/dashboard and /:id/milestones/progress.
  4. Unguarded ReputationController — added @UseGuards(JwtAuthGuard) to all routes and restricted GET /:userId, GET /:userId/history, and POST /:userId/recompute to the authenticated owner via the new CurrentUser decorator.
    Verification: tsc clean for all changed files; users.service.spec.ts (12) and users.e2e-spec.ts (2) pass. Pre-existing lint errors in users.e2e-spec.ts (the request(app.getHttpServer()) unsafe-argument) predate this change and were not introduced here.
    Branch: https://github.com/Chidubemkingsley/backend/pull/new/fix/authz-guards-and-role-assignment
    Note: I worked in the backend repo (/home/emperor-caesar/drips/backend), since the files described in the audit live there, not in smartdrop-frontend. Also worth flagging (out of scope here): UsersController.setStellarAddress has the same missing-ownership IDOR the codebase's own UsersController.setStellarAddress is authenticated but not authorized: any logged-in user can overwrite another user's payout address #39 comment calls out.

…Role

- SponsorsController: add JwtAuthGuard + ownership check on
  /sponsors/:id/dashboard and /sponsors/:id/milestones/progress so a
  sponsor's financials can only be read by that sponsor.
- ReputationController: guard all routes with JwtAuthGuard and restrict
  GET/history and POST recompute to the authenticated owner, preventing
  unauthenticated reputation-history leaks and arbitrary snapshot flooding.
- Add CurrentUser param decorator to safely extract the authenticated caller.
- Remove UsersService.addRole (unreachable via any endpoint) and document
  that role assignment is out of scope for this version.
- Rename misleading users e2e tests to describe the 403 behavior they
  actually assert against the mocked guard.
@vercel

vercel Bot commented Aug 26, 2026

Copy link
Copy Markdown

@Chidubemkingsley is attempting to deploy a commit to the chonilius' projects Team on Vercel.

A member of the Team first needs to authorize it.

@drips-wave

drips-wave Bot commented Aug 26, 2026

Copy link
Copy Markdown

@Chidubemkingsley 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

@chonilius
chonilius merged commit 4bbbf25 into MergeFi:main Aug 26, 2026
0 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment