diff --git a/src/courses/entities/course.entity.ts b/src/courses/entities/course.entity.ts index 74e784a5..809f7684 100644 --- a/src/courses/entities/course.entity.ts +++ b/src/courses/entities/course.entity.ts @@ -73,6 +73,10 @@ export class Course { @Index() instructorId: string; + @Column({ name: 'tenant_id', nullable: true }) + @Index() + tenantId?: string; + @OneToMany(() => CourseModule, (module) => module.course) modules: CourseModule[]; diff --git a/src/search/search.module.ts b/src/search/search.module.ts index 7befacdb..46565b87 100644 --- a/src/search/search.module.ts +++ b/src/search/search.module.ts @@ -4,6 +4,9 @@ import { ElasticsearchModule } from '@nestjs/elasticsearch'; import { SearchController } from './search.controller'; import { SearchService } from './search.service'; import { createElasticsearchConfig } from '../config/elasticsearch.config'; +import { TenancyModule } from '../tenancy/tenancy.module'; +import { TypeOrmModule } from '@nestjs/typeorm'; +import { Course } from '../courses/entities/course.entity'; /** * Search module supports Elasticsearch-backed course searching, @@ -12,11 +15,13 @@ import { createElasticsearchConfig } from '../config/elasticsearch.config'; @Module({ imports: [ ConfigModule, + TypeOrmModule.forFeature([Course]), ElasticsearchModule.registerAsync({ imports: [ConfigModule], inject: [ConfigService], useFactory: createElasticsearchConfig, }), + TenancyModule, ], controllers: [SearchController], providers: [SearchService], diff --git a/src/search/search.service.spec.ts b/src/search/search.service.spec.ts index 2f49a2f1..337e84f2 100644 --- a/src/search/search.service.spec.ts +++ b/src/search/search.service.spec.ts @@ -3,8 +3,165 @@ import { Test, TestingModule } from '@nestjs/testing'; import { ElasticsearchService as NestElasticsearchService } from '@nestjs/elasticsearch'; import { getRepositoryToken } from '@nestjs/typeorm'; import { SearchService } from './search.service'; -import { SEARCH_CONSTANTS } from './search.constants'; import { Course } from '../courses/entities/course.entity'; +import { IsolationService } from '../tenancy/isolation/isolation.service'; + +const TENANT_A = 'tenant-a-uuid'; +const TENANT_B = 'tenant-b-uuid'; + +/** Seed courses across two tenants */ +const allCourses: Partial[] = [ + { id: '1', title: 'JavaScript Basics', description: 'JS intro', tenantId: TENANT_A, price: 0 } as Course, + { id: '2', title: 'JavaScript Advanced', description: 'JS advanced', tenantId: TENANT_A, price: 50 } as Course, + { id: '3', title: 'JavaScript for B', description: 'JS for tenant B', tenantId: TENANT_B, price: 10 } as Course, + { id: '4', title: 'Python Basics', description: 'Python intro', tenantId: TENANT_B, price: 0 } as Course, +]; + +function makeQueryBuilder(tenantId: string | null) { + let filtered = [...allCourses]; + + const qb: any = { + _tenantId: tenantId, + _query: null as string | null, + _category: null as string[] | null, + _minPrice: undefined as number | undefined, + _maxPrice: undefined as number | undefined, + + where(cond: string, params?: any) { + if (cond.includes('tenantId')) { + filtered = filtered.filter((c) => c.tenantId === params.tenantId); + } else if (cond.includes('title ILIKE') || cond.includes('description ILIKE')) { + const q = (params.query as string).replace(/%/g, ''); + filtered = filtered.filter( + (c) => + c.title?.toLowerCase().includes(q.toLowerCase()) || + c.description?.toLowerCase().includes(q.toLowerCase()), + ); + } + return qb; + }, + andWhere(cond: string, params?: any) { + return qb.where(cond, params); + }, + orderBy() { + return qb; + }, + skip() { + return qb; + }, + take() { + return qb; + }, + select() { + return qb; + }, + getManyAndCount: jest.fn().mockImplementation(() => Promise.resolve([filtered, filtered.length])), + getMany: jest.fn().mockImplementation(() => Promise.resolve(filtered)), + }; + return qb; +} + +function buildModule(tenantId: string | null) { + const mockIsolationService = { getTenantId: jest.fn().mockReturnValue(tenantId) }; + const mockCourseRepo = { + createQueryBuilder: jest.fn().mockImplementation(() => makeQueryBuilder(tenantId)), + }; + const mockCache = { get: jest.fn().mockResolvedValue(undefined), set: jest.fn() }; + + return Test.createTestingModule({ + providers: [ + SearchService, + { provide: NestElasticsearchService, useValue: {} }, + { provide: CACHE_MANAGER, useValue: mockCache }, + { provide: getRepositoryToken(Course), useValue: mockCourseRepo }, + { provide: IsolationService, useValue: mockIsolationService }, + ], + }).compile(); +} + +describe('SearchService – tenant isolation', () => { + describe('search()', () => { + it('returns only Tenant A courses when Tenant A context is active', async () => { + const module: TestingModule = await buildModule(TENANT_A); + const service = module.get(SearchService); + + const result = await service.search('javascript'); + + expect(result.results.every((c: Course) => c.tenantId === TENANT_A)).toBe(true); + expect(result.results.some((c: Course) => c.tenantId === TENANT_B)).toBe(false); + }); + + it('returns only Tenant B courses when Tenant B context is active', async () => { + const module: TestingModule = await buildModule(TENANT_B); + const service = module.get(SearchService); + + const result = await service.search('javascript'); + + expect(result.results.every((c: Course) => c.tenantId === TENANT_B)).toBe(true); + expect(result.results.some((c: Course) => c.tenantId === TENANT_A)).toBe(false); + }); + + it('applies tenant filter even when no query parameters are provided', async () => { + const module: TestingModule = await buildModule(TENANT_A); + const service = module.get(SearchService); + + const result = await service.search(''); + + expect(result.results.every((c: Course) => c.tenantId === TENANT_A)).toBe(true); + }); + + it('cross-boundary: Tenant A search never returns Tenant B courses', async () => { + const moduleA: TestingModule = await buildModule(TENANT_A); + const serviceA = moduleA.get(SearchService); + + const resultA = await serviceA.search('javascript'); + const tenantBIds = allCourses.filter((c) => c.tenantId === TENANT_B).map((c) => c.id); + const returnedIds = resultA.results.map((c: Course) => c.id); + + tenantBIds.forEach((id) => { + expect(returnedIds).not.toContain(id); + }); + }); + }); + + describe('getAutoComplete()', () => { + it('returns only Tenant A autocomplete suggestions', async () => { + const module: TestingModule = await buildModule(TENANT_A); + const service = module.get(SearchService); + + const results = await service.getAutoComplete('java'); + + // All returned course IDs should belong to Tenant A + const tenantACourseIds = allCourses.filter((c) => c.tenantId === TENANT_A).map((c) => c.id); + results.forEach((r) => { + expect(tenantACourseIds).toContain(r.metadata?.courseId); + }); + }); + + it('autocomplete cross-boundary: Tenant A never sees Tenant B suggestions', async () => { + const module: TestingModule = await buildModule(TENANT_A); + const service = module.get(SearchService); + + const results = await service.getAutoComplete('java'); + const tenantBCourseIds = allCourses.filter((c) => c.tenantId === TENANT_B).map((c) => c.id); + + results.forEach((r) => { + expect(tenantBCourseIds).not.toContain(r.metadata?.courseId); + }); + }); + }); + + describe('buildTenantFilter()', () => { + it('returns a term filter with the given tenantId', async () => { + const module: TestingModule = await buildModule(TENANT_A); + const service = module.get(SearchService); + + expect(service.buildTenantFilter(TENANT_A)).toEqual({ term: { tenantId: TENANT_A } }); + }); + }); +}); + +// ── Existing unit tests preserved ───────────────────────────────────────────── const mockQueryBuilder = { where: jest.fn().mockReturnThis(), @@ -12,7 +169,9 @@ const mockQueryBuilder = { orderBy: jest.fn().mockReturnThis(), skip: jest.fn().mockReturnThis(), take: jest.fn().mockReturnThis(), + select: jest.fn().mockReturnThis(), getManyAndCount: jest.fn().mockResolvedValue([[], 0]), + getMany: jest.fn().mockResolvedValue([]), }; const mockCourseRepo = { @@ -35,6 +194,7 @@ describe('SearchService', () => { { provide: NestElasticsearchService, useValue: {} }, { provide: CACHE_MANAGER, useValue: mockCache }, { provide: getRepositoryToken(Course), useValue: mockCourseRepo }, + { provide: IsolationService, useValue: { getTenantId: jest.fn().mockReturnValue(null) } }, ], }).compile(); diff --git a/src/search/search.service.ts b/src/search/search.service.ts index b5e9e3ff..ab9a5528 100644 --- a/src/search/search.service.ts +++ b/src/search/search.service.ts @@ -1,10 +1,11 @@ -import { Injectable, Logger, Inject, Optional } from '@nestjs/common'; +import { Injectable, Logger, Inject, Optional } from '@nestjs/common'; import { CACHE_MANAGER } from '@nestjs/cache-manager'; import { ElasticsearchService as NestElasticsearchService } from '@nestjs/elasticsearch'; import type { Cache } from 'cache-manager'; import { InjectRepository } from '@nestjs/typeorm'; import { Repository } from 'typeorm'; import { Course } from '../courses/entities/course.entity'; +import { IsolationService } from '../tenancy/isolation/isolation.service'; export interface SearchFilters { category?: string | string[]; @@ -44,9 +45,15 @@ export class SearchService { @InjectRepository(Course) private readonly courseRepository: Repository, private readonly elasticsearch: NestElasticsearchService, + private readonly isolationService: IsolationService, @Optional() @Inject(CACHE_MANAGER) private readonly cacheManager?: Cache, ) {} + /** Returns an Elasticsearch-style term filter for the current tenant. */ + buildTenantFilter(tenantId: string): { term: { tenantId: string } } { + return { term: { tenantId } }; + } + async search( query: string, filters?: SearchFilters, @@ -55,7 +62,8 @@ export class SearchService { limit: number = 20, ): Promise { const safeQuery = query?.trim() ?? ''; - const cacheKey = `search:${safeQuery}:${JSON.stringify(filters)}:${sort}:${page}`; + const tenantId = this.isolationService.getTenantId(); + const cacheKey = `search:${tenantId}:${safeQuery}:${JSON.stringify(filters)}:${sort}:${page}`; if (this.cacheManager) { const cached = await this.cacheManager.get(cacheKey); @@ -64,8 +72,14 @@ export class SearchService { try { const qb = this.courseRepository.createQueryBuilder('course'); + + // Tenant isolation: always filter by tenantId when context is available + if (tenantId) { + qb.where('course.tenantId = :tenantId', { tenantId }); + } + if (safeQuery) { - qb.where('course.title ILIKE :query OR course.description ILIKE :query', { + qb.andWhere('course.title ILIKE :query OR course.description ILIKE :query', { query: `%${safeQuery}%`, }); } @@ -101,13 +115,22 @@ export class SearchService { async getAutoComplete(query: string): Promise { if (!query || query.length < 2) return []; - const cached = this.autocompleteCache.get(query); + const tenantId = this.isolationService.getTenantId(); + const cacheKey = `${tenantId ?? 'global'}:${query}`; + const cached = this.autocompleteCache.get(cacheKey); if (cached && Date.now() - cached.timestamp < this.CACHE_TTL_MS) return cached.results; try { - const courses = await this.courseRepository + const qb = this.courseRepository .createQueryBuilder('course') - .where('course.title ILIKE :query', { query: `${query}%` }) + .where('course.title ILIKE :query', { query: `${query}%` }); + + // Tenant isolation: filter autocomplete by current tenant + if (tenantId) { + qb.andWhere('course.tenantId = :tenantId', { tenantId }); + } + + const courses = await qb .orderBy('course.enrollmentCount', 'DESC') .take(10) .select(['course.id', 'course.title']) @@ -119,7 +142,7 @@ export class SearchService { metadata: { courseId: course.id }, })); - this.autocompleteCache.set(query, { results, timestamp: Date.now() }); + this.autocompleteCache.set(cacheKey, { results, timestamp: Date.now() }); return results; } catch (err) { this.logger.error(`Autocomplete failed: ${(err as Error).message}`);