diff --git a/src/db/rbac.rs b/src/db/rbac.rs index b90d760..f567ac9 100644 --- a/src/db/rbac.rs +++ b/src/db/rbac.rs @@ -478,24 +478,54 @@ pub enum DeleteRoleResult { HasUsers(i64), } -/// Idempotently ensure the admin role has all permissions from Permission::ALL. +/// Permissions that the moderator role should have. +/// Everything except admin-only permissions (UsersManage, SettingsManage, NotesManage). +const MODERATOR_PERMISSIONS: &[Permission] = &[ + Permission::ModsInstall, + Permission::ModsUpdate, + Permission::ModsRemove, + Permission::ModsDisable, + Permission::ModsConfigEdit, + Permission::ConvoyManage, + Permission::SvmEdit, + Permission::RequestsResolve, + Permission::ServerControl, + Permission::ServerLogs, + Permission::ServerMetrics, + Permission::HeadlessManage, + Permission::QueueManage, + Permission::ItemsGive, + Permission::NotesEdit, +]; + +/// Idempotently ensure built-in roles have their expected permissions. /// Called after migrations to pick up newly added permissions on upgrade. /// Additive only — never removes permissions. pub fn sync_builtin_role_permissions(conn: &Connection) -> rusqlite::Result<()> { - let admin_id: Option = conn - .query_row("SELECT id FROM roles WHERE name = 'admin'", [], |row| { - row.get(0) - }) - .optional()?; - - if let Some(id) = admin_id { - for perm in Permission::ALL { - conn.execute( - "INSERT OR IGNORE INTO role_permissions (role_id, permission) VALUES (?1, ?2)", - params![id, perm.as_str()], - )?; + // ponytail: single query per role, INSERT OR IGNORE is idempotent + let sync_role = |role_name: &str, perms: &[Permission]| -> rusqlite::Result<()> { + let role_id: Option = conn + .query_row( + "SELECT id FROM roles WHERE name = ?1", + params![role_name], + |row| row.get(0), + ) + .optional()?; + + if let Some(id) = role_id { + for perm in perms { + conn.execute( + "INSERT OR IGNORE INTO role_permissions (role_id, permission) VALUES (?1, ?2)", + params![id, perm.as_str()], + )?; + } } - } + Ok(()) + }; + + sync_role("admin", Permission::ALL)?; + sync_role("moderator", MODERATOR_PERMISSIONS)?; + // player intentionally has no permissions Ok(()) } @@ -534,10 +564,15 @@ mod tests { fn moderator_permissions() { let db = Database::open_in_memory().unwrap(); let perms = db.get_permissions_for_role("moderator").unwrap(); - assert!(perms.contains(&Permission::ModsInstall)); - assert!(perms.contains(&Permission::ServerControl)); + // Moderator should have exactly the MODERATOR_PERMISSIONS set + assert_eq!(perms.len(), MODERATOR_PERMISSIONS.len()); + for p in MODERATOR_PERMISSIONS { + assert!(perms.contains(p), "moderator missing {:?}", p); + } + // Admin-only permissions must be absent assert!(!perms.contains(&Permission::UsersManage)); assert!(!perms.contains(&Permission::SettingsManage)); + assert!(!perms.contains(&Permission::NotesManage)); } #[test] @@ -670,4 +705,22 @@ mod tests { assert_eq!(perms_after_sync.len(), Permission::ALL.len()); assert!(perms_after_sync.contains(&Permission::SettingsManage)); } + + #[test] + fn sync_adds_new_moderator_permissions() { + let db = Database::open_in_memory().unwrap(); + // Simulate a pre-upgrade state: remove a permission that sync should restore + db.conn().execute( + "DELETE FROM role_permissions WHERE role_id = (SELECT id FROM roles WHERE name = 'moderator') AND permission = 'items.give'", + [], + ).unwrap(); + let perms = db.get_permissions_for_role("moderator").unwrap(); + assert!(!perms.contains(&Permission::ItemsGive)); + + sync_builtin_role_permissions(db.conn()).unwrap(); + let perms = db.get_permissions_for_role("moderator").unwrap(); + assert!(perms.contains(&Permission::ItemsGive)); + // Still no admin-only permissions + assert!(!perms.contains(&Permission::UsersManage)); + } } diff --git a/src/web/auth.rs b/src/web/auth.rs index e1df849..eb546a6 100644 --- a/src/web/auth.rs +++ b/src/web/auth.rs @@ -99,6 +99,17 @@ pub fn require_permission( Ok(()) } +/// Require that the requesting user is viewing their own data or has UsersManage. +pub fn require_self_or_admin( + user: &SessionUser, + target_username: &str, +) -> std::result::Result<(), WebError> { + if user.username != target_username && !user.has_permission(Permission::UsersManage) { + return Err(WebError::Forbidden); + } + Ok(()) +} + pub fn set_session_user(session: &Session, user: &SessionUser) -> Result<()> { session .insert("user_id", user.user_id) @@ -313,6 +324,34 @@ mod tests { assert!(!user.can("nonexistent.perm")); } + #[test] + fn require_self_or_admin_allows_own_data() { + let user = SessionUser { + user_id: 1, + username: "alice".into(), + role_name: "player".into(), + role_display_name: "Player".into(), + permissions: HashSet::new(), + has_password: true, + }; + assert!(require_self_or_admin(&user, "alice").is_ok()); + assert!(require_self_or_admin(&user, "bob").is_err()); + } + + #[test] + fn require_self_or_admin_allows_users_manage() { + let user = SessionUser { + user_id: 1, + username: "admin".into(), + role_name: "admin".into(), + role_display_name: "Admin".into(), + permissions: HashSet::from([Permission::UsersManage]), + has_password: true, + }; + assert!(require_self_or_admin(&user, "alice").is_ok()); + assert!(require_self_or_admin(&user, "bob").is_ok()); + } + #[test] fn password_complexity_validation() { // Too short diff --git a/src/web/handlers/mods.rs b/src/web/handlers/mods.rs index ec9d45e..0b7c83d 100644 --- a/src/web/handlers/mods.rs +++ b/src/web/handlers/mods.rs @@ -3348,7 +3348,7 @@ pub async fn groups_partial( session: Session, ) -> actix_web::Result { let user = require_auth(&req)?; - require_permission(&user, Permission::ModsInstall)?; + require_permission(&user, Permission::ConvoyManage)?; let csrf_token = crate::web::csrf::get_or_create_token(&session); let html = render_groups_tab(&state, &csrf_token).await?; @@ -3370,7 +3370,7 @@ pub async fn new_group_card( query: Query, ) -> actix_web::Result { let user = require_auth(&req)?; - require_permission(&user, Permission::ModsInstall)?; + require_permission(&user, Permission::ConvoyManage)?; let _csrf_token = crate::web::csrf::get_or_create_token(&session); let all_mods = fetch_mods_with_client_files(&state).await?; @@ -3447,7 +3447,7 @@ pub async fn save_groups( body: web::Json, ) -> actix_web::Result { let user = require_auth(&req)?; - require_permission(&user, Permission::ModsInstall)?; + require_permission(&user, Permission::ConvoyManage)?; if !crate::web::csrf::validate_token(&session, &body.csrf_token) { return Err(WebError::Forbidden.into()); diff --git a/src/web/handlers/profiles.rs b/src/web/handlers/profiles.rs index 096d622..127e627 100644 --- a/src/web/handlers/profiles.rs +++ b/src/web/handlers/profiles.rs @@ -7,7 +7,7 @@ use askama::Template; use serde::Deserialize; use crate::spt::profiles::{load_profile_detail, load_stash_items, ProfileDetail, QuestState}; -use crate::web::auth::require_auth; +use crate::web::auth::{require_auth, require_self_or_admin}; use crate::web::csrf; use crate::web::error::WebError; use crate::web::flash::{take_flash, FlashMessage}; @@ -125,6 +125,7 @@ pub async fn profile_page( let flash = take_flash(&session); let csrf_token = csrf::get_or_create_token(&session); let profile_username = path.into_inner(); + require_self_or_admin(&user, &profile_username)?; let db = state.db.clone(); let dirs = Arc::clone(&state.dirs); @@ -252,8 +253,9 @@ pub async fn quests_partial( path: Path, query: Query, ) -> actix_web::Result { - require_auth(&req)?; + let user = require_auth(&req)?; let profile_username = path.into_inner(); + require_self_or_admin(&user, &profile_username)?; let detail = load_detail_for_user(&state, &profile_username).await?; let total = detail.quests.len(); @@ -306,8 +308,9 @@ pub async fn traders_partial( req: HttpRequest, path: Path, ) -> actix_web::Result { - require_auth(&req)?; + let user = require_auth(&req)?; let profile_username = path.into_inner(); + require_self_or_admin(&user, &profile_username)?; let detail = load_detail_for_user(&state, &profile_username).await?; let trader_displays: Vec = detail @@ -338,8 +341,9 @@ pub async fn hideout_partial( req: HttpRequest, path: Path, ) -> actix_web::Result { - require_auth(&req)?; + let user = require_auth(&req)?; let profile_username = path.into_inner(); + require_self_or_admin(&user, &profile_username)?; let detail = load_detail_for_user(&state, &profile_username).await?; let area_displays: Vec = detail @@ -405,8 +409,9 @@ pub async fn stash_partial( path: Path, query: Query, ) -> actix_web::Result { - require_auth(&req)?; + let user = require_auth(&req)?; let profile_username = path.into_inner(); + require_self_or_admin(&user, &profile_username)?; let db = state.db.clone(); let lookup_username = profile_username.clone(); diff --git a/src/web/handlers/raids.rs b/src/web/handlers/raids.rs index fb3cb7b..da1e999 100644 --- a/src/web/handlers/raids.rs +++ b/src/web/handlers/raids.rs @@ -5,7 +5,7 @@ use askama::Template; use serde::Deserialize; use crate::db::raids::{LeaderboardEntry, Raid, RaidKill, ServerRaidStats, UserRaidStats}; -use crate::web::auth::require_auth; +use crate::web::auth::{require_auth, require_self_or_admin}; use crate::web::csrf; use crate::web::error::WebError; use crate::web::flash::{take_flash, FlashMessage}; @@ -175,6 +175,7 @@ pub async fn player_raids_page( let flash = take_flash(&session); let csrf_token = csrf::get_or_create_token(&session); let profile_username = path.into_inner(); + require_self_or_admin(&user, &profile_username)?; let offset = query.offset.unwrap_or(0); let db = state.db.clone(); @@ -250,6 +251,8 @@ pub async fn raid_detail_page( .map_err(WebError::from)? .map_err(|_| WebError::NotFound)?; + require_self_or_admin(&user, &raid_username)?; + let tmpl = RaidDetailPageTemplate { user, flash,