Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions src/adapter/auth/keystone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ struct KeystoneTokenBody {
project: Option<KeystoneProject>,
roles: Vec<KeystoneRole>,
catalog: Option<Vec<KeystoneCatalogEntry>>,
/// Keystone v3 includes a `user` block with the authenticated user's UUID.
/// Optional so legacy fixtures without it deserialize cleanly.
user: Option<KeystoneUser>,
}

#[derive(Debug, Deserialize)]
Expand All @@ -39,6 +42,11 @@ struct KeystoneProject {
domain: KeystoneDomain,
}

#[derive(Debug, Deserialize)]
struct KeystoneUser {
id: String,
}

#[derive(Debug, Deserialize)]
struct KeystoneDomain {
id: String,
Expand Down Expand Up @@ -111,12 +119,15 @@ pub(super) fn parse_token(token_id: String, resp: KeystoneTokenResponse) -> Toke
})
.collect();

let user_id = body.user.map(|u| u.id).unwrap_or_default();

Token {
id: token_id,
expires_at: body.expires_at,
project,
roles,
catalog,
user_id,
}
}

Expand Down Expand Up @@ -608,6 +619,11 @@ mod tests {
r#"{
"token": {
"expires_at": "2099-12-31T23:59:59.000000Z",
"user": {
"id": "user-uuid-abc",
"name": "admin",
"domain": { "id": "default", "name": "Default" }
},
"project": {
"id": "proj-123",
"name": "admin-project",
Expand Down Expand Up @@ -680,6 +696,10 @@ mod tests {
assert_eq!(token.id, "tok-abc-123");
assert_eq!(token.project.name, "admin-project");
assert_eq!(token.project.domain_name, "Default");
assert_eq!(
token.user_id, "user-uuid-abc",
"user.id must be parsed from Keystone response (BL-P2-093)"
);
assert_eq!(token.roles.len(), 2);
assert_eq!(token.roles[0].name, "admin");
assert_eq!(token.catalog.len(), 2);
Expand All @@ -705,6 +725,9 @@ mod tests {
assert!(token.catalog.is_empty());
assert_eq!(token.roles.len(), 1);
assert!(token.project.id.is_empty());
// BL-P2-093: missing `user` block must deserialize cleanly and leave
// user_id empty (caller falls back to wire-startup username).
assert!(token.user_id.is_empty());
}

#[test]
Expand Down Expand Up @@ -843,6 +866,7 @@ mod tests {
},
roles: Vec::new(),
catalog: Vec::new(),
user_id: String::new(),
}
}

Expand Down
1 change: 1 addition & 0 deletions src/adapter/auth/keystone_project_directory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,7 @@ mod tests {
},
roles: Vec::<TokenRole>::new(),
catalog: Vec::<CatalogEntry>::new(),
user_id: String::new(),
}
}

Expand Down
1 change: 1 addition & 0 deletions src/adapter/auth/rescope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@ mod tests {
},
roles: Vec::new(),
catalog: Vec::new(),
user_id: String::new(),
}
}

Expand Down
1 change: 1 addition & 0 deletions src/adapter/auth/scoped_session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ mod tests {
},
roles: Vec::new(),
catalog: Vec::<CatalogEntry>::new(),
user_id: String::new(),
}
}

Expand Down
55 changes: 50 additions & 5 deletions src/adapter/auth/token_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,19 +121,36 @@ pub fn save_token(
}

/// Load a single token from a cache file.
/// Returns None if the file doesn't exist, is unreadable, or the token is expired.
/// Returns None if the file doesn't exist, is unreadable, the token is
/// expired, or the token predates BL-P2-093 (missing `user_id`).
/// Automatically deletes expired token files.
fn load_token_file(path: &Path) -> Option<Token> {
let data = std::fs::read(path).ok()?;
let token: Token = serde_json::from_slice(&data).ok()?;

if token.expires_at > chrono::Utc::now() + chrono::Duration::minutes(1) {
Some(token)
} else {
if token.expires_at <= chrono::Utc::now() + chrono::Duration::minutes(1) {
tracing::info!(path = %path.display(), "cached token expired, removing");
let _ = std::fs::remove_file(path);
None
return None;
}

// BL-P2-093 upgrade safety: legacy disk-cached tokens (pre-BL-P2-093)
// deserialize with `user_id = ""` via `#[serde(default)]`. Accepting
// them would let `actor_ctx.user_id` stay anchored to the wire username
// when the user switches into that cached scope post-upgrade (the
// ContextChanged emit forwards the empty `user_id` and `handle_event`
// preserves the prior value). Treat empty as a cache miss so the
// next `get_token` reauths against Keystone and persists the UUID;
// the file is left in place to be overwritten by the fresh token.
if token.user_id.is_empty() {
tracing::info!(
path = %path.display(),
"cached token lacks user_id (pre-BL-P2-093), forcing reauth"
);
return None;
}

Some(token)
}

/// Load all valid cached tokens from the cache directory.
Expand Down Expand Up @@ -223,6 +240,7 @@ mod tests {
url: "https://nova:8774/v2.1".to_string(),
}],
}],
user_id: "user-uuid-cached".to_string(),
}
}

