GET /users and GET /users/:id are fully unauthenticated, exposing every user's email and other PII with no field-level restriction

[chonilius]

Overview

GET /users and GET /users/:id have no authentication guard and no field-level restriction on what's returned — they expose the complete User entity, including email, for anyone who can reach the API:

// src/users/users.controller.ts:17-25
@Get()
list() {
  return this.usersService.list();
}

@Get(':id')
findOne(@Param('id') id: string) {
  return this.usersService.findById(id);
}

Compare to the one route on this exact controller that does get it right — @UseGuards(JwtAuthGuard) is applied to setStellarAddress (:27-29, itself the subject of a companion IDOR issue in this batch, but at least gated) — and nothing else on the controller. UsersService.list() (users.service.ts:101-103) is this.userRepo.find() with no field selection, so the response includes every column on User: email (unique, user.entity.ts:17-18), username, displayName, avatarUrl, stellarAddress, roles, createdAt/updatedAt. findById additionally loads the githubAccount relation (users.service.ts:26-33) — the select: false option on GithubAccount.accessToken/refreshToken (github-account.entity.ts:34,37) should exclude those two specific columns from that join, but everything else on the linked GitHub account (login, profileUrl, avatarUrl) is included by default alongside it.

The consequence is a fully open, unauthenticated PII enumeration surface: GET /users (which, per the companion "no pagination" issue, returns the entire user table in one call, with no limit) lets anyone scrape every registered user's email address, GitHub login, and linked Stellar wallet address in one request, no credentials required. GET /users/:id lets anyone iterating over UUIDs (or, more practically, UUIDs harvested from the equally-unauthenticated bounty.claimedById/bounty.sponsorId fields returned by GET /bounties) pull the same PII for a specific targeted individual. This is a genuinely different consequence category from the money-moving mutation endpoints the companion "no auth at all" issue is centered on — that issue's fund-safety framing (fund/claim/release/refund) doesn't naturally cover a pure-read PII exposure like this one, and its remediation (add JwtAuthGuard to the mutating routes on bounties/escrow/teams/milestones/maintenance-pool) explicitly doesn't list UsersController's GETs at all — this issue exists specifically to make sure those two GET routes aren't accidentally left out when that broader fix lands, since a plain "add guards to the mutation endpoints" pass would reasonably interpret read-only GETs as out of scope.

There's also a concrete practical exploit chain worth naming: email is exactly the field the companion OAuth account-linking issue in this repo flags as a potential account-takeover vector if GitHub ever returns an unverified email for OAuth matching. An attacker who can freely enumerate every registered user's email via this endpoint has a ready-made target list for exactly that attack, for free, with no guessing required.

Requirements

• Add authentication to GET /users and GET /users/:id — at minimum requiring a valid JWT (consistent with the rest of this batch's auth-hardening work), and consider whether email specifically should be restricted further even for authenticated callers (e.g. visible only to the user themselves or an admin, with other authenticated users seeing a public-safe subset: username, displayName, avatarUrl, stellarAddress).
• Introduce a public-facing DTO/serialization shape for User (mirroring the existing toPublicEscrow/PublicEscrow pattern already established in src/escrow/escrow-response.mapper.ts for exactly this kind of "strip sensitive fields before they reach an HTTP response" concern) rather than returning the raw entity, so email (and anything else added to User in the future) doesn't leak by default the next time a field is added to the entity.
• Coordinate with the companion "no pagination" issue — pagination alone doesn't fix the authentication/field-exposure gap, and this fix alone doesn't fix the "returns unbounded rows" gap; both need to land for GET /users to be safe.
• Add a test asserting an unauthenticated GET /users/GET /users/:id request is rejected, and that an authenticated-but-unprivileged caller's response (if the further field-restriction option is chosen) excludes email for users other than themselves.

Acceptance Criteria

• GET /users and GET /users/:id require authentication.
• Responses no longer include email (or any other field the team decides is sensitive) for arbitrary authenticated callers, only for the user themselves or an admin — or, if full exposure to any authenticated user is a deliberate product decision, that decision is documented explicitly in the PR rather than left as an accident of the entity being serialized directly.
• A public-response DTO/mapper exists for User, following the toPublicEscrow pattern already in this codebase.
• A test proves unauthenticated access is rejected and that sensitive fields are appropriately scoped for authenticated-but-unrelated callers.

Additional Notes

Precise references: src/users/users.controller.ts:17-25 (the two unguarded routes), :27-35 (the one guarded route on the same controller, showing the pattern that should have been applied here too), src/users/users.service.ts:26-37,101-103 (findById/list, both returning the raw entity with no field restriction), src/common/entities/user.entity.ts:17-18,38-39 (email, stellarAddress — the two most sensitive plain columns on this entity), src/common/entities/github-account.entity.ts:34,37 (accessToken/refreshToken's select: false, confirmed correctly scoped — the only field-level protection currently in place anywhere on this entity graph, everything else is wide open), src/escrow/escrow-response.mapper.ts:1-22 (the existing, already-adopted pattern for exactly this kind of fix elsewhere in the codebase).

Test/reproduction plan:

const res = await request(app).get('/users').expect(401);
// pre-fix: 200, full array of every user including .email for each

const res2 = await request(app).get(`/users/${someUser.id}`).expect(401);
// pre-fix: 200, { ..., email: 'someone@example.com', stellarAddress: 'G...', ... }

Cross-references: deliberately scoped separately from the companion "no auth at all" issue (that issue's own text and requirements are centered on mutating money-moving routes and explicitly does not enumerate UsersController's GETs) and from the companion "no pagination" issue (fixes a different half of the same overall exposure — this issue is about who can see the data and which fields, that issue is about how much comes back in one response). Also relevant to the open "OAuth account-linking flow" issue's own concern about unverified-email account-takeover — this issue's fix removes the free target list that attack would otherwise have.

[gramseostudio-dev]

I am ready to resolve this PII exposure vulnerability. My approach will be to secure the UsersController GET routes by implementing the JwtAuthGuard. To ensure sensitive fields like emails don't leak, I will introduce a toPublicUser DTO mapping function, mirroring the established toPublicEscrow pattern. Finally, I will write the required E2E tests to guarantee unauthenticated requests return a 401 and that authenticated responses strictly scope data access.

[grantfox-oss]

🦊 GrantFox — @gramseostudio-dev has been assigned to this issue as part of the Third Campaign campaign!

Next steps:

1. Open a Pull Request referencing this issue (e.g., Closes #64)
2. Your PR will be reviewed by the MergeFi maintainers

Good luck! Track your progress on GrantFox.