Skip to content

Commit d0de363

Browse files
committed
more coderabbit changes
1 parent fdad222 commit d0de363

File tree

4 files changed

+38
-46
lines changed

4 files changed

+38
-46
lines changed

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -212,8 +212,13 @@ export async function adminGetOrganizations(userId: string, { page, search }: Se
212212
};
213213
}
214214

215-
export async function redirectWithImpersonation(request: Request, userId: string, path: string) {
216-
const user = await requireUser(request);
215+
export async function redirectWithImpersonation(
216+
request: Request,
217+
userId: string,
218+
path: string,
219+
currentUser?: { id: string; admin: boolean }
220+
) {
221+
const user = currentUser ?? (await requireUser(request));
217222
if (!user.admin) {
218223
throw new Error("Unauthorized");
219224
}

apps/webapp/app/routes/admin.impersonate.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ async function handleImpersonationRequest(request: Request, userId: string): Pro
1212
if (!user.admin) {
1313
return redirect("/");
1414
}
15-
return redirectWithImpersonation(request, userId, "/");
15+
return redirectWithImpersonation(request, userId, "/", user);
1616
}
1717

1818
export const loader = async ({ request }: LoaderFunctionArgs) => {
@@ -42,7 +42,7 @@ export const loader = async ({ request }: LoaderFunctionArgs) => {
4242
return redirect("/");
4343
}
4444

45-
return redirectWithImpersonation(request, impersonateUserId, "/");
45+
return redirectWithImpersonation(request, impersonateUserId, "/", user);
4646
};
4747

4848
export async function action({ request }: ActionFunctionArgs) {

apps/webapp/app/routes/api.v1.plain.customer-cards.ts

Lines changed: 19 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -115,54 +115,32 @@ export async function action({ request }: ActionFunctionArgs) {
115115

116116
const { customer, cardKeys } = parsed.data;
117117

118-
// Look up the user by externalId (which is User.id)
119-
let user = null;
120-
if (customer.externalId) {
121-
user = await prisma.user.findUnique({
122-
where: { id: customer.externalId },
123-
include: {
124-
orgMemberships: {
125-
where: {
126-
organization: { deletedAt: null },
127-
},
128-
include: {
129-
organization: {
130-
include: {
131-
projects: {
132-
where: { deletedAt: null },
133-
take: 10, // Limit to recent projects
134-
orderBy: { createdAt: "desc" },
135-
},
136-
},
137-
},
138-
},
139-
},
118+
const userInclude = {
119+
orgMemberships: {
120+
where: {
121+
organization: { deletedAt: null },
140122
},
141-
});
142-
} else if (customer.email) {
143-
// Fallback to email lookup if externalId is not provided
144-
user = await prisma.user.findUnique({
145-
where: { email: customer.email },
146123
include: {
147-
orgMemberships: {
148-
where: {
149-
organization: { deletedAt: null },
150-
},
124+
organization: {
151125
include: {
152-
organization: {
153-
include: {
154-
projects: {
155-
where: { deletedAt: null },
156-
take: 10,
157-
orderBy: { createdAt: "desc" },
158-
},
159-
},
126+
projects: {
127+
where: { deletedAt: null },
128+
take: 10,
129+
orderBy: { createdAt: "desc" as const },
160130
},
161131
},
162132
},
163133
},
164-
});
165-
}
134+
},
135+
};
136+
137+
const where = customer.externalId
138+
? { id: customer.externalId }
139+
: customer.email
140+
? { email: customer.email }
141+
: null;
142+
143+
const user = where ? await prisma.user.findUnique({ where, include: userInclude }) : null;
166144

167145
// If user not found, return empty cards
168146
if (!user) {

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { createCookieSessionStorage, type Session } from "@remix-run/node";
22
import { SignJWT, jwtVerify, errors } from "jose";
3+
import { randomUUID } from "node:crypto";
34
import { singleton } from "~/utils/singleton";
45
import { createRedisClient, type RedisClient } from "~/redis.server";
56
import { env } from "~/env.server";
@@ -83,6 +84,7 @@ export async function generateImpersonationToken(userId: string): Promise<string
8384
.setExpirationTime(now + IMPERSONATION_TOKEN_EXPIRY_SECONDS)
8485
.setIssuer("https://trigger.dev")
8586
.setAudience("https://trigger.dev/admin")
87+
.setJti(randomUUID())
8688
.sign(secret);
8789

8890
return token;
@@ -109,12 +111,19 @@ export async function validateAndConsumeImpersonationToken(
109111
}
110112

111113
// Atomically mark the token as used to prevent replay attacks.
114+
// Use the jti (a short UUID) as the Redis key rather than the full JWT string.
112115
// If Redis is unavailable, fall back to JWT expiry as the only protection.
113116
if (env.CACHE_REDIS_HOST) {
117+
// Defensively reject tokens without a jti (e.g. issued before this change)
118+
if (!payload.jti) {
119+
logger.warn("Impersonation token missing jti claim, rejecting for safety");
120+
return undefined;
121+
}
122+
114123
try {
115124
const redis = getImpersonationTokenRedisClient();
116125
const result = await redis.set(
117-
token,
126+
payload.jti,
118127
"1",
119128
"EX",
120129
IMPERSONATION_TOKEN_EXPIRY_SECONDS,

0 commit comments

Comments
 (0)