From 192674a1dbe5c3598155fbd9789e39c261a4c572 Mon Sep 17 00:00:00 2001 From: dotta Date: Wed, 1 Apr 2026 08:36:25 -0500 Subject: [PATCH] Tighten authenticated company access Restrict normal board company access to explicit memberships, expose authenticated user identity in the session UI, and cover the changes with server and authenticated E2E tests. Co-Authored-By: Paperclip --- .../src/__tests__/auth-session-route.test.ts | 61 +++++++++ .../__tests__/authz-company-access.test.ts | 30 +++++ .../__tests__/company-branding-route.test.ts | 49 +++++++ server/src/app.ts | 4 +- server/src/middleware/auth.ts | 13 +- server/src/routes/authz.ts | 2 +- server/src/routes/companies.ts | 4 +- server/src/types/express.d.ts | 2 + tests/e2e/multi-user-authenticated.spec.ts | 4 + ui/src/components/Layout.tsx | 125 +++++++++++++++--- 10 files changed, 268 insertions(+), 26 deletions(-) create mode 100644 server/src/__tests__/auth-session-route.test.ts diff --git a/server/src/__tests__/auth-session-route.test.ts b/server/src/__tests__/auth-session-route.test.ts new file mode 100644 index 00000000000..91eeb8a07f2 --- /dev/null +++ b/server/src/__tests__/auth-session-route.test.ts @@ -0,0 +1,61 @@ +import express from "express"; +import request from "supertest"; +import { describe, expect, it, vi } from "vitest"; +import { actorMiddleware } from "../middleware/auth.js"; + +function createSelectChain(rows: unknown[]) { + return { + from() { + return { + where() { + return Promise.resolve(rows); + }, + }; + }, + }; +} + +function createDb() { + return { + select: vi + .fn() + .mockImplementationOnce(() => createSelectChain([])) + .mockImplementationOnce(() => createSelectChain([])), + } as any; +} + +describe("actorMiddleware authenticated session profile", () => { + it("preserves the signed-in user name and email on the board actor", async () => { + const app = express(); + app.use( + actorMiddleware(createDb(), { + deploymentMode: "authenticated", + resolveSession: async () => ({ + session: { id: "session-1", userId: "user-1" }, + user: { + id: "user-1", + name: "User One", + email: "user@example.com", + }, + }), + }), + ); + app.get("/actor", (req, res) => { + res.json(req.actor); + }); + + const res = await request(app).get("/actor"); + + expect(res.status).toBe(200); + expect(res.body).toMatchObject({ + type: "board", + userId: "user-1", + userName: "User One", + userEmail: "user@example.com", + source: "session", + companyIds: [], + memberships: [], + isInstanceAdmin: false, + }); + }); +}); diff --git a/server/src/__tests__/authz-company-access.test.ts b/server/src/__tests__/authz-company-access.test.ts index 7b30399e187..37850c306ee 100644 --- a/server/src/__tests__/authz-company-access.test.ts +++ b/server/src/__tests__/authz-company-access.test.ts @@ -45,4 +45,34 @@ describe("assertCompanyAccess", () => { expect(() => assertCompanyAccess(req, "company-1")).toThrow("Viewer access is read-only"); }); + + it("rejects signed-in instance admins without explicit company access", () => { + const req = makeReq({ + method: "GET", + actor: { + type: "board", + userId: "admin-1", + source: "session", + isInstanceAdmin: true, + companyIds: [], + memberships: [], + }, + }); + + expect(() => assertCompanyAccess(req, "company-1")).toThrow("User does not have access to this company"); + }); + + it("allows local trusted board access without explicit membership", () => { + const req = makeReq({ + method: "GET", + actor: { + type: "board", + userId: "local-board", + source: "local_implicit", + isInstanceAdmin: true, + }, + }); + + expect(() => assertCompanyAccess(req, "company-1")).not.toThrow(); + }); }); diff --git a/server/src/__tests__/company-branding-route.test.ts b/server/src/__tests__/company-branding-route.test.ts index 86d9441cd55..6d8de8b73f8 100644 --- a/server/src/__tests__/company-branding-route.test.ts +++ b/server/src/__tests__/company-branding-route.test.ts @@ -194,3 +194,52 @@ describe("PATCH /api/companies/:companyId/branding", () => { expect(mockCompanyService.update).not.toHaveBeenCalled(); }); }); + +describe("GET /api/companies", () => { + beforeEach(() => { + mockCompanyService.list.mockReset(); + mockCompanyService.stats.mockReset(); + }); + + it("filters list results to explicit memberships for signed-in instance admins", async () => { + mockCompanyService.list.mockResolvedValue([ + { ...createCompany(), id: "company-1", issuePrefix: "PAP" }, + { ...createCompany(), id: "company-2", issuePrefix: "OPS" }, + ]); + const app = createApp({ + type: "board", + userId: "admin-1", + source: "session", + isInstanceAdmin: true, + companyIds: ["company-1"], + memberships: [{ companyId: "company-1", membershipRole: "owner", status: "active" }], + }); + + const res = await request(app).get("/api/companies"); + + expect(res.status).toBe(200); + expect(res.body.map((company: { id: string }) => company.id)).toEqual(["company-1"]); + }); + + it("filters stats results to explicit memberships for signed-in instance admins", async () => { + mockCompanyService.stats.mockResolvedValue({ + "company-1": { activeAgents: 2 }, + "company-2": { activeAgents: 1 }, + }); + const app = createApp({ + type: "board", + userId: "admin-1", + source: "session", + isInstanceAdmin: true, + companyIds: ["company-1"], + memberships: [{ companyId: "company-1", membershipRole: "owner", status: "active" }], + }); + + const res = await request(app).get("/api/companies/stats"); + + expect(res.status).toBe(200); + expect(res.body).toEqual({ + "company-1": { activeAgents: 2 }, + }); + }); +}); diff --git a/server/src/app.ts b/server/src/app.ts index 5535ab3df24..2ef02d175ac 100644 --- a/server/src/app.ts +++ b/server/src/app.ts @@ -117,8 +117,8 @@ export async function createApp( }, user: { id: req.actor.userId, - email: null, - name: req.actor.source === "local_implicit" ? "Local Board" : null, + email: req.actor.userEmail ?? null, + name: req.actor.userName ?? null, }, }); }); diff --git a/server/src/middleware/auth.ts b/server/src/middleware/auth.ts index ce1360b8ce7..2438aee3069 100644 --- a/server/src/middleware/auth.ts +++ b/server/src/middleware/auth.ts @@ -23,7 +23,14 @@ export function actorMiddleware(db: Db, opts: ActorMiddlewareOptions): RequestHa return async (req, _res, next) => { req.actor = opts.deploymentMode === "local_trusted" - ? { type: "board", userId: "local-board", isInstanceAdmin: true, source: "local_implicit" } + ? { + type: "board", + userId: "local-board", + userName: "Local Board", + userEmail: null, + isInstanceAdmin: true, + source: "local_implicit", + } : { type: "none", source: "none" }; const runIdHeader = req.header("x-paperclip-run-id"); @@ -66,6 +73,8 @@ export function actorMiddleware(db: Db, opts: ActorMiddlewareOptions): RequestHa req.actor = { type: "board", userId, + userName: session.user.name ?? null, + userEmail: session.user.email ?? null, companyIds: memberships.map((row) => row.companyId), memberships, isInstanceAdmin: Boolean(roleRow), @@ -95,6 +104,8 @@ export function actorMiddleware(db: Db, opts: ActorMiddlewareOptions): RequestHa req.actor = { type: "board", userId: boardKey.userId, + userName: access.user?.name ?? null, + userEmail: access.user?.email ?? null, companyIds: access.companyIds, memberships: access.memberships, isInstanceAdmin: access.isInstanceAdmin, diff --git a/server/src/routes/authz.ts b/server/src/routes/authz.ts index 3480da493ce..6bf519d8f33 100644 --- a/server/src/routes/authz.ts +++ b/server/src/routes/authz.ts @@ -22,7 +22,7 @@ export function assertCompanyAccess(req: Request, companyId: string) { if (req.actor.type === "agent" && req.actor.companyId !== companyId) { throw forbidden("Agent key cannot access another company"); } - if (req.actor.type === "board" && req.actor.source !== "local_implicit" && !req.actor.isInstanceAdmin) { + if (req.actor.type === "board" && req.actor.source !== "local_implicit") { const allowedCompanies = req.actor.companyIds ?? []; if (!allowedCompanies.includes(companyId)) { throw forbidden("User does not have access to this company"); diff --git a/server/src/routes/companies.ts b/server/src/routes/companies.ts index 5112baac8c5..1a449066d53 100644 --- a/server/src/routes/companies.ts +++ b/server/src/routes/companies.ts @@ -60,7 +60,7 @@ export function companyRoutes(db: Db, storage?: StorageService) { router.get("/", async (req, res) => { assertBoard(req); const result = await svc.list(); - if (req.actor.source === "local_implicit" || req.actor.isInstanceAdmin) { + if (req.actor.source === "local_implicit") { res.json(result); return; } @@ -70,7 +70,7 @@ export function companyRoutes(db: Db, storage?: StorageService) { router.get("/stats", async (req, res) => { assertBoard(req); - const allowed = req.actor.source === "local_implicit" || req.actor.isInstanceAdmin + const allowed = req.actor.source === "local_implicit" ? null : new Set(req.actor.companyIds ?? []); const stats = await svc.stats(); diff --git a/server/src/types/express.d.ts b/server/src/types/express.d.ts index 50c2c3cb617..1915e2ffdb1 100644 --- a/server/src/types/express.d.ts +++ b/server/src/types/express.d.ts @@ -6,6 +6,8 @@ declare global { actor: { type: "board" | "agent" | "none"; userId?: string; + userName?: string | null; + userEmail?: string | null; agentId?: string; companyId?: string; companyIds?: string[]; diff --git a/tests/e2e/multi-user-authenticated.spec.ts b/tests/e2e/multi-user-authenticated.spec.ts index 7055d206b37..2652da389f5 100644 --- a/tests/e2e/multi-user-authenticated.spec.ts +++ b/tests/e2e/multi-user-authenticated.spec.ts @@ -278,6 +278,10 @@ test.describe("Multi-user: authenticated mode", () => { const company = await createCompanyForSession(page, companyName); const companyPrefix = company.issuePrefix ?? company.id; + await page.goto(`${BASE}/${companyPrefix}/dashboard`); + await expect(page.getByTestId("layout-account-menu-trigger")).toContainText(ownerUser.name); + await page.getByTestId("layout-account-menu-trigger").click(); + await expect(page.getByText(ownerUser.email)).toBeVisible(); const inviteUrl = await createAuthenticatedInvite(page, companyPrefix); const invited = await newPage(browser); diff --git a/ui/src/components/Layout.tsx b/ui/src/components/Layout.tsx index 8761b71c9b1..03244c7a8e6 100644 --- a/ui/src/components/Layout.tsx +++ b/ui/src/components/Layout.tsx @@ -1,6 +1,6 @@ import { useCallback, useEffect, useMemo, useRef, useState } from "react"; -import { useQuery } from "@tanstack/react-query"; -import { BookOpen, Moon, Settings, Sun } from "lucide-react"; +import { useQuery, useQueryClient } from "@tanstack/react-query"; +import { BookOpen, LogOut, Moon, Settings, Sun } from "lucide-react"; import { Link, Outlet, useLocation, useNavigate, useParams } from "@/lib/router"; import { CompanyRail } from "./CompanyRail"; import { Sidebar } from "./Sidebar"; @@ -23,6 +23,7 @@ import { useSidebar } from "../context/SidebarContext"; import { useTheme } from "../context/ThemeContext"; import { useKeyboardShortcuts } from "../hooks/useKeyboardShortcuts"; import { useCompanyPageMemory } from "../hooks/useCompanyPageMemory"; +import { authApi, type AuthSession } from "../api/auth"; import { healthApi } from "../api/health"; import { shouldSyncCompanySelectionFromRoute } from "../lib/company-selection"; import { @@ -33,7 +34,16 @@ import { queryKeys } from "../lib/queryKeys"; import { cn } from "../lib/utils"; import { NotFoundPage } from "../pages/NotFound"; import { Button } from "@/components/ui/button"; +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuLabel, + DropdownMenuSeparator, + DropdownMenuTrigger, +} from "@/components/ui/dropdown-menu"; import { Tooltip, TooltipTrigger, TooltipContent } from "@/components/ui/tooltip"; +import { Identity } from "./Identity"; const INSTANCE_SETTINGS_MEMORY_KEY = "paperclip.lastInstanceSettingsPath"; @@ -46,7 +56,78 @@ function readRememberedInstanceSettingsPath(): string { } } +function getSessionDisplayName(session: AuthSession) { + return session.user.name?.trim() || session.user.email?.trim() || session.user.id; +} + +function FooterPrimaryAction({ + session, + onSignOut, + signingOut, +}: { + session: AuthSession | null; + onSignOut: () => Promise; + signingOut: boolean; +}) { + if (!session) { + return ( + + + Documentation + + ); + } + + const displayName = getSessionDisplayName(session); + + return ( + + + + + + +
{displayName}
+
+ {session.user.email?.trim() || session.user.id} +
+ + + + + + Documentation + + + { + void onSignOut(); + }} + > + + {signingOut ? "Signing out..." : "Sign out"} + + + + ); +} + export function Layout() { + const queryClient = useQueryClient(); const { sidebarOpen, setSidebarOpen, toggleSidebar, isMobile } = useSidebar(); const { openNewIssue, openOnboarding } = useDialog(); const { togglePanelVisible } = usePanel(); @@ -67,6 +148,7 @@ export function Layout() { const lastMainScrollTop = useRef(0); const [mobileNavVisible, setMobileNavVisible] = useState(true); const [instanceSettingsTarget, setInstanceSettingsTarget] = useState(() => readRememberedInstanceSettingsPath()); + const [isSigningOut, setIsSigningOut] = useState(false); const nextTheme = theme === "dark" ? "light" : "dark"; const matchedCompany = useMemo(() => { if (!companyPrefix) return null; @@ -85,6 +167,25 @@ export function Layout() { }, refetchIntervalInBackground: true, }); + const { data: session } = useQuery({ + queryKey: queryKeys.auth.session, + queryFn: () => authApi.getSession(), + enabled: health?.deploymentMode === "authenticated", + retry: false, + }); + + const handleSignOut = useCallback(async () => { + if (isSigningOut) return; + setIsSigningOut(true); + try { + await authApi.signOut(); + queryClient.setQueryData(queryKeys.auth.session, null); + await queryClient.invalidateQueries({ queryKey: queryKeys.companies.all }); + navigate("/auth", { replace: true }); + } finally { + setIsSigningOut(false); + } + }, [isSigningOut, navigate, queryClient]); useEffect(() => { if (companiesLoading || onboardingTriggered.current) return; @@ -296,15 +397,7 @@ export function Layout() {
- - - Documentation - + {health?.version && ( @@ -354,15 +447,7 @@ export function Layout() {
- - - Documentation - + {health?.version && (