From 257ce27028ecac1ef3830419460736eac9924169 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Leite?= Date: Wed, 29 Jul 2026 20:08:40 -0300 Subject: [PATCH 1/2] feat: scope Editor user administration by group (#990 slice 2) --- .../ruscker-admin/assets/i18n/en/landing.ftl | 5 + .../ruscker-admin/assets/i18n/es/landing.ftl | 5 + .../ruscker-admin/assets/i18n/fr/landing.ftl | 5 + .../ruscker-admin/assets/i18n/pt/landing.ftl | 5 + crates/ruscker-admin/src/auth.rs | 28 +- crates/ruscker-admin/src/db/users.rs | 438 +++++++++----- crates/ruscker-admin/src/routes/admin.rs | 2 + .../ruscker-admin/src/routes/admin/users.rs | 303 +++++++--- crates/ruscker-admin/src/scope.rs | 13 + .../templates/admin/user_edit.html | 28 +- .../ruscker-admin/templates/admin/users.html | 35 +- crates/ruscker-admin/tests/rbac.rs | 551 ++++++++++++++++++ 12 files changed, 1188 insertions(+), 230 deletions(-) diff --git a/crates/ruscker-admin/assets/i18n/en/landing.ftl b/crates/ruscker-admin/assets/i18n/en/landing.ftl index 52e6303c..bb5d831c 100644 --- a/crates/ruscker-admin/assets/i18n/en/landing.ftl +++ b/crates/ruscker-admin/assets/i18n/en/landing.ftl @@ -165,6 +165,7 @@ admin-users-import-done-prefix = Imported: admin-users-import-skipped-prefix = skipped: admin-users-groups-placeholder = analysts, managers admin-users-groups-hint = Comma-separated groups control which restricted apps the user sees. +admin-users-groups-readonly = Groups managed by other teams (read-only): admin-users-col-groups = Groups admin-users-save-groups = Save groups admin-users-new-password = new password @@ -181,6 +182,10 @@ admin-users-password-rule = At least 8 characters, with at least 1 uppercase, 1 admin-users-flash-weak-password = Weak password — the policy requires at least 8 characters, with 1 uppercase, 1 lowercase, 1 digit and 1 special character. admin-users-generate-password = Generate random password admin-users-flash-exists = A user with that name already exists. +admin-users-flash-scope-role = Editors may only assign the Viewer or Editor role. +admin-users-flash-scope-groups = Editors may only assign groups they already belong to. +admin-users-flash-group-required = Add at least one of your groups so the new user remains within your scope. +admin-users-flash-self-edit = You cannot change your own role, groups or password here. Use your account page to change your password. # Admin dashboard admin-dashboard-title = Container Management diff --git a/crates/ruscker-admin/assets/i18n/es/landing.ftl b/crates/ruscker-admin/assets/i18n/es/landing.ftl index 65e26e2e..ee69e3bc 100644 --- a/crates/ruscker-admin/assets/i18n/es/landing.ftl +++ b/crates/ruscker-admin/assets/i18n/es/landing.ftl @@ -165,6 +165,7 @@ admin-users-import-done-prefix = Importados: admin-users-import-skipped-prefix = omitidos: admin-users-groups-placeholder = analistas, gestores admin-users-groups-hint = Los grupos separados por comas controlan qué apps restringidas ve el usuario. +admin-users-groups-readonly = Grupos administrados por otros equipos (solo lectura): admin-users-col-groups = Grupos admin-users-save-groups = Guardar grupos admin-users-new-password = nueva contraseña @@ -181,6 +182,10 @@ admin-users-password-rule = Mínimo 8 caracteres, con al menos 1 mayúscula, 1 m admin-users-flash-weak-password = Contraseña débil — la política exige mínimo 8 caracteres, con 1 mayúscula, 1 minúscula, 1 número y 1 carácter especial. admin-users-generate-password = Generar contraseña aleatoria admin-users-flash-exists = Ya existe un usuario con ese nombre. +admin-users-flash-scope-role = Los Editores solo pueden asignar los roles Visualizador o Editor. +admin-users-flash-scope-groups = Los Editores solo pueden asignar grupos a los que ya pertenecen. +admin-users-flash-group-required = Añada al menos uno de sus grupos para que el nuevo usuario permanezca dentro de su ámbito. +admin-users-flash-self-edit = No puede cambiar su propio rol, grupos ni contraseña aquí. Use la página de su cuenta para cambiar la contraseña. # Admin dashboard admin-dashboard-title = Gestión de Contenedores diff --git a/crates/ruscker-admin/assets/i18n/fr/landing.ftl b/crates/ruscker-admin/assets/i18n/fr/landing.ftl index 9a59190a..a1357038 100644 --- a/crates/ruscker-admin/assets/i18n/fr/landing.ftl +++ b/crates/ruscker-admin/assets/i18n/fr/landing.ftl @@ -165,6 +165,7 @@ admin-users-import-done-prefix = Importés : admin-users-import-skipped-prefix = ignorés : admin-users-groups-placeholder = analystes, gestionnaires admin-users-groups-hint = Les groupes séparés par des virgules déterminent les applis restreintes visibles par l'utilisateur. +admin-users-groups-readonly = Groupes gérés par d'autres équipes (lecture seule) : admin-users-col-groups = Groupes admin-users-save-groups = Enregistrer les groupes admin-users-new-password = nouveau mot de passe @@ -181,6 +182,10 @@ admin-users-password-rule = Au moins 8 caractères, dont 1 majuscule, 1 minuscul admin-users-flash-weak-password = Mot de passe faible — la politique exige au moins 8 caractères, dont 1 majuscule, 1 minuscule, 1 chiffre et 1 caractère spécial. admin-users-generate-password = Générer un mot de passe aléatoire admin-users-flash-exists = Un utilisateur portant ce nom existe déjà. +admin-users-flash-scope-role = Les Éditeurs ne peuvent attribuer que les rôles Lecteur ou Éditeur. +admin-users-flash-scope-groups = Les Éditeurs ne peuvent attribuer que les groupes auxquels ils appartiennent déjà. +admin-users-flash-group-required = Ajoutez au moins un de vos groupes afin que le nouvel utilisateur reste dans votre périmètre. +admin-users-flash-self-edit = Vous ne pouvez pas modifier ici votre propre rôle, vos groupes ou votre mot de passe. Utilisez la page de votre compte pour changer votre mot de passe. # Admin dashboard admin-dashboard-title = Gestion des Conteneurs diff --git a/crates/ruscker-admin/assets/i18n/pt/landing.ftl b/crates/ruscker-admin/assets/i18n/pt/landing.ftl index e560581f..045c86c2 100644 --- a/crates/ruscker-admin/assets/i18n/pt/landing.ftl +++ b/crates/ruscker-admin/assets/i18n/pt/landing.ftl @@ -169,6 +169,7 @@ admin-users-import-done-prefix = Importados: admin-users-import-skipped-prefix = ignorados: admin-users-groups-placeholder = analistas, gestores admin-users-groups-hint = Grupos separados por vírgula controlam quais apps restritos o usuário vê. +admin-users-groups-readonly = Grupos administrados por outras equipes (somente leitura): admin-users-col-groups = Grupos admin-users-save-groups = Salvar grupos admin-users-new-password = nova senha @@ -185,6 +186,10 @@ admin-users-password-rule = Mínimo de 8 caracteres, com ao menos 1 maiúscula, admin-users-flash-weak-password = Senha fraca — a política exige mínimo de 8 caracteres, com 1 maiúscula, 1 minúscula, 1 número e 1 caractere especial. admin-users-generate-password = Gerar senha aleatória admin-users-flash-exists = Já existe um usuário com esse nome. +admin-users-flash-scope-role = Editores só podem atribuir os níveis Visualizador ou Editor. +admin-users-flash-scope-groups = Editores só podem atribuir grupos dos quais já fazem parte. +admin-users-flash-group-required = Informe pelo menos um dos seus grupos para que o novo usuário permaneça no seu escopo. +admin-users-flash-self-edit = Você não pode alterar seu próprio nível, grupos ou senha nesta tela. Use a tela da sua conta para trocar sua senha. # Admin dashboard admin-dashboard-title = Gestão dos Containers diff --git a/crates/ruscker-admin/src/auth.rs b/crates/ruscker-admin/src/auth.rs index 50661e1b..a9ef4941 100644 --- a/crates/ruscker-admin/src/auth.rs +++ b/crates/ruscker-admin/src/auth.rs @@ -12,9 +12,9 @@ //! never be locked out — treat it as a sensitive secret. When it isn't //! set, admin routes 503. //! -//! Roles (see [`Role`]): **Viewer** (dashboard only), **Editor** -//! (apps + media + dashboard), and **Admin** (everything, incl. user -//! management). External IdPs (OIDC/SAML/LDAP) and per-app ACLs land +//! Roles (see [`Role`]): **Viewer** (portal only), **Editor** +//! (apps + media + group-scoped user management), and **Admin** +//! (everything). External IdPs (OIDC/SAML/LDAP) and per-app ACLs land //! in Phase 8. //! //! Cookie: the value is an opaque server-side session id (never the @@ -47,10 +47,10 @@ pub const COOKIE_NAME: &str = "ruscker_admin_session"; /// `nav_section` strings the templates already use. #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)] pub enum Role { - /// Read-only: the monitoring dashboard, nothing else. + /// Portal end user: app visibility comes from group membership. Viewer, - /// Manages apps and media (create/edit/delete + uploads) and has - /// the full dashboard, including the stop/restart actions. + /// Manages apps, media and group-scoped users and has the full + /// dashboard, including the stop/restart actions. Editor, /// Everything — credentials, landing editor, custom blocks, audit /// log. The historical single-token behaviour. @@ -64,13 +64,13 @@ impl Role { /// `landing`, `blocks`, `audit`). pub fn can_access_section(&self, section: &str) -> bool { match section { - // Dashboard + apps + media: Editor and up. A Viewer is NOT - // an admin-panel operator (#857) — it's the portal's + // Dashboard + apps + media + scoped users: Editor and up. A + // Viewer is NOT an admin-panel operator (#857) — it's the portal's // authenticated-end-user role (group-based card visibility, // #155), so it reaches NO admin section. - "dashboard" | "specs" | "images" => *self >= Role::Editor, - // Everything else — credentials, landing, users, groups, - // logs, disk, audit, system, schedules — is admin-only. + "dashboard" | "specs" | "images" | "users" => *self >= Role::Editor, + // Everything else — credentials, landing, groups, logs, disk, + // audit, system, schedules — is admin-only. _ => *self == Role::Admin, } } @@ -741,9 +741,9 @@ mod tests { assert!(Role::Admin > Role::Editor); assert!(Role::Editor > Role::Viewer); - // Dashboard + apps + media: Editor and up — a Viewer reaches no - // admin section (#857). - for sec in ["dashboard", "specs", "images"] { + // Dashboard + apps + media + scoped users: Editor and up — a Viewer + // reaches no admin section (#857). + for sec in ["dashboard", "specs", "images", "users"] { assert!(!Role::Viewer.can_access_section(sec)); assert!(Role::Editor.can_access_section(sec)); assert!(Role::Admin.can_access_section(sec)); diff --git a/crates/ruscker-admin/src/db/users.rs b/crates/ruscker-admin/src/db/users.rs index 8802dd4a..bf12ad7f 100644 --- a/crates/ruscker-admin/src/db/users.rs +++ b/crates/ruscker-admin/src/db/users.rs @@ -48,6 +48,20 @@ pub struct UserCounts { pub must_change_password: i64, } +/// Authoritative filter for the paginated users table (#990 slice 2). +/// +/// `visible_groups = None` is the unscoped Admin/break-glass view. +/// `Some(groups)` is an Editor view: Admin accounts are excluded and a row +/// must share at least one exact, case-sensitive group. Keeping this filter +/// in SQL makes the count, page offsets, search and KPI band derive from the +/// same set; filtering a fetched page in Rust would produce lying totals and +/// empty pages. +#[derive(Debug, Clone, Copy)] +pub struct UserFilter<'a> { + pub search: &'a str, + pub visible_groups: Option<&'a [String]>, +} + /// Normalize a username for storage/lookup: trimmed + lowercased. /// Usernames are case-insensitive and unique. pub fn normalize_username(raw: &str) -> String { @@ -124,18 +138,32 @@ pub async fn count_admins(db: &ConfigDb) -> Result { } /// Count all roles and pending first-password changes in one query. -/// This stays global even while the users table is searched or paginated. -pub async fn counts(db: &ConfigDb) -> Result { - let sql = "SELECT COUNT(*), - COUNT(CASE WHEN role = 'admin' THEN 1 END), - COUNT(CASE WHEN role = 'editor' THEN 1 END), - COUNT(CASE WHEN role = 'viewer' THEN 1 END), - COUNT(CASE WHEN must_change_password = TRUE THEN 1 END) - FROM users"; +/// +/// The search term intentionally does not affect KPIs, matching the existing +/// page behaviour, but the authorization scope does: an Editor's KPI band +/// must never reveal accounts outside their group boundary. +pub async fn counts(db: &ConfigDb, visible_groups: Option<&[String]>) -> Result { + const SELECT: &str = "SELECT COUNT(*), + COUNT(CASE WHEN role = 'admin' THEN 1 END), + COUNT(CASE WHEN role = 'editor' THEN 1 END), + COUNT(CASE WHEN role = 'viewer' THEN 1 END), + COUNT(CASE WHEN must_change_password = TRUE THEN 1 END) + FROM users + WHERE 1=1"; let (total, admins, editors, viewers, must_change_password): (i64, i64, i64, i64, i64) = match db { - ConfigDb::Sqlite(pool) => sqlx::query_as(sql).fetch_one(pool).await, - ConfigDb::Postgres(pool) => sqlx::query_as(sql).fetch_one(pool).await, + ConfigDb::Sqlite(pool) => { + let mut qb: sqlx::QueryBuilder = + sqlx::QueryBuilder::new(SELECT); + push_visibility_sqlite(&mut qb, visible_groups); + qb.build_query_as().fetch_one(pool).await + } + ConfigDb::Postgres(pool) => { + let mut qb: sqlx::QueryBuilder = + sqlx::QueryBuilder::new(SELECT); + push_visibility_postgres(&mut qb, visible_groups); + qb.build_query_as().fetch_one(pool).await + } } .context("count users by role")?; Ok(UserCounts { @@ -263,54 +291,135 @@ fn like_patterns_sqlite(term: &str) -> (String, String) { ) } +/// Push the search predicate shared by both SQL dialects. +/// +/// SQLite receives lowercase + uppercase variants because its built-in +/// `LIKE` only folds ASCII; Postgres uses the same predicate with `ILIKE`. +fn push_search_sqlite(qb: &mut sqlx::QueryBuilder, search: &str) { + if search.is_empty() { + return; + } + let (lo, up) = like_patterns_sqlite(search); + qb.push(" AND ("); + for (index, column) in ["username", "groups", "setor", "email", "celular"] + .into_iter() + .enumerate() + { + if index > 0 { + qb.push(" OR "); + } + if matches!(column, "setor" | "email" | "celular") { + qb.push("COALESCE(").push(column).push(", '')"); + } else { + qb.push(column); + } + qb.push(" LIKE ").push_bind(lo.clone()).push(" ESCAPE '\\'"); + qb.push(" OR "); + if matches!(column, "setor" | "email" | "celular") { + qb.push("COALESCE(").push(column).push(", '')"); + } else { + qb.push(column); + } + qb.push(" LIKE ").push_bind(up.clone()).push(" ESCAPE '\\'"); + } + qb.push(")"); +} + +fn push_search_postgres(qb: &mut sqlx::QueryBuilder, search: &str) { + if search.is_empty() { + return; + } + let pattern = like_pattern(search); + qb.push(" AND ("); + for (index, column) in ["username", "groups", "setor", "email", "celular"] + .into_iter() + .enumerate() + { + if index > 0 { + qb.push(" OR "); + } + if matches!(column, "setor" | "email" | "celular") { + qb.push("COALESCE(").push(column).push(", '')"); + } else { + qb.push(column); + } + qb.push(" ILIKE ").push_bind(pattern.clone()); + } + qb.push(")"); +} + +/// Push the Editor visibility boundary using exact CSV tokens. +/// +/// The `users.groups` column is authoritative for this page. Delimiter +/// wrapping makes `time-a` distinct from `time-ab`; `instr`/`strpos` keep +/// matching case-sensitive and avoid treating `%` or `_` in group names as +/// SQL wildcards. `Some([])` deliberately emits `1=0` (fail closed). +fn push_visibility_sqlite( + qb: &mut sqlx::QueryBuilder, + visible_groups: Option<&[String]>, +) { + let Some(groups) = visible_groups else { + return; + }; + qb.push(" AND role <> 'admin' AND ("); + if groups.is_empty() { + qb.push("1=0"); + } else { + for (index, group) in groups.iter().enumerate() { + if index > 0 { + qb.push(" OR "); + } + qb.push("instr(',' || groups || ',', ") + .push_bind(format!(",{group},")) + .push(") > 0"); + } + } + qb.push(")"); +} + +fn push_visibility_postgres( + qb: &mut sqlx::QueryBuilder, + visible_groups: Option<&[String]>, +) { + let Some(groups) = visible_groups else { + return; + }; + qb.push(" AND role <> 'admin' AND ("); + if groups.is_empty() { + qb.push("1=0"); + } else { + for (index, group) in groups.iter().enumerate() { + if index > 0 { + qb.push(" OR "); + } + qb.push("strpos(',' || groups || ',', ") + .push_bind(format!(",{group},")) + .push(") > 0"); + } + } + qb.push(")"); +} + /// Count the users matching a search term — the total behind /// [`list_page`]'s pagination (#999). A blank `search` counts every /// account; otherwise it's the same case-insensitive substring filter /// over username, groups and the profile fields. -pub async fn count_filtered(db: &ConfigDb, search: &str) -> Result { - let search = search.trim(); +pub async fn count_filtered(db: &ConfigDb, filter: &UserFilter<'_>) -> Result { + let search = filter.search.trim(); let (n,): (i64,) = match db { ConfigDb::Sqlite(pool) => { - if search.is_empty() { - sqlx::query_as("SELECT COUNT(*) FROM users") - .fetch_one(pool) - .await - } else { - // Two case variants per column — see [`like_patterns_sqlite`]. - let (lo, up) = like_patterns_sqlite(search); - sqlx::query_as( - "SELECT COUNT(*) FROM users - WHERE username LIKE ?1 ESCAPE '\\' OR username LIKE ?2 ESCAPE '\\' - OR groups LIKE ?1 ESCAPE '\\' OR groups LIKE ?2 ESCAPE '\\' - OR COALESCE(setor, '') LIKE ?1 ESCAPE '\\' OR COALESCE(setor, '') LIKE ?2 ESCAPE '\\' - OR COALESCE(email, '') LIKE ?1 ESCAPE '\\' OR COALESCE(email, '') LIKE ?2 ESCAPE '\\' - OR COALESCE(celular, '') LIKE ?1 ESCAPE '\\' OR COALESCE(celular, '') LIKE ?2 ESCAPE '\\'", - ) - .bind(lo) - .bind(up) - .fetch_one(pool) - .await - } + let mut qb: sqlx::QueryBuilder = + sqlx::QueryBuilder::new("SELECT COUNT(*) FROM users WHERE 1=1"); + push_search_sqlite(&mut qb, search); + push_visibility_sqlite(&mut qb, filter.visible_groups); + qb.build_query_as().fetch_one(pool).await } ConfigDb::Postgres(pool) => { - if search.is_empty() { - sqlx::query_as("SELECT COUNT(*) FROM users") - .fetch_one(pool) - .await - } else { - // ILIKE for case-insensitivity; `\` is pg's default escape. - sqlx::query_as( - "SELECT COUNT(*) FROM users - WHERE username ILIKE $1 - OR groups ILIKE $1 - OR COALESCE(setor, '') ILIKE $1 - OR COALESCE(email, '') ILIKE $1 - OR COALESCE(celular, '') ILIKE $1", - ) - .bind(like_pattern(search)) - .fetch_one(pool) - .await - } + let mut qb: sqlx::QueryBuilder = + sqlx::QueryBuilder::new("SELECT COUNT(*) FROM users WHERE 1=1"); + push_search_postgres(&mut qb, search); + push_visibility_postgres(&mut qb, filter.visible_groups); + qb.build_query_as().fetch_one(pool).await } } .context("count users (filtered)")?; @@ -325,7 +434,7 @@ pub async fn count_filtered(db: &ConfigDb, search: &str) -> Result { /// number. pub async fn list_page( db: &ConfigDb, - search: &str, + filter: &UserFilter<'_>, limit: i64, offset: i64, ) -> Result> { @@ -341,72 +450,35 @@ pub async fn list_page( Option, Option, ); - let search = search.trim(); + let search = filter.search.trim(); let rows: Vec = match db { ConfigDb::Sqlite(pool) => { - if search.is_empty() { - sqlx::query_as( - "SELECT id, username, role, must_change_password, groups, created_at, created_by, setor, email, celular - FROM users - ORDER BY created_at DESC, username ASC - LIMIT ?1 OFFSET ?2", - ) - .bind(limit) - .bind(offset) - .fetch_all(pool) - .await - } else { - // Two case variants per column — see [`like_patterns_sqlite`]. - let (lo, up) = like_patterns_sqlite(search); - sqlx::query_as( - "SELECT id, username, role, must_change_password, groups, created_at, created_by, setor, email, celular - FROM users - WHERE username LIKE ?1 ESCAPE '\\' OR username LIKE ?2 ESCAPE '\\' - OR groups LIKE ?1 ESCAPE '\\' OR groups LIKE ?2 ESCAPE '\\' - OR COALESCE(setor, '') LIKE ?1 ESCAPE '\\' OR COALESCE(setor, '') LIKE ?2 ESCAPE '\\' - OR COALESCE(email, '') LIKE ?1 ESCAPE '\\' OR COALESCE(email, '') LIKE ?2 ESCAPE '\\' - OR COALESCE(celular, '') LIKE ?1 ESCAPE '\\' OR COALESCE(celular, '') LIKE ?2 ESCAPE '\\' - ORDER BY created_at DESC, username ASC - LIMIT ?3 OFFSET ?4", - ) - .bind(lo) - .bind(up) - .bind(limit) - .bind(offset) - .fetch_all(pool) - .await - } + let mut qb: sqlx::QueryBuilder = sqlx::QueryBuilder::new( + "SELECT id, username, role, must_change_password, groups, created_at, created_by, setor, email, celular + FROM users + WHERE 1=1", + ); + push_search_sqlite(&mut qb, search); + push_visibility_sqlite(&mut qb, filter.visible_groups); + qb.push(" ORDER BY created_at DESC, username ASC LIMIT ") + .push_bind(limit) + .push(" OFFSET ") + .push_bind(offset); + qb.build_query_as().fetch_all(pool).await } ConfigDb::Postgres(pool) => { - if search.is_empty() { - sqlx::query_as( - "SELECT id, username, role, must_change_password, groups, created_at, created_by, setor, email, celular - FROM users - ORDER BY created_at DESC, username ASC - LIMIT $1 OFFSET $2", - ) - .bind(limit) - .bind(offset) - .fetch_all(pool) - .await - } else { - sqlx::query_as( - "SELECT id, username, role, must_change_password, groups, created_at, created_by, setor, email, celular - FROM users - WHERE username ILIKE $1 - OR groups ILIKE $1 - OR COALESCE(setor, '') ILIKE $1 - OR COALESCE(email, '') ILIKE $1 - OR COALESCE(celular, '') ILIKE $1 - ORDER BY created_at DESC, username ASC - LIMIT $2 OFFSET $3", - ) - .bind(like_pattern(search)) - .bind(limit) - .bind(offset) - .fetch_all(pool) - .await - } + let mut qb: sqlx::QueryBuilder = sqlx::QueryBuilder::new( + "SELECT id, username, role, must_change_password, groups, created_at, created_by, setor, email, celular + FROM users + WHERE 1=1", + ); + push_search_postgres(&mut qb, search); + push_visibility_postgres(&mut qb, filter.visible_groups); + qb.push(" ORDER BY created_at DESC, username ASC LIMIT ") + .push_bind(limit) + .push(" OFFSET ") + .push_bind(offset); + qb.build_query_as().fetch_all(pool).await } } .context("list users (page)")?; @@ -1184,6 +1256,13 @@ mod tests { ConfigDb::Sqlite(crate::db::open_memory().await.expect("memory db")) } + fn unscoped(search: &str) -> UserFilter<'_> { + UserFilter { + search, + visible_groups: None, + } + } + #[test] fn hash_and_verify_roundtrip() { let h = hash_password("s3cret-pw").unwrap(); @@ -1391,7 +1470,7 @@ mod tests { .unwrap(); assert_eq!(count_admins(&p).await.unwrap(), 1); assert_eq!( - counts(&p).await.unwrap(), + counts(&p, None).await.unwrap(), UserCounts { total: 3, admins: 1, @@ -1474,9 +1553,9 @@ mod tests { .unwrap(); // Unfiltered: pages partition the full set with no overlap. - assert_eq!(count_filtered(&p, "").await.unwrap(), 3); - let first = list_page(&p, "", 2, 0).await.unwrap(); - let rest = list_page(&p, "", 2, 2).await.unwrap(); + assert_eq!(count_filtered(&p, &unscoped("")).await.unwrap(), 3); + let first = list_page(&p, &unscoped(""), 2, 0).await.unwrap(); + let rest = list_page(&p, &unscoped(""), 2, 2).await.unwrap(); assert_eq!(first.len(), 2); assert_eq!(rest.len(), 1); let mut all: Vec = first @@ -1487,23 +1566,29 @@ mod tests { all.sort(); assert_eq!(all, vec!["alice", "bob", "carol"]); // Past the end ⇒ empty page, not an error. - assert!(list_page(&p, "", 2, 4).await.unwrap().is_empty()); + assert!(list_page(&p, &unscoped(""), 2, 4) + .await + .unwrap() + .is_empty()); // Search is a case-insensitive substring over username, groups // and profile fields. - assert_eq!(count_filtered(&p, "ALI").await.unwrap(), 1); - let hit = list_page(&p, "ALI", 50, 0).await.unwrap(); + assert_eq!(count_filtered(&p, &unscoped("ALI")).await.unwrap(), 1); + let hit = list_page(&p, &unscoped("ALI"), 50, 0).await.unwrap(); assert_eq!(hit.len(), 1); assert_eq!(hit[0].username, "alice"); - assert_eq!(list_page(&p, "ops", 50, 0).await.unwrap()[0].username, "bob"); assert_eq!( - list_page(&p, "@example", 50, 0).await.unwrap()[0].username, + list_page(&p, &unscoped("ops"), 50, 0).await.unwrap()[0].username, + "bob" + ); + assert_eq!( + list_page(&p, &unscoped("@example"), 50, 0).await.unwrap()[0].username, "carol" ); // LIKE wildcards in the term match literally, not as patterns. - assert_eq!(count_filtered(&p, "%").await.unwrap(), 0); - assert_eq!(count_filtered(&p, "a_i").await.unwrap(), 0); - assert_eq!(count_filtered(&p, "nobody").await.unwrap(), 0); + assert_eq!(count_filtered(&p, &unscoped("%")).await.unwrap(), 0); + assert_eq!(count_filtered(&p, &unscoped("a_i")).await.unwrap(), 0); + assert_eq!(count_filtered(&p, &unscoped("nobody")).await.unwrap(), 0); // Accented text: SQLite's LIKE only folds ASCII, so the sqlite // arm matches both Rust-cased variants of the term (codex @@ -1512,16 +1597,85 @@ mod tests { update_profile(&p, "bob", Some("Gestão"), None, None, None) .await .unwrap(); - assert_eq!(count_filtered(&p, "GESTÃO").await.unwrap(), 1); - assert_eq!(count_filtered(&p, "gestão").await.unwrap(), 1); + assert_eq!(count_filtered(&p, &unscoped("GESTÃO")).await.unwrap(), 1); + assert_eq!(count_filtered(&p, &unscoped("gestão")).await.unwrap(), 1); assert_eq!( - list_page(&p, "GESTÃO", 50, 0).await.unwrap()[0].username, + list_page(&p, &unscoped("GESTÃO"), 50, 0).await.unwrap()[0].username, "bob" ); update_profile(&p, "bob", Some("GESTÃO"), None, None, None) .await .unwrap(); - assert_eq!(count_filtered(&p, "gestão").await.unwrap(), 1); + assert_eq!(count_filtered(&p, &unscoped("gestão")).await.unwrap(), 1); + } + + #[tokio::test] + async fn scoped_filter_excludes_admins_and_matches_exact_groups_before_paging() { + let p = pool().await; + create( + &p, + "admin-shared", + "pw-admin", + Role::Admin, + false, + &["time-a".to_string()], + None, + ) + .await + .unwrap(); + create( + &p, + "viewer-shared", + "pw-viewer", + Role::Viewer, + true, + &["time-a".to_string(), "time-b".to_string()], + None, + ) + .await + .unwrap(); + create( + &p, + "viewer-prefix", + "pw-prefix", + Role::Viewer, + false, + &["time-ab".to_string()], + None, + ) + .await + .unwrap(); + let groups = vec!["time-a".to_string()]; + let filter = UserFilter { + search: "", + visible_groups: Some(&groups), + }; + + assert_eq!(count_filtered(&p, &filter).await.unwrap(), 1); + let page = list_page(&p, &filter, 50, 0).await.unwrap(); + assert_eq!(page.len(), 1); + assert_eq!(page[0].username, "viewer-shared"); + assert_eq!( + counts(&p, Some(&groups)).await.unwrap(), + UserCounts { + total: 1, + admins: 0, + editors: 0, + viewers: 1, + must_change_password: 1, + } + ); + + let empty: Vec = Vec::new(); + let fail_closed = UserFilter { + search: "", + visible_groups: Some(&empty), + }; + assert_eq!(count_filtered(&p, &fail_closed).await.unwrap(), 0); + assert!(list_page(&p, &fail_closed, 50, 0) + .await + .unwrap() + .is_empty()); } // Full user lifecycle through the `ConfigDb::Postgres` arm against a @@ -1552,13 +1706,29 @@ mod tests { assert_eq!(list_all(&db).await.unwrap().len(), 2); // Pagination + ILIKE search through the Postgres arm (#999). - assert_eq!(count_filtered(&db, "").await.unwrap(), 2); - assert_eq!(list_page(&db, "", 1, 0).await.unwrap().len(), 1); - assert_eq!(list_page(&db, "", 10, 2).await.unwrap().len(), 0); - let hit = list_page(&db, "ED", 10, 0).await.unwrap(); + assert_eq!(count_filtered(&db, &unscoped("")).await.unwrap(), 2); + assert_eq!(list_page(&db, &unscoped(""), 1, 0).await.unwrap().len(), 1); + assert_eq!( + list_page(&db, &unscoped(""), 10, 2) + .await + .unwrap() + .len(), + 0 + ); + let hit = list_page(&db, &unscoped("ED"), 10, 0).await.unwrap(); assert_eq!(hit.len(), 1); assert_eq!(hit[0].username, "ed"); - assert_eq!(count_filtered(&db, "%").await.unwrap(), 0); + assert_eq!(count_filtered(&db, &unscoped("%")).await.unwrap(), 0); + let leads = vec!["leads".to_string()]; + let scoped = UserFilter { + search: "", + visible_groups: Some(&leads), + }; + assert_eq!(count_filtered(&db, &scoped).await.unwrap(), 1); + assert_eq!( + list_page(&db, &scoped, 10, 0).await.unwrap()[0].username, + "ed" + ); // Case-insensitive login + must_change BOOLEAN round-trip. let u = verify_login(&db, "ed", "edpw1234") diff --git a/crates/ruscker-admin/src/routes/admin.rs b/crates/ruscker-admin/src/routes/admin.rs index c801a016..4b0f9ab8 100644 --- a/crates/ruscker-admin/src/routes/admin.rs +++ b/crates/ruscker-admin/src/routes/admin.rs @@ -777,6 +777,8 @@ fn section_for_admin_path(path: &str) -> &'static str { "logs" } else if path.starts_with("/admin/schedules") { "schedules" + } else if path.starts_with("/admin/users") { + "users" } else { // /admin root and anything unrecognised → dashboard (every // role can reach it). diff --git a/crates/ruscker-admin/src/routes/admin/users.rs b/crates/ruscker-admin/src/routes/admin/users.rs index 46773907..73e5b0e9 100644 --- a/crates/ruscker-admin/src/routes/admin/users.rs +++ b/crates/ruscker-admin/src/routes/admin/users.rs @@ -1,10 +1,11 @@ -//! User-account management — Admin only. +//! User-account management — global for Admins, group-scoped for Editors. //! //! Create/edit/delete the per-user accounts that back password login -//! (see [`crate::db::users`]). Guarded by [`RequireAdmin`]; the nav -//! link is hidden for non-admins. A **last-admin guard** prevents -//! deleting or demoting the only remaining admin, so an operator can't -//! lock the role out of the portal. +//! (see [`crate::db::users`]). [`EditorScope`] gives Editors delegated +//! helpdesk access only to non-Admin users who share a group. Bulk import, +//! MFA reset and account deletion remain [`RequireAdmin`] operations. A +//! **last-admin guard** prevents deleting or demoting the only remaining +//! admin, so an operator can't lock the role out of the portal. use askama::Template; use axum::{ @@ -19,6 +20,7 @@ use serde::Deserialize; use crate::auth::{RequireAdmin, Role}; use crate::db; use crate::i18n::{Locale, Locales}; +use crate::scope::EditorScope; use crate::theme::Theme; use crate::AppState; @@ -65,6 +67,9 @@ struct UsersPage<'a> { q: String, /// Username of the logged-in admin — flags the "you" row. me: String, + /// Admin/break-glass sessions may use the deliberately global controls + /// (CSV import and account deletion); Editors may not. + unscoped: bool, /// "" | "saved" | "created" | "deleted" | "last-admin" | "bad-input" /// | "weak-password" | "exists" | "imported" flash: &'static str, @@ -95,8 +100,16 @@ impl UsersPage<'_> { } /// The three roles, for the create/edit selectors. - fn all_roles(&self) -> [Role; 3] { - [Role::Viewer, Role::Editor, Role::Admin] + fn assignable_roles(&self) -> Vec { + if self.unscoped { + vec![Role::Viewer, Role::Editor, Role::Admin] + } else { + vec![Role::Viewer, Role::Editor] + } + } + + fn can_edit_user(&self, user: &db::users::UserRow) -> bool { + self.unscoped || user.username != self.me } fn kpis(&self) -> [KpiMetric; 5] { @@ -126,6 +139,11 @@ struct UserEditPage<'a> { role: Role, user: db::users::UserRow, me: String, + /// Groups this caller may replace and the foreign memberships preserved + /// read-only by `merge_preserving_out_of_scope`. + editable_groups: Vec, + readonly_groups: Vec, + unscoped: bool, /// Empty means pending/not configured; otherwise the confirmation date. mfa_enrolled_at: String, /// "" | "saved" | "mfa-reset" | "last-admin" | "bad-input" | "weak-password" @@ -141,8 +159,12 @@ impl UserEditPage<'_> { self.t(role.label_key()) } - fn all_roles(&self) -> [Role; 3] { - [Role::Viewer, Role::Editor, Role::Admin] + fn assignable_roles(&self) -> Vec { + if self.unscoped { + vec![Role::Viewer, Role::Editor, Role::Admin] + } else { + vec![Role::Viewer, Role::Editor] + } } } @@ -176,8 +198,35 @@ fn redirect_edit_flash(username: &str, flash: &str) -> Response { Redirect::to(&format!("/admin/users/{username}/edit?flash={flash}")).into_response() } +/// Resolve a path username to its authoritative `users` row and apply the +/// shared scope predicate. Every Editor-accessible username route calls this +/// before validation or mutation, returning the same 404 for a missing and a +/// foreign account so guessed usernames do not disclose membership. +async fn scoped_target( + pool: &crate::db::ConfigDb, + scope: &EditorScope, + raw_username: &str, +) -> Result { + let username = db::users::normalize_username(raw_username); + if !db::users::is_valid_username(&username) { + return Err(StatusCode::NOT_FOUND.into_response()); + } + match db::users::fetch(pool, &username).await { + Ok(Some(user)) if scope.may_touch_user(&user) => Ok(user), + Ok(Some(_)) | Ok(None) => Err(StatusCode::NOT_FOUND.into_response()), + Err(e) => { + tracing::error!(error = ?e, %username, "fetch scoped user target failed"); + Err((StatusCode::INTERNAL_SERVER_ERROR, "db error").into_response()) + } + } +} + +fn is_scoped_self_edit(scope: &EditorScope, username: &str) -> bool { + !scope.unscoped && scope.actor.as_deref() == Some(username) +} + async fn index( - admin: RequireAdmin, + scope: EditorScope, State(state): State, loc: Locale, theme: Theme, @@ -193,14 +242,15 @@ async fn index( // Server-side pagination + search (#999): count first, clamp the // requested page into range, then fetch only that page's rows. let search = q.q.as_deref().unwrap_or("").trim().to_string(); - let total = match db::users::count_filtered(pool, &search).await { + let filter = scope.user_filter(&search); + let total = match db::users::count_filtered(pool, &filter).await { Ok(n) => n, Err(e) => { tracing::error!(error = ?e, "count users failed"); return (StatusCode::INTERNAL_SERVER_ERROR, "db error").into_response(); } }; - let counts = match db::users::counts(pool).await { + let counts = match db::users::counts(pool, filter.visible_groups).await { Ok(counts) => counts, Err(e) => { tracing::error!(error = ?e, "count user KPIs failed"); @@ -210,10 +260,22 @@ async fn index( // Ceiling division (i64::div_ceil is still unstable on our floor). let pages = ((total + USERS_PER_PAGE - 1) / USERS_PER_PAGE).max(1); let page = q.page.unwrap_or(1).clamp(1, pages); - let users = match db::users::list_page(pool, &search, USERS_PER_PAGE, (page - 1) * USERS_PER_PAGE) - .await + let users = match db::users::list_page( + pool, + &filter, + USERS_PER_PAGE, + (page - 1) * USERS_PER_PAGE, + ) + .await { - Ok(u) => u, + Ok(u) if u.iter().all(|user| scope.may_touch_user(user)) => u, + Ok(_) => { + // SQL owns pagination, but `may_touch_user` remains the policy + // oracle. A mismatch is a query bug and must fail closed rather + // than render a row the predicate rejects. + tracing::error!("users scope query returned an unauthorized row"); + return (StatusCode::INTERNAL_SERVER_ERROR, "db error").into_response(); + } Err(e) => { tracing::error!(error = ?e, "list users failed"); return (StatusCode::INTERNAL_SERVER_ERROR, "db error").into_response(); @@ -228,6 +290,10 @@ async fn index( Some("weak-password") => "weak-password", Some("exists") => "exists", Some("imported") => "imported", + Some("scope-role") => "scope-role", + Some("scope-groups") => "scope-groups", + Some("group-required") => "group-required", + Some("self-edit") => "self-edit", _ => "", }; let import_summary = (flash == "imported") @@ -239,7 +305,7 @@ async fn index( locales_all: &Locale::ALL, base: state.base_path.clone(), nav_section: "users", - role: admin.role, + role: scope.role, users, page, pages, @@ -250,7 +316,8 @@ async fn index( kpi_viewers: counts.viewers, kpi_password_change: counts.must_change_password, q: search, - me: admin.actor().to_string(), + me: scope.actor().to_string(), + unscoped: scope.unscoped, flash, import_summary, }; @@ -275,7 +342,7 @@ pub struct CreateForm { } async fn create( - admin: RequireAdmin, + scope: EditorScope, State(state): State, Form(form): Form, ) -> Response { @@ -291,6 +358,17 @@ async fn create( return redirect_flash("weak-password"); } let groups = db::users::parse_groups(&form.groups); + if !scope.may_assign_role(role) { + return redirect_flash("scope-role"); + } + if !scope.may_assign_groups(&groups) { + return redirect_flash("scope-groups"); + } + if !scope.unscoped && groups.is_empty() { + // A delegated account needs a shared team boundary. Without one the + // creating Editor would immediately lose sight of the new account. + return redirect_flash("group-required"); + } // New accounts get the "change your password?" prompt on first login. match db::users::create( pool, @@ -299,7 +377,7 @@ async fn create( role, true, &groups, - Some(admin.actor()), + Some(scope.actor()), ) .await { @@ -316,7 +394,7 @@ async fn create( Some(&form.setor), Some(&form.email), Some(&form.celular), - Some(admin.actor()), + Some(scope.actor()), ) .await { @@ -338,7 +416,7 @@ struct EditQuery { } async fn edit( - admin: RequireAdmin, + scope: EditorScope, State(state): State, loc: Locale, theme: Theme, @@ -348,36 +426,40 @@ async fn edit( let Some(pool) = state.db.as_ref() else { return (StatusCode::SERVICE_UNAVAILABLE, "no db").into_response(); }; - let username = db::users::normalize_username(&username); - if !db::users::is_valid_username(&username) { - return StatusCode::NOT_FOUND.into_response(); - } - let user = match db::users::fetch(pool, &username).await { - Ok(Some(user)) => user, - Ok(None) => return StatusCode::NOT_FOUND.into_response(), - Err(e) => { - tracing::error!(error = ?e, %username, "fetch user for edit failed"); - return (StatusCode::INTERNAL_SERVER_ERROR, "db error").into_response(); - } + let user = match scoped_target(pool, &scope, &username).await { + Ok(user) => user, + Err(response) => return response, }; + let username = user.username.clone(); let flash = match q.flash.as_deref() { Some("saved") => "saved", Some("mfa-reset") => "mfa-reset", Some("last-admin") => "last-admin", Some("bad-input") => "bad-input", Some("weak-password") => "weak-password", + Some("scope-role") => "scope-role", + Some("scope-groups") => "scope-groups", + Some("self-edit") => "self-edit", _ => "", }; - let mfa_enrolled_at = match db::mfa::fetch(pool, &username).await { - Ok(row) => row - .and_then(|row| row.confirmed_at) - .map(|at| at.format("%Y-%m-%d %H:%M UTC").to_string()) - .unwrap_or_default(), - Err(e) => { - tracing::error!(error = ?e, %username, "fetch MFA status for user edit failed"); - return (StatusCode::INTERNAL_SERVER_ERROR, "db error").into_response(); + // MFA is a security control and remains Admin-only. Editors neither see + // enrollment state nor receive the reset form. + let mfa_enrolled_at = if scope.unscoped { + match db::mfa::fetch(pool, &username).await { + Ok(row) => row + .and_then(|row| row.confirmed_at) + .map(|at| at.format("%Y-%m-%d %H:%M UTC").to_string()) + .unwrap_or_default(), + Err(e) => { + tracing::error!(error = ?e, %username, "fetch MFA status for user edit failed"); + return (StatusCode::INTERNAL_SERVER_ERROR, "db error").into_response(); + } } + } else { + String::new() }; + let editable_groups = scope.merge_preserving_out_of_scope(&[], &user.groups); + let readonly_groups = scope.merge_preserving_out_of_scope(&user.groups, &[]); super::render(&UserEditPage { locale: loc, theme, @@ -385,9 +467,12 @@ async fn edit( locales_all: &Locale::ALL, base: state.base_path.clone(), nav_section: "users", - role: admin.role, + role: scope.role, user, - me: admin.actor().to_string(), + me: scope.actor().to_string(), + editable_groups, + readonly_groups, + unscoped: scope.unscoped, mfa_enrolled_at, flash, }) @@ -395,6 +480,7 @@ async fn edit( #[derive(Debug, Deserialize)] struct EditForm { + #[serde(default)] role: String, #[serde(default)] groups: String, @@ -407,7 +493,7 @@ struct EditForm { } async fn save_edit( - admin: RequireAdmin, + scope: EditorScope, State(state): State, Path(username): Path, Form(form): Form, @@ -415,22 +501,32 @@ async fn save_edit( let Some(pool) = state.db.as_ref() else { return (StatusCode::SERVICE_UNAVAILABLE, "no db").into_response(); }; - let username = db::users::normalize_username(&username); - if !db::users::is_valid_username(&username) { - return redirect_flash("bad-input"); + let current = match scoped_target(pool, &scope, &username).await { + Ok(user) => user, + Err(response) => return response, + }; + let username = current.username.clone(); + if is_scoped_self_edit(&scope, &username) { + // Editors manage their own password through `/admin/account`; letting + // this consolidated form touch self role/groups would be a direct + // privilege-escalation path. + return redirect_edit_flash(&username, "self-edit"); } let Some(role) = Role::parse(&form.role) else { return redirect_edit_flash(&username, "bad-input"); }; - let current = match db::users::fetch(pool, &username).await { - Ok(Some(user)) => user, - _ => return redirect_edit_flash(&username, "bad-input"), - }; + if !scope.may_assign_role(role) { + return redirect_edit_flash(&username, "scope-role"); + } if would_strip_last_admin(pool, &username, Some(role)).await { return redirect_edit_flash(&username, "last-admin"); } - let groups = db::users::parse_groups(&form.groups); + let requested_groups = db::users::parse_groups(&form.groups); + if !scope.may_assign_groups(&requested_groups) { + return redirect_edit_flash(&username, "scope-groups"); + } + let groups = scope.merge_preserving_out_of_scope(¤t.groups, &requested_groups); let update = db::users::AccountUpdate { role, groups: &groups, @@ -438,7 +534,7 @@ async fn save_edit( email: Some(&form.email), celular: Some(&form.celular), }; - match db::users::update_account(pool, &username, update, Some(admin.actor())).await { + match db::users::update_account(pool, &username, update, Some(scope.actor())).await { Ok(()) => { if current.role != role { // Role changes take effect immediately, including self-demotion. @@ -471,7 +567,7 @@ pub struct ProfileForm { } async fn set_profile( - admin: RequireAdmin, + scope: EditorScope, State(state): State, Path(username): Path, Form(form): Form, @@ -479,13 +575,17 @@ async fn set_profile( let Some(pool) = state.db.as_ref() else { return (StatusCode::SERVICE_UNAVAILABLE, "no db").into_response(); }; + let target = match scoped_target(pool, &scope, &username).await { + Ok(user) => user, + Err(response) => return response, + }; match db::users::update_profile( pool, - &username, + &target.username, Some(&form.setor), Some(&form.email), Some(&form.celular), - Some(admin.actor()), + Some(scope.actor()), ) .await { @@ -510,7 +610,7 @@ pub struct GroupsForm { } async fn set_groups( - admin: RequireAdmin, + scope: EditorScope, State(state): State, Path(username): Path, Form(form): Form, @@ -518,8 +618,19 @@ async fn set_groups( let Some(pool) = state.db.as_ref() else { return (StatusCode::SERVICE_UNAVAILABLE, "no db").into_response(); }; - let groups = db::users::parse_groups(&form.groups); - match db::users::set_groups(pool, &username, &groups, Some(admin.actor())).await { + let target = match scoped_target(pool, &scope, &username).await { + Ok(user) => user, + Err(response) => return response, + }; + if is_scoped_self_edit(&scope, &target.username) { + return redirect_edit_flash(&target.username, "self-edit"); + } + let requested = db::users::parse_groups(&form.groups); + if !scope.may_assign_groups(&requested) { + return redirect_edit_flash(&target.username, "scope-groups"); + } + let groups = scope.merge_preserving_out_of_scope(&target.groups, &requested); + match db::users::set_groups(pool, &target.username, &groups, Some(scope.actor())).await { Ok(()) => { state.invalidate_identity_cache(); redirect_flash("saved") @@ -533,11 +644,12 @@ async fn set_groups( #[derive(Debug, Deserialize)] pub struct RoleForm { + #[serde(default)] pub role: String, } async fn set_role( - admin: RequireAdmin, + scope: EditorScope, State(state): State, Path(username): Path, Form(form): Form, @@ -545,18 +657,38 @@ async fn set_role( let Some(pool) = state.db.as_ref() else { return (StatusCode::SERVICE_UNAVAILABLE, "no db").into_response(); }; + let target = match scoped_target(pool, &scope, &username).await { + Ok(user) => user, + Err(response) => return response, + }; + if is_scoped_self_edit(&scope, &target.username) { + return redirect_edit_flash(&target.username, "self-edit"); + } let new_role = Role::parse(&form.role).unwrap_or(Role::Viewer); + if !scope.may_assign_role(new_role) { + return redirect_edit_flash(&target.username, "scope-role"); + } // Last-admin guard: don't let the only admin be demoted. - if would_strip_last_admin(pool, &username, Some(new_role)).await { + if would_strip_last_admin(pool, &target.username, Some(new_role)).await { return redirect_flash("last-admin"); } - match db::users::set_role(pool, &username, new_role, Some(admin.actor())).await { + match db::users::set_role( + pool, + &target.username, + new_role, + Some(scope.actor()), + ) + .await + { Ok(()) => { // Kick the user's live sessions so the new role takes effect // now, not after the cookie expires (#544) — a demotion must // drop elevated access immediately. - state.admin_sessions.revoke_by_actor(&username).await; + state + .admin_sessions + .revoke_by_actor(&target.username) + .await; redirect_flash("saved") } Err(e) => { @@ -568,11 +700,12 @@ async fn set_role( #[derive(Debug, Deserialize)] pub struct ResetForm { + #[serde(default)] pub password: String, } async fn reset_password( - admin: RequireAdmin, + scope: EditorScope, State(state): State, Path(username): Path, Form(form): Form, @@ -580,15 +713,32 @@ async fn reset_password( let Some(pool) = state.db.as_ref() else { return (StatusCode::SERVICE_UNAVAILABLE, "no db").into_response(); }; + let target = match scoped_target(pool, &scope, &username).await { + Ok(user) => user, + Err(response) => return response, + }; + if is_scoped_self_edit(&scope, &target.username) { + return redirect_edit_flash(&target.username, "self-edit"); + } if !crate::auth::password_meets_policy(&form.password) { - return redirect_edit_flash(&db::users::normalize_username(&username), "weak-password"); + return redirect_edit_flash(&target.username, "weak-password"); } // Admin-assigned password ⇒ prompt the user to change it next login. - match db::users::set_password(pool, &username, &form.password, true, Some(admin.actor())).await + match db::users::set_password( + pool, + &target.username, + &form.password, + true, + Some(scope.actor()), + ) + .await { Ok(()) => { // A password reset must invalidate existing sessions (#544). - state.admin_sessions.revoke_by_actor(&username).await; + state + .admin_sessions + .revoke_by_actor(&target.username) + .await; redirect_flash("saved") } Err(e) => { @@ -598,6 +748,8 @@ async fn reset_password( } } +/// MFA reset changes a security control, not ordinary team membership, so it +/// deliberately remains Admin/break-glass only even for an in-scope user. async fn reset_mfa( admin: RequireAdmin, State(state): State, @@ -627,6 +779,9 @@ async fn reset_mfa( } } +/// Account deletion removes every membership, including groups an Editor +/// does not administer. It therefore remains Admin/break-glass only; an +/// Editor offboards someone from their team by removing the shared group. async fn delete( admin: RequireAdmin, State(state): State, @@ -635,6 +790,18 @@ async fn delete( let Some(pool) = state.db.as_ref() else { return (StatusCode::SERVICE_UNAVAILABLE, "no db").into_response(); }; + let username = db::users::normalize_username(&username); + if !db::users::is_valid_username(&username) { + return StatusCode::NOT_FOUND.into_response(); + } + match db::users::fetch(pool, &username).await { + Ok(Some(_)) => {} + Ok(None) => return StatusCode::NOT_FOUND.into_response(), + Err(e) => { + tracing::error!(error = ?e, %username, "fetch user before delete failed"); + return (StatusCode::INTERNAL_SERVER_ERROR, "db error").into_response(); + } + } // Last-admin guard: deleting the only admin would lock everyone out. if would_strip_last_admin(pool, &username, None).await { return redirect_flash("last-admin"); @@ -820,7 +987,8 @@ impl UsersImportPreviewPage<'_> { } /// `POST /admin/users/import` — multipart CSV upload → parse → preview -/// (no writes). The valid rows are committed only by `import_confirm`. +/// (no writes). Bulk import has no natural single-row scope boundary, so both +/// preview and confirm deliberately remain Admin/break-glass only. async fn import( admin: RequireAdmin, State(state): State, @@ -887,6 +1055,7 @@ pub struct ImportConfirmForm { /// so the imported initial password is changed on first login. Existing /// usernames + invalid rows are skipped (create is fail-closed on a /// duplicate anyway). Redirects with an `imported`/`skipped` summary. +/// This remains Admin-only for the same bulk-scope reason as [`import`]. async fn import_confirm( admin: RequireAdmin, State(state): State, diff --git a/crates/ruscker-admin/src/scope.rs b/crates/ruscker-admin/src/scope.rs index 5f761aa9..17e501cf 100644 --- a/crates/ruscker-admin/src/scope.rs +++ b/crates/ruscker-admin/src/scope.rs @@ -133,6 +133,19 @@ impl EditorScope { self.unscoped || matches!(requested, Role::Viewer | Role::Editor) } + /// Build the authoritative SQL filter for the users table. + /// + /// `None` means the Admin/break-glass view is unrestricted; `Some([])` + /// is a scoped Editor with no groups and therefore matches no account. + /// Keeping that distinction explicit prevents a failed scope lookup from + /// accidentally becoming the unscoped query. + pub fn user_filter<'a>(&'a self, search: &'a str) -> db::users::UserFilter<'a> { + db::users::UserFilter { + search, + visible_groups: (!self.unscoped).then_some(self.groups.as_slice()), + } + } + /// Merge an Editor's requested memberships without erasing another team's. /// /// `db::users::set_groups` replaces the whole list. For a user shared by diff --git a/crates/ruscker-admin/templates/admin/user_edit.html b/crates/ruscker-admin/templates/admin/user_edit.html index 5e78b82c..ad750998 100644 --- a/crates/ruscker-admin/templates/admin/user_edit.html +++ b/crates/ruscker-admin/templates/admin/user_edit.html @@ -14,11 +14,14 @@

{{ self.t("admin-users-edit-title -{% if flash == "last-admin" || flash == "bad-input" || flash == "weak-password" %} +{% if flash == "last-admin" || flash == "bad-input" || flash == "weak-password" || flash == "scope-role" || flash == "scope-groups" || flash == "self-edit" %} {% else if flash == "saved" || flash == "mfa-reset" %} @@ -37,12 +40,18 @@

{{ self.t("admin-users-edit-title + {% if !unscoped && user.username == me %} + + {% else %}
+ {% endif %} +{% if unscoped %}
{{ self.t("admin-users-mfa-section") }}
{% if mfa_enrolled_at != "" %} @@ -100,7 +119,9 @@

{{ self.t("admin-users-edit-title

{{ self.t("admin-users-mfa-not-configured") }}

{% endif %}

+{% endif %} +{% if unscoped || user.username != me %}
{{ self.t("admin-users-password-section") }}

{{ self.t("admin-users-password-reset-hint") }}

@@ -129,6 +150,7 @@

{{ self.t("admin-users-edit-title

+{% endif %} +{% endif %} {# ── Existing users ────────────────────────────────────────────── #} {# Search + pagination are SERVER-side (#999): with a large user base @@ -228,14 +235,18 @@

{{ self.t("admin-users-title") }} - - - -
- -
+ {% if self.can_edit_user(u) %} + + + + {% endif %} + {% if unscoped %} +
+ +
+ {% endif %} {% endfor %} @@ -254,7 +265,7 @@

{{ self.t("admin-users-title") }} {{ self.t("admin-users-prev") }} {% endif %} - {{ self.pager_status() }} + {{ self.pager_status() }} {% if page < pages %} diff --git a/crates/ruscker-admin/tests/rbac.rs b/crates/ruscker-admin/tests/rbac.rs index 413c3b21..8d690745 100644 --- a/crates/ruscker-admin/tests/rbac.rs +++ b/crates/ruscker-admin/tests/rbac.rs @@ -205,6 +205,54 @@ async fn scoped_state() -> (AppState, ruscker_admin::db::ConfigDb) { (state, db) } +/// Insert an account without spending argon2 time in authorization tests. +/// +/// These tests mint opaque sessions directly (the established RBAC harness +/// pattern), so the password hash is never read. The row itself is real and +/// authoritative: `EditorScope` refetches it from this table on every request. +async fn seed_user( + db: &ruscker_admin::db::ConfigDb, + username: &str, + role: Role, + groups: &[&str], +) { + let ruscker_admin::db::ConfigDb::Sqlite(pool) = db else { + panic!("RBAC scope fixture must use SQLite"); + }; + let now = Utc::now(); + sqlx::query( + "INSERT INTO users + (id, username, password_hash, role, must_change_password, + groups, created_at, updated_at, created_by) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)", + ) + .bind(uuid::Uuid::new_v4().to_string()) + .bind(username) + .bind("unused-test-hash") + .bind(role.as_str()) + .bind(0_i64) + .bind(groups.join(",")) + .bind(now) + .bind(now) + .bind("seed") + .execute(pool) + .await + .expect("seed scoped user"); +} + +async fn scoped_user_state() -> (AppState, ruscker_admin::db::ConfigDb) { + let (state, db) = scoped_state().await; + for (username, role, groups) in [ + ("viewer-a", Role::Viewer, &["time-a"][..]), + ("shared-user", Role::Viewer, &["time-a", "time-b"][..]), + ("viewer-b", Role::Viewer, &["time-b"][..]), + ("admin-a", Role::Admin, &["time-a"][..]), + ] { + seed_user(&db, username, role, groups).await; + } + (state, db) +} + async fn scoped_cookie(state: &AppState, role: Role, actor: Option<&str>) -> String { let id = state .admin_sessions @@ -757,3 +805,506 @@ async fn scoped_editor_cannot_assign_foreign_group_and_edit_preserves_it() { "the out-of-scope group survives the Editor's replacement save" ); } + +// ── Editor group scope over users (#990 slice 2) ─────────────────── + +#[tokio::test] +async fn scoped_editor_lists_only_shared_non_admin_users_and_matching_kpis() { + let (state, _db) = scoped_user_state().await; + let cookie = scoped_cookie(&state, Role::Editor, Some("editor-a")).await; + let response = send_request( + state, + "GET", + "/admin/users", + Some(&cookie), + Body::empty(), + None, + ) + .await; + assert_eq!(response.status(), StatusCode::OK); + let body = response_body(response).await; + + for username in ["editor-a", "viewer-a", "shared-user"] { + assert!(body.contains(username), "shared user {username} is listed"); + } + assert!(!body.contains("viewer-b"), "foreign user stays hidden"); + assert!( + !body.contains("admin-a"), + "Admin stays hidden even with a shared group" + ); + assert!( + body.contains(r#"href="/admin/users""#), + "the Users nav tab is visible to Editors" + ); + assert_eq!( + metric_values(&body).first().copied(), + Some("3"), + "Total KPI equals the scoped row set" + ); + assert!(body.contains(r#"data-users-total="3""#)); + assert!( + !body.contains(r#"href="/admin/users/editor-a/edit""#), + "the Editor's own account has no edit affordance" + ); + assert!( + !body.contains(r#"value="admin""#), + "the create-role selector does not offer Admin" + ); + assert!( + !body.contains(r#"action="/admin/users/import""#), + "bulk import is not rendered for Editors" + ); + assert!( + !body.contains("/delete\""), + "account deletion is not rendered for Editors" + ); +} + +#[tokio::test] +async fn scoped_editor_gets_404_on_every_foreign_user_route() { + let (state, _db) = scoped_user_state().await; + let cookie = scoped_cookie(&state, Role::Editor, Some("editor-a")).await; + + // Every username route opened to Editors is enumerated. The authoritative + // target row is checked before parsing form data, so malformed crafted + // POSTs cannot turn validation differences into an existence oracle. + for (method, uri) in [ + ("GET", "/admin/users/viewer-b/edit"), + ("POST", "/admin/users/viewer-b/edit"), + ("POST", "/admin/users/viewer-b/role"), + ("POST", "/admin/users/viewer-b/groups"), + ("POST", "/admin/users/viewer-b/profile"), + ("POST", "/admin/users/viewer-b/password"), + ] { + 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 user route must be 404: {method} {uri}" + ); + } +} + +#[tokio::test] +async fn scoped_editor_creation_enforces_role_groups_and_audit_actor() { + let (state, db) = scoped_user_state().await; + let cookie = scoped_cookie(&state, Role::Editor, Some("editor-a")).await; + + for (username, role, groups, flash) in [ + ("bad-admin", "admin", "time-a", "scope-role"), + ("bad-group", "viewer", "time-b", "scope-groups"), + ("no-group", "viewer", "", "group-required"), + ] { + let body = format!( + "username={username}&password=Valid%21Pass9&role={role}&groups={groups}" + ); + let response = send_request( + state.clone(), + "POST", + "/admin/users", + Some(&cookie), + Body::from(body), + Some("application/x-www-form-urlencoded"), + ) + .await; + assert_eq!(response.status(), StatusCode::SEE_OTHER); + assert_eq!( + response.headers().get("location").unwrap(), + &format!("/admin/users?flash={flash}") + ); + assert!( + ruscker_admin::db::users::fetch(&db, username) + .await + .unwrap() + .is_none(), + "rejected account {username} must not be persisted" + ); + } + + let localized = send_request( + state.clone(), + "GET", + "/admin/users?flash=group-required", + Some(&cookie), + Body::empty(), + None, + ) + .await; + let localized = response_body(localized).await; + assert!( + localized.contains("pelo menos um dos seus grupos"), + "the validation error is localized and actionable" + ); + + let created = send_request( + state, + "POST", + "/admin/users", + Some(&cookie), + Body::from( + "username=new-teammate&password=Valid%21Pass9&role=editor&groups=time-a", + ), + Some("application/x-www-form-urlencoded"), + ) + .await; + assert_eq!(created.status(), StatusCode::SEE_OTHER); + let user = ruscker_admin::db::users::fetch(&db, "new-teammate") + .await + .unwrap() + .unwrap(); + assert_eq!(user.role, Role::Editor); + assert_eq!(user.groups, vec!["time-a"]); + + let ruscker_admin::db::ConfigDb::Sqlite(pool) = &db else { + panic!("RBAC scope fixture must use SQLite"); + }; + let actors: Vec<(Option,)> = sqlx::query_as( + "SELECT actor FROM audit_log + WHERE action = 'user.create' AND target = 'user:new-teammate'", + ) + .fetch_all(pool) + .await + .unwrap(); + assert_eq!(actors, vec![(Some("editor-a".to_string()),)]); +} + +#[tokio::test] +async fn scoped_editor_cannot_delete_import_or_reset_mfa() { + let (state, db) = scoped_user_state().await; + let cookie = scoped_cookie(&state, Role::Editor, Some("editor-a")).await; + + for uri in [ + "/admin/users/viewer-a/delete", + "/admin/users/viewer-a/mfa/reset", + "/admin/users/import", + "/admin/users/import/confirm", + ] { + assert_eq!( + send(state.clone(), "POST", uri, Some(&cookie)).await, + StatusCode::FORBIDDEN, + "Admin-only operation must reject Editor: {uri}" + ); + } + assert!( + ruscker_admin::db::users::fetch(&db, "viewer-a") + .await + .unwrap() + .is_some(), + "the forbidden delete leaves the account intact" + ); +} + +#[tokio::test] +async fn scoped_editor_edit_preserves_foreign_group_and_can_reset_password() { + let (state, db) = scoped_user_state().await; + let cookie = scoped_cookie(&state, Role::Editor, Some("editor-a")).await; + + let edit = send_request( + state.clone(), + "GET", + "/admin/users/shared-user/edit", + Some(&cookie), + Body::empty(), + None, + ) + .await; + assert_eq!(edit.status(), StatusCode::OK); + let edit = response_body(edit).await; + assert!(edit.contains(r#"value="time-a""#)); + assert!( + edit.contains(r#"data-readonly-group="time-b""#), + "foreign membership is explained as read-only" + ); + + let saved = send_request( + state.clone(), + "POST", + "/admin/users/shared-user/edit", + Some(&cookie), + Body::from( + "role=editor&groups=time-a&setor=Helpdesk&email=shared%40example.com&celular=", + ), + Some("application/x-www-form-urlencoded"), + ) + .await; + assert_eq!(saved.status(), StatusCode::SEE_OTHER); + let user = ruscker_admin::db::users::fetch(&db, "shared-user") + .await + .unwrap() + .unwrap(); + assert_eq!(user.role, Role::Editor); + assert_eq!( + user.groups, + vec!["time-a", "time-b"], + "out-of-scope group survives the replacement update" + ); + + let reset = send_request( + state, + "POST", + "/admin/users/viewer-a/password", + Some(&cookie), + Body::from("password=Helpdesk%21Pass9"), + Some("application/x-www-form-urlencoded"), + ) + .await; + assert_eq!(reset.status(), StatusCode::SEE_OTHER); + assert!( + ruscker_admin::db::users::verify_login(&db, "viewer-a", "Helpdesk!Pass9") + .await + .unwrap() + .is_some(), + "Editor helpdesk reset writes the new password" + ); + + let ruscker_admin::db::ConfigDb::Sqlite(pool) = &db else { + panic!("RBAC scope fixture must use SQLite"); + }; + let wrong_actor: (i64,) = sqlx::query_as( + "SELECT COUNT(*) FROM audit_log + WHERE action IN ('user.update', 'user.password') + AND target IN ('user:shared-user', 'user:viewer-a') + AND actor <> 'editor-a'", + ) + .fetch_one(pool) + .await + .unwrap(); + assert_eq!(wrong_actor.0, 0, "Editor is the audit actor on mutations"); +} + +#[tokio::test] +async fn scoped_editor_cannot_change_own_role_groups_or_password() { + let (state, db) = scoped_user_state().await; + let cookie = scoped_cookie(&state, Role::Editor, Some("editor-a")).await; + + for (uri, body) in [ + ("/admin/users/editor-a/role", "role=admin"), + ( + "/admin/users/editor-a/groups", + "groups=time-a%2Ctime-b", + ), + ( + "/admin/users/editor-a/edit", + "role=admin&groups=time-a%2Ctime-b&setor=&email=&celular=", + ), + ( + "/admin/users/editor-a/password", + "password=Escalate%21Pass9", + ), + ] { + let response = send_request( + state.clone(), + "POST", + uri, + Some(&cookie), + Body::from(body), + Some("application/x-www-form-urlencoded"), + ) + .await; + assert_eq!( + response.status(), + StatusCode::SEE_OTHER, + "self mutation is rejected with a localized flash: {uri}" + ); + assert!( + response + .headers() + .get("location") + .unwrap() + .to_str() + .unwrap() + .contains("flash=self-edit") + ); + } + + let editor = ruscker_admin::db::users::fetch(&db, "editor-a") + .await + .unwrap() + .unwrap(); + assert_eq!(editor.role, Role::Editor); + assert_eq!(editor.groups, vec!["time-a"]); +} + +#[tokio::test] +async fn scoped_user_count_search_and_pagination_share_one_sql_filter() { + let (state, db) = scoped_user_state().await; + // The fixture starts with three visible non-Admin users. Add 49 visible + // and 60 newer foreign rows: post-page filtering would make page 1 empty, + // while the SQL-scoped implementation still yields 50 + 2 visible rows. + for i in 0..49 { + seed_user(&db, &format!("team-a-{i:02}"), Role::Viewer, &["time-a"]).await; + } + for i in 0..60 { + seed_user(&db, &format!("team-b-{i:02}"), Role::Viewer, &["time-b"]).await; + } + let cookie = scoped_cookie(&state, Role::Editor, Some("editor-a")).await; + + let page_one = send_request( + state.clone(), + "GET", + "/admin/users", + Some(&cookie), + Body::empty(), + None, + ) + .await; + let page_one = response_body(page_one).await; + assert_eq!(page_one.matches("class=\"user-cell\"").count(), 50); + assert!(page_one.contains(r#"data-users-total="52""#)); + assert_eq!(metric_values(&page_one).first().copied(), Some("52")); + + let page_two = send_request( + state.clone(), + "GET", + "/admin/users?page=2", + Some(&cookie), + Body::empty(), + None, + ) + .await; + let page_two = response_body(page_two).await; + assert_eq!(page_two.matches("class=\"user-cell\"").count(), 2); + assert!(page_two.contains(r#"data-users-page="2""#)); + assert!(page_two.contains(r#"data-users-total="52""#)); + + let foreign_search = send_request( + state, + "GET", + "/admin/users?q=viewer-b", + Some(&cookie), + Body::empty(), + None, + ) + .await; + let foreign_search = response_body(foreign_search).await; + assert!(foreign_search.contains(r#"data-users-total="0""#)); + assert_eq!(foreign_search.matches("class=\"user-cell\"").count(), 0); +} + +#[tokio::test] +async fn admin_remains_unscoped_for_all_user_operations() { + let (state, db) = scoped_user_state().await; + let cookie = scoped_cookie(&state, Role::Admin, Some("admin-op")).await; + + let list = send_request( + state.clone(), + "GET", + "/admin/users", + Some(&cookie), + Body::empty(), + None, + ) + .await; + let list = response_body(list).await; + assert!(list.contains("viewer-b")); + assert!(list.contains("admin-a")); + assert!(list.contains(r#"value="admin""#)); + assert!(list.contains(r#"action="/admin/users/import""#)); + assert!(list.contains("/delete\"")); + + assert_eq!( + send( + state.clone(), + "GET", + "/admin/users/viewer-b/edit", + Some(&cookie), + ) + .await, + StatusCode::OK + ); + let created = send_request( + state.clone(), + "POST", + "/admin/users", + Some(&cookie), + Body::from( + "username=delegated-admin&password=Admin%21Pass9&role=admin&groups=time-b", + ), + Some("application/x-www-form-urlencoded"), + ) + .await; + assert_eq!(created.status(), StatusCode::SEE_OTHER); + assert_eq!( + ruscker_admin::db::users::fetch(&db, "delegated-admin") + .await + .unwrap() + .unwrap() + .role, + Role::Admin + ); + + assert_eq!( + send( + state.clone(), + "POST", + "/admin/users/viewer-b/mfa/reset", + Some(&cookie), + ) + .await, + StatusCode::SEE_OTHER + ); + assert_eq!( + send( + state.clone(), + "POST", + "/admin/users/viewer-b/delete", + Some(&cookie), + ) + .await, + StatusCode::SEE_OTHER + ); + assert!( + ruscker_admin::db::users::fetch(&db, "viewer-b") + .await + .unwrap() + .is_none() + ); + + let boundary = "RUSCKER-CSV-BOUNDARY"; + let multipart = format!( + "--{boundary}\r\n\ + Content-Disposition: form-data; name=\"file\"; filename=\"users.csv\"\r\n\ + Content-Type: text/csv\r\n\r\n\ + username,role,password,groups\r\n\ + csv-user,viewer,Csv!Pass12,time-b\r\n\ + --{boundary}--\r\n" + ); + let preview = send_request( + state.clone(), + "POST", + "/admin/users/import", + Some(&cookie), + Body::from(multipart), + Some(&format!("multipart/form-data; boundary={boundary}")), + ) + .await; + assert_eq!(preview.status(), StatusCode::OK); + + let confirm = send_request( + state, + "POST", + "/admin/users/import/confirm", + Some(&cookie), + Body::from( + "raw_csv=username%2Crole%2Cpassword%2Cgroups%0Acsv-user%2Cviewer%2CCsv%21Pass12%2Ctime-b%0A", + ), + Some("application/x-www-form-urlencoded"), + ) + .await; + assert_eq!(confirm.status(), StatusCode::SEE_OTHER); + assert!( + ruscker_admin::db::users::fetch(&db, "csv-user") + .await + .unwrap() + .is_some(), + "Admin bulk import remains functional" + ); +} From bb2fcfc555e2d9a652281a03b82e9bb54a1f3b50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Leite?= Date: Wed, 29 Jul 2026 20:14:02 -0300 Subject: [PATCH 2/2] fix: exclude admins case-insensitively from a scoped user query (review of #990 slice 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The scope filter excluded admin accounts with `role <> 'admin'`. That holds today only because `users.role` is plain TEXT with no CHECK and every writer goes through `Role::as_str`, which emits lowercase. A row stored as `Admin` — by hand, or by a future writer; #934 (OIDC/LDAP) adds one — would slip past the single boundary that keeps an Editor away from admin accounts, and only when that admin happened to share the Editor's group. Now `lower(role) <> 'admin'` in both dialect arms. There is no index on `role` to lose, and the users table is small (hugo runs ~120 accounts). The test seeds an admin row with `Admin` casing sharing the Editor's group and asserts it stays out of the list. I verified it FAILS with the old predicate and passes with the new one, so it pins the hole rather than merely describing it. Co-Authored-By: Claude Opus 5 (1M context) --- crates/ruscker-admin/src/db/users.rs | 10 ++++-- crates/ruscker-admin/tests/rbac.rs | 50 ++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/crates/ruscker-admin/src/db/users.rs b/crates/ruscker-admin/src/db/users.rs index bf12ad7f..84c2fcc7 100644 --- a/crates/ruscker-admin/src/db/users.rs +++ b/crates/ruscker-admin/src/db/users.rs @@ -361,7 +361,12 @@ fn push_visibility_sqlite( let Some(groups) = visible_groups else { return; }; - qb.push(" AND role <> 'admin' AND ("); + // `lower(role)` rather than a bare comparison: the column is plain TEXT + // with no CHECK, so it is only lowercase because every write goes through + // `Role::as_str`. An externally provisioned row — the OIDC/LDAP work of + // #934 adds another writer — storing `Admin` must not slip past the one + // boundary that keeps Editors away from admin accounts. + qb.push(" AND lower(role) <> 'admin' AND ("); if groups.is_empty() { qb.push("1=0"); } else { @@ -384,7 +389,8 @@ fn push_visibility_postgres( let Some(groups) = visible_groups else { return; }; - qb.push(" AND role <> 'admin' AND ("); + // Same case-insensitive guard as the SQLite arm — see there for why. + qb.push(" AND lower(role) <> 'admin' AND ("); if groups.is_empty() { qb.push("1=0"); } else { diff --git a/crates/ruscker-admin/tests/rbac.rs b/crates/ruscker-admin/tests/rbac.rs index 8d690745..7d4f5c8f 100644 --- a/crates/ruscker-admin/tests/rbac.rs +++ b/crates/ruscker-admin/tests/rbac.rs @@ -1133,6 +1133,56 @@ async fn scoped_editor_cannot_change_own_role_groups_or_password() { assert_eq!(editor.groups, vec!["time-a"]); } +/// The admin-exclusion boundary must not depend on the stored casing of +/// `role`. That column is plain TEXT with no CHECK — it is lowercase only +/// because every write goes through `Role::as_str`, and #934 (OIDC/LDAP) +/// will add another writer. A row stored as `Admin` must still be invisible +/// to a scoped Editor. +#[tokio::test] +async fn scoped_editor_never_sees_an_admin_stored_with_odd_casing() { + let (state, db) = scoped_user_state().await; + let ruscker_admin::db::ConfigDb::Sqlite(pool) = &db else { + panic!("fixture must use SQLite"); + }; + let now = Utc::now(); + sqlx::query( + "INSERT INTO users + (id, username, password_hash, role, must_change_password, + groups, created_at, updated_at, created_by) + VALUES (?, ?, ?, 'Admin', 0, 'time-a', ?, ?, NULL)", + ) + .bind(uuid::Uuid::new_v4().to_string()) + .bind("shouty-admin") + .bind("unused-test-hash") + .bind(now) + .bind(now) + .execute(pool) + .await + .unwrap(); + + let cookie = scoped_cookie(&state, Role::Editor, Some("editor-a")).await; + let response = send_request( + state.clone(), + "GET", + "/admin/users", + Some(&cookie), + Body::empty(), + None, + ) + .await; + let body = String::from_utf8( + axum::body::to_bytes(response.into_body(), usize::MAX) + .await + .unwrap() + .to_vec(), + ) + .unwrap(); + assert!( + !body.contains("shouty-admin"), + "an admin row sharing the Editor's group must stay hidden regardless of casing" + ); +} + #[tokio::test] async fn scoped_user_count_search_and_pagination_share_one_sql_filter() { let (state, db) = scoped_user_state().await;