diff --git a/package-lock.json b/package-lock.json index c54d8bd..fe2e7ac 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "donetick", - "version": "1.2.29", + "version": "1.2.33", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "donetick", - "version": "1.2.29", + "version": "1.2.33", "hasInstallScript": true, "dependencies": { "@capacitor-community/speech-recognition": "^7.0.1", diff --git a/src/CapacitorListener.js b/src/CapacitorListener.js index 7ab69b3..e21d97d 100644 --- a/src/CapacitorListener.js +++ b/src/CapacitorListener.js @@ -7,6 +7,7 @@ import { Preferences } from '@capacitor/preferences' import { PushNotifications } from '@capacitor/push-notifications' import { focusManager } from '@tanstack/react-query' import { RegisterDeviceToken } from './utils/Fetcher' +import { beginOAuthExchange } from './utils/OAuthExchangeState' // React Router navigate(), injected by once the router is mounted. // Using client-side navigation (instead of window.location.href) avoids a full @@ -104,6 +105,13 @@ const handleOAuthDeepLink = async url => { return } + // Claim the exchange window synchronously, before the first await: the + // resume-driven background sync fires in the same tick as this deep link, + // and its 401 must not be mistaken for an expired session. Set after the + // early return above so the flag is only ever claimed by the navigation + // that Authenticating.jsx will clear. + beginOAuthExchange() + // Store the OAuth params for the app to pick up await Preferences.set({ key: 'oauth_callback', diff --git a/src/hooks/useSyncOnReconnect.js b/src/hooks/useSyncOnReconnect.js index 0c3220e..0293264 100644 --- a/src/hooks/useSyncOnReconnect.js +++ b/src/hooks/useSyncOnReconnect.js @@ -4,6 +4,7 @@ import { useQueryClient } from '@tanstack/react-query' import { useEffect, useRef } from 'react' import { commandQueue } from '../utils/CommandQueue' import { offlineDB } from '../utils/OfflineDB' +import { isOAuthExchangeInProgress } from '../utils/OAuthExchangeState' import { isOfflineFeatureEnabled } from '../utils/OfflineFeatureToggle' import { syncEngine } from '../utils/SyncEngine' import { networkManager } from './NetworkManager' @@ -90,6 +91,11 @@ export function useSyncOnReconnect() { const runSync = async () => { if (!isOfflineFeatureEnabled()) return + // Skip while the OAuth code exchange is in flight — there's no session + // yet, so a sync here just 401s. Note the app-resume listener fires in + // the same tick as the deep link, before the route changes, so this has + // to test the shared flag rather than the pathname. + if (isOAuthExchangeInProgress()) return const wasOffline = !networkManager.isOnline const didSync = await syncEngine.sync() if (didSync) { diff --git a/src/utils/ApiClient.js b/src/utils/ApiClient.js index 8a9ff70..25542d2 100644 --- a/src/utils/ApiClient.js +++ b/src/utils/ApiClient.js @@ -2,6 +2,7 @@ import { Preferences } from '@capacitor/preferences' import { API_URL } from '../Config' import { networkManager } from '../hooks/NetworkManager' import { logout, RefreshToken } from './Fetcher' +import { isOAuthExchangeInProgress } from './OAuthExchangeState' import { offlineDB } from './OfflineDB' import { clearAllTokens, @@ -9,6 +10,8 @@ import { saveTokens, } from './TokenStorage' +const OAUTH_EXCHANGE_IN_PROGRESS = 'OAuth exchange in progress' + class ApiClient { constructor() { this.customServerURL = `${API_URL}/api/v1` @@ -46,6 +49,13 @@ class ApiClient { } async refreshToken() { + // No session exists yet while an OAuth code exchange is in flight, so there + // is nothing to refresh. Callers must treat this as a non-fatal failure + // (see request()) rather than an expired session. + if (isOAuthExchangeInProgress()) { + return { success: false, error: OAUTH_EXCHANGE_IN_PROGRESS } + } + // Check if refresh token is expired BEFORE attempting refresh const refreshExpired = await isRefreshTokenExpired() if (refreshExpired) { @@ -132,6 +142,14 @@ class ApiClient { // Helper to avoid repeating cleanup code async handleLogout() { + // Backstop for every forced-logout path: never tear down the session while + // an OAuth exchange is running, or we clear the tokens it just saved and + // reload the page out from under it. + if (isOAuthExchangeInProgress()) { + console.log('Skipping forced logout: OAuth exchange in progress') + return + } + await clearAllTokens() try { await offlineDB.clearAll() @@ -230,6 +248,13 @@ class ApiClient { this.handleLogout() return null } + } else if (refreshResult.error === OAUTH_EXCHANGE_IN_PROGRESS) { + // Expected 401: the code exchange hasn't produced tokens yet. Fail + // just this request — logging out here would wipe storage and hard + // navigate to /login, aborting the exchange fetch mid-flight. + queuedPromise.catch(() => {}) // not returned below; keep it handled + this.processQueue(new Error(refreshResult.error), null) + return response } else if (refreshResult.error === 'Already refreshing') { // This shouldn't happen since we check isRefreshing above, but handle it anyway console.log('Already refreshing - waiting for refresh to complete') diff --git a/src/utils/OAuthExchangeState.js b/src/utils/OAuthExchangeState.js new file mode 100644 index 0000000..569bf15 --- /dev/null +++ b/src/utils/OAuthExchangeState.js @@ -0,0 +1,24 @@ +// Tracks whether an OAuth authorization-code exchange is currently in flight. +// +// During that window the app is legitimately unauthenticated: the deep link has +// arrived but Authenticating.jsx has not received tokens yet. Any 401 from an +// unrelated request (background sync, a resumed query) must NOT be treated as an +// expired session — the forced logout it triggers clears storage and does a hard +// `window.location.href = '/login'`, which tears down the page and aborts the +// in-flight code exchange. +let exchangeInProgress = false + +export const beginOAuthExchange = () => { + exchangeInProgress = true +} + +export const endOAuthExchange = () => { + exchangeInProgress = false +} + +// The in-memory flag covers the native deep-link path, where the callback +// arrives while the app is already running. On web the provider redirects with a +// full page load, so nothing has run to set the flag — the pathname check covers +// that case (and doubles as a backstop if the flag is never cleared). +export const isOAuthExchangeInProgress = () => + exchangeInProgress || window.location.pathname === '/auth/oauth2' diff --git a/src/views/Authorization/Authenticating.jsx b/src/views/Authorization/Authenticating.jsx index 4420b35..b7d53a1 100644 --- a/src/views/Authorization/Authenticating.jsx +++ b/src/views/Authorization/Authenticating.jsx @@ -8,6 +8,7 @@ import { useRef } from 'react' import { Link, useNavigate, useParams } from 'react-router-dom' import { useUserProfile } from '../../queries/UserQueries' import { apiClient } from '../../utils/ApiClient' +import { endOAuthExchange } from '../../utils/OAuthExchangeState' import { GetUserProfile } from '../../utils/Fetcher' import { saveTokens } from '../../utils/TokenStorage' import MFAVerificationModal from './MFAVerificationModal' @@ -25,11 +26,14 @@ const AuthenticationLoading = () => { useEffect(() => { if (provider === 'oauth2' && !hasCalledHandleOAuth2.current) { hasCalledHandleOAuth2.current = true - handleOAuth2() + // Release the guard once the exchange settles, so a stuck flag can never + // suppress a genuine session expiry later on. + handleOAuth2().finally(endOAuthExchange) } else if (provider !== 'oauth2') { setMessage('Unknown Authentication Provider') setSubMessage('Please contact support') } + return endOAuthExchange }, [provider]) const getUserProfileAndNavigateToHome = () => { GetUserProfile().then(data => {