Skip to content

Commit f7a8781

Browse files
carderneTrigger.dev RepoOps
authored andcommitted
fix(webapp): rotate development API keys on member removal
Rotate a member's development root API keys and revoke their additional development keys when they are removed from an organization. Credential changes and removal commit together, preserving environment history. Mono-RevId: 916f71fa04cb2c3f8f6ca8772bcda2d4e32a19f6
1 parent 02a27b5 commit f7a8781

8 files changed

Lines changed: 231 additions & 25 deletions

‎apps/webapp/app/models/api-key.server.ts‎

Lines changed: 43 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ export async function createEnvironmentApiKey(
226226
rbacController = rbac,
227227
telemetryRecorder = apiKeyTelemetry,
228228
}: {
229-
prismaClient?: Pick<PrismaClient, "apiKey" | "runtimeEnvironment" | "taskIdentifier">;
229+
prismaClient?: PrismaClient;
230230
rbacController?: Pick<HostRbacController, "prepareApiKeyPolicy">;
231231
telemetryRecorder?: ApiKeyTelemetry;
232232
} = {}
@@ -289,23 +289,54 @@ export async function createEnvironmentApiKey(
289289
const generated = generateAdditionalApiKey(environment.type);
290290
const apiKey = await (async () => {
291291
try {
292-
return await prismaClient.apiKey.create({
293-
data: {
294-
name,
295-
keyHash: generated.keyHash,
296-
lastFour: generated.lastFour,
297-
runtimeEnvironmentId: environment.id,
298-
createdByUserId: userId,
299-
expiresAt,
300-
presetId: prepared.policy.presetId,
301-
scopes: prepared.policy.scopes,
292+
const create = (client: Pick<PrismaClient, "apiKey">) =>
293+
client.apiKey.create({
294+
data: {
295+
name,
296+
keyHash: generated.keyHash,
297+
lastFour: generated.lastFour,
298+
runtimeEnvironmentId: environment.id,
299+
createdByUserId: userId,
300+
expiresAt,
301+
presetId: prepared.policy.presetId,
302+
scopes: prepared.policy.scopes,
303+
},
304+
});
305+
if (environment.type !== "DEVELOPMENT") {
306+
return await create(prismaClient);
307+
}
308+
const created = await $transaction(
309+
prismaClient,
310+
"create development API key",
311+
async (tx) => {
312+
const members = await tx.$queryRaw<Array<{ id: string }>>`
313+
SELECT m."id" FROM "OrgMember" m
314+
JOIN "RuntimeEnvironment" e ON e."orgMemberId" = m."id"
315+
WHERE e."id" = ${environment.id}
316+
AND m."organizationId" = ${environment.organizationId}
317+
AND m."userId" = ${userId}
318+
FOR UPDATE OF m
319+
`;
320+
if (members.length === 0) {
321+
return null;
322+
}
323+
return create(tx);
302324
},
303-
});
325+
{ isolationLevel: "Serializable" }
326+
);
327+
if (created === undefined) {
328+
throw new Error("Failed to create development API key");
329+
}
330+
return created;
304331
} catch (error) {
305332
telemetryRecorder.recordOperation("create", "error", "database_error");
306333
throw error;
307334
}
308335
})();
336+
if (apiKey === null) {
337+
telemetryRecorder.recordOperation("create", "rejected", "membership_removed");
338+
throw new Error("Environment not found");
339+
}
309340
telemetryRecorder.recordOperation("create", "success");
310341

311342
return { apiKey, plaintext: generated.apiKey };
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import type { PrismaTransactionClient } from "@trigger.dev/database";
2+
import { generateRootApiKey } from "~/utils/apiKeys";
3+
4+
export async function deleteOrgMember(
5+
tx: PrismaTransactionClient,
6+
member: { id: string; organizationId: string }
7+
) {
8+
// Prevent new development environments from being provisioned during removal.
9+
await tx.$queryRaw`
10+
SELECT "id" FROM "OrgMember"
11+
WHERE "id" = ${member.id} AND "organizationId" = ${member.organizationId}
12+
FOR UPDATE
13+
`;
14+
15+
const environments = await tx.runtimeEnvironment.findMany({
16+
where: {
17+
organizationId: member.organizationId,
18+
orgMemberId: member.id,
19+
type: "DEVELOPMENT",
20+
},
21+
select: { id: true },
22+
});
23+
for (const environment of environments) {
24+
await tx.runtimeEnvironment.update({
25+
where: { id: environment.id },
26+
data: {
27+
apiKey: generateRootApiKey("DEVELOPMENT").apiKey,
28+
apiKeys: {
29+
updateMany: { where: { revokedAt: null }, data: { revokedAt: new Date() } },
30+
},
31+
},
32+
});
33+
}
34+
return tx.orgMember.delete({ where: { id: member.id, organizationId: member.organizationId } });
35+
}

‎apps/webapp/app/models/orgMember.server.ts‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import { Prisma, prisma } from "~/db.server";
1+
import { $transaction, Prisma, prisma } from "~/db.server";
2+
import { deleteOrgMember } from "./deleteOrgMember.server";
23
import type { MembershipSource } from "~/models/member.server";
34
import { logger } from "~/services/logger.server";
45
import { enqueueMemberDevelopmentEnvironments } from "~/services/memberDevEnvironments.server";
@@ -8,6 +9,15 @@ import {
89
revokePersonalAccessToken,
910
} from "~/services/personalAccessToken.server";
1011

12+
async function deleteMembership(memberId: string, organizationId: string) {
13+
const result = await $transaction(prisma, "delete org member", (tx) =>
14+
deleteOrgMember(tx, { id: memberId, organizationId })
15+
);
16+
if (!result) {
17+
throw new Error("Failed to delete organization member");
18+
}
19+
}
20+
1121
export type EnsureOrgMemberParams = {
1222
userId: string;
1323
organizationId: string;
@@ -150,7 +160,7 @@ export async function ensureOrgMember(
150160
roleId,
151161
error: result.error,
152162
});
153-
await prisma.orgMember.delete({ where: { id: member.id } });
163+
await deleteMembership(member.id, organizationId);
154164
throw new Error(`ensureOrgMember: failed to apply role ${roleId}: ${result.error}`);
155165
}
156166
}
@@ -257,7 +267,7 @@ export async function removeOrgMemberForDirectory(params: {
257267
}
258268
}
259269

