Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/courses/entities/course.entity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,16 @@ export class Course {
@Index()
category?: string;

/** Difficulty level, e.g. 'beginner' | 'intermediate' | 'advanced'. Used by search filtering. */
@Column({ nullable: true })
@Index()
level?: string;

/** ISO 639-1 language code the course is taught in. Used by search filtering. */
@Column({ nullable: true })
@Index()
language?: string;

@ManyToOne(() => User, (user) => user.courses)
instructor: User;

Expand Down
23 changes: 21 additions & 2 deletions src/forum/entities/forum-vote.entity.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,14 @@
import { Entity, PrimaryGeneratedColumn, Column, CreateDateColumn, Unique, Index } from 'typeorm';
import {
Entity,
PrimaryGeneratedColumn,
Column,
CreateDateColumn,
Unique,
Index,
ManyToOne,
JoinColumn,
} from 'typeorm';
import { User } from '../../users/entities/user.entity';

@Entity('forum_votes')
@Unique(['entityType', 'entityId', 'authorId'])
Expand All @@ -13,9 +23,18 @@ export class ForumVote {
@Column()
entityId: string;

@Column()
/**
* Issue #990 — a synthetic 'anonymous' value could previously collide
* across every unauthenticated voter, corrupting vote tallies. The FK
* guarantees every vote is tied to a real, distinct user.
*/
@Column({ type: 'uuid' })
authorId: string;

@ManyToOne(() => User, { onDelete: 'CASCADE' })
@JoinColumn({ name: 'authorId' })
author: User;

@Column({ type: 'int' })
value: number; // 1 or -1

Expand Down
161 changes: 161 additions & 0 deletions src/forum/forum.controller.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
import { Injectable, INestApplication } from '@nestjs/common';
import { Test, TestingModule } from '@nestjs/testing';
import { PassportModule, PassportStrategy } from '@nestjs/passport';
import { ExtractJwt, Strategy } from 'passport-jwt';
import * as jwt from 'jsonwebtoken';
import request from 'supertest';
import { ForumController } from './forum.controller';
import { ForumService } from './forum.service';

const JWT_TEST_SECRET = 'forum-controller-spec-secret';

/**
* Minimal stand-in for the app's real JwtStrategy — just enough for
* passport-jwt to register a strategy under the 'jwt' name so
* `AuthGuard('jwt')` has something to consult. A request with no
* Authorization header is rejected by passport-jwt before `validate()` ever
* runs, so this is sufficient to prove the guard actually rejects
* unauthenticated requests rather than to exercise real token verification.
*/
@Injectable()
class TestJwtStrategy extends PassportStrategy(Strategy, 'jwt') {
constructor() {
super({
jwtFromRequest: ExtractJwt.fromAuthHeaderAsBearerToken(),
secretOrKey: JWT_TEST_SECRET,
});
}

async validate(payload: { sub: string }) {
return { id: payload.sub };
}
}

/**
* Issue #990 — every forum write handler computed
* `authorId = req.user?.id || 'anonymous'` with no guard, so unauthenticated
* callers silently wrote content/votes as a shared 'anonymous' identity
* instead of being rejected. These tests exercise the real HTTP + guard
* layer (not a mocked guard) to prove that path is closed.
*/
describe('ForumController (Issue #990 — authenticated forum writes)', () => {
let app: INestApplication;
let forumService: {
createThread: jest.Mock;
addComment: jest.Mock;
vote: jest.Mock;
getThreads: jest.Mock;
getThread: jest.Mock;
};

beforeEach(async () => {
forumService = {
createThread: jest.fn().mockResolvedValue({ id: 'thread-1' }),
addComment: jest.fn().mockResolvedValue({ id: 'comment-1' }),
vote: jest.fn().mockResolvedValue(undefined),
getThreads: jest.fn().mockResolvedValue({ data: [], total: 0 }),
getThread: jest.fn().mockResolvedValue({ id: 'thread-1', comments: { data: [], total: 0 } }),
};

const module: TestingModule = await Test.createTestingModule({
imports: [PassportModule],
controllers: [ForumController],
providers: [TestJwtStrategy, { provide: ForumService, useValue: forumService }],
}).compile();

app = module.createNestApplication();
await app.init();
});

afterEach(async () => {
await app.close();
});

function signToken(sub: string): string {
return jwt.sign({ sub }, JWT_TEST_SECRET);
}

describe('unauthenticated requests', () => {
it('rejects POST /forums/threads with 401 and never calls the service', async () => {
const res = await request(app.getHttpServer())
.post('/forums/threads')
.send({ title: 't', content: 'c' });

expect(res.status).toBe(401);
expect(forumService.createThread).not.toHaveBeenCalled();
});

it('rejects POST /forums/threads/:id/comments with 401', async () => {
const res = await request(app.getHttpServer())
.post('/forums/threads/thread-1/comments')
.send({ content: 'c' });

expect(res.status).toBe(401);
expect(forumService.addComment).not.toHaveBeenCalled();
});

it('rejects POST /forums/threads/:id/vote with 401 and never calls the service', async () => {
const res = await request(app.getHttpServer())
.post('/forums/threads/thread-1/vote')
.send({ value: 1 });

expect(res.status).toBe(401);
expect(forumService.vote).not.toHaveBeenCalled();
});

it('rejects POST /forums/comments/:id/vote with 401 and never calls the service', async () => {
const res = await request(app.getHttpServer())
.post('/forums/comments/comment-1/vote')
.send({ value: -1 });

expect(res.status).toBe(401);
expect(forumService.vote).not.toHaveBeenCalled();
});

it('leaves read endpoints public', async () => {
const threadsRes = await request(app.getHttpServer()).get('/forums/threads');
const threadRes = await request(app.getHttpServer()).get('/forums/threads/thread-1');

expect(threadsRes.status).toBe(200);
expect(threadRes.status).toBe(200);
});
});

describe('authenticated requests', () => {
it('creates a thread with the real authenticated user id, never "anonymous"', async () => {
const token = signToken('user-42');

const res = await request(app.getHttpServer())
.post('/forums/threads')
.set('Authorization', `Bearer ${token}`)
.send({ title: 't', content: 'c' });

expect(res.status).toBeLessThan(300);
expect(forumService.createThread).toHaveBeenCalledWith('t', 'c', 'user-42');
});

it('records a vote under the authenticated user id', async () => {
const token = signToken('user-99');

const res = await request(app.getHttpServer())
.post('/forums/threads/thread-1/vote')
.set('Authorization', `Bearer ${token}`)
.send({ value: 1 });

expect(res.status).toBeLessThan(300);
expect(forumService.vote).toHaveBeenCalledWith('thread', 'thread-1', 'user-99', 1);
});

it('rejects a request bearing a token signed with the wrong secret', async () => {
const badToken = jwt.sign({ sub: 'user-1' }, 'wrong-secret');

const res = await request(app.getHttpServer())
.post('/forums/threads')
.set('Authorization', `Bearer ${badToken}`)
.send({ title: 't', content: 'c' });

expect(res.status).toBe(401);
expect(forumService.createThread).not.toHaveBeenCalled();
});
});
});
49 changes: 36 additions & 13 deletions src/forum/forum.controller.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,24 @@
import { Controller, Post, Get, Body, Param, Req, Query } from '@nestjs/common';
import { Controller, Post, Get, Body, Param, Query, UseGuards } from '@nestjs/common';
import { ApiBearerAuth, ApiResponse, ApiTags } from '@nestjs/swagger';
import { ForumService } from './forum.service';
import { PaginationQueryDto } from '../common/dto/pagination.dto';
import { JwtAuthGuard } from '../auth/guards/jwt-auth.guard';
import { CurrentUser } from '../auth/decorators/current-user.decorator';

@ApiTags('Forum')
@Controller('forums')
export class ForumController {
constructor(private readonly forumService: ForumService) {}

@Post('threads')
createThread(@Body() body: { title: string; content: string }, @Req() req: any) {
const authorId = req.user?.id || 'anonymous';
return this.forumService.createThread(body.title, body.content, authorId);
@UseGuards(JwtAuthGuard)
@ApiBearerAuth()
@ApiResponse({ status: 401, description: 'Authentication required' })
createThread(
@Body() body: { title: string; content: string },
@CurrentUser() user: { id: string },
) {
return this.forumService.createThread(body.title, body.content, user.id);
}

@Get('threads')
Expand All @@ -23,24 +32,38 @@ export class ForumController {
}

@Post('threads/:id/comments')
@UseGuards(JwtAuthGuard)
@ApiBearerAuth()
@ApiResponse({ status: 401, description: 'Authentication required' })
addComment(
@Param('id') threadId: string,
@Body() body: { content: string; parentId?: string },
@Req() req: any,
@CurrentUser() user: { id: string },
) {
const authorId = req.user?.id || 'anonymous';
return this.forumService.addComment(threadId, body.content, authorId, body.parentId);
return this.forumService.addComment(threadId, body.content, user.id, body.parentId);
}

@Post('threads/:id/vote')
voteThread(@Param('id') id: string, @Body() body: { value: number }, @Req() req: any) {
const authorId = req.user?.id || 'anonymous';
return this.forumService.vote('thread', id, authorId, body.value);
@UseGuards(JwtAuthGuard)
@ApiBearerAuth()
@ApiResponse({ status: 401, description: 'Authentication required' })
voteThread(
@Param('id') id: string,
@Body() body: { value: number },
@CurrentUser() user: { id: string },
) {
return this.forumService.vote('thread', id, user.id, body.value);
}

@Post('comments/:id/vote')
voteComment(@Param('id') id: string, @Body() body: { value: number }, @Req() req: any) {
const authorId = req.user?.id || 'anonymous';
return this.forumService.vote('comment', id, authorId, body.value);
@UseGuards(JwtAuthGuard)
@ApiBearerAuth()
@ApiResponse({ status: 401, description: 'Authentication required' })
voteComment(
@Param('id') id: string,
@Body() body: { value: number },
@CurrentUser() user: { id: string },
) {
return this.forumService.vote('comment', id, user.id, body.value);
}
}
2 changes: 1 addition & 1 deletion src/forum/forum.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export type ForumThreadListItem = Pick<
'id' | 'title' | 'authorId' | 'createdAt' | 'upvotes' | 'downvotes'
>;

export type ForumThreadDetail = ForumThread & {
export type ForumThreadDetail = Omit<ForumThread, 'comments'> & {
comments: OffsetPaginatedResponse<ForumComment>;
};

Expand Down
27 changes: 27 additions & 0 deletions src/migrations/1791000000000-add-course-level-language.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { MigrationInterface, QueryRunner } from 'typeorm';

/**
* Issue #995 — `SearchFilters` has declared `level` and `language` since
* search filtering was introduced, but neither was ever backed by a column
* on `course`, so `SearchService.search()` silently ignored both filters.
* Adds the columns (mirroring the nullable, indexed `category` column) so
* the query builder can filter on them.
*/
export class AddCourseLevelLanguage1791000000000 implements MigrationInterface {
public async up(queryRunner: QueryRunner): Promise<void> {
await queryRunner.query('ALTER TABLE "course" ADD COLUMN IF NOT EXISTS "level" varchar');
await queryRunner.query('ALTER TABLE "course" ADD COLUMN IF NOT EXISTS "language" varchar');

await queryRunner.query('CREATE INDEX IF NOT EXISTS "IDX_course_level" ON "course" ("level")');
await queryRunner.query(
'CREATE INDEX IF NOT EXISTS "IDX_course_language" ON "course" ("language")',
);
}

public async down(queryRunner: QueryRunner): Promise<void> {
await queryRunner.query('DROP INDEX IF EXISTS "IDX_course_level"');
await queryRunner.query('DROP INDEX IF EXISTS "IDX_course_language"');
await queryRunner.query('ALTER TABLE "course" DROP COLUMN IF EXISTS "level"');
await queryRunner.query('ALTER TABLE "course" DROP COLUMN IF EXISTS "language"');
}
}
Loading
Loading