From d6de75dd97eefbc52e2f6ee1efab0268b60bd8fd Mon Sep 17 00:00:00 2001 From: "benjamin.747" Date: Thu, 6 Aug 2026 14:48:54 +0800 Subject: [PATCH] fix(identity): harden admin checks and campsite_user_id backfill Always treat monorepo.admin as runtime admins (union with Cedar), resolve identity backfill PK collisions before renaming handles, and return 404 when a CL reviewer is missing instead of 500. --- .../api_service/mono/admin/permissions.rs | 72 +++++++++++++------ config/config-workflow.toml | 2 +- config/config.toml | 3 +- jupiter/src/storage/cl_reviewer_storage.rs | 4 +- jupiter/src/storage/data_backfill_storage.rs | 61 ++++++++++++++-- .../AdminGroups/AddMembersDialog.tsx | 2 +- .../AdminGroups/CreateGroupDialog.tsx | 2 +- .../AdminGroups/DeleteGroupDialog.tsx | 2 +- .../AdminGroups/GroupMembersDialog.tsx | 2 +- 9 files changed, 117 insertions(+), 33 deletions(-) diff --git a/ceres/src/application/api_service/mono/admin/permissions.rs b/ceres/src/application/api_service/mono/admin/permissions.rs index 1c7cee8de..5b2ea3b14 100644 --- a/ceres/src/application/api_service/mono/admin/permissions.rs +++ b/ceres/src/application/api_service/mono/admin/permissions.rs @@ -1,13 +1,16 @@ //! Global admin permission operations. //! -//! This module provides admin permission checking for the monorepo system. -//! All admin permissions are defined in a single `.mega_cedar.json` file -//! located in the root directory (`/`). +//! Effective admins are the **union** of: +//! - `monorepo.admin` in mega config (always applied; used at monorepo init and at runtime) +//! - users in the root `/.mega_cedar.json` admin group //! //! # Design -//! - A single global admin list applies to the entire monorepo -//! - The admin configuration file is stored at `/.mega_cedar.json` -//! - Redis caching is used to avoid repeated file parsing +//! - Config admins are checked on every request (not baked into Redis), so they +//! remain valid even when Cedar/Redis is stale or missing +//! - Cedar-derived admins are Redis-cached (TTL 10 minutes) to avoid re-parsing +//! `.mega_cedar.json` + +use std::collections::BTreeSet; use common::errors::MegaError; use git_internal::internal::object::tree::Tree; @@ -15,36 +18,65 @@ use jupiter::{redis::AsyncCommands, utils::converter::FromMegaModel}; use crate::application::api_service::mono::context::AdminApplicationService; -/// Cache TTL for admin list (10 minutes). +/// Cache TTL for Cedar admin list (10 minutes). pub const ADMIN_CACHE_TTL: u64 = 600; /// The Cedar entity file name in root directory. pub const ADMIN_FILE: &str = ".mega_cedar.json"; -/// Redis cache key suffix for admin list. +/// Redis cache key suffix for Cedar admin list (config admins are merged at read time). const ADMIN_CACHE_KEY_SUFFIX: &str = "admin:list"; impl AdminApplicationService { - /// Check if a user is an admin. + /// Check if a user is an admin (config `monorepo.admin` or Cedar). pub async fn check_is_admin(&self, username: &str) -> Result { + let username = username.trim(); + if username.is_empty() { + return Ok(false); + } let admins = self.get_effective_admins().await?; - Ok(admins.contains(&username.to_string())) + Ok(admins.iter().any(|a| a == username)) } - /// Retrieve all admin usernames. + /// Retrieve all effective admin identities (config ∪ Cedar), sorted uniquely. pub async fn get_all_admins(&self) -> Result, MegaError> { self.get_effective_admins().await } - /// Get admins from cache or storage. - /// This method first attempts to read from Redis cache. On cache miss, - /// it loads the admin list from the `.mega_cedar.json` file and caches - /// the result. + /// GitHub logins (or Cedar euids) listed under `[monorepo] admin` in config. + fn config_admins(&self) -> Vec { + self.ctx + .storage() + .config() + .monorepo + .admin + .iter() + .map(|s| s.trim().to_string()) + .filter(|s| !s.is_empty()) + .collect() + } + + /// Merge Cedar admins with config admins (sorted, unique). + fn merge_with_config_admins(&self, cedar_admins: Vec) -> Vec { + let mut set: BTreeSet = cedar_admins.into_iter().collect(); + for admin in self.config_admins() { + set.insert(admin); + } + set.into_iter().collect() + } + + /// Get effective admins: Redis/Cedar list ∪ `monorepo.admin`. /// - /// If `.mega_cedar.json` (or root refs) are missing, returns an empty list - /// so callers can fall through to other authz paths (e.g. user approval) - /// instead of hard-failing the request. + /// Config admins are always merged after cache/file load so a stale Redis + /// Cedar list cannot drop configured admins. If `.mega_cedar.json` is + /// missing, config admins alone still apply. async fn get_effective_admins(&self) -> Result, MegaError> { + let cedar_admins = self.get_cedar_admins().await?; + Ok(self.merge_with_config_admins(cedar_admins)) + } + + /// Cedar-only admin list (cached). Does not include config admins. + async fn get_cedar_admins(&self) -> Result, MegaError> { if let Ok(admins) = self.get_admins_from_cache().await { return Ok(admins); } @@ -54,7 +86,7 @@ impl AdminApplicationService { Err(e) if is_admin_config_unavailable(&e) => { tracing::warn!( error = %e, - "Admin config unavailable; treating as empty admin list" + "Admin Cedar config unavailable; using monorepo.admin from config only" ); return Ok(Vec::new()); } @@ -70,7 +102,7 @@ impl AdminApplicationService { Ok(admins) } - /// Invalidate the admin list cache. + /// Invalidate the Cedar admin list cache. /// This should be called when the `.mega_cedar.json` file is modified. pub async fn invalidate_admin_cache(&self) { let mut conn = self.ctx.git_object_cache().connection.clone(); diff --git a/config/config-workflow.toml b/config/config-workflow.toml index d4d934b77..448c167b6 100644 --- a/config/config-workflow.toml +++ b/config/config-workflow.toml @@ -67,7 +67,7 @@ test_user_token = "mega" ## Mega treats files under this directory as import repo and other directories as monorepo import_dir = "/third-party" -# Set System Admin in directory init, replace the admin's github username here +# System admin GitHub logins (init + always treated as admin at runtime) admin = "admin" # Set serveral root dirs in directory init diff --git a/config/config.toml b/config/config.toml index 6421bff9e..3808f8b86 100644 --- a/config/config.toml +++ b/config/config.toml @@ -52,7 +52,8 @@ sqlx_logging = false ## Mega treats files under this directory as import repo and other directories as monorepo import_dir = "/third-party" -# Set System Admin(s) in directory init, these users will be added to the admin group +# System admin GitHub logins. Used when initializing /.mega_cedar.json and +# always treated as admins at runtime (union with Cedar admin group). # Supports multiple admins: admin = ["user1", "user2", "user3"] admin = ["benjamin-747"] diff --git a/jupiter/src/storage/cl_reviewer_storage.rs b/jupiter/src/storage/cl_reviewer_storage.rs index 645ccc969..357fd1d36 100644 --- a/jupiter/src/storage/cl_reviewer_storage.rs +++ b/jupiter/src/storage/cl_reviewer_storage.rs @@ -71,7 +71,7 @@ impl ClReviewerStorage { MegaError::Other(format!("fail to find reviewer {}", campsite_user_id)) })? .ok_or_else(|| { - MegaError::Other(format!("reviewer {} not found", campsite_user_id)) + MegaError::NotFound(format!("reviewer {} not found", campsite_user_id)) })? .into_active_model(); @@ -201,7 +201,7 @@ impl ClReviewerStorage { tracing::error!("{}", e); MegaError::Other(format!("fail to find reviewer {}", campsite_user_id)) })? - .ok_or_else(|| MegaError::Other(format!("reviewer {} not found", campsite_user_id)))? + .ok_or_else(|| MegaError::NotFound(format!("reviewer {} not found", campsite_user_id)))? .into_active_model(); rev.approved = Set(approved); diff --git a/jupiter/src/storage/data_backfill_storage.rs b/jupiter/src/storage/data_backfill_storage.rs index 971f63294..94561000e 100644 --- a/jupiter/src/storage/data_backfill_storage.rs +++ b/jupiter/src/storage/data_backfill_storage.rs @@ -155,21 +155,72 @@ impl DataBackfillStorage { let i = esc(id); let login_sql = github.map(esc).unwrap_or_default(); + // Tables where campsite_user_id is the sole PK: drop the handle + // row when the target id already exists, then rename remaining. + for table in [ + "cla_sign_status", + "user_approval_status", + "user_notification_settings", + "user_notification_preferences", + ] { + affected += exec_unprepared( + &txn, + &format!( + r#" + DELETE FROM {table} + WHERE campsite_user_id = '{h}' + AND EXISTS ( + SELECT 1 FROM {table} AS keep + WHERE keep.campsite_user_id = '{i}' + ) + "# + ), + ) + .await?; + affected += exec_unprepared( + &txn, + &format!( + "UPDATE {table} SET campsite_user_id = '{i}' WHERE campsite_user_id = '{h}'" + ), + ) + .await?; + } + + // Composite PK (item_id, campsite_user_id). + affected += exec_unprepared( + &txn, + &format!( + r#" + DELETE FROM item_assignees AS old_row + WHERE old_row.campsite_user_id = '{h}' + AND EXISTS ( + SELECT 1 FROM item_assignees AS new_row + WHERE new_row.item_id = old_row.item_id + AND new_row.item_type = old_row.item_type + AND new_row.campsite_user_id = '{i}' + ) + "# + ), + ) + .await?; + affected += exec_unprepared( + &txn, + &format!( + "UPDATE item_assignees SET campsite_user_id = '{i}' WHERE campsite_user_id = '{h}'" + ), + ) + .await?; + for table in [ "mega_cl", "mega_issue", "mega_conversation", "reactions", - "item_assignees", "mega_code_review_comment", "access_token", "ssh_keys", - "cla_sign_status", - "user_notification_settings", - "user_notification_preferences", "email_jobs", "mega_group_member", - "user_approval_status", ] { affected += exec_unprepared( &txn, diff --git a/moon/apps/web/components/AdminGroups/AddMembersDialog.tsx b/moon/apps/web/components/AdminGroups/AddMembersDialog.tsx index 25aa8ecc2..648f32bd2 100644 --- a/moon/apps/web/components/AdminGroups/AddMembersDialog.tsx +++ b/moon/apps/web/components/AdminGroups/AddMembersDialog.tsx @@ -75,7 +75,7 @@ export const AddMembersDialog = ({ groupId, onClose }: AddMembersDialogProps) => if (groupId === null) return null return ( -
+
{/* Fixed header */}
diff --git a/moon/apps/web/components/AdminGroups/CreateGroupDialog.tsx b/moon/apps/web/components/AdminGroups/CreateGroupDialog.tsx index ee835ac14..c0a850966 100644 --- a/moon/apps/web/components/AdminGroups/CreateGroupDialog.tsx +++ b/moon/apps/web/components/AdminGroups/CreateGroupDialog.tsx @@ -61,7 +61,7 @@ export const CreateGroupDialog = ({ isOpen, onClose }: CreateGroupDialogProps) = if (!isOpen) return null return ( -
+

Create New Group

diff --git a/moon/apps/web/components/AdminGroups/DeleteGroupDialog.tsx b/moon/apps/web/components/AdminGroups/DeleteGroupDialog.tsx index cea5803bf..2869584f9 100644 --- a/moon/apps/web/components/AdminGroups/DeleteGroupDialog.tsx +++ b/moon/apps/web/components/AdminGroups/DeleteGroupDialog.tsx @@ -26,7 +26,7 @@ export const DeleteGroupDialog = ({ groupId, onClose }: DeleteGroupDialogProps) if (groupId === null) return null return ( -
+

Delete Group

diff --git a/moon/apps/web/components/AdminGroups/GroupMembersDialog.tsx b/moon/apps/web/components/AdminGroups/GroupMembersDialog.tsx index 9ba3ed14e..656829eaf 100644 --- a/moon/apps/web/components/AdminGroups/GroupMembersDialog.tsx +++ b/moon/apps/web/components/AdminGroups/GroupMembersDialog.tsx @@ -46,7 +46,7 @@ export const GroupMembersDialog = ({ groupId, groupName, onClose }: GroupMembers if (groupId === null) return null return ( -
+
{/* Fixed header */}