From f41e81e0e94071b3a3ef636d0af8b0a6295ebc09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Burak=20Emre=20Kabakc=C4=B1?= Date: Fri, 15 May 2026 19:14:59 +0100 Subject: [PATCH 1/6] feat(auth-profiles): admin-pinned default app profile per connector Adds is_default_for_connector to auth_profiles with a partial unique index keeping at most one default per (org, connector_key) for oauth_app rows. getPrimaryAuthProfileForKind prefers the flagged profile so member-installed connections fall through to the admin's choice instead of "most recent". - New `manage_auth_profiles.set_default_auth_profile` action (admin-only via the existing OWNER_ADMIN_ACTIONS policy table). - Revoke / force-delete on oauth_app now flips dependent connections to pending_auth so admin app rotation doesn't silently break running connections. - Revoking the currently-pinned default also clears the flag. - Bumps packages/web pointer to the new admin settings page + install sheet UX (owletto-web#134). --- ...00_auth_profiles_default_for_connector.sql | 23 ++++ db/schema.sql | 8 ++ packages/server/src/auth/tool-access.ts | 1 + .../tools/admin/helpers/connection-helpers.ts | 1 + .../src/tools/admin/manage_auth_profiles.ts | 112 +++++++++++++++++- packages/server/src/utils/auth-profiles.ts | 50 +++++++- packages/web | 2 +- 7 files changed, 194 insertions(+), 3 deletions(-) create mode 100644 db/migrations/20260515170000_auth_profiles_default_for_connector.sql diff --git a/db/migrations/20260515170000_auth_profiles_default_for_connector.sql b/db/migrations/20260515170000_auth_profiles_default_for_connector.sql new file mode 100644 index 000000000..ae1241e77 --- /dev/null +++ b/db/migrations/20260515170000_auth_profiles_default_for_connector.sql @@ -0,0 +1,23 @@ +-- migrate:up +-- Admin-managed default app profile per (org, connector_key). +-- Today getPrimaryAuthProfileForKind picks the most-recently-updated active +-- oauth_app profile for the connector — admins have no way to designate +-- which profile members should fall through to. The flag lets the admin +-- pin a chosen profile; the resolver prefers flagged rows first. +-- +-- Constrained to oauth_app for now since that's the only kind where +-- "default for connector" is meaningful (env / interactive / browser_session +-- are picked by other rules — device binding, capture mode, etc.). + +ALTER TABLE auth_profiles + ADD COLUMN is_default_for_connector boolean NOT NULL DEFAULT false; + +CREATE UNIQUE INDEX auth_profiles_default_for_connector_unique + ON auth_profiles (organization_id, connector_key) + WHERE is_default_for_connector AND profile_kind = 'oauth_app'; + +-- migrate:down +DROP INDEX IF EXISTS auth_profiles_default_for_connector_unique; + +ALTER TABLE auth_profiles + DROP COLUMN IF EXISTS is_default_for_connector; diff --git a/db/schema.sql b/db/schema.sql index 8ecda841b..ac3946456 100644 --- a/db/schema.sql +++ b/db/schema.sql @@ -255,6 +255,7 @@ CREATE TABLE public.auth_profiles ( browser_kind text, user_data_dir text, cdp_url text, + is_default_for_connector boolean DEFAULT false NOT NULL, CONSTRAINT auth_profiles_browser_kind_check CHECK (((browser_kind IS NULL) OR (browser_kind = ANY (ARRAY['chrome'::text, 'brave'::text, 'arc'::text, 'edge'::text])))), CONSTRAINT auth_profiles_connector_key_required CHECK (((connector_key IS NOT NULL) OR (profile_kind = 'browser_session'::text))), CONSTRAINT auth_profiles_device_browser_path_mutex CHECK (((device_worker_id IS NULL) OR (profile_kind <> 'browser_session'::text) OR (user_data_dir IS NULL) OR (cdp_url IS NULL))), @@ -2874,6 +2875,12 @@ CREATE UNIQUE INDEX auth_profiles_org_slug_unique ON public.auth_profiles USING CREATE UNIQUE INDEX auth_profiles_pending_unique ON public.auth_profiles USING btree (organization_id, connector_key, profile_kind, provider) WHERE (status = 'pending_auth'::text); +-- +-- Name: auth_profiles_default_for_connector_unique; Type: INDEX; Schema: public; Owner: - +-- + +CREATE UNIQUE INDEX auth_profiles_default_for_connector_unique ON public.auth_profiles USING btree (organization_id, connector_key) WHERE (is_default_for_connector AND (profile_kind = 'oauth_app'::text)); + -- -- Name: chat_user_identities_lobu_user_idx; Type: INDEX; Schema: public; Owner: - -- @@ -4999,6 +5006,7 @@ INSERT INTO public.schema_migrations (version) VALUES ('20260515120000'), ('20260515150000'), ('20260515160000'), + ('20260515170000'), ('20260516120000'), ('20260516200000'), ('20260516200100'); diff --git a/packages/server/src/auth/tool-access.ts b/packages/server/src/auth/tool-access.ts index 445f6004e..3ea851d6c 100644 --- a/packages/server/src/auth/tool-access.ts +++ b/packages/server/src/auth/tool-access.ts @@ -49,6 +49,7 @@ const OWNER_ADMIN_ACTIONS: Record> = { 'create_auth_profile', 'update_auth_profile', 'delete_auth_profile', + 'set_default_auth_profile', ]), manage_operations: new Set(['execute', 'approve', 'reject']), manage_watchers: new Set([ diff --git a/packages/server/src/tools/admin/helpers/connection-helpers.ts b/packages/server/src/tools/admin/helpers/connection-helpers.ts index 401db3301..8c4cc6119 100644 --- a/packages/server/src/tools/admin/helpers/connection-helpers.ts +++ b/packages/server/src/tools/admin/helpers/connection-helpers.ts @@ -512,6 +512,7 @@ export function serializeAuthProfile(authProfile: AuthProfileRow): Record; @@ -212,6 +227,11 @@ export async function manageAuthProfiles( args as Extract, ctx ), + set_default_auth_profile: () => + handleSetDefaultAuthProfile( + args as Extract, + ctx + ), }); } @@ -372,6 +392,30 @@ async function handleTestAuthProfile( }; } +/** + * When an oauth_app profile is revoked/errored, any connection that mints + * tokens against it can no longer authenticate. Flip those connections to + * pending_auth so the UI surfaces the breakage; the admin re-pins or rotates + * the app, then operators re-authorize the connection. + */ +async function syncConnectionsForOAuthAppProfile( + organizationId: string, + authProfileId: number, + active: boolean +): Promise { + const sql = getDb(); + const nextConnectionStatus = active ? 'active' : 'pending_auth'; + + await sql` + UPDATE connections + SET status = ${nextConnectionStatus}, + updated_at = NOW() + WHERE organization_id = ${organizationId} + AND app_auth_profile_id = ${authProfileId} + AND deleted_at IS NULL + `; +} + async function syncConnectionsForBrowserAuthProfile( organizationId: string, authProfileId: number, @@ -728,6 +772,27 @@ async function handleUpdateAuthProfile( ); } + // Cascade for oauth_app: admins flipping an app profile to revoked/error + // need dependent connections to surface as broken (instead of silently + // continuing to point at a profile whose creds the gateway can no longer + // resolve). + if (authProfile.profile_kind === 'oauth_app') { + if (authProfile.status === 'revoked' || authProfile.status === 'error') { + await syncConnectionsForOAuthAppProfile(ctx.organizationId, authProfile.id, false); + // A revoked profile must not also be flagged as the connector default. + if (authProfile.is_default_for_connector && authProfile.connector_key) { + await setDefaultAuthProfileForConnector({ + organizationId: ctx.organizationId, + connectorKey: authProfile.connector_key, + slug: null, + }); + authProfile = { ...authProfile, is_default_for_connector: false }; + } + } else if (authProfile.status === 'active') { + await syncConnectionsForOAuthAppProfile(ctx.organizationId, authProfile.id, true); + } + } + return { action: 'update_auth_profile', auth_profile: serializeAuthProfile(authProfile) }; } @@ -769,6 +834,12 @@ async function handleDeleteAuthProfile( await syncConnectionsForBrowserAuthProfile(ctx.organizationId, existing.id, false); } + // Same for oauth_app: connection.app_auth_profile_id has ON DELETE SET NULL, + // so silent breakage if we don't flip the connection status first. + if (existing.profile_kind === 'oauth_app') { + await syncConnectionsForOAuthAppProfile(ctx.organizationId, existing.id, false); + } + const deleted = await deleteAuthProfile(ctx.organizationId, args.auth_profile_slug); if (!deleted) { return { error: `Failed to delete auth profile '${args.auth_profile_slug}'` }; @@ -780,3 +851,42 @@ async function handleDeleteAuthProfile( auth_profile_slug: args.auth_profile_slug, }; } + +async function handleSetDefaultAuthProfile( + args: Extract, + ctx: ToolContext +): Promise { + if (args.auth_profile_slug !== null) { + const target = await getAuthProfileBySlug(ctx.organizationId, args.auth_profile_slug); + if (!target) { + return { error: `Auth profile '${args.auth_profile_slug}' not found` }; + } + if (target.profile_kind !== 'oauth_app') { + return { + error: `Auth profile '${args.auth_profile_slug}' is a ${target.profile_kind} profile; only oauth_app profiles can be pinned as connector defaults.`, + }; + } + if (target.connector_key !== args.connector_key) { + return { + error: `Auth profile '${args.auth_profile_slug}' is bound to connector '${target.connector_key}', not '${args.connector_key}'.`, + }; + } + if (target.status !== 'active') { + return { + error: `Auth profile '${args.auth_profile_slug}' is ${target.status}; only active profiles can be pinned as the default.`, + }; + } + } + + const pinned = await setDefaultAuthProfileForConnector({ + organizationId: ctx.organizationId, + connectorKey: args.connector_key, + slug: args.auth_profile_slug, + }); + + return { + action: 'set_default_auth_profile', + connector_key: args.connector_key, + auth_profile: pinned ? serializeAuthProfile(pinned) : null, + }; +} diff --git a/packages/server/src/utils/auth-profiles.ts b/packages/server/src/utils/auth-profiles.ts index db6418d30..142222173 100644 --- a/packages/server/src/utils/auth-profiles.ts +++ b/packages/server/src/utils/auth-profiles.ts @@ -29,6 +29,7 @@ export interface AuthProfileRow { browser_kind: BrowserKind | null; user_data_dir: string | null; cdp_url: string | null; + is_default_for_connector: boolean; } interface BrowserSessionSummary { @@ -241,7 +242,8 @@ const AUTH_PROFILE_COLUMNS = ` id, organization_id, slug, display_name, connector_key, profile_kind, status, auth_data, account_id, provider, created_by, created_at, updated_at, - device_worker_id, browser_kind, user_data_dir, cdp_url + device_worker_id, browser_kind, user_data_dir, cdp_url, + is_default_for_connector ` as const; export async function listAuthProfiles(params: { @@ -438,6 +440,9 @@ export async function getPrimaryAuthProfileForKind(params: { const sql = getDb(); if (params.profileKind === 'oauth_app' && params.provider) { + // Admins pin a default via is_default_for_connector; that's the strongest + // preference. After that, prefer a profile bound to this specific + // connector_key over a provider-only match, then fall back to recency. const rows = await sql` SELECT ${sql.unsafe(AUTH_PROFILE_COLUMNS)} FROM auth_profiles @@ -449,6 +454,7 @@ export async function getPrimaryAuthProfileForKind(params: { OR lower(provider) = lower(${params.provider}) ) ORDER BY + CASE WHEN is_default_for_connector AND connector_key = ${params.connectorKey} THEN 0 ELSE 1 END, CASE WHEN connector_key = ${params.connectorKey} THEN 0 ELSE 1 END, updated_at DESC, id DESC @@ -502,6 +508,48 @@ export async function getPrimaryAuthProfileForKind(params: { return rows.length > 0 ? (rows[0] as AuthProfileRow) : null; } +/** + * Pin one oauth_app profile as the org default for its connector_key. Clears + * the flag on any sibling profile for the same (org, connector_key) first so + * the partial unique index never trips. + * + * Pass `slug: null` to clear the default for the connector entirely (no + * profile pinned). Returns the row that ended up flagged, or null on clear. + */ +export async function setDefaultAuthProfileForConnector(params: { + organizationId: string; + connectorKey: string; + slug: string | null; +}): Promise { + const sql = getDb(); + + return sql.begin(async (tx) => { + await tx` + UPDATE auth_profiles + SET is_default_for_connector = false, + updated_at = NOW() + WHERE organization_id = ${params.organizationId} + AND connector_key = ${params.connectorKey} + AND profile_kind = 'oauth_app' + AND is_default_for_connector + `; + + if (params.slug === null) return null; + + const rows = await tx` + UPDATE auth_profiles + SET is_default_for_connector = true, + updated_at = NOW() + WHERE organization_id = ${params.organizationId} + AND slug = ${params.slug} + AND profile_kind = 'oauth_app' + RETURNING ${tx.unsafe(AUTH_PROFILE_COLUMNS)} + `; + + return rows.length > 0 ? (rows[0] as AuthProfileRow) : null; + }) as Promise; +} + export async function resolveAuthProfileSlugToId(params: { organizationId: string; slug?: string | null; diff --git a/packages/web b/packages/web index c81613b00..4516312cf 160000 --- a/packages/web +++ b/packages/web @@ -1 +1 @@ -Subproject commit c81613b00d5b4b4e60f9930696c1244118d0f897 +Subproject commit 4516312cf933313e693a2093da4ffb024740cbc5 From 82945d230060b52c3f04f68c0b8c1c1c23ae0a99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Burak=20Emre=20Kabakc=C4=B1?= Date: Fri, 15 May 2026 19:26:20 +0100 Subject: [PATCH 2/6] feat(auth-profiles): members install own connections via admin's app Opens manage_connections.create + manage_connections.reauthenticate and manage_auth_profiles.{create,update}_auth_profile to member-write. Handler gates enforce intent so members can't bring their own org-shared credentials: - create_auth_profile / update_auth_profile reject non-oauth_account kinds for non-admins (env / oauth_app / browser_session remain admin-only). - manage_connections.create rejects an explicit app_auth_profile_slug from a non-admin unless the profile is is_default_for_connector for that connector. Omit the slug and the server auto-resolves via getPrimaryAuthProfileForKind, which already prefers the admin-pinned default after the previous commit. - 'No app credentials' error rewords for non-admins to 'ask an admin to set up the app'. Updates the tool-access policy tests to match the new boundaries. Bumps packages/web pointer for the OAuth-app indicator on connection settings (admin picker, member-readable). --- .../src/auth/__tests__/tool-access.test.ts | 31 +++++++++++++++++- packages/server/src/auth/tool-access.ts | 24 +++++++++++--- .../src/tools/admin/manage_auth_profiles.ts | 32 +++++++++++++++++++ .../src/tools/admin/manage_connections.ts | 30 +++++++++++++++-- 4 files changed, 109 insertions(+), 8 deletions(-) diff --git a/packages/server/src/auth/__tests__/tool-access.test.ts b/packages/server/src/auth/__tests__/tool-access.test.ts index 2e2d35af6..1437e06ad 100644 --- a/packages/server/src/auth/__tests__/tool-access.test.ts +++ b/packages/server/src/auth/__tests__/tool-access.test.ts @@ -55,7 +55,18 @@ describe('requiresOwnerAdmin', () => { expect( requiresOwnerAdmin('manage_connections', { action: 'update_connector_default_config' }, false) ).toBe(true); + }); + + it('should allow members to create + reauthenticate their own connections', () => { + // `manage_connections.create` and `reauthenticate` are member-write; the + // handler enforces app_auth_profile_slug must match the org default for + // non-admins, so non-admins can't bring an alternate OAuth client. + expect(requiresOwnerAdmin('manage_connections', { action: 'create' }, false)).toBe(false); + expect(requiresMemberWrite('manage_connections', { action: 'create' }, false)).toBe(true); expect(requiresOwnerAdmin('manage_connections', { action: 'reauthenticate' }, false)).toBe( + false + ); + expect(requiresMemberWrite('manage_connections', { action: 'reauthenticate' }, false)).toBe( true ); }); @@ -67,11 +78,29 @@ describe('requiresOwnerAdmin', () => { expect( requiresOwnerAdmin('manage_auth_profiles', { action: 'test_auth_profile' }, false) ).toBe(true); + expect( + requiresOwnerAdmin('manage_auth_profiles', { action: 'delete_auth_profile' }, false) + ).toBe(true); + expect( + requiresOwnerAdmin('manage_auth_profiles', { action: 'set_default_auth_profile' }, false) + ).toBe(true); + }); + + it('should allow members to create their own oauth_account profile', () => { + // create_auth_profile / update_auth_profile are member-write at the + // policy layer; the handler gates by profile_kind so non-oauth_account + // kinds (env, oauth_app, browser_session) stay admin-only. expect( requiresOwnerAdmin('manage_auth_profiles', { action: 'create_auth_profile' }, false) + ).toBe(false); + expect( + requiresMemberWrite('manage_auth_profiles', { action: 'create_auth_profile' }, false) ).toBe(true); expect( - requiresOwnerAdmin('manage_auth_profiles', { action: 'delete_auth_profile' }, false) + requiresOwnerAdmin('manage_auth_profiles', { action: 'update_auth_profile' }, false) + ).toBe(false); + expect( + requiresMemberWrite('manage_auth_profiles', { action: 'update_auth_profile' }, false) ).toBe(true); }); diff --git a/packages/server/src/auth/tool-access.ts b/packages/server/src/auth/tool-access.ts index 3ea851d6c..f9e0b3306 100644 --- a/packages/server/src/auth/tool-access.ts +++ b/packages/server/src/auth/tool-access.ts @@ -22,17 +22,32 @@ const MEMBER_WRITE_ACTIONS: Record | null> = { // via SDK namespace wrappers from inside `run_sdk`, and `routeAction` consults // these tables to fire the same per-action access decisions. manage_entity: new Set(['create', 'update', 'link', 'unlink', 'update_link']), + // Members can install connections that bind to their own OAuth account + // grant. The handler gates `app_auth_profile_slug` overrides against the + // org default + caller role, so members can't pick a non-default app + // profile. + manage_connections: new Set(['create', 'reauthenticate']), + // Members create / reconnect their own oauth_account profile. The handler + // gates `profile_kind` against role so env / oauth_app / browser_session + // stay admin-only. + manage_auth_profiles: new Set([ + 'create_auth_profile', + 'update_auth_profile', + 'test_auth_profile', + 'get_auth_profile', + ]), }; const OWNER_ADMIN_ACTIONS: Record> = { manage_entity: new Set(['delete']), manage_entity_schema: new Set(['create', 'update', 'delete', 'add_rule', 'remove_rule']), manage_connections: new Set([ - 'create', + // `create` and `reauthenticate` are in MEMBER_WRITE_ACTIONS — members + // install their own connections (handler enforces app_auth_profile slug + // override + role gates). 'update', 'delete', 'connect', - 'reauthenticate', 'test', 'install_connector', 'uninstall_connector', @@ -44,10 +59,11 @@ const OWNER_ADMIN_ACTIONS: Record> = { ]), manage_feeds: new Set(['create_feed', 'update_feed', 'delete_feed', 'trigger_feed']), manage_auth_profiles: new Set([ + // `create_auth_profile` and `update_auth_profile` are in + // MEMBER_WRITE_ACTIONS — the handler enforces oauth_account-only access + // for non-admins so members can't create org-shared credentials. 'get_auth_profile', 'test_auth_profile', - 'create_auth_profile', - 'update_auth_profile', 'delete_auth_profile', 'set_default_auth_profile', ]), diff --git a/packages/server/src/tools/admin/manage_auth_profiles.ts b/packages/server/src/tools/admin/manage_auth_profiles.ts index bedba9021..07feae501 100644 --- a/packages/server/src/tools/admin/manage_auth_profiles.ts +++ b/packages/server/src/tools/admin/manage_auth_profiles.ts @@ -30,6 +30,7 @@ import { updateAuthProfile, } from '../../utils/auth-profiles'; import { createConnectToken } from '../../utils/connect-tokens'; +import { getWorkspaceRole } from '../../utils/organization-access'; import type { ToolContext } from '../registry'; import { routeAction } from './action-router'; import { getScopedConnectorDefinition } from './connector-definition-helpers'; @@ -450,6 +451,20 @@ async function handleCreateAuthProfile( args: Extract, ctx: ToolContext ): Promise { + // Only oauth_account profiles are user-personal; every other kind is an + // org-shared credential (env keys, OAuth app client_id/secret, browser + // session, interactive). Gate non-personal kinds on admin role. + if (args.profile_kind !== 'oauth_account') { + const role = ctx.userId + ? await getWorkspaceRole(getDb(), ctx.organizationId, ctx.userId) + : null; + if (role !== 'admin' && role !== 'owner') { + return { + error: `Only admins can create ${args.profile_kind} auth profiles. Ask an organization owner or admin to configure these credentials.`, + }; + } + } + // browser_session profiles are device-scoped; connector_key is optional // (only used as a hint to look up a default cdp_url). Other kinds remain // per-connector and require it. @@ -650,6 +665,23 @@ async function handleUpdateAuthProfile( args: Extract, ctx: ToolContext ): Promise { + // Mirror create gating: only oauth_account profiles are member-editable. + // env / oauth_app / browser_session are org-shared credentials — admin only. + const existingForRoleCheck = await getAuthProfileBySlug( + ctx.organizationId, + args.auth_profile_slug + ); + if (existingForRoleCheck && existingForRoleCheck.profile_kind !== 'oauth_account') { + const role = ctx.userId + ? await getWorkspaceRole(getDb(), ctx.organizationId, ctx.userId) + : null; + if (role !== 'admin' && role !== 'owner') { + return { + error: `Only admins can modify ${existingForRoleCheck.profile_kind} auth profiles.`, + }; + } + } + let authProfile = await updateAuthProfile({ organizationId: ctx.organizationId, slug: args.auth_profile_slug, diff --git a/packages/server/src/tools/admin/manage_connections.ts b/packages/server/src/tools/admin/manage_connections.ts index 0f3016ada..6a42f1c60 100644 --- a/packages/server/src/tools/admin/manage_connections.ts +++ b/packages/server/src/tools/admin/manage_connections.ts @@ -32,6 +32,7 @@ import { import { createAuthProfile, getAuthProfileById, + getAuthProfileBySlug, getBrowserSessionReadiness, getPrimaryAuthProfileForKind, normalizeAuthValues, @@ -789,16 +790,37 @@ async function handleCreate( const sql = getDb(); const { organizationId, userId } = ctx; + // Resolve caller role once — we use it for created_by overrides, explicit + // app_auth_profile picks, and member-friendly error messages downstream. + const callerRole = userId ? await getWorkspaceRole(sql, organizationId, userId) : null; + const callerIsAdmin = callerRole === 'admin' || callerRole === 'owner'; + // Resolve effective owner — admins can create connections on behalf of other users let effectiveCreatedBy = userId; if (args.created_by && args.created_by !== userId) { - const callerRole = await getWorkspaceRole(sql, organizationId, userId!); - if (callerRole !== 'admin' && callerRole !== 'owner') { + if (!callerIsAdmin) { return { error: 'Only admins can create connections for other users.' }; } effectiveCreatedBy = args.created_by; } + // Non-admins must accept the org-default app profile — they can't pick or + // bring an alternate OAuth client. If they explicitly pass a slug, it has + // to match the admin-pinned default for the connector. + if (!callerIsAdmin && args.app_auth_profile_slug) { + const picked = await getAuthProfileBySlug(organizationId, args.app_auth_profile_slug); + if (!picked || picked.profile_kind !== 'oauth_app') { + return { error: `App auth profile '${args.app_auth_profile_slug}' not found` }; + } + const pinnedAsDefault = + picked.is_default_for_connector && picked.connector_key === args.connector_key; + if (!pinnedAsDefault) { + return { + error: `Only admins can override the OAuth app profile. Ask an admin to pin '${args.app_auth_profile_slug}' as the default for this connector, or omit app_auth_profile_slug to use the org default.`, + }; + } + } + // Ensure connector is installed from bundled catalog if needed await ensureConnectorInstalled({ organizationId, connectorKey: args.connector_key }); @@ -933,7 +955,9 @@ async function handleCreate( if (authSelection?.selectedKind === 'oauth_account') { if (!authSelection.appAuthProfile) { return { - error: 'Select or create an OAuth app profile before creating the connection.', + error: callerIsAdmin + ? 'Select or create an OAuth app profile before creating the connection.' + : `No OAuth app credentials configured for this connector. Ask an admin to set up the ${authSelection.oauthMethod?.provider ?? args.connector_key} app in /oauth-apps first.`, }; } if (authSelection.appAuthProfile.status !== 'active') { From 9a432474a0ac20ebc94ed3b57176b041ccaf6e22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Burak=20Emre=20Kabakc=C4=B1?= Date: Fri, 15 May 2026 19:34:23 +0100 Subject: [PATCH 3/6] fix(auth-profiles): tighten member-write surface from pi review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - handleCreate (manage_connections): non-admins fall through to getPrimaryAuthProfileForKind, which can return a recency-picked provider-wide row when no default is pinned. Now require the resolved profile to be is_default_for_connector === true for the exact connector_key when caller is non-admin; otherwise reject with the 'ask an admin' message. - handleUpdateAuthProfile: oauth_account ownership check — non-admins can only update profiles they created (created_by === userId), so the slug alone doesn't grant access to another member's tokens. - handleUpdateAuthProfile (revoke path): wrap status flip + cascade + default-clear in a single transaction via revokeOAuthAppProfileAtomic. Removes the window where connections still pointed at a revoked profile. - Bumps packages/web pointer for the non-admin read-only picker. --- .../src/tools/admin/manage_auth_profiles.ts | 40 ++++++++++++------ .../src/tools/admin/manage_connections.ts | 13 ++++++ packages/server/src/utils/auth-profiles.ts | 41 +++++++++++++++++++ 3 files changed, 81 insertions(+), 13 deletions(-) diff --git a/packages/server/src/tools/admin/manage_auth_profiles.ts b/packages/server/src/tools/admin/manage_auth_profiles.ts index 07feae501..872abdd47 100644 --- a/packages/server/src/tools/admin/manage_auth_profiles.ts +++ b/packages/server/src/tools/admin/manage_auth_profiles.ts @@ -25,6 +25,7 @@ import { listAuthProfiles, normalizeAuthProfileSlug, normalizeAuthValues, + revokeOAuthAppProfileAtomic, setDefaultAuthProfileForConnector, summarizeBrowserSessionAuthData, updateAuthProfile, @@ -667,19 +668,31 @@ async function handleUpdateAuthProfile( ): Promise { // Mirror create gating: only oauth_account profiles are member-editable. // env / oauth_app / browser_session are org-shared credentials — admin only. + // For oauth_account, non-admins can only touch a profile they created — the + // slug alone shouldn't let one member rotate another member's tokens. const existingForRoleCheck = await getAuthProfileBySlug( ctx.organizationId, args.auth_profile_slug ); - if (existingForRoleCheck && existingForRoleCheck.profile_kind !== 'oauth_account') { + if (existingForRoleCheck) { const role = ctx.userId ? await getWorkspaceRole(getDb(), ctx.organizationId, ctx.userId) : null; - if (role !== 'admin' && role !== 'owner') { + const callerIsAdmin = role === 'admin' || role === 'owner'; + if (existingForRoleCheck.profile_kind !== 'oauth_account' && !callerIsAdmin) { return { error: `Only admins can modify ${existingForRoleCheck.profile_kind} auth profiles.`, }; } + if ( + !callerIsAdmin && + existingForRoleCheck.profile_kind === 'oauth_account' && + existingForRoleCheck.created_by !== ctx.userId + ) { + return { + error: `You can only update OAuth account profiles you created. Ask an admin if you need to manage another member's profile.`, + }; + } } let authProfile = await updateAuthProfile({ @@ -807,19 +820,20 @@ async function handleUpdateAuthProfile( // Cascade for oauth_app: admins flipping an app profile to revoked/error // need dependent connections to surface as broken (instead of silently // continuing to point at a profile whose creds the gateway can no longer - // resolve). + // resolve). For the revoke/error case we re-do the status flip inside a + // transaction together with the cascade + default-clear so there's no + // window where connections still reference the revoked profile (the prior + // updateAuthProfile call above already wrote status, but its tx is now + // closed — this overwrite is idempotent and lands the full state change + // atomically). if (authProfile.profile_kind === 'oauth_app') { if (authProfile.status === 'revoked' || authProfile.status === 'error') { - await syncConnectionsForOAuthAppProfile(ctx.organizationId, authProfile.id, false); - // A revoked profile must not also be flagged as the connector default. - if (authProfile.is_default_for_connector && authProfile.connector_key) { - await setDefaultAuthProfileForConnector({ - organizationId: ctx.organizationId, - connectorKey: authProfile.connector_key, - slug: null, - }); - authProfile = { ...authProfile, is_default_for_connector: false }; - } + const atomic = await revokeOAuthAppProfileAtomic({ + organizationId: ctx.organizationId, + profileId: authProfile.id, + nextStatus: authProfile.status, + }); + if (atomic) authProfile = atomic; } else if (authProfile.status === 'active') { await syncConnectionsForOAuthAppProfile(ctx.organizationId, authProfile.id, true); } diff --git a/packages/server/src/tools/admin/manage_connections.ts b/packages/server/src/tools/admin/manage_connections.ts index 6a42f1c60..70d22fc70 100644 --- a/packages/server/src/tools/admin/manage_connections.ts +++ b/packages/server/src/tools/admin/manage_connections.ts @@ -965,6 +965,19 @@ async function handleCreate( error: `Selected app auth profile '${authSelection.appAuthProfile.slug}' is not active.`, }; } + // Even when the slug is omitted, non-admins can only fall through to the + // admin-pinned default for this exact connector. The resolver may + // otherwise return a recency-picked provider-wide row, which would let a + // member silently use an OAuth client the admin never blessed. + if ( + !callerIsAdmin && + (!authSelection.appAuthProfile.is_default_for_connector || + authSelection.appAuthProfile.connector_key !== args.connector_key) + ) { + return { + error: `No default OAuth app configured for this connector. Ask an admin to pin a ${authSelection.oauthMethod?.provider ?? args.connector_key} app as the default in /oauth-apps.`, + }; + } } const displayName = await resolveConnectionDisplayName({ diff --git a/packages/server/src/utils/auth-profiles.ts b/packages/server/src/utils/auth-profiles.ts index 142222173..a6e01f252 100644 --- a/packages/server/src/utils/auth-profiles.ts +++ b/packages/server/src/utils/auth-profiles.ts @@ -508,6 +508,47 @@ export async function getPrimaryAuthProfileForKind(params: { return rows.length > 0 ? (rows[0] as AuthProfileRow) : null; } +/** + * Atomically transition an oauth_app profile to a "broken" status (revoked or + * error) while also flipping every connection that minted tokens against it + * to pending_auth and clearing the connector default flag if set. Running + * these three statements in one transaction prevents the window where a + * connection still references a revoked profile, and is the only safe + * substitute for the missing FK-level cascade. + */ +export async function revokeOAuthAppProfileAtomic(params: { + organizationId: string; + profileId: number; + nextStatus: 'revoked' | 'error'; +}): Promise { + const sql = getDb(); + return sql.begin(async (tx) => { + const profileRows = await tx` + UPDATE auth_profiles + SET status = ${params.nextStatus}, + is_default_for_connector = false, + updated_at = NOW() + WHERE organization_id = ${params.organizationId} + AND id = ${params.profileId} + AND profile_kind = 'oauth_app' + RETURNING ${tx.unsafe(AUTH_PROFILE_COLUMNS)} + `; + + if (profileRows.length === 0) return null; + + await tx` + UPDATE connections + SET status = 'pending_auth', + updated_at = NOW() + WHERE organization_id = ${params.organizationId} + AND app_auth_profile_id = ${params.profileId} + AND deleted_at IS NULL + `; + + return profileRows[0] as AuthProfileRow; + }) as Promise; +} + /** * Pin one oauth_app profile as the org default for its connector_key. Clears * the flag on any sibling profile for the same (org, connector_key) first so From 549d62427d975f3943fa32b00dfb793da912bfc9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Burak=20Emre=20Kabakc=C4=B1?= Date: Sat, 16 May 2026 16:57:13 +0100 Subject: [PATCH 4/6] fix(auth-profiles): plug 5 privilege-escalation gaps from pi review - manage_connections.create: non-admins cannot bind to env profiles or oauth_account/browser_session profiles owned by other users - manage_connections.create: reject entity_link_overrides from non-admins (it mutates connector_definitions org-wide) - create_auth_profile slug-reuse: non-admins must own the existing slug before reconnecting; closes hijack-by-known-slug vector - reauthenticate: require connection-owner or admin; blocks members from disrupting another member's interactive auth - delete_auth_profile: lock profile row + sync + delete in one tx so a concurrent create can't leave an active connection with NULL app_auth_profile_id after FK ON DELETE SET NULL fires --- .../src/tools/admin/manage_auth_profiles.ts | 76 +++++++++++++++---- .../src/tools/admin/manage_connections.ts | 46 +++++++++++ 2 files changed, 106 insertions(+), 16 deletions(-) diff --git a/packages/server/src/tools/admin/manage_auth_profiles.ts b/packages/server/src/tools/admin/manage_auth_profiles.ts index 872abdd47..eb18e1578 100644 --- a/packages/server/src/tools/admin/manage_auth_profiles.ts +++ b/packages/server/src/tools/admin/manage_auth_profiles.ts @@ -512,6 +512,19 @@ async function handleCreateAuthProfile( error: `Auth profile '${existing.slug}' already exists with a different kind/connector (${existing.profile_kind} / ${existing.connector_key}) — use a new slug`, }; } + // Non-admins reusing an existing oauth_account slug must own it — + // otherwise a member who knows another member's pending profile slug + // could mint a fresh connect token for it and complete OAuth into a + // profile already referenced by someone else's connections. + const role = ctx.userId + ? await getWorkspaceRole(getDb(), ctx.organizationId, ctx.userId) + : null; + const callerIsAdmin = role === 'admin' || role === 'owner'; + if (!callerIsAdmin && existing.created_by !== ctx.userId) { + return { + error: `Auth profile '${existing.slug}' belongs to another user. Choose a different slug.`, + }; + } if (existing.status === 'active') { return { action: 'create_auth_profile', auth_profile: serializeAuthProfile(existing) }; } @@ -868,25 +881,56 @@ async function handleDeleteAuthProfile( }; } - // Clean up connect tokens referencing this profile - await sql` - UPDATE connect_tokens - SET auth_profile_id = NULL - WHERE auth_profile_id = ${existing.id} - `; + // Sync + delete must happen atomically: between flipping dependent + // connections to `pending_auth` and the DELETE, a concurrent + // `manage_connections.create` could insert a new connection referencing + // this profile. The FK `ON DELETE SET NULL` would then leave that + // connection active with `app_auth_profile_id = NULL`. Lock the profile + // row up front (`FOR UPDATE` conflicts with the FK insert's FOR KEY SHARE + // lock, so concurrent inserts block until we commit and then fail FK). + const deleted = await sql.begin(async (tx) => { + const lockRows = await tx` + SELECT id FROM auth_profiles + WHERE organization_id = ${ctx.organizationId} + AND id = ${existing.id} + FOR UPDATE + `; + if (lockRows.length === 0) return null; - // Pause browser-backed connections BEFORE deleting (ON DELETE SET NULL would orphan them) - if (existing.profile_kind === 'browser_session') { - await syncConnectionsForBrowserAuthProfile(ctx.organizationId, existing.id, false); - } + await tx` + UPDATE connect_tokens + SET auth_profile_id = NULL + WHERE auth_profile_id = ${existing.id} + `; - // Same for oauth_app: connection.app_auth_profile_id has ON DELETE SET NULL, - // so silent breakage if we don't flip the connection status first. - if (existing.profile_kind === 'oauth_app') { - await syncConnectionsForOAuthAppProfile(ctx.organizationId, existing.id, false); - } + if (existing.profile_kind === 'browser_session') { + await tx` + UPDATE connections + SET status = 'pending_auth', updated_at = NOW() + WHERE organization_id = ${ctx.organizationId} + AND auth_profile_id = ${existing.id} + AND deleted_at IS NULL + `; + } - const deleted = await deleteAuthProfile(ctx.organizationId, args.auth_profile_slug); + if (existing.profile_kind === 'oauth_app') { + await tx` + UPDATE connections + SET status = 'pending_auth', updated_at = NOW() + WHERE organization_id = ${ctx.organizationId} + AND app_auth_profile_id = ${existing.id} + AND deleted_at IS NULL + `; + } + + const deletedRows = await tx` + DELETE FROM auth_profiles + WHERE organization_id = ${ctx.organizationId} + AND id = ${existing.id} + RETURNING id + `; + return deletedRows.length > 0 ? deletedRows[0] : null; + }); if (!deleted) { return { error: `Failed to delete auth profile '${args.auth_profile_slug}'` }; } diff --git a/packages/server/src/tools/admin/manage_connections.ts b/packages/server/src/tools/admin/manage_connections.ts index 70d22fc70..957f6836e 100644 --- a/packages/server/src/tools/admin/manage_connections.ts +++ b/packages/server/src/tools/admin/manage_connections.ts @@ -804,6 +804,17 @@ async function handleCreate( effectiveCreatedBy = args.created_by; } + // `entity_link_overrides` writes to `connector_definitions` for the entire + // org. Even though `create` is now member-write, that mutation must stay + // admin-only — otherwise a member could change connector-level entity + // mapping while ostensibly installing their own connection. + if (!callerIsAdmin && args.entity_link_overrides !== undefined) { + return { + error: + 'Only admins can change connector entity-link overrides. Omit `entity_link_overrides`, or ask an admin to update them via `set_connector_entity_link_overrides`.', + }; + } + // Non-admins must accept the org-default app profile — they can't pick or // bring an alternate OAuth client. If they explicitly pass a slug, it has // to match the admin-pinned default for the connector. @@ -952,6 +963,29 @@ async function handleCreate( }; } + // Non-admin members can only bind a connection to a runtime auth profile + // they own. `env` profiles are admin-managed org-shared credentials — + // members must never bind to them. `oauth_account` and `browser_session` + // profiles are member-creatable but still per-user, so a member can't + // hijack another member's grant by passing their slug. + if (authSelection?.authProfile && !callerIsAdmin) { + const profile = authSelection.authProfile; + if (profile.profile_kind === 'env') { + return { + error: + 'Only admins can use env-credential auth profiles. Ask an admin to install this connection.', + }; + } + if ( + (profile.profile_kind === 'oauth_account' || profile.profile_kind === 'browser_session') && + profile.created_by !== ctx.userId + ) { + return { + error: `Auth profile '${profile.slug}' belongs to another user. Create your own profile (action: 'create_auth_profile') and use its slug instead.`, + }; + } + } + if (authSelection?.selectedKind === 'oauth_account') { if (!authSelection.appAuthProfile) { return { @@ -1918,6 +1952,7 @@ async function handleReauthenticate( c.status AS connection_status, c.connector_key, c.auth_profile_id, + c.created_by AS connection_created_by, ap.profile_kind, ap.status AS auth_profile_status FROM connections c @@ -1937,10 +1972,21 @@ async function handleReauthenticate( connection_status: string; connector_key: string; auth_profile_id: number | null; + connection_created_by: string | null; profile_kind: string | null; auth_profile_status: string | null; }; + // `reauthenticate` flips the connection + its interactive profile to + // `pending_auth` and kicks off an auth run — that has to be the connection + // owner or an admin/owner. Without this gate, any org member could disrupt + // (or hijack the pairing of) another member's interactive connection. + const callerRole = await getWorkspaceRole(sql, organizationId, ctx.userId); + const callerIsAdmin = callerRole === 'admin' || callerRole === 'owner'; + if (!callerIsAdmin && row.connection_created_by !== ctx.userId) { + return { error: 'You can only re-authenticate connections you created.' }; + } + if (!row.auth_profile_id || row.profile_kind !== 'interactive') { return { error: 'Connection does not use an interactive auth profile' }; } From fff25dc05fc1ee1590da58bc3eda6b5d5176f3e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Burak=20Emre=20Kabakc=C4=B1?= Date: Sat, 16 May 2026 17:02:35 +0100 Subject: [PATCH 5/6] fix(auth-profiles): preserve feed pause + explicit FK null on profile delete MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pi second pass flagged two follow-ups on the tx-wrapped delete: - browser_session delete must still pause dependent feeds + clear next_run_at (the previous syncConnectionsForBrowserAuthProfile did this and the tx version had dropped it) - the composite FK is (organization_id, *_auth_profile_id) ON DELETE SET NULL; without an explicit column list Postgres would attempt to null organization_id too (NOT NULL → constraint violation on force delete with active connections). Null the FK column explicitly inside the tx so the DELETE itself never depends on FK SET NULL. --- .../src/tools/admin/manage_auth_profiles.ts | 31 ++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/packages/server/src/tools/admin/manage_auth_profiles.ts b/packages/server/src/tools/admin/manage_auth_profiles.ts index eb18e1578..bbc84e450 100644 --- a/packages/server/src/tools/admin/manage_auth_profiles.ts +++ b/packages/server/src/tools/admin/manage_auth_profiles.ts @@ -904,12 +904,23 @@ async function handleDeleteAuthProfile( `; if (existing.profile_kind === 'browser_session') { + // Pause dependent connections + their feeds (mirrors + // syncConnectionsForBrowserAuthProfile, inlined for tx locality). await tx` UPDATE connections SET status = 'pending_auth', updated_at = NOW() WHERE organization_id = ${ctx.organizationId} AND auth_profile_id = ${existing.id} - AND deleted_at IS NULL + `; + await tx` + UPDATE feeds f + SET status = 'paused', + next_run_at = NULL, + updated_at = NOW() + FROM connections c + WHERE f.connection_id = c.id + AND c.organization_id = ${ctx.organizationId} + AND c.auth_profile_id = ${existing.id} `; } @@ -923,6 +934,24 @@ async function handleDeleteAuthProfile( `; } + // Explicitly null the FK columns before delete. The FK is composite + // `(organization_id, *_auth_profile_id) ON DELETE SET NULL`; without a + // SET NULL column list, Postgres would attempt to null organization_id + // too (NOT NULL → constraint violation). Setting only the profile-id + // column here avoids that and matches the intended semantic. + await tx` + UPDATE connections + SET auth_profile_id = NULL, updated_at = NOW() + WHERE organization_id = ${ctx.organizationId} + AND auth_profile_id = ${existing.id} + `; + await tx` + UPDATE connections + SET app_auth_profile_id = NULL, updated_at = NOW() + WHERE organization_id = ${ctx.organizationId} + AND app_auth_profile_id = ${existing.id} + `; + const deletedRows = await tx` DELETE FROM auth_profiles WHERE organization_id = ${ctx.organizationId} From 0473232e8754053be8359a079beee775c44bdf1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Burak=20Emre=20Kabakc=C4=B1?= Date: Sat, 16 May 2026 17:05:43 +0100 Subject: [PATCH 6/6] fix(db): reorder schema.sql auth_profiles index to match dbmate output --- db/schema.sql | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/db/schema.sql b/db/schema.sql index ac3946456..9ca4e6132 100644 --- a/db/schema.sql +++ b/db/schema.sql @@ -2857,6 +2857,12 @@ CREATE INDEX agents_organization_id_idx ON public.agents USING btree (organizati CREATE INDEX auth_profiles_connector_kind_idx ON public.auth_profiles USING btree (organization_id, connector_key, profile_kind, status); +-- +-- Name: auth_profiles_default_for_connector_unique; Type: INDEX; Schema: public; Owner: - +-- + +CREATE UNIQUE INDEX auth_profiles_default_for_connector_unique ON public.auth_profiles USING btree (organization_id, connector_key) WHERE (is_default_for_connector AND (profile_kind = 'oauth_app'::text)); + -- -- Name: auth_profiles_device_worker_idx; Type: INDEX; Schema: public; Owner: - -- @@ -2875,12 +2881,6 @@ CREATE UNIQUE INDEX auth_profiles_org_slug_unique ON public.auth_profiles USING CREATE UNIQUE INDEX auth_profiles_pending_unique ON public.auth_profiles USING btree (organization_id, connector_key, profile_kind, provider) WHERE (status = 'pending_auth'::text); --- --- Name: auth_profiles_default_for_connector_unique; Type: INDEX; Schema: public; Owner: - --- - -CREATE UNIQUE INDEX auth_profiles_default_for_connector_unique ON public.auth_profiles USING btree (organization_id, connector_key) WHERE (is_default_for_connector AND (profile_kind = 'oauth_app'::text)); - -- -- Name: chat_user_identities_lobu_user_idx; Type: INDEX; Schema: public; Owner: - --