-
Notifications
You must be signed in to change notification settings - Fork 3.5k
[No QA] Implement Revoke Access logic #80570
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@DylanDylann Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9156d6f45a
ℹ️ 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".
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 52c1e5cdd8
ℹ️ 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".
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
🦜 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 53680cd9..c0775d43 100644
--- a/src/languages/de.ts
+++ b/src/languages/de.ts
@@ -761,15 +761,14 @@ const translations: TranslationDeepObject<typeof en> = {
},
revoke: {
revoke: 'Widerrufen',
- title: 'Gesicht/Fingerabdruck & Passkeys',
+ title: 'Gesichts-/Fingerabdruck & Passkeys',
explanation:
- 'Die Gesichts-/Fingerabdruck- oder Passkey-Verifizierung ist auf einem oder mehreren Geräten aktiviert. Durch das Widerrufen des Zugriffs ist für die nächste Verifizierung auf jedem Gerät ein magischer Code erforderlich',
- confirmationPrompt: 'Bist du sicher? Du benötigst einen magischen Code für die nächste Verifizierung auf jedem Gerät',
+ 'Gesichts-/Fingerabdruck- oder Passkey-Verifizierung ist auf einem oder mehreren Geräten aktiviert. Das Widerrufen des Zugriffs erfordert für die nächste Verifizierung auf jedem Gerät einen magischen Code',
+ confirmationPrompt: 'Bist du sicher? Du brauchst einen magischen Code für die nächste Verifizierung auf jedem Gerät',
cta: 'Zugriff widerrufen',
- noDevices:
- 'Sie haben keine Geräte für die Gesichts-/Fingerabdruck- oder Passkey-Verifizierung registriert. Wenn Sie welche registrieren, können Sie den Zugriff hier widerrufen.',
+ noDevices: 'Du hast keine Geräte für die Gesichts-/Fingerabdruck- oder Passkey-Verifizierung registriert. Wenn du welche registrierst, kannst du deren Zugriff hier widerrufen.',
dismiss: 'Verstanden',
- error: 'Anfrage fehlgeschlagen. Versuche es später noch einmal.',
+ error: 'Anfrage fehlgeschlagen. Versuchen Sie es später erneut.',
},
},
validateCodeModal: {
diff --git a/src/languages/fr.ts b/src/languages/fr.ts
index 6ef20e53..5617dbc8 100644
--- a/src/languages/fr.ts
+++ b/src/languages/fr.ts
@@ -764,13 +764,13 @@ const translations: TranslationDeepObject<typeof en> = {
},
revoke: {
revoke: 'Révoquer',
- title: 'Reconnaissance faciale/empreinte digitale et clés d’accès',
+ title: 'Visage/empreinte digitale et clés d’accès',
explanation:
- 'La vérification par reconnaissance faciale/empreinte digitale ou par passkey est activée sur un ou plusieurs appareils. Révoquer l’accès nécessitera un code magique pour la prochaine vérification sur n’importe quel appareil',
- confirmationPrompt: 'Êtes-vous sûr(e) ? Vous aurez besoin d’un code magique pour la prochaine vérification sur n’importe quel appareil',
+ 'La vérification par reconnaissance faciale/empreinte digitale ou par passkey est activée sur un ou plusieurs appareils. La révocation de l’accès nécessitera un code magique pour la prochaine vérification sur n’importe quel appareil',
+ confirmationPrompt: 'Êtes-vous sûr ? Vous aurez besoin d’un code magique pour la prochaine vérification sur n’importe quel appareil',
cta: 'Révoquer l’accès',
noDevices:
- 'Vous n’avez enregistré aucun appareil pour la vérification par reconnaissance faciale/empreinte digitale ou par passkey. Si vous en enregistrez, vous pourrez révoquer cet accès ici.',
+ 'Vous n’avez aucun appareil enregistré pour la vérification par reconnaissance faciale/empreinte digitale ou par clé d’accès. Si vous en enregistrez, vous pourrez révoquer cet accès ici.',
dismiss: 'Compris',
error: 'La requête a échoué. Réessayez plus tard.',
},
diff --git a/src/languages/it.ts b/src/languages/it.ts
index fdc05e59..42716528 100644
--- a/src/languages/it.ts
+++ b/src/languages/it.ts
@@ -764,11 +764,11 @@ const translations: TranslationDeepObject<typeof en> = {
revoke: 'Revoca',
title: 'Riconoscimento facciale/impronta digitale e passkey',
explanation:
- 'La verifica tramite volto/impronta o passkey è abilitata su uno o più dispositivi. La revoca dell’accesso richiederà un codice magico per la prossima verifica su qualsiasi dispositivo',
- confirmationPrompt: 'Sei sicuro? Avrai bisogno di un codice magico per la prossima verifica su qualsiasi dispositivo',
+ 'La verifica tramite volto/impronta digitale o passkey è abilitata su uno o più dispositivi. La revoca dell’accesso richiederà un codice magico per la prossima verifica su qualsiasi dispositivo',
+ confirmationPrompt: 'Sei sicuro? Ti servirà un codice magico per la prossima verifica su qualsiasi dispositivo',
cta: 'Revoca accesso',
- noDevices: 'Non hai alcun dispositivo registrato per la verifica tramite volto/impronta digitale o passkey. Se ne registri uno, potrai revocare tale accesso da qui.',
- dismiss: 'Ho capito',
+ noDevices: 'Non hai alcun dispositivo registrato per la verifica con volto/impronta digitale o passkey. Se ne registri uno, potrai revocare tale accesso qui.',
+ dismiss: 'Capito',
error: 'Richiesta non riuscita. Riprova più tardi.',
},
},
diff --git a/src/languages/ja.ts b/src/languages/ja.ts
index 5b9d9fe9..23b6b952 100644
--- a/src/languages/ja.ts
+++ b/src/languages/ja.ts
@@ -761,11 +761,11 @@ const translations: TranslationDeepObject<typeof en> = {
},
revoke: {
revoke: '取り消す',
- title: '顔/指紋 & パスキー',
- explanation: '1 つ以上のデバイスで顔/指紋またはパスキーによる認証が有効になっています。アクセスを取り消すと、次回以降どのデバイスでも認証時にマジックコードが必要になります',
- confirmationPrompt: '本当によろしいですか?今後どのデバイスでも次回の認証にはマジックコードが必要になります',
+ title: '顔/指紋認証とパスキー',
+ explanation: '1 つ以上のデバイスで顔/指紋認証またはパスキー認証が有効になっています。アクセスを取り消すと、今後どのデバイスでも次回の認証時にマジックコードが必要になります',
+ confirmationPrompt: '本当によろしいですか?今後、どのデバイスでも次回の認証にはマジックコードが必要になります',
cta: 'アクセスを取り消す',
- noDevices: '顔認証/指紋認証またはパスキー認証に登録されているデバイスがありません。 \nいずれかを登録すると、そのアクセス権をここで取り消せるようになります。',
+ noDevices: '顔認証/指紋認証またはパスキー認証用に登録されているデバイスがありません。デバイスを登録すると、そのアクセスをここで取り消せるようになります。',
dismiss: '了解',
error: 'リクエストに失敗しました。後でもう一度お試しください。',
},
diff --git a/src/languages/nl.ts b/src/languages/nl.ts
index c2cba6d0..6ff63c34 100644
--- a/src/languages/nl.ts
+++ b/src/languages/nl.ts
@@ -762,14 +762,14 @@ const translations: TranslationDeepObject<typeof en> = {
},
revoke: {
revoke: 'Intrekken',
- title: 'Gezicht/vingerafdruk & passkeys',
+ title: 'Gezichts-/vingerafdruk & passkeys',
explanation:
- 'Gezichts-/vingerafdruk- of passkeys-verificatie is ingeschakeld op een of meer apparaten. Het intrekken van toegang vereist een magische code voor de volgende verificatie op elk apparaat',
+ 'Gezichts-/vingerafdruk- of passkeys-verificatie is ingeschakeld op een of meer apparaten. Toegang intrekken vereist een magische code voor de volgende verificatie op elk apparaat',
confirmationPrompt: 'Weet je het zeker? Je hebt een magische code nodig voor de volgende verificatie op elk apparaat',
cta: 'Toegang intrekken',
- noDevices: 'Je hebt geen apparaten geregistreerd voor gezichts-/vingerafdruk- of passkey-verificatie. Als je er een registreert, kun je die toegang hier intrekken.',
+ noDevices: 'Je hebt geen apparaten geregistreerd voor gezichts-/vingerafdruk- of passkeys-verificatie. Als je er een registreert, kun je die toegang hier intrekken.',
dismiss: 'Begrepen',
- error: 'Aanvraag mislukt. Probeer het later opnieuw.',
+ error: 'Verzoek mislukt. Probeer het later opnieuw.',
},
},
validateCodeModal: {
diff --git a/src/languages/pl.ts b/src/languages/pl.ts
index b92c7a74..85b76f5b 100644
--- a/src/languages/pl.ts
+++ b/src/languages/pl.ts
@@ -761,11 +761,11 @@ const translations: TranslationDeepObject<typeof en> = {
biometrics: 'Włącz szybką i bezpieczną weryfikację za pomocą twarzy lub odcisku palca. Bez haseł ani kodów.',
},
revoke: {
- revoke: 'Cofnij',
- title: 'Rozpoznawanie twarzy/odcisku palca i klucze dostępu',
+ revoke: 'Unieważnij',
+ title: 'Twarz/odcisk palca i klucze dostępowe',
explanation:
- 'Weryfikacja twarzą/odciskiem palca lub za pomocą klucza dostępu jest włączona na jednym lub więcej urządzeniach. Cofnięcie dostępu spowoduje, że przy następnej weryfikacji na dowolnym urządzeniu będzie wymagany kod magiczny',
- confirmationPrompt: 'Czy na pewno? Do kolejnego uwierzytelnienia na dowolnym urządzeniu będziesz potrzebować magicznego kodu',
+ 'Weryfikacja twarzą/odciskiem palca lub kluczem dostępu jest włączona na jednym lub większej liczbie urządzeń. Cofnięcie dostępu spowoduje, że przy następnej weryfikacji na dowolnym urządzeniu wymagany będzie magiczny kod',
+ confirmationPrompt: 'Czy na pewno? Będziesz potrzebować magicznego kodu do następnej weryfikacji na dowolnym urządzeniu',
cta: 'Cofnij dostęp',
noDevices:
'Nie masz żadnych urządzeń zarejestrowanych do weryfikacji twarzą/odciskiem palca ani kluczem dostępu. Jeśli jakieś zarejestrujesz, będziesz mógł/mogła cofnąć ten dostęp tutaj.',
diff --git a/src/languages/pt-BR.ts b/src/languages/pt-BR.ts
index a3828db2..5367605b 100644
--- a/src/languages/pt-BR.ts
+++ b/src/languages/pt-BR.ts
@@ -761,14 +761,14 @@ const translations: TranslationDeepObject<typeof en> = {
},
revoke: {
revoke: 'Revogar',
- title: 'Reconhecimento facial/digital & chaves de acesso',
+ title: 'Rosto/digital & chaves de acesso',
explanation:
- 'A verificação por rosto/digital ou passkey está ativada em um ou mais dispositivos. Revogar o acesso exigirá um código mágico para a próxima verificação em qualquer dispositivo',
+ 'A verificação por rosto/digital ou chave de acesso está ativada em um ou mais dispositivos. Revogar o acesso exigirá um código mágico para a próxima verificação em qualquer dispositivo',
confirmationPrompt: 'Você tem certeza? Você vai precisar de um código mágico para a próxima verificação em qualquer dispositivo',
cta: 'Revogar acesso',
- noDevices: 'Você não tem nenhum dispositivo registrado para verificação por rosto/digital ou por passkey. Se registrar algum, você poderá revogar esse acesso aqui.',
+ noDevices: 'Você não tem nenhum dispositivo registrado para verificação por rosto/digital ou passkey. Se registrar algum, você poderá revogar esse acesso aqui.',
dismiss: 'Entendi',
- error: 'Falha na solicitação. Tente novamente mais tarde.',
+ error: 'A solicitação falhou. Tente novamente mais tarde.',
},
},
validateCodeModal: {
diff --git a/src/languages/zh-hans.ts b/src/languages/zh-hans.ts
index 2e2d91ae..17933e34 100644
--- a/src/languages/zh-hans.ts
+++ b/src/languages/zh-hans.ts
@@ -757,13 +757,13 @@ const translations: TranslationDeepObject<typeof en> = {
},
revoke: {
revoke: '撤销',
- title: '面部识别/指纹识别与通行密钥',
- explanation: '一个或多个设备已启用面部/指纹或通行密钥验证。撤销访问权限后,下次在任何设备上进行验证时都需要输入魔法代码',
- confirmationPrompt: '你确定吗?在任何设备上进行下一次验证时,你都需要一个魔法代码',
+ title: '人脸/指纹与通行密钥',
+ explanation: '在一台或多台设备上已启用面部/指纹或通行密钥验证。撤销访问后,下次在任何设备上进行验证时都需要输入魔法验证码',
+ confirmationPrompt: '您确定吗?接下来在任何设备上的验证都需要输入魔法代码',
cta: '撤销访问权限',
- noDevices: '您尚未注册任何用于面容/指纹或通行密钥验证的设备。如果您注册了设备,您将能够在此撤销该访问权限。',
+ noDevices: '您尚未注册任何用于人脸/指纹或通行密钥验证的设备。如果您注册了设备,您将可以在此撤销该访问权限。',
dismiss: '知道了',
- error: '请求失败。请稍后重试。',
+ error: '请求失败。请稍后再试。',
},
},
validateCodeModal: {
Note You can apply these changes to your branch by copying the patch to your clipboard, then running |
Co-authored-by: Jakub Korytko <jakub@korytko.me>
|
I reverted 8fd73d6 because it changed a bunch of irrelevant strings |
|
Lint failure should go away once #80667 gets merged then we merge main |
rafecolton
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
| function getHasDevices(data: OnyxEntry<Account>) { | ||
| return data?.multifactorAuthenticationPublicKeyIDs && data.multifactorAuthenticationPublicKeyIDs.length > 0; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's move this to src/selectors/Account.ts to reuse it in src/components/TestToolMenu.tsx
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This and TestToolMenu actually have different requirements. Here, we need to know if length > 0, but there, we just need to know if it's not undefined
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh yes, I was mixing up TestToolMenu vs SecuritySettingsPage. I will make the change
| if (result.httpCode !== 200) { | ||
| setErrorMessage(translate('multifactorAuthentication.revoke.error')); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we display the server error here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This command does not throw any errors that are user-actionable, so the only reason it would fail would be something unexpected (network failure, etc). Better to show something friendly in the user's language than technobabble
|
@chuckdries @rafecolton Sorry for the delay. I've reviewed the PR and left some minor comments, nothing is breaking, just a few things to finalize the code and align with the design docs. |
|
Thanks @DylanDylann! I'll get a PR up with these addressed soon |
|
🚀 Deployed to staging by https://github.com/chuckdries in version: 9.3.11-0 🚀
|


Explanation of Change
Fixed Issues
$ #79378
PROPOSAL:
Tests
Base case
Onyx.merge('account', {multifactorAuthenticationPublicKeyIDs: ['asdf']})in your browser console (or set the NVP in your database if you have a local dev stack), then open the network tabFace/fingerprint & passkeysRevoke accessRevoke accessRevokeMultifactorAuthenticationCredentialsrequest appears in your devtools network tabNo keys registered case
Onyx.merge('account', {multifactorAuthenticationPublicKeyIDs: []})in your browser consoleFace/fingerprint & passkeysOffline tests
/multifactor-authentication/revokeQA Steps
None for now
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
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
Revoke.MFA.test.case.mp4