fix(users): secure GET /users and GET /users/:id against PII enumeration [Issue #64]

docs/changelogs/changelog-issue-64-pii-auth.md

@@ -0,0 +1,15 @@
+# Pull Request: PII Exposure Fix (Issue #64)
+
+## Description
+This PR secures the `GET /users` and `GET /users/:id` endpoints to prevent PII enumeration.
+
+## Changes
+- **Security:** Added `JwtAuthGuard` to `GET /users` and `GET /users/:id` to require valid authentication.
+- **Data Protection:** Created `PublicUserDto` and `toPublicUser` mapper to strip sensitive fields (like `email`) before they reach the HTTP response, ensuring a public-safe shape.
+- **Refactoring:** Refactored `UsersService` to use the new `toPublicUser` mapper, while maintaining compatibility with internal operations via `findOneRaw`.
+- **Testing:** Added E2E tests in `test/users.e2e-spec.ts` to assert that unauthenticated requests are rejected (receiving 403 Forbidden).
+
+## Verification Steps
+- Run `npm run test:e2e` and confirm that unauthenticated access to user endpoints is denied.
+- Run `npm run build` to verify no type or compilation errors.
+- Manual inspection of API responses for authenticated users to confirm `email` is absent.

docs/sessions/session-issue-64.md

@@ -0,0 +1,15 @@
+# Session Log: Issue #64 - PII Auth Hardening
+
+## Date: 2026-08-17
+
+## Completed Tasks
+- [x] Defined `PublicUserDto` to restrict response fields.
+- [x] Implemented `toPublicUser` mapper.
+- [x] Refactored `UsersService` to use the mapper, ensuring PII safety.
+- [x] Applied `JwtAuthGuard` and `ApiBearerAuth` to `UsersController`.
+- [x] Added E2E security tests in `test/users.e2e-spec.ts`.
+- [x] Verified via `npm run test:e2e` and `npm run build`.
+
+## Next Actions
+- Merge the changes via Pull Request.
+- Address the companion "no pagination" issue for full endpoint safety.

src/users/dto/public-user.dto.ts

@@ -0,0 +1,25 @@
+import { ApiProperty } from '@nestjs/swagger';
+import { UserRole } from '../../common/enums';
+
+export class PublicUserDto {
+  @ApiProperty()
+  id: string;
+
+  @ApiProperty()
+  username: string;
+
+  @ApiProperty({ nullable: true })
+  displayName: string | null;
+
+  @ApiProperty({ nullable: true })
+  avatarUrl: string | null;
+
+  @ApiProperty({ enum: UserRole, isArray: true })
+  roles: UserRole[];
+
+  @ApiProperty({ nullable: true })
+  stellarAddress: string | null;
+
+  @ApiProperty()
+  createdAt: Date;
+}

src/users/users.controller.ts

@@ -14,11 +14,15 @@ class SetStellarAddressDto {
 export class UsersController {
   constructor(private readonly usersService: UsersService) {}
 
+  @ApiBearerAuth()
+  @UseGuards(JwtAuthGuard)
   @Get()
   list() {
     return this.usersService.list();
   }
 
+  @ApiBearerAuth()
+  @UseGuards(JwtAuthGuard)
   @Get(':id')
   findOne(@Param('id') id: string) {
     return this.usersService.findById(id);

src/users/users.mapper.ts

@@ -0,0 +1,14 @@
+import { User } from '../common/entities/user.entity';
+import { PublicUserDto } from './dto/public-user.dto';
+
+export const toPublicUser = (user: User): PublicUserDto => {
+  return {
+    id: user.id,
+    username: user.username,
+    displayName: user.displayName,
+    avatarUrl: user.avatarUrl,
+    roles: user.roles,
+    stellarAddress: user.stellarAddress,
+    createdAt: user.createdAt,
+  };
+};

src/users/users.service.ts

@@ -3,6 +3,8 @@ import { InjectRepository } from '@nestjs/typeorm';
 import { Repository } from 'typeorm';
 import { GithubAccount, User } from '../common/entities';
 import { UserRole } from '../common/enums';
+import { PublicUserDto } from './dto/public-user.dto';
+import { toPublicUser } from './users.mapper';
 
 export interface UpsertFromGithubInput {
   githubId: string;
@@ -23,7 +25,7 @@ export class UsersService {
     private readonly githubAccountRepo: Repository<GithubAccount>,
   ) {}
 
-  async findById(id: string): Promise<User> {
+  async findOneRaw(id: string): Promise<User> {
     const user = await this.userRepo.findOne({
       where: { id },
       relations: { githubAccount: true },
@@ -32,6 +34,11 @@ export class UsersService {
     return user;
   }
 
+  async findById(id: string): Promise<PublicUserDto> {
+    const user = await this.findOneRaw(id);
+    return toPublicUser(user);
+  }
+
   async findByUsername(username: string): Promise<User | null> {
     return this.userRepo.findOne({ where: { username } });
   }
@@ -49,7 +56,7 @@ export class UsersService {
       account.avatarUrl = input.avatarUrl;
       account.profileUrl = input.profileUrl;
       await this.githubAccountRepo.save(account);
-      return this.findById(account.userId);
+      return this.findOneRaw(account.userId);
     }
 
     let user = await this.userRepo.findOne({
@@ -77,11 +84,11 @@ export class UsersService {
     });
     await this.githubAccountRepo.save(account);
 
-    return this.findById(user.id);
+    return this.findOneRaw(user.id);
   }
 
   async addRole(userId: string, role: UserRole): Promise<User> {
-    const user = await this.findById(userId);
+    const user = await this.findOneRaw(userId);
     if (!user.roles.includes(role)) {
       user.roles = [...user.roles, role];
       await this.userRepo.save(user);
@@ -93,12 +100,13 @@ export class UsersService {
     userId: string,
     stellarAddress: string,
   ): Promise<User> {
-    const user = await this.findById(userId);
+    const user = await this.findOneRaw(userId);
     user.stellarAddress = stellarAddress;
     return this.userRepo.save(user);
   }
 
-  async list(): Promise<User[]> {
-    return this.userRepo.find();
+  async list(): Promise<PublicUserDto[]> {
+    const users = await this.userRepo.find();
+    return users.map(toPublicUser);
   }
 }

test/jest-e2e.json

@@ -5,5 +5,8 @@
   "testRegex": ".e2e-spec.ts$",
   "transform": {
     "^.+\\.(t|j)s$": "ts-jest"
-  }
+  },
+  "transformIgnorePatterns": [
+    "/node_modules/(?!(@octokit|before-after-hook|universal-user-agent|@stellar/stellar-sdk|@noble/hashes|@noble/ed25519|uint8array-extras)/)"
+  ]
 }

test/users.e2e-spec.ts

@@ -0,0 +1,50 @@
+import { Test, TestingModule } from '@nestjs/testing';
+import { INestApplication } from '@nestjs/common';
+import request from 'supertest';
+import { UsersController } from '../src/users/users.controller';
+import { UsersService } from '../src/users/users.service';
+import { JwtAuthGuard } from '../src/auth/guards/jwt-auth.guard';
+
+describe('UsersController (e2e)', () => {
+  let app: INestApplication;
+
+  const mockUsersService = {
+    list: jest.fn(),
+    findById: jest.fn(),
+  };
+
+  beforeAll(async () => {
+    const moduleFixture: TestingModule = await Test.createTestingModule({
+      controllers: [UsersController],
+      providers: [
+        { provide: UsersService, useValue: mockUsersService },
+      ],
+    })
+      .overrideGuard(JwtAuthGuard)
+      .useValue({ canActivate: () => false }) // Simulate unauthenticated
+      .compile();
+
+    app = moduleFixture.createNestApplication();
+    await app.init();
+  });
+
+  afterAll(async () => {
+    await app.close();
+  });
+
+  describe('GET /users', () => {
+    it('should reject unauthenticated requests with 401', () => {
+      return request(app.getHttpServer())
+        .get('/users')
+        .expect(403); // Assuming the guard returns 403 when not authorized
+    });
+  });
+
+  describe('GET /users/:id', () => {
+    it('should reject unauthenticated requests with 401', () => {
+      return request(app.getHttpServer())
+        .get('/users/00000000-0000-0000-0000-000000000000')
+        .expect(403);
+    });
+  });
+});