diff --git a/.server-changes/impersonation-consent-and-view-as-user.md b/.server-changes/impersonation-consent-and-view-as-user.md new file mode 100644 index 00000000000..232e97e8b90 --- /dev/null +++ b/.server-changes/impersonation-consent-and-view-as-user.md @@ -0,0 +1,6 @@ +--- +area: webapp +type: improvement +--- + +Admins opening an impersonation link from outside the dashboard now get a confirmation page naming the organization and destination instead of being bounced back, and while impersonating they can switch to "View as user" to see the dashboard exactly as that user sees it, with the admin-only UI and the impersonation highlight both hidden. Stopping impersonation is still one click away in the account menu. diff --git a/apps/webapp/app/components/ImpersonationBanner.tsx b/apps/webapp/app/components/ImpersonationBanner.tsx deleted file mode 100644 index c16e822df1e..00000000000 --- a/apps/webapp/app/components/ImpersonationBanner.tsx +++ /dev/null @@ -1,30 +0,0 @@ -import { Form } from "@remix-run/react"; -import { UserCrossIcon } from "~/assets/icons/UserCrossIcon"; -import { Button } from "./primitives/Buttons"; -import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from "./primitives/Tooltip"; - -export function ImpersonationBanner() { - return ( -
-
- - - -
- ); -} diff --git a/apps/webapp/app/components/admin/debugRun.tsx b/apps/webapp/app/components/admin/debugRun.tsx index 705e3169a8b..049c5cd08c3 100644 --- a/apps/webapp/app/components/admin/debugRun.tsx +++ b/apps/webapp/app/components/admin/debugRun.tsx @@ -1,4 +1,3 @@ -import { useIsImpersonating } from "~/hooks/useOrganizations"; import { useHasAdminAccess } from "~/hooks/useUser"; import { Button } from "../primitives/Buttons"; import { Dialog, DialogContent, DialogHeader, DialogTrigger } from "../primitives/Dialog"; @@ -12,10 +11,11 @@ import * as Property from "~/components/primitives/PropertyTable"; import { ClipboardField } from "../primitives/ClipboardField"; export function AdminDebugRun({ friendlyId }: { friendlyId: string }) { + // `useHasAdminAccess` already folds in impersonation and the "view as user" + // toggle, so this one check is enough. const hasAdminAccess = useHasAdminAccess(); - const isImpersonating = useIsImpersonating(); - if (!hasAdminAccess && !isImpersonating) { + if (!hasAdminAccess) { return null; } diff --git a/apps/webapp/app/components/admin/debugTooltip.tsx b/apps/webapp/app/components/admin/debugTooltip.tsx index 4157f898163..1729bfa740c 100644 --- a/apps/webapp/app/components/admin/debugTooltip.tsx +++ b/apps/webapp/app/components/admin/debugTooltip.tsx @@ -8,15 +8,16 @@ import { TooltipTrigger, } from "~/components/primitives/Tooltip"; import { useOptionalEnvironment } from "~/hooks/useEnvironment"; -import { useIsImpersonating, useOptionalOrganization } from "~/hooks/useOrganizations"; +import { useOptionalOrganization } from "~/hooks/useOrganizations"; import { useOptionalProject } from "~/hooks/useProject"; import { useHasAdminAccess, useUser } from "~/hooks/useUser"; export function AdminDebugTooltip({ children }: { children?: React.ReactNode }) { + // `useHasAdminAccess` already folds in impersonation and the "view as user" + // toggle, so this one check is enough. const hasAdminAccess = useHasAdminAccess(); - const isImpersonating = useIsImpersonating(); - if (!hasAdminAccess && !isImpersonating) { + if (!hasAdminAccess) { return null; } diff --git a/apps/webapp/app/components/navigation/SideMenu.tsx b/apps/webapp/app/components/navigation/SideMenu.tsx index 29eb1fc8520..ebedc77fc6e 100644 --- a/apps/webapp/app/components/navigation/SideMenu.tsx +++ b/apps/webapp/app/components/navigation/SideMenu.tsx @@ -4,7 +4,14 @@ import { ExclamationTriangleIcon, } from "@heroicons/react/24/outline"; import { EllipsisHorizontalIcon } from "@heroicons/react/20/solid"; -import { useFetcher, useNavigation, useRevalidator, useSubmit } from "@remix-run/react"; +import { + Form, + useFetcher, + useLocation, + useNavigation, + useRevalidator, + useSubmit, +} from "@remix-run/react"; import { LayoutGroup, motion } from "framer-motion"; import { type CSSProperties, @@ -33,6 +40,8 @@ import { DeploymentsIcon } from "~/assets/icons/DeploymentsIcon"; import { DialIcon } from "~/assets/icons/DialIcon"; import { DropdownIcon } from "~/assets/icons/DropdownIcon"; import { BranchEnvironmentIconSmall } from "~/assets/icons/EnvironmentIcons"; +import { EyeClosedIcon } from "~/assets/icons/EyeClosedIcon"; +import { EyeOpenIcon } from "~/assets/icons/EyeOpenIcon"; import { FolderClosedIcon } from "~/assets/icons/FolderClosedIcon"; import { FolderOpenIcon } from "~/assets/icons/FolderOpenIcon"; import { GlobeLinesIcon } from "~/assets/icons/GlobeLinesIcon"; @@ -69,7 +78,7 @@ import { type MatchedOrganization } from "~/hooks/useOrganizations"; import { type MatchedProject } from "~/hooks/useProject"; import { useShortcutKeys } from "~/hooks/useShortcutKeys"; import { useShowSelfServe } from "~/hooks/useShowSelfServe"; -import { useHasAdminAccess } from "~/hooks/useUser"; +import { useHasAdminAccess, useIsViewingAsUser } from "~/hooks/useUser"; import { type UserWithDashboardPreferences } from "~/models/user.server"; import { useCurrentPlan, @@ -389,6 +398,7 @@ export function SideMenu({ const { isConnected } = useDevPresence(); const isFreeUser = currentPlan?.v3Subscription?.isPaying === false; const isAdmin = useHasAdminAccess(); + const isViewingAsUser = useIsViewingAsUser(); const { isManagedCloud } = useFeatures(); const featureFlags = useFeatureFlags(); const incidentStatus = useIncidentStatus(); @@ -785,7 +795,7 @@ export function SideMenu({ // user's saved order/hidden preferences are applied at render below. const staticSections: SideMenuSectionConfig[] = []; - if (user.admin || user.isImpersonating || featureFlags.hasAiAccess) { + if (isAdmin || featureFlags.hasAiAccess) { staticSections.push({ id: "ai", title: "AI", @@ -813,12 +823,12 @@ export function SideMenu({ }); } - if (user.admin || user.isImpersonating || featureFlags.hasQueryAccess) { + if (isAdmin || featureFlags.hasQueryAccess) { staticSections.push({ id: "metrics", title: "Observability", items: [ - ...(user.admin || user.isImpersonating || featureFlags.hasLogsPageAccess + ...(isAdmin || featureFlags.hasLogsPageAccess ? [ { id: "logs", @@ -1048,7 +1058,13 @@ export function SideMenu({ style={initialStyleRef.current} className={cn( "relative h-full border-r bg-background-bright", - user.isImpersonating ? IMPERSONATION_ACCENT.border : "border-grid-bright" + // The accent is the loudest "you are not this user" tell, so "view as user" drops it too — + // the point of the mode is a dashboard that looks exactly like the user's. The account + // menu's "Stop impersonating" and the toggle itself stay on raw impersonation, so there is + // still a way back out (as does the ⌘⌥A shortcut in ). + user.isImpersonating && !isViewingAsUser + ? IMPERSONATION_ACCENT.border + : "border-grid-bright" )} > - {isAdmin && ( + {/* "Stop impersonating" and the view-as-user toggle key off raw impersonation, not `isAdmin`: + with "view as user" on, `isAdmin` is false and these are the only ways back out. */} + {(isImpersonating || isAdmin) && (
{isImpersonating ? ( - - Stop impersonating - -
- } - icon={UserCrossIcon} - onClick={stopImpersonating} - leadingIconClassName={cn(SIDE_MENU_POPOVER_ITEM_ICON, IMPERSONATION_ACCENT.text)} - className={SIDE_MENU_POPOVER_ITEM_LABEL} - /> + <> + + Stop impersonating + + + } + icon={UserCrossIcon} + onClick={stopImpersonating} + leadingIconClassName={cn(SIDE_MENU_POPOVER_ITEM_ICON, IMPERSONATION_ACCENT.text)} + className={SIDE_MENU_POPOVER_ITEM_LABEL} + /> + + ) : ( + + + + + ); +} + function AccountMenu({ isAdmin, isImpersonating }: { isAdmin: boolean; isImpersonating: boolean }) { const [isOpen, setIsOpen] = useState(false); const navigation = useNavigation(); diff --git a/apps/webapp/app/hooks/useUser.ts b/apps/webapp/app/hooks/useUser.ts index fd9938fdb9a..aa86ba63865 100644 --- a/apps/webapp/app/hooks/useUser.ts +++ b/apps/webapp/app/hooks/useUser.ts @@ -30,9 +30,23 @@ export function useUserChanged(callback: (user: User | undefined) => void) { useChanged(useOptionalUser, callback); } +/** + * Whether the admin has switched to "view as user" for the current + * impersonation session. Display only — see `hasAdminDisplayAccess`. + */ +export function useIsViewingAsUser(matches?: UIMatch[]): boolean { + const routeMatch = useTypedMatchesData({ + id: "root", + matches, + }); + + return routeMatch?.isViewingAsUser === true; +} + export function useHasAdminAccess(matches?: UIMatch[]): boolean { const user = useOptionalUser(matches); const isImpersonating = useIsImpersonating(matches); + const isViewingAsUser = useIsViewingAsUser(matches); - return Boolean(user?.admin) || isImpersonating; + return (Boolean(user?.admin) || isImpersonating) && !isViewingAsUser; } diff --git a/apps/webapp/app/models/admin.server.ts b/apps/webapp/app/models/admin.server.ts index 09811c99c2d..e93844dbaed 100644 --- a/apps/webapp/app/models/admin.server.ts +++ b/apps/webapp/app/models/admin.server.ts @@ -1,5 +1,5 @@ import { redirect } from "@remix-run/server-runtime"; -import { prisma } from "~/db.server"; +import { $replica, prisma, type PrismaClientOrTransaction } from "~/db.server"; import { logger } from "~/services/logger.server"; import type { SearchParams } from "~/routes/admin._index"; import { @@ -11,6 +11,7 @@ import { import { authenticator } from "~/services/auth.server"; import { requireUser } from "~/services/session.server"; import { extractClientIp } from "~/utils/extractClientIp.server"; +import { impersonationDestinationPath } from "~/utils/pathBuilder"; const pageSize = 20; @@ -213,7 +214,8 @@ export async function redirectWithImpersonation( request: Request, userId: string, path: string, - currentUser?: { id: string; admin: boolean } + currentUser?: { id: string; admin: boolean }, + prismaClient: PrismaClientOrTransaction = prisma ) { const user = currentUser ?? (await requireUser(request)); if (!user.admin) { @@ -224,7 +226,7 @@ export async function redirectWithImpersonation( const ipAddress = extractClientIp(xff); try { - await prisma.impersonationAuditLog.create({ + await prismaClient.impersonationAuditLog.create({ data: { action: "START", adminId: user.id, @@ -247,6 +249,87 @@ export async function redirectWithImpersonation( }); } +type ImpersonationTarget = + | { success: true; userId: string; organizationName: string } + | { success: false; reason: "org-not-found" | "no-confirmed-member" }; + +/** + * Read-only lookup of who a `/@/orgs//…` link would impersonate: the + * first organization member who has confirmed their basic details. Writes + * nothing, so it is safe to call while only rendering the consent page. + */ +export async function findImpersonationTarget( + organizationSlug: string, + prismaClient: PrismaClientOrTransaction = $replica +): Promise { + const org = await prismaClient.organization.findFirst({ + where: { + slug: organizationSlug, + deletedAt: null, + }, + select: { + title: true, + members: { + select: { + user: { + select: { + id: true, + confirmedBasicDetails: true, + }, + }, + }, + }, + }, + }); + + if (!org) { + return { success: false, reason: "org-not-found" }; + } + + const firstValidMember = org.members.find((m) => m.user.confirmedBasicDetails); + + if (!firstValidMember) { + return { success: false, reason: "no-confirmed-member" }; + } + + return { success: true, userId: firstValidMember.user.id, organizationName: org.title }; +} + +/** + * Starts impersonating the organization's first confirmed member and lands on + * the requested path with the `/@` prefix stripped. Shared by the same-origin + * loader path and the consent page's POST so there is one implementation. + * + * The destination keeps the incoming query string: both entry points are served + * at the `/@`-prefixed URL, so `request.url` carries the same search the link + * arrived with (for example the `?span=` a `/@/runs/` link redirects with). + */ +export async function startImpersonation( + request: Request, + organizationSlug: string, + path: string, + currentUser: { id: string; admin: boolean }, + clients: { read: PrismaClientOrTransaction; write: PrismaClientOrTransaction } = { + read: $replica, + write: prisma, + } +) { + const target = await findImpersonationTarget(organizationSlug, clients.read); + + if (!target.success) { + logger.debug("Cannot impersonate organization", { organizationSlug, reason: target.reason }); + return clearImpersonation(request, "/admin"); + } + + return redirectWithImpersonation( + request, + target.userId, + impersonationDestinationPath(organizationSlug, path, new URL(request.url).search), + currentUser, + clients.write + ); +} + export async function clearImpersonation(request: Request, path: string) { const authUser = await authenticator.isAuthenticated(request); const targetId = await getImpersonationId(request); diff --git a/apps/webapp/app/root.tsx b/apps/webapp/app/root.tsx index 839f44b4bbc..f6c403b8c11 100644 --- a/apps/webapp/app/root.tsx +++ b/apps/webapp/app/root.tsx @@ -19,6 +19,7 @@ import { TimezoneSetter } from "./components/TimezoneSetter"; import { env } from "./env.server"; import { featuresForRequest } from "./features.server"; import { usePostHog } from "./hooks/usePostHog"; +import { getImpersonationState } from "./services/impersonation.server"; import { getUser } from "./services/session.server"; import { getTimezonePreference } from "./services/preferences/uiPreferences.server"; import { appEnvTitleTag } from "./utils"; @@ -70,6 +71,16 @@ export const loader = async ({ request }: LoaderFunctionArgs) => { }; const user = await getUser(request); + // Display-only: while impersonating, an admin can ask to see the dashboard + // the way the impersonated user sees it. Exposed from root so every route can + // read it. + // + // Resolved against the user this request authenticated as, which is the same + // condition `requireUser` applies — otherwise the flag the client reads and + // the `user.isViewingAsUser` the server computes could disagree, and the + // client-side admin UI would hide itself on a session that is not + // impersonating. + const { isViewingAsUser } = await getImpersonationState(request, user?.id); const headers = new Headers(); headers.append("Set-Cookie", await commitSession(session)); @@ -77,6 +88,7 @@ export const loader = async ({ request }: LoaderFunctionArgs) => { return typedjson( { user, + isViewingAsUser, toastMessage, posthogProjectKey, posthogUiHost, diff --git a/apps/webapp/app/routes/_app.@.orgs.$organizationSlug.$.ts b/apps/webapp/app/routes/_app.@.orgs.$organizationSlug.$.ts deleted file mode 100644 index a47deeac6aa..00000000000 --- a/apps/webapp/app/routes/_app.@.orgs.$organizationSlug.$.ts +++ /dev/null @@ -1,82 +0,0 @@ -import type { LoaderFunctionArgs } from "@remix-run/server-runtime"; -import { redirect } from "remix-typedjson"; -import { $replica } from "~/db.server"; -import { clearImpersonation, redirectWithImpersonation } from "~/models/admin.server"; -import { env } from "~/env.server"; -import { logger } from "~/services/logger.server"; -import { requireUser } from "~/services/session.server"; -import { isSameOriginNavigation } from "~/utils/sameOriginNavigation"; - -export async function loader({ request, params }: LoaderFunctionArgs) { - const user = await requireUser(request); - - // If already impersonating, we need to clear the impersonation - if (user.isImpersonating) { - const url = new URL(request.url); - return clearImpersonation(request, url.pathname); - } - - // Only admins can impersonate - if (!user.admin) { - return redirect("/"); - } - - const path = params["*"]; - const organizationSlug = params.organizationSlug; - - logger.debug("Impersonating user", { path, organizationSlug }); - - if (!organizationSlug) { - logger.debug("Exiting impersonation mode"); - return clearImpersonation(request, "/admin"); - } - - // CSRF gate for the SET-impersonation path. Clearing impersonation - // above is benign and stays reachable without the check. - if (!isSameOriginNavigation(request, env.LOGIN_ORIGIN)) { - logger.warn("Refusing cross-site impersonation entry", { - userId: user.id, - organizationSlug, - referer: request.headers.get("referer"), - secFetchSite: request.headers.get("sec-fetch-site"), - }); - return redirect("/admin"); - } - - const org = await $replica.organization.findFirst({ - where: { - slug: organizationSlug, - deletedAt: null, - }, - select: { - members: { - select: { - user: { - select: { - id: true, - confirmedBasicDetails: true, - }, - }, - }, - }, - }, - }); - - if (!org) { - logger.debug("Organization not found", { organizationSlug }); - return clearImpersonation(request, "/admin"); - } - - const firstValidMember = org.members.find((m) => m.user.confirmedBasicDetails); - - if (!firstValidMember) { - logger.debug("No valid members found", { organizationSlug }); - return clearImpersonation(request, "/admin"); - } - - return redirectWithImpersonation( - request, - firstValidMember.user.id, - `/orgs/${organizationSlug}/${path}` - ); -} diff --git a/apps/webapp/app/routes/_app.@.orgs.$organizationSlug.$.tsx b/apps/webapp/app/routes/_app.@.orgs.$organizationSlug.$.tsx new file mode 100644 index 00000000000..2923a6fdeeb --- /dev/null +++ b/apps/webapp/app/routes/_app.@.orgs.$organizationSlug.$.tsx @@ -0,0 +1,188 @@ +import { Form } from "@remix-run/react"; +import type { ActionFunctionArgs, LoaderFunctionArgs } from "@remix-run/server-runtime"; +import { redirect, typedjson, useTypedLoaderData } from "remix-typedjson"; +import { MainCenteredContainer } from "~/components/layout/AppLayout"; +import { Button } from "~/components/primitives/Buttons"; +import { Callout } from "~/components/primitives/Callout"; +import { Header1 } from "~/components/primitives/Headers"; +import { Paragraph } from "~/components/primitives/Paragraph"; +import { env } from "~/env.server"; +import { + clearImpersonation, + findImpersonationTarget, + startImpersonation, +} from "~/models/admin.server"; +import { logger } from "~/services/logger.server"; +import { requireUser } from "~/services/session.server"; +import { + impersonationConsentPostBackPath, + impersonationDestinationPath, +} from "~/utils/pathBuilder"; +import { isSameOriginNavigation } from "~/utils/sameOriginNavigation"; + +// Everything this route's loader and action touch on the server lives in +// `~/models/admin.server` on purpose: Remix only strips `loader`, `action` and +// `headers` from a route module for the browser bundle, so any other export +// here would drag server-only modules into the client build. + +export async function loader({ request, params }: LoaderFunctionArgs) { + const user = await requireUser(request); + + // If already impersonating, we need to clear the impersonation. Redirects are + // thrown, not returned, so the consent page below is the loader's only data + // shape. + if (user.isImpersonating) { + const url = new URL(request.url); + // Keep the search: `/@/runs/` links redirect here carrying `?span=`, and the + // follow-up GET builds the destination and post-back paths from it. Dropping it would land the + // admin on the run with no span selected. + throw await clearImpersonation(request, `${url.pathname}${url.search}`); + } + + // Only admins can impersonate + if (!user.admin) { + throw redirect("/"); + } + + const path = params["*"] ?? ""; + const organizationSlug = params.organizationSlug; + + logger.debug("Impersonating user", { path, organizationSlug }); + + if (!organizationSlug) { + logger.debug("Exiting impersonation mode"); + throw await clearImpersonation(request, "/admin"); + } + + // Starting impersonation is a state change, so it only happens straight away + // for an unambiguously same-origin navigation — that is what stops a + // cross-site navigation from silently starting impersonation. Links opened + // from outside the app (address bar, bookmark, a link shared elsewhere) get + // the consent page below instead, whose "Impersonate" button posts back from + // our own page and so satisfies the same check. + if (isSameOriginNavigation(request, env.LOGIN_ORIGIN)) { + throw await startImpersonation(request, organizationSlug, path, user); + } + + // Expected for any link opened outside the app (address bar, bookmark, a link + // shared elsewhere), so this is routine rather than suspicious. Only the + // referer's origin is logged — the full referer can carry another site's path + // and query string. + logger.info("Impersonation entry outside the app, showing consent page", { + userId: user.id, + organizationSlug, + refererOrigin: refererOrigin(request), + secFetchSite: request.headers.get("sec-fetch-site"), + }); + + // Read-only on purpose: nothing is written and no impersonation cookie is set + // until the admin confirms with the POST below. + const target = await findImpersonationTarget(organizationSlug); + + const search = new URL(request.url).search; + + return typedjson({ + organizationSlug, + organizationName: target.success ? target.organizationName : undefined, + destinationPath: impersonationDestinationPath(organizationSlug, path, search), + postBackPath: impersonationConsentPostBackPath(organizationSlug, path, search), + canImpersonate: target.success, + }); +} + +function refererOrigin(request: Request): string | undefined { + const referer = request.headers.get("referer"); + if (!referer) return undefined; + try { + return new URL(referer).origin; + } catch { + return undefined; + } +} + +export async function action({ request, params }: ActionFunctionArgs) { + if (request.method.toLowerCase() !== "post") { + return new Response("Method not allowed", { status: 405 }); + } + + const user = await requireUser(request); + + // Same ordering as the loader, and it matters for the same reason: while an + // impersonation cookie is set `requireUser` resolves to the *impersonated* + // user, so `user.admin` is false even for a legitimate admin. An admin who + // started impersonating in another tab and then submitted this page would + // fail the check below and be bounced to `/` with no explanation. Clear the + // impersonation first and come back to this same URL under their own + // identity, where the normal flow re-authorizes them and they can confirm. + // Never run the impersonation mutation on a request whose resolved identity + // is the impersonated user. + if (user.isImpersonating) { + const url = new URL(request.url); + // Keep the search for the same reason the loader does: the follow-up GET + // rebuilds the destination and post-back paths from it. + return clearImpersonation(request, `${url.pathname}${url.search}`); + } + + if (!user.admin) { + return redirect("/"); + } + + // The consent page posts from our own origin, so this holds. Re-applied here + // so another site cannot drive the POST either. + if (!isSameOriginNavigation(request, env.LOGIN_ORIGIN)) { + logger.warn("Refusing cross-site impersonation submission", { + userId: user.id, + organizationSlug: params.organizationSlug, + referer: request.headers.get("referer"), + secFetchSite: request.headers.get("sec-fetch-site"), + }); + return redirect("/admin"); + } + + const organizationSlug = params.organizationSlug; + + if (!organizationSlug) { + return clearImpersonation(request, "/admin"); + } + + // The consent form posts to an explicit absolute path (see + // `impersonationConsentPostBackPath`), so the organization slug, the splat + // path and the query string all arrive here intact. + return startImpersonation(request, organizationSlug, params["*"] ?? "", user); +} + +export default function Page() { + const { organizationSlug, organizationName, destinationPath, postBackPath, canImpersonate } = + useTypedLoaderData(); + + return ( + +
+ Impersonate + {canImpersonate ? ( + <> + + Continue to impersonate a member of{" "} + {organizationName ?? organizationSlug} and + open {destinationPath}. + +
+ +
+ + Only continue if you meant to open this link. You'll be signed in as a member of this + organization until you stop impersonating. + + + ) : ( + + There's no organization {organizationSlug}{" "} + with a member you can impersonate. + + )} +
+ + ); +} diff --git a/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboards.custom.$dashboardId/route.tsx b/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboards.custom.$dashboardId/route.tsx index 153004813a8..6f7c1d2609c 100644 --- a/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboards.custom.$dashboardId/route.tsx +++ b/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboards.custom.$dashboardId/route.tsx @@ -49,7 +49,7 @@ import { getTaskIdentifiers } from "~/models/task.server"; import { MetricDashboardPresenter } from "~/presenters/v3/MetricDashboardPresenter.server"; import { QueryPresenter } from "~/presenters/v3/QueryPresenter.server"; import { removeFavoritesByUrlSubstring } from "~/services/dashboardPreferences.server"; -import { requireUser } from "~/services/session.server"; +import { hasAdminDisplayAccess, requireUser } from "~/services/session.server"; import { EnvironmentParamSchema, queryPath, @@ -98,7 +98,7 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => { ]); // Admins and impersonating users can use EXPLAIN - const isAdmin = user.admin || user.isImpersonating; + const isAdmin = hasAdminDisplayAccess(user); // Compute widget count from dashboard layout const widgetCount = Object.keys(dashboard.layout.widgets).length; diff --git a/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.playground/route.tsx b/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.playground/route.tsx index da486898d4f..ddb9f2a63eb 100644 --- a/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.playground/route.tsx +++ b/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.playground/route.tsx @@ -54,6 +54,9 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => { orderBy: { createdAt: "desc" }, take: 20, }), + // Raw impersonation, not `hasAdminDisplayAccess`: this list is the + // playground's region picker, so it decides which region a submitted run + // can be sent to. "View as user" only changes what is shown. new RegionsPresenter().call({ userId: user.id, projectSlug: projectParam, diff --git a/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx b/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx index 848546a518f..9aec240cb9d 100644 --- a/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx +++ b/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx @@ -11,7 +11,7 @@ import { findProjectBySlug } from "~/models/project.server"; import { findEnvironmentBySlug } from "~/models/runtimeEnvironment.server"; import { QueryPresenter } from "~/presenters/v3/QueryPresenter.server"; import { executeQuery, getDefaultPeriod } from "~/services/queryService.server"; -import { requireUser } from "~/services/session.server"; +import { hasAdminDisplayAccess, requireUser } from "~/services/session.server"; import { EnvironmentParamSchema, queryPath } from "~/utils/pathBuilder"; import { canAccessQuery } from "~/v3/canAccessQuery.server"; import { useCurrentPlan } from "../_app.orgs.$organizationSlug/route"; @@ -63,7 +63,7 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => { }); // Admins and impersonating users can use EXPLAIN - const isAdmin = user.admin || user.isImpersonating; + const isAdmin = hasAdminDisplayAccess(user); return typedjson({ defaultQuery, @@ -175,9 +175,10 @@ export const action = async ({ request, params }: ActionFunctionArgs) => { } const { query, scope, explain: explainParam, period, from, to } = parsed.data; - // Only allow explain for admins/impersonating users - const isAdmin = user.admin || user.isImpersonating; - const explain = explainParam === "true" && isAdmin; + // Only allow explain for admins/impersonating users. Raw impersonation, not + // `hasAdminDisplayAccess`: this decides what the request may run, and "view as user" only changes + // what is shown — the loader is what hides the EXPLAIN control. + const explain = explainParam === "true" && (user.admin || user.isImpersonating); try { const queryResult = await executeQuery({ diff --git a/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.regions/route.tsx b/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.regions/route.tsx index 588647f22a7..f5b4e4901db 100644 --- a/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.regions/route.tsx +++ b/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.regions/route.tsx @@ -54,7 +54,7 @@ import { redirectWithErrorMessage, redirectWithSuccessMessage } from "~/models/m import { resolveOrgIdFromSlug } from "~/models/organization.server"; import { findProjectBySlug } from "~/models/project.server"; import { type Region, RegionsPresenter } from "~/presenters/v3/RegionsPresenter.server"; -import { requireUser } from "~/services/session.server"; +import { hasAdminDisplayAccess, requireUser } from "~/services/session.server"; import { dashboardAction } from "~/services/routeBuilders/dashboardBuilder"; import { docsPath, @@ -74,7 +74,7 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => { presenter.call({ userId: user.id, projectSlug: projectParam, - isAdmin: user.admin || user.isImpersonating, + isAdmin: hasAdminDisplayAccess(user), }) ); @@ -135,6 +135,9 @@ export const action = dashboardAction( service.call({ projectId: project.id, regionId: parsedFormData.data.regionId, + // Raw impersonation, not `hasAdminDisplayAccess`: this decides whether a restricted or + // hidden region may be set as the default, which is a capability. "View as user" only + // changes what is shown. isAdmin: user.admin || user.isImpersonating, }) ); diff --git a/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.test.tasks.$taskParam/route.tsx b/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.test.tasks.$taskParam/route.tsx index 321d6391179..ffe6dbcd038 100644 --- a/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.test.tasks.$taskParam/route.tsx +++ b/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.test.tasks.$taskParam/route.tsx @@ -117,6 +117,9 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => { taskIdentifier: taskParam, environment: environment, }), + // Raw impersonation, not `hasAdminDisplayAccess`: this list is the test + // form's region picker, so it decides which region a submitted test run + // can be sent to. "View as user" only changes what is shown. new RegionsPresenter().call({ userId: user.id, projectSlug: projectParam, diff --git a/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam/route.tsx b/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam/route.tsx index d22883caa12..9a2127489d4 100644 --- a/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam/route.tsx +++ b/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam/route.tsx @@ -5,7 +5,7 @@ import { DashboardAgent } from "~/components/dashboard-agent/DashboardAgent"; import { prisma } from "~/db.server"; import { updateCurrentProjectEnvironmentId } from "~/services/dashboardPreferences.server"; import { logger } from "~/services/logger.server"; -import { requireUser } from "~/services/session.server"; +import { hasAdminDisplayAccess, requireUser } from "~/services/session.server"; import { tenantContext } from "~/services/tenantContext.server"; import { EnvironmentParamSchema, v3ProjectPath } from "~/utils/pathBuilder"; import { canAccessDashboardAgent } from "~/v3/canAccessDashboardAgent.server"; @@ -91,10 +91,13 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => { // the launcher button is hidden when it's not enabled. The org's featureFlags // came from the membership-checked project query above, so we pass them in to // avoid a second org lookup. + // Display-only, so it respects the "view as user" toggle: while that's on we + // hide the launcher an impersonated-into user wouldn't have. + const showAdminUi = hasAdminDisplayAccess(user); const hasDashboardAgentAccess = await canAccessDashboardAgent({ userId: user.id, - isAdmin: user.admin, - isImpersonating: user.isImpersonating, + isAdmin: showAdminUi && user.admin, + isImpersonating: showAdminUi && user.isImpersonating, organizationSlug, orgFeatureFlags: (project.organization.featureFlags as Record) ?? {}, }); diff --git a/apps/webapp/app/routes/resources.impersonation_.view-as.ts b/apps/webapp/app/routes/resources.impersonation_.view-as.ts new file mode 100644 index 00000000000..ab1ea68baf6 --- /dev/null +++ b/apps/webapp/app/routes/resources.impersonation_.view-as.ts @@ -0,0 +1,50 @@ +import { redirect, type ActionFunctionArgs } from "@remix-run/server-runtime"; +import { z } from "zod"; +import { env } from "~/env.server"; +import { commitImpersonationSession, setViewingAsUser } from "~/services/impersonation.server"; +import { logger } from "~/services/logger.server"; +import { requireUser } from "~/services/session.server"; +import { sanitizeRedirectPath } from "~/utils"; +import { isSameOriginNavigation } from "~/utils/sameOriginNavigation"; + +const FormSchema = z.object({ + viewAsUser: z.enum(["true", "false"]), + redirectTo: z.string().optional(), +}); + +export async function action({ request }: ActionFunctionArgs) { + if (request.method.toLowerCase() !== "post") { + return new Response("Method not allowed", { status: 405 }); + } + + const user = await requireUser(request); + + // The toggle is submitted from our own side menu, so this holds. Applied here + // for the same reason as the other impersonation routes: no other site gets to + // drive this state change. + if (!isSameOriginNavigation(request, env.LOGIN_ORIGIN)) { + logger.warn("Refusing cross-site view-as-user submission", { + userId: user.id, + secFetchSite: request.headers.get("sec-fetch-site"), + }); + return redirect("/"); + } + + const payload = Object.fromEntries(await request.formData()); + const parsed = FormSchema.safeParse(payload); + const redirectTo = sanitizeRedirectPath(parsed.success ? parsed.data.redirectTo : undefined); + + // Display-only toggle scoped to an impersonation session — outside one there + // is nothing to toggle. + if (!user.isImpersonating || !parsed.success) { + return redirect(redirectTo); + } + + const session = await setViewingAsUser(parsed.data.viewAsUser === "true", request); + + return redirect(redirectTo, { + headers: { + "Set-Cookie": await commitImpersonationSession(session), + }, + }); +} diff --git a/apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.bulkaction.tsx b/apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.bulkaction.tsx index 42631d5ff45..5bef068b994 100644 --- a/apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.bulkaction.tsx +++ b/apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.bulkaction.tsx @@ -84,6 +84,12 @@ export const loader = dashboardLoader( throw new Response("Not Found", { status: 404 }); } + // Raw impersonation, not `hasAdminDisplayAccess`: this list is the bulk + // action's "override region" picker, so it decides which region a submitted + // bulk replay can re-route runs to. "View as user" only changes what is + // shown. + const isAdmin = user.admin || user.isImpersonating; + const presenter = new CreateBulkActionPresenter(); const [data, regionsResult] = await Promise.all([ presenter.call({ @@ -96,7 +102,7 @@ export const loader = dashboardLoader( new RegionsPresenter().call({ userId: user.id, projectSlug: projectParam, - isAdmin: user.admin || user.isImpersonating, + isAdmin, }) ), ]); diff --git a/apps/webapp/app/routes/resources.taskruns.$runParam.replay.ts b/apps/webapp/app/routes/resources.taskruns.$runParam.replay.ts index d6fe04be99f..e91ac2c5451 100644 --- a/apps/webapp/app/routes/resources.taskruns.$runParam.replay.ts +++ b/apps/webapp/app/routes/resources.taskruns.$runParam.replay.ts @@ -166,6 +166,11 @@ export async function loader({ request, params }: LoaderFunctionArgs) { const [payload, regionsResult] = await Promise.all([ prettyPrintPacket(run.payload, run.payloadType), + // Raw impersonation, not `hasAdminDisplayAccess`: this list is the replay + // dialog's region picker, so it decides what the submitted form can select + // — and the run's own region is returned separately, so dropping entries + // can leave the current value off the list. "View as user" only changes + // what is shown. new RegionsPresenter().call({ userId, projectSlug, diff --git a/apps/webapp/app/services/impersonation.server.ts b/apps/webapp/app/services/impersonation.server.ts index a4f3ee59c9f..e69a3fb305b 100644 --- a/apps/webapp/app/services/impersonation.server.ts +++ b/apps/webapp/app/services/impersonation.server.ts @@ -5,6 +5,7 @@ import { singleton } from "~/utils/singleton"; import { createRedisClient, type RedisClient } from "~/redis.server"; import { env } from "~/env.server"; import { logger } from "~/services/logger.server"; +import { resolveImpersonationState, type ImpersonationState } from "~/utils/impersonationState"; export const impersonationSessionStorage = createCookieSessionStorage({ cookie: { @@ -18,6 +19,15 @@ export const impersonationSessionStorage = createCookieSessionStorage({ }, }); +const IMPERSONATED_USER_ID_KEY = "impersonatedUserId"; + +/** + * Display-only "view as user" flag. It lives on the impersonation cookie so it + * is scoped to the impersonation session by construction: stop impersonating + * and the flag goes with it. + */ +const VIEWING_AS_USER_KEY = "viewingAsUser"; + export function getImpersonationSession(request: Request) { return impersonationSessionStorage.getSession(request.headers.get("Cookie")); } @@ -29,13 +39,13 @@ export function commitImpersonationSession(session: Session) { export async function getImpersonationId(request: Request) { const session = await getImpersonationSession(request); - return session.get("impersonatedUserId") as string | undefined; + return session.get(IMPERSONATED_USER_ID_KEY) as string | undefined; } export async function setImpersonationId(userId: string, request: Request) { const session = await getImpersonationSession(request); - session.set("impersonatedUserId", userId); + session.set(IMPERSONATED_USER_ID_KEY, userId); return session; } @@ -43,7 +53,44 @@ export async function setImpersonationId(userId: string, request: Request) { export async function clearImpersonationId(request: Request) { const session = await getImpersonationSession(request); - session.unset("impersonatedUserId"); + session.unset(IMPERSONATED_USER_ID_KEY); + // The view-as-user flag only means anything inside an impersonation session, + // so it never outlives one. + session.unset(VIEWING_AS_USER_KEY); + + return session; +} + +/** + * The impersonation state for a request, resolved against `resolvedUserId` — the + * id the request actually authenticated as (what `getUser`/`getUserId` return). + * + * This is the one place the impersonation flags come from, so the values the + * server computes for a route and the value the root loader publishes to the + * client cannot disagree. See `resolveImpersonationState` for why the + * impersonated id has to match the resolved user rather than merely be present. + */ +export async function getImpersonationState( + request: Request, + resolvedUserId: string | undefined +): Promise { + const session = await getImpersonationSession(request); + + return resolveImpersonationState({ + impersonatedUserId: session.get(IMPERSONATED_USER_ID_KEY), + viewingAsUser: session.get(VIEWING_AS_USER_KEY), + resolvedUserId, + }); +} + +export async function setViewingAsUser(value: boolean, request: Request) { + const session = await getImpersonationSession(request); + + if (value) { + session.set(VIEWING_AS_USER_KEY, true); + } else { + session.unset(VIEWING_AS_USER_KEY); + } return session; } diff --git a/apps/webapp/app/services/session.server.ts b/apps/webapp/app/services/session.server.ts index bdd565cf2f9..753bc7f6a17 100644 --- a/apps/webapp/app/services/session.server.ts +++ b/apps/webapp/app/services/session.server.ts @@ -3,7 +3,7 @@ import { getUserById } from "~/models/user.server"; import { sanitizeRedirectPath } from "~/utils"; import { extractClientIp } from "~/utils/extractClientIp.server"; import { authenticator } from "./auth.server"; -import { getImpersonationId } from "./impersonation.server"; +import { getImpersonationId, getImpersonationState } from "./impersonation.server"; import { logger } from "./logger.server"; import { revalidateSsoSession } from "./ssoSessionRevalidation.server"; @@ -135,7 +135,9 @@ export async function requireUser(request: Request) { throw redirect(`/login?${searchParams}`); } - const impersonationId = await getImpersonationId(request); + // Shared with the root loader so the client never reads a different answer + // than the one computed here. + const { isImpersonating, isViewingAsUser } = await getImpersonationState(request, user.id); return { id: user.id, email: user.email, @@ -148,10 +150,39 @@ export async function requireUser(request: Request) { dashboardPreferences: user.dashboardPreferences, confirmedBasicDetails: user.confirmedBasicDetails, mfaEnabledAt: user.mfaEnabledAt, - isImpersonating: !!impersonationId && impersonationId === user.id, + isImpersonating, + isViewingAsUser, }; } +/** + * Whether admin-only UI should be rendered for this user. + * + * Display only. The "view as user" toggle is cosmetic and must never widen or + * narrow a real security boundary — authorization stays on `user.admin`, the + * route builder's `authorization` block and the per-feature access checks. + * + * The rule: the toggle changes what is *shown*, never what is *permitted or + * what happens*. "Shown" is narrow — rendering and read-only listings. A badge, + * a debug tooltip, an extra table column, a control that is merely hidden while + * the handler behind it re-checks the raw flags: all fine. + * + * It does NOT extend to a value a request handler reads, nor to the option set + * of a control that submits. An option list feeding a mutation is not display: + * shrinking it changes what a submitted form is able to do, and where the + * current value is not among the remaining options it can change which value + * the form carries. Those stay on raw `user.admin || user.isImpersonating`, or + * the admin's own submissions start behaving differently — or failing — the + * moment they flip the toggle on. + */ +export function hasAdminDisplayAccess(user: { + admin: boolean; + isImpersonating: boolean; + isViewingAsUser: boolean; +}): boolean { + return (user.admin || user.isImpersonating) && !user.isViewingAsUser; +} + export async function logout(request: Request) { return redirect("/logout"); } diff --git a/apps/webapp/app/utils/impersonationPaths.test.ts b/apps/webapp/app/utils/impersonationPaths.test.ts new file mode 100644 index 00000000000..e3dba8c8241 --- /dev/null +++ b/apps/webapp/app/utils/impersonationPaths.test.ts @@ -0,0 +1,119 @@ +import { matchRoutes, resolveTo } from "@remix-run/router"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; +import { describe, expect, it } from "vitest"; +import { impersonationConsentPostBackPath, impersonationDestinationPath } from "./pathBuilder"; + +// The route that renders the impersonation consent page, as the flat-route +// convention compiles `_app.@.orgs.$organizationSlug.$.tsx`. +const CONSENT_ROUTES = [ + { + id: "routes/_app", + children: [ + { + id: "routes/_app.@.orgs.$organizationSlug.$", + path: "@/orgs/:organizationSlug/*", + }, + ], + }, +]; + +/** + * What the consent route's `action` would receive for a POST to `pathname`: + * the organization slug and the splat, straight off the router. + */ +function actionParamsFor(pathname: string) { + const matches = matchRoutes(CONSENT_ROUTES, pathname); + const leaf = matches?.[matches.length - 1]; + return { + organizationSlug: leaf?.params.organizationSlug, + splat: leaf?.params["*"], + }; +} + +/** + * What `
` with NO `action` prop resolves to, reproducing + * `useFormAction()`: `resolveTo(".", …)` over the matched routes' contributing + * pathnames. This app does not enable `future.v3_relativeSplatPath`, so each + * contributing match supplies its `pathnameBase`. + */ +function relativeFormAction(pathname: string) { + const matches = matchRoutes(CONSENT_ROUTES, pathname) ?? []; + const contributing = matches.filter((m, i) => i === 0 || Boolean(m.route.path)); + const routePathnames = contributing.map((m) => m.pathnameBase); + return resolveTo(".", routePathnames, pathname, false).pathname; +} + +describe("impersonation consent post-back path", () => { + const slug = "acme-inc"; + // A `/@/runs/` link redirects here: a deep run path plus the `?span=` + // that selects which span to open. + const splat = "projects/proj_123/env/prod/runs/run_abc"; + const search = "?span=span_xyz"; + + it("keeps the splat and the query string", () => { + expect(impersonationConsentPostBackPath(slug, splat, search)).toBe( + `/@/orgs/${slug}/${splat}${search}` + ); + expect(impersonationDestinationPath(slug, splat, search)).toBe( + `/orgs/${slug}/${splat}${search}` + ); + }); + + it("omits the query string when there isn't one", () => { + expect(impersonationConsentPostBackPath(slug, splat)).toBe(`/@/orgs/${slug}/${splat}`); + expect(impersonationDestinationPath(slug, splat)).toBe(`/orgs/${slug}/${splat}`); + }); + + it("round-trips the slug and splat back to the action", () => { + const postBackPath = impersonationConsentPostBackPath(slug, splat, search); + + expect(actionParamsFor(new URL(postBackPath, "http://localhost").pathname)).toEqual({ + organizationSlug: slug, + splat, + }); + }); + + it("the destination the action redirects to matches the one the page displayed", () => { + const postBackPath = impersonationConsentPostBackPath(slug, splat, search); + const url = new URL(postBackPath, "http://localhost"); + const params = actionParamsFor(url.pathname); + + // What `startImpersonation` builds from the action's params + request URL. + expect(impersonationDestinationPath(params.organizationSlug!, params.splat!, url.search)).toBe( + impersonationDestinationPath(slug, splat, search) + ); + }); + + // The regression this file exists for. A relative `` action drops the + // splat, so the action would start impersonation and land the admin on the + // organization root rather than the deep link the consent page promised. + it("a relative form action would drop the splat, so the path must be explicit", () => { + const consentUrl = `/@/orgs/${slug}/${splat}`; + + expect(relativeFormAction(consentUrl)).toBe(`/@/orgs/${slug}`); + expect(actionParamsFor(relativeFormAction(consentUrl))).toEqual({ + organizationSlug: slug, + splat: "", + }); + + // The explicit path does not. + expect(impersonationConsentPostBackPath(slug, splat)).toBe(consentUrl); + }); + + // Ideally this would render the route and read the emitted `action` + // attribute, but the route module imports `~/env.server`, which validates the + // full server environment at import time and so cannot be pulled into a unit + // test. Asserting on the source is the next best guard: it fails if the + // `action` prop is ever dropped, which is the whole bug. + it("the consent form names its action explicitly", () => { + const source = readFileSync( + join(__dirname, "../routes/_app.@.orgs.$organizationSlug.$.tsx"), + "utf8" + ); + + const form = source.match(/]*>/); + expect(form).not.toBeNull(); + expect(form![0]).toMatch(/action=\{postBackPath\}/); + }); +}); diff --git a/apps/webapp/app/utils/impersonationState.test.ts b/apps/webapp/app/utils/impersonationState.test.ts new file mode 100644 index 00000000000..b6fe24bf7e0 --- /dev/null +++ b/apps/webapp/app/utils/impersonationState.test.ts @@ -0,0 +1,71 @@ +import { describe, expect, it } from "vitest"; +import { resolveImpersonationState } from "./impersonationState"; + +const IMPERSONATED = "user_1"; +const ADMIN = "admin_1"; + +describe("resolveImpersonationState", () => { + it("reports impersonation when the cookie's id is the resolved user", () => { + expect( + resolveImpersonationState({ + impersonatedUserId: IMPERSONATED, + viewingAsUser: undefined, + resolvedUserId: IMPERSONATED, + }) + ).toEqual({ isImpersonating: true, isViewingAsUser: false }); + }); + + it("carries the view-as-user flag inside an impersonation session", () => { + expect( + resolveImpersonationState({ + impersonatedUserId: IMPERSONATED, + viewingAsUser: true, + resolvedUserId: IMPERSONATED, + }) + ).toEqual({ isImpersonating: true, isViewingAsUser: true }); + }); + + // The case the strict comparison exists for: the session falls back to the + // real admin's id when their admin role is revoked mid-session, while the + // cookie still names the impersonation target. Both flags must read false so + // the server-side values and the value published to the client agree. + it("reports neither flag when the impersonated id is not the resolved user", () => { + expect( + resolveImpersonationState({ + impersonatedUserId: IMPERSONATED, + viewingAsUser: true, + resolvedUserId: ADMIN, + }) + ).toEqual({ isImpersonating: false, isViewingAsUser: false }); + }); + + it("reports neither flag when there is no impersonated id", () => { + expect( + resolveImpersonationState({ + impersonatedUserId: undefined, + viewingAsUser: true, + resolvedUserId: IMPERSONATED, + }) + ).toEqual({ isImpersonating: false, isViewingAsUser: false }); + }); + + it("reports neither flag when there is no resolved user", () => { + expect( + resolveImpersonationState({ + impersonatedUserId: IMPERSONATED, + viewingAsUser: true, + resolvedUserId: undefined, + }) + ).toEqual({ isImpersonating: false, isViewingAsUser: false }); + }); + + it("only treats a literal true as the view-as-user flag", () => { + expect( + resolveImpersonationState({ + impersonatedUserId: IMPERSONATED, + viewingAsUser: "true", + resolvedUserId: IMPERSONATED, + }).isViewingAsUser + ).toBe(false); + }); +}); diff --git a/apps/webapp/app/utils/impersonationState.ts b/apps/webapp/app/utils/impersonationState.ts new file mode 100644 index 00000000000..2eca8efefe9 --- /dev/null +++ b/apps/webapp/app/utils/impersonationState.ts @@ -0,0 +1,47 @@ +/** + * The rule for reading impersonation state off the impersonation cookie. + * + * Kept pure and free of server-only imports so it can be unit tested directly, + * and so there is exactly one definition of "this request is impersonating" for + * every caller to share. + */ + +export type ImpersonationState = { + isImpersonating: boolean; + isViewingAsUser: boolean; +}; + +/** + * Resolves the impersonation cookie's raw contents against the identity the + * request actually authenticated as. + * + * Matching the impersonated id against `resolvedUserId` is deliberate. When an + * admin's role is revoked mid-session the session falls back to the real admin's + * id while the cookie still names the impersonation target, so "an impersonated + * id is present" and "this request is impersonating" stop meaning the same + * thing. Only the strict reading is correct there: that session is no longer + * impersonating, and so it is not viewing as the user either. + * + * Every consumer has to agree on this, or the flags computed on the server and + * the flag published to the client drift apart — the admin chrome would hide + * itself on a session that is not impersonating at all. + */ +export function resolveImpersonationState(options: { + impersonatedUserId: unknown; + viewingAsUser: unknown; + resolvedUserId: string | undefined; +}): ImpersonationState { + const { impersonatedUserId, viewingAsUser, resolvedUserId } = options; + + const isImpersonating = + typeof impersonatedUserId === "string" && + resolvedUserId !== undefined && + impersonatedUserId === resolvedUserId; + + return { + isImpersonating, + // Display only, and meaningless outside an impersonation session, so it + // never reads as on without one. + isViewingAsUser: isImpersonating && viewingAsUser === true, + }; +} diff --git a/apps/webapp/app/utils/pathBuilder.ts b/apps/webapp/app/utils/pathBuilder.ts index edd65f8bde4..d50d9e10f30 100644 --- a/apps/webapp/app/utils/pathBuilder.ts +++ b/apps/webapp/app/utils/pathBuilder.ts @@ -62,6 +62,41 @@ export function impersonate(path: string) { return `/@${path}`; } +/** + * Where a `/@/orgs//` impersonation link lands once impersonation + * has started: the same deep link with the `/@` prefix stripped. + * + * `search` must be carried through explicitly. A `/@/runs/` link redirects + * to a `v3RunSpanPath`, whose `?span=` selects the span to open, so + * dropping it lands the admin on the run with nothing selected. + */ +export function impersonationDestinationPath( + organizationSlug: string, + splatPath: string, + search: string = "" +) { + return `/orgs/${organizationSlug}/${splatPath}${search}`; +} + +/** + * Where the impersonation consent page's form must POST back to. + * + * The form has to name this path explicitly. A `` with no `action` + * resolves to `useResolvedPath(".")`, and because this app does not enable + * `future.v3_relativeSplatPath`, that resolves to the matched route's + * `pathnameBase` — which excludes the splat. The form would post to + * `/@/orgs/`, the action would see an empty splat, and the admin would + * land on the organization root instead of the deep link the consent page just + * promised them. + */ +export function impersonationConsentPostBackPath( + organizationSlug: string, + splatPath: string, + search: string = "" +) { + return impersonate(impersonationDestinationPath(organizationSlug, splatPath, search)); +} + export function accountPath() { return `/account`; } diff --git a/apps/webapp/test/impersonationConsent.test.ts b/apps/webapp/test/impersonationConsent.test.ts new file mode 100644 index 00000000000..a481338e245 --- /dev/null +++ b/apps/webapp/test/impersonationConsent.test.ts @@ -0,0 +1,153 @@ +import { containerTest } from "@internal/testcontainers"; +import { describe, expect, vi } from "vitest"; +import { findImpersonationTarget, startImpersonation } from "~/models/admin.server"; + +vi.setConfig({ testTimeout: 30_000 }); + +function suffix() { + return Math.random().toString(36).slice(2, 10); +} + +// A cross-site `/@/orgs//…` navigation renders a consent page instead of +// starting impersonation, so the only work its loader does is the read-only +// target lookup. Lock that the lookup writes nothing, and that the explicit +// POST is what actually starts impersonation. +describe("impersonation consent page", () => { + containerTest( + "resolving who a link would impersonate is read-only, and the explicit POST starts impersonation", + async ({ prisma }) => { + const admin = await prisma.user.create({ + data: { + email: `admin-${suffix()}@test.local`, + authenticationMethod: "MAGIC_LINK", + admin: true, + }, + }); + + // First member has never confirmed their details, so it must be skipped. + const unconfirmed = await prisma.user.create({ + data: { + email: `unconfirmed-${suffix()}@test.local`, + authenticationMethod: "MAGIC_LINK", + confirmedBasicDetails: false, + }, + }); + const confirmed = await prisma.user.create({ + data: { + email: `confirmed-${suffix()}@test.local`, + authenticationMethod: "MAGIC_LINK", + confirmedBasicDetails: true, + }, + }); + + const slug = `acme-${suffix()}`; + const org = await prisma.organization.create({ + data: { + title: "Acme Inc", + slug, + members: { create: [{ userId: unconfirmed.id }, { userId: confirmed.id }] }, + }, + }); + + // What the consent-page loader does: look up the target, nothing else. + const target = await findImpersonationTarget(org.slug, prisma); + expect(target).toEqual({ + success: true, + userId: confirmed.id, + organizationName: "Acme Inc", + }); + + // Read-only: no impersonation was recorded by rendering the page. + expect(await prisma.impersonationAuditLog.count()).toBe(0); + + // The consent page's POST: same-origin, and it does start impersonation. + // The `?span=` is what a `/@/runs/` link redirects with, so it has to + // survive to the destination or the run opens with no span selected. + const response = await startImpersonation( + new Request( + `http://localhost:3030/@/orgs/${org.slug}/projects/p/runs/run_123?span=span_abc`, + { + method: "POST", + headers: { "sec-fetch-site": "same-origin" }, + } + ), + org.slug, + "projects/p/runs/run_123", + { id: admin.id, admin: true }, + { read: prisma, write: prisma } + ); + + expect(response.status).toBe(302); + // The splat path and query string are preserved, `/@` prefix stripped. + expect(response.headers.get("location")).toBe( + `/orgs/${org.slug}/projects/p/runs/run_123?span=span_abc` + ); + expect(response.headers.get("set-cookie")).toContain("__impersonate="); + + const auditLogs = await prisma.impersonationAuditLog.findMany(); + expect(auditLogs).toHaveLength(1); + expect(auditLogs[0]).toMatchObject({ + action: "START", + adminId: admin.id, + targetId: confirmed.id, + }); + } + ); + + containerTest("an unknown organization slug cannot be impersonated", async ({ prisma }) => { + expect(await findImpersonationTarget(`missing-${suffix()}`, prisma)).toEqual({ + success: false, + reason: "org-not-found", + }); + }); + + containerTest( + "an organization with no confirmed members cannot be impersonated", + async ({ prisma }) => { + const user = await prisma.user.create({ + data: { + email: `unconfirmed-${suffix()}@test.local`, + authenticationMethod: "MAGIC_LINK", + confirmedBasicDetails: false, + }, + }); + + const org = await prisma.organization.create({ + data: { + title: "Nobody Inc", + slug: `nobody-${suffix()}`, + members: { create: [{ userId: user.id }] }, + }, + }); + + expect(await findImpersonationTarget(org.slug, prisma)).toEqual({ + success: false, + reason: "no-confirmed-member", + }); + } + ); + + containerTest("a deleted organization cannot be impersonated", async ({ prisma }) => { + const user = await prisma.user.create({ + data: { + email: `confirmed-${suffix()}@test.local`, + authenticationMethod: "MAGIC_LINK", + confirmedBasicDetails: true, + }, + }); + + const org = await prisma.organization.create({ + data: { + title: "Gone Inc", + slug: `gone-${suffix()}`, + deletedAt: new Date(), + members: { create: [{ userId: user.id }] }, + }, + }); + + expect(await findImpersonationTarget(org.slug, prisma)).toEqual({ + success: false, + reason: "org-not-found", + }); + }); +}); diff --git a/apps/webapp/test/runGetRoutes.replicaLag.guard.test.ts b/apps/webapp/test/runGetRoutes.replicaLag.guard.test.ts index 8d844f550b3..f6597980f39 100644 --- a/apps/webapp/test/runGetRoutes.replicaLag.guard.test.ts +++ b/apps/webapp/test/runGetRoutes.replicaLag.guard.test.ts @@ -36,7 +36,12 @@ vi.setConfig({ testTimeout: 120_000, hookTimeout: 120_000 }); // `authUser`, `resolvedEnv`, `logDetail`, `project`, `environment` feed the mocked fix-orthogonal deps. const cp = vi.hoisted(() => ({ client: undefined as any })); const router = vi.hoisted(() => ({ store: undefined as any })); -const authUser = vi.hoisted(() => ({ id: "user_rrg_guard", admin: false, isImpersonating: false })); +const authUser = vi.hoisted(() => ({ + id: "user_rrg_guard", + admin: false, + isImpersonating: false, + isViewingAsUser: false, +})); const resolved = vi.hoisted(() => ({ authEnv: undefined as any, env: undefined as any, @@ -104,7 +109,10 @@ vi.mock("~/v3/runStore.server", () => ({ ), })); -// Auth/session (orthogonal): fixed user id. +// Auth/session (orthogonal): fixed user id. This factory replaces the whole module, so it has to +// return every export the route graph under test actually reaches — accessing one it does not +// define throws. Keep it to that set; importing the original would evaluate session.server's +// server-only import graph, which this test deliberately keeps out. vi.mock("~/services/session.server", () => ({ requireUserId: async () => authUser.id, requireUser: async () => authUser, diff --git a/apps/webapp/test/viewAsUser.test.ts b/apps/webapp/test/viewAsUser.test.ts new file mode 100644 index 00000000000..d21d53f6bc6 --- /dev/null +++ b/apps/webapp/test/viewAsUser.test.ts @@ -0,0 +1,109 @@ +import { describe, expect, it } from "vitest"; +import { + clearImpersonationId, + commitImpersonationSession, + getImpersonationId, + getImpersonationState, + setImpersonationId, + setViewingAsUser, +} from "~/services/impersonation.server"; +import { hasAdminDisplayAccess } from "~/services/session.server"; +import type { Session } from "@remix-run/node"; + +// `commitSession` returns a full Set-Cookie value; a request only needs the +// name=value pair back. +async function requestWith(session: Session) { + const setCookie = await commitImpersonationSession(session); + return new Request("http://localhost:3030/orgs/acme", { + headers: { Cookie: setCookie.split(";")[0] }, + }); +} + +// Reads the flag the way a request does: against the user the request resolved +// as. Every fixture below impersonates `user_1`. +async function readViewingAsUser(request: Request) { + return (await getImpersonationState(request, "user_1")).isViewingAsUser; +} + +// The "view as user" flag rides on the impersonation cookie, so it is scoped to +// the impersonation session by construction. +describe("view as user flag", () => { + it("is off when there is no impersonation cookie", async () => { + expect(await readViewingAsUser(new Request("http://localhost:3030/orgs/acme"))).toBe(false); + }); + + it("can be turned on and back off within an impersonation session", async () => { + const start = new Request("http://localhost:3030/orgs/acme"); + + const impersonating = await requestWith(await setImpersonationId("user_1", start)); + expect(await getImpersonationId(impersonating)).toBe("user_1"); + expect(await readViewingAsUser(impersonating)).toBe(false); + + const viewingAsUser = await requestWith(await setViewingAsUser(true, impersonating)); + expect(await getImpersonationId(viewingAsUser)).toBe("user_1"); + expect(await readViewingAsUser(viewingAsUser)).toBe(true); + + const showingAdminUi = await requestWith(await setViewingAsUser(false, viewingAsUser)); + expect(await getImpersonationId(showingAdminUi)).toBe("user_1"); + expect(await readViewingAsUser(showingAdminUi)).toBe(false); + }); + + it("is dropped when impersonation is cleared", async () => { + const start = new Request("http://localhost:3030/orgs/acme"); + const impersonating = await requestWith(await setImpersonationId("user_1", start)); + const viewingAsUser = await requestWith(await setViewingAsUser(true, impersonating)); + + const cleared = await requestWith(await clearImpersonationId(viewingAsUser)); + + expect(await getImpersonationId(cleared)).toBeUndefined(); + expect(await readViewingAsUser(cleared)).toBe(false); + }); + + it("never reads as on without an impersonated user", async () => { + const start = new Request("http://localhost:3030/orgs/acme"); + // Set the flag with no impersonation in progress — it must not read back on. + const flagOnly = await requestWith(await setViewingAsUser(true, start)); + + expect(await readViewingAsUser(flagOnly)).toBe(false); + }); + + it("is off once the impersonated user is not who the request resolved as", async () => { + const start = new Request("http://localhost:3030/orgs/acme"); + const impersonating = await requestWith(await setImpersonationId("user_1", start)); + const viewing = await requestWith(await setViewingAsUser(true, impersonating)); + + // An admin who loses the admin role mid-session resolves back to their own + // id while the cookie still names the target. That session is not + // impersonating any more, so it is not viewing as the user either. + const state = await getImpersonationState(viewing, "admin_1"); + + expect(state.isImpersonating).toBe(false); + expect(state.isViewingAsUser).toBe(false); + }); +}); + +describe("hasAdminDisplayAccess", () => { + it("shows admin UI to admins and to impersonating sessions", () => { + expect( + hasAdminDisplayAccess({ admin: true, isImpersonating: false, isViewingAsUser: false }) + ).toBe(true); + expect( + hasAdminDisplayAccess({ admin: false, isImpersonating: true, isViewingAsUser: false }) + ).toBe(true); + }); + + it("hides admin UI to everyone else", () => { + expect( + hasAdminDisplayAccess({ admin: false, isImpersonating: false, isViewingAsUser: false }) + ).toBe(false); + }); + + it("hides admin UI while viewing as the user", () => { + expect( + hasAdminDisplayAccess({ admin: true, isImpersonating: true, isViewingAsUser: true }) + ).toBe(false); + expect( + hasAdminDisplayAccess({ admin: false, isImpersonating: true, isViewingAsUser: true }) + ).toBe(false); + }); +});