Skip to content
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

Encourage setting up a second recovery method #642

Merged
merged 4 commits into from
Dec 2, 2024

Conversation

kasparsd
Copy link
Collaborator

@kasparsd kasparsd commented Sep 19, 2024

What?

Fixes #485.

Why?

Encourage users to configure at least two two-factor methods to prevent them from being locked out of account in case of loosing access to the primary method.

How?

Showing a notice before any of the methods are configured and when only one method is configured.

Testing Instructions

  1. Setup a plugin and go to your profile to setup the methods.
  2. Select only email, save the settings and confirm that a message is shown suggesting to setup another method.

Screenshots or screencast

No methods configured:

notice-default

Only one method configured:

notice-second

Multiple notices:

multiple-notices

Changelog Entry

Added a notice to the user profile suggesting to configure at least two two-factor methods for ensuring access.

Collect all warnings into same place
sprintf(
__( 'To update your Two-Factor options, you must first revalidate your session.', 'two-factor' ) .
'<br><a class="button" href="%s">' . __( 'Revalidate now', 'two-factor' ) . '</a>',
$notices['warning two-factor-warning-revalidate-session'] = sprintf(
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like the hidden meaning of the first key string being the class suffix for notice-* but that's how we can allow for multiple notices of the same type without introducing some kind of an abstraction.

<p><?php echo $notice; ?></p>
</div>
<?php endforeach; ?>
<p>
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are now permanent instructions suggest the best-practice setup.

@jeffpaul jeffpaul added this to the 0.10.0 milestone Sep 19, 2024
?>
<h2><?php esc_html_e( 'Two-Factor Options', 'two-factor' ); ?></h2>
<?php foreach ( $notices as $notice_type => $notice ) : ?>
<div class="<?php echo esc_attr( $notice_type ? 'notice inline notice-' . $notice_type : '' ); ?>">
<p><?php echo $notice; ?></p>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see the values are already escaped early should there be a wp_kses here to be safe / should we mention the values are already escaped above?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kasparsd FYI on the question above for you

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kasparsd kasparsd merged commit 100587e into master Dec 2, 2024
48 checks passed
@kasparsd kasparsd deleted the 485-add-backup-notice branch December 2, 2024 10:29
@jeffpaul jeffpaul modified the milestones: 0.11.0, 0.10.0 Dec 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Encourage setting up a recovery factor
3 participants