Skip to content

Commit 0eca444

Browse files
authored
fix(rbac): deny a non-member session in an org context (OSS fallback) (#86)
1 parent 5e22346 commit 0eca444

3 files changed

Lines changed: 229 additions & 0 deletions

File tree

apps/webapp/test/auth-dashboard.e2e.full.test.ts

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
// Each test seeds a User + session cookie via seedTestUser / seedTestSession
33
// (helpers/seedTestSession.ts) and hits the shared webapp container.
44

5+
import { randomBytes } from "node:crypto";
6+
import type { PrismaClient } from "@trigger.dev/database";
57
import { describe, expect, it } from "vitest";
68
import { getTestServer } from "./helpers/sharedTestServer";
79
import { seedTestSession, seedTestUser } from "./helpers/seedTestSession";
@@ -115,4 +117,66 @@ describe("Dashboard", () => {
115117
expect(new URL(location, "http://localhost").pathname).toBe("/");
116118
});
117119
});
120+
121+
// Cross-tenant tenant floor on org settings routes. settings/roles is the case
122+
// the route-level membership scoping (SSO/Team) did NOT cover, so it exercises
123+
// the RBAC fallback's org-membership floor specifically: the fallback ability
124+
// is permissive (can: () => true), so that floor is the only thing stopping a
125+
// non-member from reading the org's role and permission catalogue.
126+
//
127+
// The request hits the route's own loader directly via Remix's `?_data`, which
128+
// is the exact exploit shape: a plain document GET 404s at the org layout
129+
// (membership) and never reaches this leaf, so it wouldn't test the leaf floor.
130+
// Both users have confirmedBasicDetails set so the `_app` onboarding redirect
131+
// can't stand in for the deny.
132+
describe("Org settings — cross-tenant tenant floor (settings/roles)", () => {
133+
const ROLES_ROUTE_ID = "routes/_app.orgs.$organizationSlug.settings.roles";
134+
const rolesData = (slug: string) =>
135+
`/orgs/${slug}/settings/roles?_data=${encodeURIComponent(ROLES_ROUTE_ID)}`;
136+
137+
async function seedConfirmedUser(prisma: PrismaClient) {
138+
const user = await seedTestUser(prisma);
139+
await prisma.user.update({ where: { id: user.id }, data: { confirmedBasicDetails: true } });
140+
return user;
141+
}
142+
143+
async function seedOrgWithOwner() {
144+
const server = getTestServer();
145+
const owner = await seedConfirmedUser(server.prisma);
146+
const org = await server.prisma.organization.create({
147+
data: {
148+
title: "E2E tenant-floor org",
149+
slug: `e2e-tenant-${randomBytes(6).toString("hex")}`,
150+
members: { create: { userId: owner.id, role: "ADMIN" } },
151+
},
152+
});
153+
return { server, owner, org };
154+
}
155+
156+
it("denies a non-member: no roles catalogue leaked", async () => {
157+
const { server, org } = await seedOrgWithOwner();
158+
const outsider = await seedConfirmedUser(server.prisma);
159+
const cookie = await seedTestSession({ userId: outsider.id });
160+
const res = await server.webapp.fetch(rolesData(org.slug), {
161+
redirect: "manual",
162+
headers: { Cookie: cookie },
163+
});
164+
const body = await res.text();
165+
// With the tenant floor a non-member is denied (a redirect), so they never
166+
// get the loader's 200 payload. Before the fix the permissive ability let
167+
// the loader return the org's role/permission catalogue.
168+
expect(res.status).not.toBe(200);
169+
expect(body).not.toContain("manage:members");
170+
});
171+
172+
it("allows a member: the loader returns the catalogue", async () => {
173+
const { server, owner, org } = await seedOrgWithOwner();
174+
const cookie = await seedTestSession({ userId: owner.id });
175+
const res = await server.webapp.fetch(rolesData(org.slug), {
176+
redirect: "manual",
177+
headers: { Cookie: cookie },
178+
});
179+
expect(res.status).toBe(200);
180+
});
181+
});
118182
});
Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
import { postgresTest } from "@internal/testcontainers";
2+
import plugin from "@trigger.dev/rbac";
3+
import { type PrismaClient } from "@trigger.dev/database";
4+
import { describe, expect, vi } from "vitest";
5+
import {
6+
createTestOrgProjectWithMember,
7+
createTestUser,
8+
} from "./fixtures/environmentVariablesFixtures";
9+
10+
vi.setConfig({ testTimeout: 60_000 });
11+
12+
// The RBAC fallback ability is permissive (`can: () => true` for a non-admin), so
13+
// `ability.can` is not a tenant floor. `authenticateSession` is the gate every
14+
// org-scoped dashboard route relies on; a non-member in an org context must be
15+
// denied here, or a permissive ability lets them act on any org whose slug they
16+
// know. The route-level e2e (auth-dashboard.e2e.full) covers the HTTP path; this
17+
// pins the fallback gate directly since that path can't run without a container.
18+
function fallback(prisma: PrismaClient) {
19+
// forceFallback skips the closed-source plugin and uses the in-repo fallback.
20+
return plugin.create({ primary: prisma, replica: prisma }, { forceFallback: true });
21+
}
22+
23+
const request = new Request("https://app.trigger.dev/orgs/x/settings/roles");
24+
25+
describe("RBAC fallback authenticateSession — org membership floor", () => {
26+
postgresTest("denies a non-member in an org context", async ({ prisma }) => {
27+
const { organization } = await createTestOrgProjectWithMember(prisma);
28+
const outsider = await createTestUser(prisma);
29+
30+
const result = await fallback(prisma).authenticateSession(request, {
31+
userId: outsider.id,
32+
organizationId: organization.id,
33+
});
34+
35+
expect(result).toMatchObject({ ok: false, reason: "unauthorized" });
36+
});
37+
38+
postgresTest("allows a member in an org context", async ({ prisma }) => {
39+
const { user, organization } = await createTestOrgProjectWithMember(prisma);
40+
41+
const result = await fallback(prisma).authenticateSession(request, {
42+
userId: user.id,
43+
organizationId: organization.id,
44+
});
45+
46+
expect(result.ok).toBe(true);
47+
});
48+
49+
postgresTest(
50+
"stays permissive with no org context, even for a non-member",
51+
async ({ prisma }) => {
52+
// Identity-only checks (no organizationId) predate any scope, so the floor
53+
// does not apply and the permissive baseline is preserved.
54+
const outsider = await createTestUser(prisma);
55+
56+
const result = await fallback(prisma).authenticateSession(request, { userId: outsider.id });
57+
58+
expect(result.ok).toBe(true);
59+
}
60+
);
61+
62+
// A project-only scope is still a tenant claim, so the floor resolves the
63+
// project's organization rather than letting the context through unchecked.
64+
postgresTest("denies a non-member scoped only to a project", async ({ prisma }) => {
65+
const { project } = await createTestOrgProjectWithMember(prisma);
66+
const outsider = await createTestUser(prisma);
67+
68+
const result = await fallback(prisma).authenticateSession(request, {
69+
userId: outsider.id,
70+
projectId: project.id,
71+
});
72+
73+
expect(result).toMatchObject({ ok: false, reason: "unauthorized" });
74+
});
75+
76+
postgresTest("allows a member scoped only to a project", async ({ prisma }) => {
77+
const { user, project } = await createTestOrgProjectWithMember(prisma);
78+
79+
const result = await fallback(prisma).authenticateSession(request, {
80+
userId: user.id,
81+
projectId: project.id,
82+
});
83+
84+
expect(result.ok).toBe(true);
85+
});
86+
87+
// The membership probe reads the replica first and the primary on a miss, so a
88+
// member whose row has not replicated yet is not bounced. Modelled by giving
89+
// the controller a replica that cannot see the row and a primary that can.
90+
postgresTest("allows a member the replica has not caught up on", async ({ prisma }) => {
91+
const { user, organization } = await createTestOrgProjectWithMember(prisma);
92+
const blindReplica = {
93+
...prisma,
94+
orgMember: { findFirst: async () => null },
95+
user: prisma.user,
96+
project: prisma.project,
97+
} as unknown as PrismaClient;
98+
99+
const controller = plugin.create(
100+
{ primary: prisma, replica: blindReplica },
101+
{ forceFallback: true }
102+
);
103+
const result = await controller.authenticateSession(request, {
104+
userId: user.id,
105+
organizationId: organization.id,
106+
});
107+
108+
expect(result.ok).toBe(true);
109+
});
110+
});

