Skip to content

Commit 68d26b0

Browse files
committed
fixes for review comments
1 parent c423c4f commit 68d26b0

8 files changed

Lines changed: 146 additions & 234 deletions

File tree

apps/webapp/app/presenters/v3/ApiKeysPresenter.server.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ export class ApiKeysPresenter {
6969
const keyEnvironmentId = environment.parentEnvironmentId ?? environment.id;
7070

7171
const [keyEnvironment, vercelIntegration] = await Promise.all([
72-
this.#prismaClient.runtimeEnvironment.findUniqueOrThrow({
72+
this.#prismaClient.runtimeEnvironment.findFirstOrThrow({
7373
where: { id: keyEnvironmentId },
7474
select: {
7575
id: true,

apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.apikeys/route.tsx

Lines changed: 4 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,11 @@ import { resolveOrgIdFromSlug } from "~/models/organization.server";
7575
import { findProjectBySlug } from "~/models/project.server";
7676
import { findEnvironmentBySlug } from "~/models/runtimeEnvironment.server";
7777
import { ApiKeysPresenter } from "~/presenters/v3/ApiKeysPresenter.server";
78-
import { FULL_ACCESS_PRESET_ID } from "@trigger.dev/rbac";
7978
import { canIssueAdditionalApiKeys } from "~/services/additionalApiKeyIssuance.server";
79+
import {
80+
validateCreateApiKeyPreset,
81+
type ApiKeyPreset,
82+
} from "~/services/apiKeyPresetValidation.server";
8083
import { rbac } from "~/services/rbac.server";
8184
import { dashboardAction, dashboardLoader } from "~/services/routeBuilders/dashboardBuilder";
8285
import { cn } from "~/utils/cn";
@@ -88,8 +91,6 @@ const ApiKeySearchParams = z.object({
8891
showRevoked: z.preprocess((value) => value === "true" || value === true, z.boolean()).optional(),
8992
});
9093

91-
type ApiKeyPreset = NonNullable<Awaited<ReturnType<typeof rbac.apiKeyPresets>>>[number];
92-
9394
const CreateApiKeySchema = z.object({
9495
action: z.literal("create"),
9596
name: z.string().trim().min(1).max(64),
@@ -115,59 +116,6 @@ const ApiKeyActionSchema = z.discriminatedUnion("action", [
115116
z.object({ action: z.literal("revoke"), apiKeyId: z.string().min(1) }),
116117
]);
117118

118-
function validateCreateApiKeyPreset({
119-
presets,
120-
presetId,
121-
taskScope,
122-
taskIdentifiers,
123-
hasTaskParameters,
124-
}: {
125-
presets: ApiKeyPreset[] | null;
126-
presetId?: string;
127-
taskScope?: "all" | "selected";
128-
taskIdentifiers: string[];
129-
hasTaskParameters: boolean;
130-
}): { presetId: string; usesTaskSelection: boolean } {
131-
// Always resolves to a concrete preset id. "No preset chosen" means full
132-
// access, and saying so here keeps that decision visible at the call site
133-
// instead of relying on a default inside prepareApiKeyPolicy.
134-
const fullAccess = { presetId: FULL_ACCESS_PRESET_ID, usesTaskSelection: false };
135-
136-
if (presets === null) {
137-
if (presetId !== undefined || hasTaskParameters) {
138-
throw new Error("API key access presets are not available");
139-
}
140-
return fullAccess;
141-
}
142-
143-
if (!presetId) {
144-
if (hasTaskParameters) {
145-
throw new Error("A preset is required when selecting tasks");
146-
}
147-
return fullAccess;
148-
}
149-
150-
const preset = presets.find((candidate) => candidate.id === presetId);
151-
if (!preset) {
152-
throw new Error("Invalid API key access preset");
153-
}
154-
if (!preset.available) {
155-
throw new Error("This API key access preset is not available on your plan");
156-
}
157-
158-
if (!preset.usesTaskSelection && hasTaskParameters) {
159-
throw new Error("This API key access preset does not support task selection");
160-
}
161-
if (preset.usesTaskSelection && taskScope === "selected" && taskIdentifiers.length === 0) {
162-
throw new Error("Select at least one task");
163-
}
164-
if (preset.usesTaskSelection && taskScope !== "selected" && taskIdentifiers.length > 0) {
165-
throw new Error("Task identifiers require selected task scope");
166-
}
167-
168-
return { presetId: preset.id, usesTaskSelection: preset.usesTaskSelection ?? false };
169-
}
170-
171119
type ApiKeyActionData =
172120
| { ok: true; action: "create"; apiKey: string }
173121
| { ok: false; error: string };

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ export async function canIssueAdditionalApiKeys(
1010
prismaClient: IssuancePrismaClient = prisma
1111
): Promise<boolean> {
1212
const [organization, globalFlags] = await Promise.all([
13-
prismaClient.organization.findUnique({
13+
prismaClient.organization.findFirst({
1414
where: { id: organizationId },
1515
select: { featureFlags: true },
1616
}),

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

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -299,7 +299,9 @@ export async function authenticateApiKeyWithScope(
299299
action,
300300
resource,
301301
allowJWT = false,
302-
}: { action: string; resource: RbacResource; allowJWT?: boolean }
302+
}: { action: string; resource: RbacResource; allowJWT?: boolean },
303+
authorizeBearer: typeof authenticateAuthorizeBearerWithTelemetry =
304+
authenticateAuthorizeBearerWithTelemetry
303305
): Promise<
304306
| { ok: true; authentication: ApiAuthenticationResultSuccess }
305307
| { ok: false; status: 401 | 403; error: string }
@@ -309,11 +311,7 @@ export async function authenticateApiKeyWithScope(
309311
return { ok: false, status: 401, error: "Invalid or Missing API key" };
310312
}
311313

312-
const result = await authenticateAuthorizeBearerWithTelemetry(
313-
request,
314-
{ action, resource },
315-
{ allowJWT }
316-
);
314+
const result = await authorizeBearer(request, { action, resource }, { allowJWT });
317315
if (!result.ok) {
318316
return result;
319317
}
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
import { FULL_ACCESS_PRESET_ID } from "@trigger.dev/rbac";
2+
3+
export type ApiKeyPreset = {
4+
id: string;
5+
available: boolean;
6+
label: string;
7+
description: string;
8+
usesTaskSelection?: boolean;
9+
};
10+
11+
export function validateCreateApiKeyPreset({
12+
presets,
13+
presetId,
14+
taskScope,
15+
taskIdentifiers,
16+
hasTaskParameters,
17+
}: {
18+
presets: ApiKeyPreset[] | null;
19+
presetId?: string;
20+
taskScope?: "all" | "selected";
21+
taskIdentifiers: string[];
22+
hasTaskParameters: boolean;
23+
}): { presetId: string; usesTaskSelection: boolean } {
24+
const fullAccess = { presetId: FULL_ACCESS_PRESET_ID, usesTaskSelection: false };
25+
26+
if (presets === null) {
27+
if (presetId !== undefined || hasTaskParameters) {
28+
throw new Error("API key access presets are not available");
29+
}
30+
return fullAccess;
31+
}
32+
33+
if (!presetId) {
34+
if (hasTaskParameters) {
35+
throw new Error("A preset is required when selecting tasks");
36+
}
37+
return fullAccess;
38+
}
39+
40+
const preset = presets.find((candidate) => candidate.id === presetId);
41+
if (!preset) {
42+
throw new Error("Invalid API key access preset");
43+
}
44+
if (!preset.available) {
45+
throw new Error("This API key access preset is not available on your plan");
46+
}
47+
48+
if (!preset.usesTaskSelection && hasTaskParameters) {
49+
throw new Error("This API key access preset does not support task selection");
50+
}
51+
if (preset.usesTaskSelection && taskScope === "selected" && taskIdentifiers.length === 0) {
52+
throw new Error("Select at least one task");
53+
}
54+
if (preset.usesTaskSelection && taskScope !== "selected" && taskIdentifiers.length > 0) {
55+
throw new Error("Task identifiers require selected task scope");
56+
}
57+
58+
return { presetId: preset.id, usesTaskSelection: preset.usesTaskSelection ?? false };
59+
}

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

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,18 @@ export function presentedApiKeyFromAuthentication(
3232
* Keep PAT/OAT authentication on the legacy path while routing machine API
3333
* keys through the RBAC controller, where plugin grants are applied.
3434
*/
35+
type AuthenticationDependencies = {
36+
authenticateRequest: typeof authenticateRequest;
37+
authenticateApiKeyWithScope: typeof authenticateApiKeyWithScope;
38+
};
39+
3540
export async function authenticateEnvironmentScopedApiRequest(
3641
request: Request,
3742
action: "read" | "write",
38-
resource: EnvironmentScopedResource
43+
resource: EnvironmentScopedResource,
44+
dependencies: AuthenticationDependencies = { authenticateRequest, authenticateApiKeyWithScope }
3945
): Promise<EnvironmentScopedAuthentication> {
40-
const userOrOrganizationAuthentication = await authenticateRequest(request, {
46+
const userOrOrganizationAuthentication = await dependencies.authenticateRequest(request, {
4147
personalAccessToken: true,
4248
organizationAccessToken: true,
4349
apiKey: false,
@@ -46,7 +52,7 @@ export async function authenticateEnvironmentScopedApiRequest(
4652
return { ok: true, authentication: userOrOrganizationAuthentication };
4753
}
4854

49-
const apiKeyAuthentication = await authenticateApiKeyWithScope(request, {
55+
const apiKeyAuthentication = await dependencies.authenticateApiKeyWithScope(request, {
5056
action,
5157
resource: { type: resource },
5258
});
@@ -63,9 +69,10 @@ export async function authenticateEnvironmentScopedApiRequest(
6369
/** Env var API routes: PAT/OAT on the legacy path, machine keys via RBAC. */
6470
export function authenticateEnvVarApiRequest(
6571
request: Request,
66-
action: "read" | "write"
72+
action: "read" | "write",
73+
dependencies?: AuthenticationDependencies
6774
): Promise<EnvironmentScopedAuthentication> {
68-
return authenticateEnvironmentScopedApiRequest(request, action, "envvars");
75+
return authenticateEnvironmentScopedApiRequest(request, action, "envvars", dependencies);
6976
}
7077

7178
const RESOURCE_LABELS: Record<EnvironmentScopedResource, string> = {

0 commit comments

Comments
 (0)