Skip to content

Commit 22345f6

Browse files
matt-aitkenclaude
andcommitted
fix(rbac,webapp): pass userId through plugin context, drop session-cookie helper
Background. The RBAC plugin contract took a `helpers.getSessionUserId(request)` callback at create time. The OSS rbac.server module wired it up by statically importing `getUserId` from `~/services/session.server`, which transitively loaded the entire remix-auth pipeline: rbac.server.ts → session.server.ts → auth.server.ts → emailAuth.server.tsx (throws on missing MAGIC_LINK_SECRET at module load) → email.server.ts → commonWorker.server.ts (V1 services) → marqs/index.server.ts and taskRunConcurrencyTracker.server.ts (throw on missing REDIS_HOST/REDIS_PORT at module load) Any module that pulled `rbac` in — including PAT-only call sites that have no session-cookie path at all — therefore inherited a hard dependency on the entire dashboard auth chain and the V1 queue. Webapp tests that mock `~/env.server` to a stripped object (e.g. personalAccessToken.test.ts) hit the module-load throws before their assertions ran. Contract change (packages/plugins/src/rbac.ts). - `authenticateSession` and `authenticateAuthorizeSession` now take `userId: string | null` in their `context` argument. - `RoleBasedAccessControlPlugin.create()` no longer takes a helpers arg. - The plugin treats `userId === null` as "no authenticated user" (same shape `helpers.getSessionUserId === null` returned). OSS fallback (internal-packages/rbac/src/fallback.ts) reads `context.userId` directly. LazyController (internal-packages/rbac/src/index.ts) drops the helpers parameter through. Webapp (apps/webapp/app/services/rbac.server.ts) drops the session.server import entirely. dashboardBuilder.server.ts — the only `authenticateSession` caller — resolves userId via session.server itself before calling rbac. Side change (apps/webapp/app/services/scheduleEmail.server.ts new file). Moved `scheduleEmail` out of email.server.ts to break a parallel chain: email.server.ts → commonWorker → V1 services → marqs. email.server is now just the SMTP/Resend client, as it should have been. Three caller files updated: routes/invite-resend.tsx, routes/_app.orgs.$organizationSlug.invite/route.tsx, services/mfa/multiFactorAuthentication.server.ts. Verified locally with `REDIS_HOST="" REDIS_PORT="" pnpm vitest run test/services/personalAccessToken.test.ts test/engine/triggerTask.test.ts` — both files pass (11/11) with the env mocked stripped, which was the CI failure mode on shards 1/2/4/5/6/7. Webapp + plugins + rbac typecheck clean. Cloud-repo plugin update is in a coordinated branch (rbac-packages on APIHero/cloud) — same contract change in enterprise/plugins/src/rbac/{index,controller,controller.integration.test}.ts. Changeset: minor for `@trigger.dev/plugins` (contract change). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 3290f57 commit 22345f6

10 files changed

Lines changed: 63 additions & 51 deletions

File tree

apps/webapp/app/routes/_app.orgs.$organizationSlug.invite/route.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ import { useOrganization } from "~/hooks/useOrganizations";
3232
import { inviteMembers } from "~/models/member.server";
3333
import { redirectWithSuccessMessage } from "~/models/message.server";
3434
import { TeamPresenter } from "~/presenters/TeamPresenter.server";
35-
import { scheduleEmail } from "~/services/email.server";
35+
import { scheduleEmail } from "~/services/scheduleEmail.server";
3636
import { rbac } from "~/services/rbac.server";
3737
import { requireUserId } from "~/services/session.server";
3838
import { acceptInvitePath, organizationTeamPath, v3BillingPath } from "~/utils/pathBuilder";

apps/webapp/app/routes/invite-resend.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { env } from "process";
44
import { z } from "zod";
55
import { resendInvite } from "~/models/member.server";
66
import { redirectWithSuccessMessage } from "~/models/message.server";
7-
import { scheduleEmail } from "~/services/email.server";
7+
import { scheduleEmail } from "~/services/scheduleEmail.server";
88
import { requireUserId } from "~/services/session.server";
99
import { acceptInvitePath, organizationTeamPath } from "~/utils/pathBuilder";
1010

