impl explicit session end for mfa config

new-ui/src/pages/full/ConfigureMfaPage/hooks/useConfigureMfaStore.test.ts

@@ -1,7 +1,17 @@
 import { beforeEach, describe, expect, it, vi } from 'vitest';
-import { MfaMethod } from '../../../../shared/rust-api/types';
+import { api } from '../../../../shared/rust-api/api';
+import {
+  ClientTrafficPolicy,
+  type InstanceInfo,
+  MfaMethod,
+} from '../../../../shared/rust-api/types';
 import { ConfigureMfaStep } from '../types';
-import { applyAuthorization, useConfigureMfaStore } from './useConfigureMfaStore';
+import {
+  applyAuthorization,
+  discardMfaConfiguration,
+  startMfaConfiguration,
+  useConfigureMfaStore,
+} from './useConfigureMfaStore';
 
 vi.mock('@tauri-apps/plugin-log', () => ({ error: vi.fn() }));
 vi.mock('../../../../shared/rust-api/api', () => ({ api: {} }));
@@ -67,3 +77,45 @@ describe('applyAuthorization', () => {
     expect(useConfigureMfaStore.getState().authorized).toBe(false);
   });
 });
+
+describe('startMfaConfiguration', () => {
+  const instance: InstanceInfo = {
+    id: 1,
+    name: 'instance',
+    uuid: 'instance-uuid',
+    url: 'https://core.example',
+    proxy_url: 'https://proxy.example',
+    active: false,
+    pubkey: 'pubkey',
+    client_traffic_policy: ClientTrafficPolicy.None,
+    enterprise_enabled: false,
+    disable_tunnels: false,
+    openid_display_name: null,
+    mfa_configured_methods: [],
+  };
+
+  it('waits for the previous session to end before starting a new one', async () => {
+    let endPrevious = () => {};
+    const ended = new Promise<void>((resolve) => {
+      endPrevious = resolve;
+    });
+    const mfaConfigStart = vi.fn().mockResolvedValue({
+      session_id: 'session-2',
+      available_methods: [MfaMethod.Totp],
+      email_fallback: false,
+      deadline_timestamp: 1_900_000_000,
+    });
+    Object.assign(api, { mfaConfigCancel: vi.fn(() => ended), mfaConfigStart });
+    useConfigureMfaStore.setState({ sessionId });
+
+    void discardMfaConfiguration();
+    const started = startMfaConfiguration(instance);
+    await Promise.resolve();
+    expect(mfaConfigStart).not.toHaveBeenCalled();
+
+    endPrevious();
+    await started;
+    expect(mfaConfigStart).toHaveBeenCalledOnce();
+    expect(useConfigureMfaStore.getState().sessionId).toBe('session-2');
+  });
+});

new-ui/src/pages/full/ConfigureMfaPage/hooks/useConfigureMfaStore.tsx

@@ -260,6 +260,8 @@ export const selectPendingMethod =
   (state: Store): MfaMethodValue | undefined =>
     pendingMethods(state).find((method) => mfaFactorStep(method) === step);
 
+let pendingEnd: Promise<void> = Promise.resolve();
+
 type StartOptions = Partial<ConfigureMfaOrigin> & {
   preselectedMethods?: MfaMethodValue[];
 };