Expand Down Expand Up @@ -343,6 +361,33 @@ mod tests {
assert!(loaded.is_empty());
}

#[test]
fn test_load_legacy_token_without_user_id_treated_as_cache_miss() {
// BL-P2-093 upgrade safety: pre-upgrade cached tokens deserialize
// with `user_id = ""`. `load_token_file` must reject them so the
// next get_token reauths and persists the Keystone UUID instead of
// leaving actor_ctx.user_id anchored to the wire username.
let dir = TempDir::new().unwrap();
let cache_dir = dir.path().join("cloud-legacy");
let scope = sample_scope();

let mut token = sample_token(60);
token.user_id = String::new();
save_token(&token, &cache_dir, &scope).unwrap();

let loaded = load_all_tokens(&cache_dir);
assert!(
loaded.is_empty(),
"legacy empty user_id must be treated as a cache miss"
);
// File is left in place; next save_token will overwrite with a
// fresh token carrying the Keystone UUID.
assert!(
cache_dir.join(scope.cache_key()).exists(),
"legacy file must NOT be auto-deleted (only expired tokens are)"
);
}

#[test]
fn test_load_corrupt_file_skipped() {
let dir = TempDir::new().unwrap();
Expand Down
1 change: 1 addition & 0 deletions src/adapter/auth/token_scope_fingerprint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ mod tests {
},
roles: Vec::<TokenRole>::new(),
catalog: Vec::<CatalogEntry>::new(),
user_id: String::new(),
}
}

