Skip to content

Commit 1de894a

Browse files
authored
fix(webapp): scope org settings routes to caller membership (#85)
* fix(webapp): scope org settings routes to caller membership The org settings SSO and Team routes resolved the target org from the URL slug with no membership filter, so the only actor-side gate was ability.can. That is not a tenant floor: the cloud RBAC plugin (and the OSS fallback) return a permissive ability for a non-member, so an authenticated non-member could reach these routes' loaders and actions for any org whose slug they knew, reading its SSO/directory-sync config and passing the manage:members gate on set-role. Confirmed cross-tenant on test-cloud (read of an enterprise org's full SSO/dsync config, and the set-role gate open) before this fix. Resolve the org through members: { some: { userId } } in both routes' context callbacks (new resolveOrgIdFromSlugForUser beside the unscoped resolver, and a membership filter on the sso route's local resolveOrg). A non-member now resolves to no org, which the dashboard route builder treats as no scope and fails closed. * test(webapp): cover the org-membership tenant floor on the settings resolvers Adds a postgres regression test: resolveOrgIdFromSlugForUser returns the org id for a member and null for a non-member, with the unscoped resolveOrgIdFromSlug returning the id for that same non-member to pin the gap the filter closes. Removing the members filter fails the non-member case (revert-confirmed). Both resolvers take optional replica/prisma clients (defaulting to the singletons, matching createEnvironment) so the test drives them against a real testcontainer Postgres rather than mocking. * style(test): oxfmt the org-membership resolver test
1 parent 1a16d61 commit 1de894a

4 files changed

Lines changed: 92 additions & 14 deletions

File tree

