Skip to content

Commit aa35c2e

Browse files
ilblackdragonclaude
andcommitted
refactor: remove GATEWAY_USER_TOKENS, fix review feedback
GATEWAY_USER_TOKENS never went to production — replaced entirely by DB-backed user management via /api/admin/users and /api/tokens. Removed: - UserTokenConfig struct and GATEWAY_USER_TOKENS env var parsing - user_tokens field from GatewayConfig - GatewayChannel::new_multi_auth() constructor - Env-var user migration block in main.rs (~90 lines) - multi_tenant auto-detection from GATEWAY_USER_TOKENS (now runtime via db.has_any_users() in app.rs) Review fixes (zmanian): - User ID generation: UUID instead of display-name derivation (#1) - Invitation accept moved to public router (no auth needed) (#3) - libSQL get_invitation_by_hash aligned with postgres: filters status='pending' AND expires_at > now (#4) - UUID parse: returns DatabaseError::Serialization instead of unwrap_or_default (#7) - PostgreSQL SELECT * replaced with explicit column lists (#8) - Sort order aligned (both backends use DESC) (#6) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 240cee8 commit aa35c2e

File tree

13 files changed

+74
-255
lines changed

13 files changed

+74
-255
lines changed

src/app.rs

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -342,17 +342,12 @@ impl AppBuilder {
342342
ws = ws.with_memory_layers(self.config.workspace.memory_layers.clone());
343343
let ws = Arc::new(ws);
344344

345-
// Detect multi-tenant mode: when GATEWAY_USER_TOKENS is configured,
345+
// Detect multi-tenant mode: when the database has registered users,
346346
// each authenticated user needs their own workspace scope. Use
347347
// WorkspacePool (which implements WorkspaceResolver) to create
348348
// per-user workspaces on demand instead of sharing the startup
349349
// workspace across all users.
350-
let is_multi_tenant = self
351-
.config
352-
.channels
353-
.gateway
354-
.as_ref()
355-
.is_some_and(|gw| gw.user_tokens.is_some());
350+
let is_multi_tenant = db.has_any_users().await.unwrap_or(false);
356351

357352
if is_multi_tenant {
358353
let pool = Arc::new(crate::channels::web::server::WorkspacePool::new(

src/channels/web/auth.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,11 @@ impl MultiAuthState {
6969
}
7070

7171
/// Create a multi-user auth state from a map of tokens to identities.
72+
///
73+
/// **Test-only** — production multi-user auth is DB-backed via
74+
/// `DbAuthenticator`. This constructor is kept public (not `#[cfg(test)]`)
75+
/// because integration tests in `tests/` compile the crate as a library
76+
/// where `cfg(test)` is not set.
7277
pub fn multi(tokens: HashMap<String, UserIdentity>) -> Self {
7378
let hashed_tokens: Vec<([u8; 32], UserIdentity)> = tokens
7479
.into_iter()
@@ -188,7 +193,7 @@ impl DbAuthenticator {
188193
/// Combined auth state: tries env-var tokens first, then DB-backed tokens.
189194
#[derive(Clone)]
190195
pub struct CombinedAuthState {
191-
/// In-memory tokens from GATEWAY_USER_TOKENS or GATEWAY_AUTH_TOKEN.
196+
/// In-memory tokens from GATEWAY_AUTH_TOKEN.
192197
pub env_auth: MultiAuthState,
193198
/// DB-backed token authenticator (optional — only when a database is available).
194199
pub db_auth: Option<DbAuthenticator>,

src/channels/web/handlers/invitations.rs

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -162,12 +162,7 @@ pub async fn invitations_accept_handler(
162162
return Err((StatusCode::GONE, "Invitation has expired".to_string()));
163163
}
164164

165-
// Generate a user id from the display name.
166-
let new_user_id = display_name
167-
.to_ascii_lowercase()
168-
.split_whitespace()
169-
.collect::<Vec<_>>()
170-
.join("-");
165+
let new_user_id = Uuid::new_v4().to_string();
171166

172167
let now = chrono::Utc::now();
173168
let user_record = UserRecord {

src/channels/web/handlers/users.rs

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use axum::{
77
extract::{Path, State},
88
http::StatusCode,
99
};
10+
use uuid::Uuid;
1011

1112
use crate::channels::web::auth::AuthenticatedUser;
1213
use crate::channels::web::server::GatewayState;
@@ -34,16 +35,7 @@ pub async fn users_create_handler(
3435

3536
let email = body.get("email").and_then(|v| v.as_str()).map(String::from);
3637

37-
// Generate user id: prefer email if provided, otherwise derive from display_name.
38-
let user_id = if let Some(ref e) = email {
39-
e.clone()
40-
} else {
41-
display_name
42-
.to_ascii_lowercase()
43-
.split_whitespace()
44-
.collect::<Vec<_>>()
45-
.join("-")
46-
};
38+
let user_id = Uuid::new_v4().to_string();
4739

4840
let now = chrono::Utc::now();
4941
let user_record = UserRecord {

src/channels/web/mod.rs

Lines changed: 0 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -124,49 +124,6 @@ impl GatewayChannel {
124124
}
125125
}
126126

127-
/// Create a gateway channel with a pre-built multi-user auth state.
128-
pub fn new_multi_auth(config: GatewayConfig, auth: MultiAuthState) -> Self {
129-
let auth = CombinedAuthState {
130-
env_auth: auth,
131-
db_auth: None,
132-
};
133-
let state = Arc::new(GatewayState {
134-
msg_tx: tokio::sync::RwLock::new(None),
135-
sse: Arc::new(SseManager::new()),
136-
workspace: None,
137-
workspace_pool: None,
138-
session_manager: None,
139-
log_broadcaster: None,
140-
log_level_handle: None,
141-
extension_manager: None,
142-
tool_registry: None,
143-
store: None,
144-
job_manager: None,
145-
prompt_queue: None,
146-
scheduler: None,
147-
default_user_id: config.user_id.clone(),
148-
shutdown_tx: tokio::sync::RwLock::new(None),
149-
ws_tracker: Some(Arc::new(ws::WsConnectionTracker::new())),
150-
llm_provider: None,
151-
skill_registry: None,
152-
skill_catalog: None,
153-
chat_rate_limiter: server::PerUserRateLimiter::new(30, 60),
154-
oauth_rate_limiter: server::RateLimiter::new(10, 60),
155-
registry_entries: Vec::new(),
156-
cost_guard: None,
157-
routine_engine: Arc::new(tokio::sync::RwLock::new(None)),
158-
startup_time: std::time::Instant::now(),
159-
webhook_rate_limiter: server::RateLimiter::new(10, 60),
160-
active_config: server::ActiveConfigSnapshot::default(),
161-
});
162-
163-
Self {
164-
config,
165-
state,
166-
auth,
167-
}
168-
}
169-
170127
/// Helper to rebuild state, copying existing fields and applying a mutation.
171128
fn rebuild_state(&mut self, mutate: impl FnOnce(&mut GatewayState)) {
172129
let mut new_state = GatewayState {

src/channels/web/server.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,11 @@ pub async fn start_server(
417417
.route(
418418
"/api/webhooks/u/{user_id}/{path}",
419419
post(crate::channels::web::handlers::webhooks::webhook_trigger_user_scoped_handler),
420+
)
421+
// Invitation accept (public — validated by invite token, not user auth)
422+
.route(
423+
"/api/invitations/accept",
424+
post(super::handlers::invitations::invitations_accept_handler),
420425
);
421426

422427
// Protected routes (require auth)
@@ -545,10 +550,6 @@ pub async fn start_server(
545550
get(super::handlers::invitations::invitations_list_handler)
546551
.post(super::handlers::invitations::invitations_create_handler),
547552
)
548-
.route(
549-
"/api/invitations/accept",
550-
post(super::handlers::invitations::invitations_accept_handler),
551-
)
552553
// Gateway control plane
553554
.route("/api/gateway/status", get(gateway_status_handler))
554555
// OpenAI-compatible API

src/config/agent.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use std::time::Duration;
22

3-
use crate::config::helpers::{optional_env, parse_bool_env, parse_option_env, parse_optional_env};
3+
use crate::config::helpers::{parse_bool_env, parse_option_env, parse_optional_env};
44
use crate::error::ConfigError;
55
use crate::settings::Settings;
66

@@ -34,7 +34,8 @@ pub struct AgentConfig {
3434
/// Maximum tokens per job (0 = unlimited).
3535
pub max_tokens_per_job: u64,
3636
/// Whether the deployment is multi-tenant (multiple users sharing one
37-
/// instance). Auto-detected from GATEWAY_USER_TOKENS presence.
37+
/// instance). Detected at runtime after DB initialization, not from config.
38+
/// See app.rs startup logic.
3839
pub multi_tenant: bool,
3940
}
4041

@@ -120,9 +121,9 @@ impl AgentConfig {
120121
"AGENT_MAX_TOKENS_PER_JOB",
121122
settings.agent.max_tokens_per_job,
122123
)?,
123-
// Auto-detected from GATEWAY_USER_TOKENS presence. Not a separate
124-
// knob — multi-tenant mode is always implied by configuring user tokens.
125-
multi_tenant: optional_env("GATEWAY_USER_TOKENS")?.is_some(),
124+
// Multi-tenant mode is detected at runtime after DB initialization,
125+
// not from config. See app.rs startup logic.
126+
multi_tenant: false,
126127
})
127128
}
128129
}

src/config/channels.rs

Lines changed: 1 addition & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,11 @@
11
use std::collections::HashMap;
22
use std::path::PathBuf;
33

4-
use secrecy::SecretString;
5-
use serde::Deserialize;
6-
74
use crate::bootstrap::ironclaw_base_dir;
85
use crate::config::helpers::{optional_env, parse_bool_env, parse_optional_env};
96
use crate::error::ConfigError;
107
use crate::settings::Settings;
8+
use secrecy::SecretString;
119

1210
/// Channel configurations.
1311
#[derive(Debug, Clone)]
@@ -54,18 +52,6 @@ pub struct GatewayConfig {
5452
pub workspace_read_scopes: Vec<String>,
5553
/// Memory layer definitions (JSON in env var, or from external config).
5654
pub memory_layers: Vec<crate::workspace::layer::MemoryLayer>,
57-
/// Multi-user token map. When set, each token maps to a user identity.
58-
/// Parsed from `GATEWAY_USER_TOKENS` (JSON string). When absent, falls back
59-
/// to single-user mode via `auth_token` + `user_id`.
60-
pub user_tokens: Option<HashMap<String, UserTokenConfig>>,
61-
}
62-
63-
/// Per-user token configuration for multi-user mode.
64-
#[derive(Debug, Clone, Deserialize)]
65-
pub struct UserTokenConfig {
66-
pub user_id: String,
67-
#[serde(default)]
68-
pub workspace_read_scopes: Vec<String>,
6955
}
7056

7157
/// Signal channel configuration (signal-cli daemon HTTP/JSON-RPC).
@@ -196,41 +182,6 @@ impl ChannelsConfig {
196182
}
197183
}
198184

199-
let user_tokens: Option<HashMap<String, UserTokenConfig>> =
200-
match optional_env("GATEWAY_USER_TOKENS")? {
201-
Some(json_str) => {
202-
let tokens: HashMap<String, UserTokenConfig> = serde_json::from_str(
203-
&json_str,
204-
)
205-
.map_err(|e| ConfigError::InvalidValue {
206-
key: "GATEWAY_USER_TOKENS".to_string(),
207-
message: format!(
208-
"must be valid JSON object mapping tokens to user configs: {e}"
209-
),
210-
})?;
211-
if tokens.is_empty() {
212-
return Err(ConfigError::InvalidValue {
213-
key: "GATEWAY_USER_TOKENS".to_string(),
214-
message:
215-
"token map is empty — remove the variable to use single-user mode"
216-
.to_string(),
217-
});
218-
}
219-
for (tok, cfg) in &tokens {
220-
if cfg.user_id.trim().is_empty() {
221-
return Err(ConfigError::InvalidValue {
222-
key: "GATEWAY_USER_TOKENS".to_string(),
223-
message: format!(
224-
"token '{}...' has an empty user_id",
225-
&tok[..tok.len().min(8)]
226-
),
227-
});
228-
}
229-
}
230-
Some(tokens)
231-
}
232-
None => None,
233-
};
234185
let workspace_read_scopes: Vec<String> = optional_env("WORKSPACE_READ_SCOPES")?
235186
.map(|s| {
236187
s.split(',')
@@ -261,7 +212,6 @@ impl ChannelsConfig {
261212
user_id,
262213
workspace_read_scopes,
263214
memory_layers,
264-
user_tokens,
265215
})
266216
} else {
267217
None
@@ -419,7 +369,6 @@ mod tests {
419369
user_id: "default".to_string(),
420370
workspace_read_scopes: vec![],
421371
memory_layers: vec![],
422-
user_tokens: None,
423372
};
424373
assert_eq!(cfg.host, "127.0.0.1");
425374
assert_eq!(cfg.port, 3000);
@@ -436,7 +385,6 @@ mod tests {
436385
user_id: "anon".to_string(),
437386
workspace_read_scopes: vec![],
438387
memory_layers: vec![],
439-
user_tokens: None,
440388
};
441389
assert!(cfg.auth_token.is_none());
442390
}

src/config/heartbeat.rs

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@ pub struct HeartbeatConfig {
2121
pub quiet_hours_end: Option<u32>,
2222
/// Timezone for fire_at and quiet hours evaluation (IANA name).
2323
pub timezone: Option<String>,
24-
/// When true, cycle through all users with routines. Auto-detected from
25-
/// GATEWAY_USER_TOKENS or set explicitly via HEARTBEAT_MULTI_TENANT.
24+
/// When true, cycle through all users with routines. Set explicitly via
25+
/// HEARTBEAT_MULTI_TENANT or detected at runtime after DB initialization.
2626
pub multi_tenant: bool,
2727
}
2828

@@ -105,12 +105,7 @@ impl HeartbeatConfig {
105105
}
106106
tz
107107
},
108-
// Auto-detect multi-tenant mode from GATEWAY_USER_TOKENS presence,
109-
// or allow explicit override via HEARTBEAT_MULTI_TENANT.
110-
multi_tenant: parse_bool_env(
111-
"HEARTBEAT_MULTI_TENANT",
112-
optional_env("GATEWAY_USER_TOKENS")?.is_some(),
113-
)?,
108+
multi_tenant: parse_bool_env("HEARTBEAT_MULTI_TENANT", false)?,
114109
})
115110
}
116111
}

0 commit comments

Comments
 (0)