Expand Down
85 changes: 77 additions & 8 deletions src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,11 @@ impl App {
// data starts loading immediately.
// The remaining UX bits (router/selection reset + toast) are tracked
// by BL-P2-052 Part B leftovers.
if let AppEvent::ContextChanged { ref target } = event {
if let AppEvent::ContextChanged {
ref target,
ref user_id,
} = event
{
// Codex review 3차 P1: async `ContextChanged` can arrive while
// the user is in Form/Command mode. The module reset below
// leaves the form behind, but without normalizing `input_mode`
Expand All @@ -645,14 +649,19 @@ impl App {
if let Some(cache) = &self.directory_cache {
cache.invalidate_cloud(&target.cloud);
}
// BL-P2-085 Phase 7 폴리싱: the worker reads `actor_ctx` live at
// each block emit. Without this update, audit entries from the
// worker stay anchored to the spawn-time cloud after the user
// switches.
// BL-P2-085 Phase 7 폴리싱 + BL-P2-093: the worker reads
// `actor_ctx` live at each block emit. Without this update, audit
// entries from the worker stay anchored to the spawn-time cloud
// / user after the user switches. Empty `user_id` is treated as
// "no change" so test fixtures and rescope paths that lack a
// Keystone user UUID don't overwrite a valid prior value.
if let Some(ref ctx) = self.actor_ctx
&& let Ok(mut guard) = ctx.write()
{
guard.cloud = target.cloud.clone();
if !user_id.is_empty() {
guard.user_id = user_id.clone();
}
}
self.context_indicator.set_target(target, true);
for component in self.components.values_mut() {
Expand Down Expand Up @@ -1698,9 +1707,14 @@ impl App {
tokio::spawn(async move {
match switcher.switch(request).await {
Ok((epoch, snapshot)) => {
// BL-P2-093: forward the rescoped token's Keystone user
// UUID so `handle_event(ContextChanged)` can refresh
// `actor_ctx.user_id` for subsequent audit entries.
let user_id = snapshot.token.user_id.clone();
let _ = event_tx.send(crate::context::VersionedEvent::new(
AppEvent::ContextChanged {
target: snapshot.target,
user_id,
},
epoch,
));
Expand Down Expand Up @@ -1737,9 +1751,11 @@ impl App {
tokio::spawn(async move {
match switcher.switch_back().await {
Ok((epoch, snapshot)) => {
let user_id = snapshot.token.user_id.clone();
let _ = event_tx.send(crate::context::VersionedEvent::new(
AppEvent::ContextChanged {
target: snapshot.target,
user_id,
},
epoch,
));
Expand Down Expand Up @@ -2116,6 +2132,7 @@ mod tests {
project_name: "admin".into(),
domain: "default".into(),
},
user_id: String::new(),
});
let t = app
.context_indicator
Expand All @@ -2127,6 +2144,41 @@ mod tests {
assert!(app.context_indicator.is_highlighting());
}

#[test]
fn test_context_changed_updates_actor_user_id() {
// BL-P2-093: Phase 7 폴리싱이 actor_ctx.cloud는 live 갱신하지만 user_id는
// wire-startup 시점 값에 고정. 다른 자격증명으로 재인증한 후 발생한 모든
// cross-project block audit이 이전 user_id로 기록되어 attribution을 망가뜨림.
// ContextChanged payload의 user_id는 handle_event 안에서 actor_ctx에 반영
// 되어야 한다.
use crate::context::types::ContextTarget;
use crate::worker::ActorContext;

let actor_ctx = Arc::new(std::sync::RwLock::new(ActorContext {
cloud: "cloud-A".into(),
user_id: "user-A".into(),
}));
let mut app = make_app();
app.set_actor_ctx(actor_ctx.clone());

app.handle_event(AppEvent::ContextChanged {
target: ContextTarget {
cloud: "cloud-B".into(),
project_id: "p1".into(),
project_name: "admin".into(),
domain: "default".into(),
},
user_id: "user-B".into(),
});

let guard = actor_ctx.read().expect("actor_ctx read");
assert_eq!(guard.cloud, "cloud-B", "cloud must follow ContextChanged");
assert_eq!(
guard.user_id, "user-B",
"user_id must follow ContextChanged (BL-P2-093)"
);
}

#[test]
fn test_context_changed_resets_input_mode_to_normal() {
// Codex review 3차 P1: async `ContextChanged` can arrive while the
Expand All @@ -2144,6 +2196,7 @@ mod tests {
project_name: "admin".into(),
domain: "default".into(),
},
user_id: String::new(),
});
assert_eq!(
app.input_mode,
Expand Down Expand Up @@ -2216,6 +2269,7 @@ mod tests {
project_name: "admin".into(),
domain: "default".into(),
},
user_id: String::new(),
});
assert!(
state.borrow().last_recently,
Expand Down Expand Up @@ -2274,6 +2328,7 @@ mod tests {
project_name: "admin".into(),
domain: "default".into(),
},
user_id: String::new(),
});

let mut received: Vec<Action> = Vec::new();
Expand Down Expand Up @@ -3586,6 +3641,7 @@ mod tests {
},
roles: Vec::new(),
catalog: Vec::<CatalogEntry>::new(),
user_id: String::new(),
};
let mut new_token = old_token.clone();
new_token.id = "new".into();
Expand Down Expand Up @@ -3634,9 +3690,16 @@ mod tests {
.expect("channel closed");
assert_eq!(envelope.epoch(), 1);
match envelope.into_inner() {
AppEvent::ContextChanged { target } => {
AppEvent::ContextChanged { target, user_id } => {
assert_eq!(target.project_name, "demo");
assert_eq!(target.cloud, "devstack");
// BL-P2-093: MockContextSession's rescoped token leaves
// user_id blank — `handle_event` treats empty as "no change",
// preserving the wire-startup actor_ctx.user_id.
assert!(
user_id.is_empty(),
"mock rescoped token carries empty user_id by design"
);
}
other => panic!("expected ContextChanged, got {other:?}"),
}
Expand Down Expand Up @@ -3761,7 +3824,10 @@ mod tests {
project_name: "admin".into(),
domain: "default".into(),
};
app.handle_event(AppEvent::ContextChanged { target });
app.handle_event(AppEvent::ContextChanged {
target,
user_id: String::new(),
});

// "devstack" entries should be gone; "prod" should remain.
assert!(
Expand All @@ -3786,7 +3852,10 @@ mod tests {
project_name: "admin".into(),
domain: "default".into(),
};
app.handle_event(AppEvent::ContextChanged { target });
app.handle_event(AppEvent::ContextChanged {
target,
user_id: String::new(),
});
// If we reach here without panicking, the test passes.
}
}
1 change: 1 addition & 0 deletions src/context/history.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ mod tests {
},
roles: Vec::new(),
catalog: Vec::<CatalogEntry>::new(),
user_id: String::new(),
};
ContextSnapshot {
target: target.clone(),
Expand Down
1 change: 1 addition & 0 deletions src/context/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -822,6 +822,7 @@ mod tests {
},
roles: Vec::new(),
catalog: Vec::<CatalogEntry>::new(),
user_id: String::new(),
})
}
async fn set_active(&self, _scope: TokenScope, _token: Token) -> Result<(), SwitchError> {
Expand Down
Loading
Loading