260-
await prisma.orgMember.delete({ where: { id: member.id } });
270+
await deleteMembership(member.id, organizationId);
261271
const removeRole = await rbac.removeUserRole({ userId, organizationId });
262272
if (!removeRole.ok) {
263273
logger.warn("removeOrgMemberForDirectory: failed to remove RBAC role", {

‎apps/webapp/app/models/removeTeamMember.server.ts‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
11
import type { PrismaClient } from "@trigger.dev/database";
2+
import { $transaction } from "~/db.server";
23
import { ServiceValidationError } from "~/v3/services/common.server";
3-
4-
// Leaf module with a type-only Prisma import (caller passes the client) so it
5-
// can be unit-tested without importing `~/db.server`, which eagerly connects
6-
// the global prisma singleton. ServiceValidationError is a plain error class
7-
// with no imports, so it stays leaf-safe and lets callers map it to a status.
4+
import { deleteOrgMember } from "./deleteOrgMember.server";
85
export async function removeTeamMember(
96
{
107
userId,
@@ -28,9 +25,10 @@ export async function removeTeamMember(
2825
// Serializable so the "keep at least one member" check and the delete are
2926
// atomic: at ReadCommitted two concurrent removals could each see >1 member
3027
// and both delete, orphaning the org. The guard lives here, not per-caller,
31-
// so every surface (dashboard + management API) is TOCTOU-safe. Raw
32-
// $transaction (not the ~/db.server helper) keeps this module leaf/testable.
33-
return prismaClient.$transaction(
28+
// so every surface (dashboard + management API) is TOCTOU-safe.
29+
const result = await $transaction(
30+
prismaClient,
31+
"remove team member",
3432
async (tx) => {
3533
// Scope both the lookup and the delete to org.id, so the member id is
3634
// only ever resolved within the actor's organization.
@@ -48,9 +46,13 @@ export async function removeTeamMember(
4846
throw new ServiceValidationError("Cannot remove the last member of an organization", 400);
4947
}
5048

51-
await tx.orgMember.delete({ where: { id: target.id } });
49+
await deleteOrgMember(tx, { id: target.id, organizationId: org.id });
5250
return target;
5351
},
5452
{ isolationLevel: "Serializable" }
5553
);
54+
if (!result) {
55+
throw new Error("Failed to remove organization member");
56+
}
57+
return result;
5658
}

‎apps/webapp/app/services/apiKeyTelemetry.server.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ export type ApiKeyOperationResult = "success" | "rejected" | "error";
66
export type ApiKeyOperationReason =
77
| "none"
88
| "database_error"
9+
| "membership_removed"
910
| "not_found_or_revoked"
1011
| "policy_rejected"
1112
| "policy_error";

‎apps/webapp/test/createEnvironmentApiKey.test.ts‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,12 @@ import rbacPlugin, { type RoleBaseAccessController } from "@trigger.dev/rbac";
44
import { expect, vi } from "vitest";
55
import { MAX_API_KEY_TASK_IDENTIFIERS } from "~/consts";
66
import { createEnvironmentApiKey, revokeEnvironmentApiKey } from "~/models/api-key.server";
7+
import { removeTeamMember } from "~/models/removeTeamMember.server";
78
import type { ApiKeyTelemetry } from "~/services/apiKeyTelemetry.server";
89
import {
910
createRuntimeEnvironment,
1011
createTestOrgProjectWithMember,
12+
createTestUser,
1113
uniqueId,
1214
} from "./fixtures/environmentVariablesFixtures";
1315

@@ -68,6 +70,70 @@ containerTest("standalone fallback creates one explicit full-access key", async
6870
).resolves.toBe(1);
6971
});
7072

73+
containerTest("cannot create a development key after its member is removed", async ({ prisma }) => {
74+
const { organization, project, user, orgMember } = await createTestOrgProjectWithMember(prisma);
75+
const admin = await createTestUser(prisma);
76+
await prisma.orgMember.create({
77+
data: { userId: admin.id, organizationId: organization.id, role: "ADMIN" },
78+
});
79+
const environment = await createRuntimeEnvironment(prisma, {
80+
projectId: project.id,
81+
organizationId: organization.id,
82+
orgMemberId: orgMember.id,
83+
type: "DEVELOPMENT",
84+
});
85+
const fallback = rbacPlugin.create(prisma, { forceFallback: true });
86+
let policyStarted!: () => void;
87+
let resumePolicy!: () => void;
88+
const started = new Promise<void>((resolve) => {
89+
policyStarted = resolve;
90+
});
91+
const resume = new Promise<void>((resolve) => {
92+
resumePolicy = resolve;
93+
});
94+
const telemetry = telemetryRecorder();
95+
const creation = createEnvironmentApiKey(
96+
{
97+
environmentId: environment.id,
98+
taskEnvironmentId: environment.id,
99+
userId: user.id,
100+
name: "Racing removal",
101+
presetId: "FULL_ACCESS",
102+
},
103+
{
104+
prismaClient: prisma,
105+
telemetryRecorder: telemetry,
106+
rbacController: {
107+
async prepareApiKeyPolicy(input) {
108+
policyStarted();
109+
await resume;
110+
return fallback.prepareApiKeyPolicy(input);
111+
},
112+
},
113+
}
114+
);
115+
const rejected = expect(creation).rejects.toThrow("Environment not found");
116+
await started;
117+
try {
118+
await removeTeamMember(
119+
{ userId: admin.id, slug: organization.slug, memberId: orgMember.id },
120+
prisma
121+
);
122+
} finally {
123+
resumePolicy();
124+
}
125+
await rejected;
126+
expect(telemetry.recordOperation).toHaveBeenCalledWith(
127+
"create",
128+
"rejected",
129+
"membership_removed"
130+
);
131+
expect(telemetry.recordOperation).not.toHaveBeenCalledWith("create", "error", "database_error");
132+
await expect(
133+
prisma.apiKey.findFirst({ where: { runtimeEnvironmentId: environment.id, revokedAt: null } })
134+
).resolves.toBeNull();
135+
});
136+
71137
containerTest("records successful API key revocation", async ({ prisma }) => {
72138
const { user, environment } = await setup(prisma);
73139
const apiKey = await prisma.apiKey.create({

‎apps/webapp/test/orgMember.server.test.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { randomBytes } from "node:crypto";
22
import { beforeEach, describe, expect, vi } from "vitest";
3-
import type { PrismaClient } from "@trigger.dev/database";
3+
import type { PrismaClient, PrismaTransactionClient } from "@trigger.dev/database";
44

55
const prismaHolder = vi.hoisted(() => ({
66
client: null as PrismaClient | null,
@@ -37,6 +37,11 @@ vi.mock("~/db.server", async () => {
3737

3838
return {
3939
Prisma,
40+
$transaction: (
41+
client: PrismaClient,
42+
_name: string,
43+
callback: (tx: PrismaTransactionClient) => Promise<unknown>
44+
) => client.$transaction(callback),
4045
get prisma() {
4146
if (!prismaHolder.client) {
4247
throw new Error("test prisma not set");

‎apps/webapp/test/removeTeamMember.test.ts‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@ import { containerTest } from "@internal/testcontainers";
22
import type { PrismaClient } from "@trigger.dev/database";
33
import { describe, expect, vi } from "vitest";
44
import { removeTeamMember } from "~/models/removeTeamMember.server";
5+
import rbacLoader from "@trigger.dev/rbac";
6+
import { generateAdditionalApiKey } from "~/utils/apiKeys";
7+
import { createRuntimeEnvironment, uniqueId } from "./fixtures/environmentVariablesFixtures";
58

69
vi.setConfig({ testTimeout: 60_000 });
710

@@ -36,6 +39,59 @@ async function seedOrgWithMembers(prisma: PrismaClient, slugBase: string) {
3639
}
3740

3841
describe("removeTeamMember", () => {
42+
containerTest(
43+
"rotates the removed member's root key and revokes additional development keys",
44+
async ({ prisma }) => {
45+
const a = await seedOrgWithMembers(prisma, "rotation");
46+
const project = await prisma.project.create({
47+
data: {
48+
name: "Test project",
49+
slug: uniqueId("project"),
50+
externalRef: uniqueId("project-ref"),
51+
organizationId: a.organization.id,
52+
},
53+
});
54+
const environment = await createRuntimeEnvironment(prisma, {
55+
projectId: project.id,
56+
organizationId: a.organization.id,
57+
orgMemberId: a.regularMember.id,
58+
type: "DEVELOPMENT",
59+
});
60+
const additionalKey = generateAdditionalApiKey("DEVELOPMENT");
61+
await prisma.apiKey.create({
62+
data: {
63+
name: "Development key",
64+
runtimeEnvironmentId: environment.id,
65+
keyHash: additionalKey.keyHash,
66+
lastFour: additionalKey.lastFour,
67+
scopes: ["admin"],
68+
},
69+
});
70+
const controller = rbacLoader.create(prisma, { forceFallback: true });
71+
const authenticate = (key: string) =>
72+
controller.authenticateBearer(
73+
new Request("https://example.com", {
74+
headers: { Authorization: `Bearer ${key}` },
75+
})
76+
);
77+
for (const key of [environment.apiKey, additionalKey.apiKey]) {
78+
await expect(authenticate(key)).resolves.toMatchObject({ ok: true });
79+
}
80+
81+
await removeTeamMember(
82+
{ userId: a.admin.id, slug: a.organization.slug, memberId: a.regularMember.id },
83+
prisma
84+
);
85+
86+
for (const key of [environment.apiKey, additionalKey.apiKey]) {
87+
await expect(authenticate(key)).resolves.toMatchObject({ ok: false, status: 401 });
88+
}
89+
await expect(
90+
prisma.runtimeEnvironment.findFirst({ where: { id: environment.id } })
91+
).resolves.toMatchObject({ id: environment.id, orgMemberId: null });
92+
}
93+
);
94+
3995
containerTest(
4096
"refuses to delete an OrgMember that belongs to a different org",
4197
async ({ prisma }) => {

0 commit comments

Comments
 (0)