diff --git a/README.md b/README.md index cd2441d..1f92f70 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,43 @@ 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. + +- **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 — +> 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 +130,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 +157,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..31ba492 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 row + │ │ (every guarded request — 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,71 @@ 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 guarded request: + +``` +@Router('tasks') +@UseGuards(AuthGuard, RolesGuard) ← composed left to right +export class TasksRouter { + @Query(...) list(...) ← no @Roles: any live member may 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 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 +`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. +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). ## Login lockout @@ -333,13 +404,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 + 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 | @@ -361,6 +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; 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 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 }, diff --git a/src/auth/auth-context.ts b/src/auth/auth-context.ts index 1c63752..0b518f1 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,30 @@ 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. + * + * 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; + return trpcCtx?.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..384e55e 100644 --- a/src/auth/auth.module.ts +++ b/src/auth/auth.module.ts @@ -5,15 +5,24 @@ import { } from '@nestjs/common'; import { loadEnv } from '../config/env'; import { DatabaseModule } from '../database/database.module'; +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'; 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 request, so + // the repository is re-exported below: the guard's dependency travels with + // the guard into every module that imports AuthModule. + MembershipsModule, + ], providers: [ { provide: AUTH_CONFIG, @@ -24,9 +33,10 @@ import { AppLockoutModule } from './lockout.setup'; }, AuthService, AuthGuard, + RolesGuard, AuthRouter, ], - exports: [AuthService, AuthGuard], + exports: [AuthService, AuthGuard, RolesGuard, MembershipsModule], }) export class AuthModule implements NestModule { configure(consumer: MiddlewareConsumer): void { 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; 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..08fb7c6 --- /dev/null +++ b/src/auth/roles.guard.ts @@ -0,0 +1,67 @@ +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, 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 { + 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()], + ); + 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( + auth.organization.id, + auth.user.id, + ); + if (!membership) { + throw new ForbiddenException( + 'You are no longer a member of this organization', + ); + } + if (allowed?.length && !allowed.includes(membership.role)) { + throw new ForbiddenException( + `Requires role ${allowed.join(' or ')}; you are ${membership.role}`, + ); + } + return true; + } +} 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/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/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/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/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/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/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/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.module.ts b/src/modules/tasks/tasks.module.ts index 0216a58..ce0c0ec 100644 --- a/src/modules/tasks/tasks.module.ts +++ b/src/modules/tasks/tasks.module.ts @@ -2,15 +2,19 @@ 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 { 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. +// 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]), + DrizzleModule.forFeature([TasksRepository, ProjectsRepository]), AuthModule, RequestContextModule, ], diff --git a/src/modules/tasks/tasks.router.ts b/src/modules/tasks/tasks.router.ts index 1ce8863..5c2c1d1 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 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) +@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/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`); 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, diff --git a/test/e2e/roles-authz.spec.ts b/test/e2e/roles-authz.spec.ts new file mode 100644 index 0000000..87d900d --- /dev/null +++ b/test/e2e/roles-authz.spec.ts @@ -0,0 +1,226 @@ +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; +} + +/** 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}` }, + }); + 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 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 request 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 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); +}); 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); +}); 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); diff --git a/test/integration/tenant-authz.spec.ts b/test/integration/tenant-authz.spec.ts new file mode 100644 index 0000000..5798337 --- /dev/null +++ b/test/integration/tenant-authz.spec.ts @@ -0,0 +1,284 @@ +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 { and, 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 { OrganizationOnboardingService } from '../../src/modules/onboarding/organization-onboarding.service'; +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 onboarding: OrganizationOnboardingService; +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, + 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. */ +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); + onboarding = app.get(OrganizationOnboardingService); + 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); +}); + +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'); +});