From 050e2f0b1d2643dcf7bb119e3f9b47bd545c771b Mon Sep 17 00:00:00 2001 From: feruzm Date: Thu, 17 Sep 2026 09:22:54 +0000 Subject: [PATCH 1/5] fix(push): keep the device push registration fresh - Register again when FCM rotates the token (onTokenRefresh), instead of waiting for the next cold start. - Register the account that becomes current, which covers every login method and account switches. The key login call never ran (it read a plain accessToken the login result does not carry), and HiveSigner, HiveAuth and QR logins had no call at all. - Re-register on returning to the foreground, at most once a day. - Decrypt the stored access token with the app PIN from redux and fall back to DEFAULT_PIN, await the request, and report HTTP failures to Sentry under one fingerprint. Offline failures only log, and FIS_AUTH_ERROR is treated like the other "no token on this device" errors. - Logout disables the account's row with its own token, not only when it is the current account. Logging out the last account, forgot PIN and clear data disable every account and then delete the FCM token, so the backend gets "unregistered" for it and stops sending. --- .../container/applicationContainer.tsx | 164 ++++++++++---- .../login/container/loginContainer.tsx | 75 +------ .../pinCode/container/pinCodeContainer.tsx | 9 +- .../settings/container/settingsContainer.tsx | 9 +- src/utils/pushRegistration.test.ts | 206 ++++++++++++++++++ src/utils/pushRegistration.ts | 120 ++++++++++ 6 files changed, 467 insertions(+), 116 deletions(-) create mode 100644 src/utils/pushRegistration.test.ts create mode 100644 src/utils/pushRegistration.ts diff --git a/src/screens/application/container/applicationContainer.tsx b/src/screens/application/container/applicationContainer.tsx index 2ae6701685..0e88b09362 100644 --- a/src/screens/application/container/applicationContainer.tsx +++ b/src/screens/application/container/applicationContainer.tsx @@ -90,6 +90,13 @@ import { decryptKey, encryptKey } from '../../../utils/crypto'; import parseVersionNumber from '../../../utils/parseVersionNumber'; import { setMomentLocale } from '../../../utils/time'; import { purgeExpiredCache } from '../../../redux/actions/cacheActions'; +import { + PUSH_REGISTRATION_REFRESH_MS, + decryptAccessToken, + disablePushRegistrations, + getPushSystem, +} from '../../../utils/pushRegistration'; +import { captureException, captureMessage } from '../../../utils/sentryUtils'; import { fetchSubscribedCommunities } from '../../../redux/actions/communitiesAction'; import MigrationHelpers, { repairOtherAccountsData, @@ -118,6 +125,7 @@ import { } from '../../../redux/selectors'; let firebaseOnMessageListener: any = null; +let firebaseTokenRefreshListener: (() => void) | null = null; let appStateSub: NativeEventSubscription | null = null; class ApplicationContainer extends Component { @@ -135,6 +143,8 @@ class ApplicationContainer extends Component { _fcmAvailable: boolean | null = null; // Cache FCM availability check + _lastPushRegistration = 0; + constructor(props: any) { super(props); this.state = { @@ -159,6 +169,7 @@ class ApplicationContainer extends Component { } else { console.log('FCM not available - will use WebSocket fallback when user logs in'); } + this._createTokenRefreshListener(); // set avatar cache stamp to invalidate previous session avatars dispatch(setAvatarCacheStamp(new Date().getTime())); @@ -197,7 +208,13 @@ class ApplicationContainer extends Component { }; componentDidUpdate(prevProps: any) { - const { isGlobalRenderRequired, dispatch } = this.props; + const { isGlobalRenderRequired, dispatch, currentAccount } = this.props; + + // A login (by any method) or an account switch makes a different account current. + // Register it right away instead of waiting for the next cold start. + if (currentAccount?.name && currentAccount.name !== prevProps.currentAccount?.name) { + this._registerAccountForNotifications(currentAccount); + } if (isGlobalRenderRequired !== prevProps.isGlobalRenderRequired && isGlobalRenderRequired) { this.setState( @@ -230,6 +247,11 @@ class ApplicationContainer extends Component { firebaseOnMessageListener(); } + if (firebaseTokenRefreshListener) { + firebaseTokenRefreshListener(); + firebaseTokenRefreshListener = null; + } + this._disconnectNotificationServer(); this.netListener(); @@ -326,6 +348,9 @@ class ApplicationContainer extends Component { this._refreshGlobalProps(); this._refreshUnreadActivityCount(); this._refreshUnreadChats(); + if (Date.now() - this._lastPushRegistration > PUSH_REGISTRATION_REFRESH_MS) { + this._registerDeviceForNotifications(); + } // Refresh account data to get latest profile updates from blockchain if (currentAccount?.local) { this._fetchUserDataFromDsteem(currentAccount.local); @@ -403,6 +428,21 @@ class ApplicationContainer extends Component { }); }; + // FCM rotates tokens (app data restore, reinstall over backup, periodic refresh). + // The old token stops working, so register the new one for every account now. + _createTokenRefreshListener = () => { + if (firebaseTokenRefreshListener) { + return; + } + try { + firebaseTokenRefreshListener = getMessaging().onTokenRefresh(() => { + this._registerDeviceForNotifications(); + }); + } catch (error) { + console.warn('Failed to listen for push token refresh', error); + } + }; + _handleConntectionChange = (status: any) => { const { dispatch, isConnected } = this.props; @@ -717,31 +757,16 @@ class ApplicationContainer extends Component { } }; - // update notification settings and update push token for each signed accoutn useing access tokens - _registerDeviceForNotifications = (settings?: any) => { - const { currentAccount, otherAccounts, notificationDetails, isNotificationsEnabled } = - this.props; - - const isEnabled = settings ? !!settings.notification : isNotificationsEnabled; - settings = settings || notificationDetails; - - const _enabledNotificationForAccount = (account: any) => { - const encAccessToken = account?.local?.accessToken; - // otherAccounts entries are keyed by username; name can be undefined on some - // (e.g. HiveSigner) entries, so fall back to username. - this._enableNotification( - account.name || account.username, - isEnabled, - settings, - encAccessToken, - ); - }; + // Register the push token with each signed-in account's notification settings. + _registerDeviceForNotifications = () => { + const { currentAccount, otherAccounts } = this.props; + + this._lastPushRegistration = Date.now(); - // updateing fcm token with settings; otherAccounts.forEach((account: any) => { // since there can be more than one accounts, process access tokens separate if (account?.local?.accessToken) { - _enabledNotificationForAccount(account); + this._registerAccountForNotifications(account); return; } @@ -751,11 +776,29 @@ class ApplicationContainer extends Component { const acctName = account?.name || account?.username; if (acctName && currentAccount?.name === acctName) { // fallback to current account access token to register at least the logged-in account - _enabledNotificationForAccount(currentAccount); + this._registerAccountForNotifications(currentAccount); } }); }; + _registerAccountForNotifications = (account: any) => { + const { notificationDetails, isNotificationsEnabled } = this.props; + const encAccessToken = account?.local?.accessToken; + + if (!encAccessToken) { + return; + } + + // otherAccounts entries are keyed by username; name can be undefined on some + // (e.g. HiveSigner) entries, so fall back to username. + this._enableNotification( + account.name || account.username, + isNotificationsEnabled, + notificationDetails, + encAccessToken, + ); + }; + /** * Check if FCM (Firebase Cloud Messaging) is available on this device * Returns cached result if already checked @@ -1052,7 +1095,14 @@ class ApplicationContainer extends Component { }; _logout = async (username: any) => { - const { currentAccount, otherAccounts, dispatch, intl } = this.props; + const { currentAccount, otherAccounts, dispatch, intl, pinCode } = this.props; + + // Read the token before the account data is removed below. + const loggedOutAccount = + currentAccount?.name === username + ? currentAccount + : otherAccounts.find((user: any) => (user.username || user.name) === username); + const accessToken = decryptAccessToken(loggedOutAccount?.local?.accessToken, pinCode); try { const response = await removeUserData(username); @@ -1062,13 +1112,15 @@ class ApplicationContainer extends Component { } this._updatePrevLoggedInUsersList(username); - const encAccessToken = - currentAccount.name === username ? currentAccount?.local?.accessToken : null; - this._enableNotification(username, false, null, encAccessToken); - // switch account if other account exist const _otherAccounts = otherAccounts.filter((user: any) => user.username !== username); + // Stop pushes for the logged-out account. With no account left, also delete the + // device token so nothing registered under it is delivered any more. + disablePushRegistrations([{ username, accessToken }], { + deleteToken: _otherAccounts.length === 0, + }); + if (_otherAccounts.length > 0) { const targetAccount = _otherAccounts[0]; await this._switchAccount(targetAccount); @@ -1109,12 +1161,23 @@ class ApplicationContainer extends Component { }; _enableNotification = async ( - username: any, - isEnable: any, - settings = null, - encAccesstoken = null, + username: string, + isEnable: boolean, + settings: any, + encAccesstoken: string, ) => { - const accessToken = encAccesstoken ? decryptKey(encAccesstoken, Config.DEFAULT_PIN) : null; + const { pinCode } = this.props; + const accessToken = decryptAccessToken(encAccesstoken, pinCode); + + if (!accessToken) { + // The request would be rejected without it. A stored token that does not decrypt + // means the PIN state and the stored keys disagree, which is worth knowing about. + captureMessage('Push registration skipped: stored access token did not decrypt', (scope) => { + scope.setTag('context', 'push-registration'); + scope.setFingerprint(['push-registration-decrypt']); + }); + return; + } // compile notify_types let notify_types: any[] = []; @@ -1170,14 +1233,28 @@ class ApplicationContainer extends Component { const token = await getMessaging().getToken(); console.log('FCM Token obtained:', !!token); - saveNotificationSetting( - accessToken!, - username, - `fcm-${Platform.OS}`, - Number(isEnable), - notify_types, - token, - ); + try { + await saveNotificationSetting( + accessToken, + username, + getPushSystem(), + Number(isEnable), + notify_types, + token, + ); + } catch (error) { + const status = (error as any)?.status; + if (typeof status !== 'number') { + // Offline or timed out: the next start, reconnect or daily refresh retries. + console.warn('Push registration request failed', error); + return; + } + captureException(error, (scope) => { + scope.setTag('context', 'push-registration'); + scope.setTag('status', String(status)); + scope.setFingerprint(['push-registration-http', String(status)]); + }); + } } catch (error) { // Handle platform-specific FCM errors gracefully const errorMessage = (error as any).message || ''; @@ -1190,9 +1267,12 @@ class ApplicationContainer extends Component { Platform.OS === 'android' && (errorMessage.includes('MISSING_INSTANCEID_SERVICE') || errorMessage.includes('SERVICE_NOT_AVAILABLE') || - errorMessage.includes('AUTHENTICATION_FAILED')) + errorMessage.includes('AUTHENTICATION_FAILED') || + errorMessage.includes('FIS_AUTH_ERROR')) ) { // Android: Google Play Services issues (common on emulators, custom ROMs, outdated devices) + // FIS_AUTH_ERROR is Firebase Installations failing to authenticate the device + // (ECENCY-MOBILE-28Y); the app cannot fix it either. console.log( 'Google Play Services not available or misconfigured - FCM disabled for this device', ); diff --git a/src/screens/login/container/loginContainer.tsx b/src/screens/login/container/loginContainer.tsx index 19edd28eac..ab8a8c91bb 100644 --- a/src/screens/login/container/loginContainer.tsx +++ b/src/screens/login/container/loginContainer.tsx @@ -1,15 +1,14 @@ import React, { PureComponent } from 'react'; -import { Alert, Platform } from 'react-native'; +import { Alert } from 'react-native'; import { connect } from 'react-redux'; import { injectIntl } from 'react-intl'; import Config from 'react-native-config'; -import { getMessaging } from '@react-native-firebase/messaging'; // Services and Actions import { useNavigation } from '@react-navigation/native'; import { gestureHandlerRootHOC } from 'react-native-gesture-handler'; import { SheetManager } from 'react-native-actions-sheet'; -import { getAccountsQueryOptions, saveNotificationSetting } from '@ecency/sdk'; +import { getAccountsQueryOptions } from '@ecency/sdk'; import { captureException } from '../../../utils/sentryUtils'; import { getQueryClient } from '../../../providers/queries'; import { login, loginWithSC2 } from '../../../providers/hive/auth'; @@ -22,7 +21,7 @@ import { } from '../../../redux/actions/accountAction'; import { login as loginAction, setPinCode } from '../../../redux/actions/applicationActions'; import { setInitPosts, setFeedPosts } from '../../../redux/actions/postsAction'; -import { setPushTokenSaved, setExistUser } from '../../../storage/storage'; +import { setExistUser } from '../../../storage/storage'; import { decodeBase64, encryptKey } from '../../../utils/crypto'; // Middleware @@ -44,8 +43,6 @@ import { selectIsPinCodeOpen, selectIsConnected, selectPrevLoggedInUsers, - selectNotificationDetails, - selectIsNotificationOpen, } from '../../../redux/selectors'; /* @@ -200,7 +197,6 @@ class LoginContainer extends PureComponent { // track user activity for login userActivityMutation.mutate({ pointsTy: PointActivityIds.LOGIN }); setExistUser(true); - this._setPushToken(result.name, result.accessToken); const encryptedPin = encryptKey(Config.DEFAULT_PIN!, Config.PIN_KEY!); dispatch(setPinCode(encryptedPin)); @@ -241,69 +237,6 @@ class LoginContainer extends PureComponent { }); }; - _setPushToken = async (username: any, accessToken?: string) => { - const { notificationSettings, notificationDetails } = this.props; - const notifyTypesConst = { - vote: 1, - mention: 2, - follow: 3, - comment: 4, - reblog: 5, - transfers: 6, - favorite: 13, - bookmark: 15, - tags: 23, - delegations: 10, - payouts: 19, - accountUpdate: 20, - weeklyEarnings: 21, - scheduledPublished: 22, - }; - const notifyTypes: any[] = []; - - Object.keys(notificationDetails).forEach((item) => { - const notificationType = item.replace('Notification', ''); - const notifyType = (notifyTypesConst as any)[notificationType]; - - // Only a mapped type: a settings key this map does not know would otherwise - // register as null and the device would be told nothing useful about it. - if (notificationDetails[item] && notifyType) { - notifyTypes.push(notifyType); - } - }); - - if (!accessToken) { - console.warn('Missing access token for notifications:', username); - return; - } - - getMessaging() - .getToken() - .then((token) => { - const data = { - username, - token, - system: `fcm-${Platform.OS}`, - allows_notify: Number(notificationSettings), - notify_types: notifyTypes, - }; - return saveNotificationSetting( - accessToken, - data.username, - data.system, - data.allows_notify, - data.notify_types, - data.token, - ); - }) - .then(() => { - setPushTokenSaved(true); - }) - .catch((err) => { - console.warn('Failed to register push token', err); - }); - }; - _getAccountsWithUsername = async (username: any) => { const { isConnected } = this.props; @@ -348,8 +281,6 @@ class LoginContainer extends PureComponent { const mapStateToProps = (state: any) => ({ account: state.accounts, - notificationDetails: selectNotificationDetails(state), - notificationSettings: selectIsNotificationOpen(state), isConnected: selectIsConnected(state), isPinCodeOpen: selectIsPinCodeOpen(state), prevLoggedInUsers: selectPrevLoggedInUsers(state), diff --git a/src/screens/pinCode/container/pinCodeContainer.tsx b/src/screens/pinCode/container/pinCodeContainer.tsx index 3c8726959e..e0f3e3bee1 100644 --- a/src/screens/pinCode/container/pinCodeContainer.tsx +++ b/src/screens/pinCode/container/pinCodeContainer.tsx @@ -34,6 +34,7 @@ import { // Utils import { encryptKey, decryptKey } from '../../../utils/crypto'; import MigrationHelpers from '../../../utils/migrationHelpers'; +import { disablePushRegistrations, getPushAccounts } from '../../../utils/pushRegistration'; // Component import PinCodeView from '../children/pinCodeView'; @@ -306,7 +307,13 @@ class PinCodeContainer extends Component { }; _forgotPinCode = async () => { - const { otherAccounts, dispatch } = this.props; + const { otherAccounts, currentAccount, applicationPinCode, dispatch } = this.props; + + // Every account leaves the device: stop their pushes and drop the device token. + // Tokens are read now, before the data below is wiped. + disablePushRegistrations(getPushAccounts(currentAccount, otherAccounts, applicationPinCode), { + deleteToken: true, + }); await removeAllUserData() .then(async () => { diff --git a/src/screens/settings/container/settingsContainer.tsx b/src/screens/settings/container/settingsContainer.tsx index 69b5098848..4ae41d3357 100644 --- a/src/screens/settings/container/settingsContainer.tsx +++ b/src/screens/settings/container/settingsContainer.tsx @@ -102,6 +102,7 @@ import settingsTypes from '../../../constants/settingsTypes'; import { sendEmail } from '../../../utils/sendEmail'; import { encryptKey, decryptKey } from '../../../utils/crypto'; import { openStoreListing } from '../../../utils/storeReview'; +import { disablePushRegistrations, getPushAccounts } from '../../../utils/pushRegistration'; // Component import SettingsScreen from '../screen/settingsScreen'; @@ -798,7 +799,13 @@ class SettingsContainer extends Component { }; _clearUserData = async () => { - const { otherAccounts, dispatch } = this.props; + const { otherAccounts, currentAccount, pinCode, dispatch } = this.props; + + // Every account leaves the device: stop their pushes and drop the device token. + // Tokens are read now, before the data below is wiped. + disablePushRegistrations(getPushAccounts(currentAccount, otherAccounts, pinCode), { + deleteToken: true, + }); await removeAllUserData() .then(async () => { diff --git a/src/utils/pushRegistration.test.ts b/src/utils/pushRegistration.test.ts new file mode 100644 index 0000000000..4a81763992 --- /dev/null +++ b/src/utils/pushRegistration.test.ts @@ -0,0 +1,206 @@ +// Real crypto, so the PIN fallbacks are exercised against actual ciphertext. +import { getMessaging } from '@react-native-firebase/messaging'; +import { saveNotificationSetting } from '@ecency/sdk'; +import { encryptKey } from './crypto'; +import { + decryptAccessToken, + disablePushRegistrations, + getPushAccounts, + getPushSystem, +} from './pushRegistration'; + +jest.unmock('crypto-js'); + +jest.mock('react-native-config', () => ({ DEFAULT_PIN: 'default-pin', PIN_KEY: 'pin-key' })); + +jest.mock('@ecency/sdk', () => ({ saveNotificationSetting: jest.fn() })); + +const mockMessaging = { + getToken: jest.fn(), + deleteToken: jest.fn(), +}; +jest.mock('@react-native-firebase/messaging', () => ({ + getMessaging: jest.fn(() => mockMessaging), +})); + +const saveMock = saveNotificationSetting as jest.Mock; + +const DEFAULT_PIN = 'default-pin'; +const USER_PIN = '112233'; +const appPin = (pin: string) => encryptKey(pin, 'pin-key'); + +beforeEach(() => { + jest.clearAllMocks(); + (getMessaging as jest.Mock).mockImplementation(() => mockMessaging); + mockMessaging.getToken.mockResolvedValue('fcm-token'); + mockMessaging.deleteToken.mockResolvedValue(undefined); + saveMock.mockResolvedValue({}); +}); + +describe('decryptAccessToken', () => { + it('returns undefined when there is no stored token', () => { + expect(decryptAccessToken(undefined, appPin(DEFAULT_PIN))).toBeUndefined(); + expect(decryptAccessToken('', appPin(DEFAULT_PIN))).toBeUndefined(); + }); + + it('decrypts a token stored under DEFAULT_PIN', () => { + const stored = encryptKey('code-1', DEFAULT_PIN); + expect(decryptAccessToken(stored, appPin(DEFAULT_PIN))).toBe('code-1'); + }); + + it("decrypts a token still stored under the user's own PIN", () => { + // An account not unlocked since the DEFAULT_PIN migration. Decrypting with + // DEFAULT_PIN alone, as registration used to, fails here. + const stored = encryptKey('code-2', USER_PIN); + expect(decryptAccessToken(stored, appPin(USER_PIN))).toBe('code-2'); + }); + + it('falls back to DEFAULT_PIN when the app PIN does not match the token', () => { + // A fresh login stores the token under DEFAULT_PIN before the app PIN is rewritten. + const stored = encryptKey('code-3', DEFAULT_PIN); + expect(decryptAccessToken(stored, appPin(USER_PIN))).toBe('code-3'); + }); + + it('falls back to DEFAULT_PIN when no app PIN is stored', () => { + const stored = encryptKey('code-4', DEFAULT_PIN); + expect(decryptAccessToken(stored, undefined)).toBe('code-4'); + }); + + it('returns undefined when neither PIN decrypts the token', () => { + const stored = encryptKey('code-5', 'some-other-pin'); + expect(decryptAccessToken(stored, appPin(USER_PIN))).toBeUndefined(); + expect(decryptAccessToken(stored, appPin(DEFAULT_PIN))).toBeUndefined(); + }); +}); + +describe('getPushAccounts', () => { + const account = (name: string, code?: string, keyedBy: 'name' | 'username' = 'username') => ({ + [keyedBy]: name, + local: code ? { accessToken: encryptKey(code, DEFAULT_PIN) } : {}, + }); + + it('lists each account once with its decrypted token', () => { + const current = { name: 'alice', local: { accessToken: encryptKey('alice-new', DEFAULT_PIN) } }; + const others = [account('alice', 'alice-old'), account('bob', 'bob-code')]; + + expect(getPushAccounts(current, others, appPin(DEFAULT_PIN))).toEqual([ + { username: 'alice', accessToken: 'alice-new' }, + { username: 'bob', accessToken: 'bob-code' }, + ]); + }); + + it('adds the current account when it is missing from otherAccounts', () => { + const current = { name: 'carol', local: { accessToken: encryptKey('carol', DEFAULT_PIN) } }; + + expect(getPushAccounts(current, [account('bob', 'bob-code')], appPin(DEFAULT_PIN))).toEqual([ + { username: 'bob', accessToken: 'bob-code' }, + { username: 'carol', accessToken: 'carol' }, + ]); + }); + + it("keeps the other-account token when the current account's entry has none", () => { + const current = { name: 'alice', local: {} }; + + expect(getPushAccounts(current, [account('alice', 'alice-old')], appPin(DEFAULT_PIN))).toEqual([ + { username: 'alice', accessToken: 'alice-old' }, + ]); + }); + + it('accepts entries keyed by name and skips entries with no name', () => { + const others = [account('dave', 'dave-code', 'name'), { local: {} }]; + + expect(getPushAccounts({}, others, appPin(DEFAULT_PIN))).toEqual([ + { username: 'dave', accessToken: 'dave-code' }, + ]); + }); +}); + +describe('disablePushRegistrations', () => { + it('disables every account that has a token, with the current device token', async () => { + await disablePushRegistrations( + [ + { username: 'alice', accessToken: 'alice-code' }, + { username: 'bob', accessToken: undefined }, + { username: 'carol', accessToken: 'carol-code' }, + ], + { deleteToken: false }, + ); + + expect(saveMock).toHaveBeenCalledTimes(2); + expect(saveMock).toHaveBeenCalledWith( + 'alice-code', + 'alice', + getPushSystem(), + 0, + [], + 'fcm-token', + ); + expect(saveMock).toHaveBeenCalledWith( + 'carol-code', + 'carol', + getPushSystem(), + 0, + [], + 'fcm-token', + ); + expect(mockMessaging.deleteToken).not.toHaveBeenCalled(); + }); + + it('deletes the device token only after every request has settled', async () => { + const events: string[] = []; + let finishAlice: () => void = () => undefined; + saveMock.mockImplementation( + (_code: string, username: string) => + new Promise((resolve, reject) => { + if (username === 'alice') { + finishAlice = () => { + events.push('alice settled'); + resolve(); + }; + } else { + events.push('bob settled'); + reject(new Error('Request failed with status 500')); + } + }), + ); + mockMessaging.deleteToken.mockImplementation(async () => { + events.push('token deleted'); + }); + + const done = disablePushRegistrations( + [ + { username: 'alice', accessToken: 'alice-code' }, + { username: 'bob', accessToken: 'bob-code' }, + ], + { deleteToken: true }, + ); + await new Promise((resolve) => setImmediate(resolve)); + expect(mockMessaging.deleteToken).not.toHaveBeenCalled(); + + finishAlice(); + await done; + + // A failed request does not block the others or the token delete. + expect(events).toEqual(['bob settled', 'alice settled', 'token deleted']); + }); + + it('does nothing when the device has no token', async () => { + mockMessaging.getToken.mockRejectedValue(new Error('SERVICE_NOT_AVAILABLE')); + + await expect( + disablePushRegistrations([{ username: 'alice', accessToken: 'alice-code' }], { + deleteToken: true, + }), + ).resolves.toBeUndefined(); + + expect(saveMock).not.toHaveBeenCalled(); + expect(mockMessaging.deleteToken).not.toHaveBeenCalled(); + }); + + it('does not throw when deleting the token fails', async () => { + mockMessaging.deleteToken.mockRejectedValue(new Error('offline')); + + await expect(disablePushRegistrations([], { deleteToken: true })).resolves.toBeUndefined(); + expect(mockMessaging.deleteToken).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/utils/pushRegistration.ts b/src/utils/pushRegistration.ts new file mode 100644 index 0000000000..769032c3d0 --- /dev/null +++ b/src/utils/pushRegistration.ts @@ -0,0 +1,120 @@ +import { Platform } from 'react-native'; +import Config from 'react-native-config'; +import { getMessaging } from '@react-native-firebase/messaging'; +import { saveNotificationSetting } from '@ecency/sdk'; + +import { decryptKey } from './crypto'; + +// Coming back to the foreground re-registers at most this often, so the backend keeps +// seeing a live install even when the app is resumed for days without a cold start. +export const PUSH_REGISTRATION_REFRESH_MS = 24 * 60 * 60 * 1000; + +export const getPushSystem = () => `fcm-${Platform.OS}`; + +/** + * Decrypts a stored access token for a push registration request. + * + * Stored tokens are encrypted with the app PIN kept in redux (`application.pin`, + * itself encrypted with PIN_KEY). That is DEFAULT_PIN once an account is migrated, + * but an account that has not been unlocked since the migration still uses the + * user's own PIN, so decrypting with DEFAULT_PIN alone fails for it. DEFAULT_PIN + * stays as the fallback because a fresh login stores its token under DEFAULT_PIN + * and can reach here before the redux PIN is written. + */ +export const decryptAccessToken = ( + encAccessToken: string | null | undefined, + encAppPin: string | null | undefined, +): string | undefined => { + if (!encAccessToken) { + return undefined; + } + + const appPin = encAppPin ? decryptKey(encAppPin, Config.PIN_KEY) : undefined; + const accessToken = appPin ? decryptKey(encAccessToken, appPin) : undefined; + if (accessToken || appPin === Config.DEFAULT_PIN) { + return accessToken; + } + + return decryptKey(encAccessToken, Config.DEFAULT_PIN); +}; + +export interface PushAccount { + username: string; + accessToken?: string; +} + +/** + * Every account signed in on this device, with its decrypted access token. The + * current account is normally also in `otherAccounts`; its own entry is used when it + * carries a token (its `local` data is the freshest), and added if it was missing. + */ +export const getPushAccounts = ( + currentAccount: any, + otherAccounts: any[] = [], + encAppPin: string | null | undefined, +): PushAccount[] => { + const accounts = new Map(); + otherAccounts.forEach((account) => { + const username = account?.username || account?.name; + if (username) { + accounts.set(username, account); + } + }); + if (currentAccount?.name && currentAccount.local?.accessToken) { + accounts.set(currentAccount.name, currentAccount); + } + + return Array.from(accounts, ([username, account]) => ({ + username, + accessToken: decryptAccessToken(account?.local?.accessToken, encAppPin), + })); +}; + +/** + * Turns push off for accounts leaving this device. + * + * Each account's row is disabled with the current FCM token. When no account is left + * on the device, the token itself is deleted afterwards: the backend then gets + * "unregistered" for it and stops sending, even for a row this call could not + * disable (for example an account whose access token no longer decrypts). The + * delete must come after the requests, because deleting first makes the next + * `getToken()` mint a new token and the requests would disable the wrong rows. + */ +export const disablePushRegistrations = async ( + accounts: PushAccount[], + { deleteToken }: { deleteToken: boolean }, +) => { + let token: string; + try { + token = await getMessaging().getToken(); + } catch (err) { + // No token on this device (no Play Services, simulator, offline): nothing to disable. + console.warn('Push token unavailable, skipping push deregistration', err); + return; + } + + await Promise.all( + accounts + .filter((account) => account.username && account.accessToken) + .map((account) => + saveNotificationSetting( + account.accessToken, + account.username, + getPushSystem(), + 0, + [], + token, + ).catch((err) => { + console.warn('Failed to disable push notifications for', account.username, err); + }), + ), + ); + + if (deleteToken) { + try { + await getMessaging().deleteToken(); + } catch (err) { + console.warn('Failed to delete push token', err); + } + } +}; From 0cd40f0124fe217cc9a79313a8fbe7d7f56e2b64 Mon Sep 17 00:00:00 2001 From: feruzm Date: Thu, 17 Sep 2026 09:46:44 +0000 Subject: [PATCH 2/5] fix(push): wait for a pending deregistration before registering - Deregistrations run one after another, and a registration waits for the one in progress (up to 30s) before reading the FCM token. A login right after the last logout no longer registers the token that is about to be deleted, and logout still does not wait on the network. - Token refresh and cold start register a deduplicated account list that includes the current account even when otherAccounts does not list it. - Wrap an overlong Sentry call. --- .../container/applicationContainer.tsx | 27 ++--- src/utils/pushRegistration.test.ts | 103 ++++++++++++++++++ src/utils/pushRegistration.ts | 89 +++++++++++---- 3 files changed, 179 insertions(+), 40 deletions(-) diff --git a/src/screens/application/container/applicationContainer.tsx b/src/screens/application/container/applicationContainer.tsx index 0e88b09362..375cdf13b8 100644 --- a/src/screens/application/container/applicationContainer.tsx +++ b/src/screens/application/container/applicationContainer.tsx @@ -95,6 +95,8 @@ import { decryptAccessToken, disablePushRegistrations, getPushSystem, + getSignedInAccounts, + waitForPushRelease, } from '../../../utils/pushRegistration'; import { captureException, captureMessage } from '../../../utils/sentryUtils'; import { fetchSubscribedCommunities } from '../../../redux/actions/communitiesAction'; @@ -763,21 +765,11 @@ class ApplicationContainer extends Component { this._lastPushRegistration = Date.now(); - otherAccounts.forEach((account: any) => { - // since there can be more than one accounts, process access tokens separate - if (account?.local?.accessToken) { - this._registerAccountForNotifications(account); - return; - } - - // No stored access token on this other-account entry. This is common and benign - // (HiveSigner accounts, or entries keyed only by username), so do NOT report it to - // Sentry - it previously fired an error on every launch (ECENCY-MOBILE-1QY). - const acctName = account?.name || account?.username; - if (acctName && currentAccount?.name === acctName) { - // fallback to current account access token to register at least the logged-in account - this._registerAccountForNotifications(currentAccount); - } + // Accounts without a stored access token (HiveSigner accounts, entries keyed only by + // username) are skipped without a report: reporting them fired an error on every + // launch (ECENCY-MOBILE-1QY). + getSignedInAccounts(currentAccount, otherAccounts).forEach(({ account }) => { + this._registerAccountForNotifications(account); }); }; @@ -1172,7 +1164,8 @@ class ApplicationContainer extends Component { if (!accessToken) { // The request would be rejected without it. A stored token that does not decrypt // means the PIN state and the stored keys disagree, which is worth knowing about. - captureMessage('Push registration skipped: stored access token did not decrypt', (scope) => { + const message = 'Push registration skipped: stored access token did not decrypt'; + captureMessage(message, (scope) => { scope.setTag('context', 'push-registration'); scope.setFingerprint(['push-registration-decrypt']); }); @@ -1231,6 +1224,8 @@ class ApplicationContainer extends Component { return; } + // A logout may still be disabling rows and deleting this token; read it after that. + await waitForPushRelease(); const token = await getMessaging().getToken(); console.log('FCM Token obtained:', !!token); try { diff --git a/src/utils/pushRegistration.test.ts b/src/utils/pushRegistration.test.ts index 4a81763992..08c86900ce 100644 --- a/src/utils/pushRegistration.test.ts +++ b/src/utils/pushRegistration.test.ts @@ -7,6 +7,8 @@ import { disablePushRegistrations, getPushAccounts, getPushSystem, + getSignedInAccounts, + waitForPushRelease, } from './pushRegistration'; jest.unmock('crypto-js'); @@ -115,6 +117,27 @@ describe('getPushAccounts', () => { }); }); +describe('getSignedInAccounts', () => { + it('includes the current account when otherAccounts does not list it', () => { + const current = { name: 'alice', local: { accessToken: 'enc-alice' } }; + const bob = { username: 'bob', local: { accessToken: 'enc-bob' } }; + + expect(getSignedInAccounts(current, [bob])).toEqual([ + { username: 'bob', account: bob }, + { username: 'alice', account: current }, + ]); + }); + + it('lists an account once when it is both current and in otherAccounts', () => { + const current = { name: 'alice', local: { accessToken: 'enc-new' } }; + const stored = { username: 'alice', local: { accessToken: 'enc-old' } }; + + expect(getSignedInAccounts(current, [stored])).toEqual([ + { username: 'alice', account: current }, + ]); + }); +}); + describe('disablePushRegistrations', () => { it('disables every account that has a token, with the current device token', async () => { await disablePushRegistrations( @@ -204,3 +227,83 @@ describe('disablePushRegistrations', () => { expect(mockMessaging.deleteToken).toHaveBeenCalledTimes(1); }); }); + +describe('waitForPushRelease', () => { + const deferred = () => { + let resolve: () => void = () => undefined; + const promise = new Promise((r) => { + resolve = r; + }); + return { promise, resolve }; + }; + const settle = () => new Promise((resolve) => setImmediate(resolve)); + + it('resolves at once when nothing is being released', async () => { + await expect(waitForPushRelease(10_000)).resolves.toBeUndefined(); + }); + + it('waits until a pending release has deleted the token', async () => { + const request = deferred(); + saveMock.mockImplementation(() => request.promise); + const events: string[] = []; + mockMessaging.deleteToken.mockImplementation(async () => { + events.push('token deleted'); + }); + + const release = disablePushRegistrations([{ username: 'alice', accessToken: 'code' }], { + deleteToken: true, + }); + const register = async () => { + await waitForPushRelease(10_000); + events.push('registration may read'); + }; + const waiting = register(); + await settle(); + expect(events).toEqual([]); + + request.resolve(); + await Promise.all([release, waiting]); + expect(events).toEqual(['token deleted', 'registration may read']); + }); + + it('runs releases one after another', async () => { + const first = deferred(); + saveMock.mockImplementationOnce(() => first.promise); + const events: string[] = []; + mockMessaging.getToken.mockImplementation(async () => { + events.push('token read'); + return 'fcm-token'; + }); + mockMessaging.deleteToken.mockImplementation(async () => { + events.push('token deleted'); + }); + + const releaseA = disablePushRegistrations([{ username: 'alice', accessToken: 'a' }], { + deleteToken: true, + }); + const releaseB = disablePushRegistrations([{ username: 'bob', accessToken: 'b' }], { + deleteToken: false, + }); + await settle(); + expect(events).toEqual(['token read']); + + first.resolve(); + await Promise.all([releaseA, releaseB]); + expect(events).toEqual(['token read', 'token deleted', 'token read']); + }); + + it('gives up waiting after the timeout when a release never settles', async () => { + const stuck = deferred(); + saveMock.mockImplementation(() => stuck.promise); + + const release = disablePushRegistrations([{ username: 'alice', accessToken: 'code' }], { + deleteToken: true, + }); + await expect(waitForPushRelease(20)).resolves.toBeUndefined(); + expect(mockMessaging.deleteToken).not.toHaveBeenCalled(); + + // Let the release finish so it does not hold up the next test. + stuck.resolve(); + await release; + }); +}); diff --git a/src/utils/pushRegistration.ts b/src/utils/pushRegistration.ts index 769032c3d0..231d4a40d5 100644 --- a/src/utils/pushRegistration.ts +++ b/src/utils/pushRegistration.ts @@ -44,15 +44,11 @@ export interface PushAccount { } /** - * Every account signed in on this device, with its decrypted access token. The - * current account is normally also in `otherAccounts`; its own entry is used when it - * carries a token (its `local` data is the freshest), and added if it was missing. + * Every account signed in on this device, once each. The current account is normally + * also in `otherAccounts`; its own entry is used when it carries a token (its `local` + * data is the freshest), and added if it was missing. */ -export const getPushAccounts = ( - currentAccount: any, - otherAccounts: any[] = [], - encAppPin: string | null | undefined, -): PushAccount[] => { +export const getSignedInAccounts = (currentAccount: any, otherAccounts: any[] = []) => { const accounts = new Map(); otherAccounts.forEach((account) => { const username = account?.username || account?.name; @@ -64,26 +60,27 @@ export const getPushAccounts = ( accounts.set(currentAccount.name, currentAccount); } - return Array.from(accounts, ([username, account]) => ({ + return Array.from(accounts, ([username, account]) => ({ username, account })); +}; + +/** Every account signed in on this device, with its decrypted access token. */ +export const getPushAccounts = ( + currentAccount: any, + otherAccounts: any[] = [], + encAppPin: string | null | undefined, +): PushAccount[] => + getSignedInAccounts(currentAccount, otherAccounts).map(({ username, account }) => ({ username, accessToken: decryptAccessToken(account?.local?.accessToken, encAppPin), })); -}; -/** - * Turns push off for accounts leaving this device. - * - * Each account's row is disabled with the current FCM token. When no account is left - * on the device, the token itself is deleted afterwards: the backend then gets - * "unregistered" for it and stops sending, even for a row this call could not - * disable (for example an account whose access token no longer decrypts). The - * delete must come after the requests, because deleting first makes the next - * `getToken()` mint a new token and the requests would disable the wrong rows. - */ -export const disablePushRegistrations = async ( - accounts: PushAccount[], - { deleteToken }: { deleteToken: boolean }, -) => { +// The deregistration in progress, if any. Releases run one after another, and a +// registration waits for them (see waitForPushRelease). +let pendingRelease: Promise = Promise.resolve(); + +export const PUSH_RELEASE_WAIT_MS = 30 * 1000; + +const releasePushRegistrations = async (accounts: PushAccount[], deleteToken: boolean) => { let token: string; try { token = await getMessaging().getToken(); @@ -118,3 +115,47 @@ export const disablePushRegistrations = async ( } } }; + +/** + * Turns push off for accounts leaving this device. + * + * Each account's row is disabled with the current FCM token. When no account is left + * on the device, the token itself is deleted afterwards: the backend then gets + * "unregistered" for it and stops sending, even for a row this call could not + * disable (for example an account whose access token no longer decrypts). The + * delete must come after the requests, because deleting first makes the next + * `getToken()` mint a new token and the requests would disable the wrong rows. + * + * Callers do not need to wait: the logout itself should not hang on the network. + * Registrations wait instead, through waitForPushRelease. + */ +export const disablePushRegistrations = ( + accounts: PushAccount[], + { deleteToken }: { deleteToken: boolean }, +): Promise => { + const release = pendingRelease + .catch(() => undefined) + .then(() => releasePushRegistrations(accounts, deleteToken)); + pendingRelease = release; + return release; +}; + +/** + * Resolves once no deregistration is in progress, or after `timeoutMs`. + * + * A login right after the last account logged out would otherwise read the token + * that the pending release is about to delete, and register a dead token. Waiting + * lets the release finish, so `getToken()` returns the new token. The timeout keeps a + * stuck release (a native call that never settles) from blocking registration. + */ +export const waitForPushRelease = async (timeoutMs = PUSH_RELEASE_WAIT_MS) => { + let timer: ReturnType | undefined; + const timeout = new Promise((resolve) => { + timer = setTimeout(resolve, timeoutMs); + }); + try { + await Promise.race([pendingRelease.catch(() => undefined), timeout]); + } finally { + clearTimeout(timer); + } +}; From ec23d5ee2686a04c8a741081b7edc6f72b0cf804 Mon Sep 17 00:00:00 2001 From: feruzm Date: Thu, 17 Sep 2026 10:07:44 +0000 Subject: [PATCH 3/5] fix(push): keep a token that a registration read during a stuck release - A release no longer deletes the device token when a registration read it after the release started (possible once the 30s wait gives up), so that registration does not end up on a dead token. - An account whose registration the server refused (401/403) is registered again when its access token is renewed. Other token renewals, which happen on every start and foreground, still do not register. --- .../container/applicationContainer.tsx | 25 +++++++++++++++---- src/utils/pushRegistration.test.ts | 24 ++++++++++++++++++ src/utils/pushRegistration.ts | 21 ++++++++++++++++ 3 files changed, 65 insertions(+), 5 deletions(-) diff --git a/src/screens/application/container/applicationContainer.tsx b/src/screens/application/container/applicationContainer.tsx index 375cdf13b8..c6dd825222 100644 --- a/src/screens/application/container/applicationContainer.tsx +++ b/src/screens/application/container/applicationContainer.tsx @@ -95,8 +95,8 @@ import { decryptAccessToken, disablePushRegistrations, getPushSystem, + getRegistrationToken, getSignedInAccounts, - waitForPushRelease, } from '../../../utils/pushRegistration'; import { captureException, captureMessage } from '../../../utils/sentryUtils'; import { fetchSubscribedCommunities } from '../../../redux/actions/communitiesAction'; @@ -147,6 +147,9 @@ class ApplicationContainer extends Component { _lastPushRegistration = 0; + // Accounts whose last push registration the server refused (401/403). + _rejectedPushRegistrations = new Set(); + constructor(props: any) { super(props); this.state = { @@ -213,8 +216,16 @@ class ApplicationContainer extends Component { const { isGlobalRenderRequired, dispatch, currentAccount } = this.props; // A login (by any method) or an account switch makes a different account current. - // Register it right away instead of waiting for the next cold start. - if (currentAccount?.name && currentAccount.name !== prevProps.currentAccount?.name) { + // Register it right away instead of waiting for the next cold start. The access + // token is renewed on every start and foreground, so a token change alone only + // retries an account whose last registration the server refused. + const accountChanged = + !!currentAccount?.name && currentAccount.name !== prevProps.currentAccount?.name; + const rejectedTokenRenewed = + !!currentAccount?.name && + currentAccount.local?.accessToken !== prevProps.currentAccount?.local?.accessToken && + this._rejectedPushRegistrations.has(currentAccount.name); + if (accountChanged || rejectedTokenRenewed) { this._registerAccountForNotifications(currentAccount); } @@ -1225,8 +1236,7 @@ class ApplicationContainer extends Component { } // A logout may still be disabling rows and deleting this token; read it after that. - await waitForPushRelease(); - const token = await getMessaging().getToken(); + const token = await getRegistrationToken(); console.log('FCM Token obtained:', !!token); try { await saveNotificationSetting( @@ -1237,6 +1247,7 @@ class ApplicationContainer extends Component { notify_types, token, ); + this._rejectedPushRegistrations.delete(username); } catch (error) { const status = (error as any)?.status; if (typeof status !== 'number') { @@ -1244,6 +1255,10 @@ class ApplicationContainer extends Component { console.warn('Push registration request failed', error); return; } + if (status === 401 || status === 403) { + // Retried when this account's access token is renewed (componentDidUpdate). + this._rejectedPushRegistrations.add(username); + } captureException(error, (scope) => { scope.setTag('context', 'push-registration'); scope.setTag('status', String(status)); diff --git a/src/utils/pushRegistration.test.ts b/src/utils/pushRegistration.test.ts index 08c86900ce..b37493b2ca 100644 --- a/src/utils/pushRegistration.test.ts +++ b/src/utils/pushRegistration.test.ts @@ -7,6 +7,7 @@ import { disablePushRegistrations, getPushAccounts, getPushSystem, + getRegistrationToken, getSignedInAccounts, waitForPushRelease, } from './pushRegistration'; @@ -306,4 +307,27 @@ describe('waitForPushRelease', () => { stuck.resolve(); await release; }); + + it('keeps the token when a registration read it while the release was stuck', async () => { + const stuck = deferred(); + saveMock.mockImplementation(() => stuck.promise); + + const release = disablePushRegistrations([{ username: 'alice', accessToken: 'code' }], { + deleteToken: true, + }); + await expect(getRegistrationToken(20)).resolves.toBe('fcm-token'); + + stuck.resolve(); + await release; + expect(mockMessaging.deleteToken).not.toHaveBeenCalled(); + }); + + it('still deletes the token for a release that starts after a registration', async () => { + await getRegistrationToken(20); + + await disablePushRegistrations([{ username: 'alice', accessToken: 'code' }], { + deleteToken: true, + }); + expect(mockMessaging.deleteToken).toHaveBeenCalledTimes(1); + }); }); diff --git a/src/utils/pushRegistration.ts b/src/utils/pushRegistration.ts index 231d4a40d5..9df3f1ab5b 100644 --- a/src/utils/pushRegistration.ts +++ b/src/utils/pushRegistration.ts @@ -78,9 +78,13 @@ export const getPushAccounts = ( // registration waits for them (see waitForPushRelease). let pendingRelease: Promise = Promise.resolve(); +// Counts the times a registration has read the device token (getRegistrationToken). +let registrationTokenReads = 0; + export const PUSH_RELEASE_WAIT_MS = 30 * 1000; const releasePushRegistrations = async (accounts: PushAccount[], deleteToken: boolean) => { + const readsAtStart = registrationTokenReads; let token: string; try { token = await getMessaging().getToken(); @@ -108,6 +112,12 @@ const releasePushRegistrations = async (accounts: PushAccount[], deleteToken: bo ); if (deleteToken) { + if (registrationTokenReads !== readsAtStart) { + // A registration stopped waiting (timeout) and registered this token meanwhile. + // Deleting it now would leave that registration pointing at a dead token. + console.warn('Push token was registered during deregistration, keeping it'); + return; + } try { await getMessaging().deleteToken(); } catch (err) { @@ -159,3 +169,14 @@ export const waitForPushRelease = async (timeoutMs = PUSH_RELEASE_WAIT_MS) => { clearTimeout(timer); } }; + +/** + * Reads the device token for a registration, after any deregistration in progress. + * A release that is still running when this reads the token (only possible once the + * wait timed out) no longer deletes the token. + */ +export const getRegistrationToken = async (timeoutMs = PUSH_RELEASE_WAIT_MS) => { + await waitForPushRelease(timeoutMs); + registrationTokenReads += 1; + return getMessaging().getToken(); +}; From 9269a1b3041a86c3209188cd44268cd2eb97ec38 Mon Sep 17 00:00:00 2001 From: feruzm Date: Thu, 17 Sep 2026 13:50:26 +0000 Subject: [PATCH 4/5] fix(push): register again after a release that outlived the wait When a registration stops waiting (30s), the release's disable requests are already out and can reach the backend after the registration, switching the row back off. They cannot be recalled, so getRegistrationToken now calls back once that release settles, and the container registers the account again if it is still signed in. Also: a save that throws synchronously no longer skips the other accounts or the token delete, and the release chain never rejects. --- .../container/applicationContainer.tsx | 16 +++- src/utils/pushRegistration.test.ts | 88 ++++++++++++++++++- src/utils/pushRegistration.ts | 72 ++++++++++----- 3 files changed, 150 insertions(+), 26 deletions(-) diff --git a/src/screens/application/container/applicationContainer.tsx b/src/screens/application/container/applicationContainer.tsx index c6dd825222..04f8efbe12 100644 --- a/src/screens/application/container/applicationContainer.tsx +++ b/src/screens/application/container/applicationContainer.tsx @@ -784,6 +784,16 @@ class ApplicationContainer extends Component { }); }; + _registerAgainIfSignedIn = (username: string) => { + const { currentAccount, otherAccounts } = this.props; + const entry = getSignedInAccounts(currentAccount, otherAccounts).find( + (signedIn) => signedIn.username === username, + ); + if (entry) { + this._registerAccountForNotifications(entry.account); + } + }; + _registerAccountForNotifications = (account: any) => { const { notificationDetails, isNotificationsEnabled } = this.props; const encAccessToken = account?.local?.accessToken; @@ -1236,7 +1246,11 @@ class ApplicationContainer extends Component { } // A logout may still be disabling rows and deleting this token; read it after that. - const token = await getRegistrationToken(); + // If the wait gave up, register once more when that logout has finished, so its + // late disable request cannot be the last write for this account. + const token = await getRegistrationToken({ + onReleaseSettled: () => this._registerAgainIfSignedIn(username), + }); console.log('FCM Token obtained:', !!token); try { await saveNotificationSetting( diff --git a/src/utils/pushRegistration.test.ts b/src/utils/pushRegistration.test.ts index b37493b2ca..716ab2ea1a 100644 --- a/src/utils/pushRegistration.test.ts +++ b/src/utils/pushRegistration.test.ts @@ -9,6 +9,7 @@ import { getPushSystem, getRegistrationToken, getSignedInAccounts, + PushAccount, waitForPushRelease, } from './pushRegistration'; @@ -240,7 +241,7 @@ describe('waitForPushRelease', () => { const settle = () => new Promise((resolve) => setImmediate(resolve)); it('resolves at once when nothing is being released', async () => { - await expect(waitForPushRelease(10_000)).resolves.toBeUndefined(); + await expect(waitForPushRelease(10_000)).resolves.toBe(true); }); it('waits until a pending release has deleted the token', async () => { @@ -300,7 +301,7 @@ describe('waitForPushRelease', () => { const release = disablePushRegistrations([{ username: 'alice', accessToken: 'code' }], { deleteToken: true, }); - await expect(waitForPushRelease(20)).resolves.toBeUndefined(); + await expect(waitForPushRelease(20)).resolves.toBe(false); expect(mockMessaging.deleteToken).not.toHaveBeenCalled(); // Let the release finish so it does not hold up the next test. @@ -315,7 +316,7 @@ describe('waitForPushRelease', () => { const release = disablePushRegistrations([{ username: 'alice', accessToken: 'code' }], { deleteToken: true, }); - await expect(getRegistrationToken(20)).resolves.toBe('fcm-token'); + await expect(getRegistrationToken({ timeoutMs: 20 })).resolves.toBe('fcm-token'); stuck.resolve(); await release; @@ -323,11 +324,90 @@ describe('waitForPushRelease', () => { }); it('still deletes the token for a release that starts after a registration', async () => { - await getRegistrationToken(20); + await getRegistrationToken({ timeoutMs: 20 }); await disablePushRegistrations([{ username: 'alice', accessToken: 'code' }], { deleteToken: true, }); expect(mockMessaging.deleteToken).toHaveBeenCalledTimes(1); }); + + it('asks to register again once a release that outlived the wait settles', async () => { + const stuck = deferred(); + saveMock.mockImplementation(() => stuck.promise); + const registerAgain = jest.fn(); + + const release = disablePushRegistrations([{ username: 'alice', accessToken: 'code' }], { + deleteToken: true, + }); + await getRegistrationToken({ timeoutMs: 20, onReleaseSettled: registerAgain }); + await settle(); + // The disable request is still out: registering again now could still lose to it. + expect(registerAgain).not.toHaveBeenCalled(); + + stuck.resolve(); + await release; + await settle(); + expect(registerAgain).toHaveBeenCalledTimes(1); + }); + + it('does not ask to register again when the release finished within the wait', async () => { + const quick = deferred(); + saveMock.mockImplementation(() => quick.promise); + const registerAgain = jest.fn(); + + const release = disablePushRegistrations([{ username: 'alice', accessToken: 'code' }], { + deleteToken: false, + }); + const reading = getRegistrationToken({ timeoutMs: 10_000, onReleaseSettled: registerAgain }); + quick.resolve(); + await Promise.all([release, reading]); + await settle(); + + expect(registerAgain).not.toHaveBeenCalled(); + }); + + it('does not ask to register again when nothing was being released', async () => { + const registerAgain = jest.fn(); + await getRegistrationToken({ timeoutMs: 10_000, onReleaseSettled: registerAgain }); + await settle(); + + expect(registerAgain).not.toHaveBeenCalled(); + }); + + it('survives a request that throws synchronously', async () => { + saveMock.mockImplementationOnce(() => { + throw new Error('synchronous failure'); + }); + + await disablePushRegistrations( + [ + { username: 'alice', accessToken: 'a' }, + { username: 'bob', accessToken: 'b' }, + ], + { deleteToken: true }, + ); + expect(saveMock).toHaveBeenCalledTimes(2); + expect(mockMessaging.deleteToken).toHaveBeenCalledTimes(1); + + // The chain stays usable for the next release and for waiting registrations. + await disablePushRegistrations([{ username: 'carol', accessToken: 'c' }], { + deleteToken: false, + }); + expect(saveMock).toHaveBeenCalledTimes(3); + await expect(waitForPushRelease(20)).resolves.toBe(true); + }); + + it('keeps the chain usable after a release fails unexpectedly', async () => { + // A malformed call throws inside the release itself. + await expect( + disablePushRegistrations(null as unknown as PushAccount[], { deleteToken: true }), + ).resolves.toBeUndefined(); + + await expect(waitForPushRelease(20)).resolves.toBe(true); + await disablePushRegistrations([{ username: 'alice', accessToken: 'a' }], { + deleteToken: true, + }); + expect(mockMessaging.deleteToken).toHaveBeenCalledTimes(1); + }); }); diff --git a/src/utils/pushRegistration.ts b/src/utils/pushRegistration.ts index 9df3f1ab5b..00580588b6 100644 --- a/src/utils/pushRegistration.ts +++ b/src/utils/pushRegistration.ts @@ -98,16 +98,22 @@ const releasePushRegistrations = async (accounts: PushAccount[], deleteToken: bo accounts .filter((account) => account.username && account.accessToken) .map((account) => - saveNotificationSetting( - account.accessToken, - account.username, - getPushSystem(), - 0, - [], - token, - ).catch((err) => { - console.warn('Failed to disable push notifications for', account.username, err); - }), + // Inside then(), so even a synchronous throw becomes this account's failure + // and cannot skip the other accounts or the token delete. + Promise.resolve() + .then(() => + saveNotificationSetting( + account.accessToken, + account.username, + getPushSystem(), + 0, + [], + token, + ), + ) + .catch((err) => { + console.warn('Failed to disable push notifications for', account.username, err); + }), ), ); @@ -144,27 +150,31 @@ export const disablePushRegistrations = ( { deleteToken }: { deleteToken: boolean }, ): Promise => { const release = pendingRelease - .catch(() => undefined) - .then(() => releasePushRegistrations(accounts, deleteToken)); + .then(() => releasePushRegistrations(accounts, deleteToken)) + // Never rejects: releasePushRegistrations catches its own failures, and the chain + // must stay usable for the next release and for waiting registrations. + .catch((err) => { + console.warn('Push deregistration failed', err); + }); pendingRelease = release; return release; }; /** - * Resolves once no deregistration is in progress, or after `timeoutMs`. + * Resolves to true once no deregistration is in progress, or to false after `timeoutMs`. * * A login right after the last account logged out would otherwise read the token * that the pending release is about to delete, and register a dead token. Waiting * lets the release finish, so `getToken()` returns the new token. The timeout keeps a * stuck release (a native call that never settles) from blocking registration. */ -export const waitForPushRelease = async (timeoutMs = PUSH_RELEASE_WAIT_MS) => { +export const waitForPushRelease = async (timeoutMs = PUSH_RELEASE_WAIT_MS): Promise => { let timer: ReturnType | undefined; - const timeout = new Promise((resolve) => { - timer = setTimeout(resolve, timeoutMs); + const timeout = new Promise((resolve) => { + timer = setTimeout(() => resolve(false), timeoutMs); }); try { - await Promise.race([pendingRelease.catch(() => undefined), timeout]); + return await Promise.race([pendingRelease.then(() => true), timeout]); } finally { clearTimeout(timer); } @@ -172,11 +182,31 @@ export const waitForPushRelease = async (timeoutMs = PUSH_RELEASE_WAIT_MS) => { /** * Reads the device token for a registration, after any deregistration in progress. - * A release that is still running when this reads the token (only possible once the - * wait timed out) no longer deletes the token. + * + * When the wait times out, the release is still running: + * - it no longer deletes the token this registration read; + * - its disable requests are already out and may reach the backend AFTER this + * registration, switching the row back off. They cannot be recalled, so + * `onReleaseSettled` is called once the release has settled, for the caller to + * register again. */ -export const getRegistrationToken = async (timeoutMs = PUSH_RELEASE_WAIT_MS) => { - await waitForPushRelease(timeoutMs); +export const getRegistrationToken = async ({ + timeoutMs = PUSH_RELEASE_WAIT_MS, + onReleaseSettled, +}: { timeoutMs?: number; onReleaseSettled?: () => void } = {}) => { + const release = pendingRelease; + const settled = await waitForPushRelease(timeoutMs); registrationTokenReads += 1; + + if (!settled && onReleaseSettled) { + release.then(() => { + try { + onReleaseSettled(); + } catch (err) { + console.warn('Failed to register again after push deregistration', err); + } + }); + } + return getMessaging().getToken(); }; From 8f2721ba638cee3985f6045570c5ac8ec55ec50b Mon Sep 17 00:00:00 2001 From: feruzm Date: Thu, 17 Sep 2026 14:10:14 +0000 Subject: [PATCH 5/5] fix(push): close the remaining logout/registration races From an adversarial review: - A registration now waits until no release is queued (not just the one it started behind), then registers only if the account is still signed in. A login followed by a quick logout no longer registers the logged-out account. - A release waits for registrations already in flight before sending its disable requests, so a registration cannot land after them. - A release deletes the token when a row could not be disabled (request failed or no access token), even with accounts left, and the app registers the remaining accounts with the new token. The read counter is gone: timed-out registrations are covered by the settle retry and the re-registration. - An account being logged out is never registered while the app switches to the next account (reconnect, token refresh or a retry in that window). - A 401/403 whose token was renewed meanwhile is retried at once; Sentry gets each registration problem once per account per session. - A reconnect no longer registers every account twice. --- .../container/applicationContainer.tsx | 106 ++++-- src/utils/pushRegistration.test.ts | 343 +++++++++++------- src/utils/pushRegistration.ts | 229 ++++++++---- 3 files changed, 451 insertions(+), 227 deletions(-) diff --git a/src/screens/application/container/applicationContainer.tsx b/src/screens/application/container/applicationContainer.tsx index 04f8efbe12..3bdd254cf8 100644 --- a/src/screens/application/container/applicationContainer.tsx +++ b/src/screens/application/container/applicationContainer.tsx @@ -91,11 +91,12 @@ import parseVersionNumber from '../../../utils/parseVersionNumber'; import { setMomentLocale } from '../../../utils/time'; import { purgeExpiredCache } from '../../../redux/actions/cacheActions'; import { + PUSH_REGISTRATION_DEDUPE_MS, PUSH_REGISTRATION_REFRESH_MS, + beginPushRegistration, decryptAccessToken, disablePushRegistrations, getPushSystem, - getRegistrationToken, getSignedInAccounts, } from '../../../utils/pushRegistration'; import { captureException, captureMessage } from '../../../utils/sentryUtils'; @@ -150,6 +151,13 @@ class ApplicationContainer extends Component { // Accounts whose last push registration the server refused (401/403). _rejectedPushRegistrations = new Set(); + // Accounts being logged out: still listed until the logout finishes, but never + // registered again. + _departingAccounts = new Set(); + + // Push registration problems already reported to Sentry this session. + _reportedPushProblems = new Set(); + constructor(props: any) { super(props); this.state = { @@ -396,7 +404,10 @@ class ApplicationContainer extends Component { await this._getUserDataFromRealm(); await this._refreshUnreadChats(); this._compareAndPromptForUpdate(); - this._registerDeviceForNotifications(); + // Reconnects run this twice in a row (NetInfo listener and props change). + if (Date.now() - this._lastPushRegistration > PUSH_REGISTRATION_DEDUPE_MS) { + this._registerDeviceForNotifications(); + } dispatch(purgeExpiredCache()); }; @@ -779,18 +790,35 @@ class ApplicationContainer extends Component { // Accounts without a stored access token (HiveSigner accounts, entries keyed only by // username) are skipped without a report: reporting them fired an error on every // launch (ECENCY-MOBILE-1QY). - getSignedInAccounts(currentAccount, otherAccounts).forEach(({ account }) => { - this._registerAccountForNotifications(account); + getSignedInAccounts(currentAccount, otherAccounts).forEach(({ username, account }) => { + if (!this._departingAccounts.has(username)) { + this._registerAccountForNotifications(account); + } }); }; - _registerAgainIfSignedIn = (username: string) => { + _signedInPushAccount = (username: string) => { + if (this._departingAccounts.has(username)) { + return undefined; + } const { currentAccount, otherAccounts } = this.props; - const entry = getSignedInAccounts(currentAccount, otherAccounts).find( + return getSignedInAccounts(currentAccount, otherAccounts).find( (signedIn) => signedIn.username === username, - ); - if (entry) { - this._registerAccountForNotifications(entry.account); + )?.account; + }; + + _registerAgainIfSignedIn = (username: string) => { + const account = this._signedInPushAccount(username); + if (account) { + this._registerAccountForNotifications(account); + } + }; + + // Reports a push registration problem once per session. + _reportPushProblem = (key: string, report: () => void) => { + if (!this._reportedPushProblems.has(key)) { + this._reportedPushProblems.add(key); + report(); } }; @@ -1117,6 +1145,9 @@ class ApplicationContainer extends Component { : otherAccounts.find((user: any) => (user.username || user.name) === username); const accessToken = decryptAccessToken(loggedOutAccount?.local?.accessToken, pinCode); + // Until the logout finishes the account is still listed; keep it out of registrations. + this._departingAccounts.add(username); + try { const response = await removeUserData(username); @@ -1132,6 +1163,8 @@ class ApplicationContainer extends Component { // device token so nothing registered under it is delivered any more. disablePushRegistrations([{ username, accessToken }], { deleteToken: _otherAccounts.length === 0, + // A replaced token must be registered again for whoever is still signed in. + onTokenReplaced: () => this._registerDeviceForNotifications(), }); if (_otherAccounts.length > 0) { @@ -1170,6 +1203,8 @@ class ApplicationContainer extends Component { dispatch(logoutDone()); Alert.alert(intl.formatMessage({ id: 'alert.fail' }), (err as any).message); this._repairUserAccountData(username); + } finally { + this._departingAccounts.delete(username); } }; @@ -1185,11 +1220,15 @@ class ApplicationContainer extends Component { if (!accessToken) { // The request would be rejected without it. A stored token that does not decrypt // means the PIN state and the stored keys disagree, which is worth knowing about. - const message = 'Push registration skipped: stored access token did not decrypt'; - captureMessage(message, (scope) => { - scope.setTag('context', 'push-registration'); - scope.setFingerprint(['push-registration-decrypt']); - }); + this._reportPushProblem(`decrypt:${username}`, () => + captureMessage( + 'Push registration skipped: stored access token did not decrypt', + (scope) => { + scope.setTag('context', 'push-registration'); + scope.setFingerprint(['push-registration-decrypt']); + }, + ), + ); return; } @@ -1245,13 +1284,17 @@ class ApplicationContainer extends Component { return; } - // A logout may still be disabling rows and deleting this token; read it after that. - // If the wait gave up, register once more when that logout has finished, so its - // late disable request cannot be the last write for this account. - const token = await getRegistrationToken({ + // A logout may still be disabling rows or replacing this token: register after it, + // and only if the account is still signed in by then. If the wait gave up, register + // once more when every logout has finished, so a late disable is not the last write. + const registration = await beginPushRegistration({ + stillWanted: () => !!this._signedInPushAccount(username), onReleaseSettled: () => this._registerAgainIfSignedIn(username), }); - console.log('FCM Token obtained:', !!token); + if (!registration) { + return; + } + console.log('FCM Token obtained:', !!registration.token); try { await saveNotificationSetting( accessToken, @@ -1259,7 +1302,7 @@ class ApplicationContainer extends Component { getPushSystem(), Number(isEnable), notify_types, - token, + registration.token, ); this._rejectedPushRegistrations.delete(username); } catch (error) { @@ -1270,14 +1313,23 @@ class ApplicationContainer extends Component { return; } if (status === 401 || status === 403) { - // Retried when this account's access token is renewed (componentDidUpdate). - this._rejectedPushRegistrations.add(username); + if (this._signedInPushAccount(username)?.local?.accessToken !== encAccesstoken) { + // The token was renewed while this request was out: retry with the new one. + this._registerAgainIfSignedIn(username); + } else { + // Retried when this account's access token is renewed (componentDidUpdate). + this._rejectedPushRegistrations.add(username); + } } - captureException(error, (scope) => { - scope.setTag('context', 'push-registration'); - scope.setTag('status', String(status)); - scope.setFingerprint(['push-registration-http', String(status)]); - }); + this._reportPushProblem(`http:${status}:${username}`, () => + captureException(error, (scope) => { + scope.setTag('context', 'push-registration'); + scope.setTag('status', String(status)); + scope.setFingerprint(['push-registration-http', String(status)]); + }), + ); + } finally { + registration.finish(); } } catch (error) { // Handle platform-specific FCM errors gracefully diff --git a/src/utils/pushRegistration.test.ts b/src/utils/pushRegistration.test.ts index 716ab2ea1a..0c112e1ae6 100644 --- a/src/utils/pushRegistration.test.ts +++ b/src/utils/pushRegistration.test.ts @@ -6,8 +6,8 @@ import { decryptAccessToken, disablePushRegistrations, getPushAccounts, + beginPushRegistration, getPushSystem, - getRegistrationToken, getSignedInAccounts, PushAccount, waitForPushRelease, @@ -140,12 +140,26 @@ describe('getSignedInAccounts', () => { }); }); +const deferred = () => { + let resolve: () => void = () => undefined; + let reject: (err: Error) => void = () => undefined; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + return { promise, resolve, reject }; +}; +const settle = () => new Promise((resolve) => setImmediate(resolve)); +const always = () => true; + +// Every test leaves the release chain and the in-flight set empty: each release it starts +// is awaited, and each registration it begins is finished. + describe('disablePushRegistrations', () => { - it('disables every account that has a token, with the current device token', async () => { + it('disables every account with the current device token and keeps the token', async () => { await disablePushRegistrations( [ { username: 'alice', accessToken: 'alice-code' }, - { username: 'bob', accessToken: undefined }, { username: 'carol', accessToken: 'carol-code' }, ], { deleteToken: false }, @@ -173,21 +187,14 @@ describe('disablePushRegistrations', () => { it('deletes the device token only after every request has settled', async () => { const events: string[] = []; - let finishAlice: () => void = () => undefined; - saveMock.mockImplementation( - (_code: string, username: string) => - new Promise((resolve, reject) => { - if (username === 'alice') { - finishAlice = () => { - events.push('alice settled'); - resolve(); - }; - } else { - events.push('bob settled'); - reject(new Error('Request failed with status 500')); - } - }), - ); + const alice = deferred(); + saveMock.mockImplementation((_code: string, username: string) => { + if (username === 'alice') { + return alice.promise.then(() => events.push('alice settled')); + } + events.push('bob settled'); + return Promise.reject(new Error('Request failed with status 500')); + }); mockMessaging.deleteToken.mockImplementation(async () => { events.push('token deleted'); }); @@ -199,16 +206,52 @@ describe('disablePushRegistrations', () => { ], { deleteToken: true }, ); - await new Promise((resolve) => setImmediate(resolve)); + await settle(); expect(mockMessaging.deleteToken).not.toHaveBeenCalled(); - finishAlice(); + alice.resolve(); await done; - - // A failed request does not block the others or the token delete. expect(events).toEqual(['bob settled', 'alice settled', 'token deleted']); }); + it.each([ + ['a request fails', [{ username: 'alice', accessToken: 'a' }], true], + ['an account has no access token', [{ username: 'alice', accessToken: undefined }], false], + ])( + 'with accounts left, replaces the token when %s, so the row cannot stay on', + async (_case, accounts, failRequest) => { + if (failRequest) { + saveMock.mockRejectedValueOnce(new Error('Request failed with status 401')); + } + const onTokenReplaced = jest.fn(); + + await disablePushRegistrations(accounts, { deleteToken: false, onTokenReplaced }); + + expect(mockMessaging.deleteToken).toHaveBeenCalledTimes(1); + expect(onTokenReplaced).toHaveBeenCalledTimes(1); + }, + ); + + it('asks for registration after deleting the token with no account left', async () => { + const onTokenReplaced = jest.fn(); + await disablePushRegistrations([{ username: 'alice', accessToken: 'a' }], { + deleteToken: true, + onTokenReplaced, + }); + expect(onTokenReplaced).toHaveBeenCalledTimes(1); + }); + + it('does not ask for registration when the token could not be deleted', async () => { + mockMessaging.deleteToken.mockRejectedValue(new Error('offline')); + const onTokenReplaced = jest.fn(); + + await expect( + disablePushRegistrations([], { deleteToken: true, onTokenReplaced }), + ).resolves.toBeUndefined(); + expect(mockMessaging.deleteToken).toHaveBeenCalledTimes(1); + expect(onTokenReplaced).not.toHaveBeenCalled(); + }); + it('does nothing when the device has no token', async () => { mockMessaging.getToken.mockRejectedValue(new Error('SERVICE_NOT_AVAILABLE')); @@ -217,55 +260,68 @@ describe('disablePushRegistrations', () => { deleteToken: true, }), ).resolves.toBeUndefined(); - expect(saveMock).not.toHaveBeenCalled(); expect(mockMessaging.deleteToken).not.toHaveBeenCalled(); }); - it('does not throw when deleting the token fails', async () => { - mockMessaging.deleteToken.mockRejectedValue(new Error('offline')); + it('survives a request that throws synchronously', async () => { + saveMock.mockImplementationOnce(() => { + throw new Error('synchronous failure'); + }); - await expect(disablePushRegistrations([], { deleteToken: true })).resolves.toBeUndefined(); + await disablePushRegistrations( + [ + { username: 'alice', accessToken: 'a' }, + { username: 'bob', accessToken: 'b' }, + ], + { deleteToken: false }, + ); + // Both requests were attempted, and the failed one replaced the token. + expect(saveMock).toHaveBeenCalledTimes(2); expect(mockMessaging.deleteToken).toHaveBeenCalledTimes(1); }); -}); -describe('waitForPushRelease', () => { - const deferred = () => { - let resolve: () => void = () => undefined; - const promise = new Promise((r) => { - resolve = r; - }); - return { promise, resolve }; - }; - const settle = () => new Promise((resolve) => setImmediate(resolve)); + it('keeps the chain usable after a release fails unexpectedly', async () => { + // A malformed call throws inside the release itself. + await expect( + disablePushRegistrations(null as unknown as PushAccount[], { deleteToken: true }), + ).resolves.toBeUndefined(); - it('resolves at once when nothing is being released', async () => { - await expect(waitForPushRelease(10_000)).resolves.toBe(true); + await expect(waitForPushRelease(20)).resolves.toBe(true); + await disablePushRegistrations([{ username: 'alice', accessToken: 'a' }], { + deleteToken: true, + }); + expect(mockMessaging.deleteToken).toHaveBeenCalledTimes(1); }); - it('waits until a pending release has deleted the token', async () => { - const request = deferred(); - saveMock.mockImplementation(() => request.promise); - const events: string[] = []; - mockMessaging.deleteToken.mockImplementation(async () => { - events.push('token deleted'); + it('waits for a registration already in flight before disabling', async () => { + const registration = await beginPushRegistration({ stillWanted: always }); + const release = disablePushRegistrations([{ username: 'alice', accessToken: 'a' }], { + deleteToken: false, }); + await settle(); + // The registration's request is still out: the disable must not overtake it. + expect(saveMock).not.toHaveBeenCalled(); - const release = disablePushRegistrations([{ username: 'alice', accessToken: 'code' }], { - deleteToken: true, + registration!.finish(); + await release; + expect(saveMock).toHaveBeenCalledTimes(1); + }); + + it('stops waiting for a registration that never finishes', async () => { + const registration = await beginPushRegistration({ stillWanted: always }); + await disablePushRegistrations([{ username: 'alice', accessToken: 'a' }], { + deleteToken: false, + registrationWaitMs: 20, }); - const register = async () => { - await waitForPushRelease(10_000); - events.push('registration may read'); - }; - const waiting = register(); - await settle(); - expect(events).toEqual([]); + expect(saveMock).toHaveBeenCalledTimes(1); + registration!.finish(); + }); +}); - request.resolve(); - await Promise.all([release, waiting]); - expect(events).toEqual(['token deleted', 'registration may read']); +describe('waitForPushRelease', () => { + it('resolves at once when nothing is being released', async () => { + await expect(waitForPushRelease(10_000)).resolves.toBe(true); }); it('runs releases one after another', async () => { @@ -294,7 +350,35 @@ describe('waitForPushRelease', () => { expect(events).toEqual(['token read', 'token deleted', 'token read']); }); - it('gives up waiting after the timeout when a release never settles', async () => { + it('waits for releases queued while it waits', async () => { + const first = deferred(); + const second = deferred(); + saveMock + .mockImplementationOnce(() => first.promise) + .mockImplementationOnce(() => second.promise); + + const releaseA = disablePushRegistrations([{ username: 'alice', accessToken: 'a' }], { + deleteToken: false, + }); + let waited: boolean | undefined; + const waiting = waitForPushRelease(10_000).then((result) => { + waited = result; + }); + const releaseB = disablePushRegistrations([{ username: 'bob', accessToken: 'b' }], { + deleteToken: false, + }); + + first.resolve(); + await releaseA; + await settle(); + expect(waited).toBeUndefined(); + + second.resolve(); + await Promise.all([releaseB, waiting]); + expect(waited).toBe(true); + }); + + it('gives up after the timeout when a release never settles', async () => { const stuck = deferred(); saveMock.mockImplementation(() => stuck.promise); @@ -304,110 +388,115 @@ describe('waitForPushRelease', () => { await expect(waitForPushRelease(20)).resolves.toBe(false); expect(mockMessaging.deleteToken).not.toHaveBeenCalled(); - // Let the release finish so it does not hold up the next test. stuck.resolve(); await release; }); +}); - it('keeps the token when a registration read it while the release was stuck', async () => { - const stuck = deferred(); - saveMock.mockImplementation(() => stuck.promise); +describe('beginPushRegistration', () => { + it('reads the token once releases are done and the account is still wanted', async () => { + const registration = await beginPushRegistration({ stillWanted: always }); + expect(registration?.token).toBe('fcm-token'); + registration!.finish(); + }); - const release = disablePushRegistrations([{ username: 'alice', accessToken: 'code' }], { + it('gives up when the account left while a queued release ran', async () => { + // Logout A (slow), login B waits, logout B queues a second release before the first ends. + const first = deferred(); + saveMock.mockImplementationOnce(() => first.promise); + let signedIn = true; + const events: string[] = []; + mockMessaging.getToken.mockImplementation(async () => { + events.push('token read'); + return 'fcm-token'; + }); + mockMessaging.deleteToken.mockImplementation(async () => { + events.push('token deleted'); + }); + + const releaseA = disablePushRegistrations([{ username: 'alice', accessToken: 'a' }], { + deleteToken: true, + }); + const bob = beginPushRegistration({ stillWanted: () => signedIn }); + signedIn = false; + const releaseB = disablePushRegistrations([{ username: 'bob', accessToken: 'b' }], { deleteToken: true, }); - await expect(getRegistrationToken({ timeoutMs: 20 })).resolves.toBe('fcm-token'); - stuck.resolve(); - await release; - expect(mockMessaging.deleteToken).not.toHaveBeenCalled(); + first.resolve(); + await Promise.all([releaseA, releaseB]); + await expect(bob).resolves.toBeNull(); + // Only the two releases read the token, and both deleted it. + expect(events).toEqual(['token read', 'token deleted', 'token read', 'token deleted']); }); - it('still deletes the token for a release that starts after a registration', async () => { - await getRegistrationToken({ timeoutMs: 20 }); + it('finishes its in-flight mark when the token cannot be read', async () => { + mockMessaging.getToken.mockRejectedValueOnce(new Error('SERVICE_NOT_AVAILABLE')); + await expect(beginPushRegistration({ stillWanted: always })).rejects.toThrow( + 'SERVICE_NOT_AVAILABLE', + ); - await disablePushRegistrations([{ username: 'alice', accessToken: 'code' }], { - deleteToken: true, + // A release does not wait for the failed registration. + await disablePushRegistrations([{ username: 'alice', accessToken: 'a' }], { + deleteToken: false, + registrationWaitMs: 10_000, }); - expect(mockMessaging.deleteToken).toHaveBeenCalledTimes(1); + expect(saveMock).toHaveBeenCalledTimes(1); }); - it('asks to register again once a release that outlived the wait settles', async () => { + it('asks to register again once every release has settled after a timed-out wait', async () => { const stuck = deferred(); - saveMock.mockImplementation(() => stuck.promise); + const queued = deferred(); + saveMock + .mockImplementationOnce(() => stuck.promise) + .mockImplementationOnce(() => queued.promise); const registerAgain = jest.fn(); - const release = disablePushRegistrations([{ username: 'alice', accessToken: 'code' }], { + const releaseA = disablePushRegistrations([{ username: 'alice', accessToken: 'a' }], { deleteToken: true, }); - await getRegistrationToken({ timeoutMs: 20, onReleaseSettled: registerAgain }); + const registration = await beginPushRegistration({ + stillWanted: always, + onReleaseSettled: registerAgain, + timeoutMs: 20, + }); + // The wait gave up: the registration proceeds, its retry waits for the releases. + expect(registration?.token).toBe('fcm-token'); + registration!.finish(); + const releaseB = disablePushRegistrations([{ username: 'bob', accessToken: 'b' }], { + deleteToken: false, + }); + + stuck.resolve(); + await releaseA; await settle(); - // The disable request is still out: registering again now could still lose to it. expect(registerAgain).not.toHaveBeenCalled(); - stuck.resolve(); - await release; + queued.resolve(); + await releaseB; await settle(); expect(registerAgain).toHaveBeenCalledTimes(1); }); - it('does not ask to register again when the release finished within the wait', async () => { + it('does not ask to register again when the wait did not time out', async () => { const quick = deferred(); - saveMock.mockImplementation(() => quick.promise); + saveMock.mockImplementationOnce(() => quick.promise); const registerAgain = jest.fn(); - const release = disablePushRegistrations([{ username: 'alice', accessToken: 'code' }], { + const release = disablePushRegistrations([{ username: 'alice', accessToken: 'a' }], { deleteToken: false, }); - const reading = getRegistrationToken({ timeoutMs: 10_000, onReleaseSettled: registerAgain }); + const pending = beginPushRegistration({ + stillWanted: always, + onReleaseSettled: registerAgain, + timeoutMs: 10_000, + }); quick.resolve(); - await Promise.all([release, reading]); - await settle(); - - expect(registerAgain).not.toHaveBeenCalled(); - }); - - it('does not ask to register again when nothing was being released', async () => { - const registerAgain = jest.fn(); - await getRegistrationToken({ timeoutMs: 10_000, onReleaseSettled: registerAgain }); + await release; + const registration = await pending; + registration!.finish(); await settle(); expect(registerAgain).not.toHaveBeenCalled(); }); - - it('survives a request that throws synchronously', async () => { - saveMock.mockImplementationOnce(() => { - throw new Error('synchronous failure'); - }); - - await disablePushRegistrations( - [ - { username: 'alice', accessToken: 'a' }, - { username: 'bob', accessToken: 'b' }, - ], - { deleteToken: true }, - ); - expect(saveMock).toHaveBeenCalledTimes(2); - expect(mockMessaging.deleteToken).toHaveBeenCalledTimes(1); - - // The chain stays usable for the next release and for waiting registrations. - await disablePushRegistrations([{ username: 'carol', accessToken: 'c' }], { - deleteToken: false, - }); - expect(saveMock).toHaveBeenCalledTimes(3); - await expect(waitForPushRelease(20)).resolves.toBe(true); - }); - - it('keeps the chain usable after a release fails unexpectedly', async () => { - // A malformed call throws inside the release itself. - await expect( - disablePushRegistrations(null as unknown as PushAccount[], { deleteToken: true }), - ).resolves.toBeUndefined(); - - await expect(waitForPushRelease(20)).resolves.toBe(true); - await disablePushRegistrations([{ username: 'alice', accessToken: 'a' }], { - deleteToken: true, - }); - expect(mockMessaging.deleteToken).toHaveBeenCalledTimes(1); - }); }); diff --git a/src/utils/pushRegistration.ts b/src/utils/pushRegistration.ts index 00580588b6..1b150be928 100644 --- a/src/utils/pushRegistration.ts +++ b/src/utils/pushRegistration.ts @@ -74,17 +74,55 @@ export const getPushAccounts = ( accessToken: decryptAccessToken(account?.local?.accessToken, encAppPin), })); -// The deregistration in progress, if any. Releases run one after another, and a -// registration waits for them (see waitForPushRelease). +// Deregistrations (releases) run one after another on this chain, and registrations +// wait for it. It never rejects. let pendingRelease: Promise = Promise.resolve(); -// Counts the times a registration has read the device token (getRegistrationToken). -let registrationTokenReads = 0; +// Registrations past their wait whose request has not settled yet. A release waits for +// them before it sends its disable requests. +const registrationsInFlight = new Set>(); export const PUSH_RELEASE_WAIT_MS = 30 * 1000; -const releasePushRegistrations = async (accounts: PushAccount[], deleteToken: boolean) => { - const readsAtStart = registrationTokenReads; +// Cold start and reconnect both register every account; within this window the second +// pass is skipped. +export const PUSH_REGISTRATION_DEDUPE_MS = 60 * 1000; + +/** Resolves to true when `promise` settles within `ms`, false otherwise. */ +const settlesWithin = async (promise: Promise, ms: number): Promise => { + let timer: ReturnType | undefined; + const timeout = new Promise((resolve) => { + timer = setTimeout(() => resolve(false), Math.max(ms, 0)); + }); + try { + return await Promise.race([ + promise.then( + () => true, + () => true, + ), + timeout, + ]); + } finally { + clearTimeout(timer); + } +}; + +interface ReleaseOptions { + deleteToken: boolean; + /** Called after the token was deleted, to register the accounts still signed in. */ + onTokenReplaced?: () => void; + /** How long to wait for registrations already in flight. */ + registrationWaitMs?: number; +} + +const releasePushRegistrations = async ( + accounts: PushAccount[], + { deleteToken, onTokenReplaced, registrationWaitMs = PUSH_RELEASE_WAIT_MS }: ReleaseOptions, +) => { + // A registration already on its way must land first, or it could switch a departing + // account back on after its disable. + await settlesWithin(Promise.all(registrationsInFlight), registrationWaitMs); + let token: string; try { token = await getMessaging().getToken(); @@ -94,40 +132,52 @@ const releasePushRegistrations = async (accounts: PushAccount[], deleteToken: bo return; } - await Promise.all( - accounts - .filter((account) => account.username && account.accessToken) - .map((account) => - // Inside then(), so even a synchronous throw becomes this account's failure - // and cannot skip the other accounts or the token delete. - Promise.resolve() - .then(() => - saveNotificationSetting( - account.accessToken, - account.username, - getPushSystem(), - 0, - [], - token, - ), - ) - .catch((err) => { + const disabled = await Promise.all( + accounts.map((account) => { + if (!account.username || !account.accessToken) { + return Promise.resolve(false); + } + // Inside then(), so even a synchronous throw becomes this account's failure. + return Promise.resolve() + .then(() => + saveNotificationSetting( + account.accessToken, + account.username, + getPushSystem(), + 0, + [], + token, + ), + ) + .then( + () => true, + (err) => { console.warn('Failed to disable push notifications for', account.username, err); - }), - ), + return false; + }, + ); + }), ); - if (deleteToken) { - if (registrationTokenReads !== readsAtStart) { - // A registration stopped waiting (timeout) and registered this token meanwhile. - // Deleting it now would leave that registration pointing at a dead token. - console.warn('Push token was registered during deregistration, keeping it'); - return; - } + // With no account left the token goes. With accounts left it stays, unless a row could + // not be disabled: deleting the token is then the only way to stop that account's + // pushes, and the accounts still signed in register again with the new token. + if (!deleteToken && disabled.every(Boolean)) { + return; + } + + try { + await getMessaging().deleteToken(); + } catch (err) { + console.warn('Failed to delete push token', err); + return; + } + + if (onTokenReplaced) { try { - await getMessaging().deleteToken(); + onTokenReplaced(); } catch (err) { - console.warn('Failed to delete push token', err); + console.warn('Failed to register after replacing the push token', err); } } }; @@ -135,24 +185,22 @@ const releasePushRegistrations = async (accounts: PushAccount[], deleteToken: bo /** * Turns push off for accounts leaving this device. * - * Each account's row is disabled with the current FCM token. When no account is left - * on the device, the token itself is deleted afterwards: the backend then gets - * "unregistered" for it and stops sending, even for a row this call could not - * disable (for example an account whose access token no longer decrypts). The - * delete must come after the requests, because deleting first makes the next - * `getToken()` mint a new token and the requests would disable the wrong rows. + * Each account's row is disabled with the current FCM token, after any registration + * already in flight has landed. The token is then deleted when no account is left, or + * when a row could not be disabled: the backend then gets "unregistered" for it and + * stops sending. The delete must come after the requests, because deleting first makes + * the next `getToken()` mint a new token and the requests would disable the wrong rows. * * Callers do not need to wait: the logout itself should not hang on the network. - * Registrations wait instead, through waitForPushRelease. + * Registrations wait instead, through beginPushRegistration. */ export const disablePushRegistrations = ( accounts: PushAccount[], - { deleteToken }: { deleteToken: boolean }, + options: ReleaseOptions, ): Promise => { const release = pendingRelease - .then(() => releasePushRegistrations(accounts, deleteToken)) - // Never rejects: releasePushRegistrations catches its own failures, and the chain - // must stay usable for the next release and for waiting registrations. + .then(() => releasePushRegistrations(accounts, options)) + // Never rejects, so the chain stays usable for later releases and registrations. .catch((err) => { console.warn('Push deregistration failed', err); }); @@ -162,44 +210,73 @@ export const disablePushRegistrations = ( /** * Resolves to true once no deregistration is in progress, or to false after `timeoutMs`. - * - * A login right after the last account logged out would otherwise read the token - * that the pending release is about to delete, and register a dead token. Waiting - * lets the release finish, so `getToken()` returns the new token. The timeout keeps a - * stuck release (a native call that never settles) from blocking registration. + * Releases queued while it waits count too: it returns only when the chain has stopped + * growing. */ export const waitForPushRelease = async (timeoutMs = PUSH_RELEASE_WAIT_MS): Promise => { - let timer: ReturnType | undefined; - const timeout = new Promise((resolve) => { - timer = setTimeout(() => resolve(false), timeoutMs); - }); - try { - return await Promise.race([pendingRelease.then(() => true), timeout]); - } finally { - clearTimeout(timer); + const release = pendingRelease; + const startedAt = Date.now(); + if (!(await settlesWithin(release, timeoutMs))) { + return false; + } + return release === pendingRelease + ? true + : waitForPushRelease(timeoutMs - (Date.now() - startedAt)); +}; + +const whenReleasesSettle = async (): Promise => { + const release = pendingRelease; + await release; + if (release !== pendingRelease) { + await whenReleasesSettle(); } }; +export interface PushRegistration { + token: string; + /** Call once the registration request has settled, whatever its outcome. */ + finish: () => void; +} + /** - * Reads the device token for a registration, after any deregistration in progress. + * Starts a registration: waits for deregistrations, checks the account is still wanted, + * then reads the device token. + * + * Resolves to null when `stillWanted()` is false after the wait (the account left while + * it waited). Otherwise the caller sends its request and calls `finish()`; releases that + * start meanwhile wait for it before disabling anything. * - * When the wait times out, the release is still running: - * - it no longer deletes the token this registration read; - * - its disable requests are already out and may reach the backend AFTER this - * registration, switching the row back off. They cannot be recalled, so - * `onReleaseSettled` is called once the release has settled, for the caller to - * register again. + * When the wait times out, releases are still running. Their disable requests may reach + * the backend after this registration, and a release may replace the token it read. + * Neither can be recalled, so `onReleaseSettled` is called once every release has + * settled, for the caller to register again. */ -export const getRegistrationToken = async ({ - timeoutMs = PUSH_RELEASE_WAIT_MS, +export const beginPushRegistration = async ({ + stillWanted, onReleaseSettled, -}: { timeoutMs?: number; onReleaseSettled?: () => void } = {}) => { - const release = pendingRelease; + timeoutMs = PUSH_RELEASE_WAIT_MS, +}: { + stillWanted: () => boolean; + onReleaseSettled?: () => void; + timeoutMs?: number; +}): Promise => { const settled = await waitForPushRelease(timeoutMs); - registrationTokenReads += 1; + if (!stillWanted()) { + return null; + } + + let resolveInFlight: () => void = () => undefined; + const inFlight = new Promise((resolve) => { + resolveInFlight = resolve; + }); + registrationsInFlight.add(inFlight); + const finish = () => { + registrationsInFlight.delete(inFlight); + resolveInFlight(); + }; if (!settled && onReleaseSettled) { - release.then(() => { + whenReleasesSettle().then(() => { try { onReleaseSettled(); } catch (err) { @@ -208,5 +285,11 @@ export const getRegistrationToken = async ({ }); } - return getMessaging().getToken(); + try { + const token = await getMessaging().getToken(); + return { token, finish }; + } catch (err) { + finish(); + throw err; + } };