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
1 change: 1 addition & 0 deletions devflow-docs/audit.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
22 changes: 22 additions & 0 deletions devflow-docs/backlog.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 <other-project>` 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)
Expand Down
141 changes: 141 additions & 0 deletions src/adapter/auth/keystone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> {
// 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 => "<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();
{
Expand Down Expand Up @@ -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();
Expand Down
38 changes: 38 additions & 0 deletions src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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]
Expand Down
11 changes: 11 additions & 0 deletions src/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),

Expand Down
11 changes: 11 additions & 0 deletions src/port/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),

Expand Down
27 changes: 27 additions & 0 deletions src/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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
Expand Down
Loading