Skip to content

Commit e1aeac7

Browse files
authored
fix(identity): harden admin checks and campsite_user_id backfill (#2166)
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.
1 parent a8abbbc commit e1aeac7

9 files changed

Lines changed: 117 additions & 33 deletions

File tree

ceres/src/application/api_service/mono/admin/permissions.rs

Lines changed: 52 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,50 +1,82 @@
11
//! Global admin permission operations.
22
//!
3-
//! This module provides admin permission checking for the monorepo system.
4-
//! All admin permissions are defined in a single `.mega_cedar.json` file
5-
//! located in the root directory (`/`).
3+
//! Effective admins are the **union** of:
4+
//! - `monorepo.admin` in mega config (always applied; used at monorepo init and at runtime)
5+
//! - users in the root `/.mega_cedar.json` admin group
66
//!
77
//! # Design
8-
//! - A single global admin list applies to the entire monorepo
9-
//! - The admin configuration file is stored at `/.mega_cedar.json`
10-
//! - Redis caching is used to avoid repeated file parsing
8+
//! - Config admins are checked on every request (not baked into Redis), so they
9+
//! remain valid even when Cedar/Redis is stale or missing
10+
//! - Cedar-derived admins are Redis-cached (TTL 10 minutes) to avoid re-parsing
11+
//! `.mega_cedar.json`
12+
13+
use std::collections::BTreeSet;
1114

1215
use common::errors::MegaError;
1316
use git_internal::internal::object::tree::Tree;
1417
use jupiter::{redis::AsyncCommands, utils::converter::FromMegaModel};
1518

1619
use crate::application::api_service::mono::context::AdminApplicationService;
1720

18-
/// Cache TTL for admin list (10 minutes).
21+
/// Cache TTL for Cedar admin list (10 minutes).
1922
pub const ADMIN_CACHE_TTL: u64 = 600;
2023

2124
/// The Cedar entity file name in root directory.
2225
pub const ADMIN_FILE: &str = ".mega_cedar.json";
2326

24-
/// Redis cache key suffix for admin list.
27+
/// Redis cache key suffix for Cedar admin list (config admins are merged at read time).
2528
const ADMIN_CACHE_KEY_SUFFIX: &str = "admin:list";
2629

2730
impl AdminApplicationService {
28-
/// Check if a user is an admin.
31+
/// Check if a user is an admin (config `monorepo.admin` or Cedar).
2932
pub async fn check_is_admin(&self, username: &str) -> Result<bool, MegaError> {
33+
let username = username.trim();
34+
if username.is_empty() {
35+
return Ok(false);
36+
}
3037
let admins = self.get_effective_admins().await?;
31-
Ok(admins.contains(&username.to_string()))
38+
Ok(admins.iter().any(|a| a == username))
3239
}
3340

34-
/// Retrieve all admin usernames.
41+
/// Retrieve all effective admin identities (config ∪ Cedar), sorted uniquely.
3542
pub async fn get_all_admins(&self) -> Result<Vec<String>, MegaError> {
3643
self.get_effective_admins().await
3744
}
3845

39-
/// Get admins from cache or storage.
40-
/// This method first attempts to read from Redis cache. On cache miss,
41-
/// it loads the admin list from the `.mega_cedar.json` file and caches
42-
/// the result.
46+
/// GitHub logins (or Cedar euids) listed under `[monorepo] admin` in config.
47+
fn config_admins(&self) -> Vec<String> {
48+
self.ctx
49+
.storage()
50+
.config()
51+
.monorepo
52+
.admin
53+
.iter()
54+
.map(|s| s.trim().to_string())
55+
.filter(|s| !s.is_empty())
56+
.collect()
57+
}
58+
59+
/// Merge Cedar admins with config admins (sorted, unique).
60+
fn merge_with_config_admins(&self, cedar_admins: Vec<String>) -> Vec<String> {
61+
let mut set: BTreeSet<String> = cedar_admins.into_iter().collect();
62+
for admin in self.config_admins() {
63+
set.insert(admin);
64+
}
65+
set.into_iter().collect()
66+
}
67+
68+
/// Get effective admins: Redis/Cedar list ∪ `monorepo.admin`.
4369
///
44-
/// If `.mega_cedar.json` (or root refs) are missing, returns an empty list
45-
/// so callers can fall through to other authz paths (e.g. user approval)
46-
/// instead of hard-failing the request.
70+
/// Config admins are always merged after cache/file load so a stale Redis
71+
/// Cedar list cannot drop configured admins. If `.mega_cedar.json` is
72+
/// missing, config admins alone still apply.
4773
async fn get_effective_admins(&self) -> Result<Vec<String>, MegaError> {
74+
let cedar_admins = self.get_cedar_admins().await?;
75+
Ok(self.merge_with_config_admins(cedar_admins))
76+
}
77+
78+
/// Cedar-only admin list (cached). Does not include config admins.
79+
async fn get_cedar_admins(&self) -> Result<Vec<String>, MegaError> {
4880
if let Ok(admins) = self.get_admins_from_cache().await {
4981
return Ok(admins);
5082
}
@@ -54,7 +86,7 @@ impl AdminApplicationService {
5486
Err(e) if is_admin_config_unavailable(&e) => {
5587
tracing::warn!(
5688
error = %e,
57-
"Admin config unavailable; treating as empty admin list"
89+
"Admin Cedar config unavailable; using monorepo.admin from config only"
5890
);
5991
return Ok(Vec::new());
6092
}
@@ -70,7 +102,7 @@ impl AdminApplicationService {
70102
Ok(admins)
71103
}
72104

73-
/// Invalidate the admin list cache.
105+
/// Invalidate the Cedar admin list cache.
74106
/// This should be called when the `.mega_cedar.json` file is modified.
75107
pub async fn invalidate_admin_cache(&self) {
76108
let mut conn = self.ctx.git_object_cache().connection.clone();

config/config-workflow.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ test_user_token = "mega"
6767
## Mega treats files under this directory as import repo and other directories as monorepo
6868
import_dir = "/third-party"
6969

70-
# Set System Admin in directory init, replace the admin's github username here
70+
# System admin GitHub logins (init + always treated as admin at runtime)
7171
admin = "admin"
7272

7373
# Set serveral root dirs in directory init

config/config.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,8 @@ sqlx_logging = false
5252
## Mega treats files under this directory as import repo and other directories as monorepo
5353
import_dir = "/third-party"
5454

55-
# Set System Admin(s) in directory init, these users will be added to the admin group
55+
# System admin GitHub logins. Used when initializing /.mega_cedar.json and
56+
# always treated as admins at runtime (union with Cedar admin group).
5657
# Supports multiple admins: admin = ["user1", "user2", "user3"]
5758
admin = ["benjamin-747"]
5859

jupiter/src/storage/cl_reviewer_storage.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ impl ClReviewerStorage {
7171
MegaError::Other(format!("fail to find reviewer {}", campsite_user_id))
7272
})?
7373
.ok_or_else(|| {
74-
MegaError::Other(format!("reviewer {} not found", campsite_user_id))
74+
MegaError::NotFound(format!("reviewer {} not found", campsite_user_id))
7575
})?
7676
.into_active_model();
7777

@@ -201,7 +201,7 @@ impl ClReviewerStorage {
201201
tracing::error!("{}", e);
202202
MegaError::Other(format!("fail to find reviewer {}", campsite_user_id))
203203
})?
204-
.ok_or_else(|| MegaError::Other(format!("reviewer {} not found", campsite_user_id)))?
204+
.ok_or_else(|| MegaError::NotFound(format!("reviewer {} not found", campsite_user_id)))?
205205
.into_active_model();
206206

207207
rev.approved = Set(approved);

jupiter/src/storage/data_backfill_storage.rs

Lines changed: 56 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -155,21 +155,72 @@ impl DataBackfillStorage {
155155
let i = esc(id);
156156
let login_sql = github.map(esc).unwrap_or_default();
157157

158+
// Tables where campsite_user_id is the sole PK: drop the handle
159+
// row when the target id already exists, then rename remaining.
160+
for table in [
161+
"cla_sign_status",
162+
"user_approval_status",
163+
"user_notification_settings",
164+
"user_notification_preferences",
165+
] {
166+
affected += exec_unprepared(
167+
&txn,
168+
&format!(
169+
r#"
170+
DELETE FROM {table}
171+
WHERE campsite_user_id = '{h}'
172+
AND EXISTS (
173+
SELECT 1 FROM {table} AS keep
174+
WHERE keep.campsite_user_id = '{i}'
175+
)
176+
"#
177+
),
178+
)
179+
.await?;
180+
affected += exec_unprepared(
181+
&txn,
182+
&format!(
183+
"UPDATE {table} SET campsite_user_id = '{i}' WHERE campsite_user_id = '{h}'"
184+
),
185+
)
186+
.await?;
187+
}
188+
189+
// Composite PK (item_id, campsite_user_id).
190+
affected += exec_unprepared(
191+
&txn,
192+
&format!(
193+
r#"
194+
DELETE FROM item_assignees AS old_row
195+
WHERE old_row.campsite_user_id = '{h}'
196+
AND EXISTS (
197+
SELECT 1 FROM item_assignees AS new_row
198+
WHERE new_row.item_id = old_row.item_id
199+
AND new_row.item_type = old_row.item_type
200+
AND new_row.campsite_user_id = '{i}'
201+
)
202+
"#
203+
),
204+
)
205+
.await?;
206+
affected += exec_unprepared(
207+
&txn,
208+
&format!(
209+
"UPDATE item_assignees SET campsite_user_id = '{i}' WHERE campsite_user_id = '{h}'"
210+
),
211+
)
212+
.await?;
213+
158214
for table in [
159215
"mega_cl",
160216
"mega_issue",
161217
"mega_conversation",
162218
"reactions",
163-
"item_assignees",
164219
"mega_code_review_comment",
165220
"access_token",
166221
"ssh_keys",
167-
"cla_sign_status",
168-
"user_notification_settings",
169-
"user_notification_preferences",
170222
"email_jobs",
171223
"mega_group_member",
172-
"user_approval_status",
173224
] {
174225
affected += exec_unprepared(
175226
&txn,

moon/apps/web/components/AdminGroups/AddMembersDialog.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ export const AddMembersDialog = ({ groupId, onClose }: AddMembersDialogProps) =>
7575
if (groupId === null) return null
7676

7777
return (
78-
<div className='bg-opacity-50 fixed inset-0 z-50 flex items-center justify-center bg-black p-4'>
78+
<div className='fixed inset-0 z-50 flex items-center justify-center bg-black/50 p-4'>
7979
<div className='bg-primary border-primary flex max-h-[90vh] w-full max-w-2xl flex-col rounded-lg border shadow-xl'>
8080
{/* Fixed header */}
8181
<div className='shrink-0 border-b border-gray-200 px-6 py-4 dark:border-gray-700'>

moon/apps/web/components/AdminGroups/CreateGroupDialog.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ export const CreateGroupDialog = ({ isOpen, onClose }: CreateGroupDialogProps) =
6161
if (!isOpen) return null
6262

6363
return (
64-
<div className='bg-opacity-50 fixed inset-0 z-50 flex items-center justify-center bg-black'>
64+
<div className='fixed inset-0 z-50 flex items-center justify-center bg-black/50'>
6565
<div className='bg-primary border-primary w-full max-w-md rounded-lg border p-6 shadow-lg'>
6666
<h2 className='text-primary mb-4 text-xl font-bold'>Create New Group</h2>
6767

moon/apps/web/components/AdminGroups/DeleteGroupDialog.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ export const DeleteGroupDialog = ({ groupId, onClose }: DeleteGroupDialogProps)
2626
if (groupId === null) return null
2727

2828
return (
29-
<div className='bg-opacity-50 fixed inset-0 z-50 flex items-center justify-center bg-black'>
29+
<div className='fixed inset-0 z-50 flex items-center justify-center bg-black/50'>
3030
<div className='bg-primary border-primary w-full max-w-md rounded-lg border p-6 shadow-lg'>
3131
<h2 className='text-primary mb-4 text-xl font-bold'>Delete Group</h2>
3232

moon/apps/web/components/AdminGroups/GroupMembersDialog.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ export const GroupMembersDialog = ({ groupId, groupName, onClose }: GroupMembers
4646
if (groupId === null) return null
4747

4848
return (
49-
<div className='bg-opacity-50 fixed inset-0 z-50 flex items-center justify-center bg-black p-4'>
49+
<div className='fixed inset-0 z-50 flex items-center justify-center bg-black/50 p-4'>
5050
<div className='bg-primary border-primary flex max-h-[90vh] w-full max-w-2xl flex-col rounded-lg border shadow-xl'>
5151
{/* Fixed header */}
5252
<div className='shrink-0 border-b border-gray-200 px-6 py-4 dark:border-gray-700'>

0 commit comments

Comments
 (0)