diff --git a/crates/overslash-api/src/routes/connections.rs b/crates/overslash-api/src/routes/connections.rs index 7359336a..a28f5fd2 100644 --- a/crates/overslash-api/src/routes/connections.rs +++ b/crates/overslash-api/src/routes/connections.rs @@ -885,6 +885,16 @@ struct ListConnectionsQuery { /// flag is revoked — same contract as the services list. #[serde(default)] include_user_level: bool, + /// Admin-or-self: list connections owned by this specific identity instead + /// of the caller's own. The service detail page passes the service's + /// `owner_identity_id` so an admin viewing another user's service sees that + /// user's bindable connections (connections are identity-scoped). Equal to + /// the caller's own identity → self path. A non-admin caller passing a + /// *different* identity is silently downgraded to their own list (no 403, + /// same contract as `include_user_level`). Takes precedence over + /// `include_user_level` when both are set. + #[serde(default)] + owner_identity_id: Option, } async fn list_connections( @@ -895,18 +905,29 @@ async fn list_connections( // identity row (same flag-based check as the services list — `AdminAcl` // would instead require the `overslash` service admin grant). Non-admins // passing the flag fall through to the standard self-scoped listing. - let admin_view_all = if q.include_user_level { - scope - .org() - .get_identity(scope.user_id()) - .await? - .map(|i| i.is_org_admin) - .unwrap_or(false) - } else { - false + let is_org_admin = || async { + Ok::( + scope + .org() + .get_identity(scope.user_id()) + .await? + .map(|i| i.is_org_admin) + .unwrap_or(false), + ) }; - let rows = if admin_view_all { + let rows = if let Some(owner) = q.owner_identity_id { + // Owner-scoped listing. Self is always allowed; another identity + // requires org admin, else fall through to the caller's own list. + if owner == scope.user_id() { + scope.list_my_connections().await? + } else if is_org_admin().await? { + let owner_scope = UserScope::new(scope.org_id(), owner, scope.org().db().clone()); + owner_scope.list_my_connections().await? + } else { + scope.list_my_connections().await? + } + } else if q.include_user_level && is_org_admin().await? { scope.org().list_all_connections().await? } else { scope.list_my_connections().await? diff --git a/crates/overslash-api/tests/connections_admin_view.rs b/crates/overslash-api/tests/connections_admin_view.rs index 7cb7e9c5..17acc71e 100644 --- a/crates/overslash-api/tests/connections_admin_view.rs +++ b/crates/overslash-api/tests/connections_admin_view.rs @@ -235,6 +235,108 @@ async fn non_admin_cannot_delete_another_users_connection() { assert_eq!(remaining, 1, "connection must survive"); } +/// `?owner_identity_id=` lets an org admin list a specific user's +/// connections (e.g. the owner of a service they're viewing) — only that +/// owner's rows, not the admin's own and not the whole org. +#[tokio::test] +async fn admin_owner_scoped_lists_only_that_owner() { + let (pool, fx) = common::test_pool_bootstrapped().await; + let (addr, client) = common::start_api(pool.clone()).await; + let base = format!("http://{addr}"); + + let admin_conn = + seed_connection(&pool, fx.org_id, fx.user_ids[0], "google", "admin@x", true).await; + let write_conn = + seed_connection(&pool, fx.org_id, fx.user_ids[1], "github", "write@x", true).await; + let read_conn = + seed_connection(&pool, fx.org_id, fx.user_ids[2], "github", "read@x", true).await; + make_org_admin(&pool, fx.user_ids[0]).await; + + let resp = client + .get(format!( + "{base}/v1/connections?owner_identity_id={}", + fx.user_ids[1] + )) + .header("Authorization", format!("Bearer {}", fx.admin_key)) + .send() + .await + .unwrap(); + assert_eq!(resp.status(), 200); + let conns: Vec = resp.json().await.unwrap(); + let ids: Vec<&str> = conns.iter().filter_map(|c| c["id"].as_str()).collect(); + assert!( + ids.contains(&write_conn.to_string().as_str()), + "must see the target owner's connection (got: {ids:?})" + ); + assert!( + !ids.contains(&admin_conn.to_string().as_str()) + && !ids.contains(&read_conn.to_string().as_str()), + "must see ONLY the target owner's rows (got: {ids:?})" + ); +} + +/// A non-admin passing `?owner_identity_id=` is silently downgraded to +/// their own connections (same contract as `include_user_level`, no 403). +#[tokio::test] +async fn non_admin_owner_scoped_downgrades_to_own() { + let (pool, fx) = common::test_pool_bootstrapped().await; + let (addr, client) = common::start_api(pool.clone()).await; + let base = format!("http://{addr}"); + + let admin_conn = + seed_connection(&pool, fx.org_id, fx.user_ids[0], "google", "admin@x", true).await; + let write_conn = + seed_connection(&pool, fx.org_id, fx.user_ids[1], "github", "write@x", true).await; + + // write-user (non-admin) asks for the admin-user's connections. + let resp = client + .get(format!( + "{base}/v1/connections?owner_identity_id={}", + fx.user_ids[0] + )) + .header("Authorization", format!("Bearer {}", fx.write_key)) + .send() + .await + .unwrap(); + assert_eq!(resp.status(), 200); + let conns: Vec = resp.json().await.unwrap(); + let ids: Vec<&str> = conns.iter().filter_map(|c| c["id"].as_str()).collect(); + assert!( + ids.contains(&write_conn.to_string().as_str()) + && !ids.contains(&admin_conn.to_string().as_str()), + "non-admin must fall back to their own rows only (got: {ids:?})" + ); +} + +/// `?owner_identity_id=` is always allowed and returns the caller's own +/// connections — the common case for a user viewing their own service. +#[tokio::test] +async fn owner_scoped_self_returns_own() { + let (pool, fx) = common::test_pool_bootstrapped().await; + let (addr, client) = common::start_api(pool.clone()).await; + let base = format!("http://{addr}"); + + let write_conn = + seed_connection(&pool, fx.org_id, fx.user_ids[1], "github", "write@x", true).await; + + let resp = client + .get(format!( + "{base}/v1/connections?owner_identity_id={}", + fx.user_ids[1] + )) + .header("Authorization", format!("Bearer {}", fx.write_key)) + .send() + .await + .unwrap(); + assert_eq!(resp.status(), 200); + let conns: Vec = resp.json().await.unwrap(); + let ids: Vec<&str> = conns.iter().filter_map(|c| c["id"].as_str()).collect(); + assert!( + ids.contains(&write_conn.to_string().as_str()), + "self owner scope must return own connections (got: {ids:?})" + ); +} + /// An org admin promoting another user's connection demotes the sibling within /// the **owner's** (identity, provider), not the admin's. #[tokio::test] diff --git a/dashboard/src/lib/api/services.ts b/dashboard/src/lib/api/services.ts index 30f60b73..20ab68b7 100644 --- a/dashboard/src/lib/api/services.ts +++ b/dashboard/src/lib/api/services.ts @@ -150,12 +150,18 @@ export const listServiceGroups = (serviceId: string, signal?: AbortSignal) => // -- OAuth connections -- export const listConnections = ( - opts: { includeUserLevel?: boolean } = {}, + opts: { includeUserLevel?: boolean; ownerIdentityId?: string } = {}, signal?: AbortSignal ) => { // `include_user_level=true` (admin-only) lists every user's connections // across the org; silently ignored for non-admins by the backend. - const qs = opts.includeUserLevel ? '?include_user_level=true' : ''; + // `owner_identity_id` lists a specific owner's connections (self always + // allowed, another identity is admin-only); the service detail page passes + // the service's owner so an admin sees that user's bindable connections. + const params = new URLSearchParams(); + if (opts.includeUserLevel) params.set('include_user_level', 'true'); + if (opts.ownerIdentityId) params.set('owner_identity_id', opts.ownerIdentityId); + const qs = params.toString() ? `?${params}` : ''; return session.get(`/v1/connections${qs}`, signal); }; diff --git a/dashboard/src/routes/services/[id]/+page.svelte b/dashboard/src/routes/services/[id]/+page.svelte index 023ae281..bf62e120 100644 --- a/dashboard/src/routes/services/[id]/+page.svelte +++ b/dashboard/src/routes/services/[id]/+page.svelte @@ -234,7 +234,12 @@ // Use svc.id (not name) so user-shadows-org can't return actions // from a same-named user instance. getServiceActions(fresh.id, ctrl.signal).catch(() => [] as ActionSummary[]), - listConnections({}, ctrl.signal).catch(() => [] as ConnectionSummary[]), + // Scope to the service owner so an admin viewing another user's + // service sees that user's (bindable) connections, not their own. + listConnections( + { ownerIdentityId: fresh.owner_identity_id }, + ctrl.signal + ).catch(() => [] as ConnectionSummary[]), session .get('/v1/identities', ctrl.signal) .catch(() => [] as Identity[]), @@ -333,7 +338,10 @@ await new Promise((r) => setTimeout(r, 1500)); if (ctrl.signal.aborted) return; try { - connections = await listConnections({}, ctrl.signal); + connections = await listConnections( + { ownerIdentityId: svc?.owner_identity_id }, + ctrl.signal + ); } catch { if (ctrl.signal.aborted) return; } @@ -402,7 +410,10 @@ await new Promise((r) => setTimeout(r, 1500)); if (ctrl.signal.aborted) return; try { - connections = await listConnections({}, ctrl.signal); + connections = await listConnections( + { ownerIdentityId: svc?.owner_identity_id }, + ctrl.signal + ); } catch { if (ctrl.signal.aborted) return; }