Skip to content

Commit

Permalink
Only show the reauth modal if a primary 2FA provider is enabled (#161)
Browse files Browse the repository at this point in the history
* Only show the reauth modal if 2FA by TOTP is enabled

* Refactor: DRY the Main component

* Only require revalidation if a non backup code provider is available

* Use shared utility for hasPrimaryProvider

* Separate user state from userRecord

* Destructure userRecord to be DRY
  • Loading branch information
adamwoodnz authored May 23, 2023
1 parent 6a95fb3 commit eb9483d
Show file tree
Hide file tree
Showing 10 changed files with 215 additions and 112 deletions.
37 changes: 21 additions & 16 deletions settings/src/components/account-status.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,26 +15,31 @@ import ScreenLink from './screen-link';
* Render the Account Status.
*/
export default function AccountStatus() {
const { userRecord } = useContext( GlobalContext );
const { record } = userRecord;
const emailStatus = record.pending_email ? 'pending' : 'ok';
const totpEnabled = record[ '2fa_available_providers' ].includes( 'Two_Factor_Totp' );
const webAuthnEnabled = record[ '2fa_available_providers' ].includes(
'TwoFactor_Provider_WebAuthn'
);
const primaryProviderEnabled = totpEnabled || webAuthnEnabled;
const backupCodesEnabled =
record[ '2fa_available_providers' ].includes( 'Two_Factor_Backup_Codes' );
const {
user: {
userRecord: {
record: {
'2fa_available_providers': availableProviders,
email,
pending_email: pendingEmail,
},
},
hasPrimaryProvider,
},
} = useContext( GlobalContext );
const emailStatus = pendingEmail ? 'pending' : 'ok';
const totpEnabled = availableProviders.includes( 'Two_Factor_Totp' );
const backupCodesEnabled = availableProviders.includes( 'Two_Factor_Backup_Codes' );

const backupBodyText =
! backupCodesEnabled && ! primaryProviderEnabled
! backupCodesEnabled && ! hasPrimaryProvider
? 'Please enable Two-Factor Authentication before enabling backup codes.'
: `You have
${ backupCodesEnabled ? '' : 'not' }
verified your backup codes for two-factor authentication.`;

return (
<>
<div className={ 'wporg-2fa__account-status' }>
<SettingStatusCard
screen="password"
status="enabled"
Expand All @@ -47,9 +52,9 @@ export default function AccountStatus() {
status={ emailStatus }
headerText="Account Email"
bodyText={
record.pending_email
? `Your account email is pending a change to ${ record.pending_email }.`
: `Your account email address is ${ record.email }.`
pendingEmail
? `Your account email is pending a change to ${ pendingEmail }.`
: `Your account email address is ${ email }.`
}
/>

Expand All @@ -72,7 +77,7 @@ export default function AccountStatus() {
bodyText={ backupBodyText }
disabled={ ! backupCodesEnabled }
/>
</>
</div>
);
}

Expand Down
17 changes: 13 additions & 4 deletions settings/src/components/backup-codes.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ import { refreshRecord } from '../utilities';
* Setup and manage backup codes.
*/
export default function BackupCodes() {
const { userRecord } = useContext( GlobalContext );
const {
user: { userRecord },
} = useContext( GlobalContext );
const [ regenerating, setRegenerating ] = useState( false );

const backupCodesEnabled =
Expand All @@ -35,7 +37,10 @@ export default function BackupCodes() {
* @param props.setRegenerating
*/
function Setup( { setRegenerating } ) {
const { setGlobalNotice, userRecord } = useContext( GlobalContext );
const {
setGlobalNotice,
user: { userRecord },
} = useContext( GlobalContext );
const [ backupCodes, setBackupCodes ] = useState( [] );
const [ hasPrinted, setHasPrinted ] = useState( false );

Expand Down Expand Up @@ -142,8 +147,12 @@ function CodeList( { codes } ) {
* @param props.setRegenerating
*/
function Manage( { setRegenerating } ) {
const { userRecord } = useContext( GlobalContext );
const remaining = userRecord.record[ '2fa_backup_codes_remaining' ];
const {
user: {
userRecord: { record },
},
} = useContext( GlobalContext );
const remaining = record[ '2fa_backup_codes_remaining' ];

return (
<>
Expand Down
8 changes: 6 additions & 2 deletions settings/src/components/email-address.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,12 @@ import { GlobalContext } from '../script';
* Render the Email setting.
*/
export default function EmailAddress() {
const { userRecord } = useContext( GlobalContext );
const { record, edit, save, editedRecord, hasEdits, isSaving } = userRecord;
const {
user: {
userRecord: { record, edit, save, editedRecord, hasEdits },
isSaving,
},
} = useContext( GlobalContext );
const [ emailError, setEmailError ] = useState( '' );
const [ justChangedEmail, setJustChangedEmail ] = useState( false );

Expand Down
51 changes: 27 additions & 24 deletions settings/src/components/password.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,29 +25,35 @@ import { GlobalContext } from '../script';
* Render the Password setting.
*/
export default function Password() {
const { setGlobalNotice, userRecord } = useContext( GlobalContext );
const {
setGlobalNotice,
user: {
userRecord: { hasEdits, editedRecord, record, edit, save },
isSaving,
},
} = useContext( GlobalContext );
const [ inputType, setInputType ] = useState( 'password' );
const [ hasAttemptedSave, setHasAttemptedSave ] = useState( false );
let passwordStrong = true; // Saved passwords have already passed the test.

if ( userRecord.hasEdits ) {
passwordStrong = isPasswordStrong( userRecord.editedRecord.password, userRecord.record );
if ( hasEdits ) {
passwordStrong = isPasswordStrong( editedRecord.password, record );
}

// Clear the "saved password" notice when password is being changed.
useEffect( () => {
if ( ! userRecord.hasEdits ) {
if ( ! hasEdits ) {
return;
}

setGlobalNotice( '' );
}, [ userRecord.hasEdits ] );
}, [ hasEdits ] );

/**
* Handle clicking the `Generate Password` button.
*/
const handlePasswordGenerate = useCallback( async () => {
userRecord.edit( { password: generatePassword( 24, true, true ) } );
edit( { password: generatePassword( 24, true, true ) } );
setInputType( 'text' );
}, [] );

Expand All @@ -58,11 +64,11 @@ export default function Password() {

setHasAttemptedSave( true );

if ( ! passwordStrong || userRecord.isSaving ) {
if ( ! passwordStrong || isSaving ) {
return;
}

await userRecord.save();
await save();

// Changing the password resets the nonce, which causes subsequent API requests to fail. `apiFetch()` will
// retry them automatically, but that results in an extra XHR request and a console error.
Expand All @@ -74,10 +80,10 @@ export default function Password() {

setGlobalNotice( 'New password saved.' );
},
[ passwordStrong, userRecord.isSaving ]
[ passwordStrong, isSaving ]
);

const handlePasswordChange = useCallback( ( password ) => userRecord.edit( { password } ), [] );
const handlePasswordChange = useCallback( ( password ) => edit( { password } ), [] );

const handlePasswordToggle = useCallback(
() => setInputType( inputType === 'password' ? 'text' : 'password' ),
Expand Down Expand Up @@ -109,7 +115,7 @@ export default function Password() {
help="The generate button will create a secure, random password."
label="New Password"
size="62"
value={ userRecord.editedRecord.password ?? '' }
value={ editedRecord.password ?? '' }
placeholder="Enter New Password..."
onChange={ handlePasswordChange }
/>
Expand All @@ -124,27 +130,24 @@ export default function Password() {
</Button>
</Flex>

{ userRecord.hasEdits && passwordStrong && (
{ hasEdits && passwordStrong && (
<Notice status="success" isDismissible={ false }>
<Icon icon={ check } />
Your password is strong enough to be saved.
</Notice>
) }

{ userRecord.hasEdits &&
userRecord.editedRecord.password &&
hasAttemptedSave &&
! passwordStrong && (
<Notice status="error" isDismissible={ false }>
<Icon icon={ cancelCircleFilled } />
That password is too easy to compromise. Please make it longer and/or add
random numbers/symbols.
</Notice>
) }
{ hasEdits && editedRecord.password && hasAttemptedSave && ! passwordStrong && (
<Notice status="error" isDismissible={ false }>
<Icon icon={ cancelCircleFilled } />
That password is too easy to compromise. Please make it longer and/or add random
numbers/symbols.
</Notice>
) }

<p>
<Button isPrimary disabled={ ! userRecord.editedRecord.password } type="submit">
{ userRecord.isSaving ? 'Saving...' : 'Save password' }
<Button isPrimary disabled={ ! editedRecord.password } type="submit">
{ isSaving ? 'Saving...' : 'Save password' }
</Button>

{ window.crypto?.getRandomValues && (
Expand Down
10 changes: 7 additions & 3 deletions settings/src/components/revalidate-modal.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,11 @@ export default function RevalidateModal() {
}

function RevalidateIframe() {
const { setGlobalNotice, userRecord } = useContext( GlobalContext );
const {
setGlobalNotice,
user: { userRecord },
} = useContext( GlobalContext );
const { record } = userRecord;
const ref = useRef();

useEffect( () => {
Expand All @@ -39,7 +43,7 @@ function RevalidateIframe() {

// Pretend that the expires_at is in the future (+1hr), this provides a 'faster' UI.
// This intentionally doesn't use `edit()` to prevent it attempting to update it on the server.
userRecord.record[ '2fa_revalidation' ].expires_at = new Date().getTime() / 1000 + 3600;
record[ '2fa_revalidation' ].expires_at = new Date().getTime() / 1000 + 3600;

// Refresh the user record, to fetch the correct 2fa_revalidation data.
refreshRecord( userRecord );
Expand All @@ -56,7 +60,7 @@ function RevalidateIframe() {
<iframe
title="Two-Factor Revalidation"
ref={ useMergeRefs( [ ref, useFocusableIframe() ] ) }
src={ userRecord.record[ '2fa_revalidation' ].revalidate_url }
src={ record[ '2fa_revalidation' ].revalidate_url }
/>
);
}
42 changes: 22 additions & 20 deletions settings/src/components/tests/password/password.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,17 @@ apiFetch.nonceMiddleware = { nonce: '' };

// Default mock context
const mockContext = {
userRecord: {
editedRecord: {
password: 'password',
},
edit: jest.fn(),
save: jest.fn(),
hasEdits: false,
record: {
'2fa_required': true,
user: {
userRecord: {
editedRecord: {
password: 'password',
},
edit: jest.fn(),
save: jest.fn(),
hasEdits: false,
record: {
'2fa_required': true,
},
},
isSaving: false,
},
Expand All @@ -57,8 +59,8 @@ describe( 'Password', () => {
} );

afterEach( () => {
mockContext.userRecord.edit.mockReset();
mockContext.userRecord.save.mockReset();
mockContext.user.userRecord.edit.mockReset();
mockContext.user.userRecord.save.mockReset();
mockContext.setGlobalNotice.mockReset();
} );

Expand All @@ -84,8 +86,8 @@ describe( 'Password', () => {
it( 'should display the weak password notice after submitting', () => {
// State: the user has updated their password to something weak
// although the strength is not tested here.
mockContext.userRecord.editedRecord.password = 'weak';
mockContext.userRecord.hasEdits = true;
mockContext.user.userRecord.editedRecord.password = 'weak';
mockContext.user.userRecord.hasEdits = true;

useContext.mockReturnValue( mockContext );

Expand All @@ -112,8 +114,8 @@ describe( 'Password', () => {
it( 'should display the strong password notice', () => {
// State: the user has updated their password to something strong
// although the strength is not tested here.
mockContext.userRecord.editedRecord.password = '@#4asdf34asdfasdf';
mockContext.userRecord.hasEdits = true;
mockContext.user.userRecord.editedRecord.password = '@#4asdf34asdfasdf';
mockContext.user.userRecord.hasEdits = true;

useContext.mockReturnValue( mockContext );

Expand Down Expand Up @@ -145,16 +147,16 @@ describe( 'Password', () => {
const input = getByLabelText( 'New Password' );
const newPassword = 'this is a password';
fireEvent.change( input, { target: { value: newPassword } } );
expect( mockContext.userRecord.edit ).toHaveBeenCalledWith( {
expect( mockContext.user.userRecord.edit ).toHaveBeenCalledWith( {
password: newPassword,
} );
} );

it( 'should submit form on button press', () => {
// State: the user has updated their password to something strong
// although the strength is not tested here.
mockContext.userRecord.editedRecord.password = '@#4asdf34asdfasdf';
mockContext.userRecord.hasEdits = true;
mockContext.user.userRecord.editedRecord.password = '@#4asdf34asdfasdf';
mockContext.user.userRecord.hasEdits = true;

useContext.mockReturnValue( mockContext );

Expand All @@ -171,7 +173,7 @@ describe( 'Password', () => {
const saveButton = buttons.filter( ( button ) => button.type === 'submit' )[ 0 ];
fireEvent.click( saveButton );

expect( mockContext.userRecord.save ).toHaveBeenCalled();
expect( mockContext.user.userRecord.save ).toHaveBeenCalled();
} );

it( 'should submit form on enter', () => {
Expand All @@ -186,6 +188,6 @@ describe( 'Password', () => {
const input = getByLabelText( 'New Password' );
fireEvent.submit( input );

expect( mockContext.userRecord.save ).toHaveBeenCalled();
expect( mockContext.user.userRecord.save ).toHaveBeenCalled();
} );
} );
Loading

0 comments on commit eb9483d

Please sign in to comment.