Skip to content

Commit 6551a58

Browse files
committed
fix(webapp): scope impersonation to id-matched customers and enforce session controls
1 parent 5dc5f70 commit 6551a58

3 files changed

Lines changed: 51 additions & 21 deletions

File tree

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

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -121,11 +121,22 @@ export async function action({ request }: ActionFunctionArgs) {
121121

122122
const user = where ? await prisma.user.findFirst({ where, include: userInclude }) : null;
123123

124+
/**
125+
* Impersonation is offered only when the customer was matched on `externalId` — a value we set
126+
* ourselves from `User.id`.
127+
*
128+
* Matching on email is a weaker claim: the address on a Plain customer isn't verified, and for
129+
* customers created outside our own writes it comes from whoever sent the message. Offering a
130+
* one-click impersonation link off the back of that would let an unverified address stand in
131+
* for an account, so email-matched customers get the account rows without it.
132+
*/
133+
const canImpersonate = !!customer.externalId;
134+
124135
// No matching user: still answer every requested key, with no data so Plain hides the cards.
125136
if (!user) {
137+
// Presence flags only — the identifiers themselves don't need to persist in log storage.
126138
logger.info("User not found for Plain customer card request", {
127-
customerId: customer.id,
128-
externalId: customer.externalId,
139+
hasExternalId: !!customer.externalId,
129140
hasEmail: !!customer.email,
130141
});
131142
return json({ cards: answerAllCardKeys(cardKeys, []) });
@@ -138,10 +149,21 @@ export async function action({ request }: ActionFunctionArgs) {
138149
for (const cardKey of cardKeys) {
139150
switch (cardKey) {
140151
case accountDetailsKey: {
141-
// Generate a signed one-time token for impersonation
142-
const impersonationToken = await generateImpersonationToken(user.id);
143-
// Build the impersonate URL with token for CSRF protection
144-
const impersonateUrl = `${env.APP_ORIGIN}/admin/impersonate?impersonate=${user.id}&impersonationToken=${encodeURIComponent(impersonationToken)}`;
152+
// Only mint a token when the button will actually be rendered — see `canImpersonate`.
153+
const impersonationComponents = canImpersonate
154+
? [
155+
uiComponent.spacer({ size: "M" }),
156+
uiComponent.divider({ spacingSize: "M" }),
157+
uiComponent.spacer({ size: "M" }),
158+
uiComponent.linkButton({
159+
label: "Impersonate User",
160+
// The one-time token is what protects this link against CSRF.
161+
url: `${env.APP_ORIGIN}/admin/impersonate?impersonate=${user.id}&impersonationToken=${encodeURIComponent(
162+
await generateImpersonationToken(user.id)
163+
)}`,
164+
}),
165+
]
166+
: [];
145167

146168
cards.push({
147169
key: accountDetailsKey,
@@ -221,13 +243,7 @@ export async function action({ request }: ActionFunctionArgs) {
221243
}),
222244
],
223245
}),
224-
uiComponent.spacer({ size: "M" }),
225-
uiComponent.divider({ spacingSize: "M" }),
226-
uiComponent.spacer({ size: "M" }),
227-
uiComponent.linkButton({
228-
label: "Impersonate User",
229-
url: impersonateUrl,
230-
}),
246+
...impersonationComponents,
231247
],
232248
}),
233249
],

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

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -141,14 +141,26 @@ export async function getRealUser(
141141
prismaClient: PrismaClientOrTransaction = prisma
142142
) {
143143
const authUser = await authenticator.isAuthenticated(request);
144+
145+
// Apply the same session controls `getUserId`/`getUser` apply to the real user, so this helper
146+
// can't become a way around them: a session the IdP has revoked throws to /logout here, and one
147+
// past its effective duration is caught below. Skipping either would let an admin whose session
148+
// should have ended still start impersonation.
149+
await revalidateSsoSession(request, authUser);
144150
if (!authUser?.userId) return null;
145151

146-
// Narrow select: callers only ever need the id and the admin flag. Takes a client so a caller
147-
// already scoped to one reads the admin from the same database it writes to.
148-
return prismaClient.user.findFirst({
152+
// Narrow select — callers need the id and the admin flag, plus `nextSessionEnd` for the deadline
153+
// check. Takes a client so a caller already scoped to one reads the admin from the same database
154+
// it writes to.
155+
const user = await prismaClient.user.findFirst({
149156
where: { id: authUser.userId },
150-
select: { id: true, admin: true },
157+
select: { id: true, admin: true, nextSessionEnd: true },
151158
});
159+
if (!user) return null;
160+
161+
maybeAutoLogout(request, user);
162+
163+
return user;
152164
}
153165

154166
export type UserFromSession = Awaited<ReturnType<typeof requireUser>>;

apps/webapp/app/utils/plainCustomerCards.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,11 @@ export function answerAllCardKeys<TCard extends { key: string }>(
4949
...cards,
5050
...cardKeys
5151
.filter((key) => !answered.has(key))
52-
.map((key): NoDataCard => ({
53-
key,
54-
components: null,
55-
})),
52+
.map(
53+
(key): NoDataCard => ({
54+
key,
55+
components: null,
56+
})
57+
),
5658
];
5759
}

0 commit comments

Comments
 (0)