Skip to content

Commit 1034b61

Browse files
authored
fix(webapp): let an impersonating admin preview the Queue Metrics UI (#4736)
## Summary The Queue Metrics dashboard UI is gated by a per-org feature flag, so there was no way to look at it for a real org without turning it on for every member of that org. An admin impersonating into an org now sees the metrics UI there regardless of the flag, so it can be checked against real data before anyone else in the org sees it. Nothing changes for a normal session: a member of an org whose flag is off still gets the classic Queues page, and the gated sub-routes still 404. ## Design The gate had no request and only resolved the org flag. It now takes the request and resolves impersonation itself, rather than each caller computing a boolean and passing it in, so the rule lives in one place and a new call site cannot forget it. Seven call sites gate on this, which is exactly why. Two things narrow the bypass: - It keys on **impersonation**, not `user.admin`. Impersonation is scoped to one org and is deliberate; keying on admin status would silently hand every admin the preview in their own day-to-day orgs. - It yields to the **view-as-user** toggle. That toggle exists so an impersonating admin can see what the member sees, and unreleased UI leaking through it would make it lie. Suppressing a read-only view there stays inside the display-only contract in `hasAdminDisplayAccess` (added in #4421). The bypass also stays behind the gate's existing org-membership lookup. Since the acting user id is the impersonation target, that lookup is what keeps the preview confined to the org actually being impersonated into. Verified end-to-end against a running instance across the matrix: member with the flag off gets the classic view and 404s; the same org under impersonation gets the metrics view and a 200; flipping view-as-user returns it to the member's exact experience and back; and the flag-on path is unchanged. An admin who is merely a member, not impersonating, still gets the classic view. One thing worth flagging: a few route comments say that with the flag off no metrics reads fire. That remains true for every member session and for the org as a whole, but an admin actively previewing does exercise that org's real Redis and ClickHouse reads. That is inherent to previewing, and bounded to one admin session.
1 parent 9baebbd commit 1034b61

11 files changed

Lines changed: 120 additions & 8 deletions

File tree

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ const DELAY_GRID_MS = 5 * 60 * 1000;
4141
* waiting to start, live counts and recent delay percentiles. Null when flag off.
4242
*/
4343
export async function resolveRunQueueMetrics(options: {
44+
request: Request;
4445
userId: string;
4546
organizationSlug: string;
4647
projectParam: string;
@@ -52,10 +53,10 @@ export async function resolveRunQueueMetrics(options: {
5253
queue: { name: string; concurrencyKey?: string | null };
5354
};
5455
}): Promise<RunQueueMetrics | null> {
55-
const { userId, organizationSlug, projectParam, envParam, run } = options;
56+
const { request, userId, organizationSlug, projectParam, envParam, run } = options;
5657

5758
try {
58-
if (!(await canAccessQueueMetricsUi({ userId, organizationSlug }))) {
59+
if (!(await canAccessQueueMetricsUi({ request, userId, organizationSlug }))) {
5960
return null;
6061
}
6162

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => {
6363
// URL), so gate it per-org like the rest of the Queue Metrics view.
6464
if (
6565
dashboardKey === "queues" &&
66-
!(await canAccessQueueMetricsUi({ userId: user.id, organizationSlug }))
66+
!(await canAccessQueueMetricsUi({ request, userId: user.id, organizationSlug }))
6767
) {
6868
throw new Response(undefined, { status: 404, statusText: "Not found" });
6969
}

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,11 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => {
170170

171171
// Per-org gate for the metrics UI. When off, this org gets the classic Queues page and
172172
// no metrics query fires.
173-
const queueMetricsUiEnabled = await canAccessQueueMetricsUi({ userId, organizationSlug });
173+
const queueMetricsUiEnabled = await canAccessQueueMetricsUi({
174+
request,
175+
userId,
176+
organizationSlug,
177+
});
174178

175179
const maxPeriodDays = queueMetricsUiEnabled
176180
? await queueMetricsMaxPeriodDays(environment.organizationId)

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => {
106106

107107
// This whole page is part of the metrics UI; gate it per-org (the list already hides
108108
// the only link to it, this is defense in depth).
109-
if (!(await canAccessQueueMetricsUi({ userId, organizationSlug }))) {
109+
if (!(await canAccessQueueMetricsUi({ request, userId, organizationSlug }))) {
110110
throw new Response(undefined, { status: 404, statusText: "Not found" });
111111
}
112112

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => {
159159
// Live queue counts for the sidebar Queue property (flag on only; the property itself
160160
// is not rendered without them, so flag off = no extra reads and no UI change).
161161
let queueMetrics: { live: QueueLiveCounts; ids: QueueMetricIds } | null = null;
162-
if (task.queue && (await canAccessQueueMetricsUi({ userId, organizationSlug }))) {
162+
if (task.queue && (await canAccessQueueMetricsUi({ request, userId, organizationSlug }))) {
163163
const queueName = task.queue.name;
164164
const [lengths, concurrency] = await Promise.all([
165165
engine.lengthOfQueues(environment, [queueName]),

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => {
119119
// Live queue counts (two O(1) Redis reads) shown in the sidebar; history charts fetch
120120
// client-side through the metric resource. Flag off = no extra reads at all.
121121
let queueMetrics: { live: QueueLiveCounts; ids: QueueMetricIds } | null = null;
122-
if (task.queue && (await canAccessQueueMetricsUi({ userId, organizationSlug }))) {
122+
if (task.queue && (await canAccessQueueMetricsUi({ request, userId, organizationSlug }))) {
123123
const queueName = task.queue.name;
124124
const [lengths, concurrency] = await Promise.all([
125125
engine.lengthOfQueues(environment, [queueName]),

apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,7 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => {
171171
// `type === "run" | "span"` discriminant downstream in `SpanView`.
172172
if (result.type === "run") {
173173
const queueMetrics = await resolveRunQueueMetrics({
174+
request,
174175
userId,
175176
organizationSlug,
176177
projectParam,

apps/webapp/app/routes/resources.queues.concurrency-keys.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ export const action = async ({ request }: ActionFunctionArgs) => {
107107
// this endpoint's data isn't reachable for orgs that can't see the UI. 404 (not 403) to hide it.
108108
if (
109109
!(await canAccessQueueMetricsUi({
110+
request,
110111
userId,
111112
organizationSlug: environment.organization.slug,
112113
}))
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
import { describe, expect, it } from "vitest";
2+
import { resolveQueueMetricsUiAccess } from "./queueMetricsUiAccess";
3+
4+
describe("resolveQueueMetricsUiAccess", () => {
5+
it("allows access when the org flag is on", () => {
6+
expect(
7+
resolveQueueMetricsUiAccess({
8+
flagEnabled: true,
9+
isImpersonating: false,
10+
isViewingAsUser: false,
11+
})
12+
).toBe(true);
13+
});
14+
15+
it("denies access when the org flag is off and the session is not impersonating", () => {
16+
expect(
17+
resolveQueueMetricsUiAccess({
18+
flagEnabled: false,
19+
isImpersonating: false,
20+
isViewingAsUser: false,
21+
})
22+
).toBe(false);
23+
});
24+
25+
it("allows an impersonating admin to preview the UI with the org flag off", () => {
26+
expect(
27+
resolveQueueMetricsUiAccess({
28+
flagEnabled: false,
29+
isImpersonating: true,
30+
isViewingAsUser: false,
31+
})
32+
).toBe(true);
33+
});
34+
35+
it("withholds the preview while the admin is viewing as the user", () => {
36+
expect(
37+
resolveQueueMetricsUiAccess({
38+
flagEnabled: false,
39+
isImpersonating: true,
40+
isViewingAsUser: true,
41+
})
42+
).toBe(false);
43+
});
44+
45+
it("keeps access for an org whose flag is on even while viewing as the user", () => {
46+
expect(
47+
resolveQueueMetricsUiAccess({
48+
flagEnabled: true,
49+
isImpersonating: true,
50+
isViewingAsUser: true,
51+
})
52+
).toBe(true);
53+
});
54+
});
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/**
2+
* The rule for who sees the Queue Metrics dashboard UI.
3+
*
4+
* Kept pure and free of server-only imports so it can be unit tested directly,
5+
* and so there is one definition of the rule for the server gate to share.
6+
*/
7+
8+
/**
9+
* Resolves the per-org feature flag against the request's impersonation state.
10+
*
11+
* The bypass exists so an admin can preview the UI for a real org before it is
12+
* revealed to that org's members, which the flag alone cannot express: flags are
13+
* org-scoped, so turning one on to look at the UI exposes every member of the org.
14+
*
15+
* It keys on impersonation rather than `user.admin` because impersonation is
16+
* scoped to one org and is a deliberate act, where admin status is neither — an
17+
* admin browsing their own orgs would otherwise silently get the preview
18+
* everywhere.
19+
*
20+
* It yields to `isViewingAsUser`, which is the admin asking to see exactly what
21+
* the member sees; previewing unreleased UI through that toggle would make it
22+
* lie. Suppressing the preview there only ever hides a read-only view, so it
23+
* stays inside the display-only contract that toggle is held to.
24+
*
25+
* The caller is responsible for only reporting `isImpersonating` for an
26+
* impersonation into a member of the org being resolved, so the bypass cannot
27+
* reach across orgs.
28+
*/
29+
export function resolveQueueMetricsUiAccess(options: {
30+
flagEnabled: boolean;
31+
isImpersonating: boolean;
32+
isViewingAsUser: boolean;
33+
}): boolean {
34+
const { flagEnabled, isImpersonating, isViewingAsUser } = options;
35+
36+
if (flagEnabled) {
37+
return true;
38+
}
39+
40+
return isImpersonating && !isViewingAsUser;
41+
}

0 commit comments

Comments
 (0)