Compare commits

..

4 Commits

Author SHA1 Message Date
7b7f2ac8e7 Merge branch 'refacto/sakuin/auth-cache'
Some checks failed
CI/CD — Build & Deploy / Build & Deploy (push) Failing after 14s
2026-04-05 07:41:44 +02:00
075afa1063 refacto: DTOs class-validator — validation active sur tous les endpoints 2026-04-05 07:41:01 +02:00
7106791372 refacto: AuthGuard token cache — memory TTL 5min, skip round-trip
Map<token, {user, expiresAt}> avec lazy cleanup.
Token en cache et valide → zero HTTP, latence ~0.
Expiration ou miss → round-trip SuperOAuth comme avant.
6 tests unitaires couvrent cache hit, miss, expiry, et edge cases.
2026-04-05 07:39:55 +02:00
6dcb6bf4b5 test: flip anti-regression double XP — fix merged, assertion inversée 2026-04-05 07:37:55 +02:00
9 changed files with 396 additions and 20 deletions

View File

@@ -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);
});
});

View File

@@ -6,8 +6,17 @@ import {
} from '@nestjs/common'; } from '@nestjs/common';
import { ConfigService } from '@nestjs/config'; import { ConfigService } from '@nestjs/config';
interface CacheEntry {
user: any;
expiresAt: number;
}
const TOKEN_CACHE_TTL_MS = 5 * 60 * 1000; // 5 minutes
@Injectable() @Injectable()
export class AuthGuard implements CanActivate { export class AuthGuard implements CanActivate {
private readonly cache = new Map<string, CacheEntry>();
constructor(private readonly configService: ConfigService) {} constructor(private readonly configService: ConfigService) {}
async canActivate(context: ExecutionContext): Promise<boolean> { async canActivate(context: ExecutionContext): Promise<boolean> {
@@ -18,7 +27,7 @@ export class AuthGuard implements CanActivate {
throw new UnauthorizedException('No token provided'); throw new UnauthorizedException('No token provided');
} }
const userInfo = await this.introspect(token); const userInfo = await this.resolveUser(token);
if (!userInfo) { if (!userInfo) {
throw new UnauthorizedException('Invalid token'); throw new UnauthorizedException('Invalid token');
} }
@@ -27,6 +36,27 @@ export class AuthGuard implements CanActivate {
return true; 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) {
this.cache.set(token, {
user,
expiresAt: Date.now() + TOKEN_CACHE_TTL_MS,
});
}
return user;
}
private extractToken(request: any): string | null { private extractToken(request: any): string | null {
const authHeader = request.headers.authorization; const authHeader = request.headers.authorization;
if (authHeader?.startsWith('Bearer ')) { if (authHeader?.startsWith('Bearer ')) {

View File

@@ -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;
}

View File

@@ -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);
});
});

View File

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

View File

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

View File

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

View File

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

View File

@@ -233,21 +233,9 @@ describe('ListService', () => {
await service.updateStatus(1, 1, ListStatus.COMPLETED); await service.updateStatus(1, 1, ListStatus.COMPLETED);
// THIS TEST DOCUMENTS THE BUG: updateStatus awards XP_COMPLETE // FIX MERGED: updateStatus now guards against already-completed status.
// even when the entry is already COMPLETED. // addXp should NOT be called a second time.
// expect(userService.addXp).not.toHaveBeenCalled();
// 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
}); });
}); });