From 72bcdd56fccd8fc1cbbcaf2dd8b8c50e0a40b592 Mon Sep 17 00:00:00 2001 From: rodrigobnogueira Date: Thu, 13 Aug 2026 17:23:01 -0300 Subject: [PATCH 01/12] fix(tasks): prove the project and assignee belong to the caller's org createTask() inserted a task with a caller-supplied projectId without ever checking that the project lives in the current organization, so a task could be attached to another tenant's project; assignTask() scoped the task but not the assignee, so work could be handed to a user from another org. Both checks now run inside the same synchronous @Transactional body, before any write, against the new MembershipsRepository.findByOrgAndUser() and the existing ProjectsRepository.findByIdInOrg(). Each refuses with exactly the error a nonexistent id gets, so the message is never a cross-tenant existence oracle. --- .../memberships/memberships.repository.ts | 15 ++++++++++++++ src/modules/tasks/tasks.module.ts | 12 ++++++++++- src/modules/tasks/tasks.service.ts | 20 +++++++++++++++++++ 3 files changed, 46 insertions(+), 1 deletion(-) diff --git a/src/modules/memberships/memberships.repository.ts b/src/modules/memberships/memberships.repository.ts index d03ba4b..b8ab1c1 100644 --- a/src/modules/memberships/memberships.repository.ts +++ b/src/modules/memberships/memberships.repository.ts @@ -1,5 +1,6 @@ import { Injectable } from '@nestjs/common'; import { InjectTransaction } from '@nestjs-cls/transactional'; +import { and, eq } from 'drizzle-orm'; import { DrizzleRepository } from '@nest-native/drizzle'; import type { AppDatabase } from '../../database/database'; import { @@ -19,6 +20,20 @@ export interface CreateMembershipInput { export class MembershipsRepository { constructor(@InjectTransaction() private readonly db: AppDatabase) {} + // The tenancy predicate: "is this user a member of this org, and as what?". + // RolesGuard calls it on every mutation (outside any transaction, so the + // @InjectTransaction proxy falls back to the base connection) and TasksService + // calls it inside its transaction to validate an assignee. + findByOrgAndUser(orgId: number, userId: number): Membership | undefined { + return this.db + .select() + .from(memberships) + .where( + and(eq(memberships.orgId, orgId), eq(memberships.userId, userId)), + ) + .get(); + } + create(input: CreateMembershipInput): Membership { return this.db .insert(memberships) diff --git a/src/modules/tasks/tasks.module.ts b/src/modules/tasks/tasks.module.ts index 0216a58..2e8ed92 100644 --- a/src/modules/tasks/tasks.module.ts +++ b/src/modules/tasks/tasks.module.ts @@ -2,15 +2,25 @@ import { Module } from '@nestjs/common'; import { DrizzleModule } from '@nest-native/drizzle'; import { AuthModule } from '../../auth/auth.module'; import { RequestContextModule } from '../../context/request-context.module'; +import { MembershipsRepository } from '../memberships/memberships.repository'; +import { ProjectsRepository } from '../projects/projects.repository'; import { TasksRepository } from './tasks.repository'; import { TasksRouter } from './tasks.router'; import { TasksService } from './tasks.service'; // Mirrors ProjectsModule. The transactional OutboxProducer the service injects // comes from the global MessagingModule, so no messaging wiring lives here. +// The projects/memberships repositories are the tenancy predicates the service +// checks in-transaction (owning project, org member assignee) and the ones +// RolesGuard reads — repositories are stateless, so a second forFeature +// registration is the same pattern OnboardingModule uses. @Module({ imports: [ - DrizzleModule.forFeature([TasksRepository]), + DrizzleModule.forFeature([ + TasksRepository, + ProjectsRepository, + MembershipsRepository, + ]), AuthModule, RequestContextModule, ], diff --git a/src/modules/tasks/tasks.service.ts b/src/modules/tasks/tasks.service.ts index 1da2bae..a4117f8 100644 --- a/src/modules/tasks/tasks.service.ts +++ b/src/modules/tasks/tasks.service.ts @@ -16,6 +16,8 @@ import { CURRENT_USER, } from '../../context/request-context.module'; import type { Task } from '../../database/schema'; +import { MembershipsRepository } from '../memberships/memberships.repository'; +import { ProjectsRepository } from '../projects/projects.repository'; import { OUTBOX_TOPIC_TASK_ASSIGNED, OUTBOX_TOPIC_TASK_COMPLETED, @@ -40,6 +42,9 @@ export interface AssignTaskArgs { export class TasksService { constructor( @Inject(TasksRepository) private readonly repo: TasksRepository, + @Inject(ProjectsRepository) private readonly projects: ProjectsRepository, + @Inject(MembershipsRepository) + private readonly memberships: MembershipsRepository, @Inject(CURRENT_USER) private readonly currentUser: CurrentUserContext | null, @Inject(CURRENT_ORGANIZATION) @@ -60,6 +65,13 @@ export class TasksService { createTask(input: CreateTaskArgs): Promise { const org = this.requireOrg(); const user = this.requireUser(); + // `projectId` is caller input and the tasks table carries its own org id, so + // without this the task would attach to a project of ANOTHER tenant. The + // error is identical whether the project belongs to someone else or does not + // exist at all — a distinct message would be a cross-tenant existence oracle. + if (!this.projects.findByIdInOrg(input.projectId, org.id)) { + throw new NotFoundException(`Project ${input.projectId} not found`); + } const task = this.repo.create({ orgId: org.id, projectId: input.projectId, @@ -87,6 +99,14 @@ export class TasksService { assignTask(input: AssignTaskArgs): Promise { const org = this.requireOrg(); const user = this.requireUser(); + // The task is org-scoped by the repository, but the assignee is not: only a + // member of THIS org may hold work here. Same-message rule as createTask — + // "no such user" and "member of another org" are indistinguishable. + if (!this.memberships.findByOrgAndUser(org.id, input.assigneeId)) { + throw new NotFoundException( + `User ${input.assigneeId} is not a member of this organization`, + ); + } const task = this.repo.assign(org.id, input.id, input.assigneeId); if (!task) throw new NotFoundException(`Task ${input.id} not found`); From d0df0f520fdba54d71cab3bd4b7e2bc441c98a65 Mon Sep 17 00:00:00 2001 From: rodrigobnogueira Date: Thu, 13 Aug 2026 17:23:09 -0300 Subject: [PATCH 02/12] feat(auth): authorize mutations with a database-backed RolesGuard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Membership roles existed but authorized nothing: any authenticated caller could run users.invite — including minting another admin. @Roles(...) declares the roles a procedure accepts and RolesGuard, composed after AuthGuard the ordinary Nest way (@UseGuards(AuthGuard, RolesGuard)), resolves the caller's membership in the ACTIVE organization from the database at request time. A missing membership (revoked) or a disallowed role is a ForbiddenException, so revocation lands on the next mutation instead of at token expiry. The policy stays small: users.invite is admin only, tasks create/assign/complete and projects.create accept admin or member, and reads declare no roles so they stay token-trusted. --- src/auth/auth-context.ts | 21 +++++++++ src/auth/auth.guard.ts | 18 +------- src/auth/auth.module.ts | 13 +++++- src/auth/roles.decorator.ts | 12 ++++++ src/auth/roles.guard.ts | 57 +++++++++++++++++++++++++ src/modules/projects/projects.module.ts | 4 +- src/modules/projects/projects.router.ts | 5 ++- src/modules/tasks/tasks.router.ts | 10 ++++- src/modules/users/users.module.ts | 4 +- src/modules/users/users.router.ts | 7 ++- 10 files changed, 128 insertions(+), 23 deletions(-) create mode 100644 src/auth/roles.decorator.ts create mode 100644 src/auth/roles.guard.ts diff --git a/src/auth/auth-context.ts b/src/auth/auth-context.ts index 1c63752..a28df58 100644 --- a/src/auth/auth-context.ts +++ b/src/auth/auth-context.ts @@ -1,3 +1,5 @@ +import type { ExecutionContext } from '@nestjs/common'; + export interface CurrentUserContext { id: number; email?: string; @@ -20,3 +22,22 @@ export interface AuthenticatedRequest { socket?: { remoteAddress?: string }; authContext?: AuthContext; } + +/** + * One extractor for both transports: tRPC passes its context object as the + * second handler argument (`getArgs()[1]`), Express carries it on the request. + * Shared by AuthGuard and RolesGuard so they never disagree about the caller. + */ +export function readAuthContext( + context: ExecutionContext, +): AuthContext | undefined { + const trpcCtx = context.getArgs()[1] as + | { authContext?: AuthContext } + | undefined; + if (trpcCtx?.authContext) return trpcCtx.authContext; + + const req = context.switchToHttp().getRequest< + AuthenticatedRequest | undefined + >(); + return req?.authContext; +} diff --git a/src/auth/auth.guard.ts b/src/auth/auth.guard.ts index 5dd63b4..d84c677 100644 --- a/src/auth/auth.guard.ts +++ b/src/auth/auth.guard.ts @@ -4,28 +4,14 @@ import { Injectable, UnauthorizedException, } from '@nestjs/common'; -import type { AuthContext, AuthenticatedRequest } from './auth-context'; +import { readAuthContext } from './auth-context'; @Injectable() export class AuthGuard implements CanActivate { canActivate(context: ExecutionContext): boolean { - if (!this.extractAuthContext(context)?.user) { + if (!readAuthContext(context)?.user) { throw new UnauthorizedException(); } return true; } - - private extractAuthContext( - context: ExecutionContext, - ): AuthContext | undefined { - const trpcCtx = context.getArgs()[1] as - | { authContext?: AuthContext } - | undefined; - if (trpcCtx?.authContext) return trpcCtx.authContext; - - const req = context.switchToHttp().getRequest< - AuthenticatedRequest | undefined - >(); - return req?.authContext; - } } diff --git a/src/auth/auth.module.ts b/src/auth/auth.module.ts index 462e9b5..ee26871 100644 --- a/src/auth/auth.module.ts +++ b/src/auth/auth.module.ts @@ -3,17 +3,25 @@ import { Module, type NestModule, } from '@nestjs/common'; +import { DrizzleModule } from '@nest-native/drizzle'; import { loadEnv } from '../config/env'; import { DatabaseModule } from '../database/database.module'; +import { MembershipsRepository } from '../modules/memberships/memberships.repository'; import { AUTH_CONFIG, type AuthConfig } from './auth.config'; import { AuthGuard } from './auth.guard'; import { AuthMiddleware } from './auth.middleware'; import { AuthRouter } from './auth.router'; import { AuthService } from './auth.service'; import { AppLockoutModule } from './lockout.setup'; +import { RolesGuard } from './roles.guard'; @Module({ - imports: [DatabaseModule, AppLockoutModule], + imports: [ + DatabaseModule, + AppLockoutModule, + // RolesGuard re-reads the caller's membership on every guarded mutation. + DrizzleModule.forFeature([MembershipsRepository]), + ], providers: [ { provide: AUTH_CONFIG, @@ -24,9 +32,10 @@ import { AppLockoutModule } from './lockout.setup'; }, AuthService, AuthGuard, + RolesGuard, AuthRouter, ], - exports: [AuthService, AuthGuard], + exports: [AuthService, AuthGuard, RolesGuard], }) export class AuthModule implements NestModule { configure(consumer: MiddlewareConsumer): void { diff --git a/src/auth/roles.decorator.ts b/src/auth/roles.decorator.ts new file mode 100644 index 0000000..2048b51 --- /dev/null +++ b/src/auth/roles.decorator.ts @@ -0,0 +1,12 @@ +import { SetMetadata } from '@nestjs/common'; +import type { MembershipRole } from '../database/schema'; + +export const ROLES_METADATA = 'reference-app:roles'; + +/** + * Declares which membership roles may run a procedure. Read by `RolesGuard`, + * which resolves the caller's CURRENT role from the database — the token only + * says which organization is active, never what the caller may do in it. + */ +export const Roles = (...roles: MembershipRole[]) => + SetMetadata(ROLES_METADATA, roles); diff --git a/src/auth/roles.guard.ts b/src/auth/roles.guard.ts new file mode 100644 index 0000000..a371679 --- /dev/null +++ b/src/auth/roles.guard.ts @@ -0,0 +1,57 @@ +import { + type CanActivate, + type ExecutionContext, + ForbiddenException, + Inject, + Injectable, +} from '@nestjs/common'; +import { Reflector } from '@nestjs/core'; +import type { MembershipRole } from '../database/schema'; +import { MembershipsRepository } from '../modules/memberships/memberships.repository'; +import { readAuthContext } from './auth-context'; +import { ROLES_METADATA } from './roles.decorator'; + +/** + * Authorization for mutations, composed AFTER AuthGuard: authentication proves + * WHO is calling, this proves WHAT they may do. The JWT carries the active + * organization but no role — the role is re-read from the database on every + * guarded request, so revoking a membership takes effect on the next mutation + * instead of at token expiry. Procedures without @Roles are untouched (reads + * stay token-trusted). + */ +@Injectable() +export class RolesGuard implements CanActivate { + constructor( + @Inject(Reflector) private readonly reflector: Reflector, + @Inject(MembershipsRepository) + private readonly memberships: MembershipsRepository, + ) {} + + canActivate(context: ExecutionContext): boolean { + const allowed = this.reflector.getAllAndOverride( + ROLES_METADATA, + [context.getHandler(), context.getClass()], + ); + if (!allowed?.length) return true; + + const auth = readAuthContext(context); + if (!auth?.organization) { + throw new ForbiddenException('No active organization for this session'); + } + const membership = this.memberships.findByOrgAndUser( + auth.organization.id, + auth.user.id, + ); + if (!membership) { + throw new ForbiddenException( + 'You are no longer a member of this organization', + ); + } + if (!allowed.includes(membership.role)) { + throw new ForbiddenException( + `Requires role ${allowed.join(' or ')}; you are ${membership.role}`, + ); + } + return true; + } +} diff --git a/src/modules/projects/projects.module.ts b/src/modules/projects/projects.module.ts index a9a3ad7..48ad3d4 100644 --- a/src/modules/projects/projects.module.ts +++ b/src/modules/projects/projects.module.ts @@ -2,13 +2,15 @@ import { Module } from '@nestjs/common'; import { DrizzleModule } from '@nest-native/drizzle'; import { AuthModule } from '../../auth/auth.module'; import { RequestContextModule } from '../../context/request-context.module'; +import { MembershipsRepository } from '../memberships/memberships.repository'; import { ProjectsRepository } from './projects.repository'; import { ProjectsRouter } from './projects.router'; import { ProjectsService } from './projects.service'; @Module({ imports: [ - DrizzleModule.forFeature([ProjectsRepository]), + // MembershipsRepository is here for RolesGuard on projects.create. + DrizzleModule.forFeature([ProjectsRepository, MembershipsRepository]), AuthModule, RequestContextModule, ], diff --git a/src/modules/projects/projects.router.ts b/src/modules/projects/projects.router.ts index a94666e..ba8df9b 100644 --- a/src/modules/projects/projects.router.ts +++ b/src/modules/projects/projects.router.ts @@ -3,6 +3,8 @@ import { Input, Mutation, Query, Router } from '@nest-native/trpc'; import { CacheService } from '@nest-native/cache'; import { z } from 'zod'; import { AuthGuard } from '../../auth/auth.guard'; +import { Roles } from '../../auth/roles.decorator'; +import { RolesGuard } from '../../auth/roles.guard'; import type { CurrentOrganizationContext } from '../../auth/auth-context'; import { CURRENT_ORGANIZATION } from '../../context/request-context.module'; import { ProjectsService } from './projects.service'; @@ -30,7 +32,7 @@ const GetProjectInputSchema = z.object({ * precisely. The TTL is only the backstop; the tags do the real work. */ @Router('projects') -@UseGuards(AuthGuard) +@UseGuards(AuthGuard, RolesGuard) export class ProjectsRouter { constructor( @Inject(ProjectsService) private readonly service: ProjectsService, @@ -65,6 +67,7 @@ export class ProjectsRouter { ); } + @Roles('admin', 'member') @Mutation({ input: CreateProjectInputSchema, output: ProjectSchema }) async create(@Input() input: z.infer) { const project = this.service.create(input); diff --git a/src/modules/tasks/tasks.router.ts b/src/modules/tasks/tasks.router.ts index 1ce8863..5aacbd1 100644 --- a/src/modules/tasks/tasks.router.ts +++ b/src/modules/tasks/tasks.router.ts @@ -2,6 +2,8 @@ import { Inject, UseGuards } from '@nestjs/common'; import { Input, Mutation, Query, Router } from '@nest-native/trpc'; import { z } from 'zod'; import { AuthGuard } from '../../auth/auth.guard'; +import { Roles } from '../../auth/roles.decorator'; +import { RolesGuard } from '../../auth/roles.guard'; import { TasksService } from './tasks.service'; const TaskSchema = z.object({ @@ -33,8 +35,11 @@ const ListTasksInputSchema = z.object({ projectId: z.number().int().positive(), }); +// Guards compose left to right: AuthGuard proves the caller, RolesGuard reads +// their live membership role for the procedures that declare @Roles. Reads +// declare none, so they stay token-trusted. @Router('tasks') -@UseGuards(AuthGuard) +@UseGuards(AuthGuard, RolesGuard) export class TasksRouter { constructor(@Inject(TasksService) private readonly service: TasksService) {} @@ -43,16 +48,19 @@ export class TasksRouter { return this.service.listTasks(projectId); } + @Roles('admin', 'member') @Mutation({ input: CreateTaskInputSchema, output: TaskSchema }) create(@Input() input: z.infer) { return this.service.createTask(input); } + @Roles('admin', 'member') @Mutation({ input: AssignTaskInputSchema, output: TaskSchema }) assign(@Input() input: z.infer) { return this.service.assignTask(input); } + @Roles('admin', 'member') @Mutation({ input: CompleteTaskInputSchema, output: TaskSchema }) complete(@Input('id') id: number) { return this.service.completeTask(id); diff --git a/src/modules/users/users.module.ts b/src/modules/users/users.module.ts index 7921167..e7ccf15 100644 --- a/src/modules/users/users.module.ts +++ b/src/modules/users/users.module.ts @@ -2,6 +2,7 @@ import { Module } from '@nestjs/common'; import { DrizzleModule } from '@nest-native/drizzle'; import { AuthModule } from '../../auth/auth.module'; import { RequestContextModule } from '../../context/request-context.module'; +import { MembershipsRepository } from '../memberships/memberships.repository'; import { OnboardingModule } from '../onboarding/onboarding.module'; import { UsersRepository } from './users.repository'; import { UsersRouter } from './users.router'; @@ -9,7 +10,8 @@ import { UsersService } from './users.service'; @Module({ imports: [ - DrizzleModule.forFeature([UsersRepository]), + // MembershipsRepository is here for RolesGuard on users.invite. + DrizzleModule.forFeature([UsersRepository, MembershipsRepository]), AuthModule, RequestContextModule, OnboardingModule, diff --git a/src/modules/users/users.router.ts b/src/modules/users/users.router.ts index e4ad06b..3c6fa08 100644 --- a/src/modules/users/users.router.ts +++ b/src/modules/users/users.router.ts @@ -3,6 +3,8 @@ import { Input, Mutation, Query, Router, TrpcContext } from '@nest-native/trpc'; import { z } from 'zod'; import type { AuthContext } from '../../auth/auth-context'; import { AuthGuard } from '../../auth/auth.guard'; +import { Roles } from '../../auth/roles.decorator'; +import { RolesGuard } from '../../auth/roles.guard'; import { OrganizationOnboardingService } from '../onboarding/organization-onboarding.service'; import { UsersService } from './users.service'; @@ -34,7 +36,7 @@ const InviteUserOutputSchema = z.object({ }); @Router('users') -@UseGuards(AuthGuard) +@UseGuards(AuthGuard, RolesGuard) export class UsersRouter { constructor( @Inject(UsersService) private readonly service: UsersService, @@ -52,6 +54,9 @@ export class UsersRouter { return this.service.listInCurrentOrg(); } + // Inviting a teammate — including minting another admin — is an admin-only + // act; every other procedure here is a read. + @Roles('admin') @Mutation({ input: InviteUserInputSchema, output: InviteUserOutputSchema }) async invite( @Input() input: z.infer, From 696c5378bfc8caec721839f5bce5b44850308131 Mon Sep 17 00:00:00 2001 From: rodrigobnogueira Date: Thu, 13 Aug 2026 17:23:19 -0300 Subject: [PATCH 03/12] fix(auth): pick the active organization deterministically at login The membership lookup had no ordering, so a user with more than one membership could land in a different tenant from one login to the next (whatever SQLite returned first). The oldest membership now wins, ordered by created_at with the id as the tiebreak. --- src/auth/auth.service.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/auth/auth.service.ts b/src/auth/auth.service.ts index 07f1c3e..7204643 100644 --- a/src/auth/auth.service.ts +++ b/src/auth/auth.service.ts @@ -5,7 +5,7 @@ import { Injectable, UnauthorizedException, } from '@nestjs/common'; -import { eq } from 'drizzle-orm'; +import { asc, eq } from 'drizzle-orm'; import { InjectDrizzle } from '@nest-native/drizzle'; import { LockoutService } from '@nest-native/lockout'; import type { AppDatabase } from '../database/database'; @@ -72,10 +72,14 @@ export class AuthService { } await this.lockout.reportSuccess(identity); + // The active organization is deterministic: the OLDEST membership wins + // (createdAt is ISO text, so it sorts lexicographically; id breaks ties). const membership = this.db .select() .from(memberships) .where(eq(memberships.userId, user.id)) + .orderBy(asc(memberships.createdAt), asc(memberships.id)) + .limit(1) .get(); const orgId = membership?.orgId ?? null; From 89834a4ca0eb7164f6036ae1be1c9e9ae6d6d4f9 Mon Sep 17 00:00:00 2001 From: rodrigobnogueira Date: Thu, 13 Aug 2026 17:23:19 -0300 Subject: [PATCH 04/12] fix(config): fail fast on an invalid AUTH_TTL_SECONDS The TTL was a raw Number.parseInt: a non-numeric value became NaN and signed tokens with exp: NaN (never verifiable), while zero or a negative value minted tokens that expire on arrival. It now goes through readIntFromEnv like every other positive-integer knob, so loadEnv() throws at boot. --- src/config/env.ts | 4 +++- test/integration/env.spec.ts | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/config/env.ts b/src/config/env.ts index 6c740d3..2934e29 100644 --- a/src/config/env.ts +++ b/src/config/env.ts @@ -132,7 +132,9 @@ export function loadEnv(): AppEnv { databaseUrl: readDatabaseUrl(), trpcPath: process.env.TRPC_PATH ?? '/trpc', authSecret: readAuthSecret(nodeEnv), - authTtlSeconds: Number.parseInt(process.env.AUTH_TTL_SECONDS ?? '3600', 10), + // Fail fast: a NaN TTL would sign tokens with `exp: NaN` (never valid) and + // a zero/negative one would mint tokens that are already expired. + authTtlSeconds: readIntFromEnv('AUTH_TTL_SECONDS', 3600), lockoutLimit: readIntFromEnv('LOCKOUT_LIMIT', 5), lockoutCooloffMs: readIntFromEnv('LOCKOUT_COOLOFF_MS', 15 * 60_000), cacheTtlMs: readIntFromEnv('CACHE_TTL_MS', 30_000), diff --git a/test/integration/env.spec.ts b/test/integration/env.spec.ts index 06b6dd8..148c896 100644 --- a/test/integration/env.spec.ts +++ b/test/integration/env.spec.ts @@ -11,6 +11,7 @@ import { loadEnv } from '../../src/config/env'; const KEYS = [ 'OUTBOX_POLL_MS', 'TASK_REMINDER_DELAY_MS', + 'AUTH_TTL_SECONDS', 'PORT', 'AUTH_SECRET', 'NODE_ENV', @@ -69,6 +70,23 @@ describe('loadEnv parsing', () => { assert.throws(() => loadEnv(), /Invalid TASK_REMINDER_DELAY_MS/); }); + test('AUTH_TTL_SECONDS: defaults to an hour, parses a valid TTL, rejects NaN, zero and negatives', () => { + delete process.env.AUTH_TTL_SECONDS; + assert.equal(loadEnv().authTtlSeconds, 3_600); + + process.env.AUTH_TTL_SECONDS = '900'; + assert.equal(loadEnv().authTtlSeconds, 900); + + // A raw parseInt used to let these through: NaN silently produced tokens + // with `exp: NaN` (never valid), and 0/negative ones expire on arrival. + process.env.AUTH_TTL_SECONDS = 'one-hour'; + assert.throws(() => loadEnv(), /Invalid AUTH_TTL_SECONDS/); + process.env.AUTH_TTL_SECONDS = '0'; + assert.throws(() => loadEnv(), /Invalid AUTH_TTL_SECONDS/); + process.env.AUTH_TTL_SECONDS = '-60'; + assert.throws(() => loadEnv(), /Invalid AUTH_TTL_SECONDS/); + }); + test('readPort: defaults to 3000, parses a valid port, rejects NaN and out-of-range', () => { delete process.env.PORT; assert.equal(loadEnv().port, 3000); From f93cd9b75bdb1f5b57071a916d69026f2eac567a Mon Sep 17 00:00:00 2001 From: rodrigobnogueira Date: Thu, 13 Aug 2026 17:23:19 -0300 Subject: [PATCH 05/12] chore(worker): warn when cross-process cache invalidation is off MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The worker writes read-model rows the API process caches. With CACHE_SOCKET_PATH unset its tag invalidations never leave the worker, so API reads can stay stale until CACHE_TTL_MS lapses — a misdeploy that is silent today. One startup warning, no behaviour change. --- scripts/start-worker.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/scripts/start-worker.ts b/scripts/start-worker.ts index 665633e..b488449 100644 --- a/scripts/start-worker.ts +++ b/scripts/start-worker.ts @@ -37,6 +37,15 @@ async function main(): Promise { `worker started (outbox + jobs): db=${env.databaseUrl} poll=${env.outbox.pollIntervalMs}ms batch=${env.outbox.batchSize} stuck=${env.outbox.stuckTimeoutMs}ms`, ); + // The worker writes read-model rows (activity feed) the API process caches. + // Without the shared bus its tag invalidations never leave this process, so + // the API keeps serving stale reads until each entry's TTL lapses. + if (!env.cacheSocketPath) { + logger.warn( + `CACHE_SOCKET_PATH is unset: cross-process cache invalidation is OFF, so API reads can stay stale for up to CACHE_TTL_MS (${env.cacheTtlMs}ms). Set CACHE_SOCKET_PATH to the same socket path in both processes.`, + ); + } + const reportTick = ( loop: string, report: { claimed: number; completed: number; retried: number; failed: number }, From 1fd9d67f538b4aa6a874ba62d52df619481e59fe Mon Sep 17 00:00:00 2001 From: rodrigobnogueira Date: Thu, 13 Aug 2026 17:23:30 -0300 Subject: [PATCH 06/12] test: cover cross-tenant task links, the role policy, and login determinism tenant-authz.spec.ts runs two organizations in one database: a cross-org projectId and a cross-org assignee must fail exactly like a missing one and commit nothing (no task row, no outbox event), and a user with two memberships must get the oldest one in the token on every login. roles-authz.spec.ts drives the real tRPC stack so the guard COMPOSITION is under test: a viewer cannot create/assign/complete tasks or create projects, a member cannot invite, an admin can invite another admin, and deleting a membership blocks that user's next mutation while their reads still pass. Both were verified by hand-mutation: dropping the checks, the ordering, or RolesGuard from the routers fails them. --- test/e2e/roles-authz.spec.ts | 201 ++++++++++++++++++++++ test/integration/tenant-authz.spec.ts | 233 ++++++++++++++++++++++++++ 2 files changed, 434 insertions(+) create mode 100644 test/e2e/roles-authz.spec.ts create mode 100644 test/integration/tenant-authz.spec.ts diff --git a/test/e2e/roles-authz.spec.ts b/test/e2e/roles-authz.spec.ts new file mode 100644 index 0000000..3ef4d7a --- /dev/null +++ b/test/e2e/roles-authz.spec.ts @@ -0,0 +1,201 @@ +import 'reflect-metadata'; +import { strict as assert } from 'node:assert'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { after, before, test } from 'node:test'; +import type { INestApplication } from '@nestjs/common'; +import { NestFactory } from '@nestjs/core'; +import { and, eq } from 'drizzle-orm'; +import { getDrizzleClientToken } from '@nest-native/drizzle'; +import superjson from 'superjson'; +import type { SuperJSONResult } from 'superjson'; +import type { AppDatabase } from '../../src/database/database'; +import { memberships } from '../../src/database/schema'; +import { seedDatabase } from '../../scripts/seed'; + +// RBAC over the wire: the roles the invite flow hands out must actually decide +// what a caller may do. Everything here goes through the real tRPC stack so the +// guard COMPOSITION is under test, not just the guard class. +const trpcPath = '/trpc'; +let app: INestApplication; +let baseUrl: string; +let inspect: AppDatabase; +let adminToken: string; +let memberToken: string; +let viewerToken: string; +let orgId: number; +let memberUserId: number; +let projectId: number; +let taskId: number; + +interface TrpcSuccess { result: { data: SuperJSONResult } } +interface TrpcError { error: SuperJSONResult } +interface TrpcErrorShape { data: { httpStatus: number } } + +async function post(path: string, body: unknown, token?: string) { + return fetch(`${baseUrl}${trpcPath}/${path}`, { + method: 'POST', + headers: { + 'content-type': 'application/json', + ...(token ? { authorization: `Bearer ${token}` } : {}), + }, + body: JSON.stringify(superjson.serialize(body)), + }); +} + +async function mutate(path: string, body: unknown, token: string): Promise { + const r = await post(path, body, token); + assert.equal(r.status, 200, `POST ${path} expected 200`); + const parsed = (await r.json()) as TrpcSuccess; + return superjson.deserialize(parsed.result.data); +} + +/** The tRPC-mapped HTTP status of a rejected mutation. */ +async function denied(path: string, body: unknown, token: string): Promise { + const r = await post(path, body, token); + const parsed = (await r.json()) as TrpcError; + return superjson.deserialize(parsed.error).data.httpStatus; +} + +async function login(email: string, password: string): Promise { + const result = await post('auth.login', { email, password }); + const parsed = (await result.json()) as TrpcSuccess; + return superjson.deserialize<{ token: string }>(parsed.result.data).token; +} + +async function readStatus(path: string, token: string): Promise { + const r = await fetch(`${baseUrl}${trpcPath}/${path}`, { + headers: { authorization: `Bearer ${token}` }, + }); + return r.status; +} + +before(async () => { + const dbPath = join( + tmpdir(), + `nest-native-reference-app-e2e-roles-${process.pid}-${Date.now()}.db`, + ); + process.env.DATABASE_URL = dbPath; + process.env.TRPC_PATH = trpcPath; + process.env.AUTH_SECRET = 'e2e-roles-secret-must-be-at-least-32-chars-x'; + const seeded = seedDatabase(dbPath); + orgId = seeded.org.id; + projectId = seeded.project.id; + + const { AppModule } = await import('../../src/app.module'); + app = await NestFactory.create(AppModule, { logger: false }); + await app.listen(0, '127.0.0.1'); + baseUrl = await app.getUrl(); + inspect = app.get(getDrizzleClientToken()); + + adminToken = await login('admin@acme.test', 'admin123!'); + + const invitedMember = await mutate<{ user: { id: number } }>( + 'users.invite', + { + email: 'member@acme.test', + projectName: 'Member Project', + initialPassword: 'member-pass-1', + role: 'member', + }, + adminToken, + ); + memberUserId = invitedMember.user.id; + await mutate( + 'users.invite', + { + email: 'viewer@acme.test', + projectName: 'Viewer Project', + initialPassword: 'viewer-pass-1', + role: 'viewer', + }, + adminToken, + ); + + memberToken = await login('member@acme.test', 'member-pass-1'); + viewerToken = await login('viewer@acme.test', 'viewer-pass-1'); + + const task = await mutate<{ id: number }>( + 'tasks.create', + { projectId, title: 'Work a viewer may only read' }, + adminToken, + ); + taskId = task.id; +}); + +after(async () => { + await app.close(); +}); + +test('a viewer may read but not create, assign, complete, or open a project', async () => { + assert.equal(await readStatus('projects.list', viewerToken), 200); + + assert.equal( + await denied('tasks.create', { projectId, title: 'Viewer task' }, viewerToken), + 403, + ); + assert.equal( + await denied('tasks.assign', { id: taskId, assigneeId: memberUserId }, viewerToken), + 403, + ); + assert.equal(await denied('tasks.complete', { id: taskId }, viewerToken), 403); + assert.equal( + await denied('projects.create', { name: 'Viewer Project 2' }, viewerToken), + 403, + ); +}); + +test('a member works tasks but cannot invite teammates', async () => { + const task = await mutate<{ id: number; status: string }>( + 'tasks.create', + { projectId, title: 'Member task' }, + memberToken, + ); + assert.equal(task.status, 'open'); + + assert.equal( + await denied( + 'users.invite', + { + email: 'smuggled@acme.test', + projectName: 'Smuggled Project', + initialPassword: 'smuggled-pass-1', + role: 'admin', + }, + memberToken, + ), + 403, + ); +}); + +test('an admin may invite, including minting another admin', async () => { + const result = await mutate<{ membership: { role: string } }>( + 'users.invite', + { + email: 'second.admin@acme.test', + projectName: 'Second Admin Project', + initialPassword: 'second-admin-1', + role: 'admin', + }, + adminToken, + ); + assert.equal(result.membership.role, 'admin'); +}); + +test('revoking a membership blocks the next mutation on the already-issued token', async () => { + // The JWT still says "org N" — RolesGuard re-reads the membership, so the + // revocation lands on the next mutation instead of at token expiry. + inspect + .delete(memberships) + .where( + and(eq(memberships.orgId, orgId), eq(memberships.userId, memberUserId)), + ) + .run(); + + assert.equal( + await denied('tasks.create', { projectId, title: 'After revocation' }, memberToken), + 403, + ); + // Reads stay token-trusted until the token expires — the documented model. + assert.equal(await readStatus('projects.list', memberToken), 200); +}); diff --git a/test/integration/tenant-authz.spec.ts b/test/integration/tenant-authz.spec.ts new file mode 100644 index 0000000..3db7904 --- /dev/null +++ b/test/integration/tenant-authz.spec.ts @@ -0,0 +1,233 @@ +import 'reflect-metadata'; +import { strict as assert } from 'node:assert'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { after, before, test } from 'node:test'; +import type { INestApplicationContext } from '@nestjs/common'; +import { ContextIdFactory, NestFactory } from '@nestjs/core'; +import { eq } from 'drizzle-orm'; +import { getDrizzleClientToken } from '@nest-native/drizzle'; +import { hashPassword } from '../../src/auth/password'; +import { AuthService } from '../../src/auth/auth.service'; +import type { AppDatabase } from '../../src/database/database'; +import { + memberships, + organizations, + outboxEvents, + projects, + tasks, + users, +} from '../../src/database/schema'; +import { TasksService } from '../../src/modules/tasks/tasks.service'; +import { seedDatabase } from '../../scripts/seed'; + +// Two tenants in one database. Everything here is run as the ACME admin and +// aims at RIVAL rows: the app must refuse to link them, and must refuse in a +// way that never reveals whether the foreign row exists. +const dbPath = join( + tmpdir(), + `nest-native-reference-app-tenant-authz-${process.pid}-${Date.now()}.db`, +); + +const MISSING_ID = 999_999; + +let app: INestApplicationContext; +let tasksService: TasksService; +let auth: AuthService; +let inspect: AppDatabase; +let acmeOrgId: number; +let acmeAdminId: number; +let acmeProjectId: number; +let rivalProjectId: number; +let rivalUserId: number; + +const counts = () => ({ + tasks: inspect.select().from(tasks).all().length, + outboxEvents: inspect.select().from(outboxEvents).all().length, +}); + +/** The id is the only part that may differ between the two error messages. */ +const shape = (message: string, id: number) => + message.replace(String(id), ''); + +before(async () => { + process.env.DATABASE_URL = dbPath; + process.env.AUTH_SECRET = 'tenant-authz-secret-at-least-32-chars-xxxxx'; + const seeded = seedDatabase(dbPath); + acmeOrgId = seeded.org.id; + acmeAdminId = seeded.admin.id; + acmeProjectId = seeded.project.id; + + const { AppModule } = await import('../../src/app.module'); + app = await NestFactory.createApplicationContext(AppModule, { + logger: false, + abortOnError: false, + }); + auth = app.get(AuthService); + inspect = app.get(getDrizzleClientToken()); + + const nowIso = new Date().toISOString(); + const rivalOrg = inspect + .insert(organizations) + .values({ slug: 'rival', name: 'Rival Inc', createdAt: nowIso }) + .returning() + .get(); + const rivalUser = inspect + .insert(users) + .values({ + email: 'boss@rival.test', + passwordHash: hashPassword('rival-pass-12345'), + createdAt: nowIso, + }) + .returning() + .get(); + rivalUserId = rivalUser.id; + inspect + .insert(memberships) + .values({ + orgId: rivalOrg.id, + userId: rivalUser.id, + role: 'admin', + createdAt: nowIso, + }) + .run(); + rivalProjectId = inspect + .insert(projects) + .values({ + orgId: rivalOrg.id, + name: 'Rival Roadmap', + createdBy: rivalUser.id, + createdAt: nowIso, + }) + .returning() + .get().id; + + // Same trick as tasks.workflow.spec: resolve the request-scoped service + // against a registered request carrying the ACME tenant. + const contextId = ContextIdFactory.create(); + app.registerRequestByContextId( + { + authContext: { + user: { id: acmeAdminId }, + organization: { id: acmeOrgId }, + }, + }, + contextId, + ); + tasksService = await app.resolve(TasksService, contextId); +}); + +after(async () => { + await app.close(); +}); + +test('createTask refuses a cross-org projectId exactly like a missing one, committing nothing', async () => { + const before = counts(); + + const crossOrgError = await tasksService + .createTask({ projectId: rivalProjectId, title: 'Steal the roadmap' }) + .then(() => undefined) + .catch((error: Error) => error); + const missingError = await tasksService + .createTask({ projectId: MISSING_ID, title: 'Steal nothing' }) + .then(() => undefined) + .catch((error: Error) => error); + + assert.ok(crossOrgError instanceof Error); + assert.ok(missingError instanceof Error); + assert.equal(crossOrgError.message, `Project ${rivalProjectId} not found`); + assert.equal( + shape(crossOrgError.message, rivalProjectId), + shape(missingError.message, MISSING_ID), + 'a foreign project must be indistinguishable from a nonexistent one', + ); + + // The transaction rolled back before any write: no task row, no outbox event. + assert.deepEqual(counts(), before); +}); + +test('assignTask refuses a cross-org assignee exactly like a missing one, committing nothing', async () => { + const task = await tasksService.createTask({ + projectId: acmeProjectId, + title: 'Assignable work', + }); + const before = counts(); + + const crossOrgError = await tasksService + .assignTask({ id: task.id, assigneeId: rivalUserId }) + .then(() => undefined) + .catch((error: Error) => error); + const missingError = await tasksService + .assignTask({ id: task.id, assigneeId: MISSING_ID }) + .then(() => undefined) + .catch((error: Error) => error); + + assert.ok(crossOrgError instanceof Error); + assert.ok(missingError instanceof Error); + assert.equal( + crossOrgError.message, + `User ${rivalUserId} is not a member of this organization`, + ); + assert.equal( + shape(crossOrgError.message, rivalUserId), + shape(missingError.message, MISSING_ID), + 'a foreign member must be indistinguishable from a nonexistent user', + ); + + // No assignment was written and no task.assigned event was enqueued. + assert.deepEqual(counts(), before); + const row = inspect.select().from(tasks).where(eq(tasks.id, task.id)).get(); + assert.equal(row?.assigneeId, null); + assert.equal(row?.status, 'open'); +}); + +test('login puts the OLDEST membership in the token, stably across logins', async () => { + const dualUser = inspect + .insert(users) + .values({ + email: 'dual@acme.test', + passwordHash: hashPassword('dual-pass-12345'), + createdAt: new Date().toISOString(), + }) + .returning() + .get(); + const otherOrgId = inspect + .select() + .from(organizations) + .where(eq(organizations.slug, 'rival')) + .get()?.id; + assert.ok(otherOrgId); + + // Insert the NEWER membership first, so row order (id) and createdAt order + // disagree — only an explicit ordering can pick the same one twice. + inspect + .insert(memberships) + .values({ + orgId: otherOrgId, + userId: dualUser.id, + role: 'member', + createdAt: '2026-02-01T00:00:00.000Z', + }) + .run(); + inspect + .insert(memberships) + .values({ + orgId: acmeOrgId, + userId: dualUser.id, + role: 'member', + createdAt: '2026-01-01T00:00:00.000Z', + }) + .run(); + + const first = await auth.login( + { email: 'dual@acme.test', password: 'dual-pass-12345' }, + '127.0.0.1', + ); + const second = await auth.login( + { email: 'dual@acme.test', password: 'dual-pass-12345' }, + '127.0.0.1', + ); + + assert.equal(first.organization?.id, acmeOrgId); + assert.deepEqual(second.organization, first.organization); +}); From 27c273f5d0075b6a63cf4a68edad6b4eb90b2325 Mon Sep 17 00:00:00 2001 From: rodrigobnogueira Date: Thu, 13 Aug 2026 17:23:30 -0300 Subject: [PATCH 07/12] docs: describe the token model, the role policy, and the synchronous hash The README gains an auth/tenancy/roles section (the JWT snapshots one active organization; mutations re-check live membership, reads stay token-trusted until the TTL) plus an honest callout that password hashing is scryptSync and blocks the event loop under concurrent logins. The architecture tour gains an Authorization chapter next to Authentication and the matching lifecycle, layout, and test-table rows. --- README.md | 34 +++++++++++++++++++++--- docs/architecture.md | 63 +++++++++++++++++++++++++++++++++++++++----- 2 files changed, 88 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index cd2441d..09a95db 100644 --- a/README.md +++ b/README.md @@ -27,7 +27,7 @@ Follow one journey through the code and every library shows up where a real syst | Library | Its job in the story | Where in the code | | --- | --- | --- | | [`@nest-native/drizzle`](https://github.com/nest-native/drizzle) | **Persistence** — orgs, users, projects, tasks, activity; repositories, transactions, multi-tenant scoping | `src/database/`, every `*.repository.ts` (`@DrizzleRepository`, `@InjectTransaction`) | -| [`@nest-native/trpc`](https://github.com/nest-native/trpc) | **The typed API** — task CRUD, project queries, the activity feed, all typesafe end-to-end; the superjson transformer keeps the feed's `Date`s real across the wire (the client link is *required* to match, at compile time), and failed validations reach the client as flattened Zod field errors (`error.data.zodError`) | `src/modules/*/**.router.ts` (`@Router`, `@Query`/`@Mutation`), `src/trpc/` (transformer, error formatting, response meta), generated `AppRouter` | +| [`@nest-native/trpc`](https://github.com/nest-native/trpc) | **The typed API** — task CRUD, project queries, the activity feed, all typesafe end-to-end; the superjson transformer keeps the feed's `Date`s real across the wire (the client link is *required* to match, at compile time), and failed validations reach the client as flattened Zod field errors (`error.data.zodError`); Nest enhancers compose over procedures the usual way — `@UseGuards(AuthGuard, RolesGuard)` plus per-procedure `@Roles(...)` metadata | `src/modules/*/**.router.ts` (`@Router`, `@Query`/`@Mutation`), `src/trpc/` (transformer, error formatting, response meta), generated `AppRouter` | | [`@nest-native/messaging`](https://github.com/nest-native/messaging) | **Reliable domain events** — the transactional outbox (emit in-tx) + idempotent inbox (dedup on consume) | `src/modules/{outbox,inbox,activity}/`, `OutboxProducer.enqueue` inside `@Transactional()` | | [`@nest-native/kafka`](https://github.com/nest-native/kafka) | **The event backbone** — the outbox relays through `KafkaOutboxTransport`; `@KafkaConsumer`s build read-models | the Kafka profile in `src/app.module.ts`, `src/modules/inbox/*.consumer.ts` | | [`@nest-native/jobs`](https://github.com/nest-native/jobs) | **Deferred work** — the assignment reminder: enqueued in the same transaction as the `task.assigned` projection (`uniqueKey` = the event's dedup key), executed exactly once by the worker — **plus recurring work**: a DB-stored cron schedule drives the nightly stale-task sweep | `src/modules/reminders/`, `TaskAssignedProjection` in `src/modules/activity/`, `src/database/schema/jobs.ts` | @@ -45,6 +45,34 @@ Everything above runs **with no infrastructure** by default: - **In-process (default)** — the outbox relays through an in-process transport and handlers build the activity feed synchronously. SQLite in a file, no broker. This is what the tests exercise. - **Kafka** — set `KAFKA_BROKERS` and the exact same domain code relays through `KafkaOutboxTransport` to a real cluster, with `@KafkaConsumer`s on the other side. The event bodies, dedup keys, and wire headers are identical; only the transport swaps. +## Auth, tenancy, and roles + +Login mints an HS256 JWT that **snapshots one active organization** — the +caller's *oldest* membership (`created_at`, then `id` as the tiebreak, so +repeated logins always resolve the same tenant). The token carries no role. + +- **Mutations re-check the live membership.** `RolesGuard` composes after + `AuthGuard` and reads the caller's role from the database on every procedure + that declares `@Roles(...)`: `users.invite` is **admin** only; + `tasks.create` / `.assign` / `.complete` and `projects.create` accept + **admin or member**; a **viewer** reads only. Revoking a membership therefore + blocks the next mutation instead of waiting for the token to expire. +- **Reads stay token-trusted** for the token's lifetime (`AUTH_TTL_SECONDS`, + default 3600 — an invalid value now fails at boot rather than minting tokens + that never verify). That trade-off is documented, not hidden. +- **Tenancy is proven at the write.** Inside the same transaction, + `tasks.create` requires the project to belong to the caller's org and + `tasks.assign` requires the assignee to be a member of it. Both refuse with + exactly the error a nonexistent id gets, so the API is never a cross-tenant + existence oracle. + +> **Password hashing is synchronous.** `src/auth/password.ts` uses `scryptSync` +> because a short, obviously-correct helper reads better in a reference app — +> but it blocks the event loop for every hash, so concurrent logins queue behind +> each other. Production adopters should move to an async hash or a worker pool; +> that complements `@nest-native/lockout` (which caps how many attempts reach the +> hash at all) rather than replacing it. + ## Getting started Requires **Node ≥ 22** (the AI SDK requires it). @@ -93,7 +121,7 @@ src/ app.module.ts Root module, ClsPluginTransactional, in-process/Kafka messaging profiles config/env.ts loadEnv() — single source of truth (incl. the optional kafka block) database/ DrizzleModule wiring + schema (orgs/users/projects/tasks/activity/...) + migrations - auth/ scrypt passwords, HS256 JWT, AuthGuard, middleware; @nest-native/lockout login lockout (lockout.setup.ts) + auth/ scrypt passwords, HS256 JWT, AuthGuard + RolesGuard/@Roles, middleware; @nest-native/lockout login lockout (lockout.setup.ts) cache/ @nest-native/cache read caching — tag invalidation through @stalefree/core (cache.setup.ts) context/ request-scoped CURRENT_USER / CURRENT_ORGANIZATION modules/ @@ -120,7 +148,7 @@ npm run test:cov # with c8 coverage npm run ci # typecheck, lint, complexity (≤15), test:cov, security:audit, build ``` -Coverage here is **pragmatic, not 100%** — the 100% bar belongs to the libraries. The transactional workflow, the outbox worker, the inbox dedup, the reminder job's exactly-once scheduling and execution, the AsyncAPI catalog, the AI stream, and the login-lockout gate (fail N times → 429, even the right password is refused while locked), and cache coherence (mutations refresh cached reads long before TTL — tag invalidation, not expiry) all have explicit tests. CI runs on **Node 22**. +Coverage here is **pragmatic, not 100%** — the 100% bar belongs to the libraries. The transactional workflow, the outbox worker, the inbox dedup, the reminder job's exactly-once scheduling and execution, the AsyncAPI catalog, the AI stream, and the login-lockout gate (fail N times → 429, even the right password is refused while locked), and cache coherence (mutations refresh cached reads long before TTL — tag invalidation, not expiry) all have explicit tests, as do the tenancy and role checks (cross-org project/assignee refused like a missing one with nothing committed; viewer/member/admin limits and revocation over real HTTP). CI runs on **Node 22**. Two **optional, local-only** layers sit on top (neither runs in CI, and forks work without them): diff --git a/docs/architecture.md b/docs/architecture.md index 443a94d..3d4e00e 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -150,6 +150,8 @@ service deps through `@Inject(...)` in the constructor and call them. │ - or - │ │ tRPC handler │ ←── nest-trpc-native dispatch │ AuthGuard │ reads ctx.authContext via getArgs()[1] + │ RolesGuard │ re-reads the caller's membership role + │ │ (mutations only — see Authorization) │ ParamDecorators │ @Input, @TrpcContext, @CurrentUser │ Procedure body │ └──────────────────────┘ @@ -176,10 +178,14 @@ serves both transports. ## Authentication `AuthService.login(email, password)` runs `scrypt`-verify against the stored -hash, finds the first membership row for the user, and mints a real -HS256-signed JWT containing `{ sub: userId, org: orgId, iat, exp }`. The -signing key comes from `AUTH_SECRET` (min 32 chars, required in production, -deterministic dev fallback elsewhere). +hash, picks the user's **oldest** membership (ordered by `created_at`, then +`id` as the tiebreak — so repeated logins always land on the same tenant), and +mints a real HS256-signed JWT containing +`{ sub: userId, org: orgId, iat, exp }`. The signing key comes from +`AUTH_SECRET` (min 32 chars, required in production, deterministic dev fallback +elsewhere) and the lifetime from `AUTH_TTL_SECONDS` (default 3600; a NaN, zero +or negative value fails `loadEnv()` at boot instead of minting tokens that can +never verify). JWT verification uses Node's built-in `node:crypto` HMAC — there's no JWT library dependency. See @@ -190,6 +196,49 @@ covers roundtrip, tamper, expiry, wrong-secret, malformed, and unsupported-algor Password hashing is `scrypt` with a 16-byte random salt; format is `scrypt$$`. The same helpers are reused by [`scripts/seed.ts`](https://github.com/nest-native/reference-app/blob/main/scripts/seed.ts) so seeded users can log in. +It is deliberately the **synchronous** `scryptSync` — a short, obviously-correct +helper reads better here — but that blocks the event loop for the duration of +every hash, so concurrent logins queue behind each other. A production adopter +should swap in an async hash or a worker pool; that is orthogonal to (not a +replacement for) the login lockout below, which limits how many attempts reach +the hash at all. + +## Authorization (roles + tenancy) + +The token says **who** is calling and **which** organization is active. It +deliberately says nothing about what the caller may do — that is re-read from +the database on every mutation: + +``` +@Router('tasks') +@UseGuards(AuthGuard, RolesGuard) ← composed left to right +export class TasksRouter { + @Query(...) list(...) ← no @Roles: token-trusted read + @Roles('admin', 'member') + @Mutation(...) create(...) ← RolesGuard re-reads the membership row +} +``` + +[`RolesGuard`](https://github.com/nest-native/reference-app/blob/main/src/auth/roles.guard.ts) +resolves `MembershipsRepository.findByOrgAndUser(activeOrg, caller)` at request +time and throws `ForbiddenException` when the membership is missing (revoked) +or its role is not in the procedure's `@Roles(...)` list. The policy is +deliberately small — three roles, no policy engine, no per-resource ACLs: +`users.invite` is `admin` only; `tasks.create` / `.assign` / `.complete` and +`projects.create` accept `admin` or `member`; `viewer` reads only. Reads carry +no `@Roles` and stay token-trusted until the TTL lapses, so a revoked member +loses **mutations** on their next request and **reads** when their token +expires. + +Tenancy is proven at the write, not assumed from the token. Inside the same +transaction that writes the row, `TasksService.createTask` requires the +`projectId` to resolve *within the caller's org* and `assignTask` requires the +assignee to hold a membership *in that org*. Both refuse with exactly the error +a nonexistent id gets (`Project 42 not found` / +`User 42 is not a member of this organization`) — a distinct "belongs to +someone else" message would turn the API into a cross-tenant existence oracle. +See [`test/integration/tenant-authz.spec.ts`](https://github.com/nest-native/reference-app/blob/main/test/integration/tenant-authz.spec.ts) +and [`test/e2e/roles-authz.spec.ts`](https://github.com/nest-native/reference-app/blob/main/test/e2e/roles-authz.spec.ts). ## Login lockout @@ -333,13 +382,13 @@ same image (see `docker-compose.yml`). | `app.module.ts` | Root module: imports + ClsPluginTransactional wiring | | `config/env.ts` | `loadEnv()` — single source of truth for env vars | | `database/` | `DatabaseModule` (DrizzleModule.forRoot wiring), schema, migrations | -| `auth/` | JWT helpers, scrypt password helpers, `AuthService`, middleware, `AuthGuard`, `@CurrentUser`/`@CurrentOrganization` decorators, `AuthRouter` | +| `auth/` | JWT helpers, scrypt password helpers, `AuthService`, middleware, `AuthGuard` + `RolesGuard`/`@Roles`, `@CurrentUser`/`@CurrentOrganization` decorators, `AuthRouter` | | `context/` | `RequestContextModule` — Nest request-scoped `CURRENT_USER` / `CURRENT_ORGANIZATION` providers backed by `req.authContext` | | `health/` | `/health` REST controller | | `modules/organizations/` | Repo + service + tRPC router. `organizations.current` / `.list` | | `modules/users/` | Repo + service + tRPC router. `users.me` / `.list` / `.invite` | | `modules/projects/` | Repo + service + tRPC router. `projects.list` / `.get` / `.create` | -| `modules/memberships/` | Repo only (consumed by onboarding) | +| `modules/memberships/` | Repo only (consumed by onboarding, `RolesGuard`, and the task tenancy checks) | | `modules/audit-log/` | `AuditLogService.record()` | | `modules/outbox/` | Producer, claimer, registry, fake transport, `user.invited` handler, `outbox.constants.ts` | | `modules/onboarding/` | `OrganizationOnboardingService` — the `@Transactional` workflow | @@ -361,6 +410,8 @@ same image (see `docker-compose.yml`). | `test/e2e/auth-flow.spec.ts` | Login flow over real HTTP; 401 on wrong password / no token / bad token | | `test/e2e/trpc-ping.smoke.spec.ts` | `GET /trpc/ping` returns 'pong'; `/health` returns ok | | `test/e2e/core-modules.spec.ts` | Authenticated flow over real HTTP across the three core routers | +| `test/integration/tenant-authz.spec.ts` | Cross-org project/assignee are refused like missing ones, nothing committed; login picks the oldest membership | +| `test/e2e/roles-authz.spec.ts` | RBAC over real HTTP: viewer/member/admin limits, and a revoked membership blocking the next mutation | Plus `client-smoke/client.ts` (typed client over real HTTP using the generated `AppRouter`) which is run via `npm run client-smoke` rather than From 4fce790d9a50fe9c0f489ec969a7ca68cfb0d1af Mon Sep 17 00:00:00 2001 From: rodrigobnogueira Date: Thu, 13 Aug 2026 17:46:04 -0300 Subject: [PATCH 08/12] fix(auth): resolve the caller from the transport, never the procedure input MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit readAuthContext tried the tRPC context first and fell back to switchToHttp().getRequest(). That fallback is not HTTP-specific: getRequest() is getArgs()[0] whatever the transport is, and under tRPC args[0] is the caller's own input — so a procedure whose schema kept unknown keys would let a client hand AuthGuard (and now RolesGuard) an authContext of its choosing. Nothing is exploitable today: every procedure input is a z.object and zod strips unknown keys, and parser-less procedures get undefined. But an auth extractor must not rest on that, so it branches on context.getType() instead — args[1] for the 'rpc' contexts @nest-native/trpc creates, the request only for 'http'. auth-context.spec.ts pins both directions, including a forged authContext in args[0] resolving to undefined. --- src/auth/auth-context.ts | 20 +++++++--- test/integration/auth-context.spec.ts | 56 +++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 6 deletions(-) create mode 100644 test/integration/auth-context.spec.ts diff --git a/src/auth/auth-context.ts b/src/auth/auth-context.ts index a28df58..0b518f1 100644 --- a/src/auth/auth-context.ts +++ b/src/auth/auth-context.ts @@ -27,17 +27,25 @@ export interface AuthenticatedRequest { * One extractor for both transports: tRPC passes its context object as the * second handler argument (`getArgs()[1]`), Express carries it on the request. * Shared by AuthGuard and RolesGuard so they never disagree about the caller. + * + * It BRANCHES on the transport rather than trying one then the other, because + * `switchToHttp().getRequest()` is just `getArgs()[0]` whatever the transport + * is — under tRPC that argument is the caller's own INPUT, so a fallback would + * read authentication out of the request body. Zod strips unknown keys, so no + * procedure here can be forged today; a single `.passthrough()` schema is all + * it would take, and an auth extractor must not depend on that. */ export function readAuthContext( context: ExecutionContext, ): AuthContext | undefined { + if (context.getType() === 'http') { + const req = context.switchToHttp().getRequest< + AuthenticatedRequest | undefined + >(); + return req?.authContext; + } const trpcCtx = context.getArgs()[1] as | { authContext?: AuthContext } | undefined; - if (trpcCtx?.authContext) return trpcCtx.authContext; - - const req = context.switchToHttp().getRequest< - AuthenticatedRequest | undefined - >(); - return req?.authContext; + return trpcCtx?.authContext; } diff --git a/test/integration/auth-context.spec.ts b/test/integration/auth-context.spec.ts new file mode 100644 index 0000000..d72a00c --- /dev/null +++ b/test/integration/auth-context.spec.ts @@ -0,0 +1,56 @@ +import { strict as assert } from 'node:assert'; +import { test } from 'node:test'; +import type { ExecutionContext } from '@nestjs/common'; +import { + type AuthContext, + readAuthContext, +} from '../../src/auth/auth-context'; + +// Both guards read the caller through readAuthContext, so its transport +// branching is the single place an authentication source is decided. +// This stub mirrors Nest's ExecutionContextHost: switchToHttp().getRequest() +// is just getArgs()[0], whatever the transport is. +function executionContext(type: string, args: unknown[]): ExecutionContext { + return { + getType: () => type, + getArgs: () => args, + switchToHttp: () => ({ getRequest: () => args[0] }), + } as unknown as ExecutionContext; +} + +const caller: AuthContext = { + user: { id: 7 }, + organization: { id: 3 }, +}; +const forged: AuthContext = { + user: { id: 99 }, + organization: { id: 99 }, +}; + +// @nest-native/trpc dispatches guards with args = [input, trpcCtx] and +// type 'rpc' (see its trpc-context-creator). +test('tRPC: the procedure context is the authentication source', () => { + const context = executionContext('rpc', [{ projectId: 1 }, { authContext: caller }]); + assert.deepEqual(readAuthContext(context), caller); +}); + +test('tRPC: a caller-supplied input is never an authentication source', () => { + const context = executionContext('rpc', [ + { authContext: forged }, + { authContext: undefined }, + ]); + assert.equal(readAuthContext(context), undefined); +}); + +test('HTTP: the request carries the auth context', () => { + const context = executionContext('http', [ + { headers: {}, authContext: caller }, + { statusCode: 200 }, + ]); + assert.deepEqual(readAuthContext(context), caller); +}); + +test('HTTP: an unauthenticated request yields no context', () => { + const context = executionContext('http', [{ headers: {} }, { statusCode: 200 }]); + assert.equal(readAuthContext(context), undefined); +}); From fa56bff788f25af112bbd2c3e5d417e22b7f3459 Mon Sep 17 00:00:00 2001 From: rodrigobnogueira Date: Thu, 13 Aug 2026 17:46:15 -0300 Subject: [PATCH 09/12] fix(auth): re-check the live membership on reads, not just mutations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RolesGuard returned early for any procedure without @Roles, so revocation only landed on writes. A deleted membership left the account a read-only insider for up to AUTH_TTL_SECONDS (default an hour): the member roster with everyone's email and role, every project, the activity feed, and POST /projects/:id/assistant, which streams an AI digest of that feed and bills tokens for it. The guard now resolves the membership whenever the token names an active organization and refuses when it is gone; @Roles only narrows which roles may proceed. A caller with no organization at all still passes procedures that declare no roles (users.me), and auth.me keeps no RolesGuard because it just echoes the token back. The guard is added to the routers that were authenticated but unguarded — organizations, activity — and to the assistant controller. One indexed lookup per request, deliberately uncached. roles-authz.spec.ts now asserts the revoked member gets 403 from projects.list, users.list, activity.list and the assistant endpoint, and that logging in again still works. Verified by hand-mutation: restoring either the early return or the controller's old @UseGuards(AuthGuard) fails it. --- src/auth/roles.guard.ts | 28 +++++++++++----- src/modules/activity/activity.router.ts | 3 +- .../assistant/project-assistant.controller.ts | 7 +++- .../organizations/organizations.router.ts | 3 +- src/modules/tasks/tasks.router.ts | 6 ++-- test/e2e/roles-authz.spec.ts | 33 ++++++++++++++++--- 6 files changed, 61 insertions(+), 19 deletions(-) diff --git a/src/auth/roles.guard.ts b/src/auth/roles.guard.ts index a371679..08fb7c6 100644 --- a/src/auth/roles.guard.ts +++ b/src/auth/roles.guard.ts @@ -12,12 +12,20 @@ import { readAuthContext } from './auth-context'; import { ROLES_METADATA } from './roles.decorator'; /** - * Authorization for mutations, composed AFTER AuthGuard: authentication proves - * WHO is calling, this proves WHAT they may do. The JWT carries the active - * organization but no role — the role is re-read from the database on every - * guarded request, so revoking a membership takes effect on the next mutation - * instead of at token expiry. Procedures without @Roles are untouched (reads - * stay token-trusted). + * Authorization, composed AFTER AuthGuard: authentication proves WHO is + * calling, this proves WHAT they may do. The JWT snapshots the active + * organization but no role, so every guarded request re-reads the caller's + * membership from the database: + * + * - no membership in the token's organization → refused, reads included. The + * token outlives a revocation by up to AUTH_TTL_SECONDS, and a tenant's + * member roster, project list, activity feed and AI digests are exactly what + * an offboarded account should stop seeing first. + * - `@Roles(...)` narrows a procedure further to specific roles; without it, + * holding any live membership is enough. + * + * The cost is one indexed lookup per request — authorization is deliberately + * not cached, since a stale allow is the whole problem being fixed. */ @Injectable() export class RolesGuard implements CanActivate { @@ -32,10 +40,12 @@ export class RolesGuard implements CanActivate { ROLES_METADATA, [context.getHandler(), context.getClass()], ); - if (!allowed?.length) return true; - const auth = readAuthContext(context); if (!auth?.organization) { + // Nothing tenant-scoped to authorize — a user with no membership can + // still read their own profile. A procedure that names roles has no role + // to compare, so it still refuses. + if (!allowed?.length) return true; throw new ForbiddenException('No active organization for this session'); } const membership = this.memberships.findByOrgAndUser( @@ -47,7 +57,7 @@ export class RolesGuard implements CanActivate { 'You are no longer a member of this organization', ); } - if (!allowed.includes(membership.role)) { + if (allowed?.length && !allowed.includes(membership.role)) { throw new ForbiddenException( `Requires role ${allowed.join(' or ')}; you are ${membership.role}`, ); diff --git a/src/modules/activity/activity.router.ts b/src/modules/activity/activity.router.ts index bf8d17d..f36058a 100644 --- a/src/modules/activity/activity.router.ts +++ b/src/modules/activity/activity.router.ts @@ -3,6 +3,7 @@ import { Input, Query, Router } from '@nest-native/trpc'; import { CacheService } from '@nest-native/cache'; import { z } from 'zod'; import { AuthGuard } from '../../auth/auth.guard'; +import { RolesGuard } from '../../auth/roles.guard'; import type { CurrentOrganizationContext } from '../../auth/auth-context'; import { CURRENT_ORGANIZATION } from '../../context/request-context.module'; import { ActivityService } from './activity.service'; @@ -32,7 +33,7 @@ const ListActivityInputSchema = z.object({ * inject the request-scoped CURRENT_ORGANIZATION. */ @Router('activity') -@UseGuards(AuthGuard) +@UseGuards(AuthGuard, RolesGuard) export class ActivityRouter { constructor( @Inject(ActivityService) private readonly service: ActivityService, diff --git a/src/modules/assistant/project-assistant.controller.ts b/src/modules/assistant/project-assistant.controller.ts index db51318..0937a27 100644 --- a/src/modules/assistant/project-assistant.controller.ts +++ b/src/modules/assistant/project-assistant.controller.ts @@ -13,6 +13,7 @@ import { } from '@nest-native/ai-sdk'; import { streamText } from 'ai'; import { AuthGuard } from '../../auth/auth.guard'; +import { RolesGuard } from '../../auth/roles.guard'; import { ActivityService } from '../activity/activity.service'; import { ProjectsService } from '../projects/projects.service'; import { buildActivityPrompt, buildStatusSummary } from './activity-digest'; @@ -31,6 +32,10 @@ import { resolveAssistantModel } from './assistant-model'; * never mid-stream error frames. Only once tenant scoping has passed does the * response become a stream. * + * `RolesGuard` runs alongside `AuthGuard` for the same reason: this endpoint + * spends model tokens on a tenant's activity feed, so a caller whose membership + * was revoked must lose it on their next request, not at token expiry. + * * `@AiAbortSignal()` is forwarded to `streamText` so a client disconnect * mid-stream cancels the upstream model request instead of billing for tokens * written to a dead socket. @@ -44,7 +49,7 @@ export class ProjectAssistantController { @Post(':projectId/assistant') @AiStream() - @UseGuards(AuthGuard) + @UseGuards(AuthGuard, RolesGuard) async summarize( @Param('projectId', ParseIntPipe) projectId: number, @AiAbortSignal() signal: AbortSignal, diff --git a/src/modules/organizations/organizations.router.ts b/src/modules/organizations/organizations.router.ts index 8a773b2..75728e6 100644 --- a/src/modules/organizations/organizations.router.ts +++ b/src/modules/organizations/organizations.router.ts @@ -2,6 +2,7 @@ import { Inject, UseGuards } from '@nestjs/common'; import { Query, Router } from '@nest-native/trpc'; import { z } from 'zod'; import { AuthGuard } from '../../auth/auth.guard'; +import { RolesGuard } from '../../auth/roles.guard'; import { OrganizationsService } from './organizations.service'; const OrganizationSchema = z.object({ @@ -12,7 +13,7 @@ const OrganizationSchema = z.object({ }); @Router('organizations') -@UseGuards(AuthGuard) +@UseGuards(AuthGuard, RolesGuard) export class OrganizationsRouter { constructor( @Inject(OrganizationsService) diff --git a/src/modules/tasks/tasks.router.ts b/src/modules/tasks/tasks.router.ts index 5aacbd1..5c2c1d1 100644 --- a/src/modules/tasks/tasks.router.ts +++ b/src/modules/tasks/tasks.router.ts @@ -35,9 +35,9 @@ const ListTasksInputSchema = z.object({ projectId: z.number().int().positive(), }); -// Guards compose left to right: AuthGuard proves the caller, RolesGuard reads -// their live membership role for the procedures that declare @Roles. Reads -// declare none, so they stay token-trusted. +// Guards compose left to right: AuthGuard proves the caller, RolesGuard proves +// they still hold a membership in the active org — and, for the procedures that +// declare @Roles, that their live role allows the write. @Router('tasks') @UseGuards(AuthGuard, RolesGuard) export class TasksRouter { diff --git a/test/e2e/roles-authz.spec.ts b/test/e2e/roles-authz.spec.ts index 3ef4d7a..87d900d 100644 --- a/test/e2e/roles-authz.spec.ts +++ b/test/e2e/roles-authz.spec.ts @@ -63,6 +63,13 @@ async function login(email: string, password: string): Promise { return superjson.deserialize<{ token: string }>(parsed.result.data).token; } +/** superjson-encoded query input, the way the typed client sends it. */ +function activityInput(id: number): string { + return encodeURIComponent( + JSON.stringify(superjson.serialize({ projectId: id })), + ); +} + async function readStatus(path: string, token: string): Promise { const r = await fetch(`${baseUrl}${trpcPath}/${path}`, { headers: { authorization: `Bearer ${token}` }, @@ -182,9 +189,9 @@ test('an admin may invite, including minting another admin', async () => { assert.equal(result.membership.role, 'admin'); }); -test('revoking a membership blocks the next mutation on the already-issued token', async () => { +test('revoking a membership blocks the next request on the already-issued token', async () => { // The JWT still says "org N" — RolesGuard re-reads the membership, so the - // revocation lands on the next mutation instead of at token expiry. + // revocation lands on the next request instead of at token expiry. inspect .delete(memberships) .where( @@ -196,6 +203,24 @@ test('revoking a membership blocks the next mutation on the already-issued token await denied('tasks.create', { projectId, title: 'After revocation' }, memberToken), 403, ); - // Reads stay token-trusted until the token expires — the documented model. - assert.equal(await readStatus('projects.list', memberToken), 200); + // Reads go too: the roster, the projects, the feed and the AI digest are + // exactly what an offboarded account must stop seeing first. + assert.equal(await readStatus('projects.list', memberToken), 403); + assert.equal(await readStatus('users.list', memberToken), 403); + assert.equal( + await readStatus(`activity.list?input=${activityInput(projectId)}`, memberToken), + 403, + ); + const assistant = await fetch(`${baseUrl}/projects/${projectId}/assistant`, { + method: 'POST', + headers: { authorization: `Bearer ${memberToken}` }, + }); + assert.equal(assistant.status, 403); + + // Login still works — it is the token that is stale, not the account. + const relogin = await post('auth.login', { + email: 'member@acme.test', + password: 'member-pass-1', + }); + assert.equal(relogin.status, 200); }); From 4e57f175f1855bdfc3257b165df5546c7878b459 Mon Sep 17 00:00:00 2001 From: rodrigobnogueira Date: Thu, 13 Aug 2026 17:46:24 -0300 Subject: [PATCH 10/12] fix(onboarding): refuse an invite to an address that already has an account MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit users.invite upserted the invitee: an existing account was silently returned and given a membership in the caller's organization with the caller's chosen role. An org admin could therefore attach any account — including another tenant's admin — to their org without consent, and that account then satisfied every "is a member of this org" predicate, including the assignee check assignTask just gained. The invitee got no signal at all; the supplied initialPassword was quietly discarded. An invite now only ever creates a NEW account. An address that already has one is refused identically whether it is already a member here or belongs to another tenant, so the refusal never maps addresses to organizations. It does still reveal that an account exists — erasing that needs a pending-invitation row the invitee accepts, which the doc comment points at as the production shape. tenant-authz.spec.ts asserts the ACME admin cannot pull the RIVAL admin in, the two refusals are word-for-word identical, and nothing is written. --- .../organization-onboarding.service.ts | 26 +++++++-- test/integration/tenant-authz.spec.ts | 53 ++++++++++++++++++- 2 files changed, 74 insertions(+), 5 deletions(-) diff --git a/src/modules/onboarding/organization-onboarding.service.ts b/src/modules/onboarding/organization-onboarding.service.ts index 66420ef..36be894 100644 --- a/src/modules/onboarding/organization-onboarding.service.ts +++ b/src/modules/onboarding/organization-onboarding.service.ts @@ -1,4 +1,4 @@ -import { Inject, Injectable } from '@nestjs/common'; +import { ConflictException, Inject, Injectable } from '@nestjs/common'; import { InjectTransaction, Transactional } from '@nestjs-cls/transactional'; import { eq } from 'drizzle-orm'; import { OutboxProducer } from '@nest-native/messaging'; @@ -58,7 +58,7 @@ export class OrganizationOnboardingService { // signature the decorator imposes on the caller's view of the method. @Transactional() inviteUser(input: InviteUserInput): Promise { - const user = this.upsertUser(input.email, input.initialPassword); + const user = this.createInvitee(input.email, input.initialPassword); const membership = this.memberships.create({ orgId: input.orgId, userId: user.id, @@ -99,13 +99,31 @@ export class OrganizationOnboardingService { return { user, membership, project, outboxEventId: event.id } as unknown as Promise; } - private upsertUser(email: string, initialPassword: string): User { + /** + * An invite always creates a NEW account. It deliberately does not attach an + * existing one: joining an organization is the account owner's decision, and + * an admin who could attach any address would be able to pull another + * tenant's user into this org with a role of their choosing — after which + * that account satisfies every "is a member of this org" predicate, including + * the assignee check in `TasksService.assignTask`. + * + * The "already a member here" and the "belongs to another tenant" cases raise + * the SAME error, so the refusal never reveals which tenants an address + * belongs to. It does still reveal that the address HAS an account; removing + * that last signal needs a pending-invitation row the invitee accepts, which + * is the shape a production app should reach for. + */ + private createInvitee(email: string, initialPassword: string): User { const existing = this.db .select() .from(users) .where(eq(users.email, email)) .get(); - if (existing) return existing; + if (existing) { + throw new ConflictException( + 'An account already exists for this email; it can only join an organization from its own side', + ); + } return this.db .insert(users) .values({ diff --git a/test/integration/tenant-authz.spec.ts b/test/integration/tenant-authz.spec.ts index 3db7904..5798337 100644 --- a/test/integration/tenant-authz.spec.ts +++ b/test/integration/tenant-authz.spec.ts @@ -5,7 +5,7 @@ import { join } from 'node:path'; import { after, before, test } from 'node:test'; import type { INestApplicationContext } from '@nestjs/common'; import { ContextIdFactory, NestFactory } from '@nestjs/core'; -import { eq } from 'drizzle-orm'; +import { and, eq } from 'drizzle-orm'; import { getDrizzleClientToken } from '@nest-native/drizzle'; import { hashPassword } from '../../src/auth/password'; import { AuthService } from '../../src/auth/auth.service'; @@ -18,6 +18,7 @@ import { tasks, users, } from '../../src/database/schema'; +import { OrganizationOnboardingService } from '../../src/modules/onboarding/organization-onboarding.service'; import { TasksService } from '../../src/modules/tasks/tasks.service'; import { seedDatabase } from '../../scripts/seed'; @@ -33,6 +34,7 @@ const MISSING_ID = 999_999; let app: INestApplicationContext; let tasksService: TasksService; +let onboarding: OrganizationOnboardingService; let auth: AuthService; let inspect: AppDatabase; let acmeOrgId: number; @@ -44,6 +46,9 @@ let rivalUserId: number; const counts = () => ({ tasks: inspect.select().from(tasks).all().length, outboxEvents: inspect.select().from(outboxEvents).all().length, + users: inspect.select().from(users).all().length, + memberships: inspect.select().from(memberships).all().length, + projects: inspect.select().from(projects).all().length, }); /** The id is the only part that may differ between the two error messages. */ @@ -64,6 +69,7 @@ before(async () => { abortOnError: false, }); auth = app.get(AuthService); + onboarding = app.get(OrganizationOnboardingService); inspect = app.get(getDrizzleClientToken()); const nowIso = new Date().toISOString(); @@ -231,3 +237,48 @@ test('login puts the OLDEST membership in the token, stably across logins', asyn assert.equal(first.organization?.id, acmeOrgId); assert.deepEqual(second.organization, first.organization); }); + +test('invite refuses an existing account, so no admin can attach another tenant\'s user', async () => { + const before = counts(); + + const foreignError = await onboarding + .inviteUser({ + orgId: acmeOrgId, + invitedByUserId: acmeAdminId, + email: 'boss@rival.test', + projectName: 'Poached Project', + initialPassword: 'poached-pass-12345', + }) + .then(() => undefined) + .catch((error: Error) => error); + const insiderError = await onboarding + .inviteUser({ + orgId: acmeOrgId, + invitedByUserId: acmeAdminId, + email: 'admin@acme.test', + projectName: 'Duplicate Project', + initialPassword: 'duplicate-pass-12345', + }) + .then(() => undefined) + .catch((error: Error) => error); + + assert.ok(foreignError instanceof Error); + assert.ok(insiderError instanceof Error); + assert.equal( + foreignError.message, + insiderError.message, + "the refusal must not reveal which organization an address belongs to", + ); + + // The RIVAL admin gained no foothold in ACME — which is also what keeps + // assignTask's membership predicate from being admin-grantable. + const attached = inspect + .select() + .from(memberships) + .where( + and(eq(memberships.orgId, acmeOrgId), eq(memberships.userId, rivalUserId)), + ) + .get(); + assert.equal(attached, undefined); + assert.deepEqual(counts(), before, 'a refused invite writes nothing'); +}); From 11ac52331c9892e438cd1123be5db56ea7d87800 Mon Sep 17 00:00:00 2001 From: rodrigobnogueira Date: Thu, 13 Aug 2026 17:46:33 -0300 Subject: [PATCH 11/12] refactor(memberships): register the repository once and re-export it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MembershipsRepository was registered through DrizzleModule.forFeature in five modules — auth, tasks, projects, users, onboarding — three of them added by this branch with a comment each explaining why the duplicate was there, while memberships.module.ts, whose whole job is to export that repository, sat unused and broken: it called forFeature() twice, and Nest 11 keys modules by object identity, so it exported a dynamic module the container never instantiated. That module now hoists the single forFeature() call into a constant reused by imports and exports, AuthModule imports and re-exports it, and the four duplicates go away. The guard's dependency travels with the guard: every module that already imports AuthModule for AuthGuard/RolesGuard gets the repository too, and there is now one instance of it in the app instead of five. --- src/auth/auth.module.ts | 11 ++++++----- src/modules/memberships/memberships.module.ts | 16 +++++++++++++--- src/modules/onboarding/onboarding.module.ts | 5 +++-- src/modules/projects/projects.module.ts | 4 +--- src/modules/tasks/tasks.module.ts | 14 ++++---------- src/modules/users/users.module.ts | 4 +--- 6 files changed, 28 insertions(+), 26 deletions(-) diff --git a/src/auth/auth.module.ts b/src/auth/auth.module.ts index ee26871..384e55e 100644 --- a/src/auth/auth.module.ts +++ b/src/auth/auth.module.ts @@ -3,10 +3,9 @@ import { Module, type NestModule, } from '@nestjs/common'; -import { DrizzleModule } from '@nest-native/drizzle'; import { loadEnv } from '../config/env'; import { DatabaseModule } from '../database/database.module'; -import { MembershipsRepository } from '../modules/memberships/memberships.repository'; +import { MembershipsModule } from '../modules/memberships/memberships.module'; import { AUTH_CONFIG, type AuthConfig } from './auth.config'; import { AuthGuard } from './auth.guard'; import { AuthMiddleware } from './auth.middleware'; @@ -19,8 +18,10 @@ import { RolesGuard } from './roles.guard'; imports: [ DatabaseModule, AppLockoutModule, - // RolesGuard re-reads the caller's membership on every guarded mutation. - DrizzleModule.forFeature([MembershipsRepository]), + // RolesGuard re-reads the caller's membership on every guarded request, so + // the repository is re-exported below: the guard's dependency travels with + // the guard into every module that imports AuthModule. + MembershipsModule, ], providers: [ { @@ -35,7 +36,7 @@ import { RolesGuard } from './roles.guard'; RolesGuard, AuthRouter, ], - exports: [AuthService, AuthGuard, RolesGuard], + exports: [AuthService, AuthGuard, RolesGuard, MembershipsModule], }) export class AuthModule implements NestModule { configure(consumer: MiddlewareConsumer): void { diff --git a/src/modules/memberships/memberships.module.ts b/src/modules/memberships/memberships.module.ts index cb6ea15..3d267c1 100644 --- a/src/modules/memberships/memberships.module.ts +++ b/src/modules/memberships/memberships.module.ts @@ -2,8 +2,18 @@ import { Module } from '@nestjs/common'; import { DrizzleModule } from '@nest-native/drizzle'; import { MembershipsRepository } from './memberships.repository'; +// ONE registration of the tenancy predicate for the whole app: RolesGuard reads +// it on every guarded request, TasksService validates assignees with it, and +// onboarding writes memberships through it. +// +// The `forFeature(...)` call is hoisted into a constant on purpose. It returns a +// FRESH dynamic module object each call and Nest 11 keys modules by object +// identity, so calling it twice — once for `imports`, once for `exports` — +// exports a module the container never instantiated. +const MembershipsFeature = DrizzleModule.forFeature([MembershipsRepository]); + @Module({ - imports: [DrizzleModule.forFeature([MembershipsRepository])], - exports: [DrizzleModule.forFeature([MembershipsRepository])], + imports: [MembershipsFeature], + exports: [MembershipsFeature], }) -export class MembershipsModule {} \ No newline at end of file +export class MembershipsModule {} diff --git a/src/modules/onboarding/onboarding.module.ts b/src/modules/onboarding/onboarding.module.ts index 1394114..7cb7724 100644 --- a/src/modules/onboarding/onboarding.module.ts +++ b/src/modules/onboarding/onboarding.module.ts @@ -2,7 +2,7 @@ import { Module } from '@nestjs/common'; import { DrizzleModule } from '@nest-native/drizzle'; import { DatabaseModule } from '../../database/database.module'; import { AuditLogModule } from '../audit-log/audit-log.module'; -import { MembershipsRepository } from '../memberships/memberships.repository'; +import { MembershipsModule } from '../memberships/memberships.module'; import { ProjectsRepository } from '../projects/projects.repository'; import { OrganizationOnboardingService } from './organization-onboarding.service'; @@ -17,7 +17,8 @@ import { OrganizationOnboardingService } from './organization-onboarding.service @Module({ imports: [ DatabaseModule, - DrizzleModule.forFeature([MembershipsRepository, ProjectsRepository]), + MembershipsModule, + DrizzleModule.forFeature([ProjectsRepository]), AuditLogModule, ], providers: [OrganizationOnboardingService], diff --git a/src/modules/projects/projects.module.ts b/src/modules/projects/projects.module.ts index 48ad3d4..a9a3ad7 100644 --- a/src/modules/projects/projects.module.ts +++ b/src/modules/projects/projects.module.ts @@ -2,15 +2,13 @@ import { Module } from '@nestjs/common'; import { DrizzleModule } from '@nest-native/drizzle'; import { AuthModule } from '../../auth/auth.module'; import { RequestContextModule } from '../../context/request-context.module'; -import { MembershipsRepository } from '../memberships/memberships.repository'; import { ProjectsRepository } from './projects.repository'; import { ProjectsRouter } from './projects.router'; import { ProjectsService } from './projects.service'; @Module({ imports: [ - // MembershipsRepository is here for RolesGuard on projects.create. - DrizzleModule.forFeature([ProjectsRepository, MembershipsRepository]), + DrizzleModule.forFeature([ProjectsRepository]), AuthModule, RequestContextModule, ], diff --git a/src/modules/tasks/tasks.module.ts b/src/modules/tasks/tasks.module.ts index 2e8ed92..ce0c0ec 100644 --- a/src/modules/tasks/tasks.module.ts +++ b/src/modules/tasks/tasks.module.ts @@ -2,7 +2,6 @@ import { Module } from '@nestjs/common'; import { DrizzleModule } from '@nest-native/drizzle'; import { AuthModule } from '../../auth/auth.module'; import { RequestContextModule } from '../../context/request-context.module'; -import { MembershipsRepository } from '../memberships/memberships.repository'; import { ProjectsRepository } from '../projects/projects.repository'; import { TasksRepository } from './tasks.repository'; import { TasksRouter } from './tasks.router'; @@ -10,17 +9,12 @@ import { TasksService } from './tasks.service'; // Mirrors ProjectsModule. The transactional OutboxProducer the service injects // comes from the global MessagingModule, so no messaging wiring lives here. -// The projects/memberships repositories are the tenancy predicates the service -// checks in-transaction (owning project, org member assignee) and the ones -// RolesGuard reads — repositories are stateless, so a second forFeature -// registration is the same pattern OnboardingModule uses. +// ProjectsRepository is one of the tenancy predicates the service checks +// in-transaction (the task's project must belong to the caller's org); the +// other — MembershipsRepository, for the assignee — arrives with AuthModule. @Module({ imports: [ - DrizzleModule.forFeature([ - TasksRepository, - ProjectsRepository, - MembershipsRepository, - ]), + DrizzleModule.forFeature([TasksRepository, ProjectsRepository]), AuthModule, RequestContextModule, ], diff --git a/src/modules/users/users.module.ts b/src/modules/users/users.module.ts index e7ccf15..7921167 100644 --- a/src/modules/users/users.module.ts +++ b/src/modules/users/users.module.ts @@ -2,7 +2,6 @@ import { Module } from '@nestjs/common'; import { DrizzleModule } from '@nest-native/drizzle'; import { AuthModule } from '../../auth/auth.module'; import { RequestContextModule } from '../../context/request-context.module'; -import { MembershipsRepository } from '../memberships/memberships.repository'; import { OnboardingModule } from '../onboarding/onboarding.module'; import { UsersRepository } from './users.repository'; import { UsersRouter } from './users.router'; @@ -10,8 +9,7 @@ import { UsersService } from './users.service'; @Module({ imports: [ - // MembershipsRepository is here for RolesGuard on users.invite. - DrizzleModule.forFeature([UsersRepository, MembershipsRepository]), + DrizzleModule.forFeature([UsersRepository]), AuthModule, RequestContextModule, OnboardingModule, From 0aa0d897a3bf12d9c91b6c0718c4c185fa7b3355 Mon Sep 17 00:00:00 2001 From: rodrigobnogueira Date: Thu, 13 Aug 2026 17:46:42 -0300 Subject: [PATCH 12/12] docs: revocation now lands on reads, and an invite cannot attach an account The auth section said reads stay token-trusted until the TTL; they no longer do, and the surface that changed is named: roster, projects, activity feed and the token-spending AI digest all go on the next request after a membership is revoked. Adds the invite rule (a new account only, so no admin can pull another tenant's user in), notes the residual account-exists signal, and records where the one MembershipsRepository registration lives and why forFeature() is hoisted into a constant. Lifecycle diagram, layout row and test table follow. --- README.md | 27 +++++++++++++++++--------- docs/architecture.md | 45 +++++++++++++++++++++++++++++++++----------- 2 files changed, 52 insertions(+), 20 deletions(-) diff --git a/README.md b/README.md index 09a95db..1f92f70 100644 --- a/README.md +++ b/README.md @@ -51,20 +51,29 @@ Login mints an HS256 JWT that **snapshots one active organization** — the caller's *oldest* membership (`created_at`, then `id` as the tiebreak, so repeated logins always resolve the same tenant). The token carries no role. -- **Mutations re-check the live membership.** `RolesGuard` composes after - `AuthGuard` and reads the caller's role from the database on every procedure - that declares `@Roles(...)`: `users.invite` is **admin** only; - `tasks.create` / `.assign` / `.complete` and `projects.create` accept - **admin or member**; a **viewer** reads only. Revoking a membership therefore - blocks the next mutation instead of waiting for the token to expire. -- **Reads stay token-trusted** for the token's lifetime (`AUTH_TTL_SECONDS`, - default 3600 — an invalid value now fails at boot rather than minting tokens - that never verify). That trade-off is documented, not hidden. +- **Every guarded request re-checks the live membership.** `RolesGuard` + composes after `AuthGuard` and resolves the caller's membership in the + token's organization from the database — **reads included**, so revoking a + membership blocks the next request rather than leaving the member roster, the + project list, the activity feed and the token-spending AI assistant readable + until the token expires. +- **`@Roles(...)` narrows a procedure further.** `users.invite` is **admin** + only; `tasks.create` / `.assign` / `.complete` and `projects.create` accept + **admin or member**; a **viewer** reads only. Without `@Roles`, holding any + live membership is enough. +- **The token is a snapshot, never a permission.** It lives for + `AUTH_TTL_SECONDS` (default 3600 — an invalid value now fails at boot rather + than minting tokens that never verify) and names one organization; every + authorization decision is a fresh indexed lookup, deliberately uncached. - **Tenancy is proven at the write.** Inside the same transaction, `tasks.create` requires the project to belong to the caller's org and `tasks.assign` requires the assignee to be a member of it. Both refuse with exactly the error a nonexistent id gets, so the API is never a cross-tenant existence oracle. +- **An invite creates a new account, never attaches an existing one.** Joining + an organization is the account owner's call, so an admin cannot pull another + tenant's user into their org (and then assign work to it); the refusal is the + same whether the address is already a member here or a stranger. > **Password hashing is synchronous.** `src/auth/password.ts` uses `scryptSync` > because a short, obviously-correct helper reads better in a reference app — diff --git a/docs/architecture.md b/docs/architecture.md index 3d4e00e..31ba492 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -150,8 +150,8 @@ service deps through `@Inject(...)` in the constructor and call them. │ - or - │ │ tRPC handler │ ←── nest-trpc-native dispatch │ AuthGuard │ reads ctx.authContext via getArgs()[1] - │ RolesGuard │ re-reads the caller's membership role - │ │ (mutations only — see Authorization) + │ RolesGuard │ re-reads the caller's membership row + │ │ (every guarded request — see Authorization) │ ParamDecorators │ @Input, @TrpcContext, @CurrentUser │ Procedure body │ └──────────────────────┘ @@ -207,13 +207,13 @@ the hash at all. The token says **who** is calling and **which** organization is active. It deliberately says nothing about what the caller may do — that is re-read from -the database on every mutation: +the database on every guarded request: ``` @Router('tasks') @UseGuards(AuthGuard, RolesGuard) ← composed left to right export class TasksRouter { - @Query(...) list(...) ← no @Roles: token-trusted read + @Query(...) list(...) ← no @Roles: any live member may read @Roles('admin', 'member') @Mutation(...) create(...) ← RolesGuard re-reads the membership row } @@ -225,10 +225,23 @@ time and throws `ForbiddenException` when the membership is missing (revoked) or its role is not in the procedure's `@Roles(...)` list. The policy is deliberately small — three roles, no policy engine, no per-resource ACLs: `users.invite` is `admin` only; `tasks.create` / `.assign` / `.complete` and -`projects.create` accept `admin` or `member`; `viewer` reads only. Reads carry -no `@Roles` and stay token-trusted until the TTL lapses, so a revoked member -loses **mutations** on their next request and **reads** when their token -expires. +`projects.create` accept `admin` or `member`; `viewer` reads only. + +**Reads are guarded too.** A procedure without `@Roles` still needs a live +membership: the guard is on every tenant-scoped router (`tasks`, `projects`, +`users`, `organizations`, `activity`) and on the assistant controller, so a +revoked account loses the member roster, the project list, the activity feed and +the token-spending AI digest on its *next* request instead of keeping them for +up to `AUTH_TTL_SECONDS`. The cost is one indexed lookup per request; it is +deliberately not cached, because a stale allow is exactly the failure being +prevented. `auth.me` is the exception — it only echoes the token back. + +Because the guard's dependency has to travel with the guard, the single +`DrizzleModule.forFeature([MembershipsRepository])` registration lives in +[`MembershipsModule`](https://github.com/nest-native/reference-app/blob/main/src/modules/memberships/memberships.module.ts), +which `AuthModule` imports **and re-exports**. (`forFeature()` returns a new +dynamic module object per call and Nest keys modules by identity, so that one +call is hoisted into a constant and reused by `imports` and `exports`.) Tenancy is proven at the write, not assumed from the token. Inside the same transaction that writes the row, `TasksService.createTask` requires the @@ -237,6 +250,15 @@ assignee to hold a membership *in that org*. Both refuse with exactly the error a nonexistent id gets (`Project 42 not found` / `User 42 is not a member of this organization`) — a distinct "belongs to someone else" message would turn the API into a cross-tenant existence oracle. +Membership itself is only ever granted to a **new** account: +`OrganizationOnboardingService` refuses an invite whose email already has one, +so an admin cannot attach a stranger — or another tenant's admin — to their +organization without consent, and cannot manufacture an assignee that way. The +refusal reads the same for an address that is already a member here and for one +that belongs to another tenant; erasing the last signal (that an account exists +at all) needs a pending-invitation row the invitee accepts, which is the shape a +production app should reach for. + See [`test/integration/tenant-authz.spec.ts`](https://github.com/nest-native/reference-app/blob/main/test/integration/tenant-authz.spec.ts) and [`test/e2e/roles-authz.spec.ts`](https://github.com/nest-native/reference-app/blob/main/test/e2e/roles-authz.spec.ts). @@ -388,7 +410,7 @@ same image (see `docker-compose.yml`). | `modules/organizations/` | Repo + service + tRPC router. `organizations.current` / `.list` | | `modules/users/` | Repo + service + tRPC router. `users.me` / `.list` / `.invite` | | `modules/projects/` | Repo + service + tRPC router. `projects.list` / `.get` / `.create` | -| `modules/memberships/` | Repo only (consumed by onboarding, `RolesGuard`, and the task tenancy checks) | +| `modules/memberships/` | Repo + the one `forFeature` registration of it, re-exported through `AuthModule` (consumed by onboarding, `RolesGuard`, and the task tenancy checks) | | `modules/audit-log/` | `AuditLogService.record()` | | `modules/outbox/` | Producer, claimer, registry, fake transport, `user.invited` handler, `outbox.constants.ts` | | `modules/onboarding/` | `OrganizationOnboardingService` — the `@Transactional` workflow | @@ -410,8 +432,9 @@ same image (see `docker-compose.yml`). | `test/e2e/auth-flow.spec.ts` | Login flow over real HTTP; 401 on wrong password / no token / bad token | | `test/e2e/trpc-ping.smoke.spec.ts` | `GET /trpc/ping` returns 'pong'; `/health` returns ok | | `test/e2e/core-modules.spec.ts` | Authenticated flow over real HTTP across the three core routers | -| `test/integration/tenant-authz.spec.ts` | Cross-org project/assignee are refused like missing ones, nothing committed; login picks the oldest membership | -| `test/e2e/roles-authz.spec.ts` | RBAC over real HTTP: viewer/member/admin limits, and a revoked membership blocking the next mutation | +| `test/integration/tenant-authz.spec.ts` | Cross-org project/assignee are refused like missing ones, nothing committed; an invite cannot attach an existing account; login picks the oldest membership | +| `test/e2e/roles-authz.spec.ts` | RBAC over real HTTP: viewer/member/admin limits, and a revoked membership blocking the next request — reads and the AI assistant included | +| `test/integration/auth-context.spec.ts` | The guards' caller extractor: tRPC reads the procedure context, never the caller's input | Plus `client-smoke/client.ts` (typed client over real HTTP using the generated `AppRouter`) which is run via `npm run client-smoke` rather than