Create a flow to replace the user's two-factor device #79266
Conversation
…ling for device replacement flow
melvin-bot
bot
requested review from
brunovjk and
heyjennahay
and removed request for
a team
This comment has been minimized.
This comment has been minimized.
|
Hi @brunovjk! Feel free to give this a first pass now if you like, but it won't be testable til the backend PRs go out. I'll remove the HOLD and ping you when that happens |
|
CC @trjExpensify since you were in the slack thread |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de2f21b28b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
src/pages/settings/Security/TwoFactorAuth/ReplaceDeviceVerifyOldPage.tsx
Show resolved
Hide resolved
|
I think the secret code with the copy button should be styled more like our input rows: 
And reduce the spacing between the code itself and the text above it as it's a bit too big. Otherwise this is pretty good. cc @dannymcclain for 👀 as well. I could see us simplifying it further, but I think it works relatively well as is |
|
Agree with Jon's feedback. One question about our standard copy-to-clipboard pattern though: on mobile that translates to a long press, @dubielzyk-expensify do you think that's too hidden for something like this on mobile? Most of, if not all of, the other fields people are copying are very optional copy actions—this feels a bit more critical so I can see the argument for an exposed, always visible button here. Thoughts? |
|
I lifted the secret-code-copy-button component for this flow from the 2FA enablement flow - here's what that looks like on main today 2fa.enable.main.mp4I suspect the reason that it's already different from our typical input rows is for exactly the reason @dannymcclain mentioned - copy here is a primary necessary action, especially on mobile (since it's not possible to scan the qr code unless you have a second phone handy...). Here's what the current 2fa enablement flow looks like today on mobile 
I'm happy to change how it looks but I do feel like we should have these two flows match |
|
Ok @chuckdries hear me out - can we add a container around the secret authenticator key and button like we have on the 2FA codes? And also use Expensify Mono for the secret key text? Something like one of these: 
... Also, if it's not too much to ask, can we please fix that copy button? (The icon is on the wrong side in real life but is correct in my mocks above.) |
|
👍 |
|
Love it. Great suggestion @dannymcclain and nice work @chuckdries ! #teamwork |
|
Let's gooooo! |
chuckdries
changed the title
|
@brunovjk this can now be tested against staging and prod APIs! |
This comment was marked as outdated.
This comment was marked as outdated.
Reviewer Checklist
Screenshots/VideosAndroid: HybridApp79266_android_native.movAndroid: mWeb Chrome79266_android_web.moviOS: HybridApp79266_ios_native.moviOS: mWeb Safari79266_ios_web.movMacOS: Chrome / Safari79266_web_chrome.mov |
|
The changes seem good to me (screenshots added above), however we have some warnings related to ESLint and Typescript checks. Another thing @chuckdries, did you ask for confirmation for the translations? Thank you. |
|
@brunovjk I got confirmation on the translations! I'll take a look at the lint and type stuff today |
🦜 Polyglot Parrot! 🦜Squawk! Looks like you added some shiny new English strings. Allow me to parrot them back to you in other tongues: View the translation diffdiff --git a/src/languages/de.ts b/src/languages/de.ts
index ee306448..c0b9f17d 100644
--- a/src/languages/de.ts
+++ b/src/languages/de.ts
@@ -1992,6 +1992,12 @@ const translations: TranslationDeepObject<typeof en> = {
twoFactorAuthIsRequiredCompany: 'Ihr Unternehmen erfordert eine Zwei-Faktor-Authentifizierung.',
twoFactorAuthCannotDisable: '2FA kann nicht deaktiviert werden',
twoFactorAuthRequired: 'Für Ihre Xero-Verbindung ist eine Zwei-Faktor-Authentifizierung (2FA) erforderlich und sie kann nicht deaktiviert werden.',
+ replaceDevice: 'Gerät ersetzen',
+ replaceDeviceTitle: 'Zwei-Faktor-Gerät ersetzen',
+ verifyOldDeviceTitle: 'Altes Gerät verifizieren',
+ verifyOldDeviceDescription: 'Gib den sechsstelligen Code aus deiner aktuellen Authenticator-App ein, um zu bestätigen, dass du Zugriff darauf hast.',
+ verifyNewDeviceTitle: 'Neues Gerät einrichten',
+ verifyNewDeviceDescription: 'Scanne den QR-Code mit deinem neuen Gerät und gib dann den Code ein, um die Einrichtung abzuschließen.',
},
recoveryCodeForm: {
error: {
diff --git a/src/languages/fr.ts b/src/languages/fr.ts
index 697db250..9f14a2b7 100644
--- a/src/languages/fr.ts
+++ b/src/languages/fr.ts
@@ -1997,6 +1997,12 @@ const translations: TranslationDeepObject<typeof en> = {
twoFactorAuthIsRequiredCompany: 'Votre entreprise exige l’authentification à deux facteurs.',
twoFactorAuthCannotDisable: 'Impossible de désactiver la 2FA',
twoFactorAuthRequired: 'L’authentification à deux facteurs (2FA) est requise pour votre connexion Xero et ne peut pas être désactivée.',
+ replaceDevice: 'Remplacer l’appareil',
+ replaceDeviceTitle: 'Remplacer l’appareil d’authentification à deux facteurs',
+ verifyOldDeviceTitle: 'Vérifier l’ancien appareil',
+ verifyOldDeviceDescription: 'Saisissez le code à six chiffres depuis votre application d’authentification actuelle pour confirmer que vous y avez accès.',
+ verifyNewDeviceTitle: 'Configurer un nouvel appareil',
+ verifyNewDeviceDescription: 'Scannez le code QR avec votre nouvel appareil, puis saisissez le code pour terminer la configuration.',
},
recoveryCodeForm: {
error: {
diff --git a/src/languages/it.ts b/src/languages/it.ts
index 799e8b53..12f4737f 100644
--- a/src/languages/it.ts
+++ b/src/languages/it.ts
@@ -1989,6 +1989,12 @@ const translations: TranslationDeepObject<typeof en> = {
twoFactorAuthIsRequiredCompany: 'La tua azienda richiede l’autenticazione a due fattori.',
twoFactorAuthCannotDisable: 'Impossibile disattivare l’autenticazione a due fattori (2FA)',
twoFactorAuthRequired: "Per la tua connessione a Xero è richiesta l'autenticazione a due fattori (2FA) e non può essere disattivata.",
+ replaceDevice: 'Sostituisci dispositivo',
+ replaceDeviceTitle: "Sostituisci dispositivo per l'autenticazione a due fattori",
+ verifyOldDeviceTitle: 'Verifica dispositivo precedente',
+ verifyOldDeviceDescription: 'Inserisci il codice a sei cifre dalla tua attuale app di autenticazione per confermare di avere accesso ad essa.',
+ verifyNewDeviceTitle: 'Configura nuovo dispositivo',
+ verifyNewDeviceDescription: 'Scansiona il codice QR con il tuo nuovo dispositivo, quindi inserisci il codice per completare la configurazione.',
},
recoveryCodeForm: {
error: {
diff --git a/src/languages/ja.ts b/src/languages/ja.ts
index 7016a818..b1ef3f45 100644
--- a/src/languages/ja.ts
+++ b/src/languages/ja.ts
@@ -1985,6 +1985,12 @@ const translations: TranslationDeepObject<typeof en> = {
twoFactorAuthIsRequiredCompany: 'あなたの会社では、二要素認証が必須です。',
twoFactorAuthCannotDisable: '2要素認証を無効にできません',
twoFactorAuthRequired: 'Xero 連携には二要素認証（2FA）が必須であり、無効にすることはできません。',
+ replaceDevice: 'デバイスを交換',
+ replaceDeviceTitle: '2 要素認証デバイスを交換',
+ verifyOldDeviceTitle: '古いデバイスを確認',
+ verifyOldDeviceDescription: '現在使用している認証アプリに表示されている6桁のコードを入力して、アクセスできることを確認してください。',
+ verifyNewDeviceTitle: '新しいデバイスをセットアップ',
+ verifyNewDeviceDescription: '新しいデバイスでQRコードをスキャンし、その後コードを入力して設定を完了してください。',
},
recoveryCodeForm: {
error: {
diff --git a/src/languages/nl.ts b/src/languages/nl.ts
index ba7e9f9f..a424cf59 100644
--- a/src/languages/nl.ts
+++ b/src/languages/nl.ts
@@ -1987,6 +1987,12 @@ const translations: TranslationDeepObject<typeof en> = {
twoFactorAuthIsRequiredCompany: 'Je bedrijf vereist tweefactorauthenticatie.',
twoFactorAuthCannotDisable: 'Kan 2FA niet uitschakelen',
twoFactorAuthRequired: 'Tweefactorauthenticatie (2FA) is vereist voor je Xero-verbinding en kan niet worden uitgeschakeld.',
+ replaceDevice: 'Apparaat vervangen',
+ replaceDeviceTitle: 'Twee-factorapparaat vervangen',
+ verifyOldDeviceTitle: 'Oud apparaat verifiëren',
+ verifyOldDeviceDescription: 'Voer de zescijferige code uit je huidige authenticator-app in om te bevestigen dat je er toegang toe hebt.',
+ verifyNewDeviceTitle: 'Nieuw apparaat instellen',
+ verifyNewDeviceDescription: 'Scan de QR-code met je nieuwe apparaat en voer vervolgens de code in om de installatie te voltooien.',
},
recoveryCodeForm: {
error: {
diff --git a/src/languages/pl.ts b/src/languages/pl.ts
index 699d2ed3..0af610e6 100644
--- a/src/languages/pl.ts
+++ b/src/languages/pl.ts
@@ -1985,6 +1985,12 @@ const translations: TranslationDeepObject<typeof en> = {
twoFactorAuthIsRequiredCompany: 'Twoja firma wymaga uwierzytelniania dwuskładnikowego.',
twoFactorAuthCannotDisable: 'Nie można wyłączyć 2FA',
twoFactorAuthRequired: 'Dwuskładnikowe uwierzytelnianie (2FA) jest wymagane dla Twojego połączenia z Xero i nie może zostać wyłączone.',
+ replaceDevice: 'Zastąp urządzenie',
+ replaceDeviceTitle: 'Wymień urządzenie uwierzytelniania dwuskładnikowego',
+ verifyOldDeviceTitle: 'Zweryfikuj stare urządzenie',
+ verifyOldDeviceDescription: 'Wprowadź sześciocyfrowy kod z bieżącej aplikacji uwierzytelniającej, aby potwierdzić, że masz do niej dostęp.',
+ verifyNewDeviceTitle: 'Skonfiguruj nowe urządzenie',
+ verifyNewDeviceDescription: 'Zeskanuj kod QR swoim nowym urządzeniem, a następnie wprowadź kod, aby zakończyć konfigurację.',
},
recoveryCodeForm: {
error: {
diff --git a/src/languages/pt-BR.ts b/src/languages/pt-BR.ts
index f45bb674..1da194fd 100644
--- a/src/languages/pt-BR.ts
+++ b/src/languages/pt-BR.ts
@@ -1984,6 +1984,12 @@ const translations: TranslationDeepObject<typeof en> = {
twoFactorAuthIsRequiredCompany: 'Sua empresa exige autenticação em duas etapas.',
twoFactorAuthCannotDisable: 'Não é possível desativar a 2FA',
twoFactorAuthRequired: 'A autenticação de dois fatores (2FA) é obrigatória para sua conexão com o Xero e não pode ser desativada.',
+ replaceDevice: 'Substituir dispositivo',
+ replaceDeviceTitle: 'Substituir dispositivo de autenticação em duas etapas',
+ verifyOldDeviceTitle: 'Verificar dispositivo antigo',
+ verifyOldDeviceDescription: 'Insira o código de seis dígitos do seu aplicativo autenticador atual para confirmar que você tem acesso a ele.',
+ verifyNewDeviceTitle: 'Configurar novo dispositivo',
+ verifyNewDeviceDescription: 'Escaneie o código QR com seu novo dispositivo e, em seguida, insira o código para concluir a configuração.',
},
recoveryCodeForm: {
error: {
diff --git a/src/languages/zh-hans.ts b/src/languages/zh-hans.ts
index 80fa0285..562d2423 100644
--- a/src/languages/zh-hans.ts
+++ b/src/languages/zh-hans.ts
@@ -1957,6 +1957,12 @@ const translations: TranslationDeepObject<typeof en> = {
twoFactorAuthIsRequiredCompany: '您的公司要求使用双重身份验证。',
twoFactorAuthCannotDisable: '无法禁用双重身份验证',
twoFactorAuthRequired: '您的 Xero 连接需要启用双重身份验证 (2FA)，且无法将其禁用。',
+ replaceDevice: '更换设备',
+ replaceDeviceTitle: '更换双重验证设备',
+ verifyOldDeviceTitle: '验证旧设备',
+ verifyOldDeviceDescription: '请输入您当前身份验证器应用中的六位数验证码，以确认您可以访问该应用。',
+ verifyNewDeviceTitle: '设置新设备',
+ verifyNewDeviceDescription: '使用新设备扫描二维码，然后输入代码以完成设置。',
},
recoveryCodeForm: {
error: {
Note You can apply these changes to your branch by copying the patch to your clipboard, then running |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 70c5c93ec6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
src/pages/settings/Security/TwoFactorAuth/ReplaceDeviceVerifyNewPage.tsx
Outdated
Show resolved
Hide resolved
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 93430046cb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| useEffect(() => { | ||
| if (!account?.twoFactorAuthSecretKey) { | ||
| return; | ||
| } | ||
| Navigation.navigate(ROUTES.SETTINGS_2FA_REPLACE_VERIFY_NEW); |
There was a problem hiding this comment.
This effect treats any existing twoFactorAuthSecretKey as proof that the old-device verification just succeeded. If a user previously started the replace flow and then closed the app (or otherwise left mid‑flow), that secret key remains persisted in Onyx, so reopening “Replace device” will skip the old-device verification and force them into verify‑new with a stale secret. If the backend invalidates pending secrets or the user wants to restart, they’ll be stuck in the wrong step. Consider clearing the secret when entering the replace flow (e.g., via the new clearTwoFactorAuthSecretKey) or gating the redirect on an explicit “replacement in progress” flag.
Useful? React with 👍 / 👎.
Explanation of Change
This PR introduces a new flow that allows users to change their registered two-factor authentication device in-place, rather than disabling the feature then re-enabling it. I tried to keep the changeset relatively small and follow the path of least resistance.
At this stage I'm looking for feedback from design and product. PR on hold until necessary backend PRs are mergedAPIs are now available!Video of first implementation - updated implementation videos in screenshots/videos section at bottom
2fa.replace.device.flow.2.mp4
Fixed Issues
$ https://github.com/Expensify/Expensify/issues/583049
PROPOSAL:
Tests
Happy path
Invalid codes are rejected
Offline tests
N/A - Two-factor authentication screens are unavailable offline
QA Steps
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps./** comment above it */thisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)Screenshots/Videos
Android: Native
2fa.device.change.android.mp4
iOS: Native
2fa.device.change.ios.mp4
iOS: mWeb Safari
2fa.device.change.mobile.safari.mp4
MacOS: Chrome / Safari
2fa.device.change.desktop.chrome.mp4