From e21df2d01de2d60dc999cb5d422c8b2d221c1a3a Mon Sep 17 00:00:00 2001 From: huyplb Date: Fri, 25 Sep 2026 22:08:43 -0600 Subject: [PATCH 1/2] feat(access): list roles as roles, show allow-all, one membership source The Database Access screens read the catalog incompletely, so the most powerful accounts looked like they held nothing and roles looked like users. Catalog (packages/sql db-access): - MySQL/TiDB: a locked, passwordless mysql.user row is a role. MariaDB: is_role, named bare (no host), members from roles_mapping. - MySQL family: read USER_PRIVILEGES as GLOBAL (*.*) rows and role memberships as ROLE rows, with fallbacks for logins that cannot read mysql.*. - Postgres family: read rolsuper; read schema and database ACLs with aclexplode (the usage_privileges WHERE object_type = 'SCHEMA' branch could never match). Old forms kept as fallbacks. - ClickHouse: memberships from system.role_grants; *.* is GLOBAL, not SYSTEM. - Db2: list OS groups (GRANTEETYPE 'G'). SQL Server: sysadmin as superuser, and what db_owner / db_datareader / db_datawriter imply. - reconcileDbAccess fills memberOf/members and ROLE rows from each other, so the list and the detail pane can no longer disagree. - Role grants honour WITH ADMIN OPTION; MySQL roles are named 'r'@'%'. UI: - Database Access: allow-all/superuser tag and banner (followed through roles), filter by "allow-all", a full privilege set collapsed to one ALL PRIVILEGES row with Revoke all, "grantable" instead of "*", a role picker for membership, and an "All privileges" grant kind per engine with typed-name confirmation. - User Management: allow-all tag, and Add can put the account in existing roles (UserRequest.roles), with SET DEFAULT ROLE on the MySQL family. - Permissions tab: allow-all banner on the account. Also: effective.ts used literal NUL bytes as a key separator. Co-Authored-By: Claude Opus 5.5 --- .../tests/database-access-dialects.test.ts | 130 ++++- .../components/AccessPermissionPanel.test.tsx | 29 + .../components/AccessPermissionPanel.tsx | 19 +- .../access/components/AccessView.test.tsx | 59 ++ .../access/components/UserManagement.tsx | 66 ++- .../components/DatabaseAccessModal.test.tsx | 174 +++++- .../components/DatabaseAccessModal.tsx | 452 ++++++++++++--- docs/USER_GUIDE.md | 28 + .../src/features/access/db-access.service.ts | 12 +- packages/sql/src/index.ts | 15 + .../sql/src/modules/access/db-access.test.ts | 259 +++++++++ packages/sql/src/modules/access/db-access.ts | 536 ++++++++++++++++-- packages/sql/src/modules/access/effective.ts | Bin 16346 -> 16356 bytes .../modules/access/privilege-groups.test.ts | 235 ++++++++ .../src/modules/access/privilege-groups.ts | 345 +++++++++++ .../sql/src/modules/access/user-sql.test.ts | 46 ++ packages/sql/src/modules/access/user-sql.ts | 67 ++- .../sql/src/modules/access/user-sql.types.ts | 6 + 18 files changed, 2343 insertions(+), 135 deletions(-) create mode 100644 packages/sql/src/modules/access/privilege-groups.test.ts create mode 100644 packages/sql/src/modules/access/privilege-groups.ts diff --git a/apps/e2e/src/tests/database-access-dialects.test.ts b/apps/e2e/src/tests/database-access-dialects.test.ts index 8e3bf40d..d967b44c 100644 --- a/apps/e2e/src/tests/database-access-dialects.test.ts +++ b/apps/e2e/src/tests/database-access-dialects.test.ts @@ -34,7 +34,7 @@ import { describe, it, beforeAll, afterAll, beforeEach, expect } from 'vitest'; import type { Page } from 'playwright'; import { buildDriver, quitDriver } from '../helpers/driver.js'; import { getSourceConfig, hasConfig } from '../helpers/db-config.js'; -import { deleteSavedConnections, engineAcceptsSyntax } from '../helpers/sql-exec.js'; +import { deleteSavedConnections, engineAcceptsSyntax, tryCleanup } from '../helpers/sql-exec.js'; import { clickRateLimited } from '../helpers/rate-limited.js'; import { saveScreenshot } from '../helpers/screenshot.js'; import { AppPage } from '../pages/AppPage.js'; @@ -92,6 +92,87 @@ const only = (process.env.E2E_DIALECTS ?? '') .map((s) => s.trim().toLowerCase()) .filter(Boolean); +/** + * A role, an account in it, and an allow-all grant, the way each engine spells + * them — so the test reads back what the engine really holds. + * + * `userRowName` is how the Users list names the account (MySQL family: with + * its host); `allowAll` is the tag expected on it, or null for none. + */ +function rolesFixture( + dialect: string, + runId: string, + password: string +): { + role: string; + user: string; + userRowName: string; + allowAll: 'allow-all' | 'superuser' | null; + setup: string[]; + teardown: string[]; +} | null { + const role = `fox_ro_${runId}`; + const user = `fox_ua_${runId}`; + if (dialect === 'mysql' || dialect === 'tidb') { + return { + role: `${role}@%`, + user: `${user}@%`, + userRowName: `${user}@%`, + allowAll: 'allow-all', + setup: [ + `CREATE ROLE '${role}'@'%'`, + `CREATE USER '${user}'@'%' IDENTIFIED BY '${password}'`, + `GRANT '${role}'@'%' TO '${user}'@'%'`, + `GRANT ALL PRIVILEGES ON *.* TO '${user}'@'%'`, + ], + teardown: [`DROP USER '${user}'@'%'`, `DROP ROLE '${role}'@'%'`], + }; + } + if (dialect === 'mariadb') { + return { + role, + user: `${user}@%`, + userRowName: `${user}@%`, + allowAll: 'allow-all', + setup: [ + `CREATE ROLE ${role}`, + `CREATE USER '${user}'@'%' IDENTIFIED BY '${password}'`, + `GRANT ${role} TO '${user}'@'%'`, + `GRANT ALL PRIVILEGES ON *.* TO '${user}'@'%'`, + ], + teardown: [`DROP USER '${user}'@'%'`, `DROP ROLE ${role}`], + }; + } + if (dialect === 'postgres') { + return { + role, + user, + userRowName: user, + allowAll: 'superuser', + setup: [`CREATE ROLE ${role}`, `CREATE ROLE ${user} LOGIN SUPERUSER IN ROLE ${role}`], + teardown: [`DROP ROLE ${user}`, `DROP ROLE ${role}`], + }; + } + if (dialect === 'clickhouse') { + // The e2e `default` user may not grant ALL; SELECT ON *.* is enough to + // check that an instance-wide grant is read, and it is not allow-all. + return { + role, + user, + userRowName: user, + allowAll: null, + setup: [ + `CREATE ROLE ${role}`, + `CREATE USER ${user} IDENTIFIED BY '${password}'`, + `GRANT ${role} TO ${user}`, + `GRANT SELECT ON *.* TO ${user}`, + ], + teardown: [`DROP USER ${user}`, `DROP ROLE ${role}`], + }; + } + return null; +} + const configured = ALL_DIALECTS.filter( (d) => hasConfig(d) && (only.length === 0 || only.includes(d)) ); @@ -679,6 +760,53 @@ describe.skipIf(configured.length === 0)('Database Access · User Management', ( await format.selectOption('raw'); await saveScreenshot(driver, `dbaccess-command-${dialect}`); }, 120_000); + + it('lists roles as roles, with their members, and tags an account allowed everything', async () => { + const fixture = rolesFixture(dialect, runId, password); + if (!fixture) return; + + const setup = await engineAcceptsSyntax(dialect, fixture.setup); + expect(setup.rejected ?? '', setup.rejected ?? '').toBe(''); + if (setup.skipped) { + // This login may not manage roles (the TiDB e2e user, for one). That + // says nothing about how Fox reads them, so there is nothing to test. + console.warn(`[db-access] ${dialect} could not set up roles: ${setup.skipped}`); + await tryCleanup(dialect, fixture.teardown); + return; + } + try { + await driver.locator('[data-testid="access-tab-users"]').click(); + await selectConnection(dialect); + await refreshCatalog(); + await driver.waitForSelector(rowFor(fixture.user), { timeout: 60_000 }); + await catalogIdle(); + + // The role was listed as a user on the MySQL family, which emptied + // "Roles & groups" on every one of them. + expect( + await driver.locator(rowFor(fixture.role)).first().getAttribute('data-kind'), + `${dialect} lists ${fixture.role} as a role` + ).toBe('role'); + const userRow = driver.locator(rowFor(fixture.user)).first(); + expect(await userRow.getAttribute('data-kind')).toBe('user'); + expect(await userRow.innerText(), `${dialect} shows ${fixture.user}'s role`).toContain( + fixture.role + ); + + // `GRANT ALL ON *.*` and a superuser both used to read as holding + // nothing at all. + const tag = driver.locator(`[data-testid="user-allow-all-${fixture.userRowName}"]`); + if (fixture.allowAll) { + expect(await tag.count(), `${dialect} tags ${fixture.user} as allowed everything`).toBe(1); + expect((await tag.innerText()).toLowerCase()).toBe(fixture.allowAll); + } else { + expect(await tag.count()).toBe(0); + } + await saveScreenshot(driver, `dbaccess-roles-${dialect}`); + } finally { + await tryCleanup(dialect, fixture.teardown); + } + }, 180_000); }); } diff --git a/apps/web/src/frontend/features/access/components/AccessPermissionPanel.test.tsx b/apps/web/src/frontend/features/access/components/AccessPermissionPanel.test.tsx index 6b54589e..2e85df23 100644 --- a/apps/web/src/frontend/features/access/components/AccessPermissionPanel.test.tsx +++ b/apps/web/src/frontend/features/access/components/AccessPermissionPanel.test.tsx @@ -222,3 +222,32 @@ describe('AccessPermissionPanel — one session', () => { expect(runAccessSql).not.toHaveBeenCalled(); }); }); + +describe('AccessPermissionPanel — allow-all', () => { + async function accountOf(name: string) { + render(); + fireEvent.change(screen.getByTestId('access-permission-connection'), { target: { value: 'c1' } }); + await waitFor(() => expect(screen.getByTestId(`access-permission-row-${name}`)).toBeTruthy()); + fireEvent.click(screen.getByTestId(`access-permission-row-${name}`)); + fireEvent.click(screen.getByTestId('access-permission-stage-account')); + } + + it('says an account inherits superuser through its role', async () => { + fetchDbAccess.mockResolvedValue({ + ...catalog, + principals: [ + { ...catalog.principals[0]!, superuser: false }, + { ...catalog.principals[1]!, superuser: true }, + ], + }); + await accountOf('alice'); + expect(screen.getByTestId('access-permission-allow-all').textContent).toMatch( + /Superuser.*Inherited through readonly/ + ); + }); + + it('shows no banner for an ordinary account', async () => { + await accountOf('alice'); + expect(screen.queryByTestId('access-permission-allow-all')).toBeNull(); + }); +}); diff --git a/apps/web/src/frontend/features/access/components/AccessPermissionPanel.tsx b/apps/web/src/frontend/features/access/components/AccessPermissionPanel.tsx index f3adcc48..dfacf2c7 100644 --- a/apps/web/src/frontend/features/access/components/AccessPermissionPanel.tsx +++ b/apps/web/src/frontend/features/access/components/AccessPermissionPanel.tsx @@ -21,7 +21,9 @@ import { RefreshCw, } from 'lucide-react'; import { + describeAllowAll, dialectSupportsDbAccess, + findAllowAll, userManagementSupport, privilegesForPrincipal, type DbPrincipal, @@ -474,6 +476,7 @@ export const AccessPermissionPanel: React.FC<{ {selected && stage === 'account' && ( void; -}> = ({ principal, privileges, dialect, onManageUsers }) => { +}> = ({ principal, principals, privileges, dialect, onManageUsers }) => { // Both memoised: this is an unmemoised child of a panel that owns the // principal filter, so every keystroke re-ran dropSafetyNotes, which walks // the whole privileges array. @@ -592,6 +597,10 @@ const AccountStage: React.FC<{ () => dropSafetyNotes(principal, privileges), [principal, privileges] ); + const allowAll = useMemo( + () => findAllowAll({ principal: principal.name, principals, privileges, dialect }), + [principal.name, principals, privileges, dialect] + ); const login = principal.canLogin === true ? 'Can log in' @@ -604,6 +613,14 @@ const AccountStage: React.FC<{ Account details from the GRANT catalog. Add, rename, or drop accounts under User Management — Access generates SQL only.

+ {allowAll && ( +

+ {describeAllowAll(allowAll)} +

+ )}
Name
diff --git a/apps/web/src/frontend/features/access/components/AccessView.test.tsx b/apps/web/src/frontend/features/access/components/AccessView.test.tsx index d2245d06..99c86a3f 100644 --- a/apps/web/src/frontend/features/access/components/AccessView.test.tsx +++ b/apps/web/src/frontend/features/access/components/AccessView.test.tsx @@ -634,3 +634,62 @@ describe('AccessView — Permission stale catalog', () => { expect(principals).not.toMatch(/only_on_postgres/); }); }); + +describe('AccessView — User Management roles and allow-all', () => { + const catalog = (dialect: string, principals: unknown[]) => + fetchDbAccess.mockResolvedValue({ + dialect, + schema: 'public', + mode: 'native', + support: { mode: 'native', query: true, grant: true, hint: '' }, + principals, + privileges: [], + }); + + beforeEach(() => { + fetchDbAccess.mockReset(); + fetchSchemaList.mockReset(); + fetchSchemaList.mockResolvedValue(['public']); + }); + + async function openUsers(connectionId: string, row: string) { + render(); + fireEvent.click(screen.getByTestId('access-tab-users')); + fireEvent.change(screen.getByTestId('access-connection'), { target: { value: connectionId } }); + await waitFor(() => expect(screen.getByTestId(`user-row-${row}`)).toBeTruthy()); + } + + it('puts a new account in the roles picked from the list', async () => { + catalog('postgres', [ + { name: 'readonly', kind: 'role', canLogin: false, memberOf: [], members: [] }, + { name: 'writers', kind: 'role', canLogin: false, memberOf: [], members: [] }, + ]); + await openUsers('c1', 'readonly'); + fireEvent.click(screen.getByTestId('user-add-user')); + fireEvent.change(screen.getByTestId('user-name'), { target: { value: 'analyst' } }); + fireEvent.click(screen.getByTestId('user-member-of-item-readonly')); + + const sql = screen.getByTestId('user-sql').textContent ?? ''; + expect(sql).toMatch(/CREATE ROLE "analyst"/); + expect(sql).toMatch(/GRANT "readonly" TO "analyst";/); + expect(sql).not.toMatch(/writers/); + }); + + it('tags a superuser in the list', async () => { + catalog('postgres', [ + { name: 'boss', kind: 'user', canLogin: true, memberOf: [], members: [], superuser: true }, + { name: 'alice', kind: 'user', canLogin: true, memberOf: [], members: [], superuser: false }, + ]); + await openUsers('c1', 'boss'); + expect(screen.getByTestId('user-allow-all-boss').textContent).toBe('superuser'); + expect(screen.queryByTestId('user-allow-all-alice')).toBeNull(); + }); + + it('drops a MySQL role by its account name, not as name@host@%', async () => { + catalog('mysql', [{ name: 'reader@%', kind: 'role', canLogin: false, memberOf: [], members: [] }]); + await openUsers('c2', 'reader@%'); + fireEvent.click(screen.getByTestId('user-row-reader@%')); + fireEvent.click(screen.getByTestId('user-drop-selected')); + expect(screen.getByTestId('user-sql').textContent).toMatch(/DROP ROLE 'reader'@'%';/); + }); +}); diff --git a/apps/web/src/frontend/features/access/components/UserManagement.tsx b/apps/web/src/frontend/features/access/components/UserManagement.tsx index e58a39d8..8c8848db 100644 --- a/apps/web/src/frontend/features/access/components/UserManagement.tsx +++ b/apps/web/src/frontend/features/access/components/UserManagement.tsx @@ -75,7 +75,7 @@ import { } from '../lib/accountAlterations'; import type { AccessPrincipalDraft } from '../lib/access'; import { writeClipboard } from '@/shared/utils/clipboard'; -import { dialectFamily } from '@foxschema/sql'; +import { describeAllowAll, dialectFamily, findAllowAll, type AllowAll } from '@foxschema/sql'; type Mode = 'idle' | 'add' | 'edit' | 'drop' | 'list'; @@ -190,6 +190,12 @@ export const UserManagement: React.FC<{ */ const [grantDatabases, setGrantDatabases] = useState([]); const [grantSchemas, setGrantSchemas] = useState([]); + /** + * Existing roles and groups the new account joins. Picked from the catalog + * rather than typed: the roles were listed on this very screen, and the form + * used to offer no way to use them. + */ + const [memberRoles, setMemberRoles] = useState([]); const osPasswordError = osPassword ? validateDb2OsPassword(osPassword) : null; const [copied, setCopied] = useState(false); const [copiedWithPassword, setCopiedWithPassword] = useState(false); @@ -292,6 +298,17 @@ export const UserManagement: React.FC<{ }); }, [principals, filter, kindFilter]); + /** Accounts allowed everything — superuser, all of `*.*`, or the whole database. */ + const allowAllByName = useMemo(() => { + const map = new Map(); + if (!dialect) return map; + for (const p of principals) { + const allow = findAllowAll({ principal: p.name, principals, privileges, dialect }); + if (allow) map.set(p.name, allow); + } + return map; + }, [principals, privileges, dialect]); + const nameOptions = useMemo( () => principals @@ -316,10 +333,13 @@ export const UserManagement: React.FC<{ newName, alteration, validUntil: alteration === 'expire' ? validUntil : undefined, - host: isMysqlFamily && principalType === 'user' ? host : undefined, + // Roles carry the host too: a MySQL role is `'r'@'%'`, and a role listed + // with another host must be dropped by that one. MariaDB ignores it. + host: isMysqlFamily ? host : undefined, cascade, + roles: action === 'create' ? memberRoles : undefined, }), - [action, principalType, name, newName, alteration, validUntil, host, isMysqlFamily, cascade] + [action, principalType, name, newName, alteration, validUntil, host, isMysqlFamily, cascade, memberRoles] ); const generated = useMemo(() => { @@ -549,6 +569,7 @@ export const UserManagement: React.FC<{ setListWarning(null); setName(''); setFilter(''); + setMemberRoles([]); }, [connectionId]); useEffect(() => { @@ -574,7 +595,9 @@ export const UserManagement: React.FC<{ setMode('drop'); setSelectedName(p.name); setPrincipalType(principalTypeOf(p)); - if (isMysqlFamily && p.kind === 'user') { + // A MySQL or TiDB role is an account too, listed as name@host; a MariaDB + // role has no host and no @ in its name. + if (isMysqlFamily && (p.kind === 'user' || p.name.includes('@'))) { const parsed = parseMysqlAccount(p.name); setName(parsed.name); setHost(parsed.host || '%'); @@ -588,7 +611,9 @@ export const UserManagement: React.FC<{ setMode('edit'); setSelectedName(p.name); setPrincipalType(principalTypeOf(p)); - if (isMysqlFamily && p.kind === 'user') { + // A MySQL or TiDB role is an account too, listed as name@host; a MariaDB + // role has no host and no @ in its name. + if (isMysqlFamily && (p.kind === 'user' || p.name.includes('@'))) { const parsed = parseMysqlAccount(p.name); setName(parsed.name); setHost(parsed.host || '%'); @@ -984,7 +1009,18 @@ export const UserManagement: React.FC<{ : 'text-slate-300 hover:bg-slate-900/80' }`} > - {p.name} + + {p.name} + {allowAllByName.get(p.name) && ( + + {allowAllByName.get(p.name)!.kind === 'superuser' ? 'superuser' : 'allow-all'} + + )} + {p.kind} {p.memberOf.length ? p.memberOf.join(', ') : '—'} @@ -1280,6 +1316,24 @@ export const UserManagement: React.FC<{ )} + {mode === 'add' && + !(isDb2 && principalType === 'user') && + roleOptions.some((o) => o.value !== name.trim()) && ( + + o.value).filter((v) => v !== name.trim())} + selected={memberRoles} + onChange={setMemberRoles} + emptyHint="No roles or groups listed for this connection." + /> + + )} + {mode === 'edit' && alteration === 'expire' && support.canExpire && ( o.value)).toEqual(['sections', 'membership']); + // Postgres has database-level ALL, so allow-all is offered too. + expect([...kind.options].map((o) => o.value)).toEqual(['sections', 'membership', 'all']); expect(screen.getByTestId('db-access-permission-sections')).toBeTruthy(); fireEvent.change(kind, { target: { value: 'membership' } }); @@ -309,3 +310,174 @@ describe('DatabaseAccessModal — dialect-aware general CREATE', () => { expect(sql).toMatch(/alice/i); }); }); + +describe('DatabaseAccessModal — roles and allow-all', () => { + /** What `GRANT ALL PRIVILEGES ON *.*` leaves in MySQL's USER_PRIVILEGES. */ + const MYSQL_ALL = [ + 'ALTER', 'ALTER ROUTINE', 'CREATE', 'CREATE ROUTINE', 'CREATE TEMPORARY TABLES', 'CREATE USER', + 'CREATE VIEW', 'DELETE', 'DROP', 'EVENT', 'EXECUTE', 'FILE', 'INDEX', 'INSERT', 'LOCK TABLES', + 'PROCESS', 'REFERENCES', 'RELOAD', 'REPLICATION CLIENT', 'REPLICATION SLAVE', 'SELECT', + 'SHOW DATABASES', 'SHOW VIEW', 'SHUTDOWN', 'SUPER', 'TRIGGER', 'UPDATE', + ]; + const row = (grantee: string, privilege: string, objectType: string, extra: object = {}) => ({ + grantee, + privilege, + objectType, + objectSchema: null, + objectName: null, + grantable: false, + grantor: null, + state: null, + ...extra, + }); + + function catalog(dialect: string, principals: unknown[], privileges: unknown[]) { + useSyncStore.setState({ + connections: [ + { id: 'c1', name: 'prod', dialect, schema: 'demo_a', database: 'demo_a', hasPassword: true }, + ], + } as never); + fetchDbAccess.mockResolvedValue({ + dialect, + schema: 'demo_a', + mode: 'native', + support: { mode: 'native', query: true, grant: true, hint: 'catalog' }, + principals, + privileges, + }); + } + + async function open(name: string) { + render( undefined} />); + fireEvent.change(screen.getByTestId('db-access-connection'), { target: { value: 'c1' } }); + await waitFor(() => expect(fetchDbAccess).toHaveBeenCalled()); + await waitFor(() => expect(screen.getByTestId(`db-access-principal-${name}`)).toBeTruthy()); + fireEvent.click(screen.getByTestId(`db-access-principal-${name}`)); + } + + it('says ALL PRIVILEGES ON *.* once, tags the account, and revokes it whole', async () => { + catalog( + 'mysql', + [{ name: 'app@%', kind: 'user', canLogin: true, memberOf: [], members: [] }], + MYSQL_ALL.map((p) => row('app@%', p, 'GLOBAL')) + ); + await open('app@%'); + + expect(screen.getByTestId('db-access-allow-all-app@%').textContent).toBe('allow-all'); + expect(screen.getByTestId('db-access-allow-all-banner').textContent).toMatch( + /every privilege on the whole server/ + ); + const group = screen.getByTestId('db-access-privgroup-0'); + expect(group.getAttribute('data-all')).toBe('true'); + expect(group.textContent).toMatch(/ALL PRIVILEGES/); + expect(group.textContent).toMatch(/every database \(\*\.\*\)/); + // Collapsed: 27 privileges are not 27 rows until asked for. + expect(screen.queryByTestId('db-access-revoke-0')).toBeNull(); + fireEvent.click(screen.getByTestId('db-access-privgroup-toggle-0')); + expect(screen.getByTestId('db-access-revoke-0')).toBeTruthy(); + + fireEvent.click(screen.getByTestId('db-access-revoke-all-0')); + expect(screen.getByTestId('db-access-confirm').textContent).toMatch( + "REVOKE ALL PRIVILEGES ON *.* FROM 'app'@'%';" + ); + }); + + it('shows a superuser inherited through a role, and finds it by filter', async () => { + catalog( + 'postgres', + [ + { name: 'admins', kind: 'role', canLogin: false, memberOf: [], members: ['alice'], superuser: true }, + { name: 'alice', kind: 'user', canLogin: true, memberOf: ['admins'], members: [], superuser: false }, + { name: 'bob', kind: 'user', canLogin: true, memberOf: [], members: [], superuser: false }, + ], + [] + ); + await open('alice'); + expect(screen.getByTestId('db-access-allow-all-banner').textContent).toMatch( + /Superuser.*Inherited through admins/ + ); + + fireEvent.change(screen.getByTestId('db-access-filter'), { target: { value: 'allow-all' } }); + expect(screen.queryByTestId('db-access-principal-bob')).toBeNull(); + expect(screen.getByTestId('db-access-principal-alice')).toBeTruthy(); + expect(screen.getByTestId('db-access-principal-admins')).toBeTruthy(); + }); + + it('marks WITH GRANT OPTION as grantable, not with an asterisk', async () => { + catalog( + 'postgres', + [{ name: 'alice', kind: 'user', canLogin: true, memberOf: [], members: [] }], + [row('alice', 'SELECT', 'TABLE', { objectSchema: 'public', objectName: 'orders', grantable: true })] + ); + await open('alice'); + const privileges = screen.getByTestId('db-access-privileges').textContent ?? ''; + expect(screen.getByTestId('db-access-grantable')).toBeTruthy(); + expect(privileges).not.toMatch(/SELECT \*/); + }); + + it('offers the roles the principal is not yet in, and still takes a typed name', async () => { + catalog( + 'postgres', + [ + { name: 'analysts', kind: 'role', canLogin: false, memberOf: [], members: ['alice'] }, + { name: 'readers', kind: 'role', canLogin: false, memberOf: [], members: [] }, + { name: 'ops', kind: 'group', canLogin: false, memberOf: [], members: [] }, + { name: 'alice', kind: 'user', canLogin: true, memberOf: ['analysts'], members: [] }, + ], + [row('alice', 'analysts', 'ROLE', { objectName: 'analysts' })] + ); + await open('alice'); + fireEvent.change(screen.getByTestId('db-access-grant-kind'), { target: { value: 'membership' } }); + + const pick = screen.getByTestId('db-access-grant-role') as HTMLSelectElement; + const offered = [...pick.options].map((o) => o.textContent); + expect(offered).toEqual(['readers', 'ops', 'Other… (type a name)']); + await waitFor(() => + expect(screen.getByTestId('db-access-grant-sql').textContent).toBe('GRANT "readers" TO "alice";') + ); + expect(screen.queryByTestId('db-access-grant-name')).toBeNull(); + + fireEvent.change(pick, { target: { value: pick.options[2]!.value } }); + fireEvent.change(screen.getByTestId('db-access-grant-name'), { target: { value: 'auditors' } }); + expect(screen.getByTestId('db-access-grant-sql').textContent).toBe('GRANT "auditors" TO "alice";'); + }); + + it('grants everything only after the name is typed, and says what it confers', async () => { + catalog( + 'mysql', + [{ name: 'app@%', kind: 'user', canLogin: true, memberOf: [], members: [] }], + [] + ); + await open('app@%'); + fireEvent.change(screen.getByTestId('db-access-grant-kind'), { target: { value: 'all' } }); + + const target = screen.getByTestId('db-access-all-target') as HTMLSelectElement; + expect([...target.options].map((o) => o.value)).toEqual(['server', 'database']); + expect(screen.getByTestId('db-access-all-note').textContent).toMatch(/Critical.*every database/); + expect(screen.getByTestId('db-access-grant-sql').textContent).toBe( + "GRANT ALL PRIVILEGES ON *.* TO 'app'@'%';" + ); + fireEvent.change(target, { target: { value: 'database' } }); + expect(screen.getByTestId('db-access-grant-sql').textContent).toBe( + "GRANT ALL PRIVILEGES ON `demo_a`.* TO 'app'@'%';" + ); + + fireEvent.click(screen.getByTestId('db-access-grant')); + const run = screen.getByTestId('db-access-confirm-run') as HTMLButtonElement; + expect(run.disabled).toBe(true); + fireEvent.change(screen.getByTestId('db-access-confirm-type'), { target: { value: 'app' } }); + expect(run.disabled).toBe(true); + fireEvent.change(screen.getByTestId('db-access-confirm-type'), { target: { value: 'app@%' } }); + expect(run.disabled).toBe(false); + fireEvent.click(run); + await waitFor(() => expect(executeSql).toHaveBeenCalled()); + expect((executeSql.mock.calls[0][1] as string[]).join('\n')).toMatch(/ON `demo_a`\.\*/); + }); + + it('does not offer allow-all where the engine has none', async () => { + catalog('sqlite', [{ name: 'x', kind: 'user', canLogin: true, memberOf: [], members: [] }], []); + render( undefined} />); + fireEvent.change(screen.getByTestId('db-access-connection'), { target: { value: 'c1' } }); + expect(screen.queryByText('All privileges (allow-all)')).toBeNull(); + }); +}); diff --git a/apps/web/src/frontend/features/utilities/components/DatabaseAccessModal.tsx b/apps/web/src/frontend/features/utilities/components/DatabaseAccessModal.tsx index 4248bb9b..3839b822 100644 --- a/apps/web/src/frontend/features/utilities/components/DatabaseAccessModal.tsx +++ b/apps/web/src/frontend/features/utilities/components/DatabaseAccessModal.tsx @@ -18,10 +18,16 @@ import { X, } from 'lucide-react'; import { + allPrivilegeTargets, buildGrantRevokeSql, + describeAllowAll, dialectSupportsDbAccess, + findAllowAll, groupDbPrincipals, + groupPrivileges, + privilegeTargetLabel, privilegesForPrincipal, + type AllowAll, type DbPrincipal, type DbPrivilege, type DbPrivilegeObjectType, @@ -53,9 +59,39 @@ type ConfirmAction = { title: string; sql: string; kind: 'grant' | 'revoke'; + /** + * The name the reader must type before Run is enabled. Set only for an + * allow-all grant, where one mis-click hands over the whole server. + */ + typeToConfirm?: string; + /** Said above the SQL when set: what running it really does. */ + note?: string; }; const LS_CONN = 'foxschema-utilities-db-access-connection'; +/** The role picker's "type a name" choice — a value no role can have. */ +const OTHER_ROLE = '\u0000other'; + +/** A shortened list of privilege names for a collapsed group. */ +function privilegeSummary(names: readonly string[]): string { + if (names.length <= 3) return names.join(', '); + return `${names.slice(0, 3).join(', ')} +${names.length - 3}`; +} + +function granteeKindOf(p: DbPrincipal): 'user' | 'role' | 'group' { + return p.kind === 'group' ? 'group' : p.kind === 'role' ? 'role' : 'user'; +} + +/** Marks an account allowed everything. The word, not a colour, carries it. */ +const AllowAllTag: React.FC<{ allow: AllowAll; testId: string }> = ({ allow, testId }) => ( + + {allow.kind === 'superuser' ? 'superuser' : 'allow-all'} + +); const GRANT_PRIV_META = PERMISSION_META.find((m) => m.id === 'editor.grant'); export const DatabaseAccessModal: React.FC = ({ @@ -84,6 +120,8 @@ export const DatabaseAccessModal: React.FC = ({ const [filter, setFilter] = useState(''); const [selectedName, setSelectedName] = useState(null); const [expandedGroups, setExpandedGroups] = useState>(() => new Set(['role', 'user'])); + /** Privilege groups (per object) the reader has opened to see each privilege. */ + const [openPrivGroups, setOpenPrivGroups] = useState>(() => new Set()); const [confirm, setConfirm] = useState(null); const loadToken = useRef(0); @@ -92,8 +130,10 @@ export const DatabaseAccessModal: React.FC = ({ const [grantSchema, setGrantSchema] = useState(''); const [grantName, setGrantName] = useState(''); const [grantWithOption, setGrantWithOption] = useState(false); - /** Object grants use the sectioned UX; this toggle is only for role membership. */ - const [grantKind, setGrantKind] = useState<'sections' | 'membership'>('sections'); + /** Object grants use the sectioned UX; membership and allow-all have their own forms. */ + const [grantKind, setGrantKind] = useState<'sections' | 'membership' | 'all'>('sections'); + const [allTargetId, setAllTargetId] = useState(''); + const [confirmTyped, setConfirmTyped] = useState(''); const conn = connections.find((c) => c.id === connectionId); // File dialects carry no password; asking for one blocked the utility outright. @@ -154,18 +194,38 @@ export const DatabaseAccessModal: React.FC = ({ // eslint-disable-next-line react-hooks/exhaustive-deps }, [open, connectionId, needsPassword]); + /** + * Who may do anything, and through whom. Worked out once per catalog read: + * the tag in the list, the filter and the banner all read this. + */ + const allowAllByName = useMemo(() => { + const map = new Map(); + if (!dialect) return map; + for (const p of principals) { + const allow = findAllowAll({ principal: p.name, principals, privileges, dialect }); + if (allow) map.set(p.name, allow); + } + return map; + }, [principals, privileges, dialect]); + const groups = useMemo(() => { const q = filter.trim().toLowerCase(); + // "allow-all" and "superuser" find the accounts that are allowed + // everything — the ones a reader most needs to find, and could not. + const allowAllQuery = q === 'allow-all' || q === 'superuser'; const filtered = q - ? principals.filter( - (p) => + ? principals.filter((p) => { + const allow = allowAllByName.get(p.name); + if (allowAllQuery) return q === 'superuser' ? allow?.kind === 'superuser' : Boolean(allow); + return ( p.name.toLowerCase().includes(q) || p.memberOf.some((m) => m.toLowerCase().includes(q)) || p.members.some((m) => m.toLowerCase().includes(q)) - ) + ); + }) : principals; return groupDbPrincipals(filtered); - }, [principals, filter]); + }, [principals, filter, allowAllByName]); const selected = principals.find((p) => p.name === selectedName) ?? null; const allSelectedPrivs = selected ? privilegesForPrincipal(privileges, selected.name) : []; @@ -174,6 +234,45 @@ export const DatabaseAccessModal: React.FC = ({ // sections below from reading as one. const selectedPrivs = allSelectedPrivs.filter((p) => p.objectType !== 'ROLE'); const selectedMemberships = allSelectedPrivs.filter((p) => p.objectType === 'ROLE'); + const selectedPrivGroups = groupPrivileges(selectedPrivs, dialect); + const selectedAllowAll = selected ? allowAllByName.get(selected.name) ?? null : null; + /** + * Roles and groups the selected principal could be added to: every one the + * catalog listed, less itself and the ones it already belongs to. A typed + * name is still possible, for a catalog the login may only partly read. + */ + const roleChoices = useMemo( + () => + selected + ? principals + .filter((p) => p.kind !== 'user' && p.name !== selected.name) + .filter((p) => !selected.memberOf.some((m) => m.toLowerCase() === p.name.toLowerCase())) + .map((p) => p.name) + : [], + [principals, selected] + ); + const [roleChoice, setRoleChoice] = useState(''); + const typingRole = roleChoices.length === 0 || roleChoice === OTHER_ROLE; + useEffect(() => { + // A new principal means a new list; start on its first role, not a stale one. + setRoleChoice(''); + setGrantName(''); + }, [selectedName]); + useEffect(() => { + if (grantKind !== 'membership' || typingRole) return; + const pick = roleChoices.includes(roleChoice) ? roleChoice : roleChoices[0] ?? ''; + if (pick !== roleChoice) setRoleChoice(pick); + if (pick !== grantName) setGrantName(pick); + }, [grantKind, typingRole, roleChoices, roleChoice, grantName]); + + const togglePrivGroup = (key: string) => { + setOpenPrivGroups((prev) => { + const next = new Set(prev); + if (next.has(key)) next.delete(key); + else next.add(key); + return next; + }); + }; const grantPreview = useMemo(() => { if (!dialect || !selected || grantKind !== 'membership') return null; @@ -200,6 +299,31 @@ export const DatabaseAccessModal: React.FC = ({ conn?.schema, ]); + /** The allow-all grants this engine has, for the "All privileges" kind. */ + const allTargets = useMemo( + () => (dialect ? allPrivilegeTargets(dialect, { database: conn?.database, schema: conn?.schema }) : []), + [dialect, conn?.database, conn?.schema] + ); + const allTarget = allTargets.find((t) => t.id === allTargetId) ?? allTargets[0] ?? null; + const allPreview = useMemo(() => { + if (!dialect || !selected || grantKind !== 'all' || !allTarget) return null; + return buildGrantRevokeSql({ + dialect, + action: 'grant', + privilege: allTarget.privilege, + objectType: allTarget.objectType, + objectSchema: allTarget.objectSchema, + objectName: allTarget.objectName, + grantee: selected.name, + granteeKind: granteeKindOf(selected), + }); + }, [dialect, selected, grantKind, allTarget]); + useEffect(() => { + // A new confirmation starts empty: a name typed for the last one must not + // arm this one. + setConfirmTyped(''); + }, [confirm]); + const runSql = async (sql: string, kind: 'grant' | 'revoke') => { if (!connectionId || !canGrant) return; setRunning(true); @@ -406,6 +530,12 @@ export const DatabaseAccessModal: React.FC = ({ }`} > {p.name} + {allowAllByName.get(p.name) && ( + + )} {p.kind !== 'user' && p.members.length > 0 && ( {p.members.length} member{p.members.length === 1 ? '' : 's'} @@ -449,6 +579,15 @@ export const DatabaseAccessModal: React.FC = ({ )} + {selectedAllowAll && ( +

+ {describeAllowAll(selectedAllowAll)} +

+ )} +
Object privileges ({selectedPrivs.length}) @@ -468,51 +607,138 @@ export const DatabaseAccessModal: React.FC = ({ - {selectedPrivs.map((priv, i) => { - const on = - [priv.objectSchema, priv.objectName].filter(Boolean).join('.') || - priv.objectType; + {selectedPrivGroups.map((group, gi) => { + const on = privilegeTargetLabel(group); + const deny = group.state === 'deny' ? 'DENY ' : ''; + // One privilege on an object needs no group row. More + // than one collapses, and a complete set says ALL — + // thirty rows for `ON *.*` hid the one fact that mattered. + const single = group.privileges.length === 1 && !group.all; + const expanded = single || openPrivGroups.has(group.key); + const implied = group.privileges.some((p) => p.source === 'implied'); + const Chevron = expanded ? ChevronDown : ChevronRight; + const names = group.privileges.map((p) => p.privilege); + const revokeAll = () => { + // A lone ALL / CONTROL row is revoked by its own name; + // an expanded set by the engine's ALL. + const lone = group.privileges.length === 1 ? group.privileges[0]!.privilege : 'ALL'; + const built = buildGrantRevokeSql({ + dialect, + action: 'revoke', + privilege: lone, + objectType: group.objectType, + objectSchema: group.objectSchema, + objectName: group.objectName, + grantee: selected.name, + granteeKind: granteeKindOf(selected), + }); + if ('error' in built) { + setError(built.error); + return; + } + setConfirm({ title: 'Revoke all privileges', sql: built.sql, kind: 'revoke' }); + }; return ( - - - {priv.state === 'deny' ? 'DENY ' : ''} - {priv.privilege} - {priv.grantable ? ' *' : ''} - - {on} - - - - + + + + ({group.privileges.length}) + + + {on} + + {group.all && !implied && group.state !== 'deny' && ( + + )} + + + )} + {expanded && + group.privileges.map((priv) => { + const i = selectedPrivs.indexOf(priv); + return ( + + + {priv.state === 'deny' ? 'DENY ' : ''} + {priv.privilege} + {priv.grantable && ( + + grantable + + )} + {priv.source === 'implied' && ( + + implied by this fixed role + + )} + + {single ? on : ''} + + {priv.source !== 'implied' && ( + + )} + + + ); + })} + ); })} @@ -522,9 +748,10 @@ export const DatabaseAccessModal: React.FC = ({ {selectedPrivs.length > 0 && (

- * may pass the privilege on to - others (WITH GRANT OPTION). DENY{' '} - overrides any grant of the same privilege. + grantable may pass the + privilege on to others (WITH GRANT OPTION).{' '} + DENY overrides any grant of the same + privilege.

)} @@ -631,7 +858,7 @@ export const DatabaseAccessModal: React.FC = ({ data-testid="db-access-grant-kind" value={grantKind} onChange={(e) => { - const v = e.target.value as 'sections' | 'membership'; + const v = e.target.value as 'sections' | 'membership' | 'all'; setGrantKind(v); setGrantObjectType(v === 'membership' ? 'ROLE' : 'TABLE'); }} @@ -639,6 +866,7 @@ export const DatabaseAccessModal: React.FC = ({ > + {allTargets.length > 0 && } @@ -662,18 +890,100 @@ export const DatabaseAccessModal: React.FC = ({ /> )} - {grantKind === 'membership' && ( - <> + {grantKind === 'all' && allTarget && ( +
+

+ Critical · + {allTarget.note} +

+ {allPreview && 'sql' in allPreview && ( + 
+                        {allPreview.sql}
+
+ )} + {allPreview && 'error' in allPreview && ( +

{allPreview.error}

+ )} + +
+ )} + + {grantKind === 'membership' && ( + <> + {roleChoices.length > 0 && ( + + )} + {typingRole && ( + + )}