apps/webapp/app/models/organization.server.ts

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
Prisma as PrismaNamespace,
1616
prisma,
1717
type PrismaClientOrTransaction,
18+
type PrismaReplicaClient,
1819
} from "~/db.server";
1920
import { env } from "~/env.server";
2021
import { featuresForUrl } from "~/features.server";
@@ -41,22 +42,49 @@ const nanoid = customAlphabet("1234567890abcdef", 4);
4142
* miss, so replica lag never leaves a real org unresolved, which the dashboard
4243
* route builder treats as an unauthorized request.
4344
*/
44-
export async function resolveOrgIdFromSlug(slug: string): Promise<string | null> {
45-
const fromReplica = await $replica.organization.findFirst({
45+
export async function resolveOrgIdFromSlug(
46+
slug: string,
47+
replicaClient: PrismaReplicaClient = $replica,
48+
prismaClient: PrismaClientOrTransaction = prisma
49+
): Promise<string | null> {
50+
const fromReplica = await replicaClient.organization.findFirst({
4651
where: { slug },
4752
select: { id: true },
4853
});
4954
if (fromReplica) {
5055
return fromReplica.id;
5156
}
5257

53-
const fromPrimary = await prisma.organization.findFirst({
58+
const fromPrimary = await prismaClient.organization.findFirst({
5459
where: { slug },
5560
select: { id: true },
5661
});
5762
return fromPrimary?.id ?? null;
5863
}
5964

65+
/**
66+
* Like `resolveOrgIdFromSlug`, but only resolves an org the user is a member of. `ability.can` is not
67+
* a tenant floor (the OSS fallback and the cloud plugin both return a permissive ability for a
68+
* non-member), so a route that scopes only by slug lets a non-member reach the handler; the
69+
* membership filter here is the tenant floor. Returns null for a non-member, which the dashboard
70+
* route builder treats as no scope and fails closed.
71+
*/
72+
export async function resolveOrgIdFromSlugForUser(
73+
slug: string,
74+
userId: string,
75+
replicaClient: PrismaReplicaClient = $replica,
76+
prismaClient: PrismaClientOrTransaction = prisma
77+
): Promise<string | null> {
78+
const where = { slug, members: { some: { userId } } };
79+
const fromReplica = await replicaClient.organization.findFirst({ where, select: { id: true } });
80+
if (fromReplica) {
81+
return fromReplica.id;
82+
}
83+
84+
const fromPrimary = await prismaClient.organization.findFirst({ where, select: { id: true } });
85+
return fromPrimary?.id ?? null;
86+
}
87+
6088
export async function createOrganization(
6189
{
6290
title,

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

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import {
3636
import { Select, SelectItem } from "~/components/primitives/Select";
3737
import { Switch } from "~/components/primitives/Switch";
3838
import { prisma } from "~/db.server";
39+
import { getUserId } from "~/services/session.server";
3940
import { useOrganization } from "~/hooks/useOrganizations";
4041
import { rbac } from "~/services/rbac.server";
4142
import { ssoController } from "~/services/sso.server";
@@ -53,11 +54,14 @@ export const meta: MetaFunction = () => [{ title: "SSO & Directory Sync | Trigge
5354

5455
const Params = z.object({ organizationSlug: z.string() });
5556

56-
async function resolveOrg(slug: string) {
57+
async function resolveOrg(slug: string, userId: string) {
58+
// Scoped to membership: ability.can is not a tenant floor (the cloud RBAC
59+
// plugin returns a permissive ability for a non-member), so without the
60+
// members filter a non-member reaches the handler for any org slug.
5761
// Primary (not replica): this scopes the RBAC/entitlement checks, so lag
5862
// could run them against a stale/missing org.
5963
return prisma.organization.findFirst({
60-
where: { slug },
64+
where: { slug, members: { some: { userId } } },
6165
select: { id: true, title: true },
6266
});
6367
}
@@ -117,8 +121,10 @@ const EMPTY_SSO_STATUS = {
117121
export const loader = dashboardLoader(
118122
{
119123
params: Params,
120-
context: async (params) => {
121-
const org = await resolveOrg(params.organizationSlug);
124+
context: async (params, request) => {
125+
const userId = await getUserId(request);
126+
if (!userId) return {};
127+
const org = await resolveOrg(params.organizationSlug, userId);
122128
return org ? { organizationId: org.id, orgTitle: org.title } : {};
123129
},
124130
// Plan-gated before role-gated: non-Enterprise orgs render the upsell for
@@ -211,8 +217,10 @@ const ActionSchema = z.discriminatedUnion("action", [
211217
export const action = dashboardAction(
212218
{
213219
params: Params,
214-
context: async (params) => {
215-
const org = await resolveOrg(params.organizationSlug);
220+
context: async (params, request) => {
221+
const userId = await getUserId(request);
222+
if (!userId) return {};
223+
const org = await resolveOrg(params.organizationSlug, userId);
216224
return org ? { organizationId: org.id } : {};
217225
},
218226
authorization: { action: "manage", resource: { type: "sso" } },

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

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@ import { useOrganization } from "~/hooks/useOrganizations";
4343
import { useUser } from "~/hooks/useUser";
4444
import { removeTeamMember } from "~/models/removeTeamMember.server";
4545
import { redirectWithSuccessMessage } from "~/models/message.server";
46-
import { resolveOrgIdFromSlug } from "~/models/organization.server";
46+
import { resolveOrgIdFromSlugForUser } from "~/models/organization.server";
47+
import { getUserId } from "~/services/session.server";
4748
import { TeamPresenter } from "~/presenters/TeamPresenter.server";
4849
import { getCurrentPlan, getSelfServePurchaseBlockReason } from "~/services/platform.v3.server";
4950
import { rbac } from "~/services/rbac.server";
@@ -77,8 +78,10 @@ const Params = z.object({
7778
export const loader = dashboardLoader(
7879
{
7980
params: Params,
80-
context: async (params) => {
81-
const orgId = await resolveOrgIdFromSlug(params.organizationSlug);
81+
context: async (params, request) => {
82+
const userId = await getUserId(request);
83+
if (!userId) return {};
84+
const orgId = await resolveOrgIdFromSlugForUser(params.organizationSlug, userId);
8285
return orgId ? { organizationId: orgId } : {};
8386
},
8487
authorization: { action: "read", resource: { type: "members" } },
@@ -138,8 +141,10 @@ const SetRoleSchema = z.object({
138141
export const action = dashboardAction(
139142
{
140143
params: Params,
141-
context: async (params) => {
142-
const orgId = await resolveOrgIdFromSlug(params.organizationSlug);
144+
context: async (params, request) => {
145+
const userId = await getUserId(request);
146+
if (!userId) return {};
147+
const orgId = await resolveOrgIdFromSlugForUser(params.organizationSlug, userId);
143148
return orgId ? { organizationId: orgId } : {};
144149
},
145150
// No top-level authorization — different intents have different
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
import { postgresTest } from "@internal/testcontainers";
2+
import { describe, expect, vi } from "vitest";
3+
4+
vi.setConfig({ testTimeout: 60_000 });
5+
import { resolveOrgIdFromSlug, resolveOrgIdFromSlugForUser } from "~/models/organization.server";
6+
import {
7+
createTestOrgProjectWithMember,
8+
createTestUser,
9+
} from "./fixtures/environmentVariablesFixtures";
10+
11+
// The org settings routes resolve their org through this helper, so a non-member resolving to null
12+
// is what makes the dashboard route builder fail closed. ability.can is not a tenant floor (the RBAC
13+
// plugin and the OSS fallback both return a permissive ability for a non-member), so without the
14+
// membership filter a non-member reached those routes for any org whose slug they knew: a live
15+
// cross-tenant read of SSO/directory-sync config and an open set-role gate, confirmed on test-cloud.
16+
describe("resolveOrgIdFromSlugForUser", () => {
17+
postgresTest("resolves an org the user is a member of", async ({ prisma }) => {
18+
const { user, organization } = await createTestOrgProjectWithMember(prisma);
19+
20+
const resolved = await resolveOrgIdFromSlugForUser(organization.slug, user.id, prisma, prisma);
21+
22+
expect(resolved).toBe(organization.id);
23+
});
24+
25+
postgresTest("returns null for a non-member, the tenant floor", async ({ prisma }) => {
26+
const { organization: target } = await createTestOrgProjectWithMember(prisma);
27+
const outsider = await createTestUser(prisma);
28+
29+
const resolved = await resolveOrgIdFromSlugForUser(target.slug, outsider.id, prisma, prisma);
30+
31+
// The unscoped resolver still hands the same non-member the org id: this is the exact gap the
32+
// membership filter closes, and why scoping by slug alone was the hole.
33+
const unscoped = await resolveOrgIdFromSlug(target.slug, prisma, prisma);
34+
expect(unscoped).toBe(target.id);
35+
expect(resolved).toBeNull();
36+
});
37+
});

0 commit comments

Comments
 (0)