Merge branch 'security/sakuin/guards-controllers'

# Conflicts:
#	backend/src/work/work.controller.ts

backend/package-lock.json

@@ -13,6 +13,7 @@
         "@nestjs/config": "^4.0.3",
         "@nestjs/core": "^11.1.17",
         "@nestjs/platform-express": "^11.1.17",
+        "@nestjs/throttler": "^6.5.0",
         "@nestjs/typeorm": "^11.0.0",
         "class-transformer": "^0.5.1",
         "class-validator": "^0.15.1",
@@ -2141,6 +2142,17 @@
         }
       }
     },
+    "node_modules/@nestjs/throttler": {
+      "version": "6.5.0",
+      "resolved": "https://registry.npmjs.org/@nestjs/throttler/-/throttler-6.5.0.tgz",
+      "integrity": "sha512-9j0ZRfH0QE1qyrj9JjIRDz5gQLPqq9yVC2nHsrosDVAfI5HHw08/aUAWx9DZLSdQf4HDkmhTTEGLrRFHENvchQ==",
+      "license": "MIT",
+      "peerDependencies": {
+        "@nestjs/common": "^7.0.0 || ^8.0.0 || ^9.0.0 || ^10.0.0 || ^11.0.0",
+        "@nestjs/core": "^7.0.0 || ^8.0.0 || ^9.0.0 || ^10.0.0 || ^11.0.0",
+        "reflect-metadata": "^0.1.13 || ^0.2.0"
+      }
+    },
     "node_modules/@nestjs/typeorm": {
       "version": "11.0.0",
       "resolved": "https://registry.npmjs.org/@nestjs/typeorm/-/typeorm-11.0.0.tgz",

backend/package.json

@@ -21,6 +21,7 @@
     "@nestjs/config": "^4.0.3",
     "@nestjs/core": "^11.1.17",
     "@nestjs/platform-express": "^11.1.17",
+    "@nestjs/throttler": "^6.5.0",
     "@nestjs/typeorm": "^11.0.0",
     "class-transformer": "^0.5.1",
     "class-validator": "^0.15.1",

backend/src/app.module.ts

@@ -1,6 +1,8 @@
 import { Module } from '@nestjs/common';
+import { APP_GUARD } from '@nestjs/core';
 import { ConfigModule, ConfigService } from '@nestjs/config';
 import { TypeOrmModule } from '@nestjs/typeorm';
+import { ThrottlerModule, ThrottlerGuard } from '@nestjs/throttler';
 import { getDatabaseConfig } from './config/database.config';
 import { HealthModule } from './health/health.module';
 import { AuthModule } from './auth/auth.module';
@@ -11,6 +13,7 @@ import { ListModule } from './list/list.module';
 @Module({
   imports: [
     ConfigModule.forRoot({ isGlobal: true }),
+    ThrottlerModule.forRoot([{ ttl: 60000, limit: 60 }]),
     TypeOrmModule.forRootAsync({
       inject: [ConfigService],
       useFactory: getDatabaseConfig,
@@ -21,5 +24,8 @@ import { ListModule } from './list/list.module';
     WorkModule,
     ListModule,
   ],
+  providers: [
+    { provide: APP_GUARD, useClass: ThrottlerGuard },
+  ],
 })
 export class AppModule {}

backend/src/auth/auth.guard.spec.ts

@@ -0,0 +1,153 @@
+import { ExecutionContext, UnauthorizedException } from '@nestjs/common';
+import { ConfigService } from '@nestjs/config';
+import { AuthGuard } from './auth.guard';
+
+// ── helpers ──────────────────────────────────────────────────────────
+
+const SUPEROAUTH_URL = 'http://oauth.test';
+
+const fakeUser = {
+  id: 42,
+  nickname: 'tetard',
+  email: 'tetard@test.com',
+};
+
+const oauthResponse = {
+  success: true,
+  data: {
+    user: fakeUser,
+    linkedProviders: [{ avatar: 'https://img.test/avatar.png' }],
+  },
+};
+
+function makeContext(token?: string): ExecutionContext {
+  const request: any = {
+    headers: token ? { authorization: `Bearer ${token}` } : {},
+    cookies: {},
+  };
+  return {
+    switchToHttp: () => ({
+      getRequest: () => request,
+    }),
+  } as unknown as ExecutionContext;
+}
+
+function makeGuard(): AuthGuard {
+  const configService = {
+    get: jest.fn().mockReturnValue(SUPEROAUTH_URL),
+  } as unknown as ConfigService;
+  return new AuthGuard(configService);
+}
+
+// ── tests ────────────────────────────────────────────────────────────
+
+describe('AuthGuard', () => {
+  let fetchSpy: jest.SpiedFunction<typeof global.fetch>;
+
+  beforeEach(() => {
+    fetchSpy = jest.spyOn(global, 'fetch').mockResolvedValue({
+      ok: true,
+      json: async () => oauthResponse,
+    } as Response);
+  });
+
+  afterEach(() => {
+    fetchSpy.mockRestore();
+  });
+
+  it('calls SuperOAuth when token is not cached', async () => {
+    const guard = makeGuard();
+    const ctx = makeContext('fresh-token');
+
+    await guard.canActivate(ctx);
+
+    expect(fetchSpy).toHaveBeenCalledTimes(1);
+    expect(fetchSpy).toHaveBeenCalledWith(
+      `${SUPEROAUTH_URL}/api/v1/user/profile`,
+      { headers: { Authorization: 'Bearer fresh-token' } },
+    );
+  });
+
+  it('returns cached user without calling SuperOAuth on second call', async () => {
+    const guard = makeGuard();
+
+    // First call — populates cache
+    await guard.canActivate(makeContext('cached-token'));
+    expect(fetchSpy).toHaveBeenCalledTimes(1);
+
+    // Second call — should hit cache
+    const ctx2 = makeContext('cached-token');
+    await guard.canActivate(ctx2);
+    expect(fetchSpy).toHaveBeenCalledTimes(1); // still 1 — no new fetch
+
+    const req = ctx2.switchToHttp().getRequest() as any;
+    expect(req.user).toEqual({
+      id: 42,
+      nickname: 'tetard',
+      avatar: 'https://img.test/avatar.png',
+    });
+  });
+
+  it('calls SuperOAuth again when cached entry has expired', async () => {
+    const guard = makeGuard();
+
+    // First call
+    await guard.canActivate(makeContext('expiring-token'));
+    expect(fetchSpy).toHaveBeenCalledTimes(1);
+
+    // Fast-forward time past TTL (5 min + 1 ms)
+    const realNow = Date.now;
+    Date.now = jest.fn().mockReturnValue(realNow() + 5 * 60 * 1000 + 1);
+
+    try {
+      await guard.canActivate(makeContext('expiring-token'));
+      expect(fetchSpy).toHaveBeenCalledTimes(2); // re-fetched
+    } finally {
+      Date.now = realNow;
+    }
+  });
+
+  it('throws UnauthorizedException when no token is provided', async () => {
+    const guard = makeGuard();
+    await expect(guard.canActivate(makeContext())).rejects.toThrow(
+      UnauthorizedException,
+    );
+    expect(fetchSpy).not.toHaveBeenCalled();
+  });
+
+  it('throws UnauthorizedException when SuperOAuth rejects the token', async () => {
+    fetchSpy.mockResolvedValueOnce({
+      ok: false,
+      json: async () => ({}),
+    } as Response);
+
+    const guard = makeGuard();
+    await expect(guard.canActivate(makeContext('bad-token'))).rejects.toThrow(
+      UnauthorizedException,
+    );
+  });
+
+  it('does not cache a failed introspection', async () => {
+    fetchSpy.mockResolvedValue({
+      ok: false,
+      json: async () => ({}),
+    } as Response);
+
+    const guard = makeGuard();
+
+    // First call fails
+    await expect(guard.canActivate(makeContext('retry-token'))).rejects.toThrow(
+      UnauthorizedException,
+    );
+
+    // Restore successful response
+    fetchSpy.mockResolvedValue({
+      ok: true,
+      json: async () => oauthResponse,
+    } as Response);
+
+    // Second call should hit SuperOAuth again (not cached failure)
+    await guard.canActivate(makeContext('retry-token'));
+    expect(fetchSpy).toHaveBeenCalledTimes(2);
+  });
+});

backend/src/auth/auth.guard.ts

@@ -4,13 +4,34 @@ import {
   Injectable,
   UnauthorizedException,
 } from '@nestjs/common';
+import { Reflector } from '@nestjs/core';
 import { ConfigService } from '@nestjs/config';
+import { IS_PUBLIC_KEY } from './public.decorator';
+
+interface CacheEntry {
+  user: any;
+  expiresAt: number;
+}
+
+const TOKEN_CACHE_TTL_MS = 5 * 60 * 1000; // 5 minutes
+const MAX_CACHE_SIZE = 1000;
 
 @Injectable()
 export class AuthGuard implements CanActivate {
-  constructor(private readonly configService: ConfigService) {}
+  private readonly cache = new Map<string, CacheEntry>();
+
+  constructor(
+    private readonly configService: ConfigService,
+    private readonly reflector: Reflector,
+  ) {}
 
   async canActivate(context: ExecutionContext): Promise<boolean> {
+    const isPublic = this.reflector.getAllAndOverride<boolean>(IS_PUBLIC_KEY, [
+      context.getHandler(),
+      context.getClass(),
+    ]);
+    if (isPublic) return true;
+
     const request = context.switchToHttp().getRequest();
     const token = this.extractToken(request);
 
@@ -18,7 +39,7 @@ export class AuthGuard implements CanActivate {
       throw new UnauthorizedException('No token provided');
     }
 
-    const userInfo = await this.introspect(token);
+    const userInfo = await this.resolveUser(token);
     if (!userInfo) {
       throw new UnauthorizedException('Invalid token');
     }
@@ -27,6 +48,31 @@ export class AuthGuard implements CanActivate {
     return true;
   }
 
+  private async resolveUser(token: string): Promise<any> {
+    const cached = this.cache.get(token);
+    if (cached && cached.expiresAt > Date.now()) {
+      return cached.user;
+    }
+
+    // Lazy cleanup: remove this expired entry
+    if (cached) {
+      this.cache.delete(token);
+    }
+
+    const user = await this.introspect(token);
+    if (user) {
+      if (this.cache.size >= MAX_CACHE_SIZE) {
+        const oldest = this.cache.keys().next().value;
+        if (oldest) this.cache.delete(oldest);
+      }
+      this.cache.set(token, {
+        user,
+        expiresAt: Date.now() + TOKEN_CACHE_TTL_MS,
+      });
+    }
+    return user;
+  }
+
   private extractToken(request: any): string | null {
     const authHeader = request.headers.authorization;
     if (authHeader?.startsWith('Bearer ')) {

backend/src/auth/public.decorator.ts

@@ -0,0 +1,4 @@
+import { SetMetadata } from '@nestjs/common';
+
+export const IS_PUBLIC_KEY = 'isPublic';
+export const Public = () => SetMetadata(IS_PUBLIC_KEY, true);

backend/src/list/dto/add-to-list.dto.ts

@@ -0,0 +1,11 @@
+import { IsEnum, IsInt, Min } from 'class-validator';
+import { ListStatus } from '../user-work.entity';
+
+export class AddToListDto {
+  @IsInt()
+  @Min(1)
+  anilistId: number;
+
+  @IsEnum(ListStatus)
+  status: ListStatus;
+}

backend/src/list/dto/dto-validation.spec.ts

@@ -0,0 +1,168 @@
+import { validate } from 'class-validator';
+import { plainToInstance } from 'class-transformer';
+import { AddToListDto } from './add-to-list.dto';
+import { UpdateProgressDto } from './update-progress.dto';
+import { UpdateStatusDto } from './update-status.dto';
+import { SetScoreDto } from './set-score.dto';
+import { ListStatus } from '../user-work.entity';
+
+// Helper: transform plain object to class instance and validate
+async function check<T extends object>(
+  cls: new () => T,
+  plain: Record<string, any>,
+): Promise<string[]> {
+  const instance = plainToInstance(cls, plain);
+  const errors = await validate(instance);
+  return errors.flatMap((e) => Object.values(e.constraints ?? {}));
+}
+
+// ─── AddToListDto ─────────────────────────────────────────────
+
+describe('AddToListDto', () => {
+  it('should accept valid input', async () => {
+    const errors = await check(AddToListDto, {
+      anilistId: 12345,
+      status: ListStatus.WATCHING,
+    });
+    expect(errors).toHaveLength(0);
+  });
+
+  it('should reject missing anilistId', async () => {
+    const errors = await check(AddToListDto, {
+      status: ListStatus.WATCHING,
+    });
+    expect(errors.length).toBeGreaterThan(0);
+  });
+
+  it('should reject non-integer anilistId', async () => {
+    const errors = await check(AddToListDto, {
+      anilistId: 'abc',
+      status: ListStatus.WATCHING,
+    });
+    expect(errors.length).toBeGreaterThan(0);
+  });
+
+  it('should reject anilistId < 1', async () => {
+    const errors = await check(AddToListDto, {
+      anilistId: 0,
+      status: ListStatus.WATCHING,
+    });
+    expect(errors.length).toBeGreaterThan(0);
+  });
+
+  it('should reject missing status', async () => {
+    const errors = await check(AddToListDto, {
+      anilistId: 12345,
+    });
+    expect(errors.length).toBeGreaterThan(0);
+  });
+
+  it('should reject invalid status', async () => {
+    const errors = await check(AddToListDto, {
+      anilistId: 12345,
+      status: 'binge_watching',
+    });
+    expect(errors.length).toBeGreaterThan(0);
+  });
+
+  it('should accept all valid ListStatus values', async () => {
+    for (const status of Object.values(ListStatus)) {
+      const errors = await check(AddToListDto, { anilistId: 1, status });
+      expect(errors).toHaveLength(0);
+    }
+  });
+});
+
+// ─── UpdateProgressDto ───────────────────────────────────────
+
+describe('UpdateProgressDto', () => {
+  it('should accept valid progress', async () => {
+    const errors = await check(UpdateProgressDto, { progress: 5 });
+    expect(errors).toHaveLength(0);
+  });
+
+  it('should accept progress = 0', async () => {
+    const errors = await check(UpdateProgressDto, { progress: 0 });
+    expect(errors).toHaveLength(0);
+  });
+
+  it('should reject negative progress', async () => {
+    const errors = await check(UpdateProgressDto, { progress: -1 });
+    expect(errors.length).toBeGreaterThan(0);
+  });
+
+  it('should reject non-integer progress', async () => {
+    const errors = await check(UpdateProgressDto, { progress: 3.5 });
+    expect(errors.length).toBeGreaterThan(0);
+  });
+
+  it('should reject missing progress', async () => {
+    const errors = await check(UpdateProgressDto, {});
+    expect(errors.length).toBeGreaterThan(0);
+  });
+
+  it('should reject string progress', async () => {
+    const errors = await check(UpdateProgressDto, { progress: 'five' });
+    expect(errors.length).toBeGreaterThan(0);
+  });
+});
+
+// ─── UpdateStatusDto ─────────────────────────────────────────
+
+describe('UpdateStatusDto', () => {
+  it('should accept valid status', async () => {
+    const errors = await check(UpdateStatusDto, {
+      status: ListStatus.COMPLETED,
+    });
+    expect(errors).toHaveLength(0);
+  });
+
+  it('should reject missing status', async () => {
+    const errors = await check(UpdateStatusDto, {});
+    expect(errors.length).toBeGreaterThan(0);
+  });
+
+  it('should reject invalid status string', async () => {
+    const errors = await check(UpdateStatusDto, { status: 'finished' });
+    expect(errors.length).toBeGreaterThan(0);
+  });
+});
+
+// ─── SetScoreDto ─────────────────────────────────────────────
+
+describe('SetScoreDto', () => {
+  it('should accept valid score', async () => {
+    const errors = await check(SetScoreDto, { score: 8.5 });
+    expect(errors).toHaveLength(0);
+  });
+
+  it('should accept score = 0', async () => {
+    const errors = await check(SetScoreDto, { score: 0 });
+    expect(errors).toHaveLength(0);
+  });
+
+  it('should accept score = 10', async () => {
+    const errors = await check(SetScoreDto, { score: 10 });
+    expect(errors).toHaveLength(0);
+  });
+
+  it('should reject score > 10', async () => {
+    const errors = await check(SetScoreDto, { score: 11 });
+    expect(errors.length).toBeGreaterThan(0);
+  });
+
+  it('should reject negative score', async () => {
+    const errors = await check(SetScoreDto, { score: -1 });
+    expect(errors.length).toBeGreaterThan(0);
+  });
+
+  it('should reject missing score', async () => {
+    const errors = await check(SetScoreDto, {});
+    expect(errors.length).toBeGreaterThan(0);
+  });
+
+  it('should reject string score', async () => {
+    const errors = await check(SetScoreDto, { score: 'great' });
+    expect(errors.length).toBeGreaterThan(0);
+  });
+});

backend/src/list/dto/set-score.dto.ts

@@ -0,0 +1,8 @@
+import { IsNumber, Min, Max } from 'class-validator';
+
+export class SetScoreDto {
+  @IsNumber()
+  @Min(0)
+  @Max(10)
+  score: number;
+}

backend/src/list/dto/update-progress.dto.ts

@@ -0,0 +1,7 @@
+import { IsInt, Min } from 'class-validator';
+
+export class UpdateProgressDto {
+  @IsInt()
+  @Min(0)
+  progress: number;
+}

backend/src/list/dto/update-status.dto.ts

@@ -0,0 +1,7 @@
+import { IsEnum } from 'class-validator';
+import { ListStatus } from '../user-work.entity';
+
+export class UpdateStatusDto {
+  @IsEnum(ListStatus)
+  status: ListStatus;
+}

backend/src/list/list.controller.ts

@@ -15,6 +15,10 @@ import { ListService } from './list.service';
 import { ListStatus } from './user-work.entity';
 import { AuthGuard } from '../auth/auth.guard';
 import { UserService } from '../user/user.service';
+import { AddToListDto } from './dto/add-to-list.dto';
+import { UpdateProgressDto } from './dto/update-progress.dto';
+import { UpdateStatusDto } from './dto/update-status.dto';
+import { SetScoreDto } from './dto/set-score.dto';
 
 @Controller('api/list')
 @UseGuards(AuthGuard)
@@ -37,7 +41,7 @@ export class ListController {
   @Post()
   async addToList(
     @Req() req: any,
-    @Body() body: { anilistId: number; status: ListStatus },
+    @Body() body: AddToListDto,
   ) {
     const user = await this.userService.findOrCreate({
       id: req.user.id,
@@ -51,7 +55,7 @@ export class ListController {
   async updateProgress(
     @Req() req: any,
     @Param('id', ParseIntPipe) id: number,
-    @Body() body: { progress: number },
+    @Body() body: UpdateProgressDto,
   ) {
     const user = await this.userService.findOrCreate({
       id: req.user.id,
@@ -65,7 +69,7 @@ export class ListController {
   async updateStatus(
     @Req() req: any,
     @Param('id', ParseIntPipe) id: number,
-    @Body() body: { status: ListStatus },
+    @Body() body: UpdateStatusDto,
   ) {
     const user = await this.userService.findOrCreate({
       id: req.user.id,
@@ -79,7 +83,7 @@ export class ListController {
   async setScore(
     @Req() req: any,
     @Param('id', ParseIntPipe) id: number,
-    @Body() body: { score: number },
+    @Body() body: SetScoreDto,
   ) {
     const user = await this.userService.findOrCreate({
       id: req.user.id,

backend/src/list/list.service.spec.ts

@@ -233,21 +233,9 @@ describe('ListService', () => {
 
       await service.updateStatus(1, 1, ListStatus.COMPLETED);
 
-      // THIS TEST DOCUMENTS THE BUG: updateStatus awards XP_COMPLETE
-      // even when the entry is already COMPLETED.
-      //
-      // Current behavior (BUG): addXp IS called => test expects the call.
-      // When the fix lands, flip this assertion to:
-      //   expect(userService.addXp).not.toHaveBeenCalled();
-      //
-      // Until then, this test proves the double-XP path exists.
-      const secondXpCalls = userService.addXp.mock.calls.filter(
-        ([, amount]) => amount === XP_COMPLETE,
-      );
-      expect(secondXpCalls).toHaveLength(1); // BUG: this SHOULD be 0
-
-      // Total across both steps = 2 x XP_COMPLETE — the double-XP bug
-      // After fix, total should be exactly 1 x XP_COMPLETE
+      // FIX MERGED: updateStatus now guards against already-completed status.
+      // addXp should NOT be called a second time.
+      expect(userService.addXp).not.toHaveBeenCalled();
     });
   });
 

backend/src/user/user.controller.ts

@@ -2,12 +2,12 @@ import { Controller, Get, Req, UseGuards } from '@nestjs/common';
 import { UserService } from './user.service';
 import { AuthGuard } from '../auth/auth.guard';
 
+@UseGuards(AuthGuard)
 @Controller('api/user')
 export class UserController {
   constructor(private readonly userService: UserService) {}
 
   @Get('me')
-  @UseGuards(AuthGuard)
   async me(@Req() req: any) {
     const user = await this.userService.findOrCreate({
       id: req.user.id,

backend/src/work/work.controller.ts

@@ -1,10 +1,16 @@
-import { Controller, Get, Query } from '@nestjs/common';
+import { Controller, Get, Query, UseGuards } from '@nestjs/common';
+import { Throttle } from '@nestjs/throttler';
+import { AuthGuard } from '../auth/auth.guard';
+import { Public } from '../auth/public.decorator';
 import { WorkService } from './work.service';
 
+@UseGuards(AuthGuard)
 @Controller('api/works')
 export class WorkController {
   constructor(private readonly workService: WorkService) {}
 
+  @Public()
+  @Throttle([{ ttl: 60000, limit: 20 }])
   @Get('search')
   async search(
     @Query('q') query: string,