From 9bcf0054af48cb8c9d2d396526e7050d2c2525af Mon Sep 17 00:00:00 2001 From: bluejayA Date: Wed, 13 May 2026 09:59:54 +0900 Subject: [PATCH 1/3] feat(BL-P2-083): interim session-expired guard for rescoped tokens MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BL-P2-052 Part A (full background auto-refresh) is scoped beyond P0/P1, but the 55-minute deterministic auth failure that follows every rescoped session ships with P0/P1 today. Without this guard, the failure surfaces as `ApiError::AuthFailed("refresh_token refused: active scope ... differs from initial scope ...")` — indistinguishable from real credential rejection. Operators have prompted users to "check credentials" when the actual fix is `:switch-context` to reauth, and we have no telemetry on how often this happens. This change splits the path off cleanly: - `ApiError::SessionExpired { project }` — distinct from AuthFailed so callers (worker funnel + future BL-P2-052 Part A) can branch. - `KeystoneAuthAdapter::get_token` — pre-empts the existing 1-minute fast path for rescoped sessions only (active scope != initial scope): if the cached token is within 5 minutes of expiry (or missing for an active rescoped key), short-circuit with `SessionExpired` and `tracing::warn!(project, expires_at, "session_expired")`. Initial-scope tokens fall through to refresh_token unchanged — those CAN be self-refreshed via do_authenticate. - `worker::api_error` — routes `ApiError::SessionExpired` to a dedicated `AppEvent::SessionExpired { project }` variant instead of the generic `AppEvent::ApiError` funnel. - `App::generate_toast` — Korean reauth message: " 세션이 만료 되었습니다. :switch-context로 재전환하거나 앱을 다시 시작하세요." Telemetry: tracing::warn at the adapter (structured fields for grep) + tracing::warn at the worker (operation tag for find-by-operation workflows). Both fire on the same event so analysts can correlate. TDD: 5 new tests covering the full path: - test_get_token_returns_session_expired_for_expired_rescoped_token - test_get_token_returns_session_expired_within_5min_margin_for_rescoped - test_get_token_initial_scope_near_expiry_does_not_return_session_expired - test_api_error_routes_session_expired_to_dedicated_event - test_generate_toast_for_session_expired_uses_reauth_message cargo test = 1509 passed (1504 → +5). clippy clean. fmt clean. Out of scope (BL-P2-083 spec): background auto-refresh for rescoped sessions (BL-P2-052 Part A). The interim guard ships in front so BL-P2-052 arrives with telemetry-justified rollout instead of guessing how often 55-minute failures occur. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/adapter/auth/keystone.rs | 138 +++++++++++++++++++++++++++++++++++ src/app.rs | 34 +++++++++ src/event.rs | 11 +++ src/port/error.rs | 11 +++ src/worker.rs | 27 +++++++ 5 files changed, 221 insertions(+) diff --git a/src/adapter/auth/keystone.rs b/src/adapter/auth/keystone.rs index 64f96b5..4fe1a66 100644 --- a/src/adapter/auth/keystone.rs +++ b/src/adapter/auth/keystone.rs @@ -433,11 +433,53 @@ impl AuthProvider for KeystoneAuthAdapter { /// Get a valid token string. If near-expiry (<1min), refresh first. /// Uses a Mutex to prevent thundering herd — only one refresh at a time. + /// + /// BL-P2-083: for rescoped sessions (active scope differs from the + /// credential's initial scope), an additional preemptive guard kicks + /// in when the cached token is within 5 minutes of expiry — those + /// can't be self-refreshed (the C1 guard in `refresh_token` rejects + /// rescope refresh on principle), and falling through to that path + /// surfaces an `AuthFailed` which is indistinguishable from real + /// credential rejection. We return `ApiError::SessionExpired { project }` + /// instead so the UI can prompt the user to switch context. #[tracing::instrument(skip(self))] async fn get_token(&self) -> ApiResult { // Ensure refresh loop is running (idempotent — handles cached token from disk) self.start_refresh_loop().await; + // BL-P2-083: rescoped session + near-expiry → explicit SessionExpired. + // Runs BEFORE the existing 1-minute fast path so a rescoped token + // with 2 minutes left doesn't get served and then fail mid-request. + // Initial-scope tokens fall through unchanged — refresh_token can + // mint a fresh one for them via do_authenticate. + let scope = self.active_scope_snapshot(); + let initial = self.initial_scope(); + if scope != initial { + let cached_expires_at = { + let map = self.token_map.read().unwrap_or_else(|e| e.into_inner()); + map.get(&scope).map(|t| t.expires_at) + }; + let near_expiry = match cached_expires_at { + Some(at) => at <= Utc::now() + chrono::Duration::minutes(5), + // Cached entry missing for a rescoped scope is also a + // session-lost case — treat as expired rather than letting + // the slow path fall through to AuthFailed. + None => true, + }; + if near_expiry { + let project = match &scope { + TokenScope::Project { name, .. } => name.clone(), + TokenScope::Unscoped => "".to_string(), + }; + tracing::warn!( + project = %project, + expires_at = ?cached_expires_at, + "session_expired: rescoped token expired or within 5-minute margin; reauth required" + ); + return Err(ApiError::SessionExpired { project }); + } + } + // Fast path: token is still valid for active scope let scope = self.active_scope_snapshot(); { @@ -812,6 +854,102 @@ mod tests { assert!(err.is_err()); } + // ---------- BL-P2-083: interim session-expired guard ---------- + // Rescoped sessions (set_active called with a non-initial scope) cannot + // be self-refreshed by KeystoneAuthAdapter — the C1 guard in + // `refresh_token` returns AuthFailed, which is indistinguishable from + // genuine credential rejection. BL-P2-083 surfaces this as the explicit + // `ApiError::SessionExpired` variant + tracing telemetry so the user + // gets "switch context to reauth" instead of "credentials rejected." + + fn rescoped_token(id: &str, project_name: &str, expires_in_minutes: i64) -> Token { + Token { + id: id.into(), + expires_at: Utc::now() + chrono::Duration::minutes(expires_in_minutes), + project: ProjectScope { + id: format!("id-{project_name}"), + name: project_name.into(), + domain_id: "default".into(), + domain_name: "default".into(), + }, + roles: Vec::new(), + catalog: Vec::new(), + user_id: "user-uuid".into(), + } + } + + fn install_rescoped_token(adapter: &KeystoneAuthAdapter, token: Token, project_name: &str) { + // Swap active scope to the rescoped target and stage its token. + let scope = TokenScope::Project { + name: project_name.into(), + domain: "default".into(), + }; + { + let mut map = adapter.token_map.write().unwrap_or_else(|e| e.into_inner()); + map.insert(scope.clone(), token); + } + { + let mut active = adapter.active_scope.lock().unwrap_or_else(|e| e.into_inner()); + *active = scope; + } + } + + #[tokio::test] + async fn test_get_token_returns_session_expired_for_expired_rescoped_token() { + // Past-expiry rescoped token: refresh would hit the C1 guard + // (AuthFailed). The interim guard must short-circuit with the + // explicit SessionExpired variant before that mis-classification + // can occur. + let adapter = KeystoneAuthAdapter::new(sample_credential_password()).unwrap(); + let token = rescoped_token("tok-rescoped-stale", "demo", -1); // expired 1 min ago + install_rescoped_token(&adapter, token, "demo"); + + let err = adapter.get_token().await.expect_err("must error"); + match err { + ApiError::SessionExpired { project } => assert_eq!(project, "demo"), + other => panic!("expected SessionExpired, got {other:?}"), + } + } + + #[tokio::test] + async fn test_get_token_returns_session_expired_within_5min_margin_for_rescoped() { + // Rescoped token with only 2 minutes left — inside the 5-minute + // preemptive margin so callers get a clear reauth prompt before + // any in-flight operation can fail mid-request. + let adapter = KeystoneAuthAdapter::new(sample_credential_password()).unwrap(); + let token = rescoped_token("tok-rescoped-soon", "demo", 2); + install_rescoped_token(&adapter, token, "demo"); + + let err = adapter.get_token().await.expect_err("must error"); + assert!( + matches!(err, ApiError::SessionExpired { ref project } if project == "demo"), + "expected SessionExpired, got {err:?}" + ); + } + + #[tokio::test] + async fn test_get_token_initial_scope_near_expiry_does_not_return_session_expired() { + // Initial-scope token near expiry must NOT surface SessionExpired — + // refresh_token can self-refresh it (C1 guard only blocks rescoped + // refreshes). The interim guard is rescope-only on purpose; any + // error here comes from the refresh HTTP path (unreachable in + // unit test → Network/Unexpected), not from the guard. + let adapter = KeystoneAuthAdapter::new(sample_credential_password()).unwrap(); + let initial_scope = adapter.initial_scope(); + let mut near_expiry = sample_token("tok-near", "admin-project"); + near_expiry.expires_at = Utc::now() + chrono::Duration::seconds(30); + { + let mut map = adapter.token_map.write().unwrap_or_else(|e| e.into_inner()); + map.insert(initial_scope, near_expiry); + } + + let err = adapter.get_token().await.expect_err("must error"); + assert!( + !matches!(err, ApiError::SessionExpired { .. }), + "initial-scope expiry must not be classified as SessionExpired, got {err:?}" + ); + } + #[tokio::test] async fn test_get_catalog() { let adapter = KeystoneAuthAdapter::new(sample_credential_password()).unwrap(); diff --git a/src/app.rs b/src/app.rs index fd9ec71..efb793e 100644 --- a/src/app.rs +++ b/src/app.rs @@ -1460,6 +1460,17 @@ impl App { "Auth".into(), String::new(), ), + // BL-P2-083: dedicated message so the user knows to re-run + // `:switch-context` rather than retry credentials. Carries the + // active project name for clarity in multi-project workflows. + AppEvent::SessionExpired { project } => ( + format!( + "{project} 세션이 만료되었습니다. :switch-context로 재전환하거나 앱을 다시 시작하세요." + ), + ToastLevel::Error, + "SessionExpired".into(), + project.clone(), + ), AppEvent::PermissionDenied { operation } => ( format!("Permission denied: {operation}"), ToastLevel::Error, @@ -3156,6 +3167,29 @@ mod tests { assert!(entry.message.contains("quota exceeded")); } + // --- BL-P2-083: SessionExpired toast --- + + #[test] + fn test_generate_toast_for_session_expired_uses_reauth_message() { + let mut app = make_app(); + app.handle_event(AppEvent::SessionExpired { + project: "demo".into(), + }); + assert_eq!(app.activity_log.entries().len(), 1); + let entry = &app.activity_log.entries()[0]; + assert!( + !entry.success, + "SessionExpired must surface as a failure in the activity log" + ); + assert_eq!(entry.operation, "SessionExpired"); + assert_eq!(entry.resource_name, "demo"); + assert!( + entry.message.contains("세션이 만료") && entry.message.contains(":switch-context"), + "toast must include reauth prompt: {msg}", + msg = entry.message, + ); + } + // --- BL-P2-085 Step 11c: cross-project block toast --- #[test] diff --git a/src/event.rs b/src/event.rs index 7528e0b..8f087e5 100644 --- a/src/event.rs +++ b/src/event.rs @@ -202,6 +202,17 @@ pub enum AppEvent { action: String, }, + /// BL-P2-083: the rescoped session reached near-expiry and cannot be + /// auto-refreshed by the auth adapter (C1 guard rejects rescope-side + /// refresh on principle). The worker converts `ApiError::SessionExpired` + /// into this variant so the UI can prompt "switch context to reauth" + /// rather than the generic "credentials rejected" message produced by + /// `AppEvent::ApiError` / `AppEvent::AuthFailed`. `project` carries the + /// active project name for the user-facing toast. + SessionExpired { + project: String, + }, + // System CloudSwitched(String), diff --git a/src/port/error.rs b/src/port/error.rs index 1e2981f..2dee6ad 100644 --- a/src/port/error.rs +++ b/src/port/error.rs @@ -9,6 +9,17 @@ pub enum ApiError { #[error("Token expired")] TokenExpired, + /// BL-P2-083: rescoped session reached near-expiry and cannot be + /// auto-refreshed (the C1 guard in `KeystoneAuthAdapter::refresh_token` + /// blocks refreshing tokens whose scope differs from the credential's + /// initial scope — that's the rescope adapter's job, not the auth + /// adapter's). Distinct from `AuthFailed` so callers can surface a + /// "switch context to reauth" prompt rather than "credentials + /// rejected." `project` carries the active project's name for the + /// user-facing message. + #[error("Session expired for project {project}; switch context to reauthenticate")] + SessionExpired { project: String }, + #[error("Forbidden: {0}")] Forbidden(String), diff --git a/src/worker.rs b/src/worker.rs index baf3f15..69c1e9a 100644 --- a/src/worker.rs +++ b/src/worker.rs @@ -1300,6 +1300,16 @@ async fn poll_migration_progress( } fn api_error(operation: &str, error: crate::port::error::ApiError) -> AppEvent { + // BL-P2-083: split SessionExpired off from the generic ApiError path so + // the UI gets a dedicated "switch context to reauth" prompt instead of + // "Auth failed: ..." that's indistinguishable from credential rejection. + // The adapter-level tracing::warn already covers the structured field + // logging; here we keep parity with the legacy error trace for + // operability (find-by-operation grep still works). + if let crate::port::error::ApiError::SessionExpired { project } = error { + tracing::warn!(operation, project = %project, "session_expired surfaced to UI"); + return AppEvent::SessionExpired { project }; + } tracing::error!(operation, error = %error, "API call failed"); AppEvent::ApiError { operation: operation.to_string(), @@ -2374,6 +2384,23 @@ mod tests { } } + // BL-P2-083: SessionExpired bypasses the generic ApiError funnel so the + // UI can surface the dedicated "switch context to reauth" toast. The + // funnel test above guards the default path; this one guards the split. + #[test] + fn test_api_error_routes_session_expired_to_dedicated_event() { + let event = api_error( + "FetchServers", + crate::port::error::ApiError::SessionExpired { + project: "demo".into(), + }, + ); + match event { + AppEvent::SessionExpired { project } => assert_eq!(project, "demo"), + other => panic!("expected SessionExpired, got {other:?}"), + } + } + // -- BL-P2-086: live-migrate "No valid host" enrichment -- // // Nova returns "No valid host was found" as a generic message when From a6f8befc66579a4a3e25bcee7eb7486d08a60b12 Mon Sep 17 00:00:00 2001 From: bluejayA Date: Wed, 13 May 2026 11:06:20 +0900 Subject: [PATCH 2/3] fix(BL-P2-083): point SessionExpired toast at app restart only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex review P2: the original toast suggested either `:switch-context` or app restart, but `KeystoneRescopeAdapter::rescope` authenticates the rescope request with the (now-expired) current token, so for a fully-expired session `:switch-context` fails with a generic `RescopeRejected` — the user follows the prompt and hits a second cryptic error. The interim toast now recommends only the path that always works (app restart). The "still rescopeable within the 5-min margin" UX nuance is tracked as BL-P2-096 follow-up. Also registers BL-P2-096 in devflow-docs/backlog.md so the smart recovery work doesn't get lost. Co-Authored-By: Claude Opus 4.7 (1M context) --- devflow-docs/backlog.md | 22 ++++++++++++++++++++++ src/app.rs | 18 ++++++++++++------ 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/devflow-docs/backlog.md b/devflow-docs/backlog.md index 451b72d..eec1fd2 100644 --- a/devflow-docs/backlog.md +++ b/devflow-docs/backlog.md @@ -356,6 +356,28 @@ BL-P2-080 Unit 3(`.github/workflows/ci.yml::devstack-integration`)은 placeholde **Ref**: cargo-review branch-full report 2026-05-12 (Suggestions #1). +### BL-P2-096: SessionExpired smart recovery — distinguish near-expiry vs already-expired in toast and switch path (BL-P2-083 follow-up) +**Priority**: Medium +**Parent**: BL-P2-083 codex-review branch-diff P2 (2026-05-13) +**Category**: Auth / UX + +**Description**: BL-P2-083's interim guard surfaces `SessionExpired` for both states: +- (a) "still rescopeable" — within 5-min margin but > 1 min remaining; `:switch-context ` would actually work +- (b) "fully expired or lost" — already past expires_at, or active scope's token missing entirely; only app restart works + +The current interim toast safely recommends restart in both cases (Codex P2 fix — `:switch-context` was misleading because `KeystoneRescopeAdapter::rescope` authenticates with the current (expired) token and fails with `RescopeRejected`). State (a) is salvageable via switch-context but the user is told to restart anyway. + +본 BL에서: +1. `ApiError::SessionExpired` payload에 `expired: bool` 추가 (true = past expires_at; false = within margin) +2. `keystone.rs::get_token`에서 두 케이스 구분해 채움 +3. `App::generate_toast`에서 expired=true → 재시작 안내, expired=false → "지금 :switch-context로 전환 후 재시도 가능, 실패 시 재시작" 안내 +4. (선택) `spawn_switch` 실패 경로에서 `RescopeRejected` + 만료 토큰 감지 → 자동으로 `AppEvent::SessionExpired { expired: true }` 재발행 (cryptic 에러 방지) +5. 추가 tests: state-별 토스트 메시지, spawn_switch 폴백 (선택 항목 구현 시) + +**Out of scope**: 백그라운드 auto-refresh — BL-P2-052 Part A 영역. + +**Ref**: 2026-05-13 codex-review branch-diff P2 on BL-P2-083 PR #88. + ### BL-P2-095: App-side success audit attribution should use Keystone UUID (BL-P2-093 follow-up) **Priority**: Medium **Parent**: BL-P2-093 codex-review branch-diff open question (2026-05-12) diff --git a/src/app.rs b/src/app.rs index efb793e..9692b3d 100644 --- a/src/app.rs +++ b/src/app.rs @@ -1460,12 +1460,18 @@ impl App { "Auth".into(), String::new(), ), - // BL-P2-083: dedicated message so the user knows to re-run - // `:switch-context` rather than retry credentials. Carries the - // active project name for clarity in multi-project workflows. + // BL-P2-083: dedicated message that recommends the only path + // that reliably works for a fully-expired rescoped session — + // app restart. `:switch-context` was tempting to suggest but + // the rescope adapter uses the (expired) current token to + // authenticate the rescope request, so an already-expired + // session can't recover via switch — it surfaces as a + // generic RescopeRejected (Codex review P2). Smart recovery + // for the near-expiry-but-still-rescopeable window is tracked + // as BL-P2-096. AppEvent::SessionExpired { project } => ( format!( - "{project} 세션이 만료되었습니다. :switch-context로 재전환하거나 앱을 다시 시작하세요." + "{project} 세션이 만료되었습니다. 앱을 다시 시작해 재인증하세요." ), ToastLevel::Error, "SessionExpired".into(), @@ -3184,8 +3190,8 @@ mod tests { assert_eq!(entry.operation, "SessionExpired"); assert_eq!(entry.resource_name, "demo"); assert!( - entry.message.contains("세션이 만료") && entry.message.contains(":switch-context"), - "toast must include reauth prompt: {msg}", + entry.message.contains("세션이 만료") && entry.message.contains("다시 시작"), + "toast must include expired notice + restart prompt: {msg}", msg = entry.message, ); } From efe44c0b9c0ed9105414d23662d6fc241b04c239 Mon Sep 17 00:00:00 2001 From: bluejayA Date: Wed, 13 May 2026 11:19:56 +0900 Subject: [PATCH 3/3] =?UTF-8?q?style(BL-P2-083):=20cargo=20fmt=20=E2=80=94?= =?UTF-8?q?=20match=20CI=20rustfmt=20formatting?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Local rustfmt was lenient with chain wrap + format!() single-line; CI's stricter rustfmt flagged two diffs. No semantic change. Co-Authored-By: Claude Opus 4.7 (1M context) --- devflow-docs/audit.md | 1 + src/adapter/auth/keystone.rs | 5 ++++- src/app.rs | 4 +--- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/devflow-docs/audit.md b/devflow-docs/audit.md index 18cd7a5..5005b87 100644 --- a/devflow-docs/audit.md +++ b/devflow-docs/audit.md @@ -433,3 +433,4 @@ - 2026-05-11T10:23:03Z — file-edit — devflow-docs/backlog.md - 2026-05-12T04:17:11Z — file-edit — devflow-docs/backlog.md - 2026-05-12T11:39:13Z — file-edit — devflow-docs/backlog.md +- 2026-05-13T01:53:38Z — file-edit — devflow-docs/backlog.md diff --git a/src/adapter/auth/keystone.rs b/src/adapter/auth/keystone.rs index 4fe1a66..4d0d381 100644 --- a/src/adapter/auth/keystone.rs +++ b/src/adapter/auth/keystone.rs @@ -889,7 +889,10 @@ mod tests { map.insert(scope.clone(), token); } { - let mut active = adapter.active_scope.lock().unwrap_or_else(|e| e.into_inner()); + let mut active = adapter + .active_scope + .lock() + .unwrap_or_else(|e| e.into_inner()); *active = scope; } } diff --git a/src/app.rs b/src/app.rs index 9692b3d..e537aba 100644 --- a/src/app.rs +++ b/src/app.rs @@ -1470,9 +1470,7 @@ impl App { // for the near-expiry-but-still-rescopeable window is tracked // as BL-P2-096. AppEvent::SessionExpired { project } => ( - format!( - "{project} 세션이 만료되었습니다. 앱을 다시 시작해 재인증하세요." - ), + format!("{project} 세션이 만료되었습니다. 앱을 다시 시작해 재인증하세요."), ToastLevel::Error, "SessionExpired".into(), project.clone(),