apps/webapp/app/services/email.server.ts

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import type { SendEmailOptions } from "remix-auth-email-link";
44
import { redirect } from "remix-typedjson";
55
import { env } from "~/env.server";
66
import type { AuthUser } from "./authUser";
7-
import { commonWorker } from "~/v3/commonWorker.server";
87
import { logger } from "./logger.server";
98
import { singleton } from "~/utils/singleton";
109
import { assertEmailAllowed } from "~/utils/email";
@@ -92,15 +91,6 @@ export async function sendPlainTextEmail(options: SendPlainTextOptions) {
9291
return client.sendPlainText(options);
9392
}
9493

95-
export async function scheduleEmail(data: DeliverEmail, delay?: { seconds: number }) {
96-
const availableAt = delay ? new Date(Date.now() + delay.seconds * 1000) : undefined;
97-
await commonWorker.enqueue({
98-
job: "scheduleEmail",
99-
payload: data,
100-
availableAt,
101-
});
102-
}
103-
10494
export async function sendEmail(data: DeliverEmail) {
10595
return client.send(data);
10696
}

apps/webapp/app/services/mfa/multiFactorAuthentication.server.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { createHash } from "@better-auth/utils/hash";
77
import { createOTP } from "@better-auth/utils/otp";
88
import { base32 } from "@better-auth/utils/base32";
99
import { z } from "zod";
10-
import { scheduleEmail } from "../email.server";
10+
import { scheduleEmail } from "../scheduleEmail.server";
1111

1212
const generateRandomString = createRandomStringGenerator("A-Z", "0-9");
1313

apps/webapp/app/services/rbac.server.ts

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,6 @@ import { $replica, prisma } from "~/db.server";
22
import type { PrismaClient } from "@trigger.dev/database";
33
import plugin from "@trigger.dev/rbac";
44
import { env } from "~/env.server";
5-
import { getUserId } from "./session.server";
6-
7-
async function getSessionUserId(request: Request): Promise<string | null> {
8-
const id = await getUserId(request);
9-
return id ?? null;
10-
}
115

