feat: improve data table roles and grants UX (#11562)

* feat: improve data table roles and grants UX

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* fix: keep selected bordered badges the same height as unselected ones

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* fix: refuse taking over roles with members and test grant statements

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* fix: keep grant filter chips while a filter is set

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* fix: check a taken-over role can log in with the new password

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* feat: select several grant filter tags at once

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* feat: explain where schema and table permissions are set

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* fix: keep the grant create row on one line

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* feat: show grants as a role and its privileges

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* feat: show grants in a table with role, privileges and scope

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* fix: hide the grant scope when it can only be the object itself

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* feat: grant schema usage along with grants inside the schema

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* feat: confirm before closing the roles drawer with unsaved changes

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* chore: update ee-repo-ref to c4002f777f56d90bb0471164291889850c85179d

This commit updates the EE repository reference after PR #848 was merged in windmill-ee-private.

Previous ee-repo-ref: 7937634a52d04caeb083da94e189705cd00796b3

New ee-repo-ref: c4002f777f56d90bb0471164291889850c85179d

Automated by sync-ee-ref workflow.

---------

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-authored-by: windmill-internal-app[bot] <windmill-internal-app[bot]@users.noreply.github.com>

backend/ee-repo-ref.txt

@@ -1 +1 @@
-1600d49ad5513af8b54ad65688d3d1c7edc977d9
+6f23819e947b18878000e580ddefccfd01a73166

backend/windmill-api-integration-tests/tests/datatable_roles.rs

@@ -417,6 +417,89 @@ async fn concurrent_role_creations_both_survive(db: Pool<Postgres>) -> anyhow::R
     outcome
 }
 
+#[cfg(all(feature = "private", feature = "enterprise"))]
+#[sqlx::test(migrations = "../migrations", fixtures("base", "datatable_roles"))]
+async fn an_existing_role_is_taken_over_only_when_asked_and_never_with_members(
+    db: Pool<Postgres>,
+) -> anyhow::Result<()> {
+    initialize_tracing().await;
+    let server = ApiServer::start(db.clone()).await?;
+    let port = server.addr.port();
+
+    let suffix: String = uuid::Uuid::new_v4().simple().to_string()[..8].to_string();
+    let plain = format!("wmtest_plain_{suffix}");
+    let group = format!("wmtest_group_{suffix}");
+    let member = format!("wmtest_member_{suffix}");
+    for sql in [
+        // An expired password and no connection slot: kept as is, the login Windmill sets is dead.
+        format!(
+            "CREATE ROLE \"{plain}\" LOGIN PASSWORD 'theirs' VALID UNTIL '2000-01-01' \
+             CONNECTION LIMIT 0"
+        ),
+        format!("CREATE ROLE \"{group}\" NOLOGIN"),
+        format!("CREATE ROLE \"{member}\" LOGIN"),
+        format!("GRANT \"{group}\" TO \"{member}\""),
+    ] {
+        sqlx::query(&sql).execute(&db).await?;
+    }
+
+    let create = |name: String, take_over: bool| async move {
+        let resp = authed(
+            client().post(format!(
+                "http://localhost:{port}/api/settings/datatable_roles"
+            )),
+            "SECRET_TOKEN",
+        )
+        .json(&json!({ "name": name, "take_over": take_over }))
+        .send()
+        .await?;
+        Ok::<_, anyhow::Error>((resp.status().as_u16(), resp.text().await?))
+    };
+    let outcome = async {
+        let (status, body) = create(plain.clone(), false).await?;
+        assert_eq!(
+            status, 409,
+            "an existing role is not taken without asking: {body}"
+        );
+
+        // The group's member would inherit every grant the data table role gets.
+        let (status, body) = create(group.clone(), true).await?;
+        assert_eq!(status, 400, "{body}");
+        assert!(
+            body.contains(&member),
+            "the refusal names the member: {body}"
+        );
+
+        let (status, body) = create(plain.clone(), true).await?;
+        assert_eq!(status, 200, "{body}");
+        let catalog = windmill_common::datatable_roles::read_role_catalog(
+            &db,
+            windmill_common::datatable_roles::DatatableRoleCluster::Instance,
+        )
+        .await?;
+        let taken = catalog
+            .values()
+            .find(|r| r.name == plain)
+            .ok_or_else(|| anyhow::anyhow!("{plain} is not in the catalog: {catalog:?}"))?;
+        let options = (*db.connect_options())
+            .clone()
+            .username(&plain)
+            .password(taken.pwd.as_deref().unwrap_or_default());
+        let mut login = <sqlx::PgConnection as sqlx::Connection>::connect_with(&options).await?;
+        sqlx::query("SELECT 1").execute(&mut login).await?;
+        Ok::<_, anyhow::Error>(())
+    }
+    .await;
+
+    // Roles are cluster-wide and outlive this test's database, whatever happened above.
+    for name in [&member, &group, &plain] {
+        let _ = sqlx::query(&format!("DROP ROLE IF EXISTS \"{name}\""))
+            .execute(&db)
+            .await;
+    }
+    outcome
+}
+
 #[cfg(all(feature = "private", feature = "enterprise"))]
 #[sqlx::test(migrations = "../migrations", fixtures("base", "datatable_roles"))]
 async fn a_role_delete_that_fails_part_way_leaves_the_role_disabled(

backend/windmill-api-workspaces/src/datatable_acl.rs

@@ -683,6 +683,9 @@ pub(crate) struct CatalogFacts {
     pub(crate) former_owner: Option<FormerOwnerDefaults>,
     /// For a revoke: each grant it takes back.
     pub(crate) revoked_grants: Vec<RevokedGrant>,
+    /// For a grant on what a schema holds: the role cannot reach the schema yet, so the grant
+    /// would give it nothing it can use.
+    pub(crate) lacks_schema_usage: bool,
 }
 
 /// A grant a revoke takes back, as the catalog records it: what `source` gave on `object` (the
@@ -1526,7 +1529,24 @@ async fn build_plan(
             )));
         }
     }
