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/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/adapter/auth/keystone.rs b/src/adapter/auth/keystone.rs index 64f96b5..4d0d381 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,105 @@ 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..e537aba 100644 --- a/src/app.rs +++ b/src/app.rs @@ -1460,6 +1460,21 @@ impl App { "Auth".into(), String::new(), ), + // 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} 세션이 만료되었습니다. 앱을 다시 시작해 재인증하세요."), + ToastLevel::Error, + "SessionExpired".into(), + project.clone(), + ), AppEvent::PermissionDenied { operation } => ( format!("Permission denied: {operation}"), ToastLevel::Error, @@ -3156,6 +3171,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("다시 시작"), + "toast must include expired notice + restart 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