126
// plugin.create() is synchronous — returns a lazy controller that resolves
137
// any installed RBAC plugin on first call. Top-level await is not used
@@ -17,10 +11,19 @@ async function getSessionUserId(request: Request): Promise<string | null> {
1711
// they don't pile up on the primary. Writes (role mutations) still go
1812
// through the primary. Same separation findEnvironmentByApiKey used
1913
// before this PR moved bearer auth into the RBAC plugin.
14+
//
15+
// Session-cookie userId resolution lives at the call site (see
16+
// dashboardBuilder.server.ts), not here. Statically importing
17+
// `~/services/session.server` from this module dragged the entire
18+
// remix-auth pipeline (auth.server → emailAuth/gitHubAuth/googleAuth,
19+
// each validating their secret at module load) into anything that
20+
// transitively imported `rbac` — including PAT auth callers that have
21+
// no session-cookie path at all. Passing userId through the
22+
// `authenticateSession` context decouples the plugin host from the
23+
// host's session implementation.
2024
export const rbac = plugin.create(
2125
// $replica is structurally a PrismaClient minus `$transaction` — the
2226
// RBAC fallback only uses `findFirst` on it, so the cast is safe.
2327
{ primary: prisma, replica: $replica as PrismaClient },
24-
{ getSessionUserId },
2528
{ forceFallback: env.RBAC_FORCE_FALLBACK }
2629
);

apps/webapp/app/services/routeBuilders/dashboardBuilder.server.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import { json, redirect } from "@remix-run/server-runtime";
77
import type { RbacAbility } from "@trigger.dev/rbac";
88
import { rbac } from "~/services/rbac.server";
9+
import { getUserId } from "~/services/session.server";
910
import type {
1011
AuthorizationOption,
1112
DashboardLoaderOptions,
@@ -85,7 +86,15 @@ export async function authenticateAndAuthorize<
8586
const ctx = (options.context
8687
? await options.context(parsedParams, request)
8788
: ({} as TContext)) as TContext;
88-
const auth = await rbac.authenticateSession(request, ctx);
89+
// Resolve userId from the session cookie *here* (the dashboard
90+
// request boundary) and feed it into the rbac plugin context. The
91+
// plugin no longer takes a `helpers.getSessionUserId` callback —
92+
// statically importing session.server from rbac.server dragged the
93+
// entire remix-auth strategy chain (each strategy validates its
94+
// secret at module load) into anything that pulled `rbac` in,
95+
// including PAT-only callers.
96+
const userId = (await getUserId(request)) ?? null;
97+
const auth = await rbac.authenticateSession(request, { ...ctx, userId });
8998
if (!auth.ok) {
9099
if (auth.reason === "unauthenticated") {
91100
return { ok: false, response: loginRedirectFor(request, options.loginRedirect) };
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import type { DeliverEmail } from "emails";
2+
import { commonWorker } from "~/v3/commonWorker.server";
3+
4+
// Lives outside email.server.ts so that the SMTP/Resend client module
5+
// stays a leaf dependency. Pulling commonWorker from email.server poisoned
6+
// every consumer of the auth chain (auth → emailAuth → email) with the
7+
// V1+V2 worker tree, which transitively loads marqs and trips Redis-env
8+
// guards in any vitest file whose import graph reaches it.
9+
export async function scheduleEmail(data: DeliverEmail, delay?: { seconds: number }) {
10+
const availableAt = delay ? new Date(Date.now() + delay.seconds * 1000) : undefined;
11+
await commonWorker.enqueue({
12+
job: "scheduleEmail",
13+
payload: data,
14+
availableAt,
15+
});
16+
}

internal-packages/rbac/src/fallback.ts

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -46,21 +46,16 @@ export class RoleBaseAccessFallback {
4646
this.clients = resolvePrismaClients(prisma);
4747
}
4848

49-
create(
50-
helpers: { getSessionUserId: (request: Request) => Promise<string | null> }
51-
): RoleBaseAccessFallbackController {
52-
return new RoleBaseAccessFallbackController(this.clients, helpers);
49+
create(): RoleBaseAccessFallbackController {
50+
return new RoleBaseAccessFallbackController(this.clients);
5351
}
5452
}
5553

5654
class RoleBaseAccessFallbackController implements RoleBaseAccessController {
5755
private readonly prisma: PrismaClient; // alias for primary — used by writes
5856
private readonly replica: PrismaClient;
5957

60-
constructor(
61-
clients: FallbackPrismaClients,
62-
private readonly helpers: { getSessionUserId: (request: Request) => Promise<string | null> }
63-
) {
58+
constructor(clients: FallbackPrismaClients) {
6459
this.prisma = clients.primary;
6560
this.replica = clients.replica;
6661
}
@@ -212,13 +207,12 @@ class RoleBaseAccessFallbackController implements RoleBaseAccessController {
212207
}
213208

214209
async authenticateSession(
215-
request: Request,
216-
context: { organizationId?: string; projectId?: string }
210+
_request: Request,
211+
context: { userId: string | null; organizationId?: string; projectId?: string }
217212
): Promise<SessionAuthResult> {
218-
const userId = await this.helpers.getSessionUserId(request);
219-
if (!userId) return { ok: false, reason: "unauthenticated" };
213+
if (!context.userId) return { ok: false, reason: "unauthenticated" };
220214

221-
const user = await this.replica.user.findFirst({ where: { id: userId } });
215+
const user = await this.replica.user.findFirst({ where: { id: context.userId } });
222216
if (!user) return { ok: false, reason: "unauthenticated" };
223217

224218
const subject: RbacSubject = {
@@ -251,7 +245,7 @@ class RoleBaseAccessFallbackController implements RoleBaseAccessController {
251245

252246
async authenticateAuthorizeSession(
253247
request: Request,
254-
context: { organizationId?: string; projectId?: string },
248+
context: { userId: string | null; organizationId?: string; projectId?: string },
255249
check: { action: string; resource: RbacResource | RbacResource[] }
256250
): Promise<SessionAuthResult> {
257251
const auth = await this.authenticateSession(request, context);

internal-packages/rbac/src/index.ts

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,6 @@ import type { PrismaClient } from "@trigger.dev/database";
1212
import { RoleBaseAccessFallback } from "./fallback.js";
1313
export type { RoleBaseAccessController, RbacAbility, RbacResource } from "@trigger.dev/plugins";
1414

15-
type RbacHelpers = { getSessionUserId: (request: Request) => Promise<string | null> };
16-
1715
// Either a single PrismaClient (used for both writes and reads — fine
1816
// for callers that don't have a separate replica), or `{primary, replica}`
1917
// where reads on the auth hot path go to the replica. The fallback
@@ -53,24 +51,23 @@ export function withActionAliases(underlying: RbacAbility): RbacAbility {
5351
class LazyController implements RoleBaseAccessController {
5452
private readonly _init: Promise<RoleBaseAccessController>;
5553

56-
constructor(prisma: RbacPrismaInput, helpers: RbacHelpers, options?: RbacCreateOptions) {
57-
this._init = this.load(prisma, helpers, options);
54+
constructor(prisma: RbacPrismaInput, options?: RbacCreateOptions) {
55+
this._init = this.load(prisma, options);
5856
}
5957

6058
private async load(
6159
prisma: RbacPrismaInput,
62-
helpers: RbacHelpers,
6360
options?: RbacCreateOptions
6461
): Promise<RoleBaseAccessController> {
6562
if (options?.forceFallback) {
66-
return new RoleBaseAccessFallback(prisma).create(helpers);
63+
return new RoleBaseAccessFallback(prisma).create();
6764
}
6865
const moduleName = "@triggerdotdev/plugins/rbac";
6966
try {
7067
const module = await import(moduleName);
7168
const plugin: RoleBasedAccessControlPlugin = module.default;
7269
console.log("RBAC: using plugin implementation");
73-
return plugin.create(helpers);
70+
return plugin.create();
7471
} catch (err) {
7572
// The dynamic import either succeeded or failed for one of two
7673
// distinct reasons. Distinguishing them is critical for debugging
@@ -108,7 +105,7 @@ class LazyController implements RoleBaseAccessController {
108105
"RBAC: no plugin installed (ERR_MODULE_NOT_FOUND); using fallback"
109106
);
110107
}
111-
return new RoleBaseAccessFallback(prisma).create(helpers);
108+
return new RoleBaseAccessFallback(prisma).create();
112109
}
113110
}
114111

@@ -246,10 +243,9 @@ class RoleBaseAccess {
246243
// plugin on first call.
247244
create(
248245
prisma: RbacPrismaInput,
249-
helpers: RbacHelpers,
250246
options?: RbacCreateOptions
251247
): RoleBaseAccessController {
252-
return new LazyController(prisma, helpers, options);
248+
return new LazyController(prisma, options);
253249
}
254250
}
255251

packages/plugins/src/rbac.ts

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -122,10 +122,16 @@ export interface RoleBaseAccessController {
122122
// options.allowJWT: when true, accepts PUBLIC_JWT tokens in addition to environment API keys
123123
authenticateBearer(request: Request, options?: { allowJWT?: boolean }): Promise<BearerAuthResult>;
124124

125-
// Dashboard loaders/actions (session cookie): one DB query → user + pre-built ability
125+
// Dashboard loaders/actions (session cookie): one DB query → user + pre-built ability.
126+
// The caller resolves `userId` from the session cookie and passes it in.
127+
// (`null` means "no authenticated user"; the plugin returns `{ ok: false,
128+
// reason: "unauthenticated" }`.) The plugin used to take a
129+
// `helpers.getSessionUserId(request)` callback at create-time; pulling the
130+
// userId resolution into the caller drops a static module-load coupling
131+
// from the plugin's host module to the host's session-cookie code.
126132
authenticateSession(
127133
request: Request,
128-
context: { organizationId?: string; projectId?: string }
134+
context: { userId: string | null; organizationId?: string; projectId?: string }
129135
): Promise<SessionAuthResult>;
130136

131137
// PAT-authenticated routes (Authorization: Bearer tr_pat_…). The token
@@ -154,7 +160,7 @@ export interface RoleBaseAccessController {
154160

155161
authenticateAuthorizeSession(
156162
request: Request,
157-
context: { organizationId?: string; projectId?: string },
163+
context: { userId: string | null; organizationId?: string; projectId?: string },
158164
check: { action: string; resource: RbacResource | RbacResource[] }
159165
): Promise<SessionAuthResult>;
160166

@@ -251,7 +257,5 @@ export type RoleMutationResult =
251257
export type RoleAssignmentResult = { ok: true } | { ok: false; error: string };
252258

253259
export interface RoleBasedAccessControlPlugin {
254-
create(
255-
helpers: { getSessionUserId: (request: Request) => Promise<string | null> }
256-
): RoleBaseAccessController | Promise<RoleBaseAccessController>;
260+
create(): RoleBaseAccessController | Promise<RoleBaseAccessController>;
257261
}

0 commit comments

Comments
 (0)