@@ -268,6 +270,8 @@ export const startMfaConfiguration = async (
   instance: InstanceInfo,
   { preselectedMethods = [], source = null, location = null }: StartOptions = {},
 ): Promise<void> => {
+  // Core ends every session of the user, so an end still in flight would take this one with it.
+  await pendingEnd;
   const response = await api.mfaConfigStart(instance.id);
   dismissEdgeComsError();
   useConfigureMfaStore.getState().start(instance, response, { source, location });
@@ -289,14 +293,20 @@ export const applyAuthorization = (
   store.authorize(result);
 };
 
-/** A copy the proxy still holds expires on its own, so a failed cancel is not worth raising. */
-export const discardMfaConfiguration = async (): Promise<void> => {
-  const { sessionId } = useConfigureMfaStore.getState();
-  useConfigureMfaStore.getState().reset();
-  if (!isPresent(sessionId)) return;
+const endSession = async (sessionId: string): Promise<void> => {
   try {
     await api.mfaConfigCancel(sessionId);
   } catch (err) {
     void logError(`Failed to cancel MFA configuration session: ${err}`);
   }
 };
+
+/** Also ends the session on Core. One the proxy still holds expires on its own, so a failure is
+ *  not worth raising. */
+export const discardMfaConfiguration = async (): Promise<void> => {
+  const { sessionId } = useConfigureMfaStore.getState();
+  useConfigureMfaStore.getState().reset();
+  if (!isPresent(sessionId)) return;
+  pendingEnd = endSession(sessionId);
+  await pendingEnd;
+};

src-tauri/core/src/mfa_config.rs

@@ -7,8 +7,8 @@ use chrono::Utc;
 use defguard_client_proto::defguard::client_types::{
     CodeMfaSetupFinishRequest, CodeMfaSetupFinishResponse, CodeMfaSetupStartRequest,
     CodeMfaSetupStartResponse, MfaConfigAuthorizeRequest, MfaConfigAuthorizeResponse,
-    MfaConfigFido2ChallengeRequest, MfaConfigFido2ChallengeResponse, MfaConfigSendCodeRequest,
-    MfaConfigStartRequest, MfaConfigStartResponse, MfaMethod,
+    MfaConfigEndRequest, MfaConfigFido2ChallengeRequest, MfaConfigFido2ChallengeResponse,
+    MfaConfigSendCodeRequest, MfaConfigStartRequest, MfaConfigStartResponse, MfaMethod,
 };
 use reqwest::{Response, StatusCode, Url};
 use serde::{de::DeserializeOwned, Serialize};
@@ -32,6 +32,7 @@ const AUTHORIZE: &str = "api/v1/mfa-config/authorize";
 const FIDO2_CHALLENGE: &str = "api/v1/mfa-config/fido2-challenge";
 const SETUP_START: &str = "api/v1/mfa-config/setup/start";
 const SETUP_FINISH: &str = "api/v1/mfa-config/setup/finish";
+const END: &str = "api/v1/mfa-config/end";
 
 // mirrors the methods Core accepts in mfa_config_authorize, keep in step
 pub const AUTHORIZING_METHODS: &[MfaMethod] = &[
@@ -439,5 +440,13 @@ pub async fn mfa_config_setup_finish(
     parse(post(&proxy_url, SETUP_FINISH, &request).await?).await
 }
 
+/// Lets Core drop the session before its deadline. The proxy answers with an empty 200.
+pub async fn mfa_config_end(proxy_url: Url, session_token: String) -> Result<(), MfaConfigError> {
+    debug!("Ending MFA configuration session");
+    let request = MfaConfigEndRequest { session_token };
+    post(&proxy_url, END, &request).await?;
+    Ok(())
+}
+
 #[cfg(test)]
 mod tests;

src-tauri/core/src/mfa_config/tests.rs

@@ -158,6 +158,22 @@ async fn test_send_code_tolerates_empty_body() {
         .unwrap();
 }
 
+#[tokio::test]
+async fn test_end_sends_session_token() {
+    let server = MockServer::start().await;
+    Mock::given(method("POST"))
+        .and(path(format!("/{END}")))
+        .and(body_partial_json(json!({ "session_token": SESSION_TOKEN })))
+        .respond_with(ResponseTemplate::new(200))
+        .expect(1)
+        .mount(&server)
+        .await;
+
+    mfa_config_end(mock_url(&server), SESSION_TOKEN.into())
+        .await
+        .unwrap();
+}
+
 #[tokio::test]
 async fn test_setup_start_returns_totp_secret() {
     let server = MockServer::start().await;

src-tauri/src/commands.rs

@@ -2802,13 +2802,14 @@ fn abort_mfa_config_attempt(state: &AppState, uid: Uuid) {
     }
 }
 
-fn cancel_mfa_config_session(state: &AppState, uid: Uuid) {
-    state
+fn cancel_mfa_config_session(state: &AppState, uid: Uuid) -> Option<MfaConfigSession> {
+    let session = state
         .mfa_config_sessions
         .lock()
         .expect("mfa_config_sessions mutex poisoned")
         .remove(&uid);
     abort_mfa_config_attempt(state, uid);
+    session
 }
 
 /// unlike mfa_config_cancel, the session survives for another method
@@ -2823,14 +2824,25 @@ pub async fn mfa_config_abort_attempt(
     Ok(())
 }
 
+/// Awaited rather than spawned: Core ends every MFA configuration session of the user, so an
+/// end landing after the next `mfa_config_start` would kill that session too.
 #[tauri::command(async)]
 pub async fn mfa_config_cancel(
     session_id: String,
     state: State<'_, AppState>,
 ) -> Result<(), String> {
     debug!("Cancelling MFA configuration session");
     let uid = parse_mfa_config_session_id(&session_id)?;
-    cancel_mfa_config_session(&state, uid);
+    let Some(session) = cancel_mfa_config_session(&state, uid) else {
+        return Ok(());
+    };
+    if session.is_expired(Utc::now().timestamp()) {
+        return Ok(());
+    }
+    // Best effort, an older proxy lacks the route and the session dies at its deadline anyway.
+    if let Err(err) = mfa_config::mfa_config_end(session.proxy_url, session.session_token).await {
+        warn!("Failed to end MFA configuration session: {err}");
+    }
     Ok(())
 }