Skip to content

Commit ea706ee

Browse files
committed
chore(webapp): trim impersonation flag comments
1 parent 49069e0 commit ea706ee

5 files changed

Lines changed: 14 additions & 30 deletions

File tree

apps/webapp/app/env.server.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -332,9 +332,7 @@ const EnvironmentSchema = z
332332
.refine(isValidRegex, "WHITELISTED_EMAILS must be a valid regex.")
333333
.optional(),
334334
ADMIN_EMAILS: z.string().refine(isValidRegex, "ADMIN_EMAILS must be a valid regex.").optional(),
335-
// When disabled, user impersonation is fully off for this instance:
336-
// existing impersonation cookies are ignored, the start endpoints 404,
337-
// and the impersonation UI isn't rendered.
335+
// Instance-level kill switch for user impersonation.
338336
IMPERSONATION_ENABLED: BoolEnv.default(true),
339337
REMIX_APP_PORT: z.string().optional(),
340338
// Opt-in, dev-only: stream this process's logs over a local telnet/TCP socket on this port.

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

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,8 @@ import { env } from "~/env.server";
1616

1717
const pageSize = 20;
1818

19-
/**
20-
* Guard for everything that starts an impersonation (the model function and
21-
* the routes that render or serve the flow). With IMPERSONATION_ENABLED off,
22-
* those surfaces don't exist: 404, not 403, so the instance doesn't advertise
23-
* the feature. Stopping an impersonation is deliberately never gated.
24-
*/
19+
// 404, not 403, so a disabled instance doesn't advertise the feature.
20+
// Stopping an impersonation is deliberately never gated.
2521
export function requireImpersonationEnabled(): void {
2622
if (!env.IMPERSONATION_ENABLED) {
2723
throw new Response("Not Found", { status: 404 });
@@ -347,8 +343,7 @@ export async function startImpersonation(
347343

348344
export async function clearImpersonation(request: Request, path: string) {
349345
const authUser = await authenticator.isAuthenticated(request);
350-
// Raw read: stopping must clear and audit the session even when the gated
351-
// reader no longer resolves it (IMPERSONATION_ENABLED off).
346+
// Raw read: stops must audit and clear even with IMPERSONATION_ENABLED off.
352347
const targetId = await getRawImpersonationId(request);
353348

354349
if (targetId && authUser?.userId) {

apps/webapp/app/root.tsx

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -118,9 +118,8 @@ export const loader = async ({ request }: LoaderFunctionArgs) => {
118118
// the `user.isViewingAsUser` the server computes could disagree, and the
119119
// client-side admin UI would hide itself on a session that is not
120120
// impersonating.
121-
// Flag off: actively terminate any lingering impersonation session (STOP
122-
// audit row + cookie cleared + one self-redirect) instead of leaving an
123-
// inert cookie that would resurrect if the flag were ever re-enabled.
121+
// Flag off: terminate lingering impersonation sessions (audit + clear)
122+
// rather than leaving a cookie that would resurrect on a later re-enable.
124123
if (!env.IMPERSONATION_ENABLED && (await getRawImpersonationId(request))) {
125124
const url = new URL(request.url);
126125
throw await clearImpersonation(request, `${url.pathname}${url.search}`);

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

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,17 +37,13 @@ export function commitImpersonationSession(session: Session) {
3737
}
3838

3939
export async function getImpersonationId(request: Request) {
40-
// Flag off: any impersonation cookie is inert, however it was obtained.
4140
if (!env.IMPERSONATION_ENABLED) return undefined;
4241

4342
return getRawImpersonationId(request);
4443
}
4544

46-
/**
47-
* The raw cookie value, ignoring IMPERSONATION_ENABLED. Only for terminating
48-
* or auditing a session the gated reader no longer resolves — never for
49-
* authorizing anything.
50-
*/
45+
// Ignores IMPERSONATION_ENABLED — only for terminating or auditing a session
46+
// the gated reader no longer resolves, never for authorizing anything.
5147
export async function getRawImpersonationId(request: Request) {
5248
const session = await getImpersonationSession(request);
5349

apps/webapp/test/impersonationDisabled.test.ts

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,11 @@ function suffix() {
1616
return Math.random().toString(36).slice(2, 10);
1717
}
1818

19-
// IMPERSONATION_ENABLED=false must make impersonation fully inert: starting
20-
// one 404s, and an existing cookie resolves to nothing however it was
21-
// obtained. Stopping stays possible with the flag off — that's how lingering
22-
// sessions get terminated — and must still clear the cookie.
19+
// IMPERSONATION_ENABLED=false: starting 404s, cookies resolve to nothing,
20+
// stopping still works so lingering sessions can be terminated.
2321
describe("impersonation disabled", () => {
2422
postgresTest("the flag defaults to enabled", async () => {
25-
// Flipping this default would kill impersonation on every existing
26-
// deployment that never heard of the flag.
23+
// Flipping the default would kill impersonation on every existing deployment.
2724
expect(env.IMPERSONATION_ENABLED).toBe(true);
2825
});
2926

@@ -50,8 +47,8 @@ describe("impersonation disabled", () => {
5047
new Request("http://localhost:3030/", { headers: { Cookie: cookie } });
5148

5249
expect(await getImpersonationId(requestWithCookie())).toBe(target.id);
53-
// resolvedUserId must match the impersonated id for the state to count as
54-
// impersonating — that's what getUserId resolves to while the cookie works.
50+
// resolvedUserId must be the impersonated id or the state is false even
51+
// with the flag on, making the disabled assertion below vacuous.
5552
const enabledState = await getImpersonationState(requestWithCookie(), target.id);
5653
expect(enabledState.isImpersonating).toBe(true);
5754

@@ -76,8 +73,7 @@ describe("impersonation disabled", () => {
7673
const disabledState = await getImpersonationState(requestWithCookie(), target.id);
7774
expect(disabledState.isImpersonating).toBe(false);
7875

79-
// The ungated reader still sees the cookie — it's what stop/scrub paths
80-
// use to terminate a session the gated reader no longer resolves.
76+
// The ungated reader still sees the cookie (stop/scrub paths need it).
8177
expect(await getRawImpersonationId(requestWithCookie())).toBe(target.id);
8278

8379
// Stopping works with the flag off and clears the cookie.

0 commit comments

Comments
 (0)