internal-packages/rbac/src/fallback.ts

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,21 @@ class RoleBaseAccessFallbackController implements RoleBaseAccessController {
9898
const user = await this.replica.user.findFirst({ where: { id: context.userId } });
9999
if (!user) return { ok: false, reason: "unauthenticated" };
100100

101+
// A non-member in a scoped context is denied, not handed a permissive
102+
// ability. buildFallbackAbility is permissive for a non-admin
103+
// (can: () => true), so ability.can is not a tenant floor; returning it here
104+
// let a non-member act on any org whose slug they knew. An unscoped context
105+
// stays permissive (identity-only checks predate any scope), and a platform
106+
// admin keeps their ability.
107+
if (!user.admin) {
108+
const denied = await this.deniedByMembership(
109+
context.organizationId,
110+
context.projectId,
111+
user.id
112+
);
113+
if (denied) return { ok: false, reason: "unauthorized" };
114+
}
115+
101116
const subject: RbacSubject = {
102117
type: "user",
103118
userId: user.id,
@@ -113,6 +128,46 @@ class RoleBaseAccessFallbackController implements RoleBaseAccessController {
113128
};
114129
}
115130

131+
/**
132+
* Whether a non-admin user is outside the tenant a scoped context names. A project-only scope
133+
* resolves through the project's organization, so the floor holds whichever scope a route
134+
* resolves; an unscoped context is not a tenant claim and is never denied here.
135+
*
136+
* Both lookups read the replica first and fall back to the primary before denying: org creation,
137+
* invite acceptance and SSO provisioning all write to the primary, so a member who just joined
138+
* must not be bounced while the row replicates.
139+
*/
140+
private async deniedByMembership(
141+
organizationId: string | undefined,
142+
projectId: string | undefined,
143+
userId: string
144+
): Promise<boolean> {
145+
let orgId = organizationId;
146+
147+
if (!orgId && projectId) {
148+
const project =
149+
(await this.replica.project.findFirst({
150+
where: { id: projectId },
151+
select: { organizationId: true },
152+
})) ??
153+
(await this.prisma.project.findFirst({
154+
where: { id: projectId },
155+
select: { organizationId: true },
156+
}));
157+
// An unresolvable project names no tenant, so there is nothing to deny against.
158+
if (!project) return false;
159+
orgId = project.organizationId;
160+
}
161+
162+
if (!orgId) return false;
163+
164+
const where = { organizationId: orgId, userId };
165+
const member =
166+
(await this.replica.orgMember.findFirst({ where, select: { id: true } })) ??
167+
(await this.prisma.orgMember.findFirst({ where, select: { id: true } }));
168+
return !member;
169+
}
170+
116171
async authenticateAuthorizeBearer(
117172
request: Request,
118173
check: { action: string; resource: RbacResource | RbacResource[] },

0 commit comments

Comments
 (0)