mirror of
https://github.com/nearai/ironclaw.git
synced 2026-09-03 08:06:01 +08:00
fix: address remaining review comments (round 2)
- Secrets handlers: normalize name to lowercase before store operations, validate target user_id exists (returns 404 if not found) - libSQL: propagate cost parsing errors instead of unwrap_or_default() in both user_usage_stats and user_summary_stats - users_list_handler: propagate user_summary_stats DB errors (was silently swallowed with unwrap_or_default) - loadUsers: distinguish 401/403 (admin required) from other errors - Docs: fix users.id type (TEXT not UUID), remove "invitation flow" from V14 migration comment Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -529,7 +529,7 @@ All error responses return a plain text body with the error message and the corr
|
||||
|
||||
| Column | Type (PG / libSQL) | Notes |
|
||||
|--------|--------------------|-------|
|
||||
| `id` | `UUID` / `TEXT` | Primary key, UUID v4 |
|
||||
| `id` | `TEXT` / `TEXT` | Primary key; values are UUID v4 strings |
|
||||
| `email` | `TEXT UNIQUE` | Nullable |
|
||||
| `display_name` | `TEXT NOT NULL` | |
|
||||
| `status` | `TEXT NOT NULL` | `"active"` or `"suspended"` |
|
||||
|
||||
@@ -1,10 +1,10 @@
|
||||
-- User management tables for multi-tenant deployments.
|
||||
--
|
||||
-- Replaces the static GATEWAY_USER_TOKENS env var with DB-backed
|
||||
-- user registration, API token management, and invitation flow.
|
||||
-- user registration and API token management.
|
||||
|
||||
CREATE TABLE users (
|
||||
id TEXT PRIMARY KEY, -- matches existing user_id pattern (string, not UUID)
|
||||
id TEXT PRIMARY KEY, -- stored as TEXT; values are UUIDv4 strings
|
||||
email TEXT UNIQUE, -- nullable for token-only users
|
||||
display_name TEXT NOT NULL,
|
||||
status TEXT NOT NULL DEFAULT 'active', -- active | suspended | deactivated
|
||||
|
||||
@@ -27,6 +27,18 @@ pub async fn secrets_put_handler(
|
||||
Path((user_id, name)): Path<(String, String)>,
|
||||
Json(body): Json<serde_json::Value>,
|
||||
) -> Result<Json<serde_json::Value>, (StatusCode, String)> {
|
||||
let name = name.to_lowercase();
|
||||
|
||||
let store = state.store.as_ref().ok_or((
|
||||
StatusCode::SERVICE_UNAVAILABLE,
|
||||
"Database not available".to_string(),
|
||||
))?;
|
||||
store
|
||||
.get_user(&user_id)
|
||||
.await
|
||||
.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?
|
||||
.ok_or((StatusCode::NOT_FOUND, "User not found".to_string()))?;
|
||||
|
||||
let secrets = state.secrets_store.as_ref().ok_or((
|
||||
StatusCode::SERVICE_UNAVAILABLE,
|
||||
"Secrets store not available".to_string(),
|
||||
@@ -67,7 +79,7 @@ pub async fn secrets_put_handler(
|
||||
|
||||
Ok(Json(serde_json::json!({
|
||||
"user_id": user_id,
|
||||
"name": name.to_lowercase(),
|
||||
"name": name,
|
||||
"status": "created",
|
||||
})))
|
||||
}
|
||||
@@ -112,6 +124,18 @@ pub async fn secrets_delete_handler(
|
||||
AdminUser(_admin): AdminUser,
|
||||
Path((user_id, name)): Path<(String, String)>,
|
||||
) -> Result<Json<serde_json::Value>, (StatusCode, String)> {
|
||||
let name = name.to_lowercase();
|
||||
|
||||
let store = state.store.as_ref().ok_or((
|
||||
StatusCode::SERVICE_UNAVAILABLE,
|
||||
"Database not available".to_string(),
|
||||
))?;
|
||||
store
|
||||
.get_user(&user_id)
|
||||
.await
|
||||
.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?
|
||||
.ok_or((StatusCode::NOT_FOUND, "User not found".to_string()))?;
|
||||
|
||||
let secrets = state.secrets_store.as_ref().ok_or((
|
||||
StatusCode::SERVICE_UNAVAILABLE,
|
||||
"Secrets store not available".to_string(),
|
||||
|
||||
@@ -119,7 +119,10 @@ pub async fn users_list_handler(
|
||||
.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?;
|
||||
|
||||
// Fetch per-user summary stats in a single batch query.
|
||||
let summary_stats = store.user_summary_stats(None).await.unwrap_or_default();
|
||||
let summary_stats = store
|
||||
.user_summary_stats(None)
|
||||
.await
|
||||
.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?;
|
||||
|
||||
let stats_map: std::collections::HashMap<String, _> = summary_stats
|
||||
.into_iter()
|
||||
|
||||
@@ -4352,13 +4352,16 @@ function loadUsers() {
|
||||
apiFetch('/api/admin/users').then(function(data) {
|
||||
renderUsersList(data.users || []);
|
||||
}).catch(function(err) {
|
||||
// Non-admin users get 403 — show a message instead of an error
|
||||
var tbody = document.getElementById('users-tbody');
|
||||
var empty = document.getElementById('users-empty');
|
||||
if (tbody) tbody.innerHTML = '';
|
||||
if (empty) {
|
||||
empty.style.display = 'block';
|
||||
empty.textContent = 'Admin access required to manage users.';
|
||||
if (err.status === 403 || err.status === 401) {
|
||||
empty.textContent = 'Admin access required to manage users.';
|
||||
} else {
|
||||
empty.textContent = 'Failed to load users: ' + err.message;
|
||||
}
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
@@ -487,7 +487,9 @@ impl UserStore for LibSqlBackend {
|
||||
.map_err(|e| DatabaseError::Query(e.to_string()))?
|
||||
{
|
||||
let cost_str = get_text(&row, 5);
|
||||
let total_cost = rust_decimal::Decimal::from_str_exact(&cost_str).unwrap_or_default();
|
||||
let total_cost = rust_decimal::Decimal::from_str_exact(&cost_str).map_err(|e| {
|
||||
DatabaseError::Query(format!("invalid cost value '{}': {}", cost_str, e))
|
||||
})?;
|
||||
stats.push(crate::db::UserUsageStats {
|
||||
user_id: get_text(&row, 0),
|
||||
model: get_text(&row, 1),
|
||||
@@ -555,7 +557,9 @@ impl UserStore for LibSqlBackend {
|
||||
.map_err(|e| DatabaseError::Query(e.to_string()))?
|
||||
{
|
||||
let cost_str = get_text(&row, 2);
|
||||
let total_cost = rust_decimal::Decimal::from_str_exact(&cost_str).unwrap_or_default();
|
||||
let total_cost = rust_decimal::Decimal::from_str_exact(&cost_str).map_err(|e| {
|
||||
DatabaseError::Query(format!("invalid cost value '{}': {}", cost_str, e))
|
||||
})?;
|
||||
stats.push(crate::db::UserSummaryStats {
|
||||
user_id: get_text(&row, 0),
|
||||
job_count: row
|
||||
|
||||
Reference in New Issue
Block a user