diff --git a/settings/src/components/account-status.js b/settings/src/components/account-status.js index 3b730dc1..44abc60b 100644 --- a/settings/src/components/account-status.js +++ b/settings/src/components/account-status.js @@ -121,7 +121,11 @@ function SettingStatusCard( { screen, status, headerText, bodyText, disabled = f return ( - { disabled ? cardContent : } + { disabled ? ( + cardContent + ) : ( + + ) } ); } diff --git a/settings/src/components/backup-codes.js b/settings/src/components/backup-codes.js index 18c92f1f..0ad6ec5c 100644 --- a/settings/src/components/backup-codes.js +++ b/settings/src/components/backup-codes.js @@ -17,10 +17,18 @@ import { refreshRecord } from '../utilities'; */ export default function BackupCodes() { const { - user: { backupCodesEnabled }, + user: { backupCodesEnabled, totpEnabled }, + navigateToScreen, } = useContext( GlobalContext ); const [ regenerating, setRegenerating ] = useState( false ); + // If TOTP hasn't been enabled, the user should not have access to BackupCodes component. + // This is primarily added to prevent users from accessing through the URL. + if ( ! totpEnabled ) { + navigateToScreen( { nextScreen: 'account-status' } ); + return; + } + if ( backupCodesEnabled && ! regenerating ) { return ; } @@ -38,10 +46,12 @@ function Setup( { setRegenerating } ) { const { setGlobalNotice, user: { userRecord }, + setError, + error, + hasBackupCodesPrinted, + setHasBackupCodesPrinted, } = useContext( GlobalContext ); const [ backupCodes, setBackupCodes ] = useState( [] ); - const [ hasPrinted, setHasPrinted ] = useState( false ); - const [ error, setError ] = useState( '' ); // Generate new backup codes and save them in usermeta. useEffect( () => { @@ -71,12 +81,24 @@ function Setup( { setRegenerating } ) { }, [] ); // Finish the setup process. - const handleFinished = useCallback( () => { + const handleFinished = useCallback( async () => { + // TODO: Add try catch here after https://github.com/WordPress/wporg-two-factor/pull/187/files is merged. // The codes have already been saved to usermeta, see `generateCodes()` above. - refreshRecord( userRecord ); // This has the intended side-effect of redirecting to the Manage screen. + await refreshRecord( userRecord ); // This has the intended side-effect of redirecting to the Manage screen. setGlobalNotice( 'Backup codes have been enabled.' ); setRegenerating( false ); - } ); + }, [] ); + + const handleCheckboxChange = useCallback( + ( checked ) => { + setHasBackupCodesPrinted( checked ); + // Error should disappear when the user interacts with the checkbox again. + if ( 'checkbox_confirmation_required' === error.code ) { + setError( '' ); + } + }, + [ error.code ] + ); return ( <> @@ -88,33 +110,31 @@ function Setup( { setRegenerating } ) {

Please print the codes and keep them in a safe place.

- { error ? ( + + + + + Without access to the one-time password app or a backup code, you will lose access + to your account. Once you navigate away from this page, you will not be able to view + these codes again. + + + { error && ( { error.message } - ) : ( - <> - - - - - Without access to the one-time password app or a backup code, you will lose - access to your account. Once you navigate away from this page, you will not - be able to view these codes again. - - - - ) } + +

-

@@ -163,8 +183,14 @@ function Manage( { setRegenerating } ) { const { user: { userRecord: { record }, + setHasBackupCodesPrinted, }, } = useContext( GlobalContext ); + + // In Manage screen, always initialize "hasBackupCodesPrinted" as true. + // Otherwise, if users leave the account screen and return, it would default back to false, and the 'Back' button would be unclickable. + useEffect( () => setHasBackupCodesPrinted( true ), [] ); + const remaining = record[ '2fa_backup_codes_remaining' ]; return ( @@ -194,6 +220,7 @@ function Manage( { setRegenerating } ) { isSecondary onClick={ () => { setRegenerating( true ); + setHasBackupCodesPrinted( false ); } } > Generate new backup codes diff --git a/settings/src/components/revalidate-modal.js b/settings/src/components/revalidate-modal.js index 3dbbafd2..8408577c 100644 --- a/settings/src/components/revalidate-modal.js +++ b/settings/src/components/revalidate-modal.js @@ -1,16 +1,19 @@ /** * WordPress dependencies */ -import { useContext, useEffect, useRef } from '@wordpress/element'; +import { useCallback, useContext, useEffect, useRef } from '@wordpress/element'; import { GlobalContext } from '../script'; import { Modal } from '@wordpress/components'; import { useMergeRefs, useFocusableIframe } from '@wordpress/compose'; import { refreshRecord } from '../utilities'; export default function RevalidateModal() { - const { clickScreenLink } = useContext( GlobalContext ); + const { navigateToScreen } = useContext( GlobalContext ); - const goBack = ( event ) => clickScreenLink( event, 'account-status' ); + const goBack = useCallback( ( event ) => { + event.preventDefault(); + navigateToScreen( { nextScreen: 'account-status' } ); + }, [] ); return ( { - function maybeRefreshUser( { data: { type, message } = {} } ) { + async function maybeRefreshUser( { data: { type, message } = {} } ) { if ( type !== 'reValidationComplete' ) { return; } @@ -49,7 +52,13 @@ function RevalidateIframe() { record[ '2fa_revalidation' ].expires_at = new Date().getTime() / 1000 + 3600; // Refresh the user record, to fetch the correct 2fa_revalidation data. - refreshRecord( userRecord ); + try { + await refreshRecord( userRecord ); + } catch ( error ) { + // TODO: handle error more properly here, likely by showing a error notice + // eslint-disable-next-line no-console + console.error( 'Failed to refresh user record:', error ); + } } window.addEventListener( 'message', maybeRefreshUser ); diff --git a/settings/src/components/screen-link.js b/settings/src/components/screen-link.js index 5221e755..d0d3973d 100644 --- a/settings/src/components/screen-link.js +++ b/settings/src/components/screen-link.js @@ -1,19 +1,34 @@ /** * WordPress dependencies */ -import { useContext } from '@wordpress/element'; +import { useCallback, useContext } from '@wordpress/element'; /** * Internal dependencies */ import { GlobalContext } from '../script'; -export default function ScreenLink( { screen, anchorText, buttonStyle = false, ariaLabel } ) { - const { clickScreenLink } = useContext( GlobalContext ); +/** + * + * @param props + * @param props.currentScreen + * @param props.nextScreen + * @param props.anchorText + * @param props.buttonStyle + * @param props.ariaLabel + */ +export default function ScreenLink( { + currentScreen = '', + nextScreen, + anchorText, + buttonStyle = false, + ariaLabel, +} ) { + const { navigateToScreen } = useContext( GlobalContext ); const classes = []; const screenUrl = new URL( document.location.href ); - screenUrl.searchParams.set( 'screen', screen ); + screenUrl.searchParams.set( 'screen', nextScreen ); if ( 'primary' === buttonStyle ) { classes.push( 'components-button' ); @@ -23,10 +38,18 @@ export default function ScreenLink( { screen, anchorText, buttonStyle = false, a classes.push( 'is-secondary' ); } + const onClick = useCallback( + ( event ) => { + event.preventDefault(); + navigateToScreen( { currentScreen, nextScreen } ); + }, + [ navigateToScreen ] + ); + return ( clickScreenLink( event, screen ) } + onClick={ onClick } className={ classes.join( ' ' ) } aria-label={ ariaLabel } > diff --git a/settings/src/components/screen-navigation.js b/settings/src/components/screen-navigation.js new file mode 100644 index 00000000..abfd95af --- /dev/null +++ b/settings/src/components/screen-navigation.js @@ -0,0 +1,43 @@ +/** + * WordPress dependencies + */ +import { Icon, chevronLeft } from '@wordpress/icons'; +import { Card, CardHeader, CardBody } from '@wordpress/components'; + +/** + * Internal dependencies + */ +import ScreenLink from './screen-link'; + +/** + * @param props + * @param props.children + * @param props.screen + */ +const ScreenNavigation = ( { screen, children } ) => ( + + + + + Back + + } + /> + +

+ { screen + .replace( '-', ' ' ) + .replace( 'totp', 'Two-Factor Authentication' ) + .replace( 'webauthn', 'Two-Factor Security Key' ) } +

+
+ { children } +
+); + +export default ScreenNavigation; diff --git a/settings/src/components/screen-navigation.scss b/settings/src/components/screen-navigation.scss new file mode 100644 index 00000000..fdfd62c8 --- /dev/null +++ b/settings/src/components/screen-navigation.scss @@ -0,0 +1,22 @@ +.wporg-2fa__navigation { + padding: 24px 0 !important; /* Override Gutenberg auto-generated. */ + justify-content: center !important; + position: relative; + + a { + display: flex; + align-items: center; + position: absolute; + left: 24px; + } + + svg { + fill: #4ca6cf; + } + + h3 { + margin: unset; + text-transform: capitalize; + } + +} \ No newline at end of file diff --git a/settings/src/components/tests/utlitites.test.js b/settings/src/components/tests/utlitites.test.js index 0254742e..fa967af5 100644 --- a/settings/src/components/tests/utlitites.test.js +++ b/settings/src/components/tests/utlitites.test.js @@ -5,6 +5,7 @@ */ import { useSelect } from '@wordpress/data'; import { useEntityRecord } from '@wordpress/core-data'; +import { renderHook } from '@testing-library/react'; /** * Local dependencies @@ -23,7 +24,7 @@ describe( 'useUser', () => { it( 'should call useEntityRecord with correct arguments', () => { useEntityRecord.mockReturnValue( { record: {} } ); - useUser( 1 ); + renderHook( () => useUser( 1 ) ); expect( useEntityRecord ).toHaveBeenCalledWith( 'root', 'user', 1 ); } ); @@ -32,7 +33,9 @@ describe( 'useUser', () => { useEntityRecord.mockReturnValue( { record: {} } ); useSelect.mockReturnValue( true ); - const user = useUser( 1 ); + const { + result: { current: user }, + } = renderHook( () => useUser( 1 ) ); expect( useSelect ).toHaveBeenCalled(); expect( user.isSaving ).toBeTruthy(); @@ -43,7 +46,9 @@ describe( 'useUser', () => { record: { password: 'test-password' }, } ); - const user = useUser( 1 ); + const { + result: { current: user }, + } = renderHook( () => useUser( 1 ) ); expect( user.userRecord.record.password ).toBe( 'test-password' ); } ); @@ -51,7 +56,9 @@ describe( 'useUser', () => { it( 'should set hasPrimaryProvider to false if user.userRecord.record is undefined', () => { useEntityRecord.mockReturnValue( {} ); - const user = useUser( 1 ); + const { + result: { current: user }, + } = renderHook( () => useUser( 1 ) ); expect( user.hasPrimaryProvider ).toBe( false ); } ); @@ -59,7 +66,9 @@ describe( 'useUser', () => { it( 'should set hasPrimaryProvider to false if 2fa_available_providers is undefined', () => { useEntityRecord.mockReturnValue( { record: {} } ); - const user = useUser( 1 ); + const { + result: { current: user }, + } = renderHook( () => useUser( 1 ) ); expect( user.hasPrimaryProvider ).toBe( false ); } ); @@ -67,7 +76,9 @@ describe( 'useUser', () => { it( 'should set hasPrimaryProvider to false if 2fa_available_providers is empty', () => { useEntityRecord.mockReturnValue( { record: { '2fa_available_providers': [] } } ); - const user = useUser( 1 ); + const { + result: { current: user }, + } = renderHook( () => useUser( 1 ) ); expect( user.hasPrimaryProvider ).toBe( false ); } ); @@ -77,7 +88,9 @@ describe( 'useUser', () => { record: { '2fa_available_providers': [ 'Two_Factor_Backup_Codes' ] }, } ); - const user = useUser( 1 ); + const { + result: { current: user }, + } = renderHook( () => useUser( 1 ) ); expect( user.hasPrimaryProvider ).toBe( false ); } ); @@ -87,7 +100,9 @@ describe( 'useUser', () => { record: { '2fa_available_providers': [ 'Two_Factor_Totp', 'Two_Factor_Backup_Codes' ] }, } ); - const user = useUser( 1 ); + const { + result: { current: user }, + } = renderHook( () => useUser( 1 ) ); expect( user.hasPrimaryProvider ).toBe( true ); } ); @@ -102,7 +117,9 @@ describe( 'useUser', () => { }, } ); - const user = useUser( 1 ); + const { + result: { current: user }, + } = renderHook( () => useUser( 1 ) ); expect( user.hasPrimaryProvider ).toBe( true ); } ); @@ -123,8 +140,8 @@ describe( 'refreshRecord', () => { mockRecord.save.mockReset(); } ); - it( 'should call edit and save methods on the record object', () => { - refreshRecord( mockRecord ); + it( 'should call edit and save methods on the record object', async () => { + await refreshRecord( mockRecord ); expect( mockRecord.edit ).toHaveBeenCalledWith( { refreshRecordFakeKey: '', diff --git a/settings/src/components/totp.js b/settings/src/components/totp.js index c3c85927..0bb1821f 100644 --- a/settings/src/components/totp.js +++ b/settings/src/components/totp.js @@ -74,8 +74,8 @@ function Setup() { }, } ); - refreshRecord( userRecord ); - clickScreenLink( event, 'backup-codes' ); + await refreshRecord( userRecord ); + clickScreenLink( { event, nextScreen: 'backup-codes' } ); setGlobalNotice( 'Successfully enabled One Time Passwords.' ); // Must be After `clickScreenEvent` clears it. } catch ( handleEnableError ) { setError( handleEnableError.message ); @@ -304,7 +304,7 @@ function Manage() { data: { user_id: userRecord.record.id }, } ); - refreshRecord( userRecord ); + await refreshRecord( userRecord ); setGlobalNotice( 'Successfully disabled One Time Passwords.' ); } catch ( handleDisableError ) { setError( handleDisableError.message ); @@ -321,8 +321,8 @@ function Manage() {

Make sure you've created{ ' ' } - and saved them in a - safe location, in case you ever lose your device. You may also need them when + and saved them in + a safe location, in case you ever lose your device. You may also need them when transitioning to a new device. Without them you may permanently lose access to your account.

diff --git a/settings/src/script.js b/settings/src/script.js index 3ceccb3d..08662651 100644 --- a/settings/src/script.js +++ b/settings/src/script.js @@ -9,14 +9,13 @@ import { useState, createRoot, } from '@wordpress/element'; -import { Icon, chevronLeft } from '@wordpress/icons'; -import { Card, CardHeader, CardBody, Spinner } from '@wordpress/components'; +import { Spinner } from '@wordpress/components'; /** * Internal dependencies */ import { useUser } from './utilities'; -import ScreenLink from './components/screen-link'; +import ScreenNavigation from './components/screen-navigation'; import AccountStatus from './components/account-status'; import Password from './components/password'; import EmailAddress from './components/email-address'; @@ -59,10 +58,14 @@ function Main( { userId } ) { const { userRecord: { record, edit, hasEdits, hasResolved }, hasPrimaryProvider, + hasBackupCodesPrinted, } = user; const [ globalNotice, setGlobalNotice ] = useState( '' ); + const [ error, setError ] = useState( '' ); let currentUrl = new URL( document.location.href ); + let initialScreen = currentUrl.searchParams.get( 'screen' ); + const [ screen, setScreen ] = useState( initialScreen ); // The index is the URL slug and the value is the React component. const components = { @@ -70,7 +73,7 @@ function Main( { userId } ) { email: , password: , totp: , - 'backup-codes': , + 'backup-codes': , }; // TODO: Only enable WebAuthn UI in development, until it's finished. @@ -81,17 +84,12 @@ function Main( { userId } ) { // The screens where a recent two factor challenge is required. const twoFactorRequiredScreens = [ 'webauthn', 'totp', 'backup-codes' ]; - let initialScreen = currentUrl.searchParams.get( 'screen' ); - if ( ! components[ initialScreen ] ) { initialScreen = 'account-status'; currentUrl.searchParams.set( 'screen', initialScreen ); window.history.pushState( {}, '', currentUrl ); } - const [ screen, setScreen ] = useState( initialScreen ); - const currentScreen = components[ screen ]; - // Listen for back/forward button clicks. useEffect( () => { window.addEventListener( 'popstate', handlePopState ); @@ -116,10 +114,28 @@ function Main( { userId } ) { * * This is used in conjunction with real links in order to preserve deep linking and other foundational * behaviors that are broken otherwise. + * + * @param currentScreen Optional. Only needed when you'd like to do some processing on the current screen before navigating away. + * @param nextScreen Next screen you're navigating to */ - const clickScreenLink = useCallback( - ( event, nextScreen ) => { - event.preventDefault(); + const navigateToScreen = useCallback( + ( { currentScreen, nextScreen } ) => { + if ( 'backup-codes' === currentScreen ) { + // If there's already another error, do not overwrite it with the error below. + // Because usually, if there are other errors, it means there's a problem with the backup code generation. + // When this happens, users shouldn't need to confirm the checkbox to leave the current screen. + if ( ! error && ! hasBackupCodesPrinted ) { + setError( { + code: 'checkbox_confirmation_required', + message: 'Confirmation is required. Please check the checkbox to continue.', + } ); + return; + } + + if ( error.code === 'checkbox_confirmation_required' ) { + return; + } + } // Reset to initial after navigating away from a page. // Note: password was initially not in record, this would prevent incomplete state @@ -136,60 +152,44 @@ function Main( { userId } ) { currentUrl.searchParams.set( 'screen', nextScreen ); window.history.pushState( {}, '', currentUrl ); + setError( '' ); setGlobalNotice( '' ); setScreen( nextScreen ); }, - [ hasEdits ] + [ hasEdits, hasBackupCodesPrinted, error ] ); if ( ! hasResolved ) { return ; } - let screenContent = currentScreen; - - if ( 'account-status' !== screen ) { - screenContent = ( - - - - - Back - - } - /> - -

- { screen - .replace( '-', ' ' ) - .replace( 'totp', 'Two-Factor Authentication' ) - .replace( 'webauthn', 'Two-Factor Security Key' ) } -

-
- { currentScreen } -
+ const currentScreenComponent = + 'account-status' === screen ? ( + components[ screen ] + ) : ( + { components[ screen ] } ); - } else if ( + + const isRevalidationExpired = twoFactorRequiredScreens.includes( screen ) && hasPrimaryProvider && - record[ '2fa_revalidation' ]?.expires_at <= new Date().getTime() / 1000 - ) { - screenContent = ( - <> - { currentScreen } - - - ); - } + record[ '2fa_revalidation' ]?.expires_at <= new Date().getTime() / 1000; + + const shouldRevalidate = 'revalidation_required' === error.code || isRevalidationExpired; return ( - + - { screenContent } + { currentScreenComponent } + { shouldRevalidate && } ); } diff --git a/settings/src/style.scss b/settings/src/style.scss index 5ddc1549..e3ed7684 100644 --- a/settings/src/style.scss +++ b/settings/src/style.scss @@ -50,29 +50,6 @@ $alert-blue: #72aee6; flex-wrap: wrap; } -.wporg-2fa__navigation { - padding: 24px 0 !important; /* Override Gutenberg auto-generated. */ - justify-content: center !important; - position: relative; - - a { - display: flex; - align-items: center; - position: absolute; - left: 24px; - } - - svg { - fill: #4ca6cf; - } - - h3 { - margin: unset; - text-transform: capitalize; - } - -} - .wporg-2fa__token { letter-spacing: .3em; } @@ -105,5 +82,6 @@ $alert-blue: #72aee6; @import "components/setup-progress-bar"; @import "components/global-notice"; @import "components/screen-link"; +@import "components/screen-navigation"; @import "components/auto-tabbing-input"; @import "components/revalidate-modal"; diff --git a/settings/src/utilities.js b/settings/src/utilities.js index 0ae9f639..1b013f28 100644 --- a/settings/src/utilities.js +++ b/settings/src/utilities.js @@ -1,5 +1,6 @@ import { useSelect } from '@wordpress/data'; import { store as coreDataStore, useEntityRecord } from '@wordpress/core-data'; +import { useState } from '@wordpress/element'; /** * Get the user. @@ -8,6 +9,7 @@ import { store as coreDataStore, useEntityRecord } from '@wordpress/core-data'; */ export function useUser( userId ) { const userRecord = useEntityRecord( 'root', 'user', userId ); + const [ hasBackupCodesPrinted, setHasBackupCodesPrinted ] = useState( false ); const isSaving = useSelect( ( select ) => select( coreDataStore ).isSavingEntityRecord( 'root', 'user', userId ) ); @@ -25,6 +27,8 @@ export function useUser( userId ) { totpEnabled, backupCodesEnabled, webAuthnEnabled, + hasBackupCodesPrinted, + setHasBackupCodesPrinted, }; }