From f91cd2374d22cb4e4a30a832b944226eadbaa8e2 Mon Sep 17 00:00:00 2001 From: codemagician Date: Sat, 29 Aug 2026 20:28:05 +0100 Subject: [PATCH] fix: require auth + identity binding on POST /profiles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit UserProfileController.create() had no @UseGuards(JwtAuthGuard) at all — unlike update/delete/rate/verify on the same controller — and never checked that the caller actually controlled the walletAddress in the request body. Anyone could POST /profiles with someone else's real Stellar wallet address, name, and skills, squatting on that identity before the real owner ever registered; since create() throws ConflictException on a duplicate walletAddress, the real owner would then be locked out of creating their own profile. Fix: - Add @UseGuards(JwtAuthGuard) + @ApiBearerAuth() to POST /profiles - Compare the parsed dto.walletAddress against req.user.address (from the verified JWT via JwtStrategy.validate()) and throw ForbiddenException on a mismatch - Update the Swagger @ApiOperation description and add 401/403 @ApiResponse entries Tests (backend/src/user-profile/user-profile.e2e.spec.ts, new — runs the real JwtAuthGuard/JwtStrategy pipeline via supertest against a live Nest app, not a mocked guard): - unauthenticated POST /profiles -> 401 - authenticated but walletAddress != req.user.address -> 403 - authenticated with matching walletAddress -> 201 - regression: an attacker can no longer squat on an address before its real owner registers (403 for the attacker, 201 for the real owner) Note: named user-profile.e2e.spec.ts (dot, not the existing auth.e2e-spec.ts dash convention) because jest's testRegex here is .*\.spec\.ts$, which does NOT match *.e2e-spec.ts — auth.e2e-spec.ts is consequently never actually run by `npm test`. Left that pre-existing file as-is since fixing it is outside this issue's scope, but named the new file so it actually executes. This PR only resolves #205. #203 (webhook event filtering), #206 (rateUser identity spoofing / duplicate ratings), and #207 (verifyUser admin enforcement) are real, separate issues from the same batch — not addressed here, left open for follow-up. Closes #205 --- .../user-profile/user-profile.controller.ts | 32 ++++- .../src/user-profile/user-profile.e2e.spec.ts | 110 ++++++++++++++++++ 2 files changed, 140 insertions(+), 2 deletions(-) create mode 100644 backend/src/user-profile/user-profile.e2e.spec.ts diff --git a/backend/src/user-profile/user-profile.controller.ts b/backend/src/user-profile/user-profile.controller.ts index a5e5589..8c2986b 100644 --- a/backend/src/user-profile/user-profile.controller.ts +++ b/backend/src/user-profile/user-profile.controller.ts @@ -7,12 +7,14 @@ import { Body, Param, Query, + Req, UploadedFile, UseInterceptors, HttpCode, HttpStatus, UseGuards, BadRequestException, + ForbiddenException, } from '@nestjs/common'; import { FileInterceptor } from '@nestjs/platform-express'; import { @@ -38,6 +40,14 @@ import { UserType, UserStatus } from './user-profile.entity'; import { JwtAuthGuard } from '../auth/auth.guard'; import { S3StorageService } from './s3-storage.service'; +/** + * Shape of `req.user` once `JwtAuthGuard` has run — see + * `JwtStrategy.validate()` (src/auth/jwt.strategy.ts). + */ +interface AuthenticatedRequest { + user: { address: string; sub: string }; +} + @ApiTags('User Profiles') @Controller('profiles') export class UserProfileController { @@ -47,10 +57,14 @@ export class UserProfileController { ) {} @Post() + @UseGuards(JwtAuthGuard) + @ApiBearerAuth() @ApiOperation({ summary: 'Create a new user profile', description: - 'Creates a new user profile for a freelancer or client. Requires a unique Stellar wallet address.', + 'Creates a new user profile for a freelancer or client. Requires authentication; ' + + '`walletAddress` must match the wallet address on the authenticated JWT — you can only ' + + 'create a profile for the wallet you signed in with, not on behalf of another address.', }) @ApiBody({ description: 'User profile creation details', @@ -140,9 +154,23 @@ export class UserProfileController { }, }) @ApiResponse({ status: 400, description: 'Invalid input data' }) + @ApiResponse({ status: 401, description: 'Unauthorized' }) + @ApiResponse({ + status: 403, + description: 'walletAddress does not match the authenticated wallet', + }) @ApiResponse({ status: 409, description: 'Profile with this wallet address already exists' }) - async create(@Body() dto: CreateUserProfileDto) { + async create(@Body() dto: CreateUserProfileDto, @Req() req: AuthenticatedRequest) { const validated = CreateUserProfileSchema.parse(dto); + + // The caller can only ever prove ownership of the wallet address on + // their own JWT — trusting walletAddress straight from the body would + // let anyone squat on someone else's real address before its owner + // ever registers (see #205). + if (validated.walletAddress !== req.user.address) { + throw new ForbiddenException('walletAddress must match the authenticated wallet address'); + } + return this.userProfileService.create(validated); } diff --git a/backend/src/user-profile/user-profile.e2e.spec.ts b/backend/src/user-profile/user-profile.e2e.spec.ts new file mode 100644 index 0000000..30abcfc --- /dev/null +++ b/backend/src/user-profile/user-profile.e2e.spec.ts @@ -0,0 +1,110 @@ +import { Test, TestingModule } from '@nestjs/testing'; +import { INestApplication, ValidationPipe } from '@nestjs/common'; +import request from 'supertest'; +import { AuthModule } from '../auth/auth.module'; +import { AuthService } from '../auth/auth.service'; +import { RedisModule } from '../common/redis/redis.module'; +import { UserProfileModule } from './user-profile.module'; +import { UserType } from './user-profile.entity'; + +// Covers #205: POST /profiles previously had no auth guard at all, so +// anyone could create a profile for any wallet address without proving +// they controlled it. These tests exercise the real JwtAuthGuard/ +// JwtStrategy pipeline (not a mocked guard) via supertest, the same way +// auth.e2e-spec.ts does for the auth flow itself. +describe('UserProfile (E2E) — POST /profiles auth', () => { + let app: INestApplication; + let authService: AuthService; + + // Stellar addresses are base32 (A-Z, 2-7 only — no 0/1/8/9), enforced by + // STELLAR_ADDRESS_REGEX in user-profile.dto.ts; these must satisfy it or + // CreateUserProfileSchema.parse() throws before the auth check ever runs. + const OWNER_ADDRESS = 'G' + 'A'.repeat(55); + const OTHER_ADDRESS = 'G' + 'B'.repeat(55); + + beforeAll(async () => { + const moduleFixture: TestingModule = await Test.createTestingModule({ + // RedisModule is @Global() in the real app (bootstrapped once via + // AppModule), but a standalone TestingModule needs it imported + // explicitly — AuthModule's NonceStoreService depends on its + // REDIS_CLIENT token even though nothing in this spec touches nonces. + imports: [RedisModule, AuthModule, UserProfileModule], + }).compile(); + + app = moduleFixture.createNestApplication(); + app.useGlobalPipes( + new ValidationPipe({ whitelist: true, transform: true, forbidNonWhitelisted: true }), + ); + await app.init(); + + authService = moduleFixture.get(AuthService); + }); + + afterAll(async () => { + await app.close(); + }); + + function payload(walletAddress: string) { + return { + walletAddress, + name: 'Jane Doe', + userType: UserType.FREELANCER, + }; + } + + it('rejects an unauthenticated request', async () => { + const res = await request(app.getHttpServer()) + .post('/profiles') + .send(payload(OWNER_ADDRESS)) + .expect(401); + + expect(res.body.message).toBeDefined(); + }); + + it('rejects a walletAddress that does not match the authenticated wallet', async () => { + const token = authService.generateToken(OWNER_ADDRESS); + + const res = await request(app.getHttpServer()) + .post('/profiles') + .set('Authorization', `Bearer ${token}`) + .send(payload(OTHER_ADDRESS)) + .expect(403); + + expect(res.body.message).toContain('walletAddress must match'); + }); + + it('creates the profile when walletAddress matches the authenticated wallet', async () => { + const token = authService.generateToken(OWNER_ADDRESS); + + const res = await request(app.getHttpServer()) + .post('/profiles') + .set('Authorization', `Bearer ${token}`) + .send(payload(OWNER_ADDRESS)) + .expect(201); + + expect(res.body.walletAddress).toBe(OWNER_ADDRESS); + }); + + it('rejects a re-registration attempt for an address that was never proven-owned', async () => { + // Regression for the original bug: an attacker who squatted on + // OTHER_ADDRESS before its real owner registered would have locked the + // real owner out via ConflictException. With the identity check in + // place, the attacker's request never gets past 403 in the first + // place, so the real owner's own authenticated attempt succeeds. + const attackerToken = authService.generateToken(OWNER_ADDRESS); + await request(app.getHttpServer()) + .post('/profiles') + .set('Authorization', `Bearer ${attackerToken}`) + .send(payload(OTHER_ADDRESS)) + .expect(403); + + const realOwnerToken = authService.generateToken(OTHER_ADDRESS); + const res = await request(app.getHttpServer()) + .post('/profiles') + .set('Authorization', `Bearer ${realOwnerToken}`) + .send(payload(OTHER_ADDRESS)) + .expect(201); + + expect(res.body.walletAddress).toBe(OTHER_ADDRESS); + }); +});