From 7ff7fb0135307abb177b9d364d26fce6ea75a78b Mon Sep 17 00:00:00 2001 From: huyplb Date: Sun, 25 Oct 2026 09:28:23 -0600 Subject: [PATCH 1/3] wip: workspace review fixes (role ceiling, restore archived, Escape, sharing-limits e2e) --- apps/e2e/scripts/run-all.mjs | 4 +- apps/e2e/src/generated/test-ids.ts | 6 + .../tests/workspaces-sharing-limits.test.ts | 159 ++++++++++++++++++ .../components/AdminWorkspacesPanel.test.tsx | 18 +- .../admin/components/AdminWorkspacesPanel.tsx | 15 +- .../features/workspaces/api/workspacesApi.ts | 5 + .../components/WorkspaceRailButton.test.tsx | 10 +- .../components/WorkspaceRailButton.tsx | 10 +- .../WorkspaceSettingsDialog.test.tsx | 123 ++++++++++++++ .../components/WorkspaceSettingsDialog.tsx | 26 ++- .../src/frontend/features/workspaces/index.ts | 1 + docs/testing/TEST_IDS.md | 4 +- packages/server/src/api/http-contract.test.ts | 1 + .../workspace-directory.service.test.ts | 14 ++ .../workspaces/workspace-directory.service.ts | 36 +++- .../workspace-invites.service.test.ts | 3 +- .../workspaces/workspace-invites.service.ts | 11 +- .../workspaces/workspace-role-ceiling.test.ts | 144 ++++++++++++++++ .../features/workspaces/workspaces.routes.ts | 10 ++ .../platform/workspaces/workspace.service.ts | 77 ++++++++- packages/shared/src/permissions.ts | 10 ++ 21 files changed, 664 insertions(+), 23 deletions(-) create mode 100644 apps/e2e/src/tests/workspaces-sharing-limits.test.ts create mode 100644 apps/web/src/frontend/features/workspaces/components/WorkspaceSettingsDialog.test.tsx create mode 100644 packages/server/src/features/workspaces/workspace-role-ceiling.test.ts diff --git a/apps/e2e/scripts/run-all.mjs b/apps/e2e/scripts/run-all.mjs index 307f47991..0da73e8ee 100644 --- a/apps/e2e/scripts/run-all.mjs +++ b/apps/e2e/scripts/run-all.mjs @@ -107,8 +107,8 @@ const ALWAYS = [ }, { key: 'workspaces', - file: 'src/tests/workspaces.test.ts', - label: 'Workspaces: create, switch, settings', + file: 'src/tests/workspaces.test.ts src/tests/workspaces-sharing-limits.test.ts', + label: 'Workspaces: create, switch, settings, sharing limits', }, { key: 'sql-editor', diff --git a/apps/e2e/src/generated/test-ids.ts b/apps/e2e/src/generated/test-ids.ts index 0fb3edf09..166bd207d 100644 --- a/apps/e2e/src/generated/test-ids.ts +++ b/apps/e2e/src/generated/test-ids.ts @@ -639,6 +639,8 @@ export const TestIds = { adminWorkspaceMembers: (id: string | number) => `admin-workspace-members-${id}`, /** button · Open */ adminWorkspaceOpen: (id: string | number) => `admin-workspace-open-${id}`, + /** button · Restore */ + adminWorkspaceRestore: (id: string | number) => `admin-workspace-restore-${id}`, /** div */ adminWorkspaces: 'admin-workspaces', /** p */ @@ -3503,6 +3505,8 @@ export const TestIds = { WorkspaceSettingsDialog: { /** li */ workspaceInvite: (id: string | number) => `workspace-invite-${id}`, + /** p */ + workspaceInviteCeiling: 'workspace-invite-ceiling', /** input · teammate@company.com */ workspaceInviteEmail: 'workspace-invite-email', /** form */ @@ -3706,6 +3710,7 @@ export type TestId = | `admin-workspace-join-${string}` | `admin-workspace-members-${string}` | `admin-workspace-open-${string}` + | `admin-workspace-restore-${string}` | 'admin-workspaces' | 'admin-workspaces-error' | 'admin-workspaces-list' @@ -5151,6 +5156,7 @@ export type TestId = | `workflow-variables-environment-${string}` | 'workflow-view' | `workspace-invite-${string}` + | 'workspace-invite-ceiling' | 'workspace-invite-email' | 'workspace-invite-form' | 'workspace-invite-new-account' diff --git a/apps/e2e/src/tests/workspaces-sharing-limits.test.ts b/apps/e2e/src/tests/workspaces-sharing-limits.test.ts new file mode 100644 index 000000000..3cb5889cd --- /dev/null +++ b/apps/e2e/src/tests/workspaces-sharing-limits.test.ts @@ -0,0 +1,159 @@ +/** + * Sharing a workspace lends no more than the sharer holds, and an archived + * workspace can come back — in a real browser against the running server. + * + * A personal workspace's account runs who is in it whatever its own role, so + * before the ceiling a viewer account could invite a colleague as owner, and + * the colleague got DDL, DML and GRANT on the viewer's saved connections. + * + * Runs as the suites' admin plus two throwaway viewer accounts, and leaves the + * admin in its own workspace, where the other suites expect it. + */ +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { randomBytes } from 'node:crypto'; +import type { Page } from 'playwright'; +import { buildDriver, quitDriver, clickWhen, waitFor, BASE_URL } from '../helpers/driver.js'; +import { sessionCookie } from '../helpers/app-session.js'; +import { saveScreenshot } from '../helpers/screenshot.js'; +import { byTestId } from '../helpers/test-ids.js'; + +const API_URL = process.env.E2E_API_URL ?? 'http://127.0.0.1:3210'; +const RUN = Date.now().toString(36); +const SHARER_EMAIL = `e2e-wssharer-${RUN}@foxschema.test`; +const PEER_EMAIL = `e2e-wspeer-${RUN}@foxschema.test`; + +let admin = ''; +let adminPersonalId = ''; +let sharer = { id: '', cookie: '' }; +let peer = { id: '', cookie: '' }; +const created: string[] = []; +const pages: Page[] = []; + +async function api(method: string, path: string, body?: unknown, cookie = admin) { + const res = await fetch(`${API_URL}/api${path}`, { + method, + headers: { ...(body !== undefined ? { 'content-type': 'application/json' } : {}), cookie }, + ...(body !== undefined ? { body: JSON.stringify(body) } : {}), + }); + return { status: res.status, json: (await res.json().catch(() => null)) as any }; +} + +/** A viewer account, signed in: its id and session cookie. */ +async function viewerAccount(email: string): Promise<{ id: string; cookie: string }> { + const password = `ws-${randomBytes(8).toString('hex')}`; + const added = await api('POST', '/admin/users', { email, password, role: 'viewer' }); + expect(added.status, JSON.stringify(added.json)).toBe(200); + const login = await fetch(`${API_URL}/api/auth/login`, { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ email, password }), + }); + expect(login.status).toBe(200); + return { id: added.json.user.id, cookie: (login.headers.get('set-cookie') ?? '').split(';')[0]! }; +} + +/** A browser signed in with `cookie`, past onboarding. */ +async function browserFor(cookie: string): Promise { + const page = await buildDriver({ signedIn: false }); + pages.push(page); + const [name, ...rest] = cookie.split('='); + await page.context().addCookies([{ name: name!, value: rest.join('='), url: BASE_URL }]); + await page.goto(BASE_URL); + await page.waitForSelector(`${byTestId('onboarding-skip')}, ${byTestId('profile-menu-trigger')}`, { timeout: 30_000 }); + const skip = page.locator(byTestId('onboarding-skip')); + if (await skip.count()) await skip.click(); + return page; +} + +beforeAll(async () => { + admin = await sessionCookie(); + const list = await api('GET', '/workspaces'); + expect(list.status, JSON.stringify(list.json)).toBe(200); + adminPersonalId = list.json.workspaces.find((w: { personal: boolean }) => w.personal).id; + await api('POST', `/workspaces/${adminPersonalId}/select`, {}); + sharer = await viewerAccount(SHARER_EMAIL); + peer = await viewerAccount(PEER_EMAIL); +}, 120_000); + +afterAll(async () => { + for (const page of pages) await quitDriver(page); + for (const id of [sharer.id, peer.id]) if (id) await api('PUT', `/admin/users/${id}/active`, { active: false }); + for (const id of created) await api('POST', `/workspaces/${id}/archive`, {}); + await api('POST', `/workspaces/${adminPersonalId}/select`, {}); +}); + +describe('workspace sharing limits', () => { + it('a viewer sharing its own workspace is offered viewer, and the server refuses more', async () => { + const own = (await api('GET', '/workspaces', undefined, sharer.cookie)).json.workspaces.find( + (w: { personal: boolean }) => w.personal + ); + expect(own.role).toBe('viewer'); + + // The settings dialog offers what the server will accept, and says why. + const page = await browserFor(sharer.cookie); + await clickWhen(page, byTestId('workspace-rail-btn')); + await clickWhen(page, byTestId('workspace-menu-settings')); + await waitFor(page, byTestId('workspace-invite-role'), 15_000); + const offered = await page.evaluate(() => + [...(document.querySelector('[data-testid="workspace-invite-role"]') as HTMLSelectElement).options].map((o) => [ + o.value, + !o.disabled, + ]) + ); + expect(offered).toEqual([ + ['viewer', true], + ['editor', false], + ['owner', false], + ]); + expect(await page.locator(byTestId('workspace-invite-ceiling')).innerText()).toMatch(/up to viewer/); + await saveScreenshot(page, 'workspaces-sharing-ceiling'); + + // The server refuses the same, whatever the client sends. + for (const role of ['owner', 'editor']) { + const refused = await api('POST', `/workspaces/${own.id}/invites`, { email: PEER_EMAIL, role }, sharer.cookie); + expect(refused.status, `${role}: ${JSON.stringify(refused.json)}`).toBe(403); + } + const sent = await api('POST', `/workspaces/${own.id}/invites`, { email: PEER_EMAIL, role: 'viewer' }, sharer.cookie); + expect(sent.status, JSON.stringify(sent.json)).toBe(200); + + // The colleague joins as a viewer and acts as one: it can read, not write. + const accepted = await api('POST', `/workspaces/invites/${sent.json.invite.id}/accept`, {}, peer.cookie); + expect(accepted.status, JSON.stringify(accepted.json)).toBe(200); + expect((await api('POST', `/workspaces/${own.id}/select`, {}, peer.cookie)).status).toBe(200); + const me = (await api('GET', '/auth/me', undefined, peer.cookie)).json; + expect(me.workspace).toMatchObject({ id: own.id, role: 'viewer' }); + for (const p of ['editor.dml', 'editor.ddl', 'editor.grant', 'schema.migrate']) { + expect(me.user.permissions, p).not.toContain(p); + } + + // Nor can the sharer raise it afterwards. + const raised = await api('PUT', `/workspaces/${own.id}/members/${peer.id}`, { role: 'owner' }, sharer.cookie); + expect(raised.status, JSON.stringify(raised.json)).toBe(403); + }, 120_000); + + it('an admin restores an archived workspace from Access control', async () => { + const made = await api('POST', '/workspaces', { name: `e2e-restore-${RUN}` }); + expect(made.status, JSON.stringify(made.json)).toBe(200); + const id = made.json.workspace.id as string; + created.push(id); + expect((await api('POST', `/workspaces/${id}/archive`, {})).status).toBe(200); + expect((await api('GET', '/workspaces')).json.workspaces.some((w: { id: string }) => w.id === id)).toBe(false); + + const page = await buildDriver(); + pages.push(page); + await page.goto(BASE_URL); + await clickWhen(page, byTestId('profile-menu-trigger')); + await clickWhen(page, byTestId('profile-access-control')); + await clickWhen(page, byTestId('admin-tab-workspaces')); + await clickWhen(page, byTestId('admin-workspaces-show-archived')); + await clickWhen(page, byTestId(`admin-workspace-restore-${id}`)); + // Back: the row offers Members again instead of Restore. + await waitFor(page, byTestId(`admin-workspace-members-${id}`), 15_000); + expect(await page.locator(byTestId(`admin-workspace-restore-${id}`)).count()).toBe(0); + await saveScreenshot(page, 'workspaces-admin-restored'); + + expect((await api('GET', '/workspaces')).json.workspaces.find((w: { id: string }) => w.id === id)).toMatchObject({ + role: 'owner', + }); + }, 120_000); +}); diff --git a/apps/web/src/frontend/features/admin/components/AdminWorkspacesPanel.test.tsx b/apps/web/src/frontend/features/admin/components/AdminWorkspacesPanel.test.tsx index 74d25b558..fca3ad3cc 100644 --- a/apps/web/src/frontend/features/admin/components/AdminWorkspacesPanel.test.tsx +++ b/apps/web/src/frontend/features/admin/components/AdminWorkspacesPanel.test.tsx @@ -12,6 +12,7 @@ const apiAdminListWorkspaces = vi.fn(); const apiArchiveWorkspace = vi.fn(); const apiSelectWorkspace = vi.fn(); const apiSetWorkspaceMember = vi.fn(); +const apiRestoreWorkspace = vi.fn(); vi.mock('@/features/workspaces', () => ({ loadWorkspaceSettingsDialog: async () => ({ default: () => null }), @@ -19,6 +20,7 @@ vi.mock('@/features/workspaces', () => ({ apiArchiveWorkspace: (...a: unknown[]) => apiArchiveWorkspace(...a), apiSelectWorkspace: (...a: unknown[]) => apiSelectWorkspace(...a), apiSetWorkspaceMember: (...a: unknown[]) => apiSetWorkspaceMember(...a), + apiRestoreWorkspace: (...a: unknown[]) => apiRestoreWorkspace(...a), })); import { AdminWorkspacesPanel } from './AdminWorkspacesPanel'; @@ -28,7 +30,7 @@ const personal = { ...shared, id: 'ben-own', name: "ben's workspace", personalOw const joined = { ...shared, id: 'ops', name: 'Ops', adminIsMember: true }; beforeEach(() => { - for (const f of [apiAdminListWorkspaces, apiArchiveWorkspace, apiSelectWorkspace, apiSetWorkspaceMember]) f.mockReset(); + for (const f of [apiAdminListWorkspaces, apiArchiveWorkspace, apiSelectWorkspace, apiSetWorkspaceMember, apiRestoreWorkspace]) f.mockReset(); apiAdminListWorkspaces.mockResolvedValue([shared, personal, joined]); useAuthStore.setState({ user: { id: 'admin', email: 'admin@x.com', onboardingCompleted: true, role: 'admin', permissions: [] }, @@ -81,4 +83,18 @@ describe('AdminWorkspacesPanel', () => { fireEvent.click(screen.getByTestId('admin-workspaces-show-archived')); await waitFor(() => expect(apiAdminListWorkspaces).toHaveBeenLastCalledWith(true)); }); + + it('restores an archived workspace, and offers nothing else on it', async () => { + apiRestoreWorkspace.mockResolvedValue(undefined); + apiAdminListWorkspaces.mockResolvedValue([{ ...shared, archived: true }]); + render(); + await screen.findByTestId('admin-workspace-restore-books'); + for (const gone of ['join', 'open', 'members', 'archive']) { + expect(screen.queryByTestId(`admin-workspace-${gone}-books`), gone).toBeNull(); + } + fireEvent.click(screen.getByTestId('admin-workspace-restore-books')); + await waitFor(() => expect(apiRestoreWorkspace).toHaveBeenCalledWith('books')); + // And reads the list again, so the row shows it is back. + await waitFor(() => expect(apiAdminListWorkspaces).toHaveBeenCalledTimes(2)); + }); }); diff --git a/apps/web/src/frontend/features/admin/components/AdminWorkspacesPanel.tsx b/apps/web/src/frontend/features/admin/components/AdminWorkspacesPanel.tsx index b7dc753f0..d2ddd411b 100644 --- a/apps/web/src/frontend/features/admin/components/AdminWorkspacesPanel.tsx +++ b/apps/web/src/frontend/features/admin/components/AdminWorkspacesPanel.tsx @@ -8,7 +8,8 @@ * * An admin opens one only by joining it, and joining puts the admin in its * member list, so nobody's workspace is looked into silently. From here an - * admin can also run its members (hand ownership on) and archive it. + * admin can also run its members (hand ownership on), archive it, and bring an + * archived one back. */ import React, { lazy, useCallback, useEffect, useState } from 'react'; import { Layers, Loader2 } from 'lucide-react'; @@ -17,6 +18,7 @@ import { MountWhenOpened } from '@/shared/components/MountWhenOpened'; import { apiAdminListWorkspaces, apiArchiveWorkspace, + apiRestoreWorkspace, apiSelectWorkspace, apiSetWorkspaceMember, loadWorkspaceSettingsDialog, @@ -143,6 +145,17 @@ export const AdminWorkspacesPanel: React.FC<{ reload?: () => void }> = ({ reload Members )} + {w.archived && ( + + )} {!w.archived && !w.personalOwner && ( + {grantCeiling !== 'owner' && ( +

+ {ceilingHint} +

+ )} {newAccount && (
{newAccount.delivery === 'email' @@ -312,7 +334,7 @@ export const WorkspaceSettingsDialog: React.FC<{ className="bg-slate-950 border border-slate-700 rounded px-2 py-1 text-xs text-slate-100" > {ROLES.map((r) => ( - ))} diff --git a/apps/web/src/frontend/features/workspaces/index.ts b/apps/web/src/frontend/features/workspaces/index.ts index da046073f..44bcd61a7 100644 --- a/apps/web/src/frontend/features/workspaces/index.ts +++ b/apps/web/src/frontend/features/workspaces/index.ts @@ -10,6 +10,7 @@ export { apiAdminListWorkspaces, apiArchiveWorkspace, + apiRestoreWorkspace, apiSelectWorkspace, apiSetWorkspaceMember, type AdminWorkspaceRow, diff --git a/docs/testing/TEST_IDS.md b/docs/testing/TEST_IDS.md index 5a6cf0f12..2d6c26b55 100644 --- a/docs/testing/TEST_IDS.md +++ b/docs/testing/TEST_IDS.md @@ -8,7 +8,7 @@ or `TestIds` for the same tree with autocomplete. `{name}` marks a part filled i time; "in FilterPicker" marks an ID a shared component draws from the prop written here. How to write a test with them: [WRITING_E2E.md](WRITING_E2E.md). -Every control has a test ID: **988** buttons, text boxes, selects and textareas. +Every control has a test ID: **989** buttons, text boxes, selects and textareas. ## access @@ -338,6 +338,7 @@ Every control has a test ID: **988** buttons, text boxes, selects and textareas. - `admin-workspace-join-{id}` · button · Join - `admin-workspace-members-{id}` · button · Members - `admin-workspace-open-{id}` · button · Open + - `admin-workspace-restore-{id}` · button · Restore - `admin-workspaces` · div - `admin-workspaces-error` · p - `admin-workspaces-list` · ul @@ -1852,6 +1853,7 @@ Every control has a test ID: **988** buttons, text boxes, selects and textareas. - `workspace-rail-menu` · div - **WorkspaceSettingsDialog** · `features/workspaces/components/WorkspaceSettingsDialog.tsx` - `workspace-invite-{id}` · li + - `workspace-invite-ceiling` · p - `workspace-invite-email` · input · teammate@company.com - `workspace-invite-form` · form - `workspace-invite-new-account` · div diff --git a/packages/server/src/api/http-contract.test.ts b/packages/server/src/api/http-contract.test.ts index 59d746364..4848f5999 100644 --- a/packages/server/src/api/http-contract.test.ts +++ b/packages/server/src/api/http-contract.test.ts @@ -176,6 +176,7 @@ const ROUTES: RouteExpectation[] = [ { method: 'POST', path: '/api/workspaces', status: 400 }, { method: 'PATCH', path: '/api/workspaces/:id', status: 404 }, { method: 'POST', path: '/api/workspaces/:id/archive', status: 404 }, + { method: 'POST', path: '/api/workspaces/:id/restore', status: 404 }, { method: 'GET', path: '/api/workspaces/:id/members', status: 404 }, { method: 'PUT', path: '/api/workspaces/:id/members/:userId', status: 400 }, { method: 'DELETE', path: '/api/workspaces/:id/members/:userId', status: 404 }, diff --git a/packages/server/src/features/workspaces/workspace-directory.service.test.ts b/packages/server/src/features/workspaces/workspace-directory.service.test.ts index b33930fd0..d884ae54f 100644 --- a/packages/server/src/features/workspaces/workspace-directory.service.test.ts +++ b/packages/server/src/features/workspaces/workspace-directory.service.test.ts @@ -139,6 +139,20 @@ describe('a shared workspace', () => { expect((await dir.listMine(admin.id)).some((w) => w.id === temp.id)).toBe(false); await expect(dir.select(admin.id, temp.id)).rejects.toMatchObject({ code: 'not_found' }); }); + + it('an admin brings an archived workspace back, members and all; nobody else can', async () => { + const temp = await dir.create(actor(admin), 'Back again'); + await dir.addMember(actor(admin), temp.id, ana.id, 'owner'); + await dir.archive(actor(ana), temp.id); + expect((await dir.listMine(ana.id)).some((w) => w.id === temp.id)).toBe(false); + + await expect(dir.restore(actor(ana), temp.id)).rejects.toMatchObject({ code: 'forbidden' }); + await dir.restore(actor(admin), temp.id); + expect((await dir.listMine(ana.id)).find((w) => w.id === temp.id)).toMatchObject({ role: 'owner', memberCount: 2 }); + await dir.select(ana.id, temp.id); + // Restoring one that is not archived says so rather than reporting success. + await expect(dir.restore(actor(admin), temp.id)).rejects.toMatchObject({ code: 'not_found' }); + }); }); describe('personal workspaces', () => { diff --git a/packages/server/src/features/workspaces/workspace-directory.service.ts b/packages/server/src/features/workspaces/workspace-directory.service.ts index f861858f7..a1232ff2d 100644 --- a/packages/server/src/features/workspaces/workspace-directory.service.ts +++ b/packages/server/src/features/workspaces/workspace-directory.service.ts @@ -18,6 +18,7 @@ import { randomUUID } from 'node:crypto'; import { isWorkspaceRole, + lowerWorkspaceRole, permissionSatisfied, type AppRole, type Permission, @@ -27,6 +28,11 @@ import { getStore } from '../../database/store'; import type { MetadataStore } from '../../database/stores/types'; import { RbacModule } from '../../platform/authorization/rbac.service'; import { ServiceError } from '../../platform/contracts/actor'; +import { + assertRoleGrantable, + effectiveMemberRole, + workspaceRoleCeiling, +} from '../../platform/workspaces/workspace.service'; export const MEMBERS_CREATE_KEY = 'workspaces.members_create'; @@ -118,7 +124,7 @@ export class WorkspaceDirectory { if (actor.appRole === 'admin') return ws; // Everyone runs the membership of their own workspace, whatever its role there. if (permission === 'workspace.members' && ws.personal_owner_id === actor.userId) return ws; - const role = await this.roleIn(store, workspaceId, actor.userId); + const role = await effectiveMemberRole(store, workspaceId, actor.userId); if (!role) throw new ServiceError('not_found', 'Workspace not found'); if (!permissionSatisfied(await this.rbac.permissionsForRole(role), permission)) { throw new ServiceError('forbidden', `Permission denied: this needs "${permission}" in this workspace.`); @@ -140,12 +146,14 @@ export class WorkspaceDirectory { visibility: string; join_role: string; personal_owner_id: string | null; + owner_role: string | null; role: string; member_count: number; }>( - `SELECT w.id, w.name, w.visibility, w.join_role, w.personal_owner_id, m.role, + `SELECT w.id, w.name, w.visibility, w.join_role, w.personal_owner_id, o.app_role AS owner_role, m.role, (SELECT COUNT(*) FROM workspace_members x WHERE x.workspace_id = w.id) AS member_count FROM workspaces w JOIN workspace_members m ON m.workspace_id = w.id + LEFT JOIN users o ON o.id = w.personal_owner_id WHERE m.user_id = ? AND w.archived_at IS NULL ORDER BY CASE WHEN w.personal_owner_id = ? THEN 0 ELSE 1 END, w.name`, [userId, userId] @@ -158,7 +166,8 @@ export class WorkspaceDirectory { visibility: r.visibility === 'public' ? 'public' : 'private', joinRole: isWorkspaceRole(r.join_role) ? r.join_role : 'viewer', personal: r.personal_owner_id === userId, - role: r.role as WorkspaceRole, + // What it acts with, not what was stored (effectiveMemberRole). + role: lowerWorkspaceRole(r.role as WorkspaceRole, workspaceRoleCeiling(r.personal_owner_id, r.owner_role)), memberCount: Number(r.member_count), })); } @@ -288,12 +297,30 @@ export class WorkspaceDirectory { await store.run('UPDATE workspaces SET archived_at = ? WHERE id = ?', [new Date().toISOString(), workspaceId]); } + /** + * Bring an archived workspace back, with its members and rows as they were. + * Archiving promised its data is kept; without this, keeping it was all it + * did. Admins only: an archived workspace is hidden from its own members. + */ + async restore(actor: WorkspaceActor, workspaceId: string): Promise { + if (actor.appRole !== 'admin') throw new ServiceError('forbidden', 'Only an admin restores an archived workspace.'); + const store = await getStore(); + const result = await store.run('UPDATE workspaces SET archived_at = NULL WHERE id = ? AND archived_at IS NOT NULL', [ + workspaceId, + ]); + if (result.changes === 0) throw new ServiceError('not_found', 'No archived workspace with that id.'); + } + async members(actor: WorkspaceActor, workspaceId: string): Promise { const store = await getStore(); const ws = await this.workspace(store, workspaceId); if (actor.appRole !== 'admin' && !(await this.roleIn(store, workspaceId, actor.userId))) { throw new ServiceError('not_found', 'Workspace not found'); } + const owner = ws.personal_owner_id + ? await store.get<{ app_role: string | null }>('SELECT app_role FROM users WHERE id = ?', [ws.personal_owner_id]) + : undefined; + const ceiling = workspaceRoleCeiling(ws.personal_owner_id, owner?.app_role); const rows = await store.all<{ user_id: string; email: string; role: string; added_by: string | null; created_at: string }>( `SELECT m.user_id, u.email, m.role, a.email AS added_by, m.created_at FROM workspace_members m @@ -308,7 +335,7 @@ export class WorkspaceDirectory { .map((r) => ({ userId: r.user_id, email: r.email, - role: r.role as WorkspaceRole, + role: lowerWorkspaceRole(r.role as WorkspaceRole, ceiling), personalOwner: ws.personal_owner_id === r.user_id, addedBy: r.added_by, createdAt: r.created_at, @@ -394,6 +421,7 @@ export class WorkspaceDirectory { if (ws.personal_owner_id === userId) { throw new ServiceError('conflict', 'Your role in your own workspace follows your account role.'); } + await assertRoleGrantable(store, actor, workspaceId, role); // A personal workspace always keeps its account, so it never runs out of an owner. if (current === 'owner' && role !== 'owner' && !ws.personal_owner_id) await this.assertOwnerRemains(store, workspaceId, userId); await store.run('UPDATE workspace_members SET role = ? WHERE workspace_id = ? AND user_id = ?', [role, workspaceId, userId]); diff --git a/packages/server/src/features/workspaces/workspace-invites.service.test.ts b/packages/server/src/features/workspaces/workspace-invites.service.test.ts index da4b7499b..f80fa6f3c 100644 --- a/packages/server/src/features/workspaces/workspace-invites.service.test.ts +++ b/packages/server/src/features/workspaces/workspace-invites.service.test.ts @@ -104,7 +104,8 @@ describe('inviting an existing account', () => { const { invite } = await invites.invite(actor(viewer), ownId, guest.email, 'viewer'); expect(invite.workspaceId).toBe(ownId); await invites.accept(guest.id, invite.id); - await dir.setMemberRole(actor(viewer), ownId, guest.id, 'editor'); + // It runs who is in it, but lends no more than it holds (workspace-role-ceiling.test.ts). + await expect(dir.setMemberRole(actor(viewer), ownId, guest.id, 'editor')).rejects.toMatchObject({ code: 'forbidden' }); await dir.removeMember(actor(viewer), ownId, guest.id); // …but not into someone else's. const ownerOwn = (await dir.listMine(owner.id)).find((w) => w.personal)!.id; diff --git a/packages/server/src/features/workspaces/workspace-invites.service.ts b/packages/server/src/features/workspaces/workspace-invites.service.ts index ff8299ae4..ab90b36b2 100644 --- a/packages/server/src/features/workspaces/workspace-invites.service.ts +++ b/packages/server/src/features/workspaces/workspace-invites.service.ts @@ -22,6 +22,7 @@ import type { MetadataStore } from '../../database/stores/types'; import { RbacModule } from '../../platform/authorization/rbac.service'; import { AuthModule, type IssuedCode } from '../../platform/identity/auth.service'; import { ServiceError } from '../../platform/contracts/actor'; +import { assertRoleGrantable, effectiveMemberRole } from '../../platform/workspaces/workspace.service'; import type { WorkspaceActor } from './workspace-directory.service'; export const OWNERS_INVITE_NEW_KEY = 'workspaces.owners_invite_new'; @@ -69,12 +70,9 @@ export class WorkspaceInvites { private async requireMembers(store: MetadataStore, actor: WorkspaceActor, workspaceId: string) { const ws = await this.liveWorkspace(store, workspaceId); if (actor.appRole === 'admin' || ws.personal_owner_id === actor.userId) return ws; - const row = await store.get<{ role: string }>( - 'SELECT role FROM workspace_members WHERE workspace_id = ? AND user_id = ?', - [workspaceId, actor.userId] - ); - if (!row || !isWorkspaceRole(row.role)) throw new ServiceError('not_found', 'Workspace not found'); - if (!permissionSatisfied(await this.rbac.permissionsForRole(row.role), 'workspace.members')) { + const role = await effectiveMemberRole(store, workspaceId, actor.userId); + if (!role) throw new ServiceError('not_found', 'Workspace not found'); + if (!permissionSatisfied(await this.rbac.permissionsForRole(role), 'workspace.members')) { throw new ServiceError('forbidden', 'Only someone who manages members here can invite.'); } return ws; @@ -95,6 +93,7 @@ export class WorkspaceInvites { if (!/^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(email)) throw new ServiceError('invalid_input', 'Enter an email address.'); const store = await getStore(); const ws = await this.requireMembers(store, actor, workspaceId); + await assertRoleGrantable(store, actor, workspaceId, role); let user = await store.get<{ id: string }>('SELECT id FROM users WHERE email = ?', [email]); let newAccount: IssuedCode | undefined; diff --git a/packages/server/src/features/workspaces/workspace-role-ceiling.test.ts b/packages/server/src/features/workspaces/workspace-role-ceiling.test.ts new file mode 100644 index 000000000..8394444c6 --- /dev/null +++ b/packages/server/src/features/workspaces/workspace-role-ceiling.test.ts @@ -0,0 +1,144 @@ +/** + * Fox Schema (foxschema) + * Copyright 2024-2026 Huy Phan + * SPDX-License-Identifier: Apache-2.0 + * + * Nobody gives more than they can do, and a personal workspace lends no more + * than its account holds. + * + * A personal workspace's account manages its members whatever its role there, + * so a viewer account could invite a colleague as owner. The colleague then + * had DDL, DML, GRANT and schema migrate on the viewer's saved connections — + * credentials an admin handed out for reading — which neither account was + * ever given. + */ +import { describe, it, expect, beforeAll } from 'vitest'; + +process.env.APP_DB_PATH = ':memory:'; +process.env.APP_ENCRYPTION_KEY = '0'.repeat(64); + +import { getStore } from '../../database/store'; +import { AuthModule, type AuthUser } from '../../platform/identity/auth.service'; +import { RbacModule } from '../../platform/authorization/rbac.service'; +import { writeAdminPolicy } from '../../platform/authorization/admin-policy.service'; +import { WorkspaceDirectory, type WorkspaceActor } from './workspace-directory.service'; +import { WorkspaceInvites } from './workspace-invites.service'; + +const auth = new AuthModule(); +const dir = new WorkspaceDirectory(); +const invites = new WorkspaceInvites(); + +let admin: AuthUser; +let viewerAcct: AuthUser; // a viewer account, sharing its own workspace +let editorAcct: AuthUser; // an editor account, sharing its own workspace +let colleague: AuthUser; // invited into both +let teamOwner: AuthUser; // owner of a shared workspace + +const actor = (u: AuthUser): WorkspaceActor => ({ userId: u.id, appRole: u.role, permissions: new Set(u.permissions) }); +const personalOf = async (u: AuthUser) => (await dir.listMine(u.id)).find((w) => w.personal)!.id; +const roleOf = async (u: AuthUser, wsId: string) => (await dir.listMine(u.id)).find((w) => w.id === wsId)?.role; +const permsIn = async (u: AuthUser, wsId: string) => new Set((await auth.inWorkspace(u, wsId)).permissions); + +beforeAll(async () => { + await writeAdminPolicy(await getStore(), 'several'); + admin = await auth.createUser('ceil-admin@example.com', 'correct-horse-9', 'editor'); + await new RbacModule().setUserRole(admin.id, 'admin'); + admin = { ...admin, role: 'admin' }; + viewerAcct = await auth.createUser('ceil-viewer@example.com', 'correct-horse-9', 'viewer'); + editorAcct = await auth.createUser('ceil-editor@example.com', 'correct-horse-9', 'editor'); + colleague = await auth.createUser('ceil-colleague@example.com', 'correct-horse-9', 'viewer'); + teamOwner = await auth.createUser('ceil-owner@example.com', 'correct-horse-9', 'editor'); +}); + +describe('a personal workspace lends no more than its account holds', () => { + it("a viewer account can share its workspace, but only as viewer", async () => { + const ws = await personalOf(viewerAcct); + for (const role of ['owner', 'editor'] as const) { + await expect(invites.invite(actor(viewerAcct), ws, colleague.email, role)).rejects.toMatchObject({ + code: 'forbidden', + }); + } + const { invite } = await invites.invite(actor(viewerAcct), ws, colleague.email, 'viewer'); + await invites.accept(colleague.id, invite.id); + expect(await roleOf(colleague, ws)).toBe('viewer'); + + // Nor can it raise the colleague afterwards. + await expect(dir.setMemberRole(actor(viewerAcct), ws, colleague.id, 'owner')).rejects.toMatchObject({ + code: 'forbidden', + }); + expect(await roleOf(colleague, ws)).toBe('viewer'); + }); + + it('an editor account shares up to editor, never owner', async () => { + const ws = await personalOf(editorAcct); + await expect(invites.invite(actor(editorAcct), ws, colleague.email, 'owner')).rejects.toMatchObject({ + code: 'forbidden', + }); + const { invite } = await invites.invite(actor(editorAcct), ws, colleague.email, 'editor'); + await invites.accept(colleague.id, invite.id); + expect(await roleOf(colleague, ws)).toBe('editor'); + expect((await permsIn(colleague, ws)).has('editor.ddl')).toBe(true); + expect((await permsIn(colleague, ws)).has('editor.grant')).toBe(false); + }); + + it('a role stored above the ceiling acts at the ceiling — written before this rule, or by an older build', async () => { + const ws = await personalOf(viewerAcct); + const store = await getStore(); + await store.run('UPDATE workspace_members SET role = ? WHERE workspace_id = ? AND user_id = ?', [ + 'owner', + ws, + colleague.id, + ]); + + const perms = await permsIn(colleague, ws); + for (const p of ['editor.ddl', 'editor.dml', 'editor.grant', 'schema.migrate', 'secrets.view'] as const) { + expect(perms.has(p), p).toBe(false); + } + expect(perms.has('editor.run')).toBe(true); + expect(await roleOf(colleague, ws)).toBe('viewer'); + expect((await dir.members(actor(viewerAcct), ws)).find((m) => m.userId === colleague.id)?.role).toBe('viewer'); + // And it cannot run the membership with the role it does not really have. + await expect(invites.invite(actor(colleague), ws, teamOwner.email, 'viewer')).rejects.toMatchObject({ + code: 'forbidden', + }); + await expect(dir.setMemberRole(actor(colleague), ws, colleague.id, 'owner')).rejects.toMatchObject({ + code: 'forbidden', + }); + }); + + it('follows the account: an account raised to editor lends editor', async () => { + const ws = await personalOf(viewerAcct); + await new RbacModule().setUserRole(viewerAcct.id, 'editor'); + try { + expect(await roleOf(colleague, ws)).toBe('editor'); + } finally { + await new RbacModule().setUserRole(viewerAcct.id, 'viewer'); + } + expect(await roleOf(colleague, ws)).toBe('viewer'); + }); +}); + +describe('a shared workspace', () => { + let teamId: string; + + beforeAll(async () => { + teamId = (await dir.create(actor(admin), 'Ceiling team')).id; + await dir.addMember(actor(admin), teamId, teamOwner.id, 'owner'); + }); + + it('its owners still give any role, owner included', async () => { + const { invite } = await invites.invite(actor(teamOwner), teamId, colleague.email, 'owner'); + await invites.accept(colleague.id, invite.id); + expect(await roleOf(colleague, teamId)).toBe('owner'); + expect((await permsIn(colleague, teamId)).has('editor.grant')).toBe(true); + await dir.setMemberRole(actor(teamOwner), teamId, colleague.id, 'viewer'); + expect(await roleOf(colleague, teamId)).toBe('viewer'); + }); + + it('an admin gives any role anywhere, a personal workspace included', async () => { + const ws = await personalOf(viewerAcct); + await dir.setMemberRole(actor(admin), ws, colleague.id, 'editor'); + // Stored as given; it still acts at the workspace's ceiling. + expect(await roleOf(colleague, ws)).toBe('viewer'); + }); +}); diff --git a/packages/server/src/features/workspaces/workspaces.routes.ts b/packages/server/src/features/workspaces/workspaces.routes.ts index 10d393e08..49f905e3d 100644 --- a/packages/server/src/features/workspaces/workspaces.routes.ts +++ b/packages/server/src/features/workspaces/workspaces.routes.ts @@ -125,6 +125,16 @@ export function createWorkspaceRoutes( } }); + /** An admin brings an archived workspace back. */ + router.post('/:id/restore', async (req: AuthedRequest, res: FastifyReply) => { + try { + await directory.restore(actorOf(req), id(req)); + res.send({ ok: true }); + } catch (error) { + fail(res, error); + } + }); + router.get('/:id/members', async (req: AuthedRequest, res: FastifyReply) => { try { res.send({ members: await directory.members(actorOf(req), id(req)) }); diff --git a/packages/server/src/platform/workspaces/workspace.service.ts b/packages/server/src/platform/workspaces/workspace.service.ts index 60d781cd3..320f68fd0 100644 --- a/packages/server/src/platform/workspaces/workspace.service.ts +++ b/packages/server/src/platform/workspaces/workspace.service.ts @@ -18,6 +18,8 @@ import { randomUUID } from 'node:crypto'; import { isAppRole, isWorkspaceRole, + lowerWorkspaceRole, + workspaceRoleAtMost, type AppRole, type WorkspaceRole, } from '@foxschema/shared'; @@ -39,6 +41,66 @@ export function personalRoleFor(accountRole: AppRole | string | null | undefined return isWorkspaceRole(accountRole) ? accountRole : 'viewer'; } +/** + * The most anyone can do in a workspace. A shared one: owner. A personal one: + * what its own account may do there. Sharing a personal workspace lends its + * connections; it never lends more than the account holds — otherwise a viewer + * could invite a colleague as owner, and between them run DDL and GRANT with + * credentials an admin handed out for reading. + */ +export function workspaceRoleCeiling( + personalOwnerId: string | null | undefined, + personalOwnerAccountRole: string | null | undefined +): WorkspaceRole { + return personalOwnerId ? personalRoleFor(personalOwnerAccountRole) : 'owner'; +} + +/** + * `userId`'s role in `workspaceId` as it takes effect: the stored role, capped + * at the workspace's ceiling. Null when not a member. The cap is applied on + * read, so a membership granted before the ceiling existed, or an account + * demoted since, cannot keep more than the ceiling allows. + */ +export async function effectiveMemberRole( + store: MetadataStore, + workspaceId: string, + userId: string +): Promise { + const row = await store.get<{ role: string; personal_owner_id: string | null; owner_role: string | null }>( + `SELECT m.role, w.personal_owner_id, o.app_role AS owner_role + FROM workspace_members m + JOIN workspaces w ON w.id = m.workspace_id + LEFT JOIN users o ON o.id = w.personal_owner_id + WHERE m.workspace_id = ? AND m.user_id = ?`, + [workspaceId, userId] + ); + if (!row || !isWorkspaceRole(row.role)) return null; + return lowerWorkspaceRole(row.role, workspaceRoleCeiling(row.personal_owner_id, row.owner_role)); +} + +/** + * Refuse to give `role` in `workspaceId` when it is more than the granter may + * do there. An admin may give any role. Everyone else gives at most their own + * effective role — which in a personal workspace is its ceiling, and in a + * shared one is owner for anyone allowed to manage members at all, unless an + * admin widened `workspace.members` to a lesser role. + */ +export async function assertRoleGrantable( + store: MetadataStore, + actor: { userId: string; appRole: AppRole | string }, + workspaceId: string, + role: WorkspaceRole +): Promise { + if (actor.appRole === 'admin') return; + const own = (await effectiveMemberRole(store, workspaceId, actor.userId)) ?? 'viewer'; + if (!workspaceRoleAtMost(role, own)) { + throw new ServiceError( + 'forbidden', + `You can give at most the ${own} role here: nobody gives more than they can do themselves.` + ); + } +} + /** "ana's workspace" for ana@example.com. */ export function personalWorkspaceName(email: string): string { const local = (email.split('@')[0] ?? '').trim(); @@ -133,14 +195,23 @@ async function memberWorkspace( userId: string, workspaceId: string ): Promise { - const row = await store.get<{ id: string; name: string; personal_owner_id: string | null; role: string }>( - `SELECT w.id, w.name, w.personal_owner_id, m.role + const row = await store.get<{ + id: string; + name: string; + personal_owner_id: string | null; + owner_role: string | null; + role: string; + }>( + `SELECT w.id, w.name, w.personal_owner_id, o.app_role AS owner_role, m.role FROM workspaces w JOIN workspace_members m ON m.workspace_id = w.id + LEFT JOIN users o ON o.id = w.personal_owner_id WHERE w.id = ? AND m.user_id = ? AND w.archived_at IS NULL`, [workspaceId, userId] ); if (!row || !isWorkspaceRole(row.role)) return null; - return { id: row.id, name: row.name, personal: row.personal_owner_id === userId, role: row.role }; + // The role this request acts with: capped like everywhere else (effectiveMemberRole). + const role = lowerWorkspaceRole(row.role, workspaceRoleCeiling(row.personal_owner_id, row.owner_role)); + return { id: row.id, name: row.name, personal: row.personal_owner_id === userId, role }; } /** diff --git a/packages/shared/src/permissions.ts b/packages/shared/src/permissions.ts index ab887ba40..49060c0ec 100644 --- a/packages/shared/src/permissions.ts +++ b/packages/shared/src/permissions.ts @@ -272,6 +272,16 @@ export function isWorkspaceRole(value: unknown): value is WorkspaceRole { return typeof value === 'string' && (WORKSPACE_ROLES as readonly string[]).includes(value); } +/** True when `role` grants no more than `ceiling`: viewer < editor < owner. */ +export function workspaceRoleAtMost(role: WorkspaceRole, ceiling: WorkspaceRole): boolean { + return WORKSPACE_ROLES.indexOf(role) <= WORKSPACE_ROLES.indexOf(ceiling); +} + +/** The lower of two workspace roles. */ +export function lowerWorkspaceRole(a: WorkspaceRole, b: WorkspaceRole): WorkspaceRole { + return workspaceRoleAtMost(a, b) ? a : b; +} + /** * Permissions that belong to the install rather than to a workspace, so they * come from the account role. `editor.advanced` runs code on the server: From a56e82015e3d45dcf0dfa416117489a0062207e0 Mon Sep 17 00:00:00 2001 From: huyplb Date: Sun, 25 Oct 2026 10:21:40 -0600 Subject: [PATCH 2/3] refactor(workspaces): one capped-role read, one permission check, scripts capped too - effectiveMemberRole / cappedRole: every read of a member's role goes through the ceiling. Workspace scripts read the raw stored role, so a colleague stored as editor in a viewer's personal workspace could still save and delete its scripts; the admin list counted them as owners. - requireWorkspacePermission + assertRoleGrantable replace the copies in the directory and invite services. - workspaceGrantCeiling in @foxschema/shared: the dialog offers what the server accepts, from the same rule. - Settings dialog builds its role options once and registers Escape once. - e2e: apiCall / signInAs / browserAs helpers in helpers/accounts.ts. Co-Authored-By: Claude Opus 5.5 --- apps/e2e/src/helpers/accounts.ts | 52 ++++++++ .../tests/workspaces-sharing-limits.test.ts | 32 +---- .../admin/components/AdminWorkspacesPanel.tsx | 93 +++++++------- .../features/workspaces/api/workspacesApi.ts | 3 +- .../WorkspaceSettingsDialog.test.tsx | 52 ++++---- .../components/WorkspaceSettingsDialog.tsx | 46 +++---- .../workspaces/workspace-directory.service.ts | 117 ++++++++---------- .../workspaces/workspace-invites.service.ts | 30 +++-- .../workspaces/workspace-role-ceiling.test.ts | 40 +++--- .../workspaces/workspace-scripts.service.ts | 18 +-- .../platform/workspaces/workspace.service.ts | 97 ++++++++++----- packages/shared/src/permissions.ts | 9 ++ 12 files changed, 320 insertions(+), 269 deletions(-) create mode 100644 apps/e2e/src/helpers/accounts.ts diff --git a/apps/e2e/src/helpers/accounts.ts b/apps/e2e/src/helpers/accounts.ts new file mode 100644 index 000000000..de84e15e0 --- /dev/null +++ b/apps/e2e/src/helpers/accounts.ts @@ -0,0 +1,52 @@ +/** + * Fox Schema (foxschema) + * Copyright 2024-2026 Huy Phan + * SPDX-License-Identifier: Apache-2.0 + * + * Calling the API directly, and acting as a second account in a browser. + * + * The calls go to the API port, not through the Vite proxy (see app-session.ts). + */ +import type { Page } from 'playwright'; +import { buildDriver, BASE_URL } from './driver.js'; +import { byTestId } from './test-ids.js'; + +export const API_URL = process.env.E2E_API_URL ?? 'http://127.0.0.1:3210'; + +export interface ApiResult { + status: number; + json: any; +} + +/** One request to `/api` as the session in `cookie`. A body that is not JSON reads as null. */ +export async function apiCall(method: string, path: string, body: unknown, cookie: string): Promise { + const res = await fetch(`${API_URL}/api${path}`, { + method, + headers: { ...(body !== undefined ? { 'content-type': 'application/json' } : {}), cookie }, + ...(body !== undefined ? { body: JSON.stringify(body) } : {}), + }); + return { status: res.status, json: await res.json().catch(() => null) }; +} + +/** Signs `email` in and returns its session cookie as `name=value`. */ +export async function signInAs(email: string, password: string): Promise { + const res = await fetch(`${API_URL}/api/auth/login`, { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ email, password }), + }); + if (res.status !== 200) throw new Error(`Signing in as ${email} failed: HTTP ${res.status}`); + return (res.headers.get('set-cookie') ?? '').split(';')[0]!; +} + +/** A browser signed in with `cookie`, past onboarding. Close it with quitDriver. */ +export async function browserAs(cookie: string): Promise { + const page = await buildDriver({ signedIn: false }); + const [name, ...rest] = cookie.split('='); + await page.context().addCookies([{ name: name!, value: rest.join('='), url: BASE_URL }]); + await page.goto(BASE_URL); + await page.waitForSelector(`${byTestId('onboarding-skip')}, ${byTestId('profile-menu-trigger')}`, { timeout: 30_000 }); + const skip = page.locator(byTestId('onboarding-skip')); + if (await skip.count()) await skip.click(); + return page; +} diff --git a/apps/e2e/src/tests/workspaces-sharing-limits.test.ts b/apps/e2e/src/tests/workspaces-sharing-limits.test.ts index 3cb5889cd..d6f9ced54 100644 --- a/apps/e2e/src/tests/workspaces-sharing-limits.test.ts +++ b/apps/e2e/src/tests/workspaces-sharing-limits.test.ts @@ -13,11 +13,11 @@ import { afterAll, beforeAll, describe, expect, it } from 'vitest'; import { randomBytes } from 'node:crypto'; import type { Page } from 'playwright'; import { buildDriver, quitDriver, clickWhen, waitFor, BASE_URL } from '../helpers/driver.js'; +import { apiCall, browserAs, signInAs } from '../helpers/accounts.js'; import { sessionCookie } from '../helpers/app-session.js'; import { saveScreenshot } from '../helpers/screenshot.js'; import { byTestId } from '../helpers/test-ids.js'; -const API_URL = process.env.E2E_API_URL ?? 'http://127.0.0.1:3210'; const RUN = Date.now().toString(36); const SHARER_EMAIL = `e2e-wssharer-${RUN}@foxschema.test`; const PEER_EMAIL = `e2e-wspeer-${RUN}@foxschema.test`; @@ -29,39 +29,20 @@ let peer = { id: '', cookie: '' }; const created: string[] = []; const pages: Page[] = []; -async function api(method: string, path: string, body?: unknown, cookie = admin) { - const res = await fetch(`${API_URL}/api${path}`, { - method, - headers: { ...(body !== undefined ? { 'content-type': 'application/json' } : {}), cookie }, - ...(body !== undefined ? { body: JSON.stringify(body) } : {}), - }); - return { status: res.status, json: (await res.json().catch(() => null)) as any }; -} +const api = (method: string, path: string, body?: unknown, cookie = admin) => apiCall(method, path, body, cookie); /** A viewer account, signed in: its id and session cookie. */ async function viewerAccount(email: string): Promise<{ id: string; cookie: string }> { const password = `ws-${randomBytes(8).toString('hex')}`; const added = await api('POST', '/admin/users', { email, password, role: 'viewer' }); expect(added.status, JSON.stringify(added.json)).toBe(200); - const login = await fetch(`${API_URL}/api/auth/login`, { - method: 'POST', - headers: { 'content-type': 'application/json' }, - body: JSON.stringify({ email, password }), - }); - expect(login.status).toBe(200); - return { id: added.json.user.id, cookie: (login.headers.get('set-cookie') ?? '').split(';')[0]! }; + return { id: added.json.user.id, cookie: await signInAs(email, password) }; } -/** A browser signed in with `cookie`, past onboarding. */ +/** A browser as `cookie`, closed after the suite. */ async function browserFor(cookie: string): Promise { - const page = await buildDriver({ signedIn: false }); + const page = await browserAs(cookie); pages.push(page); - const [name, ...rest] = cookie.split('='); - await page.context().addCookies([{ name: name!, value: rest.join('='), url: BASE_URL }]); - await page.goto(BASE_URL); - await page.waitForSelector(`${byTestId('onboarding-skip')}, ${byTestId('profile-menu-trigger')}`, { timeout: 30_000 }); - const skip = page.locator(byTestId('onboarding-skip')); - if (await skip.count()) await skip.click(); return page; } @@ -71,8 +52,7 @@ beforeAll(async () => { expect(list.status, JSON.stringify(list.json)).toBe(200); adminPersonalId = list.json.workspaces.find((w: { personal: boolean }) => w.personal).id; await api('POST', `/workspaces/${adminPersonalId}/select`, {}); - sharer = await viewerAccount(SHARER_EMAIL); - peer = await viewerAccount(PEER_EMAIL); + [sharer, peer] = await Promise.all([viewerAccount(SHARER_EMAIL), viewerAccount(PEER_EMAIL)]); }, 120_000); afterAll(async () => { diff --git a/apps/web/src/frontend/features/admin/components/AdminWorkspacesPanel.tsx b/apps/web/src/frontend/features/admin/components/AdminWorkspacesPanel.tsx index d2ddd411b..7e34cb442 100644 --- a/apps/web/src/frontend/features/admin/components/AdminWorkspacesPanel.tsx +++ b/apps/web/src/frontend/features/admin/components/AdminWorkspacesPanel.tsx @@ -103,10 +103,22 @@ export const AdminWorkspacesPanel: React.FC<{ reload?: () => void }> = ({ reload {w.memberCount} {w.memberCount === 1 ? 'member' : 'members'}

- {w.adminIsMember ? ( + {w.adminIsMember && ( + you’re in it + )} + {w.archived ? ( + + ) : ( <> - you’re in it - {!w.archived && ( + {w.adminIsMember ? ( + ) : ( + )} - - ) : ( - !w.archived && ( - ) - )} - {!w.archived && ( - - )} - {w.archived && ( - - )} - {!w.archived && !w.personalOwner && ( - + {!w.personalOwner && ( + + )} + )} ))} diff --git a/apps/web/src/frontend/features/workspaces/api/workspacesApi.ts b/apps/web/src/frontend/features/workspaces/api/workspacesApi.ts index 2b4c7f5f6..1e26bfec8 100644 --- a/apps/web/src/frontend/features/workspaces/api/workspacesApi.ts +++ b/apps/web/src/frontend/features/workspaces/api/workspacesApi.ts @@ -7,9 +7,10 @@ * switching is `select` and a reload — every request after it, from any * tab, acts there. */ +import type { WorkspaceRole } from '@foxschema/shared'; import { api } from '@/shared/api/client'; -export type WorkspaceRole = 'viewer' | 'editor' | 'owner'; +export type { WorkspaceRole }; export interface WorkspaceItem { id: string; diff --git a/apps/web/src/frontend/features/workspaces/components/WorkspaceSettingsDialog.test.tsx b/apps/web/src/frontend/features/workspaces/components/WorkspaceSettingsDialog.test.tsx index bd9803a30..80982725e 100644 --- a/apps/web/src/frontend/features/workspaces/components/WorkspaceSettingsDialog.test.tsx +++ b/apps/web/src/frontend/features/workspaces/components/WorkspaceSettingsDialog.test.tsx @@ -51,6 +51,16 @@ function signIn(role: string) { } as never); } +/** Opens the dialog as an account with `accountRole`, on `workspace` with these members. */ +async function openAs(accountRole: string, workspace: object & { id: string }, members: object[], onClose = () => undefined) { + signIn(accountRole); + apiListWorkspaces.mockResolvedValue({ currentId: workspace.id, workspaces: [workspace], canCreate: false }); + apiWorkspaceMembers.mockResolvedValue(members); + const view = render(); + await screen.findByTestId('workspace-member-role-bo'); + return view; +} + /** The roles a select offers, and which of them can be picked. */ function options(testId: string) { return [...(screen.getByTestId(testId) as HTMLSelectElement).options].map((o) => ({ role: o.value, enabled: !o.disabled })); @@ -64,12 +74,7 @@ beforeEach(() => { describe('WorkspaceSettingsDialog · roles on offer', () => { it('a viewer sharing its own workspace is offered viewer, and told why not more', async () => { - signIn('viewer'); - apiListWorkspaces.mockResolvedValue({ currentId: 'own', workspaces: [personal('viewer')], canCreate: false }); - apiWorkspaceMembers.mockResolvedValue([self('viewer'), guest]); - render( undefined} />); - - await screen.findByTestId('workspace-member-role-bo'); + await openAs('viewer', personal('viewer'), [self('viewer'), guest]); const expected = [ { role: 'viewer', enabled: true }, { role: 'editor', enabled: false }, @@ -81,43 +86,36 @@ describe('WorkspaceSettingsDialog · roles on offer', () => { }); it('an editor account offers up to editor in its own workspace', async () => { - signIn('editor'); - apiListWorkspaces.mockResolvedValue({ currentId: 'own', workspaces: [personal('editor')], canCreate: false }); - apiWorkspaceMembers.mockResolvedValue([self('editor'), guest]); - render( undefined} />); - - await screen.findByTestId('workspace-member-role-bo'); + await openAs('editor', personal('editor'), [self('editor'), guest]); expect(options('workspace-invite-role').filter((o) => o.enabled).map((o) => o.role)).toEqual(['viewer', 'editor']); }); it('an owner of a shared workspace, and an admin anywhere, are offered every role', async () => { - signIn('editor'); - apiListWorkspaces.mockResolvedValue({ currentId: 'team', workspaces: [team], canCreate: false }); - apiWorkspaceMembers.mockResolvedValue([{ ...self('owner'), personalOwner: false }, guest]); - const { unmount } = render( undefined} />); - await screen.findByTestId('workspace-member-role-bo'); + const { unmount } = await openAs('editor', team, [{ ...self('owner'), personalOwner: false }, guest]); expect(options('workspace-invite-role').every((o) => o.enabled)).toBe(true); expect(screen.queryByTestId('workspace-invite-ceiling')).toBeNull(); unmount(); - signIn('admin'); - apiListWorkspaces.mockResolvedValue({ currentId: 'own', workspaces: [personal('viewer')], canCreate: false }); - apiWorkspaceMembers.mockResolvedValue([self('owner'), guest]); - render( undefined} />); - await screen.findByTestId('workspace-member-role-bo'); + await openAs('admin', personal('viewer'), [self('owner'), guest]); expect(options('workspace-invite-role').every((o) => o.enabled)).toBe(true); }); }); describe('WorkspaceSettingsDialog · closing', () => { it('Escape closes it', async () => { - signIn('editor'); - apiListWorkspaces.mockResolvedValue({ currentId: 'team', workspaces: [team], canCreate: false }); - apiWorkspaceMembers.mockResolvedValue([guest]); const onClose = vi.fn(); - render(); - await screen.findByTestId('workspace-member-role-bo'); + await openAs('editor', team, [guest], onClose); fireEvent.keyDown(window, { key: 'Escape' }); await waitFor(() => expect(onClose).toHaveBeenCalledTimes(1)); }); + + it('Escape calls the onClose of the latest render', async () => { + const first = vi.fn(); + const latest = vi.fn(); + const { rerender } = await openAs('editor', team, [guest], first); + rerender(); + fireEvent.keyDown(window, { key: 'Escape' }); + await waitFor(() => expect(latest).toHaveBeenCalledTimes(1)); + expect(first).not.toHaveBeenCalled(); + }); }); diff --git a/apps/web/src/frontend/features/workspaces/components/WorkspaceSettingsDialog.tsx b/apps/web/src/frontend/features/workspaces/components/WorkspaceSettingsDialog.tsx index 374fb6471..738f9e7f5 100644 --- a/apps/web/src/frontend/features/workspaces/components/WorkspaceSettingsDialog.tsx +++ b/apps/web/src/frontend/features/workspaces/components/WorkspaceSettingsDialog.tsx @@ -7,10 +7,10 @@ * it and archiving it. What is editable follows the caller's role in this * workspace; the server enforces the same. */ -import React, { useCallback, useEffect, useState } from 'react'; +import React, { useCallback, useEffect, useRef, useState } from 'react'; import { createPortal } from 'react-dom'; import { Layers, X } from 'lucide-react'; -import { workspaceRoleAtMost } from '@foxschema/shared'; +import { WORKSPACE_ROLES, workspaceGrantCeiling, workspaceRoleAtMost } from '@foxschema/shared'; import { useAuthStore } from '@/platform/authStore'; import { apiArchiveWorkspace, @@ -30,8 +30,6 @@ import { type WorkspaceRole, } from '../api/workspacesApi'; -const ROLES: WorkspaceRole[] = ['viewer', 'editor', 'owner']; - export const WorkspaceSettingsDialog: React.FC<{ workspaceId: string; /** Its name when the caller is not a member (an admin managing it), so it is not in their list. */ @@ -67,13 +65,16 @@ export const WorkspaceSettingsDialog: React.FC<{ load().catch((e: unknown) => setError(e instanceof Error ? e.message : 'Could not load the workspace')); }, [load]); + // Registered once: both callers pass a fresh onClose on every render. + const onCloseRef = useRef(onClose); + onCloseRef.current = onClose; useEffect(() => { const onKey = (e: KeyboardEvent) => { - if (e.key === 'Escape' && !e.defaultPrevented) onClose(); + if (e.key === 'Escape' && !e.defaultPrevented) onCloseRef.current(); }; window.addEventListener('keydown', onKey); return () => window.removeEventListener('keydown', onKey); - }, [onClose]); + }, []); const run = async (action: () => Promise, done?: string) => { setBusy(true); @@ -90,17 +91,24 @@ export const WorkspaceSettingsDialog: React.FC<{ } }; - const isOwner = workspace?.role === 'owner' || me?.role === 'admin'; - /** Who may invite and change roles: owners, admins, and everyone in their own workspace. */ - const managesMembers = isOwner || workspace?.personal === true; /** * The highest role the caller can give here: any for an admin, otherwise its * own. In a personal workspace that is the account's role, so a viewer shares - * it as viewer. The server refuses the same (assertRoleGrantable). + * it as viewer. The same rule the server enforces (workspaceGrantCeiling). */ - const grantCeiling: WorkspaceRole = me?.role === 'admin' ? 'owner' : (workspace?.role ?? 'viewer'); - const canGive = (r: WorkspaceRole) => workspaceRoleAtMost(r, grantCeiling); + const grantCeiling = workspaceGrantCeiling(me?.role ?? '', workspace?.role ?? null); + const isOwner = grantCeiling === 'owner'; + /** Who may invite and change roles: owners, admins, and everyone in their own workspace. */ + const managesMembers = isOwner || workspace?.personal === true; const ceilingHint = `You can give up to ${grantCeiling} here: nobody gives more than they can do themselves.`; + const roleOptions = WORKSPACE_ROLES.map((r) => { + const allowed = workspaceRoleAtMost(r, grantCeiling); + return ( + + ); + }); return createPortal(
setInviteRole(e.target.value as WorkspaceRole)} className="bg-slate-950 border border-slate-700 rounded px-2 py-1.5 text-xs text-slate-100" > - {ROLES.map((r) => ( - - ))} + {roleOptions}