From 2ae413c5eaf39bf3e9ce471ad866c3c64d4363f4 Mon Sep 17 00:00:00 2001 From: Anthony Stirling <77850077+Frooodle@users.noreply.github.com> Date: Sat, 31 Jan 2026 20:28:59 +0000 Subject: [PATCH] Stop attempting to refresh Spring tokens in desktop (#5610) # Description of Changes --- ## Checklist ### General - [ ] I have read the [Contribution Guidelines](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/CONTRIBUTING.md) - [ ] I have read the [Stirling-PDF Developer Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/DeveloperGuide.md) (if applicable) - [ ] I have read the [How to add new languages to Stirling-PDF](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md) (if applicable) - [ ] I have performed a self-review of my own code - [ ] My changes generate no new warnings ### Documentation - [ ] I have updated relevant docs on [Stirling-PDF's doc repo](https://github.com/Stirling-Tools/Stirling-Tools.github.io/blob/main/docs/) (if functionality has heavily changed) - [ ] I have read the section [Add New Translation Tags](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md#add-new-translation-tags) (for new translation tags only) ### Translations (if applicable) - [ ] I ran [`scripts/counter_translation.py`](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/docs/counter_translation.md) ### UI Changes (if applicable) - [ ] Screenshots or videos demonstrating the UI changes are attached (e.g., as comments or direct attachments in the PR) ### Testing (if applicable) - [ ] I have tested my changes locally. Refer to the [Testing Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/DeveloperGuide.md#6-testing) for more details. --------- Co-authored-by: James Brunton --- .../api/AuthControllerLoginTest.java | 1 + frontend/src-tauri/src/commands/auth.rs | 114 ++++++++++++++ frontend/src-tauri/src/commands/mod.rs | 3 + frontend/src-tauri/src/lib.rs | 6 + .../src/desktop/services/apiClientSetup.ts | 96 +++++++----- frontend/src/desktop/services/authService.ts | 143 ++++++++++++++++-- 6 files changed, 309 insertions(+), 54 deletions(-) diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/security/controller/api/AuthControllerLoginTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/security/controller/api/AuthControllerLoginTest.java index 29dabe152..86bcf50e8 100644 --- a/app/proprietary/src/test/java/stirling/software/proprietary/security/controller/api/AuthControllerLoginTest.java +++ b/app/proprietary/src/test/java/stirling/software/proprietary/security/controller/api/AuthControllerLoginTest.java @@ -182,6 +182,7 @@ class AuthControllerLoginTest { mockMvc.perform(post("/api/v1/auth/refresh")) .andExpect(status().isOk()) + .andExpect(jsonPath("$.user").exists()) .andExpect(jsonPath("$.session.access_token").value("new-token")) .andExpect(jsonPath("$.session.expires_in").value(3600)); } diff --git a/frontend/src-tauri/src/commands/auth.rs b/frontend/src-tauri/src/commands/auth.rs index 43fa9128c..65f8f6623 100644 --- a/frontend/src-tauri/src/commands/auth.rs +++ b/frontend/src-tauri/src/commands/auth.rs @@ -11,8 +11,11 @@ use rand::distributions::Alphanumeric; const STORE_FILE: &str = "connection.json"; const USER_INFO_KEY: &str = "user_info"; +const TOKENS_STORE_FILE: &str = "tokens.json"; +const REFRESH_TOKEN_STORE_KEY: &str = "refresh_token"; const KEYRING_SERVICE: &str = "stirling-pdf"; const KEYRING_TOKEN_KEY: &str = "auth-token"; +const KEYRING_REFRESH_TOKEN_KEY: &str = "refresh-token"; #[derive(Debug, Serialize, Deserialize, Clone)] pub struct UserInfo { @@ -31,6 +34,11 @@ fn get_keyring_entry() -> Result { Ok(entry) } +fn get_refresh_token_keyring_entry() -> Result { + Entry::new(KEYRING_SERVICE, KEYRING_REFRESH_TOKEN_KEY) + .map_err(|e| format!("Failed to access keyring: {}", e)) +} + #[tauri::command] pub async fn save_auth_token(_app_handle: AppHandle, token: String) -> Result<(), String> { let trimmed = token.trim(); @@ -101,6 +109,112 @@ pub async fn clear_auth_token(_app_handle: AppHandle) -> Result<(), String> { } } +#[tauri::command] +pub async fn save_refresh_token(app_handle: AppHandle, token: String) -> Result<(), String> { + log::info!("Saving refresh token - trying keyring first"); + + let entry = get_refresh_token_keyring_entry()?; + + // Try keyring (works in production with code signing) + match entry.set_password(&token) { + Ok(_) => { + // Verify it persists (fails in unsigned dev builds) + match entry.get_password() { + Ok(saved) if saved == token => { + log::info!("✅ Refresh token saved to keyring (production mode)"); + return Ok(()); + } + _ => { + log::info!("Keyring doesn't persist - using Tauri Store fallback (dev mode)"); + } + } + } + Err(e) => { + log::info!("Keyring failed: {} - using Tauri Store fallback", e); + } + } + + // Fallback to Tauri Store (dev mode without code signing) + let store = app_handle + .store(TOKENS_STORE_FILE) + .map_err(|e| format!("Failed to access tokens store: {}", e))?; + + store.set( + REFRESH_TOKEN_STORE_KEY, + serde_json::to_value(&token) + .map_err(|e| format!("Failed to serialize token: {}", e))?, + ); + + store + .save() + .map_err(|e| format!("Failed to save tokens store: {}", e))?; + + log::info!("✅ Refresh token saved to Tauri Store (fallback)"); + Ok(()) +} + +#[tauri::command] +pub async fn get_refresh_token(app_handle: AppHandle) -> Result, String> { + // Try keyring first (production) + let entry = get_refresh_token_keyring_entry()?; + match entry.get_password() { + Ok(token) => { + log::info!("✅ Refresh token retrieved from keyring"); + return Ok(Some(token)); + } + Err(keyring::Error::NoEntry) => { + log::debug!("No token in keyring, trying Tauri Store"); + } + Err(e) => { + log::warn!("Keyring error: {} - trying Tauri Store", e); + } + } + + // Fallback to Tauri Store (dev) + let store = app_handle + .store(TOKENS_STORE_FILE) + .map_err(|e| format!("Failed to access tokens store: {}", e))?; + + let token: Option = store + .get(REFRESH_TOKEN_STORE_KEY) + .and_then(|v| serde_json::from_value(v.clone()).ok()); + + if token.is_some() { + log::info!("✅ Refresh token retrieved from Tauri Store"); + } else { + log::info!("No refresh token found"); + } + + Ok(token) +} + +#[tauri::command] +pub async fn clear_refresh_token(app_handle: AppHandle) -> Result<(), String> { + log::info!("Clearing refresh token from all storage"); + + // Clear from keyring + let entry = get_refresh_token_keyring_entry()?; + match entry.delete_credential() { + Ok(_) => log::info!("Cleared from keyring"), + Err(keyring::Error::NoEntry) => log::debug!("Not in keyring"), + Err(e) => log::warn!("Keyring clear error: {}", e), + } + + // Clear from Tauri Store + let store = app_handle + .store(TOKENS_STORE_FILE) + .map_err(|e| format!("Failed to access tokens store: {}", e))?; + + store.delete(REFRESH_TOKEN_STORE_KEY); + + store + .save() + .map_err(|e| format!("Failed to save tokens store: {}", e))?; + + log::info!("✅ Refresh token cleared"); + Ok(()) +} + #[tauri::command] pub async fn save_user_info( app_handle: AppHandle, diff --git a/frontend/src-tauri/src/commands/mod.rs b/frontend/src-tauri/src/commands/mod.rs index 6e058a5be..30904bbd9 100644 --- a/frontend/src-tauri/src/commands/mod.rs +++ b/frontend/src-tauri/src/commands/mod.rs @@ -14,11 +14,14 @@ pub use connection::{ }; pub use auth::{ clear_auth_token, + clear_refresh_token, clear_user_info, get_auth_token, + get_refresh_token, get_user_info, login, save_auth_token, + save_refresh_token, save_user_info, start_oauth_login, }; diff --git a/frontend/src-tauri/src/lib.rs b/frontend/src-tauri/src/lib.rs index 61cbd6d43..ad49ea16e 100644 --- a/frontend/src-tauri/src/lib.rs +++ b/frontend/src-tauri/src/lib.rs @@ -9,17 +9,20 @@ use commands::{ cleanup_backend, clear_auth_token, clear_opened_files, + clear_refresh_token, clear_user_info, is_default_pdf_handler, get_auth_token, get_backend_port, get_connection_config, get_opened_files, + get_refresh_token, get_user_info, is_first_launch, login, reset_setup_completion, save_auth_token, + save_refresh_token, save_user_info, set_connection_mode, set_as_default_pdf_handler, @@ -143,6 +146,9 @@ pub fn run() { save_auth_token, get_auth_token, clear_auth_token, + save_refresh_token, + get_refresh_token, + clear_refresh_token, save_user_info, get_user_info, clear_user_info, diff --git a/frontend/src/desktop/services/apiClientSetup.ts b/frontend/src/desktop/services/apiClientSetup.ts index a61f554dc..d70bc2afa 100644 --- a/frontend/src/desktop/services/apiClientSetup.ts +++ b/frontend/src/desktop/services/apiClientSetup.ts @@ -6,6 +6,7 @@ import { createBackendNotReadyError } from '@app/constants/backendErrors'; import { operationRouter } from '@app/services/operationRouter'; import { authService } from '@app/services/authService'; import { connectionModeService } from '@app/services/connectionModeService'; +import { STIRLING_SAAS_URL } from '@app/constants/connection'; import i18n from '@app/i18n'; const BACKEND_TOAST_COOLDOWN_MS = 4000; @@ -34,37 +35,40 @@ export function setupApiInterceptors(client: AxiosInstance): void { async (config: InternalAxiosRequestConfig) => { const extendedConfig = config as ExtendedRequestConfig; - // Get the operation name from config if provided - const operation = extendedConfig.operationName; + try { + // Get the appropriate base URL for this request + const baseUrl = await operationRouter.getBaseUrl(extendedConfig.url); - // Get the appropriate base URL for this operation - const baseUrl = await operationRouter.getBaseUrl(operation); - - // Build the full URL - if (extendedConfig.url && !extendedConfig.url.startsWith('http')) { - extendedConfig.url = `${baseUrl}${extendedConfig.url}`; - } - - localStorage.setItem('server_url', baseUrl); - - // Debug logging - console.debug(`[apiClientSetup] Request to: ${extendedConfig.url}`); - - // Add auth token for remote requests and enable credentials - const isRemote = await operationRouter.isSelfHostedMode(); - if (isRemote) { - // Self-hosted mode: enable credentials for session management - extendedConfig.withCredentials = true; - - const token = await authService.getAuthToken(); - if (token) { - extendedConfig.headers.Authorization = `Bearer ${token}`; - } else { - console.warn('[apiClientSetup] Self-hosted mode but no auth token available'); + // Build the full URL + if (extendedConfig.url && !extendedConfig.url.startsWith('http')) { + extendedConfig.url = `${baseUrl}${extendedConfig.url}`; } - } else { - // SaaS mode: disable credentials (security disabled on local backend) - extendedConfig.withCredentials = false; + + localStorage.setItem('server_url', baseUrl); + + // Debug logging + console.debug(`[apiClientSetup] Request to: ${extendedConfig.url}`); + + // Add auth token for remote requests and enable credentials + const isRemote = await operationRouter.isSelfHostedMode(); + if (isRemote) { + // Self-hosted mode: enable credentials for session management + extendedConfig.withCredentials = true; + + const token = await authService.getAuthToken(); + if (token) { + extendedConfig.headers.Authorization = `Bearer ${token}`; + } else { + console.warn('[apiClientSetup] Self-hosted mode but no auth token available'); + } + } else { + // SaaS mode: disable credentials (security disabled on local backend) + extendedConfig.withCredentials = false; + } + } catch (error) { + console.error('[apiClientSetup] Error in request interceptor:', error); + // Continue with request even if routing/auth logic fails + // This ensures requests aren't blocked by interceptor errors } // Backend readiness check (for local backend) @@ -108,23 +112,37 @@ export function setupApiInterceptors(client: AxiosInstance): void { } originalRequest._retry = true; + console.debug(`[apiClientSetup] 401 error, attempting token refresh for: ${originalRequest.url}`); + const isRemote = await operationRouter.isSelfHostedMode(); + let refreshed = false; + if (isRemote) { + // Self-hosted mode: use Spring Boot refresh endpoint const serverConfig = await connectionModeService.getServerConfig(); if (serverConfig) { - const refreshed = await authService.refreshToken(serverConfig.url); - if (refreshed) { - // Retry the original request with new token - const token = await authService.getAuthToken(); - if (token) { - originalRequest.headers.Authorization = `Bearer ${token}`; - } - return client(originalRequest); - } + refreshed = await authService.refreshToken(serverConfig.url); } + } else { + // SaaS mode: use Supabase refresh endpoint + refreshed = await authService.refreshSupabaseToken(STIRLING_SAAS_URL); } - // Refresh failed or not in remote mode - user needs to login again + if (refreshed) { + // Retry the original request with new token + const token = await authService.getAuthToken(); + console.debug(`[apiClientSetup] Token refreshed, retrying request to: ${originalRequest.url}`); + + if (token) { + originalRequest.headers.Authorization = `Bearer ${token}`; + } else { + console.error(`[apiClientSetup] No token available after successful refresh!`); + } + + return client.request(originalRequest); + } + + // Refresh failed - user needs to login again alert({ alertType: 'error', title: i18n.t('auth.sessionExpired', 'Session Expired'), diff --git a/frontend/src/desktop/services/authService.ts b/frontend/src/desktop/services/authService.ts index 41adf954e..b3f3da911 100644 --- a/frontend/src/desktop/services/authService.ts +++ b/frontend/src/desktop/services/authService.ts @@ -40,6 +40,7 @@ export class AuthService { private userInfo: UserInfo | null = null; private cachedToken: string | null = null; private authListeners = new Set<(status: AuthStatus, userInfo: UserInfo | null) => void>(); + private refreshPromise: Promise | null = null; static getInstance(): AuthService { if (!AuthService.instance) { @@ -51,15 +52,17 @@ export class AuthService { /** * Save token to all storage locations and notify listeners */ - private async saveTokenEverywhere(token: string): Promise { + private async saveTokenEverywhere(token: string, refreshToken?: string | null): Promise { // Validate token before caching if (!token || token.trim().length === 0) { console.warn('[Desktop AuthService] Attempted to save invalid/empty token'); throw new Error('Invalid token'); } + console.log(`[Desktop AuthService] Saving token (length: ${token.length})`); + + // Save access token to Tauri secure store (primary) try { - // Save to Tauri store await invoke('save_auth_token', { token }); console.log('[Desktop AuthService] ✅ Token saved to Tauri store'); } catch (error) { @@ -67,8 +70,8 @@ export class AuthService { // Don't throw - we can still use localStorage } + // Sync to localStorage for web layer (fallback) try { - // Sync to localStorage for web layer localStorage.setItem('stirling_jwt', token); console.log('[Desktop AuthService] ✅ Token saved to localStorage'); } catch (error) { @@ -79,6 +82,19 @@ export class AuthService { this.cachedToken = token; console.log('[Desktop AuthService] ✅ Token cached in memory'); + // Save refresh token if provided (keyring with Tauri Store fallback) + if (refreshToken) { + console.log('[Desktop AuthService] Saving refresh token to secure storage...'); + try { + await invoke('save_refresh_token', { token: refreshToken }); + console.log('[Desktop AuthService] ✅ Refresh token saved to secure storage'); + // Only remove from localStorage after successful save + localStorage.removeItem('stirling_refresh_token'); + } catch (error) { + console.error('[Desktop AuthService] ❌ Failed to save refresh token:', error); + } + } + // Notify other parts of the system window.dispatchEvent(new CustomEvent('jwt-available')); console.log('[Desktop AuthService] Dispatched jwt-available event'); @@ -112,6 +128,19 @@ export class AuthService { return localStorageToken; } + /** + * Get refresh token from secure storage (keyring or Tauri Store fallback) + */ + private async getRefreshToken(): Promise { + const token = await invoke('get_refresh_token'); + if (token) { + console.log('[Desktop AuthService] ✅ Refresh token retrieved from secure storage'); + } else { + console.log('[Desktop AuthService] No refresh token in secure storage'); + } + return token; + } + /** * Clear token from all storage locations */ @@ -120,20 +149,28 @@ export class AuthService { this.cachedToken = null; console.log('[Desktop AuthService] Cache invalidated'); - // Best effort: clear Tauri keyring + // Best effort: clear Tauri keyring (both access and refresh tokens) try { await invoke('clear_auth_token'); - console.log('[Desktop AuthService] Cleared Tauri keyring token'); + console.log('[Desktop AuthService] Cleared Tauri keyring access token'); } catch (error) { - console.warn('[Desktop AuthService] Failed to clear Tauri keyring token', error); + console.warn('[Desktop AuthService] Failed to clear Tauri keyring access token', error); + } + + try { + await invoke('clear_refresh_token'); + console.log('[Desktop AuthService] Cleared Tauri keyring refresh token'); + } catch (error) { + console.warn('[Desktop AuthService] Failed to clear Tauri keyring refresh token', error); } // Best effort: clear web storage try { localStorage.removeItem('stirling_jwt'); - console.log('[Desktop AuthService] Cleared localStorage token'); + localStorage.removeItem('stirling_refresh_token'); + console.log('[Desktop AuthService] Cleared localStorage tokens'); } catch (error) { - console.warn('[Desktop AuthService] Failed to clear localStorage token', error); + console.warn('[Desktop AuthService] Failed to clear localStorage tokens', error); } } @@ -469,6 +506,21 @@ export class AuthService { } async refreshToken(serverUrl: string): Promise { + // Prevent concurrent refresh attempts - reuse in-flight refresh + if (this.refreshPromise) { + console.log('[Desktop AuthService] Refresh already in progress, awaiting existing refresh'); + return this.refreshPromise; + } + + this.refreshPromise = this._doRefreshToken(serverUrl); + try { + return await this.refreshPromise; + } finally { + this.refreshPromise = null; + } + } + + private async _doRefreshToken(serverUrl: string): Promise { try { console.log('[Desktop AuthService] Refreshing auth token'); this.setAuthStatus('refreshing', this.userInfo); @@ -511,6 +563,68 @@ export class AuthService { } } + async refreshSupabaseToken(authServerUrl: string): Promise { + // Prevent concurrent refresh attempts - reuse in-flight refresh + if (this.refreshPromise) { + console.log('[Desktop AuthService] Refresh already in progress, awaiting existing refresh'); + return this.refreshPromise; + } + + this.refreshPromise = this._doRefreshSupabaseToken(authServerUrl); + try { + return await this.refreshPromise; + } finally { + this.refreshPromise = null; + } + } + + private async _doRefreshSupabaseToken(authServerUrl: string): Promise { + try { + console.log('[Desktop AuthService] Refreshing Supabase token'); + this.setAuthStatus('refreshing', this.userInfo); + + const refreshToken = await this.getRefreshToken(); + if (!refreshToken) { + console.error('[Desktop AuthService] No refresh token available'); + this.setAuthStatus('unauthenticated', null); + return false; + } + + // Call Supabase refresh endpoint + const response = await axios.post( + `${authServerUrl}/auth/v1/token?grant_type=refresh_token`, + { + refresh_token: refreshToken, + }, + { + headers: { + 'apikey': SUPABASE_KEY, + 'Content-Type': 'application/json', + }, + } + ); + + const { access_token, refresh_token: newRefreshToken } = response.data; + + // Save new tokens + await this.saveTokenEverywhere(access_token, newRefreshToken); + + const userInfo = await this.getUserInfo(); + this.setAuthStatus('authenticated', userInfo); + + console.log('[Desktop AuthService] Supabase token refreshed successfully'); + return true; + } catch (error) { + console.error('[Desktop AuthService] Supabase token refresh failed:', error); + this.setAuthStatus('unauthenticated', null); + + // Clear stored credentials on refresh failure + await this.logout(); + + return false; + } + } + async initializeAuthState(): Promise { console.log('[Desktop AuthService] Initializing auth state...'); // If we are on the login/setup screen, don't auto-restore a previous session; clear instead @@ -532,11 +646,7 @@ export class AuthService { const userInfo = await this.getUserInfo(); if (token && userInfo) { - console.log('[Desktop AuthService] Found token, syncing to all storage locations'); - - // Ensure token is in both Tauri store and localStorage - await this.saveTokenEverywhere(token); - + console.log('[Desktop AuthService] Found existing token and user info'); this.setAuthStatus('authenticated', userInfo); console.log('[Desktop AuthService] Auth state initialized as authenticated'); } else { @@ -576,9 +686,12 @@ export class AuthService { }); console.log('[Desktop AuthService] OAuth authentication successful, storing tokens'); + console.log('[Desktop AuthService] OAuth result - has access_token:', !!result.access_token); + console.log('[Desktop AuthService] OAuth result - has refresh_token:', !!result.refresh_token); + console.log('[Desktop AuthService] OAuth result - expires_in:', result.expires_in); - // Save token to all storage locations - await this.saveTokenEverywhere(result.access_token); + // Save token and refresh token to all storage locations + await this.saveTokenEverywhere(result.access_token, result.refresh_token); // Fetch user info from Supabase using the access token const userInfo = await this.fetchSupabaseUserInfo(authServerUrl, result.access_token);