diff --git a/crates/ruscker-admin/assets/i18n/en/landing.ftl b/crates/ruscker-admin/assets/i18n/en/landing.ftl index f863fee..52e6303 100644 --- a/crates/ruscker-admin/assets/i18n/en/landing.ftl +++ b/crates/ruscker-admin/assets/i18n/en/landing.ftl @@ -342,6 +342,7 @@ spec-form-error-cpu = CPU limit must be a positive number (e.g. 0.5). spec-form-error-memory = Memory limit must be a size like 512m or 1.5g. spec-form-error-replica-range = Max replicas must be greater than or equal to min replicas. spec-form-error-stale = Someone else saved this app while you were editing. Review the current values below and submit again. +spec-form-error-editor-scope = A restricted app must keep at least one of your groups and cannot include groups outside your Editor scope. # Admin image library admin-images-title = Media library diff --git a/crates/ruscker-admin/assets/i18n/es/landing.ftl b/crates/ruscker-admin/assets/i18n/es/landing.ftl index 6c5a5e4..65e26e2 100644 --- a/crates/ruscker-admin/assets/i18n/es/landing.ftl +++ b/crates/ruscker-admin/assets/i18n/es/landing.ftl @@ -342,6 +342,7 @@ spec-form-error-cpu = El límite de CPU debe ser un número positivo (ej.: 0.5). spec-form-error-memory = El límite de memoria debe ser un tamaño como 512m o 1.5g. spec-form-error-replica-range = Réplicas máx. debe ser mayor o igual que réplicas mín. spec-form-error-stale = Otra persona guardó esta app mientras editabas. Revisa los valores actuales abajo y envía de nuevo. +spec-form-error-editor-scope = Una app restringida debe conservar al menos uno de tus grupos y no puede incluir grupos fuera de tu alcance de Editor. # Admin image library admin-images-title = Biblioteca multimedia diff --git a/crates/ruscker-admin/assets/i18n/fr/landing.ftl b/crates/ruscker-admin/assets/i18n/fr/landing.ftl index d898083..9a59190 100644 --- a/crates/ruscker-admin/assets/i18n/fr/landing.ftl +++ b/crates/ruscker-admin/assets/i18n/fr/landing.ftl @@ -342,6 +342,7 @@ spec-form-error-cpu = La limite CPU doit être un nombre positif (ex. 0.5). spec-form-error-memory = La limite mémoire doit être une taille comme 512m ou 1.5g. spec-form-error-replica-range = Le max de réplicas doit être supérieur ou égal au min. spec-form-error-stale = Une autre personne a enregistré cette app pendant votre édition. Vérifiez les valeurs actuelles ci-dessous et soumettez à nouveau. +spec-form-error-editor-scope = Une app restreinte doit conserver au moins un de vos groupes et ne peut pas inclure de groupes hors de votre périmètre d'Éditeur. # Admin image library admin-images-title = Bibliothèque de médias diff --git a/crates/ruscker-admin/assets/i18n/pt/landing.ftl b/crates/ruscker-admin/assets/i18n/pt/landing.ftl index d93dfb2..e560581 100644 --- a/crates/ruscker-admin/assets/i18n/pt/landing.ftl +++ b/crates/ruscker-admin/assets/i18n/pt/landing.ftl @@ -346,6 +346,7 @@ spec-form-error-cpu = O limite de CPU deve ser um número positivo (ex.: 0.5). spec-form-error-memory = O limite de memória deve ser um tamanho como 512m ou 1.5g. spec-form-error-replica-range = Réplicas máx. deve ser maior ou igual a réplicas mín. spec-form-error-stale = Outra pessoa salvou este app enquanto você editava. Revise os valores atuais abaixo e envie novamente. +spec-form-error-editor-scope = Um app restrito deve manter pelo menos um dos seus grupos e não pode incluir grupos fora do seu escopo de Editor. # Admin image library admin-images-title = Biblioteca de mídia diff --git a/crates/ruscker-admin/src/routes/admin/dashboard.rs b/crates/ruscker-admin/src/routes/admin/dashboard.rs index 03f05c7..00afe9c 100644 --- a/crates/ruscker-admin/src/routes/admin/dashboard.rs +++ b/crates/ruscker-admin/src/routes/admin/dashboard.rs @@ -39,6 +39,7 @@ use dashmap::DashMap; use crate::auth::{AdminSession, RequireEditor, Role}; use crate::i18n::{Locale, Locales}; +use crate::scope::EditorScope; use crate::theme::Theme; use crate::AppState; @@ -72,17 +73,49 @@ const SSE_INTERVAL: Duration = Duration::from_secs(5); /// handler. const SSE_KEEPALIVE_INTERVAL: Duration = Duration::from_secs(15); -/// Per-locale memoized dashboard snapshot (#291). Building a snapshot -/// reads the registry, fetches the effective spec catalog (a DB query), -/// and assembles the rows; with N open dashboard tabs each running its -/// own [`SSE_INTERVAL`] loop that was N× the work every tick. The whole -/// snapshot is the same for everyone in a given locale (only the state -/// labels are localized), so cache it for one interval and let every -/// tab/connection in that locale reuse it. Keyed by locale (≤4 entries); -/// a clone of the cached snapshot is cheap next to a fresh DB build. -static SNAPSHOT_CACHE: LazyLock)>> = +/// Memoized dashboard snapshot (#291). Building one reads the registry, +/// fetches the effective spec catalog (a DB query) and assembles the rows; +/// with N open dashboard tabs each running its own [`SSE_INTERVAL`] loop +/// that was N× the work every tick. +/// +/// Keyed by locale **and scope** (#990). The snapshot used to be identical +/// for everyone in a locale, but a scoped Editor sees only their groups' +/// rows, so a locale-only key would hand one team's containers to another. +/// Sharing survives where it matters: every Admin/break-glass view shares +/// one entry, and two Editors with the same group set share theirs — which +/// keeps the #291 guarantee for the multi-tab case that made the admin +/// hang on an HTTP/1.1 front end (#1039/#1040) instead of dropping caching +/// for Editors altogether. +static SNAPSHOT_CACHE: LazyLock> = LazyLock::new(DashMap::new); +/// `(locale, scope)` — see [`SNAPSHOT_CACHE`]. +type SnapshotCacheKey = (Locale, ScopeKey); + +/// When it was built, and the snapshot itself behind an `Arc` so serving a +/// cache hit clones a pointer rather than the whole row set. +type CachedSnapshot = (Instant, Arc); + +/// Cache identity of a viewer's authorization scope: `None` for the +/// unscoped Admin/token view, or the caller's groups sorted and joined so +/// two Editors with the same memberships hit the same entry. Sorting +/// matters — `["a","b"]` and `["b","a"]` are the same scope and must not +/// split the cache. +type ScopeKey = Option; + +fn scope_key(scope: &EditorScope) -> ScopeKey { + if scope.unscoped { + return None; + } + let mut groups: Vec<&str> = scope.groups.iter().map(String::as_str).collect(); + groups.sort_unstable(); + groups.dedup(); + // `\u{1f}` (unit separator) can't appear in a group name, so the join is + // unambiguous where a plain comma would let `["a,b"]` collide with + // `["a","b"]`. + Some(groups.join("\u{1f}")) +} + /// One row of the replicas table — flattened for the template /// and also serialized as JSON over the SSE stream so the /// client-side patcher can update in place. @@ -364,7 +397,15 @@ async fn index( if !session.role.can_access_section("dashboard") { return Redirect::to("/").into_response(); } - let snap = build_snapshot(&state, loc).await; + let scope = EditorScope::from_editor( + RequireEditor { + role: session.role, + actor: session.actor, + }, + state.db.as_ref(), + ) + .await; + let snap = build_snapshot(&state, loc, &scope).await; let snapshot_json = json_for_html_script(&snap); let page = DashboardPage { locale: loc, @@ -373,7 +414,7 @@ async fn index( locales_all: &Locale::ALL, base: state.base_path.clone(), nav_section: "dashboard", - role: session.role, + role: scope.role, backend_connected: snap.backend_connected, total_containers: snap.total_containers, total_sessions: snap.total_sessions, @@ -396,18 +437,30 @@ async fn index( /// Snapshot for the dashboard + SSE, memoized per locale for one /// [`SSE_INTERVAL`] (#291) so N concurrent tabs share one build instead /// of each re-querying the DB every tick. -async fn build_snapshot(state: &AppState, locale: Locale) -> DashboardSnapshot { - if let Some(entry) = SNAPSHOT_CACHE.get(&locale) { +async fn build_snapshot( + state: &AppState, + locale: Locale, + scope: &EditorScope, +) -> DashboardSnapshot { + // Keyed by (locale, scope): a scoped Editor must never read an entry + // built for another group set, but two viewers with the SAME scope + // still share one build — that's the #291 guarantee for many tabs. + let key = (locale, scope_key(scope)); + if let Some(entry) = SNAPSHOT_CACHE.get(&key) { if entry.0.elapsed() < SSE_INTERVAL { return (*entry.1).clone(); } } - let snap = build_snapshot_uncached(state, locale).await; - SNAPSHOT_CACHE.insert(locale, (Instant::now(), Arc::new(snap.clone()))); + let snap = build_snapshot_uncached(state, locale, scope).await; + SNAPSHOT_CACHE.insert(key, (Instant::now(), Arc::new(snap.clone()))); snap } -async fn build_snapshot_uncached(state: &AppState, locale: Locale) -> DashboardSnapshot { +async fn build_snapshot_uncached( + state: &AppState, + locale: Locale, + scope: &EditorScope, +) -> DashboardSnapshot { let backend_connected = state.backend.is_some(); // Snapshot the registry once. Cloning `Replica` is cheap @@ -419,11 +472,23 @@ async fn build_snapshot_uncached(state: &AppState, locale: Locale) -> DashboardS // #202 / admin-added specs), which aren't in the YAML config. The // old config-keyed walk silently dropped every such replica from // the dashboard even though it was running. - let snap: Vec = { + let mut snap: Vec = { let reg = state.replicas.read().await; reg.all().cloned().collect() }; + let catalog = crate::catalog::effective_specs_cached(state).await; + if !scope.unscoped { + snap.retain(|replica| { + // Replica rows carry only a spec id. Missing catalog metadata is + // never equivalent to an open app: scoped Editors fail closed. + catalog + .iter() + .find(|spec| spec.id == replica.spec_id) + .is_some_and(|spec| scope.may_touch_spec(spec)) + }); + } + let total_containers = snap.len(); let total_sessions: u32 = snap.iter().map(|r| r.sessions_active).sum(); let spec_count: usize = snap @@ -431,14 +496,20 @@ async fn build_snapshot_uncached(state: &AppState, locale: Locale) -> DashboardS .map(|r| r.spec_id.as_str()) .collect::>() .len(); - let tracker_sessions = state.sessions.len(); + let tracker_sessions = if scope.unscoped { + state.sessions.len() + } else { + // SessionStore exposes only a global length. For a scoped dashboard, + // use the visible replicas' committed counts rather than leaking the + // global tracker cardinality through a KPI. + total_sessions as usize + }; // Resolve display names from the effective catalog (DB ∪ YAML, // DB-first) fetched **once** per snapshot — not a `find_spec` DB // round-trip per spec, which on the 5s SSE tick (× every open tab) // hammered the DB (#281). Still DB-first, so a renamed-in-admin spec // shows its current name (#275). - let catalog = crate::catalog::effective_specs_cached(state).await; let name_of: std::collections::HashMap = snap .iter() .map(|r| r.spec_id.clone()) @@ -585,16 +656,11 @@ fn sse_no_buffer_headers() -> [(axum::http::HeaderName, &'static str); 2] { /// `no-store` so the browser always re-polls (freshness comes from the /// server-side cache, not the HTTP cache). async fn snapshot( - session: AdminSession, + scope: EditorScope, State(state): State, loc: Locale, ) -> Response { - // Same gate as the dashboard page (#857): a Viewer never gets the - // live metrics feed. - if !session.role.can_access_section("dashboard") { - return (StatusCode::FORBIDDEN, "forbidden").into_response(); - } - let snap = build_snapshot(&state, loc).await; + let snap = build_snapshot(&state, loc, &scope).await; ( [(axum::http::header::CACHE_CONTROL, "no-store")], axum::Json(snap), @@ -614,7 +680,7 @@ async fn snapshot( async fn logs( // Container logs can carry sensitive data — gate behind Editor+ // (Viewers can see the dashboard, not the logs). #261 - editor: RequireEditor, + scope: EditorScope, State(state): State, loc: Locale, theme: Theme, @@ -625,6 +691,11 @@ async fn logs( }; let rid = ReplicaId(uuid); + let (_, scoped_spec) = match scoped_replica_spec(&state, &scope, &rid).await { + Ok(found) => found, + Err(response) => return response, + }; + let Some(backend) = state.backend.as_ref() else { return ( StatusCode::SERVICE_UNAVAILABLE, @@ -636,18 +707,10 @@ async fn logs( // Resolve display_name + spec_id for the heading. Read the spec id // from the registry under the lock, then drop it before the DB-first // name lookup (#275) so we never hold the registry lock across I/O. - let spec_id_of_replica = { - let reg = state.replicas.read().await; - let found = reg.all().find(|r| r.id == rid).map(|r| r.spec_id.clone()); - found - }; - let (display_name, spec_id) = match spec_id_of_replica { - Some(sid) => { - let dn = crate::routes::proxy::find_spec(&state, &sid) - .await - .and_then(|s| s.display_name.clone()) - .unwrap_or_else(|| sid.clone()); - (dn, sid) + let (display_name, spec_id) = match scoped_spec { + Some(spec) => { + let display_name = spec.display_name.clone().unwrap_or_else(|| spec.id.clone()); + (display_name, spec.id) } // Replica not in registry — still try to fetch logs (it may have // just been dropped from the registry but the container lingers). @@ -674,7 +737,7 @@ async fn logs( locales_all: &Locale::ALL, base: state.base_path.clone(), nav_section: "dashboard", - role: editor.role, + role: scope.role, display_name, spec_id, replica_id, @@ -693,7 +756,7 @@ async fn logs( /// Seeded with the last 100 lines so turning Live on shows /// recent context, not just lines that arrive after connect. async fn logs_stream( - _: RequireEditor, + scope: EditorScope, State(state): State, Path(replica_id): Path, ) -> Response { @@ -701,6 +764,9 @@ async fn logs_stream( Ok(r) => r, Err(resp) => return *resp, }; + if let Err(response) = scoped_replica_spec(&state, &scope, &rid).await { + return response; + } let Some(backend) = state.backend.clone() else { return (StatusCode::SERVICE_UNAVAILABLE, "no backend").into_response(); }; @@ -731,6 +797,48 @@ fn parse_replica_id(s: &str) -> Result> { .map_err(|_| Box::new((StatusCode::BAD_REQUEST, "invalid replica id").into_response())) } +/// Resolve a replica through the effective app catalog and enforce its scope. +/// +/// A scoped Editor gets 404 both when the replica/spec is missing and when +/// the spec belongs to another group. This is intentionally checked before +/// backend access so a guessed UUID cannot reveal a foreign container through +/// a 503/transport error. Admin and break-glass callers remain unrestricted, +/// including the historical best-effort access to a just-orphaned container. +async fn scoped_replica_spec( + state: &AppState, + scope: &EditorScope, + replica_id: &ReplicaId, +) -> Result<(Option, Option), Response> { + let spec_id = { + let registry = state.replicas.read().await; + let found = registry + .all() + .find(|replica| replica.id == *replica_id) + .map(|replica| replica.spec_id.clone()); + found + }; + + let Some(spec_id) = spec_id else { + return if scope.unscoped { + Ok((None, None)) + } else { + Err((StatusCode::NOT_FOUND, "replica not found").into_response()) + }; + }; + let catalog = crate::catalog::effective_specs_cached(state).await; + let spec = catalog.iter().find(|spec| spec.id == spec_id).cloned(); + if !scope.unscoped + && !spec + .as_ref() + .is_some_and(|spec| scope.may_touch_spec(spec)) + { + // Missing effective metadata also fails closed; it does not make the + // replica's app "open". + return Err((StatusCode::NOT_FOUND, "replica not found").into_response()); + } + Ok((Some(spec_id), spec)) +} + /// POST `/admin/dashboard/replicas/{id}/stop` — stop a replica /// and drop it from the registry. The auto-scaler will respawn /// it to `min-replicas` on its next tick if the spec demands a @@ -741,7 +849,7 @@ fn parse_replica_id(s: &str) -> Result> { /// Redirects back to the dashboard so the browser lands on a /// fresh render. async fn stop_replica( - editor: RequireEditor, + scope: EditorScope, State(state): State, Path(replica_id): Path, ) -> Response { @@ -749,6 +857,9 @@ async fn stop_replica( Ok(r) => r, Err(resp) => return *resp, }; + if let Err(response) = scoped_replica_spec(&state, &scope, &rid).await { + return response; + } let Some(backend) = state.backend.as_ref() else { return (StatusCode::SERVICE_UNAVAILABLE, "no backend").into_response(); }; @@ -765,7 +876,7 @@ async fn stop_replica( state.sessions.drop_replica(&rid).await; // Destructive operational action → audit row (#745); config // mutations were audited, replica stop/restart only hit the log. - record_replica_action(&state, editor.actor(), "replica.stop", &replica_id).await; + record_replica_action(&state, scope.actor(), "replica.stop", &replica_id).await; tracing::info!(replica = %replica_id, "replica stopped via dashboard"); Redirect::to("/admin/dashboard").into_response() } @@ -787,7 +898,7 @@ async fn record_replica_action(state: &AppState, actor: &str, action: &str, repl /// respawn ~one tick later), restart brings capacity back /// right away. async fn restart_replica( - editor: RequireEditor, + scope: EditorScope, State(state): State, Path(replica_id): Path, ) -> Response { @@ -795,6 +906,10 @@ async fn restart_replica( Ok(r) => r, Err(resp) => return *resp, }; + let (spec_id, scoped_spec) = match scoped_replica_spec(&state, &scope, &rid).await { + Ok(found) => found, + Err(response) => return response, + }; let Some(backend) = state.backend.as_ref() else { return (StatusCode::SERVICE_UNAVAILABLE, "no backend").into_response(); }; @@ -802,17 +917,12 @@ async fn restart_replica( // Resolve the spec before stopping so a restart of a // since-deleted spec fails cleanly without first killing // the running container. - let spec_id = { - let reg = state.replicas.read().await; - let found = reg.all().find(|r| r.id == rid).map(|r| r.spec_id.clone()); - found - }; let Some(spec_id) = spec_id else { return (StatusCode::NOT_FOUND, "replica not found").into_response(); }; // DB-first (admin edits + showcase seed), not just the YAML config — // otherwise a DB-only spec's replica couldn't be restarted (#257). - let Some(spec) = crate::routes::proxy::find_spec(&state, &spec_id).await else { + let Some(spec) = scoped_spec else { return ( StatusCode::CONFLICT, format!("spec `{spec_id}` no longer exists; cannot restart"), @@ -836,7 +946,7 @@ async fn restart_replica( ) .into_response(); } - record_replica_action(&state, editor.actor(), "replica.restart", &replica_id).await; + record_replica_action(&state, scope.actor(), "replica.restart", &replica_id).await; tracing::info!(replica = %replica_id, spec = %spec.id, "replica restarted via dashboard"); Redirect::to("/admin/dashboard").into_response() } @@ -1179,6 +1289,42 @@ mod tests { assert!(html.contains(">2/10<"), "summed sessions on the head"); } + /// The snapshot cache is keyed by scope as well as locale (#990): two + /// Editors with the same groups must share an entry (that's the #291 + /// many-tabs guarantee), while a different group set — or the unscoped + /// Admin view — must never read someone else's rows. + #[test] + fn snapshot_cache_key_separates_scopes_but_shares_equal_ones() { + let scoped = |groups: &[&str]| EditorScope { + role: Role::Editor, + actor: Some("editor".into()), + groups: groups.iter().map(|g| (*g).to_string()).collect(), + unscoped: false, + }; + let admin = EditorScope { + role: Role::Admin, + actor: Some("admin".into()), + groups: Vec::new(), + unscoped: true, + }; + + assert_eq!(scope_key(&admin), None, "the unscoped view is one entry"); + assert_ne!(scope_key(&scoped(&["a"])), scope_key(&admin)); + assert_ne!(scope_key(&scoped(&["a"])), scope_key(&scoped(&["b"]))); + // Order and duplicates are not different scopes. + assert_eq!( + scope_key(&scoped(&["a", "b"])), + scope_key(&scoped(&["b", "a"])) + ); + assert_eq!(scope_key(&scoped(&["a", "a"])), scope_key(&scoped(&["a"]))); + // A group name containing the separator we join on can't forge + // another scope's key. + assert_ne!( + scope_key(&scoped(&["a", "b"])), + scope_key(&scoped(&["a,b"])) + ); + } + #[test] fn metric_cards_show_aggregated_counts() { let html = render_with( diff --git a/crates/ruscker-admin/src/routes/admin/spec_form.rs b/crates/ruscker-admin/src/routes/admin/spec_form.rs index 066041a..ca40e9a 100644 --- a/crates/ruscker-admin/src/routes/admin/spec_form.rs +++ b/crates/ruscker-admin/src/routes/admin/spec_form.rs @@ -28,9 +28,10 @@ use serde::{Deserialize, Serialize}; use serde_yaml_ng::Value as YamlValue; use std::collections::HashMap; -use crate::auth::{RequireEditor, Role}; +use crate::auth::Role; use crate::db; use crate::i18n::{Locale, Locales}; +use crate::scope::EditorScope; use crate::theme::Theme; use crate::view_model::DisplayType; use crate::AppState; @@ -78,7 +79,7 @@ struct ImageCheckResult { /// (#498). Pull-free and Editor-gated; a quick yes/no, no registry round /// trip (that's slice B's explicit Pull button). async fn image_check( - _: RequireEditor, + _scope: EditorScope, State(state): State, Query(q): Query, ) -> Json { @@ -166,7 +167,7 @@ const PULL_JOB_TTL_SECS: u64 = 300; /// editor then opens an EventSource on `…/image-pull/events?job=`. /// Editor-gated; the POST inherits the chrome CSRF (Fetch-Metadata) guard. async fn image_pull_start( - _: RequireEditor, + _scope: EditorScope, State(state): State, Form(q): Form, ) -> Response { @@ -277,7 +278,7 @@ async fn start_pull( /// `{ "job": "" }` for the row to follow over SSE. Editor-gated; /// inherits the chrome CSRF guard. async fn image_repull( - _: RequireEditor, + scope: EditorScope, State(state): State, Path(id): Path, ) -> Response { @@ -292,6 +293,11 @@ async fn image_repull( return (StatusCode::INTERNAL_SERVER_ERROR, "db error").into_response(); } }; + // A direct re-pull URL is an operation on the app, not on the shared + // media library. Hide a foreign app exactly like the edit/delete routes. + if !scope.may_touch_spec(&spec) { + return (StatusCode::NOT_FOUND, "no such app").into_response(); + } let Some(image) = spec .container_image .as_deref() @@ -330,7 +336,7 @@ struct PullEventsQuery { /// already-running pull's progress over SSE (default events = lines, then /// one terminal `done` event), then drops the job. An unknown or /// already-followed token ⇒ 404. Editor-gated (RBAC preserved). -async fn image_pull_events(_: RequireEditor, Query(q): Query) -> Response { +async fn image_pull_events(_scope: EditorScope, Query(q): Query) -> Response { use axum::http::header::{HeaderName, CACHE_CONTROL}; use axum::response::sse::{Event, KeepAlive, Sse}; @@ -393,7 +399,7 @@ async fn resolve_pull_creds( /// Mirror of the form fields. Strings are unconditional so empty /// inputs round-trip as `""` rather than disappearing; conversion /// to [`Spec`] handles "empty means None". -#[derive(Debug, Default, Serialize, Deserialize)] +#[derive(Clone, Debug, Default, Serialize, Deserialize)] #[serde(default)] pub struct SpecForm { pub id: String, @@ -1290,10 +1296,11 @@ impl<'a> SpecFormPage<'a> { /// wired or the query fails — the picker degrades to the text field. /// Distinct, sorted `template-properties.subject` values across the /// effective catalog (#746) — real data instead of a fixed list. -async fn subject_suggestions(state: &AppState) -> Vec { +async fn subject_suggestions(state: &AppState, scope: &EditorScope) -> Vec { let specs = crate::catalog::effective_specs(state.db.as_ref(), &state.config).await; let mut subjects: Vec = specs .iter() + .filter(|spec| scope.may_touch_spec(spec)) .filter_map(|sp| sp.template_properties.get_str("subject")) .map(|t| t.trim().to_string()) .filter(|t| !t.is_empty()) @@ -1332,7 +1339,7 @@ async fn credential_names(state: &AppState) -> Vec { /// every effective spec's `access-groups` and every user's memberships, /// sorted and de-duplicated. Empty when no DB is wired — the picker then /// just offers the "add group" input. -async fn group_names(state: &AppState) -> Vec { +async fn group_names(state: &AppState, scope: &EditorScope) -> Vec { let mut set = std::collections::BTreeSet::new(); for s in crate::catalog::effective_specs(state.db.as_ref(), &state.config).await { if let Some(groups) = s.access_groups.as_ref() { @@ -1346,11 +1353,15 @@ async fn group_names(state: &AppState) -> Vec { } } } - set.into_iter().collect() + let known: Vec = set.into_iter().collect(); + // The picker must not advertise another team's vocabulary. Reuse the + // scope merge primitive with an empty base to retain only assignable + // groups for Editors; Admins receive the full set unchanged. + scope.merge_preserving_out_of_scope(&[], &known) } async fn new_form( - editor: RequireEditor, + scope: EditorScope, State(state): State, loc: Locale, theme: Theme, @@ -1362,7 +1373,7 @@ async fn new_form( locales_all: &Locale::ALL, base: state.base_path.clone(), nav_section: "specs", - role: editor.role, + role: scope.role, mode: FormMode::New, form: SpecForm { // Sensible defaults for a new app @@ -1375,8 +1386,8 @@ async fn new_form( just_created: false, errors: Vec::new(), logo_images: logo_filenames(&state).await, - subject_suggestions: subject_suggestions(&state).await, - available_groups: group_names(&state).await, + subject_suggestions: subject_suggestions(&state, &scope).await, + available_groups: group_names(&state, &scope).await, credential_names: credential_names(&state).await, }; super::render(&page) @@ -1390,7 +1401,7 @@ struct EditFormQuery { } async fn edit_form( - editor: RequireEditor, + scope: EditorScope, State(state): State, loc: Locale, theme: Theme, @@ -1408,6 +1419,11 @@ async fn edit_form( return (StatusCode::INTERNAL_SERVER_ERROR, "db error").into_response(); } }; + // List filtering is not authorization. A foreign app deliberately + // returns 404 so its id is not disclosed through a direct URL (#990). + if !scope.may_touch_spec(&spec) { + return (StatusCode::NOT_FOUND, "spec not found").into_response(); + } let page = SpecFormPage { locale: loc, theme, @@ -1415,10 +1431,15 @@ async fn edit_form( locales_all: &Locale::ALL, base: state.base_path.clone(), nav_section: "specs", - role: editor.role, + role: scope.role, mode: FormMode::Edit, form: { let mut f = SpecForm::from_spec(&spec); + let visible_groups = scope.merge_preserving_out_of_scope( + &[], + spec.access_groups.as_deref().unwrap_or_default(), + ); + f.access_groups = visible_groups.join(", "); // Optimistic-concurrency token (#745): the template emits it // as a hidden field; `update` rejects a stale submit. f.base_version = db::specs::fetch_version(pool, &id) @@ -1432,8 +1453,8 @@ async fn edit_form( just_created: q.created.is_some(), errors: Vec::new(), logo_images: logo_filenames(&state).await, - subject_suggestions: subject_suggestions(&state).await, - available_groups: group_names(&state).await, + subject_suggestions: subject_suggestions(&state, &scope).await, + available_groups: group_names(&state, &scope).await, credential_names: credential_names(&state).await, }; super::render(&page) @@ -1480,7 +1501,7 @@ fn pick_copy_id(base: &str, taken: &std::collections::HashSet) -> String /// a handy way to fork a YAML-defined spec into an editable DB one. The /// registry password is write-only (#260), so it isn't carried over. async fn duplicate_form( - editor: RequireEditor, + scope: EditorScope, State(state): State, loc: Locale, theme: Theme, @@ -1510,7 +1531,18 @@ async fn duplicate_form( return (StatusCode::INTERNAL_SERVER_ERROR, "db error").into_response(); } }; + // Duplicating is still an operation on the source app. Return 404 at + // the boundary so a guessed foreign id is indistinguishable from one + // that does not exist. + if !scope.may_touch_spec(&spec) { + return (StatusCode::NOT_FOUND, "spec not found").into_response(); + } let mut form = SpecForm::from_spec(&spec); + let visible_groups = scope.merge_preserving_out_of_scope( + &[], + spec.access_groups.as_deref().unwrap_or_default(), + ); + form.access_groups = visible_groups.join(", "); form.id = unique_copy_id(&state, &spec.id).await; let page = SpecFormPage { locale: loc, @@ -1519,21 +1551,21 @@ async fn duplicate_form( locales_all: &Locale::ALL, base: state.base_path.clone(), nav_section: "specs", - role: editor.role, + role: scope.role, mode: FormMode::New, form, just_created: false, errors: Vec::new(), logo_images: logo_filenames(&state).await, - subject_suggestions: subject_suggestions(&state).await, - available_groups: group_names(&state).await, + subject_suggestions: subject_suggestions(&state, &scope).await, + available_groups: group_names(&state, &scope).await, credential_names: credential_names(&state).await, }; super::render(&page) } async fn create( - editor: RequireEditor, + scope: EditorScope, State(state): State, loc: Locale, theme: Theme, @@ -1545,6 +1577,20 @@ async fn create( let mut errors = form.validate(FormMode::New); + if errors.is_empty() { + let candidate = match form.clone().into_spec(None, scope.role) { + Ok(spec) => spec, + Err(e) => { + tracing::error!(error = ?e, "form → spec failed during scope validation"); + return (StatusCode::BAD_REQUEST, "invalid form data").into_response(); + } + }; + let requested = candidate.access_groups.as_deref().unwrap_or_default(); + if !scope.may_assign_groups(requested) || !scope.may_touch_spec(&candidate) { + errors.push("spec-form-error-editor-scope"); + } + } + // Uniqueness check if errors.is_empty() { match db::specs::fetch_one(pool, form.id.trim()).await { @@ -1562,7 +1608,7 @@ async fn create( &state, loc, theme, - editor.role, + &scope, FormMode::New, form, errors, @@ -1571,7 +1617,7 @@ async fn create( } let id = form.id.trim().to_string(); - let spec = match form.into_spec(None, editor.role) { + let spec = match form.into_spec(None, scope.role) { Ok(s) => s, Err(e) => { tracing::error!(error = ?e, "form → spec failed"); @@ -1582,7 +1628,7 @@ async fn create( // insert_new fails CLOSED on an existing id (#745) — the friendly // pre-check above is just UX; this is the race-proof gate (the old // upsert silently overwrote the loser of a concurrent create). - match db::specs::insert_new(pool, &spec, Some(editor.actor())).await { + match db::specs::insert_new(pool, &spec, Some(scope.actor())).await { // Land on the new app's edit form with `?created=1` so the page // shows the post-create confirmation (#835): keep editing here, or // jump to the apps list. @@ -1594,7 +1640,7 @@ async fn create( &state, loc, theme, - editor.role, + &scope, FormMode::New, SpecForm::from_spec(&spec), vec!["spec-form-error-id-duplicate"], @@ -1609,7 +1655,7 @@ async fn create( } async fn update( - editor: RequireEditor, + scope: EditorScope, State(state): State, loc: Locale, theme: Theme, @@ -1625,13 +1671,57 @@ async fn update( // log target). Renaming is a separate planned action. form.id = id.clone(); - let errors = form.validate(FormMode::Edit); + // Load the existing spec as the merge base so fields the form + // doesn't model (registry creds, lifetimes, limits, scaling, custom + // template-properties) survive the edit instead of being wiped. + let base = match db::specs::fetch_one(pool, &id).await { + Ok(Some(base)) => base, + Ok(None) => { + return (StatusCode::NOT_FOUND, format!("spec `{id}` not found")).into_response(); + } + Err(e) => { + tracing::error!(error = ?e, id, "load base spec failed"); + return (StatusCode::INTERNAL_SERVER_ERROR, "db error").into_response(); + } + }; + // This is the *edit* path — the spec must already exist (#261). Check + // scope before validation so even a malformed crafted POST cannot use + // response differences to confirm an out-of-scope app id (#990). + if !scope.may_touch_spec(&base) { + return (StatusCode::NOT_FOUND, "spec not found").into_response(); + } + + let mut errors = form.validate(FormMode::Edit); + let mut candidate = if errors.is_empty() { + match form.clone().into_spec(Some(&base), scope.role) { + Ok(spec) => Some(spec), + Err(e) => { + tracing::error!(error = ?e, "form → spec failed during scope validation"); + return (StatusCode::BAD_REQUEST, "invalid form data").into_response(); + } + } + } else { + None + }; + if let Some(spec) = candidate.as_mut() { + let requested = spec.access_groups.clone().unwrap_or_default(); + if !scope.may_assign_groups(&requested) { + errors.push("spec-form-error-editor-scope"); + } else { + let existing = base.access_groups.as_deref().unwrap_or_default(); + let merged = scope.merge_preserving_out_of_scope(existing, &requested); + spec.access_groups = (!merged.is_empty()).then_some(merged); + if !scope.may_touch_spec(spec) { + errors.push("spec-form-error-editor-scope"); + } + } + } if !errors.is_empty() { return render_form_with_errors( &state, loc, theme, - editor.role, + &scope, FormMode::Edit, form, errors, @@ -1639,24 +1729,6 @@ async fn update( .await; } - // Load the existing spec as the merge base so fields the form - // doesn't model (registry creds, lifetimes, limits, scaling, custom - // template-properties) survive the edit instead of being wiped. - let base = match db::specs::fetch_one(pool, &id).await { - Ok(b) => b, - Err(e) => { - tracing::error!(error = ?e, id, "load base spec failed"); - return (StatusCode::INTERNAL_SERVER_ERROR, "db error").into_response(); - } - }; - // This is the *edit* path — the spec must already exist. Without - // this guard `into_spec(None)` + `upsert_one` would silently - // (re)create a spec at this id (e.g. a stale tab POSTing to a - // since-deleted spec). #261 - if base.is_none() { - return (StatusCode::NOT_FOUND, format!("spec `{id}` not found")).into_response(); - } - // Optimistic concurrency (#745): the form carries the version it // was rendered against; if someone saved meanwhile, re-render with // a conflict error instead of silently last-write-winning over @@ -1671,7 +1743,7 @@ async fn update( &state, loc, theme, - editor.role, + &scope, FormMode::Edit, stale, vec!["spec-form-error-stale"], @@ -1680,15 +1752,9 @@ async fn update( } } - let spec = match form.into_spec(base.as_ref(), editor.role) { - Ok(s) => s, - Err(e) => { - tracing::error!(error = ?e, "form → spec failed"); - return (StatusCode::BAD_REQUEST, "invalid form data").into_response(); - } - }; + let spec = candidate.expect("validated form always builds a candidate spec"); - match db::specs::upsert_one(pool, &spec, Some(editor.actor())).await { + match db::specs::upsert_one(pool, &spec, Some(scope.actor())).await { Ok(_) => Redirect::to(&format!("/admin/specs/{}/edit", id)).into_response(), Err(e) => { tracing::error!(error = ?e, "save failed"); @@ -1698,14 +1764,26 @@ async fn update( } async fn delete( - editor: RequireEditor, + scope: EditorScope, State(state): State, Path(id): Path, ) -> Response { let Some(pool) = state.db.as_ref() else { return (StatusCode::SERVICE_UNAVAILABLE, "no db").into_response(); }; - match db::specs::delete_one(pool, &id, Some(editor.actor())).await { + let spec = match db::specs::fetch_one(pool, &id).await { + Ok(Some(spec)) => spec, + Ok(None) => return (StatusCode::NOT_FOUND, "spec not found").into_response(), + Err(e) => { + tracing::error!(error = ?e, id, "load spec for delete failed"); + return (StatusCode::INTERNAL_SERVER_ERROR, "db error").into_response(); + } + }; + if !scope.may_touch_spec(&spec) { + // 404 avoids disclosing that a guessed foreign id exists. + return (StatusCode::NOT_FOUND, "spec not found").into_response(); + } + match db::specs::delete_one(pool, &id, Some(scope.actor())).await { Ok(_) => { // Reap the app's containers so a delete doesn't leave orphans // eating disk (#453). Best-effort and logged inside; the DB row @@ -1724,7 +1802,7 @@ async fn render_form_with_errors( state: &AppState, loc: Locale, theme: Theme, - role: Role, + scope: &EditorScope, mode: FormMode, form: SpecForm, errors: Vec<&'static str>, @@ -1736,14 +1814,14 @@ async fn render_form_with_errors( locales_all: &Locale::ALL, base: state.base_path.clone(), nav_section: "specs", - role, + role: scope.role, mode, form, just_created: false, errors, logo_images: logo_filenames(state).await, - subject_suggestions: subject_suggestions(state).await, - available_groups: group_names(state).await, + subject_suggestions: subject_suggestions(state, scope).await, + available_groups: group_names(state, scope).await, credential_names: credential_names(state).await, }; let body = match page.render() { diff --git a/crates/ruscker-admin/src/routes/admin/specs.rs b/crates/ruscker-admin/src/routes/admin/specs.rs index 4d1690a..6b179b2 100644 --- a/crates/ruscker-admin/src/routes/admin/specs.rs +++ b/crates/ruscker-admin/src/routes/admin/specs.rs @@ -16,8 +16,9 @@ use chrono::{DateTime, Utc}; use serde::{Deserialize, Serialize}; use sqlx::FromRow; -use crate::auth::{RequireEditor, Role}; +use crate::auth::Role; use crate::i18n::{Locale, Locales}; +use crate::scope::EditorScope; use crate::theme::Theme; use crate::AppState; @@ -51,7 +52,7 @@ struct ToggleResult { /// flag from the Apps table's star (#521), without opening the editor. /// Editor-gated; returns the new state as JSON for the optimistic UI. async fn toggle_featured( - editor: RequireEditor, + scope: EditorScope, State(state): State, Path(id): Path, ) -> Response { @@ -66,10 +67,15 @@ async fn toggle_featured( return (StatusCode::INTERNAL_SERVER_ERROR, "db error").into_response(); } }; + // Out-of-scope specs deliberately look absent: returning 403 here would + // confirm an app id that the Editor is not allowed to discover (#990). + if !scope.may_touch_spec(&spec) { + return (StatusCode::NOT_FOUND, "spec not found").into_response(); + } let now_featured = !spec.is_featured(); // None when off so a normal spec carries no `featured` noise in JSON. spec.featured = now_featured.then_some(true); - if let Err(e) = crate::db::specs::upsert_one(db, &spec, Some(editor.actor())).await { + if let Err(e) = crate::db::specs::upsert_one(db, &spec, Some(scope.actor())).await { tracing::error!(id, error = ?e, "save featured toggle failed"); return (StatusCode::INTERNAL_SERVER_ERROR, "save failed").into_response(); } @@ -95,7 +101,7 @@ struct StateToggleResult { /// toggle (#787 — the original form POST + redirect reloaded the page /// and threw the scroll back to the top). async fn toggle_state( - editor: RequireEditor, + scope: EditorScope, State(state): State, Path(id): Path, ) -> Response { @@ -110,13 +116,18 @@ async fn toggle_state( return (StatusCode::INTERNAL_SERVER_ERROR, "db error").into_response(); } }; + // Hide existence across Editor group boundaries; list filtering is only + // presentation and direct URLs must be protected independently (#990). + if !scope.may_touch_spec(&spec) { + return (StatusCode::NOT_FOUND, "spec not found").into_response(); + } let next_active = !spec.template_properties.is_active(); // `set_state`, not `upsert_one` (#780): the list sorts by // `updated_at` DESC and the upsert path stamps it, so archiving made // the row jump to the top of the table. The dedicated writer leaves // `updated_at` alone (a visibility flip is not a content edit) while // still bumping the version and auditing archive/unarchive. - if let Err(e) = crate::db::specs::set_state(db, &id, next_active, Some(editor.actor())).await { + if let Err(e) = crate::db::specs::set_state(db, &id, next_active, Some(scope.actor())).await { tracing::error!(id, error = ?e, "save state toggle failed"); return (StatusCode::INTERNAL_SERVER_ERROR, "save failed").into_response(); } @@ -261,8 +272,11 @@ struct ImportRow { async fn build_import_rows( pool: &crate::db::ConfigDb, raw: &str, + scope: &EditorScope, + effective_specs: &[ruscker_config::Spec], ) -> Result<(Vec, usize), String> { - let config = ruscker_config::Config::from_yaml(raw).map_err(|e| format!("{e}"))?; + let config = ruscker_config::Config::from_yaml(raw) + .map_err(|e| format!("YAML parse failed: {e}"))?; let report = ruscker_config::validate::run(&config); let warning_count = report.warnings.len() + config.raw_warnings.len(); let existing: std::collections::HashSet = crate::db::specs::list_all(pool) @@ -271,25 +285,101 @@ async fn build_import_rows( .into_iter() .map(|s| s.id) .collect(); - let rows = config - .proxy - .specs - .iter() - .map(|s| ImportRow { - exists: existing.contains(&s.id), - kind: match s.kind() { + let mut rows = Vec::with_capacity(config.proxy.specs.len()); + for spec in &config.proxy.specs { + if existing.contains(&spec.id) { + // A persisted id that disappeared from the effective catalog is + // not an open app. Scoped Editors fail closed because the lean + // existence query carries no ACL metadata (#990). + match effective_specs.iter().find(|current| current.id == spec.id) { + Some(current) if !scope.may_touch_spec(current) => { + return Err(format!("app `{}` is outside your Editor scope", spec.id)); + } + None if !scope.unscoped => { + return Err(format!("app `{}` is outside your Editor scope", spec.id)); + } + _ => {} + } + } else { + let requested = spec.access_groups.as_deref().unwrap_or_default(); + if !scope.may_assign_groups(requested) + || (!spec.is_open() && !scope.may_touch_spec(spec)) + { + return Err(format!( + "restricted app `{}` must use only groups in your Editor scope", + spec.id + )); + } + } + rows.push(ImportRow { + exists: existing.contains(&spec.id), + kind: match spec.kind() { ruscker_config::SpecKind::Shiny => "shiny", ruscker_config::SpecKind::InteractiveApp => "interactive", ruscker_config::SpecKind::Api => "api", ruscker_config::SpecKind::External => "external", }, - display_name: s.display_name.clone().unwrap_or_default(), - id: s.id.clone(), - }) - .collect(); + display_name: spec.display_name.clone().unwrap_or_default(), + id: spec.id.clone(), + }); + } Ok((rows, warning_count)) } +/// Apply Editor scope to the selected YAML specs immediately before import. +/// +/// The preview is only UX; a crafted confirm POST must pass this same +/// server-side gate. Existing foreign memberships are retained through the +/// slice-0 merge primitive, while a new/restricted app must stay reachable +/// to its creator and may name only groups the Editor owns. +fn prepare_scoped_import( + config: &mut ruscker_config::Config, + ids: &[String], + persisted_ids: &std::collections::HashSet, + effective_specs: &[ruscker_config::Spec], + scope: &EditorScope, +) -> Result<(), String> { + use std::collections::HashSet; + + let selected: HashSet<&str> = ids.iter().map(String::as_str).collect(); + for spec in config + .proxy + .specs + .iter_mut() + .filter(|spec| selected.contains(spec.id.as_str())) + { + let current = effective_specs.iter().find(|current| current.id == spec.id); + if persisted_ids.contains(&spec.id) && current.is_none() && !scope.unscoped { + // A DB row without effective ACL metadata is not a new/open app. + // Fail closed before import_selected can overwrite it. + return Err(format!("app `{}` is outside your Editor scope", spec.id)); + } + if current.is_some_and(|current| !scope.may_touch_spec(current)) { + return Err(format!("app `{}` is outside your Editor scope", spec.id)); + } + + let requested = spec.access_groups.clone().unwrap_or_default(); + if !scope.may_assign_groups(&requested) { + return Err(format!( + "restricted app `{}` may only use groups in your Editor scope", + spec.id + )); + } + if let Some(current) = current { + let existing = current.access_groups.as_deref().unwrap_or_default(); + let merged = scope.merge_preserving_out_of_scope(existing, &requested); + spec.access_groups = (!merged.is_empty()).then_some(merged); + } + if !spec.is_open() && !scope.may_touch_spec(spec) { + return Err(format!( + "restricted app `{}` must retain at least one of your groups", + spec.id + )); + } + } + Ok(()) +} + /// JSON shape returned by `POST /admin/specs/import/preview` for the live /// editor: either an error (parse failure) or the parsed rows + warnings. #[derive(Serialize)] @@ -394,7 +484,7 @@ fn build_flash(locales: &Locales, loc: Locale, q: &SpecsQuery) -> Option } async fn index( - editor: RequireEditor, + scope: EditorScope, State(state): State, loc: Locale, theme: Theme, @@ -444,9 +534,9 @@ async fn index( // Card logo + access-groups per spec for the table (#623). The effective // catalog deserializes each spec once — fine here (admin page, not the // proxy/dashboard hot path the lean SELECT of #588 protects). + let catalog = crate::catalog::effective_specs_cached(&state).await; let meta: std::collections::HashMap, Vec, bool)> = - crate::catalog::effective_specs_cached(&state) - .await + catalog .iter() .map(|s| { let logo = s.template_properties.get_str("logo").map(str::to_string); @@ -457,6 +547,19 @@ async fn index( }) .collect(); + if !scope.unscoped { + specs.retain(|row| { + // IMPORTANT: never authorize against the enriched `SpecRow`. + // A lean-SELECT row missing from the effective catalog has empty + // `access_groups` and would look open. A scoped Editor therefore + // fails closed unless the id resolves to the authoritative Spec. + catalog + .iter() + .find(|spec| spec.id == row.id) + .is_some_and(|spec| scope.may_touch_spec(spec)) + }); + } + // Per-row access total + sparkline (#549). `featured` already came from // the lean SELECT (#588), so there's no second config_json deserialize. for row in &mut specs { @@ -481,6 +584,16 @@ async fn index( if db_ids.contains(&s.id) { continue; } + if !scope.unscoped + && !catalog + .iter() + .find(|spec| spec.id == s.id) + .is_some_and(|spec| scope.may_touch_spec(spec)) + { + // Missing effective metadata also fails closed here. Treating + // the row's empty fallback ACL as "open" would leak it. + continue; + } let kind = match s.kind() { ruscker_config::SpecKind::Shiny => "shiny", ruscker_config::SpecKind::InteractiveApp => "interactive", @@ -527,7 +640,7 @@ async fn index( locales_all: &Locale::ALL, base: state.base_path.clone(), nav_section: "specs", - role: editor.role, + role: scope.role, specs, kpi_total, kpi_active, @@ -553,7 +666,7 @@ async fn index( /// with a checkbox — so the operator confirms which to import. Nothing is /// written to the DB here. async fn import( - editor: RequireEditor, + scope: EditorScope, State(state): State, loc: Locale, theme: Theme, @@ -588,10 +701,12 @@ async fn import( }; // Parse + diff via the shared core (#623). Parse failure → error flash. - let (rows, warning_count) = match build_import_rows(pool, &raw).await { - Ok(v) => v, - Err(e) => return redirect_err(&format!("YAML parse failed: {e}")), - }; + let effective_specs = crate::catalog::effective_specs_cached(&state).await; + let (rows, warning_count) = + match build_import_rows(pool, &raw, &scope, &effective_specs).await { + Ok(v) => v, + Err(e) => return redirect_err(&e), + }; let page = ImportPreviewPage { locale: loc, @@ -600,7 +715,7 @@ async fn import( locales_all: &Locale::ALL, base: state.base_path.clone(), nav_section: "specs", - role: editor.role, + role: scope.role, rows, raw_yaml: raw, warning_count, @@ -610,7 +725,7 @@ async fn import( /// `GET /admin/specs/import` — the live 2-pane import editor (#623). async fn import_editor( - editor: RequireEditor, + scope: EditorScope, State(state): State, loc: Locale, theme: Theme, @@ -622,7 +737,7 @@ async fn import_editor( locales_all: &Locale::ALL, base: state.base_path.clone(), nav_section: "specs", - role: editor.role, + role: scope.role, }; super::render(&page) } @@ -637,7 +752,7 @@ struct PreviewForm { } async fn import_preview( - _: RequireEditor, + scope: EditorScope, State(state): State, axum::Form(form): axum::Form, ) -> Response { @@ -655,7 +770,8 @@ async fn import_preview( }) .into_response(); } - match build_import_rows(pool, &form.yaml).await { + let effective_specs = crate::catalog::effective_specs_cached(&state).await; + match build_import_rows(pool, &form.yaml, &scope, &effective_specs).await { Ok((rows, warning_count)) => { let update_count = rows.iter().filter(|r| r.exists).count(); let new_count = rows.len() - update_count; @@ -685,7 +801,7 @@ async fn import_preview( /// preview form re-posts the original YAML (hidden) plus one `ids` field /// per selected spec, as multipart so repeated `ids` collect cleanly. async fn import_confirm( - editor: RequireEditor, + scope: EditorScope, State(state): State, mut multipart: Multipart, ) -> Response { @@ -733,15 +849,28 @@ async fn import_confirm( Ok(c) => c, Err(e) => return redirect_err(&format!("YAML parse failed: {e}")), }; + let effective_specs = crate::catalog::effective_specs_cached(&state).await; + let persisted_ids = match crate::db::specs::list_all(pool).await { + Ok(specs) => specs.into_iter().map(|spec| spec.id).collect(), + Err(error) => { + tracing::error!(error = ?error, "load catalog ids for scoped import failed"); + return redirect_err("could not verify Editor scope"); + } + }; + if let Err(error) = + prepare_scoped_import(&mut config, &ids, &persisted_ids, &effective_specs, &scope) + { + return redirect_err(&error); + } // #560 B: lift inline registry passwords into the encrypted credential // store and rewire the selected specs to reference them by name, so the // plaintext never lands in `config_json`. Runs before the spec upsert // (it mutates the specs that get imported). Best-effort. - let creds = extract_inline_credentials(&state, &mut config, &ids, editor.actor()).await; + let creds = extract_inline_credentials(&state, &mut config, &ids, scope.actor()).await; match crate::db::specs::import_selected(pool, &config, &ids).await { Ok(r) => { // #560 A: bring the specs' --images-dir logos into the Media library. - let logos = import_referenced_logos(&state, &config, &ids, editor.actor()).await; + let logos = import_referenced_logos(&state, &config, &ids, scope.actor()).await; tracing::info!( created = r.created, updated = r.updated, unchanged = r.unchanged, selected = ids.len(), credentials = creds, logos = logos, diff --git a/crates/ruscker-admin/src/scope.rs b/crates/ruscker-admin/src/scope.rs index c268ca9..5f761aa 100644 --- a/crates/ruscker-admin/src/scope.rs +++ b/crates/ruscker-admin/src/scope.rs @@ -39,7 +39,7 @@ pub struct EditorScope { } impl EditorScope { - async fn from_editor(editor: RequireEditor, db: Option<&ConfigDb>) -> Self { + pub(crate) async fn from_editor(editor: RequireEditor, db: Option<&ConfigDb>) -> Self { if editor.role == Role::Admin { return Self { role: editor.role, @@ -73,6 +73,14 @@ impl EditorScope { } } + /// Audit actor carried by the authenticated session. + /// + /// The break-glass token has no account username, so it keeps the same + /// stable `token` label used by [`RequireEditor::actor`]. + pub fn actor(&self) -> &str { + self.actor.as_deref().unwrap_or("token") + } + fn has_group(&self, group: &str) -> bool { self.groups.iter().any(|owned| owned == group) } diff --git a/crates/ruscker-admin/tests/rbac.rs b/crates/ruscker-admin/tests/rbac.rs index 216aaad..413c3b2 100644 --- a/crates/ruscker-admin/tests/rbac.rs +++ b/crates/ruscker-admin/tests/rbac.rs @@ -11,11 +11,15 @@ //! before the handler runs, and an *unauthenticated* request is //! redirected to the login form. We assert on those three shapes. -use axum::body::Body; +use axum::body::{to_bytes, Body}; use axum::http::{Request, StatusCode}; +use axum::response::Response; +use chrono::Utc; use ruscker_admin::auth::{AdminAuth, Role, COOKIE_NAME}; use ruscker_admin::{router, AppState}; -use ruscker_config::Config; +use ruscker_config::{Config, Spec}; +use ruscker_core::{Replica, ReplicaId, ReplicaState}; +use std::net::SocketAddr; use std::sync::Arc; use tower::ServiceExt; @@ -77,13 +81,146 @@ async fn cookie_for(state: &AppState, role: Role) -> String { } async fn send(state: AppState, method: &str, uri: &str, cookie: Option<&str>) -> StatusCode { + send_request(state, method, uri, cookie, Body::empty(), None) + .await + .status() +} + +async fn send_request( + state: AppState, + method: &str, + uri: &str, + cookie: Option<&str>, + body: Body, + content_type: Option<&str>, +) -> Response { let app = router(state); let mut builder = Request::builder().method(method).uri(uri); if let Some(c) = cookie { builder = builder.header("cookie", c); } - let req = builder.body(Body::empty()).unwrap(); - app.oneshot(req).await.unwrap().status() + if let Some(value) = content_type { + builder = builder.header("content-type", value); + } + let req = builder.body(body).unwrap(); + app.oneshot(req).await.unwrap() +} + +async fn response_body(response: Response) -> String { + let bytes = to_bytes(response.into_body(), 2 * 1024 * 1024) + .await + .expect("read response body"); + String::from_utf8(bytes.to_vec()).expect("response body is utf-8") +} + +const TIME_A_REPLICA: &str = "aaaaaaaa-1111-4111-8111-111111111111"; +const TIME_B_REPLICA: &str = "bbbbbbbb-2222-4222-8222-222222222222"; +const OPEN_REPLICA: &str = "cccccccc-3333-4333-8333-333333333333"; + +fn replica(id: &str, spec_id: &str) -> Replica { + Replica { + id: ReplicaId(uuid::Uuid::parse_str(id).expect("valid replica id")), + spec_id: spec_id.to_string(), + container_id: format!("container-{spec_id}"), + upstream: "127.0.0.1:3838" + .parse::() + .expect("valid upstream"), + state: ReplicaState::Ready, + started_at: Utc::now(), + sessions_active: 0, + sessions_max: 1, + host: None, + } +} + +fn app(yaml: &str) -> Spec { + serde_yaml_ng::from_str(yaml).expect("parse scoped test app") +} + +/// Real SQLite catalog + real opaque account session for the #990 scope +/// integration tests. `time-a` deliberately also carries `legacy-ops`: +/// the Editor shares one group and may edit it, but must not erase the +/// foreign membership that is hidden from their form. +async fn scoped_state() -> (AppState, ruscker_admin::db::ConfigDb) { + let mut state = state(); + state.config = Arc::new( + Config::from_yaml("proxy:\n title: Scoped test\n specs: []\n") + .expect("parse empty scoped config"), + ); + let path = + std::env::temp_dir().join(format!("ruscker-rbac-scope-{}.db", uuid::Uuid::new_v4())); + let pool = ruscker_admin::db::open(&path).await.expect("open scoped db"); + let db = ruscker_admin::db::ConfigDb::Sqlite(pool); + // `db::open` seeds the product showcase. This fixture needs a closed, + // exact three-app catalog so row/KPI assertions document scope precisely. + for seeded in ruscker_admin::db::specs::list_all(&db) + .await + .expect("list seeded showcase") + { + ruscker_admin::db::specs::delete_one(&db, &seeded.id, Some("test-reset")) + .await + .expect("remove seeded showcase app"); + } + for spec in [ + app( + "id: time-a\n\ + display-name: Time A\n\ + container-image: org/time-a:1\n\ + access-groups: [time-a, legacy-ops]\n", + ), + app( + "id: time-b\n\ + display-name: Time B\n\ + container-image: org/time-b:1\n\ + access-groups: [time-b]\n", + ), + app( + "id: open-app\n\ + display-name: Open App\n\ + container-image: org/open:1\n", + ), + ] { + ruscker_admin::db::specs::upsert_one(&db, &spec, Some("seed")) + .await + .expect("seed scoped app"); + } + ruscker_admin::db::users::create( + &db, + "editor-a", + "EditorPass9!", + Role::Editor, + false, + &["time-a".to_string()], + Some("seed"), + ) + .await + .expect("create scoped Editor"); + { + let mut registry = state.replicas.write().await; + registry.add(replica(TIME_A_REPLICA, "time-a")); + registry.add(replica(TIME_B_REPLICA, "time-b")); + registry.add(replica(OPEN_REPLICA, "open-app")); + } + state.db = Some(db.clone()); + (state, db) +} + +async fn scoped_cookie(state: &AppState, role: Role, actor: Option<&str>) -> String { + let id = state + .admin_sessions + .create(role, actor.map(str::to_string)) + .await; + format!("{COOKIE_NAME}={id}") +} + +fn metric_values(body: &str) -> Vec<&str> { + const OPEN: &str = "
"; + body.match_indices(OPEN) + .filter_map(|(start, _)| { + let value = &body[start + OPEN.len()..]; + value.find("
").map(|end| value[..end].trim()) + }) + .collect() } // ── Viewer: no panel — portal authenticated-user role (#857) ───────── @@ -296,3 +433,327 @@ async fn following_an_unknown_pull_job_is_404_for_editor() { "unknown pull token ⇒ 404, never a side effect" ); } + +// ── Editor group scope over applications (#990 slice 1) ───────────── + +#[tokio::test] +async fn scoped_editor_lists_only_shared_and_open_apps_with_matching_kpis() { + let (state, _db) = scoped_state().await; + let cookie = scoped_cookie(&state, Role::Editor, Some("editor-a")).await; + + let response = send_request( + state.clone(), + "GET", + "/admin/specs", + Some(&cookie), + Body::empty(), + None, + ) + .await; + assert_eq!(response.status(), StatusCode::OK); + let body = response_body(response).await; + assert!(body.contains("data-id=\"time-a\""), "shared app is listed"); + assert!(body.contains("data-id=\"open-app\""), "open app is global"); + assert!( + !body.contains("data-id=\"time-b\""), + "foreign-team app stays hidden" + ); + let listed = body.matches("2" + ), + "container KPI must equal the filtered replica grid" + ); +} + +#[tokio::test] +async fn scoped_editor_gets_404_on_every_foreign_app_or_replica_id_route() { + let (state, _db) = scoped_state().await; + let cookie = scoped_cookie(&state, Role::Editor, Some("editor-a")).await; + + // Every current app route carrying `{id}` is enumerated here. Filtering + // the list alone is not authorization: a typed/guessed URL must fail. + for (method, uri) in [ + ("GET", "/admin/specs/time-b/edit"), + ("GET", "/admin/specs/time-b/duplicate"), + ("POST", "/admin/specs/time-b"), + ("POST", "/admin/specs/time-b/delete"), + ("POST", "/admin/specs/time-b/featured/toggle"), + ("POST", "/admin/specs/time-b/state/toggle"), + ("POST", "/admin/specs/time-b/repull"), + ] { + let response = send_request( + state.clone(), + method, + uri, + Some(&cookie), + Body::empty(), + (method == "POST").then_some("application/x-www-form-urlencoded"), + ) + .await; + assert_eq!( + response.status(), + StatusCode::NOT_FOUND, + "foreign app route must be 404: {method} {uri}" + ); + } + + // Replica ids are just another path to the owning spec. Logs, live logs, + // stop and restart all resolve the effective Spec before backend access. + for (method, uri) in [ + ( + "GET", + "/admin/dashboard/logs/bbbbbbbb-2222-4222-8222-222222222222", + ), + ( + "GET", + "/admin/dashboard/logs/bbbbbbbb-2222-4222-8222-222222222222/stream", + ), + ( + "POST", + "/admin/dashboard/replicas/bbbbbbbb-2222-4222-8222-222222222222/stop", + ), + ( + "POST", + "/admin/dashboard/replicas/bbbbbbbb-2222-4222-8222-222222222222/restart", + ), + ] { + let status = send(state.clone(), method, uri, Some(&cookie)).await; + assert_eq!( + status, + StatusCode::NOT_FOUND, + "foreign replica route must be 404: {method} {uri}" + ); + } +} + +#[tokio::test] +async fn admin_remains_unscoped_for_foreign_apps_and_replicas() { + let (state, db) = scoped_state().await; + let cookie = scoped_cookie(&state, Role::Admin, Some("admin")).await; + + let response = send_request( + state.clone(), + "GET", + "/admin/specs", + Some(&cookie), + Body::empty(), + None, + ) + .await; + assert_eq!(response.status(), StatusCode::OK); + let body = response_body(response).await; + for id in ["time-a", "time-b", "open-app"] { + assert!(body.contains(&format!("data-id=\"{id}\"")), "Admin sees {id}"); + } + assert_eq!(metric_values(&body).first().copied(), Some("3")); + + assert_eq!( + send( + state.clone(), + "GET", + "/admin/specs/time-b/edit", + Some(&cookie), + ) + .await, + StatusCode::OK + ); + assert_eq!( + send( + state.clone(), + "GET", + "/admin/specs/time-b/duplicate", + Some(&cookie), + ) + .await, + StatusCode::OK + ); + assert_eq!( + send( + state.clone(), + "POST", + "/admin/specs/time-b/featured/toggle", + Some(&cookie), + ) + .await, + StatusCode::OK + ); + assert_eq!( + send( + state.clone(), + "POST", + "/admin/specs/time-b/state/toggle", + Some(&cookie), + ) + .await, + StatusCode::OK + ); + for uri in [ + "/admin/specs/time-b/repull", + "/admin/dashboard/replicas/bbbbbbbb-2222-4222-8222-222222222222/stop", + "/admin/dashboard/replicas/bbbbbbbb-2222-4222-8222-222222222222/restart", + ] { + assert_eq!( + send(state.clone(), "POST", uri, Some(&cookie)).await, + StatusCode::SERVICE_UNAVAILABLE, + "Admin passes scope and reaches the intentionally absent backend: {uri}" + ); + } + + let response = send_request( + state.clone(), + "POST", + "/admin/specs/time-b", + Some(&cookie), + Body::from( + "display_name=Time+B+admin&display_type=app&state=active&\ + container_image=org%2Ftime-b%3A2&inject_base_href=on&access_groups=time-b", + ), + Some("application/x-www-form-urlencoded"), + ) + .await; + assert_eq!(response.status(), StatusCode::SEE_OTHER); + assert_eq!( + ruscker_admin::db::specs::fetch_one(&db, "time-b") + .await + .unwrap() + .unwrap() + .display_name + .as_deref(), + Some("Time B admin") + ); + + assert_eq!( + send( + state, + "POST", + "/admin/specs/time-b/delete", + Some(&cookie), + ) + .await, + StatusCode::SEE_OTHER, + "Admin can delete the foreign-team app" + ); + assert!( + ruscker_admin::db::specs::fetch_one(&db, "time-b") + .await + .unwrap() + .is_none() + ); +} + +#[tokio::test] +async fn scoped_editor_cannot_create_restricted_app_with_foreign_group() { + let (state, db) = scoped_state().await; + let cookie = scoped_cookie(&state, Role::Editor, Some("editor-a")).await; + let response = send_request( + state, + "POST", + "/admin/specs", + Some(&cookie), + Body::from( + "id=foreign-new&display_name=Foreign+new&display_type=app&state=active&\ + container_image=org%2Fforeign%3A1&inject_base_href=on&access_groups=time-b", + ), + Some("application/x-www-form-urlencoded"), + ) + .await; + assert_eq!(response.status(), StatusCode::UNPROCESSABLE_ENTITY); + let body = response_body(response).await; + assert!( + body.contains("fora do seu escopo de Editor"), + "localized scope validation must explain the rejection" + ); + assert!( + ruscker_admin::db::specs::fetch_one(&db, "foreign-new") + .await + .unwrap() + .is_none(), + "rejected app must not be persisted" + ); +} + +#[tokio::test] +async fn scoped_editor_cannot_assign_foreign_group_and_edit_preserves_it() { + let (state, db) = scoped_state().await; + let cookie = scoped_cookie(&state, Role::Editor, Some("editor-a")).await; + + let edit = send_request( + state.clone(), + "GET", + "/admin/specs/time-a/edit", + Some(&cookie), + Body::empty(), + None, + ) + .await; + assert_eq!(edit.status(), StatusCode::OK); + let edit_body = response_body(edit).await; + assert!( + !edit_body.contains("legacy-ops"), + "foreign memberships are preserved server-side, not exposed as editable pills" + ); + + let rejected = send_request( + state.clone(), + "POST", + "/admin/specs/time-a", + Some(&cookie), + Body::from( + "display_name=Time+A&display_type=app&state=active&\ + container_image=org%2Ftime-a%3A1&inject_base_href=on&access_groups=time-b", + ), + Some("application/x-www-form-urlencoded"), + ) + .await; + assert_eq!(rejected.status(), StatusCode::UNPROCESSABLE_ENTITY); + + let saved = send_request( + state, + "POST", + "/admin/specs/time-a", + Some(&cookie), + Body::from( + "display_name=Time+A+edited&display_type=app&state=active&\ + container_image=org%2Ftime-a%3A2&inject_base_href=on&access_groups=time-a", + ), + Some("application/x-www-form-urlencoded"), + ) + .await; + assert_eq!(saved.status(), StatusCode::SEE_OTHER); + let spec = ruscker_admin::db::specs::fetch_one(&db, "time-a") + .await + .unwrap() + .unwrap(); + assert_eq!(spec.display_name.as_deref(), Some("Time A edited")); + assert_eq!( + spec.access_groups.as_deref(), + Some(&["legacy-ops".to_string(), "time-a".to_string()][..]), + "the out-of-scope group survives the Editor's replacement save" + ); +}