diff --git a/apps/api/package.json b/apps/api/package.json index 2c81e971..233cf442 100644 --- a/apps/api/package.json +++ b/apps/api/package.json @@ -95,14 +95,15 @@ }, "collectCoverageFrom": [ "**/*.(t|j)s", - "!main.ts", + "!src/main.ts", "!**/*.module.ts", "!**/*.abstract.ts", - "!test/**", - "!db/seed.ts", - "!db/truncate.ts", - "!db/verify.ts", - "!scripts/**" + "!src/test/**", + "!src/db/seed.ts", + "!src/db/truncate.ts", + "!src/db/verify.ts", + "!src/scripts/**", + "!test/**" ], "coverageDirectory": "./coverage", "testEnvironment": "node", diff --git a/apps/api/src/modules/identity/auth/auth.controller.spec.ts b/apps/api/src/modules/identity/auth/auth.controller.spec.ts index 8836250c..c8a00859 100644 --- a/apps/api/src/modules/identity/auth/auth.controller.spec.ts +++ b/apps/api/src/modules/identity/auth/auth.controller.spec.ts @@ -1,8 +1,7 @@ import { Test, TestingModule } from '@nestjs/testing'; import { AuthController } from './auth.controller'; import { AuthService } from './auth.service'; -import { USER_PROVIDER } from '@nexiom/identity'; -import { TenantsService } from '../tenants/tenants.service'; +import { USER_PROVIDER, TENANT_PROVIDER } from '@nexiom/identity'; import { InvitationsService } from '../invitations/invitations.service'; import { Request, Response } from 'express'; @@ -36,7 +35,7 @@ describe('AuthController', () => { forceVerifyEmail: jest.fn(), }; - const mockTenantsService = { + const mockTenantProvider = { provisionTenantForUser: jest.fn(), }; @@ -58,8 +57,8 @@ describe('AuthController', () => { useValue: mockUserProvider, }, { - provide: TenantsService, - useValue: mockTenantsService, + provide: TENANT_PROVIDER, + useValue: mockTenantProvider, }, { provide: InvitationsService, @@ -91,7 +90,7 @@ describe('AuthController', () => { }; mockAuthService.getSessionFromHeaders.mockResolvedValue(mockSessionData); - mockTenantsService.provisionTenantForUser.mockResolvedValue({ + mockTenantProvider.provisionTenantForUser.mockResolvedValue({ id: 'org-123', name: 'New Org', }); @@ -99,7 +98,7 @@ describe('AuthController', () => { const result = await controller.provisionTenant(mockRequest); expect(result).toBeDefined(); - expect(mockTenantsService.provisionTenantForUser).toHaveBeenCalledWith( + expect(mockTenantProvider.provisionTenantForUser).toHaveBeenCalledWith( 'user-123', ); }); diff --git a/apps/api/src/modules/identity/auth/auth.controller.ts b/apps/api/src/modules/identity/auth/auth.controller.ts index b97cafa8..d013fe19 100644 --- a/apps/api/src/modules/identity/auth/auth.controller.ts +++ b/apps/api/src/modules/identity/auth/auth.controller.ts @@ -8,10 +8,17 @@ import { UnauthorizedException, Inject, Logger, + BadRequestException, } from '@nestjs/common'; import { AuthService } from './auth.service'; -import { USER_PROVIDER, IUserProvider, Session, User } from '@nexiom/identity'; -import { TenantsService } from '../tenants/tenants.service'; +import { + USER_PROVIDER, + IUserProvider, + Session, + User, + TENANT_PROVIDER, + ITenantProvider, +} from '@nexiom/identity'; import { z } from 'zod'; import { createZodDto } from 'nestjs-zod'; import { Signup, CompleteInvite } from '../users/users.validation'; @@ -19,7 +26,6 @@ import { Response, Request } from 'express'; import { toNodeHandler } from 'better-auth/node'; import { InvitationsService } from '../invitations/invitations.service'; import { toWebHeaders } from '../../../shared/utils/headers.util'; -import { BadRequestException } from '@nestjs/common'; /** * Handles authentication-related operations such as user login. @@ -39,7 +45,7 @@ export class AuthController { constructor( private readonly authService: AuthService, @Inject(USER_PROVIDER) private readonly userProvider: IUserProvider, - private readonly tenantsService: TenantsService, + @Inject(TENANT_PROVIDER) private readonly tenantProvider: ITenantProvider, private readonly invitationsService: InvitationsService, ) {} @@ -83,7 +89,7 @@ export class AuthController { throw new UnauthorizedException('No Session Found'); } - return this.tenantsService.provisionTenantForUser(sessionData.user.id); + return this.tenantProvider.provisionTenantForUser(sessionData.user.id); } /** diff --git a/apps/api/src/modules/identity/auth/auth.service.spec.ts b/apps/api/src/modules/identity/auth/auth.service.spec.ts index b589be8e..e474aea3 100644 --- a/apps/api/src/modules/identity/auth/auth.service.spec.ts +++ b/apps/api/src/modules/identity/auth/auth.service.spec.ts @@ -1,7 +1,6 @@ import { Test, TestingModule } from '@nestjs/testing'; import { AuthService } from './auth.service'; -import { AUTH_PROVIDER } from '@nexiom/identity'; -import { TenantsService } from '../tenants/tenants.service'; +import { AUTH_PROVIDER, TENANT_PROVIDER } from '@nexiom/identity'; describe('AuthService', () => { let service: AuthService; @@ -15,7 +14,7 @@ describe('AuthService', () => { getHandler: jest.fn(), }; - const mockTenantsService = { + const mockTenantProvider = { findAllForUser: jest.fn(), }; @@ -28,8 +27,8 @@ describe('AuthService', () => { useValue: mockAuthProvider, }, { - provide: TenantsService, - useValue: mockTenantsService, + provide: TENANT_PROVIDER, + useValue: mockTenantProvider, }, ], }).compile(); @@ -89,12 +88,12 @@ describe('AuthService', () => { ]; mockAuthProvider.validateSession.mockResolvedValue(mockSession); - mockTenantsService.findAllForUser.mockResolvedValue(mockTenants); + mockTenantProvider.findAllForUser.mockResolvedValue(mockTenants); const result = await service.getEnrichedSession(token); expect(mockAuthProvider.validateSession).toHaveBeenCalledWith(token); - expect(mockTenantsService.findAllForUser).toHaveBeenCalledWith('u1'); + expect(mockTenantProvider.findAllForUser).toHaveBeenCalledWith('u1'); // Should pick 'org-new' because it is newer expect(result?.user.organizationId).toBe('org-new'); @@ -105,7 +104,7 @@ describe('AuthService', () => { it('should return enriched session without organizationId if no tenant', async () => { const token = 'valid-token'; const mockSession = { session: { id: 's1' }, user: { id: 'u1' } }; - mockTenantsService.findAllForUser.mockResolvedValue([]); + mockTenantProvider.findAllForUser.mockResolvedValue([]); mockAuthProvider.validateSession.mockResolvedValue(mockSession); @@ -153,7 +152,7 @@ describe('AuthService', () => { providers: [ AuthService, { provide: AUTH_PROVIDER, useValue: providerWithoutSetPassword }, - { provide: TenantsService, useValue: mockTenantsService }, + { provide: TENANT_PROVIDER, useValue: mockTenantProvider }, ], }).compile(); const localService = module.get(AuthService); diff --git a/apps/api/src/modules/identity/auth/auth.service.ts b/apps/api/src/modules/identity/auth/auth.service.ts index 76a04b3d..a980ce3c 100644 --- a/apps/api/src/modules/identity/auth/auth.service.ts +++ b/apps/api/src/modules/identity/auth/auth.service.ts @@ -7,14 +7,15 @@ import { Session, User, CreateUserInput, + TENANT_PROVIDER, + ITenantProvider, } from '@nexiom/identity'; -import { TenantsService } from '../tenants/tenants.service'; @Injectable() export class AuthService { constructor( @Inject(AUTH_PROVIDER) private readonly authProvider: IAuthProvider, - private readonly tenantsService: TenantsService, + @Inject(TENANT_PROVIDER) private readonly tenantProvider: ITenantProvider, ) {} async login(credentials: LoginCredentials): Promise { @@ -40,7 +41,7 @@ export class AuthService { const { session, user } = validSession; // Enrichment: Check if user has a tenant - const tenants = await this.tenantsService.findAllForUser(user.id); + const tenants = await this.tenantProvider.findAllForUser(user.id); // Deterministic selection: Sort by creation date (newest first) // using slice() to avoid mutating the original array const sortedTenants = tenants diff --git a/apps/api/src/modules/identity/system-admin/system-admin.controller.spec.ts b/apps/api/src/modules/identity/system-admin/system-admin.controller.spec.ts index d476bb30..963a4703 100644 --- a/apps/api/src/modules/identity/system-admin/system-admin.controller.spec.ts +++ b/apps/api/src/modules/identity/system-admin/system-admin.controller.spec.ts @@ -1,84 +1,60 @@ +import { Test, TestingModule } from '@nestjs/testing'; import { SystemAdminController } from './system-admin.controller'; - -import { NodePgDatabase } from 'drizzle-orm/node-postgres'; -import * as schema from '../../../db/schema'; import { BadRequestException, NotFoundException } from '@nestjs/common'; - -interface MockDb { - query: { - user: { findMany: jest.Mock; findFirst: jest.Mock }; - organization: { findFirst: jest.Mock; findMany: jest.Mock }; - member: { findFirst: jest.Mock }; - invitation: { findFirst: jest.Mock }; - }; - select: jest.Mock; - from: jest.Mock; - insert: jest.Mock; - update: jest.Mock; - delete: jest.Mock; - transaction: jest.Mock; - // Chain helpers - limit: jest.Mock; - offset: jest.Mock; - orderBy: jest.Mock; - groupBy: jest.Mock; - leftJoin: jest.Mock; - set: jest.Mock; - where: jest.Mock; - values: jest.Mock; - returning: jest.Mock; -} - -import { IAuthProvider } from '@nexiom/identity'; +import { + AUTH_PROVIDER, + USER_PROVIDER, + TENANT_PROVIDER, +} from '@nexiom/identity'; +import { SystemAdminGuard } from '../auth/system-admin.guard'; +import { PlatformGuard } from '../auth/platform.guard'; describe('SystemAdminController', () => { let controller: SystemAdminController; - let mockDb: MockDb; - let mockIdentityProvider: { - getSessionFromHeaders: jest.Mock; - createInvitation: jest.Mock; + const mockHeaders: Record = {}; + + const mockAuthProvider = { + getSessionFromHeaders: jest.fn(), + createInvitation: jest.fn(), + }; + + const mockUserProvider = { + findById: jest.fn(), + findByEmail: jest.fn(), + findAll: jest.fn(), + create: jest.fn(), + update: jest.fn(), + delete: jest.fn(), + count: jest.fn(), + }; + + const mockTenantProvider = { + findById: jest.fn(), + findBySlug: jest.fn(), + findAll: jest.fn(), + createTenant: jest.fn(), + update: jest.fn(), + delete: jest.fn(), }; - beforeEach(() => { + beforeEach(async () => { jest.clearAllMocks(); - mockIdentityProvider = { - getSessionFromHeaders: jest.fn(), - createInvitation: jest.fn(), - }; - - // Reset mocks with full structure - /* eslint-disable @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-return */ - mockDb = { - query: { - user: { findMany: jest.fn(), findFirst: jest.fn() }, - organization: { findFirst: jest.fn(), findMany: jest.fn() }, - member: { findFirst: jest.fn() }, - invitation: { findFirst: jest.fn() }, - }, - select: jest.fn().mockReturnThis(), - from: jest.fn().mockImplementation(() => Promise.resolve([{ count: 5 }])), - insert: jest.fn(), - update: jest.fn(), - delete: jest.fn(), - - transaction: jest.fn((cb) => cb(mockDb)), // Mock transaction execution - // Chain method definitions - limit: jest.fn().mockReturnThis(), - offset: jest.fn().mockReturnThis(), - orderBy: jest.fn().mockReturnThis(), - groupBy: jest.fn().mockReturnThis(), - leftJoin: jest.fn().mockReturnThis(), - set: jest.fn().mockReturnThis(), - where: jest.fn().mockReturnThis(), - values: jest.fn().mockReturnThis(), - returning: jest.fn(), - }; - - controller = new SystemAdminController( - mockDb as unknown as NodePgDatabase, - mockIdentityProvider as unknown as IAuthProvider, - ); + const module: TestingModule = await Test.createTestingModule({ + controllers: [SystemAdminController], + providers: [ + { provide: AUTH_PROVIDER, useValue: mockAuthProvider }, + { provide: USER_PROVIDER, useValue: mockUserProvider }, + { provide: TENANT_PROVIDER, useValue: mockTenantProvider }, + ], + }) + .overrideGuard(SystemAdminGuard) + .useValue({ canActivate: jest.fn(() => true) }) + .overrideGuard(PlatformGuard) + .useValue({ canActivate: jest.fn(() => true) }) + .compile(); + + controller = module.get(SystemAdminController); }); it('should be defined', () => { @@ -87,149 +63,94 @@ describe('SystemAdminController', () => { describe('listUsers', () => { it('should return paginated users and total count', async () => { - const mockUsers = [{ id: '1', name: 'User 1' }]; - mockDb.query.user.findMany.mockResolvedValue(mockUsers); + const mockResult = { data: [{ id: '1', name: 'User 1' }], total: 1 }; + mockUserProvider.findAll.mockResolvedValue(mockResult); const result = await controller.listUsers('1', '10'); - expect(result).toEqual({ - data: mockUsers, - total: 5, + expect(result).toEqual(mockResult); + expect(mockUserProvider.findAll).toHaveBeenCalledWith({ + page: 1, + limit: 10, }); - expect(mockDb.query.user.findMany).toHaveBeenCalledWith( - expect.objectContaining({ limit: 10, offset: 0 }), - ); - }); - - it('should handle invalid pagination params', async () => { - mockDb.query.user.findMany.mockResolvedValue([]); - - await controller.listUsers('bad', 'bad'); - - expect(mockDb.query.user.findMany).toHaveBeenCalledWith( - expect.objectContaining({ limit: 10, offset: 0 }), - ); }); it('should use default pagination parameters', async () => { - mockDb.query.user.findMany.mockResolvedValue([]); + mockUserProvider.findAll.mockResolvedValue({ data: [], total: 0 }); - // Testing default params - await controller.listUsers(); + await controller.listUsers(); // Defaults - expect(mockDb.query.user.findMany).toHaveBeenCalledWith( - expect.objectContaining({ limit: 10, offset: 0 }), - ); + expect(mockUserProvider.findAll).toHaveBeenCalledWith({ + page: 1, + limit: 10, + }); }); }); - describe('listTenants', () => { - // Define strict interface for our chainable builder - interface MockQueryBuilder { - leftJoin: jest.Mock; - groupBy: jest.Mock; - limit: jest.Mock; - offset: jest.Mock; - orderBy: jest.Mock; - execute: () => Promise; - } - - const createMockBuilder = (result: unknown): MockQueryBuilder => ({ - leftJoin: jest.fn().mockReturnThis(), - groupBy: jest.fn().mockReturnThis(), - limit: jest.fn().mockReturnThis(), - offset: jest.fn().mockReturnThis(), - orderBy: jest.fn().mockReturnThis(), - execute: jest.fn().mockResolvedValue(result), - }); - - it('should return paginated tenants with aggregated user count', async () => { - // Mock db.select chain for tenants - const mockResult = [{ id: 't1', name: 'Tenant 1', userCount: 2 }]; - - // Call 1: Data - const mockQueryBuilder = createMockBuilder(mockResult); - mockDb.from.mockReturnValueOnce(mockQueryBuilder); + describe('createSystemInvitation', () => { + it('should throw BadRequestException if unauthorized', async () => { + mockAuthProvider.getSessionFromHeaders.mockResolvedValue(null); - // Call 2: Total Count - mockDb.from.mockReturnValueOnce(Promise.resolve([{ count: 5 }])); - - const result = await controller.listTenants('1', '10'); - - expect(result).toEqual({ - data: [{ id: 't1', name: 'Tenant 1', userCount: 2 }], - total: 5, - }); - - // Verify specific builder usage - expect(mockQueryBuilder.limit).toHaveBeenCalledWith(10); - expect(mockQueryBuilder.offset).toHaveBeenCalledWith(0); - expect(mockQueryBuilder.execute).toHaveBeenCalled(); + await expect( + controller.createSystemInvitation( + { email: 'test@example.com', role: 'platform_admin' }, + mockHeaders, + ), + ).rejects.toThrow(BadRequestException); + expect(mockAuthProvider.createInvitation).not.toHaveBeenCalled(); }); - it('should clamp pagination parameters', async () => { - const mockBuilder = createMockBuilder([]); - - mockDb.from.mockReturnValueOnce(mockBuilder); - mockDb.from.mockReturnValueOnce(Promise.resolve([{ count: 0 }])); + it('should create system invitation', async () => { + mockAuthProvider.getSessionFromHeaders.mockResolvedValue({ + user: { id: 'admin1' }, + }); + const mockInvitation = { id: 'inv1', email: 'test@example.com' }; + mockAuthProvider.createInvitation.mockResolvedValue(mockInvitation); - // Request 1000 items (should clamp to 100) - await controller.listTenants('1', '1000'); + const result = await controller.createSystemInvitation( + { email: 'test@example.com', role: 'platform_admin' }, + mockHeaders, + ); - expect(mockBuilder.limit).toHaveBeenCalledWith(100); - expect(mockBuilder.offset).toHaveBeenCalledWith(0); - expect(mockBuilder.execute).toHaveBeenCalled(); + expect(result).toEqual(mockInvitation); + expect(mockAuthProvider.getSessionFromHeaders).toHaveBeenCalled(); + expect(mockAuthProvider.createInvitation).toHaveBeenCalledWith({ + email: 'test@example.com', + role: 'platform_admin', + organizationId: null, + inviterId: 'admin1', + }); }); + }); - it('should use default pagination parameters', async () => { - const mockBuilder = createMockBuilder([]); - mockDb.from.mockReturnValueOnce(mockBuilder); - mockDb.from.mockReturnValueOnce(Promise.resolve([{ count: 0 }])); + describe('listTenants', () => { + it('should return paginated tenants', async () => { + const mockResult = { data: [{ id: 't1', name: 'Tenant 1' }], total: 1 }; + mockTenantProvider.findAll.mockResolvedValue(mockResult); - // Testing default params - await controller.listTenants(); + const result = await controller.listTenants('1', '10'); - expect(mockBuilder.limit).toHaveBeenCalledWith(10); - expect(mockBuilder.offset).toHaveBeenCalledWith(0); - expect(mockBuilder.execute).toHaveBeenCalled(); + expect(result).toEqual(mockResult); + expect(mockTenantProvider.findAll).toHaveBeenCalledWith({ + page: 1, + limit: 10, + }); }); }); describe('getTenant', () => { - const createMockBuilder = (result: unknown) => ({ - leftJoin: jest.fn().mockReturnThis(), - groupBy: jest.fn().mockReturnThis(), - limit: jest.fn().mockReturnThis(), - offset: jest.fn().mockReturnThis(), - orderBy: jest.fn().mockReturnThis(), - where: jest.fn().mockReturnThis(), // Added where - execute: jest.fn().mockResolvedValue(result), - }); - - it('should return a single tenant', async () => { - const mockResult = [{ id: 't1', name: 'Tenant 1' }]; - const mockQueryBuilder = createMockBuilder(mockResult); - - // Mock select().from() chain - const mockSelect = { - from: jest.fn().mockReturnValue(mockQueryBuilder), - }; - mockDb.select.mockReturnValueOnce(mockSelect); + it('should return a tenant by ID', async () => { + const mockTenant = { id: 't1', name: 'Tenant 1' }; + mockTenantProvider.findById.mockResolvedValue(mockTenant); const result = await controller.getTenant('t1'); - expect(result).toEqual(mockResult[0]); - expect(mockSelect.from).toHaveBeenCalledWith(schema.organization); - expect(mockQueryBuilder.where).toHaveBeenCalled(); - expect(mockQueryBuilder.execute).toHaveBeenCalled(); + expect(result).toEqual(mockTenant); + expect(mockTenantProvider.findById).toHaveBeenCalledWith('t1'); }); it('should throw NotFoundException if tenant not found', async () => { - const mockQueryBuilder = createMockBuilder([]); - const mockSelect = { - from: jest.fn().mockReturnValue(mockQueryBuilder), - }; - mockDb.select.mockReturnValueOnce(mockSelect); + mockTenantProvider.findById.mockResolvedValue(null); await expect(controller.getTenant('missing')).rejects.toThrow( NotFoundException, @@ -239,7 +160,7 @@ describe('SystemAdminController', () => { describe('createTenant', () => { it('should throw BadRequestException if slug exists', async () => { - mockDb.query.organization.findFirst.mockResolvedValue({ id: 'existing' }); + mockTenantProvider.findBySlug.mockResolvedValue({ id: 'existing' }); await expect( controller.createTenant({ @@ -250,13 +171,10 @@ describe('SystemAdminController', () => { ).rejects.toThrow(BadRequestException); }); - it('should create tenant if slug is unique', async () => { - mockDb.query.organization.findFirst.mockResolvedValue(null); - mockDb.insert = jest.fn().mockReturnValue({ - values: jest.fn().mockReturnValue({ - returning: jest.fn().mockResolvedValue([{ id: 'new', slug: 'test' }]), - }), - }); + it('should create tenant via provider if slug is unique', async () => { + mockTenantProvider.findBySlug.mockResolvedValue(null); + const mockCreated = { id: 'new', slug: 'test' }; + mockTenantProvider.createTenant.mockResolvedValue(mockCreated); const result = await controller.createTenant({ name: 'Test', @@ -264,319 +182,160 @@ describe('SystemAdminController', () => { logo: '', }); - expect(result).toEqual({ id: 'new', slug: 'test' }); + expect(result).toEqual(mockCreated); + expect(mockTenantProvider.createTenant).toHaveBeenCalledWith({ + name: 'Test', + slug: 'test', + logo: '', + }); }); }); describe('createUser', () => { it('should throw BadRequestException if email exists', async () => { - mockDb.query.user.findFirst.mockResolvedValue({ id: 'existing' }); + mockUserProvider.findByEmail.mockResolvedValue({ id: 'existing' }); await expect( controller.createUser({ name: 'Test', - email: 'test@example.com', + email: 'taken@example.com', systemRole: 'platform_user', }), ).rejects.toThrow(BadRequestException); }); - it('should create user if email is unique', async () => { - mockDb.query.user.findFirst.mockResolvedValue(null); - mockDb.insert = jest.fn().mockReturnValue({ - values: jest.fn().mockReturnValue({ - returning: jest.fn().mockResolvedValue([ - { - id: 'new', - email: 'test@example.com', - name: 'Test', - systemRole: 'platform_user', - }, - ]), - }), - }); + it('should create user via provider', async () => { + mockUserProvider.findByEmail.mockResolvedValue(null); + const mockUser = { + id: 'u1', + email: 'new@example.com', + systemRole: 'platform_user', + }; + mockUserProvider.create.mockResolvedValue(mockUser); const result = await controller.createUser({ name: 'Test', - email: 'test@example.com', + email: 'new@example.com', systemRole: 'platform_user', }); - expect(result).toEqual({ - id: 'new', - email: 'test@example.com', + expect(result).toEqual(mockUser); + expect(mockUserProvider.create).toHaveBeenCalledWith({ name: 'Test', + email: 'new@example.com', systemRole: 'platform_user', }); - expect(mockDb.insert).toHaveBeenCalledWith(schema.user); + expect(mockUserProvider.update).not.toHaveBeenCalled(); }); }); describe('updateTenant', () => { it('should throw NotFoundException if tenant not found', async () => { - mockDb.query.organization.findFirst.mockResolvedValue(null); + mockTenantProvider.findById.mockResolvedValue(null); await expect( controller.updateTenant('missing', { name: 'New' }), ).rejects.toThrow(NotFoundException); }); - it('should throw BadRequestException if new slug overlaps', async () => { - mockDb.query.organization.findFirst - .mockResolvedValueOnce({ id: 't1', slug: 'old' }) // Existing target - .mockResolvedValueOnce({ id: 't2', slug: 'taken' }); // Collision check + it('should throw BadRequestException if slug overlaps', async () => { + mockTenantProvider.findById.mockResolvedValue({ + id: 't1', + slug: 'old-slug', + }); + mockTenantProvider.findBySlug.mockResolvedValue({ + id: 't2', // diff ID + slug: 'taken', + }); await expect( controller.updateTenant('t1', { slug: 'taken' }), ).rejects.toThrow(BadRequestException); }); - it('should throw BadRequestException if payload is empty', async () => { - mockDb.query.organization.findFirst.mockResolvedValue({ id: 't1' }); - // Valid tenant, but empty update - await expect(controller.updateTenant('t1', {})).rejects.toThrow( - BadRequestException, - ); - }); + it('should update tenant via provider', async () => { + mockTenantProvider.findById.mockResolvedValue({ id: 't1' }); + // Slug check: returns null or same ID + mockTenantProvider.findBySlug.mockResolvedValue(null); + const mockUpdated = { id: 't1', name: 'New' }; + mockTenantProvider.update.mockResolvedValue(mockUpdated); - it('should update tenant successfully', async () => { - // 1. Find existing - mockDb.query.organization.findFirst - .mockResolvedValueOnce({ id: 't1', slug: 'old' }) // Existing - .mockResolvedValueOnce(null); // Collision check (slug change) - - // 2. Update - mockDb.update = jest.fn().mockReturnValue({ - set: jest.fn().mockReturnValue({ - where: jest.fn().mockReturnValue({ - returning: jest - .fn() - .mockResolvedValue([{ id: 't1', slug: 'new-slug', name: 'New' }]), - }), - }), - }); + const result = await controller.updateTenant('t1', { name: 'New' }); - const result = await controller.updateTenant('t1', { + expect(result).toEqual(mockUpdated); + expect(mockTenantProvider.update).toHaveBeenCalledWith('t1', { name: 'New', - slug: 'new-slug', }); - - expect(result).toEqual({ id: 't1', slug: 'new-slug', name: 'New' }); - // Verify update called with correct args - expect(mockDb.update).toHaveBeenCalled(); - }); - - it('should update specific fields (status only)', async () => { - mockDb.query.organization.findFirst.mockResolvedValue({ id: 't1' }); - - const mockReturning = jest - .fn() - .mockResolvedValue([{ id: 't1', status: 'suspended' }]); - const mockSet = jest.fn().mockReturnValue({ - where: jest.fn().mockReturnValue({ returning: mockReturning }), - }); - mockDb.update = jest.fn().mockReturnValue({ set: mockSet }); - - await controller.updateTenant('t1', { status: 'suspended' }); - - // Verify payload passed to set() - expect(mockSet).toHaveBeenCalledWith( - expect.objectContaining({ status: 'suspended' }), - ); - }); - - it('should update all fields including metadata', async () => { - mockDb.query.organization.findFirst - .mockResolvedValueOnce({ id: 't1', slug: 'old' }) - .mockResolvedValueOnce(null); - - const mockSet = jest.fn().mockReturnValue({ - where: jest.fn().mockReturnValue({ - returning: jest.fn().mockResolvedValue([{ id: 't1' }]), - }), - }); - mockDb.update = jest.fn().mockReturnValue({ set: mockSet }); - - const payload = { - name: 'Full Update', - slug: 'full-update', - logo: 'logo.png', - status: 'active' as const, - metadata: { key: 'value' }, - }; - - await controller.updateTenant('t1', payload); - - expect(mockSet).toHaveBeenCalledWith( - expect.objectContaining({ - name: 'Full Update', - slug: 'full-update', - logo: 'logo.png', - status: 'active', - metadata: JSON.stringify({ key: 'value' }), - }), - ); }); }); describe('deleteTenant', () => { it('should throw NotFoundException if tenant not found', async () => { - mockDb.query.organization.findFirst.mockResolvedValue(null); + mockTenantProvider.findById.mockResolvedValue(null); await expect(controller.deleteTenant('missing')).rejects.toThrow( NotFoundException, ); }); - it('should delete tenant successfully inside transaction', async () => { - // 1. Find existing - mockDb.query.organization.findFirst.mockResolvedValue({ id: 't1' }); - - // 2. Setup Deletes - // We expect 3 deletes: member, invitation, organization - // We'll mock the delete chain to return a 'where' mock - const mockWhere = jest.fn().mockResolvedValue({}); - mockDb.delete = jest.fn().mockReturnValue({ - where: mockWhere, - }); + it('should delete tenant via provider', async () => { + mockTenantProvider.findById.mockResolvedValue({ id: 't1' }); await controller.deleteTenant('t1'); - expect(mockDb.transaction).toHaveBeenCalled(); - - // Verify calls inside transaction - // Since it's inside transaction, we check the calls on mockDb (because our mock transaction calls cb(mockDb)) - // Call 1: Member - expect(mockDb.delete).toHaveBeenNthCalledWith(1, schema.member); - // Call 2: Invitation - expect(mockDb.delete).toHaveBeenNthCalledWith(2, schema.invitation); - // Call 3: Organization - expect(mockDb.delete).toHaveBeenNthCalledWith(3, schema.organization); - - // Verify 3 executions of 'where' - expect(mockWhere).toHaveBeenCalledTimes(3); + expect(mockTenantProvider.delete).toHaveBeenCalledWith('t1'); }); }); describe('updateUser', () => { it('should throw NotFoundException if user not found', async () => { - mockDb.query.user.findFirst.mockResolvedValue(null); + mockUserProvider.findById.mockResolvedValue(null); await expect( controller.updateUser('missing', { name: 'New' }), ).rejects.toThrow(NotFoundException); }); - it('should throw BadRequestException if payload is empty', async () => { - mockDb.query.user.findFirst.mockResolvedValue({ id: 'u1' }); - - await expect(controller.updateUser('u1', {})).rejects.toThrow( - BadRequestException, - ); - }); - - it('should update user successfully', async () => { - mockDb.query.user.findFirst.mockResolvedValue({ id: 'u1' }); - - mockDb.update = jest.fn().mockReturnValue({ - set: jest.fn().mockReturnValue({ - where: jest.fn().mockReturnValue({ - returning: jest - .fn() - .mockResolvedValue([ - { id: 'u1', name: 'New Name', systemRole: 'platform_admin' }, - ]), - }), - }), - }); - - const result = await controller.updateUser('u1', { - name: 'New Name', - systemRole: 'platform_admin', - }); - - expect(result).toEqual({ + it('should throw BadRequestException if email overlaps', async () => { + mockUserProvider.findById.mockResolvedValue({ id: 'u1', - name: 'New Name', - systemRole: 'platform_admin', + email: 'old@example.com', }); - expect(mockDb.update).toHaveBeenCalledWith(schema.user); - }); - - it('should update user specific fields (emailVerified)', async () => { - mockDb.query.user.findFirst.mockResolvedValue({ id: 'u1' }); - - const mockSet = jest.fn().mockReturnValue({ - where: jest.fn().mockReturnValue({ - returning: jest.fn().mockResolvedValue([{ id: 'u1' }]), - }), + mockUserProvider.findByEmail.mockResolvedValue({ + id: 'u2', // Diff ID + email: 'taken@example.com', }); - mockDb.update = jest.fn().mockReturnValue({ set: mockSet }); - - await controller.updateUser('u1', { emailVerified: true }); - - expect(mockSet).toHaveBeenCalledWith( - expect.objectContaining({ emailVerified: true }), - ); - }); - - it('should throw BadRequestException if email already taken (pre-check)', async () => { - // 1. Return payload user first - mockDb.query.user.findFirst - .mockResolvedValueOnce({ id: 'u1', email: 'old@example.com' }) - // 2. Return collision user next - .mockResolvedValueOnce({ id: 'u2', email: 'taken@example.com' }); await expect( controller.updateUser('u1', { email: 'taken@example.com' }), ).rejects.toThrow(BadRequestException); }); - it('should throw BadRequestException on race condition (duplicate key)', async () => { - // 1. Return payload user (user exists) - mockDb.query.user.findFirst.mockResolvedValueOnce({ - id: 'u1', - email: 'old@example.com', - }); + it('should update user via provider', async () => { + mockUserProvider.findById.mockResolvedValue({ id: 'u1' }); + mockUserProvider.update.mockResolvedValue({ id: 'u1', name: 'New' }); - // 2. Return null for uniqueness check (simulate pre-check pass) - mockDb.query.user.findFirst.mockResolvedValueOnce(null); - - // 3. Mock db update to throw unique constraint error (race condition hit) - mockDb.update = jest.fn().mockReturnValue({ - set: jest.fn().mockReturnValue({ - where: jest.fn().mockReturnValue({ - returning: jest - .fn() - .mockRejectedValue( - new Error('duplicate key value violates unique constraint'), - ), - }), - }), - }); + const result = await controller.updateUser('u1', { name: 'New' }); - await expect( - controller.updateUser('u1', { email: 'race@example.com' }), - ).rejects.toThrow(BadRequestException); + expect(result).toEqual({ id: 'u1', name: 'New' }); + expect(mockUserProvider.update).toHaveBeenCalledWith('u1', { + name: 'New', + }); }); }); describe('getUser', () => { - it('should return a single user', async () => { - const mockUser = { id: 'u1', name: 'User 1' }; - mockDb.query.user.findFirst.mockResolvedValue(mockUser); + it('should return user by ID', async () => { + const mockUser = { id: 'u1' }; + mockUserProvider.findById.mockResolvedValue(mockUser); const result = await controller.getUser('u1'); expect(result).toEqual(mockUser); - - expect(mockDb.query.user.findFirst).toHaveBeenCalledWith( - expect.objectContaining({ where: expect.anything() }), - ); }); it('should throw NotFoundException if user not found', async () => { - mockDb.query.user.findFirst.mockResolvedValue(null); + mockUserProvider.findById.mockResolvedValue(null); await expect(controller.getUser('missing')).rejects.toThrow( NotFoundException, @@ -586,127 +345,90 @@ describe('SystemAdminController', () => { describe('deleteUser', () => { it('should throw NotFoundException if user not found', async () => { - mockDb.query.user.findFirst.mockResolvedValue(null); + mockUserProvider.findById.mockResolvedValue(null); await expect(controller.deleteUser('missing')).rejects.toThrow( NotFoundException, ); }); - it('should delete user and dependencies transactionally', async () => { - mockDb.query.user.findFirst.mockResolvedValue({ - id: 'u1', - systemRole: 'platform_user', - }); // Normal user - - const mockWhere = jest.fn().mockResolvedValue({}); - - mockDb.delete = jest.fn().mockReturnValue({ - where: mockWhere, - }); - - await controller.deleteUser('u1'); - - expect(mockDb.transaction).toHaveBeenCalled(); - - // Verify deletion order inside transaction - // 1. Memberships - expect(mockDb.delete).toHaveBeenNthCalledWith(1, schema.member); - // 2. Invitations (Inviter) - expect(mockDb.delete).toHaveBeenNthCalledWith(2, schema.invitation); - // 3. Invitations (Recipient) - expect(mockDb.delete).toHaveBeenNthCalledWith(3, schema.invitation); - // 4. Sessions - expect(mockDb.delete).toHaveBeenNthCalledWith(4, schema.session); - // 5. Accounts - expect(mockDb.delete).toHaveBeenNthCalledWith(5, schema.account); - // 6. User - expect(mockDb.delete).toHaveBeenNthCalledWith(6, schema.user); - - expect(mockWhere).toHaveBeenCalledTimes(6); - }); - it('should prevent deleting the last platform admin', async () => { - mockDb.query.user.findFirst.mockResolvedValue({ + mockUserProvider.findById.mockResolvedValue({ id: 'admin1', systemRole: 'platform_admin', }); - - // Mock db.select().from().where() - const mockBuilder = { - where: jest.fn().mockResolvedValue([{ count: 0 }]), - }; - // mockDb.from returns the builder (because db.select() returns 'this', and 'this.from' is called) - mockDb.from.mockReturnValueOnce(mockBuilder); + // count returns 1 (only this user left) + mockUserProvider.count.mockResolvedValue(1); await expect(controller.deleteUser('admin1')).rejects.toThrow( BadRequestException, ); + expect(mockUserProvider.count).toHaveBeenCalledWith({ + systemRole: 'platform_admin', + }); }); it('should allow deleting platform admin if others exist', async () => { - mockDb.query.user.findFirst.mockResolvedValue({ + mockUserProvider.findById.mockResolvedValue({ id: 'admin1', systemRole: 'platform_admin', }); + // count returns 2 + mockUserProvider.count.mockResolvedValue(2); - // Mock db.select().from().where() - const mockBuilder = { - where: jest.fn().mockResolvedValue([{ count: 1 }]), - }; - mockDb.from.mockReturnValueOnce(mockBuilder); + await controller.deleteUser('admin1'); - const mockWhere = jest.fn().mockResolvedValue({}); - mockDb.delete = jest.fn().mockReturnValue({ where: mockWhere }); + expect(mockUserProvider.delete).toHaveBeenCalledWith('admin1'); + }); - await controller.deleteUser('admin1'); + it('should delete normal user', async () => { + mockUserProvider.findById.mockResolvedValue({ + id: 'u1', + systemRole: 'platform_user', + }); + + await controller.deleteUser('u1'); - expect(mockDb.transaction).toHaveBeenCalled(); + expect(mockUserProvider.delete).toHaveBeenCalledWith('u1'); + // Should not check admin count for normal user + expect(mockUserProvider.count).not.toHaveBeenCalled(); }); }); describe('inviteUser', () => { - const mockHeaders = {}; - it('should throw NotFoundException if user not found', async () => { - mockDb.query.user.findFirst.mockResolvedValue(null); + mockUserProvider.findById.mockResolvedValue(null); await expect( - controller.inviteUser('missing', mockHeaders as Record), + controller.inviteUser('missing', mockHeaders), ).rejects.toThrow(NotFoundException); }); - it('should throw BadRequestException if unauthorized (no session)', async () => { - mockDb.query.user.findFirst.mockResolvedValue({ id: 'u1' }); - mockIdentityProvider.getSessionFromHeaders.mockResolvedValue(null); + it('should throw BadRequestException if unauthorized', async () => { + mockUserProvider.findById.mockResolvedValue({ id: 'u1' }); + mockAuthProvider.getSessionFromHeaders.mockResolvedValue(null); - await expect( - controller.inviteUser('u1', mockHeaders as Record), - ).rejects.toThrow(BadRequestException); + await expect(controller.inviteUser('u1', mockHeaders)).rejects.toThrow( + BadRequestException, + ); }); - it('should create system invitation successfully', async () => { - const mockUser = { + it('should create system invite', async () => { + mockUserProvider.findById.mockResolvedValue({ id: 'u1', email: 'test@example.com', systemRole: 'platform_admin', - }; - const mockSession = { user: { id: 'admin1' } }; - - mockDb.query.user.findFirst.mockResolvedValue(mockUser); - mockIdentityProvider.getSessionFromHeaders.mockResolvedValue(mockSession); - mockIdentityProvider.createInvitation.mockResolvedValue({ id: 'inv1' }); + }); + mockAuthProvider.getSessionFromHeaders.mockResolvedValue({ + user: { id: 'admin1' }, + }); - const result = await controller.inviteUser( - 'u1', - mockHeaders as Record, - ); + await controller.inviteUser('u1', mockHeaders); - expect(result).toEqual({ success: true }); - expect(mockIdentityProvider.createInvitation).toHaveBeenCalledWith({ - email: mockUser.email, - role: mockUser.systemRole, - organizationId: null, + expect(mockAuthProvider.createInvitation).toHaveBeenCalledWith({ + email: 'test@example.com', + role: 'platform_admin', + organizationId: null, // System invite inviterId: 'admin1', }); }); diff --git a/apps/api/src/modules/identity/system-admin/system-admin.controller.ts b/apps/api/src/modules/identity/system-admin/system-admin.controller.ts index cf7a53a3..e58432db 100644 --- a/apps/api/src/modules/identity/system-admin/system-admin.controller.ts +++ b/apps/api/src/modules/identity/system-admin/system-admin.controller.ts @@ -13,13 +13,16 @@ import { NotFoundException, Headers as RequestHeaders, } from '@nestjs/common'; -import { AUTH_PROVIDER, IAuthProvider } from '@nexiom/identity'; +import { + AUTH_PROVIDER, + IAuthProvider, + USER_PROVIDER, + IUserProvider, + TENANT_PROVIDER, + ITenantProvider, +} from '@nexiom/identity'; import { SystemAdminGuard } from '../auth/system-admin.guard'; import { PlatformGuard } from '../auth/platform.guard'; -import { DRIZZLE_DB } from '../../../db/db.provider'; -import { NodePgDatabase } from 'drizzle-orm/node-postgres'; -import * as schema from '../../../db/schema'; -import { desc, count, eq, ne, and } from 'drizzle-orm'; import { CreateTenantValidation, UpdateTenantValidation, @@ -27,13 +30,13 @@ import { CreateUserValidation, CreateSystemInvitationValidation, } from './system-admin.validation'; -import { v4 as uuidv4 } from 'uuid'; @Controller('admin') export class SystemAdminController { constructor( - @Inject(DRIZZLE_DB) private readonly db: NodePgDatabase, @Inject(AUTH_PROVIDER) private readonly authProvider: IAuthProvider, + @Inject(USER_PROVIDER) private readonly userProvider: IUserProvider, + @Inject(TENANT_PROVIDER) private readonly tenantProvider: ITenantProvider, ) {} @Post('users/:id/invite') @@ -42,9 +45,7 @@ export class SystemAdminController { @Param('id') id: string, @RequestHeaders() headers: Record, ) { - const user = await this.db.query.user.findFirst({ - where: eq(schema.user.id, id), - }); + const user = await this.userProvider.findById(id); if (!user) { throw new NotFoundException('User not found'); @@ -55,7 +56,7 @@ export class SystemAdminController { // Get current admin ID from session (via Headers -> BetterAuth) const session = await this.authProvider.getSessionFromHeaders(webHeaders); - if (!session || !session.user) { + if (!session?.user) { throw new BadRequestException('Unauthorized'); } @@ -64,7 +65,6 @@ export class SystemAdminController { email: user.email, role: user.systemRole || 'platform_user', organizationId: null, // System Invite - inviterId: session.user.id, }); @@ -82,7 +82,7 @@ export class SystemAdminController { // Get current admin ID from session const session = await this.authProvider.getSessionFromHeaders(webHeaders); - if (!session || !session.user) { + if (!session?.user) { throw new BadRequestException('Unauthorized'); } @@ -90,7 +90,6 @@ export class SystemAdminController { email: body.email, role: body.role, // Zod handles default organizationId: null, // System invitation - inviterId: session.user.id, }); @@ -111,25 +110,17 @@ export class SystemAdminController { @UseGuards(SystemAdminGuard) async createTenant(@Body() input: CreateTenantValidation) { // Check if slug exists - const existing = await this.db.query.organization.findFirst({ - where: eq(schema.organization.slug, input.slug), - }); + const existing = await this.tenantProvider.findBySlug(input.slug); if (existing) { throw new BadRequestException('Slug is already taken by another tenant'); } - const [tenant] = await this.db - .insert(schema.organization) - .values({ - id: uuidv4(), - name: input.name, - slug: input.slug, - logo: input.logo, - createdAt: new Date(), - status: 'active', - }) - .returning(); + const tenant = await this.tenantProvider.createTenant({ + name: input.name, + slug: input.slug, + logo: input.logo, + }); return tenant; } @@ -138,26 +129,16 @@ export class SystemAdminController { @UseGuards(SystemAdminGuard) async createUser(@Body() input: CreateUserValidation) { // Check if email already exists - const existing = await this.db.query.user.findFirst({ - where: eq(schema.user.email, input.email), - }); + const existing = await this.userProvider.findByEmail(input.email); if (existing) { throw new BadRequestException('User with this email already exists'); } - const [user] = await this.db - .insert(schema.user) - .values({ - id: uuidv4(), - name: input.name, - email: input.email, - systemRole: input.systemRole || 'platform_user', - emailVerified: false, - createdAt: new Date(), - updatedAt: new Date(), - }) - .returning(); + // Now uses single-step creation via Adapter logic + const user = await this.userProvider.create({ + ...input, + }); return user; } @@ -168,9 +149,7 @@ export class SystemAdminController { @Param('id') id: string, @Body() input: UpdateTenantValidation, ) { - const tenant = await this.db.query.organization.findFirst({ - where: eq(schema.organization.id, id), - }); + const tenant = await this.tenantProvider.findById(id); if (!tenant) { throw new NotFoundException('Tenant not found'); @@ -178,64 +157,33 @@ export class SystemAdminController { // Check slug uniqueness if changing if (input.slug && input.slug !== tenant.slug) { - const existing = await this.db.query.organization.findFirst({ - where: and( - eq(schema.organization.slug, input.slug), - ne(schema.organization.id, id), - ), - }); + const existing = await this.tenantProvider.findBySlug(input.slug); - if (existing) { + if (existing && existing.id !== id) { throw new BadRequestException( 'Slug is already taken by another tenant', ); } } - const updatePayload: Partial = {}; - if (input.name !== undefined) updatePayload.name = input.name; - if (input.slug !== undefined) updatePayload.slug = input.slug; - if (input.logo !== undefined) updatePayload.logo = input.logo; - if (input.status !== undefined) updatePayload.status = input.status; - if (input.metadata !== undefined) - updatePayload.metadata = JSON.stringify(input.metadata); - - if (Object.keys(updatePayload).length === 0) { + if (Object.keys(input).length === 0) { throw new BadRequestException('No fields to update'); } - const [updated] = await this.db - .update(schema.organization) - .set(updatePayload) - .where(eq(schema.organization.id, id)) - .returning(); - + const updated = await this.tenantProvider.update(id, input); return updated; } @Delete('tenants/:id') @UseGuards(SystemAdminGuard) async deleteTenant(@Param('id') id: string) { - const tenant = await this.db.query.organization.findFirst({ - where: eq(schema.organization.id, id), - }); + const tenant = await this.tenantProvider.findById(id); if (!tenant) { throw new NotFoundException('Tenant not found'); } - await this.db.transaction(async (tx) => { - // Hard delete dependent records and organization - await tx - .delete(schema.member) - .where(eq(schema.member.organizationId, id)); - await tx - .delete(schema.invitation) - .where(eq(schema.invitation.organizationId, id)); - await tx - .delete(schema.organization) - .where(eq(schema.organization.id, id)); - }); + await this.tenantProvider.delete(id); return { success: true }; } @@ -248,30 +196,18 @@ export class SystemAdminController { ) { const MAX_PAGE_SIZE = 100; // Basic pagination (Convert to Number safely) - const p = Math.max(1, parseInt(page) || 1); + const p = Math.max(1, Number.parseInt(page) || 1); const limit = Math.max( 1, - Math.min(MAX_PAGE_SIZE, parseInt(pageSize) || 10), + Math.min(MAX_PAGE_SIZE, Number.parseInt(pageSize) || 10), ); - const offset = (p - 1) * limit; - const users = await this.db.query.user.findMany({ + const result = await this.userProvider.findAll({ + page: p, limit, - offset, - orderBy: (users, { desc }) => [desc(users.createdAt)], - }); + }); // No tenantId -> Global list - // Total count for pagination - const totalResult = await this.db - .select({ count: count() }) - .from(schema.user); - const total = Number(totalResult[0]?.count || 0); - - return { - // Envelope - data: users, - total, - }; + return result; // Envelope { data, total } matches } @Patch('users/:id') @@ -280,9 +216,7 @@ export class SystemAdminController { @Param('id') id: string, @Body() input: UpdateUserValidation, ) { - const user = await this.db.query.user.findFirst({ - where: eq(schema.user.id, id), - }); + const user = await this.userProvider.findById(id); if (!user) { throw new NotFoundException('User not found'); @@ -290,54 +224,25 @@ export class SystemAdminController { // Check email uniqueness if changing if (input.email && input.email !== user.email) { - const existing = await this.db.query.user.findFirst({ - where: and(eq(schema.user.email, input.email), ne(schema.user.id, id)), - }); + const existing = await this.userProvider.findByEmail(input.email); - if (existing) { + if (existing && existing.id !== id) { throw new BadRequestException('Email already in use'); } } - const updatePayload: Partial = {}; - if (input.name !== undefined) updatePayload.name = input.name; - if (input.systemRole !== undefined) - updatePayload.systemRole = input.systemRole; - if (input.email !== undefined) updatePayload.email = input.email; - if (input.emailVerified !== undefined) - updatePayload.emailVerified = input.emailVerified; - - if (Object.keys(updatePayload).length === 0) { + if (Object.keys(input).length === 0) { throw new BadRequestException('No fields to update'); } - try { - const [updated] = await this.db - .update(schema.user) - .set(updatePayload) - .where(eq(schema.user.id, id)) - .returning(); - - return updated; - } catch (error) { - // Catch race conditions for unique constraints - if ( - error instanceof Error && - (error.message.includes('unique') || - error.message.includes('duplicate')) - ) { - throw new BadRequestException('Email already in use'); - } - throw error; - } + const updated = await this.userProvider.update(id, input); + return updated; } @Get('users/:id') @UseGuards(PlatformGuard) async getUser(@Param('id') id: string) { - const user = await this.db.query.user.findFirst({ - where: eq(schema.user.id, id), - }); + const user = await this.userProvider.findById(id); if (!user) { throw new NotFoundException('User not found'); @@ -349,9 +254,7 @@ export class SystemAdminController { @Delete('users/:id') @UseGuards(SystemAdminGuard) async deleteUser(@Param('id') id: string) { - const user = await this.db.query.user.findFirst({ - where: eq(schema.user.id, id), - }); + const user = await this.userProvider.findById(id); if (!user) { throw new NotFoundException('User not found'); @@ -359,48 +262,21 @@ export class SystemAdminController { // Safety: Prevent deleting the last platform admin if (user.systemRole === 'platform_admin') { - const adminCountResult = await this.db - .select({ count: count() }) - .from(schema.user) - .where( - and( - eq(schema.user.systemRole, 'platform_admin'), - ne(schema.user.id, id), - ), - ); + const adminCount = await this.userProvider.count({ + systemRole: 'platform_admin', + }); - const otherAdmins = Number(adminCountResult[0]?.count || 0); - if (otherAdmins === 0) { + // The count includes the current target user. + // A count of 1 means this is the LAST admin left. + // Therefore, we must prevent deletion if count <= 1. + if (adminCount <= 1) { throw new BadRequestException( 'Cannot delete the last Platform Administrator', ); } } - // Transactional cleanup - await this.db.transaction(async (tx) => { - // 1. Delete memberships - await tx.delete(schema.member).where(eq(schema.member.userId, id)); - - // 2. Delete invitations - // - Created by this user (Inviter) - await tx - .delete(schema.invitation) - .where(eq(schema.invitation.inviterId, id)); - - // - Sent TO this user's email (Recipient) - // Note: We use user.email here, which we fetched above. - await tx - .delete(schema.invitation) - .where(eq(schema.invitation.email, user.email)); - - // 3. Delete session/account/etc (BetterAuth handles this typically if cascading, but we do manual for safety) - await tx.delete(schema.session).where(eq(schema.session.userId, id)); - await tx.delete(schema.account).where(eq(schema.account.userId, id)); - - // 4. Delete user - await tx.delete(schema.user).where(eq(schema.user.id, id)); - }); + await this.userProvider.delete(id); return { success: true }; } @@ -412,69 +288,24 @@ export class SystemAdminController { @Query('pageSize') pageSize = '10', ) { const MAX_PAGE_SIZE = 100; - const p = Math.max(1, parseInt(page) || 1); + const p = Math.max(1, Number.parseInt(page) || 1); const limit = Math.max( 1, - Math.min(MAX_PAGE_SIZE, parseInt(pageSize) || 10), + Math.min(MAX_PAGE_SIZE, Number.parseInt(pageSize) || 10), ); - const offset = (p - 1) * limit; - - const tenantsData = await this.db - .select({ - id: schema.organization.id, - name: schema.organization.name, - slug: schema.organization.slug, - createdAt: schema.organization.createdAt, - logo: schema.organization.logo, - metadata: schema.organization.metadata, - updatedAt: schema.organization.updatedAt, - status: schema.organization.status, // Included status - userCount: count(schema.member.id), - }) - .from(schema.organization) - .leftJoin( - schema.member, - eq(schema.organization.id, schema.member.organizationId), - ) - .groupBy(schema.organization.id) - .limit(limit) - .offset(offset) - .orderBy(desc(schema.organization.createdAt)) - .execute(); - - // Total count of tenants - const totalResult = await this.db - .select({ count: count() }) - .from(schema.organization); - const total = Number(totalResult[0]?.count || 0); - - return { - data: tenantsData.map((t) => ({ - ...t, - userCount: Number(t.userCount), - })), - total, - }; + + const result = await this.tenantProvider.findAll({ + page: p, + limit, + }); + + return result; // Envelope { data, total } matches } @Get('tenants/:id') @UseGuards(PlatformGuard) async getTenant(@Param('id') id: string) { - const [tenant] = await this.db - .select({ - id: schema.organization.id, - name: schema.organization.name, - slug: schema.organization.slug, - createdAt: schema.organization.createdAt, - logo: schema.organization.logo, - metadata: schema.organization.metadata, - updatedAt: schema.organization.updatedAt, - status: schema.organization.status, - }) - .from(schema.organization) - .where(eq(schema.organization.id, id)) - .limit(1) - .execute(); + const tenant = await this.tenantProvider.findById(id); if (!tenant) { throw new NotFoundException('Tenant not found'); diff --git a/apps/api/src/modules/identity/tenants/tenants.controller.spec.ts b/apps/api/src/modules/identity/tenants/tenants.controller.spec.ts index 4d6c08ae..b6b7a621 100644 --- a/apps/api/src/modules/identity/tenants/tenants.controller.spec.ts +++ b/apps/api/src/modules/identity/tenants/tenants.controller.spec.ts @@ -1,15 +1,14 @@ import { Test, TestingModule } from '@nestjs/testing'; import { Request } from 'express'; import { TenantsController } from './tenants.controller'; -import { TenantsService } from './tenants.service'; +import { TENANT_PROVIDER, Tenant } from '@nexiom/identity'; import { AuthGuard } from '../auth/auth.guard'; -import { Organization } from './tenant.schema'; import { UpdateTenantStatus } from './tenants.validation'; describe('TenantsController', () => { let controller: TenantsController; - const mockTenantsService = { + const mockTenantProvider = { findAllForUser: jest.fn(), updateStatus: jest.fn(), }; @@ -23,8 +22,8 @@ describe('TenantsController', () => { controllers: [TenantsController], providers: [ { - provide: TenantsService, - useValue: mockTenantsService, + provide: TENANT_PROVIDER, + useValue: mockTenantProvider, }, ], }) @@ -43,7 +42,7 @@ describe('TenantsController', () => { describe('findAll', () => { it('should return an array of organizations for the user', async () => { - const result: Organization[] = [ + const result: Tenant[] = [ { id: '1', name: 'Test Org', @@ -51,17 +50,17 @@ describe('TenantsController', () => { logo: null, createdAt: new Date(), updatedAt: new Date(), - metadata: null, + metadata: undefined, status: 'active', }, ]; - mockTenantsService.findAllForUser.mockResolvedValue(result); + mockTenantProvider.findAllForUser.mockResolvedValue(result); const req = { user: { id: 'user-1' } } as unknown as Request & { user: { id: string }; }; expect(await controller.findAll(req)).toBe(result); - expect(mockTenantsService.findAllForUser).toHaveBeenCalledWith('user-1'); + expect(mockTenantProvider.findAllForUser).toHaveBeenCalledWith('user-1'); }); }); @@ -70,10 +69,10 @@ describe('TenantsController', () => { const id = '1'; const status: UpdateTenantStatus = { status: 'disabled' }; const result = { id, status: status.status }; - mockTenantsService.updateStatus.mockResolvedValue(result); + mockTenantProvider.updateStatus.mockResolvedValue(result); expect(await controller.updateStatus(id, status)).toBe(result); - expect(mockTenantsService.updateStatus).toHaveBeenCalledWith( + expect(mockTenantProvider.updateStatus).toHaveBeenCalledWith( id, status.status, ); diff --git a/apps/api/src/modules/identity/tenants/tenants.controller.ts b/apps/api/src/modules/identity/tenants/tenants.controller.ts index ce102210..89169b2b 100644 --- a/apps/api/src/modules/identity/tenants/tenants.controller.ts +++ b/apps/api/src/modules/identity/tenants/tenants.controller.ts @@ -6,24 +6,27 @@ import { Body, UseGuards, Req, + Inject, } from '@nestjs/common'; import { Request } from 'express'; -import { TenantsService } from './tenants.service'; import { AuthGuard } from '../auth/auth.guard'; import { UpdateTenantStatus } from './tenants.validation'; +import { TENANT_PROVIDER, ITenantProvider } from '@nexiom/identity'; @Controller('tenants') @UseGuards(AuthGuard) export class TenantsController { - constructor(private readonly tenantsService: TenantsService) {} + constructor( + @Inject(TENANT_PROVIDER) private readonly tenantProvider: ITenantProvider, + ) {} @Get() findAll(@Req() req: Request & { user: { id: string } }) { - return this.tenantsService.findAllForUser(req.user.id); + return this.tenantProvider.findAllForUser(req.user.id); } @Patch(':id') updateStatus(@Param('id') id: string, @Body() body: UpdateTenantStatus) { - return this.tenantsService.updateStatus(id, body.status); + return this.tenantProvider.updateStatus(id, body.status); } } diff --git a/apps/api/src/modules/identity/tenants/tenants.module.ts b/apps/api/src/modules/identity/tenants/tenants.module.ts index 83675caa..e8a51b8a 100644 --- a/apps/api/src/modules/identity/tenants/tenants.module.ts +++ b/apps/api/src/modules/identity/tenants/tenants.module.ts @@ -1,12 +1,10 @@ import { Module } from '@nestjs/common'; -import { TenantsService } from './tenants.service'; import { TenantsController } from './tenants.controller'; -import { DbModule } from '../../../db/db.module'; @Module({ - imports: [DbModule], + imports: [], controllers: [TenantsController], - providers: [TenantsService], - exports: [TenantsService], // Export for AuthModule to use + providers: [], + exports: [], }) export class TenantsModule {} diff --git a/apps/api/src/modules/identity/tenants/tenants.service.spec.ts b/apps/api/src/modules/identity/tenants/tenants.service.spec.ts deleted file mode 100644 index 1d49e976..00000000 --- a/apps/api/src/modules/identity/tenants/tenants.service.spec.ts +++ /dev/null @@ -1,187 +0,0 @@ -import { Test, TestingModule } from '@nestjs/testing'; -import { TenantsService } from './tenants.service'; -import { DRIZZLE_DB } from '../../../db/db.provider'; -import { DbOrganization as Organization } from '../../../db/schema'; - -const mockOrganizations: Organization[] = [ - { - id: '1', - name: 'Test Org 1', - slug: 'test-org-1', - logo: null, - createdAt: new Date(), - updatedAt: new Date(), - metadata: null, - status: 'active', - deletedAt: null, - }, - { - id: '2', - name: 'Test Org 2', - slug: 'test-org-2', - logo: null, - createdAt: new Date(), - updatedAt: new Date(), - metadata: null, - status: 'disabled', - deletedAt: null, - }, -]; - -// Define a type for our mock that satisfies the functionality we use -// We don't try to implement the full NodePgDatabase interface as it's too complex to mock fully manually -interface MockDrizzle { - select: jest.Mock; - from: jest.Mock; - where: jest.Mock; - limit: jest.Mock; - execute: jest.Mock; - insert: jest.Mock; - values: jest.Mock; - returning: jest.Mock; - transaction: jest.Mock; - update: jest.Mock; - set: jest.Mock; - innerJoin: jest.Mock; - - query: MockQuery; -} - -interface MockQuery { - user: { - findFirst: jest.Mock; - }; -} - -// Instantiate with type safety -const mockDb: MockDrizzle = { - select: jest.fn().mockReturnThis(), - from: jest.fn().mockReturnThis(), - where: jest.fn().mockReturnThis(), - limit: jest.fn().mockReturnThis(), - execute: jest.fn(), - insert: jest.fn().mockReturnThis(), - values: jest.fn().mockReturnThis(), - returning: jest.fn(), - transaction: jest.fn(), // Placeholder, implementation below to avoid circular ref - update: jest.fn().mockReturnThis(), - set: jest.fn().mockReturnThis(), - innerJoin: jest.fn().mockReturnThis(), - query: { - user: { - findFirst: jest.fn(), - }, - }, -}; - -// Implement circular transaction logic safely -mockDb.transaction.mockImplementation( - (cb: (tx: MockDrizzle) => Promise) => { - return cb(mockDb); - }, -); - -describe('TenantsService', () => { - let service: TenantsService; - - beforeEach(async () => { - const module: TestingModule = await Test.createTestingModule({ - providers: [ - TenantsService, - { - provide: DRIZZLE_DB, - useValue: mockDb, - }, - ], - }).compile(); - - service = module.get(TenantsService); - - jest.clearAllMocks(); - }); - - it('should be defined', () => { - expect(service).toBeDefined(); - }); - - describe('findAllForUser', () => { - it('should return tenants for the user', async () => { - // Mock the join query chain - // db.select().from().innerJoin().where().execute() - mockDb.execute.mockResolvedValue(mockOrganizations); - - const result = await service.findAllForUser('user-1'); - - expect(result).toEqual(mockOrganizations); - expect(mockDb.innerJoin).toHaveBeenCalled(); - expect(mockDb.where).toHaveBeenCalled(); - }); - }); - - describe('updateStatus', () => { - it('should update tenant status', async () => { - const updatedOrg: Organization = { - ...mockOrganizations[0], - status: 'disabled', - }; - mockDb.returning.mockResolvedValue([updatedOrg]); - - const result = await service.updateStatus('1', 'disabled'); - - expect(result).toEqual(updatedOrg); - expect(mockDb.update).toHaveBeenCalled(); - }); - - it('should throw error if tenant not found', async () => { - mockDb.returning.mockResolvedValue([]); - - await expect(service.updateStatus('999', 'disabled')).rejects.toThrow(); - }); - }); - - describe('createTenant', () => { - it('should create organization and member transactionally', async () => { - const newOrg: Organization = { ...mockOrganizations[0] }; - // specialized mocks for tx - mockDb.returning.mockResolvedValueOnce([newOrg]); // for org insert - // member insert doesn't return anything we check explicitly here, - // but the tx should return the org. - - const result = await service.createTenant('user-1', 'Test Corp'); - - expect(result).toEqual(newOrg); - expect(mockDb.transaction).toHaveBeenCalled(); - expect(mockDb.insert).toHaveBeenCalledTimes(2); // Org + Member - }); - }); - - describe('provisionTenantForUser', () => { - it('should provision a tenant with generated name', async () => { - const mockUser = { id: 'user-1', email: 'test@example.com' }; - const newOrg: Organization = { - ...mockOrganizations[0], - name: 'Organization X', - }; - - // Mock user lookup - mockDb.query.user.findFirst.mockResolvedValueOnce(mockUser); - - // Mock createTenant tx - mockDb.returning.mockResolvedValueOnce([newOrg]); // Org insert - - const result = await service.provisionTenantForUser('user-1'); - - expect(mockDb.query.user.findFirst).toHaveBeenCalled(); // User lookup - expect(mockDb.transaction).toHaveBeenCalled(); // createTenant - expect(result).toEqual(newOrg); - }); - - it('should throw if user not found', async () => { - mockDb.query.user.findFirst.mockResolvedValueOnce(null); // No user - - await expect( - service.provisionTenantForUser('user-999'), - ).rejects.toThrow(); - }); - }); -}); diff --git a/apps/api/src/modules/identity/tenants/tenants.service.ts b/apps/api/src/modules/identity/tenants/tenants.service.ts deleted file mode 100644 index 2e91ca15..00000000 --- a/apps/api/src/modules/identity/tenants/tenants.service.ts +++ /dev/null @@ -1,118 +0,0 @@ -import { Injectable, Inject } from '@nestjs/common'; -import { DRIZZLE_DB } from '../../../db/db.provider'; -import { NodePgDatabase } from 'drizzle-orm/node-postgres'; -import * as schema from '../../../db/schema'; -import { eq } from 'drizzle-orm'; -import { randomUUID } from 'crypto'; - -@Injectable() -export class TenantsService { - constructor( - @Inject(DRIZZLE_DB) private readonly db: NodePgDatabase, - ) {} - - async findAllForUser(userId: string) { - const query = this.db - .select({ - id: schema.organization.id, - name: schema.organization.name, - slug: schema.organization.slug, - logo: schema.organization.logo, - createdAt: schema.organization.createdAt, - metadata: schema.organization.metadata, - status: schema.organization.status, - memberRole: schema.member.role, // Optional: Return their role in that org - }) - .from(schema.organization) - .innerJoin( - schema.member, - eq(schema.member.organizationId, schema.organization.id), - ) - .where(eq(schema.member.userId, userId)); - - return query.execute(); - } - - async updateStatus(id: string, status: 'active' | 'disabled' | 'suspended') { - const result = await this.db - .update(schema.organization) - .set({ status }) - .where(eq(schema.organization.id, id)) - .returning(); - - if (!result[0]) { - throw new Error(`Organization with id ${id} not found`); - } - return result[0]; - } - - /** - * Domain Logic: Create a new Tenant (Organization) and assign the Creator as Admin. - */ - async createTenant(userId: string, name: string) { - const orgId = randomUUID(); - const slug = this.generateSlug(name); - - return await this.db.transaction(async (tx) => { - // 1. Create Organization - const [org] = await tx - .insert(schema.organization) - .values({ - id: orgId, - name: name, - slug: slug, - createdAt: new Date(), - status: 'active', - }) - .returning(); - - // 2. Add Member (Admin) - await tx.insert(schema.member).values({ - id: randomUUID(), - organizationId: orgId, - userId: userId, - role: 'admin', - createdAt: new Date(), - }); - - return org; - }); - } - - /** - * Orchestration Logic: Auto-provision a tenant for a given User ID. - * Derives company name from user metadata if possible, or generates a default. - */ - async provisionTenantForUser( - userId: string, - ): Promise { - // 1. Fetch User to get name/email for auto-naming (optional, but good UX) - const user = await this.db.query.user.findFirst({ - where: eq(schema.user.id, userId), - }); - - if (!user) { - throw new Error(`User with ID ${userId} not found`); - } - - // 2. Generate Company Name - // Logic: Try Company Name field? user doesn't have it standard. - // Use fallback: "Organization " - const randomSuffix = Math.random().toString(36).substring(7); - const companyName = `Organization ${randomSuffix}`; - - // 3. Create - return this.createTenant(userId, companyName); - } - - private generateSlug(name: string): string { - return ( - name - .toLowerCase() - .replace(/\s+/g, '-') - .replace(/[^a-z0-9-]/g, '') + - '-' + - randomUUID().slice(0, 4) - ); - } -} diff --git a/apps/api/src/modules/identity/users/users.controller.spec.ts b/apps/api/src/modules/identity/users/users.controller.spec.ts index 3da87bdd..842158b3 100644 --- a/apps/api/src/modules/identity/users/users.controller.spec.ts +++ b/apps/api/src/modules/identity/users/users.controller.spec.ts @@ -94,12 +94,12 @@ describe('UsersController', () => { } as unknown as Request & { user: { organizationId?: string } }; const users = [{ id: '1' }]; - mockUserProvider.findAll.mockResolvedValue(users); + mockUserProvider.findAll.mockResolvedValue({ data: users, total: 1 }); const result = await controller.findAll(req); - expect(result).toEqual(users); + expect(result).toEqual({ data: users, total: 1 }); - expect(mockUserProvider.findAll).toHaveBeenCalledWith(tenantId); + expect(mockUserProvider.findAll).toHaveBeenCalledWith({ tenantId }); }); }); diff --git a/apps/api/src/modules/identity/users/users.controller.ts b/apps/api/src/modules/identity/users/users.controller.ts index f505dcc2..e2ecc157 100644 --- a/apps/api/src/modules/identity/users/users.controller.ts +++ b/apps/api/src/modules/identity/users/users.controller.ts @@ -43,7 +43,7 @@ export class UsersController { * @returns List of users. */ @Get() - findAll(@Req() req: Request & { user: { organizationId?: string } }) { + async findAll(@Req() req: Request & { user: { organizationId?: string } }) { // AuthGuard guarantees session is valid and populates user info // We use 'organizationId' (mapped in getSessionWithOrg) const tenantId = req.user?.organizationId; @@ -55,7 +55,8 @@ export class UsersController { return []; } - return this.userProvider.findAll(tenantId); + const result = await this.userProvider.findAll({ tenantId }); + return result; } /** diff --git a/apps/api/src/modules/identity/users/users.module.ts b/apps/api/src/modules/identity/users/users.module.ts index 4f14fbc7..6ced066f 100644 --- a/apps/api/src/modules/identity/users/users.module.ts +++ b/apps/api/src/modules/identity/users/users.module.ts @@ -1,9 +1,8 @@ import { Module } from '@nestjs/common'; import { UsersController } from './users.controller'; -import { DbModule } from '../../../db/db.module'; @Module({ - imports: [DbModule], + imports: [], controllers: [UsersController], providers: [], }) diff --git a/docs/draft/identity_module_design_doc.md b/docs/draft/identity_module_design_doc.md new file mode 100644 index 00000000..7df93506 --- /dev/null +++ b/docs/draft/identity_module_design_doc.md @@ -0,0 +1,227 @@ +# Identity Module Architecture: The "Guardianship" Kernel + +**Status:** RFC (Request for Comments) +**Author:** Staff Architect +**Version:** 2.0 (Aligns with Nexiom Master Architecture) + +--- + +## 1. Architectural Context (C4 Model) + +The **Identity Module** (`@nexiom/identity`) is the "Security Kernel" of the Nexiom Platform. It is NOT just a user table; it is the **Authority** for: + +1. **Authentication:** Who are you? (Users/Machines) +2. **Multitenancy:** Which data silo do you own? (Tenant Resolution) +3. **Authorization:** What can you click? (CBAC: Capability-Based Access Control) +4. **Credental Management:** How do we talk to external apps? (Layer 5 Support) + +### 1.1 System Context Diagram (Level 1) + +```mermaid +C4Context + title System Context: Identity Module + + Person(user, "User", "System Admin or Tenant Member") + System_Ext(auth_provider, "Auth Provider", "BetterAuth / Clerk / Supabase") + + System_Boundary(nexiom, "Nexiom Platform") { + System(api, "API Monolith", "NestJS Backend") + System(identity, "Identity Kernel", "@nexiom/identity Package") + System(db, "Database", "Postgres (Public & Tenant Schemas)") + } + + Rel(user, api, "Uses", "HTTPS/JSON") + Rel(api, identity, "Delegates Auth To", "Interface Calls") + Rel(identity, auth_provider, "Verifies Tokens With", "HTTP/SDK") + Rel(identity, db, "Reads/Writes User Data", "SQL/ORM") +``` + +### 1.2 Container Diagram (Level 2) + +Shows how the Identity Package is embedded within the Monolith but logically isolated. + +```mermaid +C4Container + title Container Diagram: Identity Package Integration + + Container_Boundary(apps, "Applications") { + Container(api_layer, "API Layer", "NestJS Controllers", "Handles HTTP, Validation, Routing") + } + + Container_Boundary(libs, "Shared Libraries") { + Container(identity_pkg, "Identity Package", "@nexiom/identity", "Interfaces, Adapters, Guards") + } + + ContainerDb(db_users, "Public Schema", "Postgres", "Users, Tenants, Memberships") + + Rel(api_layer, identity_pkg, "Injects Interfaces", "Dependency Injection") + Rel(identity_pkg, db_users, "Manages", "Drizzle ORM") +``` + +--- + +## 2. The Identity & The 6-Layer Pipeline + +Identity is an **Orthogonal Concern**—it intersects the pipeline layers rather than being a step within them. + +### 2.1 Interaction Flow + +```mermaid +sequenceDiagram + autonumber + participant Gateway as Layer 1 (Gateway) + participant Worker as Layer 2-4 (Pipeline) + participant Delivery as Layer 5 (Delivery) + participant Identity as @nexiom/identity + participant DB as Public Schema + + Note over Gateway: 1. Ingestion + Gateway->>Identity: Validate Webhook Signature? + Identity-->>Gateway: OK (Shared Secret Check) + + Note over Worker: 2. Processing + Worker->>Identity: Resolve Tenant Context (envoy -> uuid) + Identity->>DB: SELECT id FROM tenants WHERE slug = 'envoy' + DB-->>Identity: tenant_uuid + Identity-->>Worker: Context { tenantId: '...' } + + Note over Delivery: 5. Execution + Delivery->>Identity: Get External App Credentials (Connection) + Identity->>DB: Decrypt OAuth Tokens + DB-->>Identity: Access Token + Identity-->>Delivery: Credentials +``` + +### 2.2 Layer Responsibilities + +| Pipeline Layer | Identity Responsibility | Interface Used | +| :--- | :--- | :--- | +| **L1: Gateway** | **Tenant Resolution** (Subdomain/Path) & **Signature Verification** (HMAC). | `ITenantProvider` | +| **L2: Replica** | **Context Injection** (Setting `AsyncLocalStorage`). | `ITenantProvider` | +| **L3: Norm** | N/A (Pure Logic) | N/A | +| **L4: Outbound**| N/A (Pure Logic) | N/A | +| **L5: Delivery** | **Credential Retrieval** (Decrypting OAuth tokens for destinations). | `ICredentialProvider` (Future) | +| **API (UI)** | **User Auth** (Session) & **RBAC** (Guards). | `IAuthProvider`, `IPermissionProvider` | + +--- + +## 3. Core Design Decisions (ADRs) + +### ADR-001: The Adapter Pattern + +* **Context:** We want to support "SaaS-in-a-Box" where users can bring their own Auth (Clerk, Auth0) or Database. +* **Decision:** All Identity logic must sit behind **Interfaces**. The API Layer NEVER imports the ORM directly for Identity tables. +* **Consequences:** + * (+) Zero vendor lock-in. + * (+) Easy to mock for TDD. + * (-) Slight boilerplate overhead (Interface + Adapter). + +### ADR-002: Capability-Based Access Control (CBAC) + +* **Context:** "Roles" (Admin, User) are too rigid for complex B2B apps. +* **Decision:** We perform checks on **Capabilities** (`create:tenant`, `read:report`), not Roles. +* **Consequences:** + * (+) Granular control. + * (+) Creating custom roles is just grouping capabilities. + +--- + +## 4. Source Code Structure (The Blueprint) + +### 4.1 Directory Map + +```text +packages/identity/ +├── src/ +│ ├── interfaces/ # 👈 THE LAW (Contracts) +│ │ ├── auth.provider.interface.ts +│ │ ├── user.provider.interface.ts +│ │ ├── tenant.provider.interface.ts +│ │ ├── permission.provider.interface.ts +│ │ +│ ├── adapters/ # 👈 THE WORKERS (Implementations) +│ │ ├── better-auth.adapter.ts +│ │ ├── drizzle-user.adapter.ts +│ │ ├── drizzle-tenant.adapter.ts +│ │ +│ ├── constants.ts # Dependency Injection Tokens +│ └── identity.module.ts # NestJS Dynamic Module +``` + +### 4.2 Key Interface Definitions + +#### `ITenantProvider` + +The "Landlord" of the system. + +```typescript +export interface ITenantProvider { + // Discovery + findBySlug(slug: string): Promise; + findById(id: string): Promise; + + // Resolution + resolveContext(request: Request): Promise; + + // Lifecycle + create(userId: string, name: string): Promise; + provisionTenantForUser(userId: string): Promise; // "Sign up and give me a workspace" +} +``` + +#### `IPermissionProvider` + +The "Bouncer" of the system. + +```typescript +export interface IPermissionProvider { + // The only method you actually need + can(user: User, action: string, resource: string): Promise; + + // Example usage: + // can(user, 'delete', 'production_db') +} +``` + +--- + +## 5. Flow Diagram: The "Strict Layer" Request + +How a `GET /admin/users` request travels through the refined architecture. + +```mermaid +sequenceDiagram + autonumber + participant Client + participant Controller as SystemAdminController + participant Guard as AuthGuard + participant UserProvider as DI Token (USER_PROVIDER) + participant Adapter as DrizzleUserAdapter + participant DB as Postgres + + Client->>Controller: GET /admin/users + + rect rgb(240, 248, 255) + Note right of Client: 1. Authentication Layer + Controller->>Guard: canActivate() + Guard->>Adapter: validateSession(token) + Adapter-->>Guard: User Session + end + + rect rgb(255, 240, 245) + Note right of Client: 2. Application Layer + Controller->>UserProvider: findAll({ tenantId: '...' }) + Note over Controller: Controller does NOT know about Drizzle/SQL + end + + rect rgb(240, 255, 240) + Note right of Client: 3. Infrastructure Layer + UserProvider->>Adapter: Adapter.findAll() + Adapter->>DB: db.select().from(users)... + DB-->>Adapter: Result Rows + Adapter-->>UserProvider: Mapped User[] + end + + UserProvider-->>Controller: User[] + Controller-->>Client: 200 OK +``` diff --git a/docs/old/nexiom_tech_stack_and_repo_plan.md b/docs/old/nexiom_tech_stack_and_repo_plan.md index 4538408b..f052922a 100644 --- a/docs/old/nexiom_tech_stack_and_repo_plan.md +++ b/docs/old/nexiom_tech_stack_and_repo_plan.md @@ -38,6 +38,7 @@ The repository follows a standard **Turborepo** layout. We strictly separate **S │ │ │ │ ├── auth.controller.ts # Login / Session Endpoints │ │ │ │ ├── user.controller.ts # Profile / Invites Endpoints │ │ │ │ └── tenant.controller.ts # Org / Schema Endpoints +│ │ │ ├── /notification # Notification Controller │ │ │ ├── /billing # Billing Controller │ │ │ └── /engine # ⚙️ Integration API │ │ │ ├── /webhooks # Ingestion Endpoint diff --git a/docs/refactor/modules/identity/task/01c_strict_layer_enforcement.md b/docs/refactor/modules/identity/task/01c_strict_layer_enforcement.md new file mode 100644 index 00000000..0edb24de --- /dev/null +++ b/docs/refactor/modules/identity/task/01c_strict_layer_enforcement.md @@ -0,0 +1,58 @@ +# Task 01c: Strict Layer Enforcement + +**Priority:** CRITICAL +**Estimated Time:** 4 hours +**Assignee:** Coder +**Status:** Ready to Start + +--- + +## Objective + +Fix the architectural violations in the Identity module. Specifically, `SystemAdminController` is bypassing the adapter layer and accessing the database directly, leading to brittle tests and vendor lock-in. + +**Goal:** Ensure all Controllers interact ONLY with Interfaces (`IUserProvider`, `ITenantProvider`), never raw DB connections. + +--- + +## Directives + +> [!IMPORTANT] +> **Strict Rule:** No Controller in `apps/api` shall import `DRIZZLE_DB` or `NodePgDatabase`. + +## Implementation Steps + +### 1. Enhance Interfaces + +- [ ] **Modify `IUserProvider`**: Add methods to support Admin Console requirements. + - `findAll(options?: { page: number; limit: number; search?: string }): Promise<{ data: User[]; total: number }>` + - `count(): Promise` +- [ ] **Modify `ITenantProvider`**: Add methods for Admin listing. + - `findAll(options?: { page: number; limit: number }): Promise<{ data: Tenant[]; total: number }>` + +### 2. Implement Adapters + +- [ ] **Update `DrizzleUserAdapter`**: Implement the new `findAll` and `count` methods using Drizzle. +- [ ] **Update `DrizzleTenantAdapter`**: Implement the new listing logic. + +### 3. Refactor Controller + +- [ ] **Target:** `apps/api/src/modules/identity/system-admin/system-admin.controller.ts` +- [ ] **Action:** Remove `@Inject(DRIZZLE_DB) private readonly db`. +- [ ] **Action:** Inject `@Inject(USER_PROVIDER) private readonly userProvider`. +- [ ] **Action:** Inject `@Inject(TENANT_PROVIDER) private readonly tenantProvider`. +- [ ] **Action:** Replace all `this.db.query...` calls with `this.userProvider...` or `this.tenantProvider...` calls. + +### 4. Rewrite Tests + +- [ ] **Target:** `apps/api/src/modules/identity/system-admin/system-admin.controller.spec.ts` +- [ ] **Action:** Remove all `MockDb` and Drizzle-chain mocking. +- [ ] **Action:** Mock the `IUserProvider` and `ITenantProvider` interfaces. +- [ ] **Verification:** Tests should look like: `expect(userProvider.findAll).toHaveBeenCalledWith(...)`. + +--- + +## Verification Plan + +- [ ] **Search:** Grep for `DRIZZLE_DB` in `apps/api/src/modules/identity/`. Result should be empty (except for Module definition). +- [ ] **Test:** Run `pnpm test apps/api`. Tests must pass without mocking Drizzle internals. diff --git a/docs/refactor/modules/identity/task/01d_retire_legacy_tenants_service.md b/docs/refactor/modules/identity/task/01d_retire_legacy_tenants_service.md new file mode 100644 index 00000000..b3e4b95d --- /dev/null +++ b/docs/refactor/modules/identity/task/01d_retire_legacy_tenants_service.md @@ -0,0 +1,48 @@ +# Task 01d: Retire Legacy TenantsService + +**Priority:** HIGH +**Estimated Time:** 2 hours +**Assignee:** Coder +**Status:** Ready to Start + +--- + +## Objective + +The `TenantsService` class in `apps/api/src/modules/identity/tenants/tenants.service.ts` is a legacy artifact that violates the Strict Layer Enforcement by injecting `DRIZZLE_DB` directly. + +It duplicates logic (e.g., `provisionTenantForUser`) that is now correctly implemented in the `DrizzleTenantAdapter`. + +**Goal:** Remove `TenantsService` entirely and refactor its consumers to use `ITenantProvider`. + +--- + +## Implementation Steps + +### 1. Refactor Consumers + +- [ ] **Target:** `apps/api/src/modules/identity/tenants/tenants.controller.ts` + - Inject `ITenantProvider` (token: `TENANT_PROVIDER`) instead of `TenantsService`. + - Update methods to call provider methods directly. +- [ ] **Target:** `apps/api/src/modules/identity/auth/auth.service.ts` + - Inject `ITenantProvider` instead of `TenantsService`. + - Update `getEnrichedSession` to use `tenantProvider.findAllForUser`. + +### 2. Verify Adapter Capabilities + +- [ ] Ensure `DrizzleTenantAdapter` implements `provisionTenantForUser` correctly (it generates a slug and creates the org/member). +- [ ] Ensure `findAllForUser` is implemented. + +### 3. Cleanup + +- [ ] **Delete:** `apps/api/src/modules/identity/tenants/tenants.service.ts` +- [ ] **Delete:** `apps/api/src/modules/identity/tenants/tenants.service.spec.ts` +- [ ] **Remove Export:** Update `apps/api/src/modules/identity/tenants/tenants.module.ts` to stop exporting `TenantsService`. + +--- + +## Verification Plan + +- [ ] `grep -r "TenantsService" apps/api` should return 0 results (except maybe in migration files if any). +- [ ] `pnpm test apps/api` should pass. +- [ ] `grep -r "DRIZZLE_DB" apps/api/src/modules/identity` should return 0 results. diff --git a/packages/identity/src/adapters/drizzle-tenant.adapter.ts b/packages/identity/src/adapters/drizzle-tenant.adapter.ts index babce727..92a21b9d 100644 --- a/packages/identity/src/adapters/drizzle-tenant.adapter.ts +++ b/packages/identity/src/adapters/drizzle-tenant.adapter.ts @@ -1,7 +1,11 @@ import { NodePgDatabase } from "drizzle-orm/node-postgres"; -import { eq } from "drizzle-orm"; +import { eq, count, ilike, desc, and } from "drizzle-orm"; import { v4 as uuidv4 } from "uuid"; -import { ITenantProvider, Tenant as TenantInterface } from "../interfaces"; +import { + ITenantProvider, + Tenant as TenantInterface, + UpdateTenantInput, +} from "../interfaces"; import * as schema from "../schema"; export class DrizzleTenantAdapter implements ITenantProvider { @@ -52,6 +56,95 @@ export class DrizzleTenantAdapter implements ITenantProvider { throw new Error("Failed to generate unique slug for tenant"); } + async createTenant(input: { + name: string; + slug: string; + logo?: string | null; + }): Promise { + const slug = input.slug.trim(); + if (!slug) { + throw new Error("Tenant slug is required"); + } + + try { + const [org] = await this.db + .insert(schema.organization) + .values({ + id: uuidv4(), + name: input.name, + slug, + logo: input.logo, + createdAt: new Date(), + status: "active", + }) + .returning(); + + return this.mapTenant(org); + } catch (error: any) { + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + if (error.code === "23505") { + throw new Error("Tenant with this slug already exists"); + } + throw error; + } + } + + async update(id: string, input: UpdateTenantInput): Promise { + // Validate slug if provided + if (input.slug !== undefined) { + const trimmedSlug = input.slug.trim(); + if (!trimmedSlug) { + throw new Error("Tenant slug cannot be empty"); + } + input = { ...input, slug: trimmedSlug }; + } + + const { metadata, ...rest } = input; + + try { + const [updated] = await this.db + .update(schema.organization) + .set({ + ...rest, + ...(metadata ? { metadata: JSON.stringify(metadata) } : {}), + updatedAt: new Date(), + }) + .where(eq(schema.organization.id, id)) + .returning(); + + if (!updated) { + throw new Error("Tenant not found"); + } + + return this.mapTenant(updated); + } catch (error: any) { + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-call + if (error.code === "23505" && error.detail?.includes("slug")) { + throw new Error("Tenant with this slug already exists"); + } + throw error; + } + } + + async delete(id: string): Promise { + await this.db.transaction(async (tx) => { + await tx + .delete(schema.member) + .where(eq(schema.member.organizationId, id)); + await tx + .delete(schema.invitation) + .where(eq(schema.invitation.organizationId, id)); + const [deleted] = await tx + .delete(schema.organization) + .where(eq(schema.organization.id, id)) + .returning({ id: schema.organization.id }); + + if (!deleted) { + throw new Error("Tenant not found"); + } + }); + } + // ... private generateSlug(name: string): string { @@ -86,6 +179,44 @@ export class DrizzleTenantAdapter implements ITenantProvider { })); } + async findAll(options?: { + page?: number; + limit?: number; + search?: string; + }): Promise<{ data: TenantInterface[]; total: number }> { + const page = Math.max(1, Math.floor(Number(options?.page) || 1)); + const limit = Math.max(1, Math.floor(Number(options?.limit) || 10)); + const offset = (page - 1) * limit; + + const filters = []; + if (options?.search) { + filters.push(ilike(schema.organization.name, `%${options.search}%`)); + } + + const dataQuery = this.db + .select() + .from(schema.organization) + .where(filters.length ? and(...filters) : undefined) + .limit(limit) + .offset(offset) + .orderBy( + desc(schema.organization.createdAt), + desc(schema.organization.id), + ); + + const [countResult] = await this.db + .select({ count: count(schema.organization.id) }) + .from(schema.organization) + .where(filters.length ? and(...filters) : undefined); + + const tenants = await dataQuery; + + return { + data: tenants.map((t) => this.mapTenant(t)), + total: Number(countResult?.count || 0), + }; + } + async findById(id: string): Promise { const org = await this.db.query.organization.findFirst({ where: eq(schema.organization.id, id), @@ -93,6 +224,13 @@ export class DrizzleTenantAdapter implements ITenantProvider { return org ? this.mapTenant(org) : null; } + async findBySlug(slug: string): Promise { + const org = await this.db.query.organization.findFirst({ + where: eq(schema.organization.slug, slug), + }); + return org ? this.mapTenant(org) : null; + } + async updateStatus( id: string, status: TenantInterface["status"], diff --git a/packages/identity/src/adapters/drizzle-user.adapter.ts b/packages/identity/src/adapters/drizzle-user.adapter.ts index 9a9bebef..dd1a28fb 100644 --- a/packages/identity/src/adapters/drizzle-user.adapter.ts +++ b/packages/identity/src/adapters/drizzle-user.adapter.ts @@ -1,5 +1,5 @@ import { NodePgDatabase } from "drizzle-orm/node-postgres"; -import { eq } from "drizzle-orm"; +import { eq, and, ilike, count, desc } from "drizzle-orm"; import { IUserProvider, CreateUserInput, @@ -17,7 +17,25 @@ export class DrizzleUserAdapter implements IUserProvider { async create(input: CreateUserInput): Promise { // Delegate to AuthProvider to handle account creation (and password hashing) - return await this.authProvider.createUser(input); + const user = await this.authProvider.createUser(input); + + // If systemRole is provided, we need to update the user record immediately + // because the Auth Provider might not support custom fields during creation + if (input.systemRole) { + try { + return await this.update(user.id, { systemRole: input.systemRole }); + } catch (error) { + // Compensating transaction: best‑effort cleanup without masking the root cause + try { + await this.delete(user.id); + } catch (cleanupError) { + void cleanupError; // optional: log cleanupError + } + throw error; + } + } + + return user; } async update(id: string, input: UpdateUserInput): Promise { @@ -84,26 +102,112 @@ export class DrizzleUserAdapter implements IUserProvider { return user ? this.mapUser(user) : null; } - async findAll(tenantId?: string): Promise { - if (tenantId) { - const users = await this.db - .select({ - user: schema.user, - }) + async findAll(options?: { + page?: number; + limit?: number; + search?: string; + tenantId?: string; + systemRole?: string; + }): Promise<{ data: UserInterface[]; total: number }> { + const page = Math.max(1, Math.floor(Number(options?.page) || 1)); + const MAX_LIMIT = 100; + const limit = Math.min( + MAX_LIMIT, + Math.max(1, Math.floor(Number(options?.limit) || 10)), + ); + const offset = (page - 1) * limit; + + const filters = this.buildUserFilters(options); + + if (options?.tenantId) { + // Tenant-scoped (requires Join) + const dataQuery = this.db + .select({ user: schema.user }) .from(schema.user) .innerJoin(schema.member, eq(schema.member.userId, schema.user.id)) - .where(eq(schema.member.organizationId, tenantId)); + .where(filters.length ? and(...filters) : undefined) + .limit(limit) + .offset(offset) + .orderBy(desc(schema.user.createdAt)); + + const [countResult] = await this.db + .select({ count: count(schema.user.id) }) + .from(schema.user) + .innerJoin(schema.member, eq(schema.member.userId, schema.user.id)) + .where(filters.length ? and(...filters) : undefined); + + const users = await dataQuery; + return { + data: users.map((u) => this.mapUser(u.user)), + total: Number(countResult?.count || 0), + }; + } else { + // Global list (Admin) + const dataQuery = this.db + .select() + .from(schema.user) + .where(filters.length ? and(...filters) : undefined) + .limit(limit) + .offset(offset) + .orderBy(desc(schema.user.createdAt)); - return users.map((u) => this.mapUser(u.user)); + const [countResult] = await this.db + .select({ count: count(schema.user.id) }) + .from(schema.user) + .where(filters.length ? and(...filters) : undefined); + + const users = await dataQuery; + return { + data: users.map((u) => this.mapUser(u)), + total: Number(countResult?.count || 0), + }; } + } - // Global list (careful with this in prod) - const users = await this.db - .select() + async count(filters?: { + tenantId?: string; + search?: string; + systemRole?: string; + }): Promise { + const whereConditions = this.buildUserFilters(filters); + + if (filters?.tenantId) { + const [result] = await this.db + .select({ count: count(schema.user.id) }) + .from(schema.user) + .innerJoin(schema.member, eq(schema.member.userId, schema.user.id)) + .where(whereConditions.length ? and(...whereConditions) : undefined); + return Number(result?.count || 0); + } + + const [result] = await this.db + .select({ count: count(schema.user.id) }) .from(schema.user) - .limit(100) - .orderBy(schema.user.createdAt); - return users.map((u) => this.mapUser(u)); + .where(whereConditions.length ? and(...whereConditions) : undefined); + + return Number(result?.count || 0); + } + + private buildUserFilters(filters?: { + tenantId?: string; + search?: string; + systemRole?: string; + }) { + const whereConditions = []; + + if (filters?.search) { + whereConditions.push(ilike(schema.user.email, `%${filters.search}%`)); + } + + if (filters?.systemRole) { + whereConditions.push(eq(schema.user.systemRole, filters.systemRole)); + } + + if (filters?.tenantId) { + whereConditions.push(eq(schema.member.organizationId, filters.tenantId)); + } + + return whereConditions; } async forceVerifyEmail(userId: string): Promise { diff --git a/packages/identity/src/interfaces/tenant-provider.interface.ts b/packages/identity/src/interfaces/tenant-provider.interface.ts index 7e498d97..cccbefef 100644 --- a/packages/identity/src/interfaces/tenant-provider.interface.ts +++ b/packages/identity/src/interfaces/tenant-provider.interface.ts @@ -1,11 +1,38 @@ import { Tenant } from "./types"; +export interface UpdateTenantInput { + name?: string; + slug?: string; + logo?: string | null; + status?: Tenant["status"]; + metadata?: Record; +} + export interface ITenantProvider { + // User-scoped creation (auto-adds member) create(userId: string, name: string): Promise; + // Admin creation (pure tenant) + createTenant(input: { + name: string; + slug: string; + logo?: string | null; + }): Promise; + + update(id: string, input: UpdateTenantInput): Promise; + + delete(id: string): Promise; + findAllForUser(userId: string): Promise<(Tenant & { memberRole?: string })[]>; + findAll(options?: { + page?: number; + limit?: number; + search?: string; + }): Promise<{ data: Tenant[]; total: number }>; + findById(id: string): Promise; + findBySlug(slug: string): Promise; updateStatus(id: string, status: Tenant["status"]): Promise; diff --git a/packages/identity/src/interfaces/user-provider.interface.ts b/packages/identity/src/interfaces/user-provider.interface.ts index 4ad68f45..804e3cde 100644 --- a/packages/identity/src/interfaces/user-provider.interface.ts +++ b/packages/identity/src/interfaces/user-provider.interface.ts @@ -6,6 +6,7 @@ export interface CreateUserInput { firstName?: string; lastName?: string; role?: string; + systemRole?: string; companyName?: string; // Optional: for auto-provisioning } @@ -24,7 +25,22 @@ export interface IUserProvider { findByEmail(email: string): Promise; - findAll(tenantId?: string): Promise; + findAll(options?: { + page?: number; + limit?: number; + search?: string; + tenantId?: string; + systemRole?: string; + }): Promise<{ data: User[]; total: number }>; + + // Kept for backward compatibility if needed, but the above covers it + // findAll(tenantId?: string): Promise; // Removed in favor of options forceVerifyEmail(userId: string): Promise; + + count(filters?: { + tenantId?: string; + search?: string; + systemRole?: string; + }): Promise; }