-    let facts = CatalogFacts { other_pg_roles, existing_objects, former_owner, revoked_grants };
+    let lacks_schema_usage = match (&change, target) {
+        (AclChange::Grant { scope, .. }, AclTarget::Schema { schema })
+            if !matches!(scope, GrantScope::Target) =>
+        {
+            !has_schema_usage(client, schema, &pg_role).await?
+        }
+        (AclChange::Grant { .. }, AclTarget::Table { schema, .. }) => {
+            !has_schema_usage(client, schema, &pg_role).await?
+        }
+        _ => false,
+    };
+    let facts = CatalogFacts {
+        other_pg_roles,
+        existing_objects,
+        former_owner,
+        revoked_grants,
+        lacks_schema_usage,
+    };
     let mut plan =
         crate::datatable_acl_oss::plan_statements(target, &change, dbname, &pg_role, &facts)?;
     if matches!(change, AclChange::SetOwner { .. }) {
@@ -1540,6 +1560,27 @@ async fn build_plan(
     Ok(plan)
 }
 
+/// Whether `pg_role` may look into `schema`, directly or through a role it inherits from.
+async fn has_schema_usage(
+    client: &tokio_postgres::Client,
+    schema: &str,
+    pg_role: &str,
+) -> Result<bool> {
+    Ok(client
+        .query_one(
+            "SELECT has_schema_privilege($1::name, $2::text, 'USAGE')",
+            &[&pg_role, &schema],
+        )
+        .await
+        .map_err(|e| {
+            Error::internal_err(format!(
+                "Failed to read the role's privileges on schema {schema}: {}",
+                pg_error_message(&e)
+            ))
+        })?
+        .get(0))
+}
+
 /// Postgres only hands an object to a role that could have created it: a table to one with
 /// `CREATE` on its schema, a schema to one with `CREATE` on the database.
 async fn missing_owner_privilege(

backend/windmill-api/openapi.yaml

@@ -1765,6 +1765,12 @@ paths:
                   type: string
                 cluster:
                   $ref: "#/components/schemas/DatatableRoleCluster"
+                take_over:
+                  type: boolean
+                  description: >
+                    manage a Postgres role of that name that already exists on the cluster,
+                    replacing its password, instead of refusing it. A role with cluster-wide
+                    attributes or memberships is refused either way
       responses:
         "200":
           description: the created role
@@ -1772,6 +1778,8 @@ paths:
             application/json:
               schema:
                 $ref: "#/components/schemas/InstanceDatatableRole"
+        "409":
+          description: a Postgres role of that name already exists on the cluster and take_over was not set
 
   /settings/datatable_roles/{id}:
     post:
@@ -34525,6 +34533,11 @@ components:
           type: boolean
         cluster:
           $ref: "#/components/schemas/DatatableRoleCluster"
+        in_use:
+          type: boolean
+          description: >-
+            Whether any data table, in any workspace, lets someone connect as this role. Only set
+            in a data table's available_roles.
 
     DatatableRoleTenants:
       type: object

backend/windmill-common/src/datatable_roles.rs

@@ -331,6 +331,12 @@ pub async fn converge_connect_grants_with(
 /// `CREATE ROLE <name> LOGIN PASSWORD ...; GRANT <name> TO custom_instance_user` on `cluster`. No
 /// privileges beyond that — an admin grants them through SQL or the ACL editor.
 ///
+/// A role of that name that already exists is refused with a 409, unless `take_over`: then its
+/// password is replaced and it is managed from here on like one created here. A role holding
+/// cluster-wide privileges, or inheriting another role's, is refused either way, since every tenant
+/// would get them. Returns whether an existing role was taken over: undoing a failed take-over
+/// must not drop a role that was there before.
+///
 /// On Windmill's own cluster the DDL runs on `tx`, so it commits with the catalog row. The external
 /// cluster is another server: the role is created there before `tx` commits, and callers MUST drop
 /// it again ([`drop_datatable_role`]) if `tx` then fails to commit.
@@ -343,8 +349,10 @@ pub async fn create_datatable_role(
     cluster: DatatableRoleCluster,
     name: &str,
     password: &str,
-) -> Result<()> {
-    crate::datatable_roles_oss::create_datatable_role(db, tx, cluster, name, password).await
+    take_over: bool,
+) -> Result<bool> {
+    crate::datatable_roles_oss::create_datatable_role(db, tx, cluster, name, password, take_over)
+        .await
 }
 
 /// Authorization: alters a cluster-wide Postgres login. Callers MUST restrict this to superadmin

backend/windmill-common/src/datatable_roles_oss.rs

@@ -125,7 +125,8 @@ mod ce {
         _cluster: DatatableRoleCluster,
         _name: &str,
         _password: &str,
-    ) -> Result<()> {
+        _take_over: bool,
+    ) -> Result<bool> {
         Err(unavailable())
     }
 

frontend/src/lib/components/DBManager.svelte

@@ -375,7 +375,8 @@
 			}
 		if (!isInstanceDatatable(datatable))
 			return {
-				reason: 'Only data tables stored in the instance database have roles and grants.',
+				reason:
+					"Roles and grants are not supported on data tables backed by a Postgres resource. Store the data table on Windmill's database or on an external Postgres instance (set up by a superadmin in instance settings) to use them.",
 				ee: false
 			}
 		if (needsAdmin && !canManageDatatable)
@@ -458,6 +459,11 @@
 	// default to open — actually be folded; a plain "expanded" set could never
 	// close them, since the default would keep winning.
 	// What the permissions drawer is open on, if anything: a schema, or a table.
+	let aclEditor = $state<PgAclEditor | undefined>(undefined)
+	/** The open access editor lists the data table's roles, which the roles drawer can change. */
+	export function refreshAcl() {
+		aclEditor?.refresh()
+	}
 	let aclDrawer = $state<{ datatable: string | undefined; target: AclTarget } | undefined>(
 		undefined
 	)
@@ -1466,7 +1472,7 @@
 <Portal>
 	<Drawer open={!!aclDrawer} size="900px" on:close={() => (aclDrawer = undefined)}>
 		<DrawerContent
-			title="Access — {aclLabel(aclDrawer?.target)}"
+			title="Access: {aclLabel(aclDrawer?.target)}"
 			on:close={() => (aclDrawer = undefined)}
 			tooltip="Who owns this, and what each role may do with it."
 		>
@@ -1474,7 +1480,18 @@
 				{@const dt = aclDrawer.datatable ?? currentDatatable}
 				{#if dt}
 					{#key `${dt}~${JSON.stringify(aclDrawer.target)}`}
-						<PgAclEditor {workspace} datatable={dt} target={aclDrawer.target} />
+						<PgAclEditor
+							bind:this={aclEditor}
+							{workspace}
+							datatable={dt}
+							target={aclDrawer.target}
+							manageRoles={onDatatableAction
+								? {
+										open: () => onDatatableAction?.(dt, 'roles'),
+										blocker: accessBlocker(dt, true)?.reason
+									}
+								: undefined}
+						/>
 					{/key}
 				{/if}
 			{/if}

frontend/src/lib/components/DBManagerDrawer.svelte

@@ -311,6 +311,7 @@
 	function refreshRoles() {
 		datatables.refetch()
 		usableRoles.refetch()
+		dbManagerContent?.dbManager()?.refreshAcl()
 	}
 
 	async function handleExportSchema(explicitSource?: string) {

frontend/src/lib/components/DropdownV2Inner.svelte

@@ -107,9 +107,19 @@
 				<DropdownSubmenuItem {item} {builders} {meltItem} close={close ?? (() => {})} />
 			{:else if item.disabled && item.tooltip}
 				<!-- Wrapper carries the native `title`; the disabled button's `pointer-events-none`
-				     lets the hover reach it so the user learns why the item is disabled. -->
-				<div title={item.tooltip} class="w-full">
-					{@render menuItem(item)}
+				     lets the hover reach it so the user learns why the item is disabled. The ⓘ sits
+				     outside the disabled button, the only place it can receive a hover. -->
+				<div class="relative w-full">
+					<div title={item.tooltip} class="w-full pr-6">
+						{@render menuItem(item)}
+					</div>
+					<div class="absolute right-3 top-1/2 -translate-y-1/2 flex">
+						<Tooltip>
+							{#snippet text()}
+								{item.tooltip}
+							{/snippet}
+						</Tooltip>
+					</div>
 				</div>
 			{:else}
 				{@render menuItem(item)}

frontend/src/lib/components/common/badge/Badge.svelte

@@ -85,7 +85,8 @@
 		['dark-indigo']: 'bg-indigo-700 text-indigo-100 dark:bg-indigo-800 dark:text-indigo-200',
 		['dark-orange']: 'bg-orange-700 text-orange-100 dark:bg-orange-800 dark:text-orange-200',
 		['dark-violet']: 'bg-violet-700 text-violet-100 dark:bg-violet-800 dark:text-violet-200',
-		transparent: 'bg-surface-accent-selected text-accent border-gray-400 dark:border-gray-500'
+		// Keeps the unselected chip's border, so selecting one does not change its height.
+		transparent: 'bg-surface-accent-selected text-accent border border-surface-accent-selected'
 	}
 
 	const hovers: Partial<Record<BadgeColor, string>> = {

frontend/src/lib/components/common/confirmationModal/ConfirmationModal.svelte

@@ -29,6 +29,8 @@
 		 * up confines it. Off by default: it moves the dialog out of its DOM position, so opt
 		 * in per call site rather than assuming every caller wants it. */
 		alwaysPortal?: boolean
+		/** For a notice with nothing to decide: the confirm button is the only one. */
+		hideCancel?: boolean
 		children?: Snippet
 		onConfirmed?: () => void | Promise<void>
 		onCanceled?: () => void
@@ -47,6 +49,7 @@
 		trashbin = false,
 		zIndexClass = 'z-[9999]',
 		alwaysPortal = false,
+		hideCancel = false,
 		children,
 		onConfirmed,
 		onCanceled
@@ -228,15 +231,17 @@
 								{/if}
 								<span class="min-w-20">{confirmationText} </span>
 							</Button>
-							<Button
-								disabled={loading}
-								on:click={() => (dispatch('canceled'), onCanceled?.())}
-								variant="default"
-								size="sm"
-								shortCut={{ key: 'Esc', hide: !keyListen, withoutModifier: true }}
-							>
-								Cancel
-							</Button>
+							{#if !hideCancel}
+								<Button
+									disabled={loading}
+									on:click={() => (dispatch('canceled'), onCanceled?.())}
+									variant="default"
+									size="sm"
+									shortCut={{ key: 'Esc', hide: !keyListen, withoutModifier: true }}
+								>
+									Cancel
+								</Button>
+							{/if}
 						</div>
 					</div>
 				</div>

frontend/src/lib/components/common/drawer/Disposable.svelte

@@ -32,6 +32,9 @@
 		children?: import('svelte').Snippet<[any]>
 		onOpen?: () => void
 		onClose?: () => void
+		/** Asked before a click away or Escape closes it; resolving false keeps it open, e.g. to
+		 *  confirm discarding unsaved changes. */
+		confirmClose?: () => boolean | Promise<boolean>
 	}
 
 	let {
@@ -42,9 +45,25 @@
 		minZIndex = 0,
 		children,
 		onOpen,
-		onClose
+		onClose,
+		confirmClose
 	}: Props = $props()
 
+	// While the question is on screen, the keys and clicks are its own.
+	let confirming = false
+	async function requestClose() {
+		if (confirming) return
+		if (confirmClose) {
+			confirming = true
+			try {
+				if (!(await confirmClose())) return
+			} finally {
+				confirming = false
+			}
+		}
+		closeDrawer()
+	}
+
 	let offset = $state(untrack(() => initialOffset))
 	// Note: when a Modal with minZIndex is open, all disposables (including
 	// already-open Drawers) are elevated. This is acceptable — relative
@@ -115,7 +134,7 @@
 		const last = stack.val[stack.val.length - 1]
 		if (last === id) {
 			e.stopPropagation()
-			closeDrawer()
+			requestClose()
 		}
 	}
 
@@ -125,7 +144,16 @@
 		if (open) {
 			switch (event.key) {
 				case 'Escape':
+					// The confirmation on screen answers this Escape itself, as its Cancel.
+					if (confirming) break
 					if ((id == stack.val[stack.val.length - 1] || stack.val.length == 0) && !preventEscape) {
+						if (confirmClose) {
+							event.preventDefault()
+							event.stopPropagation()
+							event.stopImmediatePropagation()
+							requestClose()
+							break
+						}
 						stack.val.pop()
 						event.preventDefault()
 						event.stopPropagation()

frontend/src/lib/components/common/drawer/Drawer.svelte

@@ -28,6 +28,8 @@
 		positionClass?: string | undefined
 		name?: string
 		children?: import('svelte').Snippet<[any]>
+		/** Asked before a click away or Escape closes it; resolving false keeps it open. */
+		confirmClose?: () => boolean | Promise<boolean>
 	}
 
 	let {
@@ -43,7 +45,8 @@
 		class: clazz = '',
 		positionClass = undefined,
 		name = undefined,
-		children: children_render
+		children: children_render,
+		confirmClose
 	}: Props = $props()
 
 	if (open === undefined) {
@@ -146,6 +149,7 @@
 		onOpen={() => dispatch('open')}
 		onClose={() => dispatch('close')}
 		{preventEscape}
+		{confirmClose}
 	>
 		{#snippet children({ handleClickAway, zIndex, isTop })}
 			<aside

frontend/src/lib/components/datatableAcl/PgAclEditor.svelte

@@ -1,36 +1,48 @@
 <script lang="ts">
 	import { WorkspaceService, type AclChange, type AclTarget, type DatatableAclInfo } from '$lib/gen'
 	import { resource } from 'runed'
-	import { Trash2 } from 'lucide-svelte'
+	import type { Snippet } from 'svelte'
+	import { Tooltip } from '../meltComponents'
+	import { KeyRound, Trash2 } from 'lucide-svelte'
 	import { sendUserToast } from '$lib/toast'
 	import { Alert, Button } from '../common'
 	import ConfirmationModal from '../common/confirmationModal/ConfirmationModal.svelte'
 	import Select from '../select/Select.svelte'
+	import PgGrantBuilder from './PgGrantBuilder.svelte'
 	import DataTable from '../table/DataTable.svelte'
 	import Head from '../table/Head.svelte'
 	import Row from '../table/Row.svelte'
 	import Cell from '../table/Cell.svelte'
-	import PgGrantBuilder from './PgGrantBuilder.svelte'
+	import Badge from '../common/badge/Badge.svelte'
 	import {
 		ADMIN_ROLE,
 		blockingSources,
 		grantKey,
-		grantScopeLabel,
+		grantCoverage,
+		scopesOf,
 		groupGrants,
 		revocablePrivileges,
 		revokeScopeOf,
-		uncoveredCreators
+		uncoveredCreators,
+		type AclScope,
+		type GroupedGrant
 	} from './aclScopes'
 
 	let {
 		workspace,
 		datatable,
-		target
+		target,
+		disabledReason,
+		manageRoles
 	}: {
 		workspace: string
 		datatable: string
 		/** What owner and grants are read and written for. */
 		target: AclTarget
+		/** Shows owner and grants without letting them change, with this reason on hover. */
+		disabledReason?: string
+		/** Offered at the bottom of every role picker. `blocker` keeps the entry, disabled, with why. */
+		manageRoles?: { open: () => void; blocker?: string }
 	} = $props()
 
 	const acl = resource(
@@ -45,6 +57,11 @@
 			})
 	)
 
+	/** Re-read owner, grants and the roles they can name, e.g. after the data table's roles changed. */
+	export function refresh() {
+		acl.refetch()
+	}
+
 	// Nothing is written before its SQL has been shown, and the apply runs exactly that SQL: the
 	// server plans again and refuses if the result differs.
 	let pending = $state<
@@ -54,7 +71,32 @@
 	let applying = $state(false)
 
 	const info: DatatableAclInfo | undefined = $derived(acl.current)
+	const controlsDisabled = $derived(planning || applying || !!disabledReason)
 	const grantRows = $derived(groupGrants(info?.grants ?? []))
+	// Several tags of one kind widen the list; tags of both kinds narrow it to grants matching each.
+	let roleFilters = $state<string[]>([])
+	let privilegeFilters = $state<string[]>([])
+	// A selected tag stays offered after its last grant goes, so it can still be cleared.
+	const roleChips = $derived(
+		[...new Set([...grantRows.map((g) => g.grantee), ...roleFilters])].sort()
+	)
+	const privilegeChips = $derived(
+		[...new Set([...grantRows.flatMap((g) => g.privileges), ...privilegeFilters])].sort()
+	)
+	const shownGrantRows = $derived(
+		grantRows.filter(
+			(g) =>
+				(roleFilters.length === 0 || roleFilters.includes(g.grantee)) &&
+				(privilegeFilters.length === 0 || g.privileges.some((p) => privilegeFilters.includes(p)))
+		)
+	)
+	/** What a grant on the object itself reads as, in the scope picker's words. */
+	const targetLabel = $derived(scopesOf(target.kind)[0].label)
+	// Most grants are on the object itself, which the column would only repeat.
+	const showCoverage = $derived(grantRows.some((g) => grantCoverage(g, target) !== undefined))
+	const columns = $derived(showCoverage ? 4 : 3)
+	const toggled = (list: string[], value: string) =>
+		list.includes(value) ? list.filter((v) => v !== value) : [...list, value]
 	const ownerItems = $derived(
 		info
 			? (info.roles.includes(info.owner) ? info.roles : [info.owner, ...info.roles]).map((r) => ({
@@ -68,6 +110,28 @@
 		pending?.change.type === 'revoke' && (pending.change.objects?.length ?? 0) > 1
 	)
 
+	/** Why a row's delete button is disabled, or undefined when it can revoke. */
+	function unrevocableReason(
+		grant: GroupedGrant,
+		revokeScope: AclScope | undefined,
+		revocable: string[],
+		blocked: string[]
+	): string | undefined {
+		// What `admin` holds is what every role here connects through.
+		if (grant.grantee === ADMIN_ROLE)
+			return 'admin is the connection every role reaches this data table through, so its grants are not changed here'
+		if (!info?.roles.includes(grant.grantee))
+			return `Only grants to this data table's roles are revoked here, and ${grant.grantee} is not one`
+		if (!revokeScope) return 'Grants on types are read only here'
+		if (revocable.length === 0)
+			return grant.future
+				? 'Default privileges set database-wide are not revoked from here'
+				: "CONNECT follows the data table's roles and TEMPORARY is left as Postgres sets it, so nothing here can be revoked"
+		if (blocked.length > 0)
+			return `Only ${blocked.join(', ')} can revoke this: Postgres takes a grant back through the role that made it`
+		return undefined
+	}
+
 	function errorText(e: any): string {
 		return e?.body ?? e?.message ?? String(e)
 	}
@@ -108,6 +172,60 @@
 	}
 </script>
 
+<!-- A disabled control receives no hover, so the tooltip's trigger is a wrapper the hover falls
+     through to. -->
+{#snippet withDisabledReason(content: Snippet, widthClass: string)}
+	{#if disabledReason}
+		<Tooltip class="block {widthClass}" anchor="cursor">
+			<div class="pointer-events-none">{@render content()}</div>
+			{#snippet text()}{disabledReason}{/snippet}
+		</Tooltip>
+	{:else}
+		{@render content()}
+	{/if}
+{/snippet}
+
+<!-- The home page's filter chips, any number of them selected at once. -->
+{#snippet filterChip(label: string, selected: boolean, toggle: () => void)}
+	<Badge color="transparent" clickable {selected} onclick={toggle}>
+		{label}
+		{#if selected}&cross;{/if}
+	</Badge>
+{/snippet}
+
+<!-- Styled like the picker's tags in the row that creates a grant. -->
+{#snippet tag(text: string)}
+	<span class="inline-flex items-center min-h-6 px-2 border bg-surface rounded-full text-emphasis">
+		{text}
+	</span>
+{/snippet}
+
+{#snippet manageRolesEntry({ close }: { close: () => void })}
+	<div class="flex items-center gap-1 border-t p-1">
+		<Button
+			unifiedSize="sm"
+			variant="subtle"
+			startIcon={{ icon: KeyRound }}
+			disabled={!!manageRoles?.blocker}
+			btnClasses="w-full justify-start"
+			wrapperClasses="grow"
+			onClick={() => {
+				close()
+				manageRoles?.open()
+			}}
+		>
+			Manage roles
+		</Button>
+		{#if manageRoles?.blocker}
+			<span class="pr-2 flex">
+				<Tooltip>
+					{#snippet text()}{manageRoles?.blocker}{/snippet}
+				</Tooltip>
+			</span>
+		{/if}
+	</div>
+{/snippet}
+
 {#if acl.error}
 	<Alert type="error" title="Could not read access" size="xs">{errorText(acl.error)}</Alert>
 {:else if !info}
@@ -137,22 +255,26 @@
 					</span>
 				</div>
 				{#if info.editable}
-					<Select
-						items={ownerItems}
-						disabled={planning || applying}
-						size="sm"
-						class="w-64"
-						bind:value={
-							() => info.owner,
-							(role) => {
-								// The select shows what the database says; a pick is a request, and only the
-								// applied change moves it.
-								if (role && role !== info.owner) {
-									confirm({ type: 'set_owner', role }, `Ownership transferred to ${role}`)
+					{#snippet ownerSelect()}
+						<Select
+							items={ownerItems}
+							disabled={controlsDisabled}
+							size="sm"
+							class="w-64"
+							bind:value={
+								() => info.owner,
+								(role) => {
+									// The select shows what the database says; a pick is a request, and only the
+									// applied change moves it.
+									if (role && role !== info.owner) {
+										confirm({ type: 'set_owner', role }, `Ownership transferred to ${role}`)
+									}
 								}
 							}
-						}
-					/>
+							bottomSnippet={manageRoles ? manageRolesEntry : undefined}
+						/>
+					{/snippet}
+					{@render withDisabledReason(ownerSelect, 'w-64')}
 				{:else}
 					<span class="font-mono text-xs">{info.owner}</span>
 				{/if}
@@ -168,75 +290,76 @@
 						: 'What each role may do here, beyond what it owns.'}
 				</span>
 			</div>
-			{#if info.editable}
-				<PgGrantBuilder
-					{target}
-					roles={info.roles}
-					disabled={planning || applying}
-					supportsMaintain={info.supports_maintain}
-					dbname={info.dbname}
-					onAdd={({ role, privileges, scope }) =>
-						confirm(
-							{ type: 'grant', role, privileges, scope },
-							`Granted ${privileges.join(', ')} to ${role}`
-						)}
-				/>
+			<!-- A set filter keeps its chips, or a revoke that leaves one grant would strand it. -->
+			{#if grantRows.length > 1 || roleFilters.length > 0 || privilegeFilters.length > 0}
+				<div class="flex flex-wrap items-center gap-2">
+					{#each roleChips as role (`role:${role}`)}
+						{@render filterChip(role, roleFilters.includes(role), () => {
+							roleFilters = toggled(roleFilters, role)
+						})}
+					{/each}
+					{#each privilegeChips as privilege (`privilege:${privilege}`)}
+						{@render filterChip(privilege, privilegeFilters.includes(privilege), () => {
+							privilegeFilters = toggled(privilegeFilters, privilege)
+						})}
+					{/each}
+				</div>
 			{/if}
-			{#if grantRows.length === 0}
-				<span class="text-xs text-secondary">No grants yet.</span>
-			{:else}
-				<DataTable size="xs">
-					<Head>
-						<tr>
-							<Cell head first>Role</Cell>
-							<Cell head>Privileges</Cell>
+			<DataTable size="xs">
+				<Head>
+					<tr>
+						<Cell head first>Role</Cell>
+						<Cell head>Privileges</Cell>
+						{#if showCoverage}
 							<Cell head>On</Cell>
-							<Cell head last></Cell>
-						</tr>
-					</Head>
-					<tbody class="divide-y">
-						{#each grantRows as grant (grantKey(grant))}
-							{@const revokeScope = revokeScopeOf(grant)}
-							{@const revocable = revocablePrivileges(grant, target)}
-							{@const blocked = blockingSources(grant, revocable)}
-							{@const uncovered = uncoveredCreators(grant, info.roles)}
-							<Row>
-								<Cell first>{grant.grantee}</Cell>
-								<Cell wrap
-									><span class="font-mono text-2xs">{grant.privileges.join(', ')}</span></Cell
-								>
-								<Cell wrap>
-									{grantScopeLabel(grant)}
-									{#if blocked.length > 0}
-										<span
-											class="text-2xs text-secondary"
-											title="Only this role can take the grant back: Postgres revokes a grant through the role that made it"
-										>
-											from {blocked.join(', ')}
-										</span>
-									{/if}
-									{#if uncovered.length > 0}
-										<span
-											class="text-2xs text-secondary"
-											title="A default privilege covers only the roles it was granted for: grant it again to cover these"
-										>
-											· not for what {uncovered.join(', ')}
-											{uncovered.length === 1 ? 'creates' : 'create'}
-										</span>
-									{/if}
+						{/if}
+						<Cell head last />
+					</tr>
+				</Head>
+				<tbody class="divide-y">
+					{#each shownGrantRows as grant (grantKey(grant))}
+						{@const revokeScope = revokeScopeOf(grant)}
+						{@const revocable = revocablePrivileges(grant, target)}
+						{@const blocked = blockingSources(grant, revocable)}
+						{@const uncovered = uncoveredCreators(grant, info.roles)}
+						{@const unrevocable = unrevocableReason(grant, revokeScope, revocable, blocked)}
+						<Row>
+							<Cell first class="font-medium text-emphasis">{grant.grantee}</Cell>
+							<Cell wrap>
+								<div class="flex flex-wrap items-center gap-1">
+									{#each grant.privileges as privilege (privilege)}
+										{@render tag(privilege)}
+									{/each}
+								</div>
+								{#if blocked.length > 0 || uncovered.length > 0}
+									<div class="mt-0.5 text-2xs text-secondary">
+										{[
+											blocked.length > 0 && `granted by ${blocked.join(', ')}`,
+											uncovered.length > 0 &&
+												`not for what ${uncovered.join(', ')} ${uncovered.length === 1 ? 'creates' : 'create'}: a default privilege only covers the roles it was granted for`
+										]
+											.filter(Boolean)
+											.join(' · ')}
+									</div>
+								{/if}
+							</Cell>
+							{#if showCoverage}
+								<Cell wrap class="text-secondary">
+									{grantCoverage(grant, target) ?? targetLabel}
 								</Cell>
-								<Cell last>
-									<!-- What `admin` holds is what every role here connects through, so it is not
-									this editor's to take away. -->
-									{#if info.editable && revokeScope && revocable.length > 0 && blocked.length === 0 && info.roles.includes(grant.grantee) && grant.grantee !== ADMIN_ROLE}
+							{/if}
+							<Cell last class="w-10">
+								{#if info.editable}
+									{#snippet revokeButton()}
 										<Button
 											unifiedSize="xs"
 											variant="subtle"
 											iconOnly
 											startIcon={{ icon: Trash2 }}
-											title="Revoke {revocable.join(', ')}"
-											disabled={planning || applying}
-											onClick={() =>
+											title={unrevocable ?? `Revoke ${revocable.join(', ')}`}
+											disabled={controlsDisabled || !!unrevocable}
+											onClick={() => {
+												if (!revokeScope) return
 												confirm(
 													{
 														type: 'revoke',
@@ -246,15 +369,52 @@
 														objects: grant.objects
 													},
 													`Revoked ${revocable.join(', ')} from ${grant.grantee}`
-												)}
+												)
+											}}
 										/>
+									{/snippet}
+									{#if unrevocable && !disabledReason}
+										<Tooltip>
+											<div class="pointer-events-none">{@render revokeButton()}</div>
+											{#snippet text()}{unrevocable}{/snippet}
+										</Tooltip>
+									{:else}
+										{@render withDisabledReason(revokeButton, 'w-fit')}
 									{/if}
-								</Cell>
-							</Row>
-						{/each}
-					</tbody>
-				</DataTable>
-			{/if}
+								{/if}
+							</Cell>
+						</Row>
+					{:else}
+						<Row>
+							<Cell first last colspan={columns} class="text-secondary">
+								{grantRows.length === 0 ? 'No grants yet.' : 'No grant matches the filters.'}
+							</Cell>
+						</Row>
+					{/each}
+					{#if info.editable}
+						<!-- The grant to create: a row of the table, set apart until it exists. -->
+						<Row class="bg-surface-secondary">
+							<Cell first last colspan={columns}>
+								{#snippet grantBuilder()}
+									<PgGrantBuilder
+										{target}
+										roles={info.roles}
+										disabled={controlsDisabled}
+										manageRoles={manageRoles ? manageRolesEntry : undefined}
+										supportsMaintain={info.supports_maintain}
+										onAdd={({ role, privileges, scope }) =>
+											confirm(
+												{ type: 'grant', role, privileges, scope },
+												`Granted ${privileges.join(', ')} to ${role}`
+											)}
+									/>
+								{/snippet}
+								{@render withDisabledReason(grantBuilder, 'w-full')}
+							</Cell>
+						</Row>
+					{/if}
+				</tbody>
+			</DataTable>
 		</section>
 	</div>
 {/if}

frontend/src/lib/components/datatableAcl/PgGrantBuilder.svelte

@@ -1,17 +1,18 @@
 <script lang="ts">
 	import type { AclTarget } from '$lib/gen'
+	import type { Snippet } from 'svelte'
 	import { Plus } from 'lucide-svelte'
 	import { Button } from '../common'
 	import Select from '../select/Select.svelte'
 	import MultiSelect from '../select/MultiSelect.svelte'
-	import { privilegesOf, scopeSql, scopesOf, type AclScope } from './aclScopes'
+	import { privilegesOf, scopesOf, type AclScope } from './aclScopes'
 
 	let {
 		target,
 		roles,
 		supportsMaintain = false,
-		dbname,
 		disabled = false,
+		manageRoles,
 		onAdd
 	}: {
 		target: AclTarget
@@ -19,9 +20,9 @@
 		roles: string[]
 		/** Postgres 17+, which has one more table privilege to offer. */
 		supportsMaintain?: boolean
-		/** Names the database in the statement a database target builds. */
-		dbname?: string
 		disabled?: boolean
+		/** Rendered at the bottom of the role picker, to reach the data table's roles. */
+		manageRoles?: Snippet<[{ close: () => void }]>
 		onAdd: (grant: { role: string; privileges: string[]; scope: AclScope }) => void
 	} = $props()
 
@@ -29,26 +30,24 @@
 	let scope = $state<AclScope>('target')
 	let privileges = $state<string[]>([])
 
+	const scopes = $derived(scopesOf(target.kind))
 	const available = $derived(privilegesOf(scope, target.kind, supportsMaintain))
-	const statement = $derived(
-		privileges.length && role
-			? `GRANT ${privileges.join(', ')} ON ${scopeSql(scope, target, dbname)} TO ${role}`
-			: undefined
-	)
 </script>
 
-<div class="flex flex-col gap-2 border rounded-md p-3">
-	<div class="flex flex-wrap items-center gap-2 text-xs text-secondary">
-		<span class="font-mono text-primary">GRANT</span>
-		<MultiSelect
-			bind:value={privileges}
-			items={available.map((p) => ({ value: p, label: p }))}
-			placeholder="privileges"
-			{disabled}
-			size="sm"
-			class="min-w-48"
-		/>
-		<span class="font-mono text-primary">ON</span>
+<!-- Laid out like the grants above it, so it reads as the statement it will run. -->
+<div class="flex items-center gap-2 text-xs text-secondary">
+	<span class="font-mono shrink-0">GRANT</span>
+	<MultiSelect
+		bind:value={privileges}
+		items={available.map((p) => ({ value: p, label: p }))}
+		placeholder="privileges"
+		{disabled}
+		size="md"
+		class="min-w-0 flex-[4]"
+	/>
+	<!-- A database or a table has only itself to grant on. -->
+	{#if scopes.length > 1}
+		<span class="font-mono shrink-0">ON</span>
 		<Select
 			bind:value={
 				() => scope,
@@ -61,35 +60,34 @@
 					privileges = privileges.filter((p) => allowed.includes(p))
 				}
 			}
-			items={scopesOf(target.kind)}
+			items={scopes}
 			{disabled}
-			size="sm"
-			class="w-52"
+			size="md"
+			class="min-w-0 flex-[3]"
 		/>
-		<span class="font-mono text-primary">TO</span>
-		<Select
-			bind:value={role}
-			items={roles.map((r) => ({ value: r, label: r }))}
-			placeholder="role"
-			{disabled}
-			size="sm"
-			class="w-40"
-		/>
-		<Button
-			unifiedSize="sm"
-			variant="default"
-			startIcon={{ icon: Plus }}
-			disabled={disabled || !role || privileges.length === 0}
-			onClick={() => {
-				if (!role) return
-				onAdd({ role, privileges: [...privileges], scope })
-				privileges = []
-			}}
-		>
-			Add
-		</Button>
-	</div>
-	{#if statement}
-		<pre class="text-2xs text-tertiary overflow-x-auto">{statement}</pre>
 	{/if}
+	<span class="font-mono shrink-0">TO</span>
+	<Select
+		bind:value={role}
+		items={roles.map((r) => ({ value: r, label: r }))}
+		placeholder="role"
+		{disabled}
+		size="md"
+		class="min-w-0 flex-[2]"
+		bottomSnippet={manageRoles}
+	/>
+	<Button
+		unifiedSize="md"
+		variant="default"
+		wrapperClasses="shrink-0"
+		startIcon={{ icon: Plus }}
+		disabled={disabled || !role || privileges.length === 0}
+		onClick={() => {
+			if (!role) return
+			onAdd({ role, privileges: [...privileges], scope })
+			privileges = []
+		}}
+	>
+		Create
+	</Button>
 </div>

frontend/src/lib/components/datatableAcl/aclScopes.test.ts

@@ -3,10 +3,12 @@ import type { AclGrant } from '$lib/gen'
 import {
 	blockingSources,
 	grantKey,
+	grantCoverage,
 	groupGrants,
 	revocablePrivileges,
 	revokeScopeOf,
-	uncoveredCreators
+	uncoveredCreators,
+	type GroupedGrant
 } from './aclScopes'
 
 const table = (name: string) => ({ name, kind: 'TABLE' })
@@ -176,3 +178,29 @@ describe('uncoveredCreators', () => {
 		).toEqual([])
 	})
 })
+
+describe('grantCoverage', () => {
+	const schema = { kind: 'schema' as const, schema: 'sales' }
+	const row = (grant: Partial<GroupedGrant>): GroupedGrant => ({
+		grantee: 'analytics',
+		privileges: ['SELECT'],
+		objects: [],
+		sources: [],
+		...grant
+	})
+
+	it('says nothing for the object itself, and names what is created later', () => {
+		expect(grantCoverage(row({}), schema)).toBeUndefined()
+		expect(grantCoverage(row({ future: 'TABLES' }), schema)).toBe('tables created later')
+		expect(grantCoverage(row({ future: 'TABLES' }), { kind: 'database' })).toBe(
+			'tables created later, in every schema'
+		)
+	})
+
+	it('names a routine with its arguments, and summarizes past three objects', () => {
+		const routine = row({ objects: [{ name: 'refresh', kind: 'FUNCTION', args: 'integer' }] })
+		expect(grantCoverage(routine, schema)).toBe('function refresh(integer)')
+		const many = row({ objects: ['a', 'b', 'c', 'd', 'e'].map(table) })
+		expect(grantCoverage(many, schema)).toBe('tables a, b, c and 2 more')
+	})
+})

frontend/src/lib/components/datatableAcl/aclScopes.ts

@@ -73,28 +73,6 @@ export function privilegesOf(
 	}
 }
 
-/** What a statement built at this scope reads as, for the builder's own preview. */
-export function scopeSql(scope: AclScope, target: AclTarget, dbname?: string): string {
-	if (target.kind === 'database') return `DATABASE ${dbname ?? ''}`.trim()
-	const schema = target.schema
-	switch (scope) {
-		case 'target':
-			return target.kind === 'schema' ? `SCHEMA ${schema}` : `TABLE ${schema}.${target.table}`
-		case 'all_tables':
-			return `ALL TABLES IN SCHEMA ${schema}`
-		case 'all_sequences':
-			return `ALL SEQUENCES IN SCHEMA ${schema}`
-		case 'all_functions':
-			return `ALL FUNCTIONS IN SCHEMA ${schema}`
-		case 'future_tables':
-			return `TABLES (default privileges in ${schema})`
-		case 'future_sequences':
-			return `SEQUENCES (default privileges in ${schema})`
-		case 'future_functions':
-			return `FUNCTIONS (default privileges in ${schema})`
-	}
-}
-
 /** One row of the grants table: the same privileges on several objects read as one line, since
  * granting them per object is what `ON ALL TABLES` does. */
 export type GroupedGrant = {
@@ -195,17 +173,21 @@ export function revocablePrivileges(grant: GroupedGrant, target: AclTarget): str
 	return grant.privileges
 }
 
-/** How a row reads back: what it covers, in one phrase. */
-export function grantScopeLabel(grant: GroupedGrant): string {
-	if (grant.future) return `${grant.future.toLowerCase()} created later`
-	if (grant.objects.length === 1) {
-		const object = grant.objects[0]
-		// A routine's arguments are part of what it is, so two of the same name would otherwise
-		// read as one row twice.
-		const args = object.args !== undefined ? `(${object.args})` : ''
-		return `${object.kind.toLowerCase()} ${object.name}${args}`
+/** How many objects a folded row names before it summarizes the rest. */
+const LISTED_OBJECTS = 3
+
+/** What a row covers, when that is not the editor's own object: the objects inside it, or what is
+ * created later. Undefined for a grant on the object itself, which needs no saying. */
+export function grantCoverage(grant: GroupedGrant, target: AclTarget): string | undefined {
+	if (grant.future) {
+		const created = `${grant.future.toLowerCase()} created later`
+		return target.kind === 'database' ? `${created}, in every schema` : created
 	}
-	if (grant.objects.length > 1)
-		return `${grant.objects.length} ${grant.objects[0].kind.toLowerCase()}s`
-	return 'itself'
+	if (grant.objects.length === 0) return undefined
+	// A routine's arguments are part of what it is: two of the same name are different objects.
+	const names = grant.objects.map((o) => (o.args !== undefined ? `${o.name}(${o.args})` : o.name))
+	const listed = names.slice(0, LISTED_OBJECTS).join(', ')
+	const rest = names.length - LISTED_OBJECTS
+	const kind = grant.objects[0].kind.toLowerCase()
+	return `${names.length > 1 ? `${kind}s` : kind} ${listed}${rest > 0 ? ` and ${rest} more` : ''}`
 }

frontend/src/lib/components/workspaceSettings/DataTablePermissionsButton.svelte

@@ -10,7 +10,7 @@
 	import DataTable from '../table/DataTable.svelte'
 	import Head from '../table/Head.svelte'
 	import Row from '../table/Row.svelte'
-	import { KeyRound } from 'lucide-svelte'
+	import { CircleHelp, KeyRound } from 'lucide-svelte'
 	import {
 		FolderService,
 		GroupService,
@@ -25,7 +25,13 @@
 	import { deepEqual } from 'fast-equals'
 	import { ADMIN_DATATABLE_ROLE, isDatatableRoleName } from '../dbTypes'
 	import PgAclEditor from '../datatableAcl/PgAclEditor.svelte'
-	import InstanceRolesButton from './InstanceRolesButton.svelte'
+	import ConfirmationModal from '../common/confirmationModal/ConfirmationModal.svelte'
+	import { createAsyncConfirmationModal } from '../common/confirmationModal/asyncConfirmationModal.svelte'
+	import {
+		pgInstanceName as pgInstanceNameOf,
+		handleRoleCreationError,
+		offerUnusedRoleDrop
+	} from './datatableRoleModals'
 
 	let {
 		workspace,
@@ -44,10 +50,8 @@
 	} = $props()
 
 	// `id` is the instance role's catalog id (or the reserved `admin`), which is what the tenant
-	// lists are keyed by — so renaming a role instance-side moves nothing here. A row without an id
-	// names a role the instance does not define yet: it cannot be saved until a superadmin creates
-	// it, and takes the new role's id once they have.
-	type EditedRole = { id: string | undefined; name: string | undefined; tenants: string[] }
+	// lists are keyed by — so renaming a role instance-side moves nothing here.
+	type EditedRole = { id: string; name: string | undefined; tenants: string[] }
 	type Edited = { permissioned: boolean; roles: EditedRole[]; defaultRoleId: string }
 
 	let drawerOpen = $state(false)
@@ -72,7 +76,7 @@
 
 	const editable = $derived(!!info?.editable)
 	const governing = $derived(info?.governing_workspace_id)
-	// The instance catalog, read again after the instance roles drawer changes it.
+	// The instance catalog, with the roles created from this drawer since it was loaded.
 	let catalog = $state<InstanceDatatableRole[] | undefined>(undefined)
 	const availableRoles: InstanceDatatableRole[] = $derived(catalog ?? info?.available_roles ?? [])
 	// A role is a login on one cluster, and this drawer's is the data table's: name it, or the
@@ -80,11 +84,10 @@
 	const clusterName = $derived(
 		info?.cluster === 'external_instance' ? 'the external cluster' : "Windmill's database"
 	)
+	const pgInstanceName = $derived(pgInstanceNameOf(info?.cluster))
 	const unusedRoles = $derived(availableRoles.filter((r) => !roles.some((row) => row.id === r.id)))
-	const pendingRoles = $derived(roles.filter((r) => r.id === undefined))
-	let instanceRoles: InstanceRolesButton | undefined = $state(undefined)
-
-	const roleKey = (role: EditedRole) => role.id ?? `pending:${role.name}`
+	const confirmationModal = createAsyncConfirmationModal()
+	let aclEditor = $state<PgAclEditor | undefined>(undefined)
 
 	const hasUnsavedChanges = $derived(
 		!deepEqual($state.snapshot(saved), {
@@ -152,9 +155,9 @@
 		roles.push({ id: role.id, name: role.name, tenants: [] })
 	}
 
-	/** Adds a role by name: the instance's role of that name, or a pending row for one it does not
-	 * define yet. */
-	function addRoleByName(typed: string) {
+	/** Adds the instance's role of that name. One the instance does not define is created first,
+	 * which only a superadmin may do. */
+	async function addRoleByName(typed: string) {
 		const name = typed.trim()
 		if (!isDatatableRoleName(name) || name.toLowerCase() === ADMIN_DATATABLE_ROLE) {
 			sendUserToast(
@@ -165,35 +168,70 @@
 		}
 		if (roles.some((r) => r.name === name)) return
 		const existing = availableRoles.find((r) => r.name === name)
-		roles.push({ id: existing?.id, name, tenants: [] })
+		if (existing) {
+			roles.push({ id: existing.id, name, tenants: [] })
+			return
+		}
+		if (!$superadmin) {
+			await confirmationModal.ask({
+				title: `Role ${name} does not exist`,
+				children: `This role doesn't exist in ${pgInstanceName}. Ask a superadmin to create it.`,
+				confirmationText: 'OK',
+				type: 'info',
+				hideCancel: true
+			})
+			return
+		}
+		let created: InstanceDatatableRole | undefined
+		let failure: unknown
+		await confirmationModal.ask({
+			title: `Role ${name} does not exist`,
+			children: `(Superadmin) Create this role in ${pgInstanceName}?`,
+			confirmationText: 'Create role',
+			type: 'info',
+			onConfirmed: async () => {
+				try {
+					created = await SettingService.createInstanceDatatableRole({
+						requestBody: { name, cluster: info?.cluster }
+					})
+				} catch (e) {
+					failure = e
+				}
+			}
+		})
+		if (created) addCreatedRole(created)
+		// Handled once this modal has closed: the take-over offer opens in the same one.
+		if (failure) {
+			await handleRoleCreationError(failure, {
+				name,
+				cluster: info?.cluster,
+				confirmationModal,
+				onTakenOver: addCreatedRole
+			})
+		}
+	}
+
+	function addCreatedRole(created: InstanceDatatableRole) {
+		catalog = [...availableRoles, created]
+		if (!roles.some((r) => r.id === created.id)) {
+			roles.push({ id: created.id, name: created.name, tenants: [] })
+		}
 	}
 
 	function removeRole(role: EditedRole) {
-		const key = roleKey(role)
-		roles = roles.filter((r) => roleKey(r) !== key)
-		if (role.id !== undefined && defaultRoleId === role.id) defaultRoleId = ADMIN_DATATABLE_ROLE
-	}
-
-	/** Reads the instance catalog again and gives each pending row the id of the role now defined
-	 * under its name. */
-	async function refreshCatalog() {
-		let fresh: InstanceDatatableRole[]
-		try {
-			fresh = await SettingService.listInstanceDatatableRoles({ cluster: info?.cluster })
-		} catch (e) {
-			sendUserToast(e?.body ?? e?.message ?? String(e), true)
-			return
-		}
-		catalog = fresh
-		for (const row of roles) {
-			if (row.id !== undefined) continue
-			const created = fresh.find((r) => r.name === row.name)
-			if (created) row.id = created.id
-		}
+		roles = roles.filter((r) => r.id !== role.id)
+		if (defaultRoleId === role.id) defaultRoleId = ADMIN_DATATABLE_ROLE
 	}
 
 	async function save() {
 		saving = true
+		// Turning roles off clears the whole block, so every role it named is removed.
+		const kept = permissioned ? roles.map((r) => r.id) : []
+		const removedIds = saved.permissioned
+			? saved.roles
+					.map((r) => r.id)
+					.filter((id) => id !== ADMIN_DATATABLE_ROLE && !kept.includes(id))
+			: []
 		try {
 			const msg = await WorkspaceService.setDatatablePermissions({
 				workspace,
@@ -201,11 +239,27 @@
 				requestBody: {
 					permissioned,
 					default_role: defaultRoleId,
-					roles: roles.map((r) => ({ id: r.id!, tenants: $state.snapshot(r.tenants) }))
+					roles: roles.map((r) => ({ id: r.id, tenants: $state.snapshot(r.tenants) }))
 				}
 			})
 			sendUserToast(msg)
 			await load()
+			// A role is a login of the whole Postgres instance: whether this was the last data table
+			// naming it is only known once the save has landed.
+			const unused = availableRoles.filter((r) => removedIds.includes(r.id) && r.in_use === false)
+			let dropped = false
+			for (const role of unused) {
+				dropped =
+					(await offerUnusedRoleDrop({
+						role,
+						cluster: info?.cluster,
+						superadmin: !!$superadmin,
+						confirmationModal
+					})) || dropped
+			}
+			if (dropped) await load()
+			// The grants list the roles they can name, which the save just changed.
+			aclEditor?.refresh()
 			onSaved?.()
 		} catch (e) {
 			sendUserToast(e?.body ?? e?.message ?? String(e), true)
@@ -214,6 +268,17 @@
 		}
 	}
 
+	/** Whether the drawer may close: unsaved roles are only lost once the user says so. */
+	async function confirmDiscard(): Promise<boolean> {
+		if (!hasUnsavedChanges || !drawerOpen) return true
+		return await confirmationModal.ask({
+			title: 'Discard unsaved changes?',
+			children: `The roles of ${datatable} have changes that are not saved. Closing the drawer discards them.`,
+			confirmationText: 'Discard changes',
+			type: 'danger'
+		})
+	}
+
 	export function open() {
 		drawerOpen = true
 		load()
@@ -232,10 +297,12 @@
 	/>
 {/if}
 
-<Drawer bind:open={drawerOpen} size="900px">
+<Drawer bind:open={drawerOpen} size="900px" confirmClose={confirmDiscard}>
 	<DrawerContent
-		title="Roles — {datatable}"
-		on:close={() => (drawerOpen = false)}
+		title="Roles: {datatable}"
+		on:close={async () => {
+			if (await confirmDiscard()) drawerOpen = false
+		}}
 		tooltip="A data table role is a Postgres login. A job that names one connects as it, and Postgres decides what it may touch — grant it privileges under Access. Roles are defined for the whole instance; here you say who may use each one on this data table."
 	>
 		{#snippet titleExtra()}
@@ -293,10 +360,13 @@
 				{/if}
 
 				{#if permissioned}
-					{#if editable && availableRoles.length === 0}
-						<Alert type="warning" title="No role defined on this instance" size="xs">
-							Only <span class="font-mono">admin</span> can be used until a superadmin creates a data
-							table role. Type a name below to add one.
+					{#if editable && !$superadmin}
+						<Alert type="info" title="Only superadmins can create roles" size="xs">
+							A data table role is a Postgres login on {pgInstanceName}, so only a superadmin can
+							create one. Ask a superadmin for any role you need that is not listed.
+							{#if availableRoles.length === 0}
+								Until then, only <span class="font-mono">admin</span> can be used.
+							{/if}
 						</Alert>
 					{/if}
 
@@ -329,7 +399,7 @@
 							</tr>
 						</Head>
 						<tbody class="divide-y bg-surface-tertiary">
-							{#each roles as role (roleKey(role))}
+							{#each roles as role (role.id)}
 								{@const isAdmin = role.id === ADMIN_DATATABLE_ROLE}
 								<Row>
 									<Cell first class="w-56 align-top">
@@ -339,23 +409,6 @@
 												<span class="text-2xs text-secondary italic">
 													no longer defined on {clusterName}
 												</span>
-											{:else if role.id === undefined}
-												<Alert type="warning" title="This role does not exist yet" size="xs">
-													{#if $superadmin}
-														<div class="flex flex-col items-start gap-1">
-															<span>Create it on {clusterName} to use it here.</span>
-															<Button
-																unifiedSize="xs"
-																variant="default"
-																on:click={() => instanceRoles?.open(role.name)}
-															>
-																Create it
-															</Button>
-														</div>
-													{:else}
-														Only a superadmin can create it on {clusterName}.
-													{/if}
-												</Alert>
 											{/if}
 										</div>
 									</Cell>
@@ -371,11 +424,11 @@
 									<Cell class="w-20 align-top">
 										<div class="flex justify-center pt-2">
 											<Checkbox
-												checked={role.id !== undefined && defaultRoleId === role.id}
-												disabled={!editable || role.id === undefined}
+												checked={defaultRoleId === role.id}
+												disabled={!editable}
 												title="Use this role when a job names none"
 												onChange={() => {
-													if (role.id !== undefined) defaultRoleId = role.id
+													defaultRoleId = role.id
 												}}
 											/>
 										</div>
@@ -388,7 +441,7 @@
 								</Row>
 							{/each}
 							{#if editable}
-								<Row class="!border-0">
+								<Row>
 									<Cell colspan={4} class="pt-2 pb-2">
 										<div class="flex justify-center">
 											<Select
@@ -418,9 +471,32 @@
 
 			<!-- Grants only matter to a data table under roles: without them every job connects as the
 			default login, whatever is granted here. -->
-			{#if info?.supported && permissioned && !hasUnsavedChanges}
+			{#if info?.supported && permissioned}
 				<div class="mt-6 pt-6 border-t">
-					<PgAclEditor {workspace} {datatable} target={{ kind: 'database' }} />
+					<PgAclEditor
+						bind:this={aclEditor}
+						{workspace}
+						{datatable}
+						target={{ kind: 'database' }}
+						disabledReason={hasUnsavedChanges ? 'Save the new roles to continue' : undefined}
+					/>
+					<div class="mt-3">
+						<Button
+							unifiedSize="xs"
+							variant="subtle"
+							startIcon={{ icon: CircleHelp }}
+							on:click={() =>
+								confirmationModal.ask({
+									title: 'Permissions on schemas and tables',
+									children: `<p>In the database manager, click the <b>⋮</b> menu of a schema or a table, then <b>Access</b>, to add grants on it or change its owner.</p>`,
+									confirmationText: 'OK',
+									type: 'info',
+									hideCancel: true
+								})}
+						>
+							How to use permissions on schemas and tables?
+						</Button>
+					</div>
 				</div>
 			{/if}
 		{/if}
@@ -430,10 +506,7 @@
 				<Button
 					variant="accent"
 					unifiedSize="md"
-					disabled={!hasUnsavedChanges || loading || !!loadError || pendingRoles.length > 0}
-					title={pendingRoles.length > 0
-						? 'Create the roles that do not exist yet, or remove them'
-						: undefined}
+					disabled={!hasUnsavedChanges || loading || !!loadError}
 					loading={saving}
 					on:click={save}
 				>
@@ -444,11 +517,4 @@
 	</DrawerContent>
 </Drawer>
 
-{#if $superadmin}
-	<InstanceRolesButton
-		bind:this={instanceRoles}
-		hideTrigger
-		cluster={info?.cluster}
-		onChanged={refreshCatalog}
-	/>
-{/if}
+<ConfirmationModal {...confirmationModal.props} />

frontend/src/lib/components/workspaceSettings/DataTableRolesSection.svelte

@@ -15,6 +15,7 @@
 	import { SettingService, type DatatableRoleCluster, type InstanceDatatableRole } from '$lib/gen'
 	import { sendUserToast } from '$lib/toast'
 	import { isCloudHosted } from '$lib/cloud'
+	import { handleRoleCreationError } from './datatableRoleModals'
 
 	let {
 		initialName = '',
@@ -134,14 +135,28 @@
 	async function create() {
 		const name = newName.trim()
 		if (!name) return
-		await run(
-			() =>
-				SettingService.createInstanceDatatableRole({
-					requestBody: { name, cluster }
-				}),
-			`Created the data table role ${name}`
-		)
-		newName = ''
+		busy = true
+		try {
+			await SettingService.createInstanceDatatableRole({ requestBody: { name, cluster } })
+			sendUserToast(`Created the data table role ${name}`)
+			newName = ''
+		} catch (e) {
+			const createdOn = cluster
+			handleRoleCreationError(e, {
+				name,
+				cluster: createdOn,
+				confirmationModal,
+				onTakenOver: async () => {
+					newName = ''
+					if (cluster === createdOn) await load()
+					onChanged?.()
+				}
+			})
+		} finally {
+			await load()
+			busy = false
+			onChanged?.()
+		}
 	}
 
 	async function remove(role: InstanceDatatableRole) {

frontend/src/lib/components/workspaceSettings/datatableRoleModals.ts

@@ -0,0 +1,120 @@
+import { SettingService, type DatatableRoleCluster, type InstanceDatatableRole } from '$lib/gen'
+import { sendUserToast } from '$lib/toast'
+import type { ConfirmationModalHandle } from '../common/confirmationModal/asyncConfirmationModal.svelte'
+
+export function pgInstanceName(cluster: DatatableRoleCluster | undefined): string {
+	return cluster === 'external_instance'
+		? 'the external Postgres instance'
+		: 'the Windmill Postgres instance'
+}
+
+function escapeHtml(text: string): string {
+	return text.replace(
+		/[&<>"']/g,
+		(c) => ({ '&': '&amp;', '<': '&lt;', '>': '&gt;', '"': '&quot;', "'": '&#39;' })[c]!
+	)
+}
+
+/** Reports a failed role creation. One refused because Postgres already has a role of that name
+ * (a 409) opens the offer to take that role over instead. `confirmationModal` must be closed: the
+ * offer takes it over. */
+export async function handleRoleCreationError(
+	e: any,
+	opts: {
+		name: string
+		cluster: DatatableRoleCluster | undefined
+		confirmationModal: ConfirmationModalHandle
+		onTakenOver: (role: InstanceDatatableRole) => void
+	}
+) {
+	if (e?.status !== 409) {
+		sendUserToast(e?.body ?? e?.message ?? String(e), true)
+		return
+	}
+	await offerTakeover(opts)
+}
+
+/** A role no data table names any more is still a login on its Postgres instance. A superadmin is
+ * offered to drop it; anyone else is told to ask one. Resolves to whether it was dropped. */
+export async function offerUnusedRoleDrop({
+	role,
+	cluster,
+	superadmin,
+	confirmationModal
+}: {
+	role: InstanceDatatableRole
+	cluster: DatatableRoleCluster | undefined
+	superadmin: boolean
+	confirmationModal: ConfirmationModalHandle
+}): Promise<boolean> {
+	const name = `<span class="font-mono">${escapeHtml(role.name)}</span>`
+	const instance = pgInstanceName(cluster)
+	if (!superadmin) {
+		await confirmationModal.ask({
+			title: `Role ${role.name} is no longer used`,
+			children: `<p>No data table uses the role ${name} any more, but its login still exists in ${instance}. Ask a superadmin to drop it there.</p>`,
+			confirmationText: 'OK',
+			type: 'info',
+			hideCancel: true
+		})
+		return false
+	}
+	let dropped = false
+	await confirmationModal.ask({
+		title: `Drop the role ${role.name}?`,
+		children: `<div class="flex flex-col gap-2">
+			<p>No data table uses the role ${name} any more, but its login still exists in ${instance}. Windmill can drop it.</p>
+			<p>Dropping it <b>deletes the login ${name}</b> from ${instance}: anything still connecting as it, such as a script with <span class="font-mono">-- role ${escapeHtml(role.name)}</span>, fails from then on. Everything it owns in the databases Windmill manages there is handed back to admin, and its grants are revoked. This cannot be undone.</p>
+			<p>Keep it if you plan to give it to a data table again.</p>
+		</div>`,
+		confirmationText: 'Drop role',
+		type: 'danger',
+		onConfirmed: async () => {
+			try {
+				await SettingService.deleteInstanceDatatableRole({ id: role.id })
+				dropped = true
+				sendUserToast(`Dropped the role ${role.name}`)
+			} catch (e) {
+				sendUserToast(e?.body ?? e?.message ?? String(e), true)
+			}
+		}
+	})
+	return dropped
+}
+
+async function offerTakeover({
+	name,
+	cluster,
+	confirmationModal,
+	onTakenOver
+}: {
+	name: string
+	cluster: DatatableRoleCluster | undefined
+	confirmationModal: ConfirmationModalHandle
+	onTakenOver: (role: InstanceDatatableRole) => void
+}) {
+	const role = `<span class="font-mono">${escapeHtml(name)}</span>`
+	let taken: InstanceDatatableRole | undefined
+	await confirmationModal.ask({
+		title: `Let Windmill take over the role ${name}?`,
+		children: `<div class="flex flex-col gap-2">
+			<p>A Postgres role named ${role} already exists in ${pgInstanceName(cluster)}, and Windmill did not create it. A data table role logs in with a password only Windmill knows, so Windmill has to manage it to use it.</p>
+			<p>Taking it over <b>replaces the password of ${role}</b> with a random one and lets it log in. Anything that logs in as ${role} today with its current password, such as another application, a person or another Windmill instance, will be locked out. The old password cannot be restored.</p>
+			<p>From then on Windmill manages ${role} like a role it created: deleting it in Windmill drops it from Postgres. A role that is a superuser, can create roles or databases, is a member of other roles or has members of its own is refused.</p>
+		</div>`,
+		confirmationText: 'Replace password and take over',
+		type: 'danger',
+		onConfirmed: async () => {
+			try {
+				taken = await SettingService.createInstanceDatatableRole({
+					requestBody: { name, cluster, take_over: true }
+				})
+			} catch (e) {
+				sendUserToast(e?.body ?? e?.message ?? String(e), true)
+			}
+		}
+	})
+	if (!taken) return
+	sendUserToast(`Windmill now manages the role ${name}`)
+	onTakenOver(taken)
+}