From c09cf92809e992a7dd1b8c02b42cb37c9a848649 Mon Sep 17 00:00:00 2001 From: Jon Tzeng Date: Thu, 6 Feb 2025 16:42:49 -0800 Subject: [PATCH] Refactor otp/2fa modal and `dataStore` - Isolate the modals to their own file - Create a hook to ensure changes to otp disk settings are picked up by the `NotificationService` - Pull all logic for otp reminder notification state into `NotificationService` --- .../notification/NotificationView.tsx | 5 +- .../scenes/NotificationCenterScene.tsx | 5 +- .../services/NotificationService.ts | 34 +++--- src/util/otpReminder.tsx | 112 ++++++++---------- src/util/otpUtils.ts | 75 ++++++++++++ yarn.lock | 8 +- 6 files changed, 148 insertions(+), 91 deletions(-) create mode 100644 src/util/otpUtils.ts diff --git a/src/components/notification/NotificationView.tsx b/src/components/notification/NotificationView.tsx index f2571dc4260..8e92c26ac4d 100644 --- a/src/components/notification/NotificationView.tsx +++ b/src/components/notification/NotificationView.tsx @@ -16,7 +16,7 @@ import { config } from '../../theme/appConfig' import { useDispatch, useSelector } from '../../types/reactRedux' import { NavigationBase } from '../../types/routerTypes' import { getThemedIconUri } from '../../util/CdnUris' -import { getOtpReminderModal } from '../../util/otpReminder' +import { showOtpReminderModal } from '../../util/otpReminder' import { openBrowserUri } from '../../util/WebUtils' import { EdgeAnim, fadeIn, fadeOut } from '../common/EdgeAnim' import { styled } from '../hoc/styled' @@ -83,8 +83,7 @@ const NotificationViewComponent = (props: Props) => { }) const handleOtpReminderPress = useHandler(async () => { await handleOtpReminderClose() - const otpReminderModal = await getOtpReminderModal(account) - if (otpReminderModal != null) await otpReminderModal() + await showOtpReminderModal(account) }) const handleLayout = useHandler((event: LayoutChangeEvent) => { diff --git a/src/components/scenes/NotificationCenterScene.tsx b/src/components/scenes/NotificationCenterScene.tsx index d50ebc58961..0aac8af2095 100644 --- a/src/components/scenes/NotificationCenterScene.tsx +++ b/src/components/scenes/NotificationCenterScene.tsx @@ -11,7 +11,7 @@ import { config } from '../../theme/appConfig' import { useDispatch, useSelector } from '../../types/reactRedux' import { EdgeAppSceneProps, NavigationBase } from '../../types/routerTypes' import { getThemedIconUri } from '../../util/CdnUris.ts' -import { getOtpReminderModal } from '../../util/otpReminder.tsx' +import { showOtpReminderModal } from '../../util/otpReminder.tsx' import { openBrowserUri } from '../../util/WebUtils.ts' import { SceneWrapper } from '../common/SceneWrapper' import { SectionHeader } from '../common/SectionHeader' @@ -43,8 +43,7 @@ export const NotificationCenterScene = (props: Props) => { }) const handleOtpReminderPress = useHandler(async () => { - const otpReminderModal = await getOtpReminderModal(account) - if (otpReminderModal != null) await otpReminderModal() + await showOtpReminderModal(account) }) const handle2FaEnabledPress = useHandler(async () => { diff --git a/src/components/services/NotificationService.ts b/src/components/services/NotificationService.ts index e07b84ce19f..40fd6c6a141 100644 --- a/src/components/services/NotificationService.ts +++ b/src/components/services/NotificationService.ts @@ -2,11 +2,10 @@ import { EdgeAccount } from 'edge-core-js' import { getLocalAccountSettings, useAccountSettings, writeAccountNotifInfo } from '../../actions/LocalSettingsActions' import { useAsyncEffect } from '../../hooks/useAsyncEffect.ts' -import { useAsyncValue } from '../../hooks/useAsyncValue.ts' import { useWatch } from '../../hooks/useWatch.ts' import { useSelector } from '../../types/reactRedux.ts' import { asNotifInfo } from '../../types/types.ts' -import { getOtpReminderModal } from '../../util/otpReminder.tsx' +import { OTP_REMINDER_MILLISECONDS, useOtpSettings } from '../../util/otpUtils.ts' const PRIORITY_NOTIFICATION_KEYS = ['ip2FaReminder', 'lightAccountReminder', 'otpReminder', 'pwReminder'] @@ -61,23 +60,26 @@ export const updateNotificationInfo = async ( */ export const NotificationService = (props: Props) => { const { account } = props + const { notifState, accountNotifDismissInfo } = useAccountSettings() + const otpSettings = useOtpSettings() - const isLightAccountReminder = account.id != null && account.username == null - const detectedTokensRedux = useSelector(state => state.core.enabledDetectedTokens) - const isPwReminder = useSelector(state => state.ui.passwordReminder.needsPasswordCheck) const wallets = useWatch(account, 'currencyWallets') const otpKey = useWatch(account, 'otpKey') - const isIp2faReminder = !isLightAccountReminder && otpKey == null && accountNotifDismissInfo != null && !accountNotifDismissInfo.ip2FaNotifShown - const [isOtpReminderModal] = useAsyncValue(async () => { - try { - const otpReminderModal = await getOtpReminderModal(account) - return otpReminderModal != null - } catch (error) { - return false - } - }, [account]) + const detectedTokensRedux = useSelector(state => state.core.enabledDetectedTokens) + const isPwReminder = useSelector(state => state.ui.passwordReminder.needsPasswordCheck) + + const isLightAccountReminder = account.id != null && account.username == null + + const isOtpReminder = + otpKey == null && + !isLightAccountReminder && + !otpSettings.dontAsk && + ((otpSettings.lastChecked == null && (account.created == null || Date.now() > account.created.valueOf() + OTP_REMINDER_MILLISECONDS)) || + (otpSettings.lastChecked != null && Date.now() > otpSettings.lastChecked.valueOf() + OTP_REMINDER_MILLISECONDS)) + + const isIp2faReminder = !isLightAccountReminder && otpKey == null && accountNotifDismissInfo != null && !accountNotifDismissInfo.ip2FaNotifShown // Update notification info with // 1. Date last received if transitioning from incomplete to complete @@ -98,13 +100,13 @@ export const NotificationService = (props: Props) => { await updateNotificationInfo(account, 'ip2FaReminder', isIp2faReminder) await updateNotificationInfo(account, 'lightAccountReminder', isLightAccountReminder) - await updateNotificationInfo(account, 'otpReminder', isOtpReminderModal ?? false) + await updateNotificationInfo(account, 'otpReminder', isOtpReminder ?? false) await updateNotificationInfo(account, 'pwReminder', isPwReminder) // cleanup: return () => {} }, - [isIp2faReminder, isLightAccountReminder, isOtpReminderModal, isPwReminder, wallets, detectedTokensRedux, notifState], + [isIp2faReminder, isLightAccountReminder, isOtpReminder, isPwReminder, wallets, detectedTokensRedux, notifState], 'NotificationServices' ) diff --git a/src/util/otpReminder.tsx b/src/util/otpReminder.tsx index d2ca4facea7..2fbadfde8dc 100644 --- a/src/util/otpReminder.tsx +++ b/src/util/otpReminder.tsx @@ -6,80 +6,62 @@ import { sprintf } from 'sprintf-js' import { ButtonsModal } from '../components/modals/ButtonsModal' import { Airship } from '../components/services/AirshipInstance' import { lstrings } from '../locales/strings' - -const OTP_REMINDER_MILLISECONDS = 7 * 24 * 60 * 60 * 1000 -const OTP_REMINDER_STORE_NAME = 'app.edge.login' -const OTP_REMINDER_KEY_NAME_LAST_OTP_CHECKED = 'lastOtpCheck' -const OTP_REMINDER_KEY_NAME_DONT_ASK = 'OtpDontAsk' +import { OTP_REMINDER_MILLISECONDS, readOtpSettings, writeOtpSettings } from './otpUtils' /** - * Check and return a potential 2fa reminder modal if needed to be shown onPress - * of its corresponding notification card. + * Return an otp reminder modal, with or without a "don't ask again" button, + * depending on if they've seen this before. */ -export async function getOtpReminderModal(account: EdgeAccount): Promise<(() => Promise) | undefined> { - const { otpKey, dataStore, username, created } = account - if (username == null || otpKey != null) return - const [dontAsk, lastOtpCheckString]: [string | null, string | null] = await Promise.all([ - dataStore.getItem(OTP_REMINDER_STORE_NAME, OTP_REMINDER_KEY_NAME_DONT_ASK).catch(() => null), - dataStore.getItem(OTP_REMINDER_STORE_NAME, OTP_REMINDER_KEY_NAME_LAST_OTP_CHECKED).catch(() => null) - ]) - if (dontAsk) return +export async function showOtpReminderModal(account: EdgeAccount): Promise { + const { created } = account + const { lastChecked } = await readOtpSettings(account) + Keyboard.dismiss() // Return a modal if we have never shown it before, and the account is old // enough: - const lastOtpCheck = lastOtpCheckString != null ? parseInt(lastOtpCheckString) : null - if (lastOtpCheck == null && (created == null || Date.now() > created.valueOf() + OTP_REMINDER_MILLISECONDS)) { - return async () => { - Keyboard.dismiss() - const result = await Airship.show<'yes' | 'no' | undefined>(bridge => ( - - )) - if (result === 'yes') { - await enableOtp(account) - } else { - await account.dataStore.setItem(OTP_REMINDER_STORE_NAME, OTP_REMINDER_KEY_NAME_LAST_OTP_CHECKED, Date.now().toString()) - } + if (lastChecked == null && (created == null || Date.now() > created.valueOf() + OTP_REMINDER_MILLISECONDS)) { + const result = await Airship.show<'yes' | 'no' | undefined>(bridge => ( + + )) + if (result === 'yes') { + await enableOtp(account) + } else { + await writeOtpSettings(account, { lastChecked: new Date(Date.now()) }) } - } - - // Return a modal with the "Don't ask again" button if we have waited long - // enough since the last time we showed a 2fa reminder modal: - if (lastOtpCheck != null && Date.now() > lastOtpCheck + OTP_REMINDER_MILLISECONDS) { - return async () => { - Keyboard.dismiss() - const result = await Airship.show<'enable' | 'cancel' | 'dontAsk' | undefined>(bridge => ( - - )) - if (result === 'enable') { - await enableOtp(account) - } else if (result === 'dontAsk') { - await account.dataStore.setItem(OTP_REMINDER_STORE_NAME, OTP_REMINDER_KEY_NAME_DONT_ASK, 'true') - } else { - await account.dataStore.setItem(OTP_REMINDER_STORE_NAME, OTP_REMINDER_KEY_NAME_LAST_OTP_CHECKED, Date.now().toString()) - } + } else { + // Return a modal with the "Don't ask again" button if we showed the first + // modal already: + const result = await Airship.show<'enable' | 'cancel' | 'dontAsk' | undefined>(bridge => ( + + )) + if (result === 'enable') { + await enableOtp(account) + } else if (result === 'dontAsk') { + await writeOtpSettings(account, { dontAsk: true }) + } else { + await writeOtpSettings(account, { lastChecked: new Date(Date.now()) }) } } - return undefined } /** diff --git a/src/util/otpUtils.ts b/src/util/otpUtils.ts new file mode 100644 index 00000000000..0d14f294de6 --- /dev/null +++ b/src/util/otpUtils.ts @@ -0,0 +1,75 @@ +import { asBoolean, asDate, asObject, asOptional } from 'cleaners' +import { EdgeAccount } from 'edge-core-js' +import React from 'react' +import { makeEvent } from 'yavent' + +const OTP_REMINDER_STORE_NAME = 'app.edge.login' +const OTP_REMINDER_KEY_NAME_LAST_OTP_CHECKED = 'lastOtpCheck' +const OTP_REMINDER_KEY_NAME_DONT_ASK = 'OtpDontAsk' + +export const OTP_REMINDER_MILLISECONDS = 7 * 24 * 60 * 60 * 1000 + +// Combined settings interface +export interface OtpSettings { + lastChecked: Date | null + dontAsk: boolean +} + +// Cleaner for the settings +export const asOtpSettings = asObject({ + lastChecked: asOptional(asDate, null), + dontAsk: asOptional(asBoolean, false) +}) + +// Local state management +let localOtpSettings: OtpSettings = { + lastChecked: null, + dontAsk: false +} + +// Event emitter for settings changes +const [watchOtpSettings, emitOtpSettings] = makeEvent() + +export const getOtpSettings = (): OtpSettings => localOtpSettings + +// React hook for accessing OTP settings +export function useOtpSettings(): OtpSettings { + const [, setOtpSettings] = React.useState(getOtpSettings()) + React.useEffect(() => watchOtpSettings(setOtpSettings), []) + return localOtpSettings +} + +// Read settings from disk +export async function readOtpSettings(account: EdgeAccount): Promise { + const [lastCheckedStr, dontAskStr] = await Promise.all([ + account.dataStore.getItem(OTP_REMINDER_STORE_NAME, OTP_REMINDER_KEY_NAME_LAST_OTP_CHECKED).catch(() => null), + account.dataStore.getItem(OTP_REMINDER_STORE_NAME, OTP_REMINDER_KEY_NAME_DONT_ASK).catch(() => null) + ]) + + const settings: OtpSettings = { + lastChecked: lastCheckedStr != null ? new Date(parseInt(lastCheckedStr)) : null, + dontAsk: dontAskStr === 'true' + } + + localOtpSettings = settings + return settings +} + +// Write settings to disk +export async function writeOtpSettings(account: EdgeAccount, settings: Partial): Promise { + const newSettings = { ...localOtpSettings, ...settings } + + const promises: Array> = [] + + if (settings.lastChecked !== undefined) { + promises.push(account.dataStore.setItem(OTP_REMINDER_STORE_NAME, OTP_REMINDER_KEY_NAME_LAST_OTP_CHECKED, settings.lastChecked?.toString() ?? '0')) + } + + if (settings.dontAsk !== undefined) { + promises.push(account.dataStore.setItem(OTP_REMINDER_STORE_NAME, OTP_REMINDER_KEY_NAME_DONT_ASK, settings.dontAsk?.toString() ?? 'false')) + } + + await Promise.all(promises) + localOtpSettings = newSettings + emitOtpSettings(newSettings) +} diff --git a/yarn.lock b/yarn.lock index 2d958e7119b..d8304eecf55 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2175,9 +2175,9 @@ randombytes "^2.1.0" text-encoding "0.7.0" -"@fioprotocol/fiosdk@git+https://github.com/jon-edge/fiosdk_typescript.git#92a0fb895b2ce57e5955cd30cb4b7fa2bcc66bf2": +"@fioprotocol/fiosdk@https://github.com/jon-edge/fiosdk_typescript.git#92a0fb895b2ce57e5955cd30cb4b7fa2bcc66bf2": version "1.9.0" - resolved "git+https://github.com/jon-edge/fiosdk_typescript.git#92a0fb895b2ce57e5955cd30cb4b7fa2bcc66bf2" + resolved "https://github.com/jon-edge/fiosdk_typescript.git#92a0fb895b2ce57e5955cd30cb4b7fa2bcc66bf2" dependencies: "@fioprotocol/fiojs" "1.0.1" "@types/text-encoding" "0.0.35" @@ -10566,9 +10566,9 @@ eyes@^0.1.8: resolved "https://registry.yarnpkg.com/eyes/-/eyes-0.1.8.tgz#62cf120234c683785d902348a800ef3e0cc20bc0" integrity sha512-GipyPsXO1anza0AOZdy69Im7hGFCNB7Y/NGjDlZGJ3GJJLtwNSb2vrzYrTYJRrRloVx7pl+bhUaTB8yiccPvFQ== -"eztz.js@git+https://github.com/EdgeApp/eztz.git#edge-fixes": +"eztz.js@https://github.com/EdgeApp/eztz.git#edge-fixes": version "0.0.1" - resolved "git+https://github.com/EdgeApp/eztz.git#eefa603586810c3d62f852e7f28cfe57c523b7db" + resolved "https://github.com/EdgeApp/eztz.git#eefa603586810c3d62f852e7f28cfe57c523b7db" dependencies: bignumber.js "^7.2.1" bip39 "^3.0.2"