fix: Ensure two-factor methods are fully configured before enabling them by kasparsd · Pull Request #798 · WordPress/two-factor

kasparsd · 2026-02-17T09:43:10Z

What?

Check that a method is fully configured (TOTP secret stored, backup codes stored) before enabling it for the user.

Why?

Fixes #797, fixes #796, fixes #157.

How?

Introduce an error store for the plugin.
Forward any errors to the profile edit logic.
Render provider-specific errors inline the fields.

Testing Instructions

Go to Two Factor settings on a fresh user profile.
Enable "Authentication Code" method and save the profile.
Confirm that an error was shown at the top of the page and next to the provider.
Confirm that the method was not stored as enabled.

~~Note: the error handling on user profile pages don't allow us to easily render a notice when the method is not actually enabled. This needs to be implemented in a follow-up request.~~ (added this here since there was no decent way to show the errors both at the top of the page and in the context of the actual provider config).

Screenshots or screencast

All errors added to the top of the profile page if validation fails:

And provider-specific errors rendered inline the provider config:

Changelog Entry

Added - New feature.
Changed - Existing functionality.
Deprecated - Soon-to-be removed feature.
Removed - Feature.
Fixed - Bug fix.
Security - Vulnerability.

github-actions · 2026-02-17T09:43:21Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Unlinked Accounts

The following contributors have not linked their GitHub and WordPress.org accounts: @WordMessie.

Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Unlinked contributors: WordMessie.

Co-authored-by: kasparsd <[email protected]>
Co-authored-by: georgestephanis <[email protected]>
Co-authored-by: masteradhoc <[email protected]>
Co-authored-by: dknauss <[email protected]>
Co-authored-by: crstauf <[email protected]>
Co-authored-by: simonwheatley <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

kasparsd · 2026-02-17T09:44:15Z

class-two-factor-core.php

 	private static function render_user_providers_form( $user, $providers ) {
 		$primary_provider_key      = self::get_primary_provider_key_selected_for_user( $user );
-		$enabled_providers         = self::get_enabled_providers_for_user( $user );
+		$available_providers       = self::get_available_providers_for_user( $user );


Use the "enabled && configured" list of providers for representing the "enabled" state. This variable is used to represent the "enabled" checkbox for each provider.

kasparsd · 2026-02-17T09:46:04Z

class-two-factor-core.php

+			foreach ( $enabled_providers as $provider_key => $provider ) {
+				if ( ! $provider->is_available_for_user( $user ) ) {
+					// TODO: Use the `user_profile_update_errors` filter to show these errors too.
+					unset( $enabled_providers[ $provider_key ] );


Prevent storing the provider as enabled, if it isn't fully configured.

kasparsd · 2026-02-17T11:49:12Z

class-two-factor-core.php

 		add_filter( 'manage_users_columns', array( __CLASS__, 'filter_manage_users_columns' ) );
 		add_filter( 'wpmu_users_columns', array( __CLASS__, 'filter_manage_users_columns' ) );
 		add_filter( 'manage_users_custom_column', array( __CLASS__, 'manage_users_custom_column' ), 10, 3 );
+		add_action( 'user_profile_update_errors', array( __CLASS__, 'action_user_profile_update_errors' ) );


Avoid displaying "Updated" message by forwarding our errors to edit_user() early in wp-admin/user-edit.php.

kasparsd · 2026-02-17T11:49:49Z

class-two-factor-core.php

-				esc_html__( 'To update your Two-Factor options, you must first revalidate your session.', 'two-factor' ) .
-					' <a class="button" href="%s">' . esc_html__( 'Revalidate now', 'two-factor' ) . '</a>',
-				esc_url( $url )
+			self::add_error(


Make the existing warnings use our shared error API.

Copilot

Pull request overview

This PR addresses critical UX issues where two-factor authentication methods could be marked as enabled without being properly configured, leading to silent failures and user confusion during login. The changes introduce validation to ensure TOTP secrets and backup codes are stored before methods can be enabled, and add comprehensive error messaging to guide users through proper configuration.

Changes:

Added validation in user_two_factor_options_update() to check is_available_for_user() before enabling providers
Introduced $profile_errors error store with add_error() and action_user_profile_update_errors() to collect and display validation errors
Updated UI to render provider-specific errors inline and generic errors at the top of the form

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-17T12:28:24Z

class-two-factor-core.php

+			self::$profile_errors,
+			static function ( WP_Error $error ) {
+				$error_data = $error->get_error_data();
+				return ! empty( $error_data['provider'] ); // Where the associated provider is not set.


The logic here is inverted. The condition should use empty() instead of ! empty() to filter for generic errors (those WITHOUT a provider set). Currently, this filters for errors WITH a provider set, which means provider-specific errors will be shown in the generic error section at the top, and generic errors won't be shown at all.

Suggested change

return ! empty( $error_data['provider'] ); // Where the associated provider is not set.

return empty( $error_data['provider'] ); // Where the associated provider is not set.

Copilot · 2026-02-17T12:28:24Z

class-two-factor-core.php

 				( ! $existing_providers && $enabled_providers ) ||
 				// Has providers, and is disabling one (or more), but remaining with 2FA.
-				( $existing_providers && $enabled_providers && array_diff( $existing_providers, $enabled_providers ) )
+				( $existing_providers && $enabled_providers && array_diff_key( $existing_providers, $enabled_providers ) )


This line uses array_diff_key but $existing_providers is an array of string keys (not an associative array). The function get_enabled_providers_for_user() returns a simple array of provider key strings. You should use array_diff instead of array_diff_key here, or convert $existing_providers to an associative array with array_flip before using it with array_diff_key. The current code will likely result in unexpected behavior when checking if providers were disabled.

Copilot · 2026-02-17T12:28:25Z

class-two-factor-core.php

+		return array_filter(
+			self::$profile_errors,
+			static function ( WP_Error $error ) use ( $provider_key ) {
+				$error_data = $error->get_error_data(); // This currently supports only one error per instance.


This comment is misleading. The code retrieves all error data (not just one error code), and WP_Error can contain multiple error codes and messages. The comment should be clarified or removed, as it doesn't accurately describe a limitation of the implementation.

Suggested change

$error_data = $error->get_error_data(); // This currently supports only one error per instance.

$error_data = $error->get_error_data(); // Note: This inspects only the data associated with the first error code in the WP_Error instance.

Copilot · 2026-02-17T12:28:25Z

class-two-factor-core.php

+				wp_admin_notice(
+					implode( '</p><p>', $error->get_error_messages() ),
+					array(
+						'type'               => $error->get_error_data()['type'] ?? 'error',


The code accesses $error->get_error_data()['type'] directly with the null coalescing operator. However, if get_error_data() returns a non-array value (e.g., a string or null), this could trigger a PHP warning or error when trying to access it as an array. Consider checking if the result is an array first, or ensure all error data is consistently an array throughout the codebase.

Suggested change

wp_admin_notice(

implode( '</p><p>', $error->get_error_messages() ),

array(

'type' => $error->get_error_data()['type'] ?? 'error',

$error_data = $error->get_error_data();

$type = 'error';

if ( is_array( $error_data ) && isset( $error_data['type'] ) ) {

$type = $error_data['type'];

}

wp_admin_notice(

implode( '</p><p>', $error->get_error_messages() ),

array(

'type' => $type,

Copilot · 2026-02-17T12:28:25Z

class-two-factor-core.php

+			foreach ( $profile_error->get_error_codes() as $code ) {
+				foreach ( $profile_error->get_error_messages( $code ) as $message ) {
+					$errors->add( $code, $message );
+				}
+			}
+		}
+	}
+


The nested loops through error codes and messages are redundant in this context. Since each WP_Error instance in $profile_errors is keyed by its error code (line 392), each instance will only have that single error code. The inner loops can be simplified to just get all error messages and add them once with their code. Consider using $errors->merge_from($profile_error) if available, or simplify to: foreach ($profile_error->get_error_messages() as $message) { $errors->add($code, $message); }

Suggested change

foreach ( $profile_error->get_error_codes() as $code ) {

foreach ( $profile_error->get_error_messages( $code ) as $message ) {

$errors->add( $code, $message );

}

}

}

}

$code = $profile_error->get_error_code();

foreach ( $profile_error->get_error_messages() as $message ) {

$errors->add( $code, $message );

}

}

}

georgestephanis · 2026-02-17T20:19:46Z

There's a couple oddities that Copilot flagged that I'm not sure on, I've not dug deep into this yet.

masteradhoc · 2026-02-17T21:22:12Z

@kasparsd just tested this and works great.

went on a new profile
tried to add the "Authenticator App" and "Recovery Codes" without generating the backup codes or entering the TOTP
this message shows on top of the edit user page

4) also shows on the respective part

Also tested the flow when the Authenticator App gets resetted / disabled. Works also then!

LGTM!

dknauss · 2026-02-18T12:31:33Z

Works perfectly and looks great @kasparsd!

kasparsd added 3 commits February 17, 2026 10:59

Specify the return types

c2cd777

Only configured providers should be set to enabled

e547cfc

Ensure the method is configured before storing it as enabled

cbe4754

kasparsd added 13 commits February 17, 2026 11:48

this is now a key

b908f44

Specify the return type

a807a58

Account for the list being instances of providers

4460a6e

Add error tracking

0bb6825

Relay our errors to WP core profile logic

c6283a8

Pass along our errors

7ebde70

Describe the return type

93c16c4

Introduce our own error API

3badf51

Ensure there are errors to render

6a52da8

Add an error per provider

f434e31

Type inputs

2f6316d

Per linter

1a7853d

This is actually different from all others

8d1c840

kasparsd commented Feb 17, 2026

View reviewed changes

kasparsd mentioned this pull request Feb 17, 2026

Silent fallback to recovery codes when TOTP key is missing #796

Open

kasparsd added 3 commits February 17, 2026 14:03

Per linter

a5397aa

Per linter

fdec4b0

Per phpstan this is always WP_Error

9707660

kasparsd requested a review from Copilot February 17, 2026 12:21

Copilot started reviewing on behalf of kasparsd February 17, 2026 12:22 View session

kasparsd requested review from georgestephanis and jeffpaul February 17, 2026 12:22

Copilot AI reviewed Feb 17, 2026

View reviewed changes

masteradhoc added this to the 0.16.0 milestone Feb 18, 2026

	return ! empty( $error_data['provider'] ); // Where the associated provider is not set.
	return empty( $error_data['provider'] ); // Where the associated provider is not set.

	$error_data = $error->get_error_data(); // This currently supports only one error per instance.
	$error_data = $error->get_error_data(); // Note: This inspects only the data associated with the first error code in the WP_Error instance.

Conversation

kasparsd commented Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What?

Why?

How?

Testing Instructions

Screenshots or screencast

Changelog Entry

Uh oh!

github-actions bot commented Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Unlinked Accounts

Uh oh!

kasparsd Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

kasparsd Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

kasparsd Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

kasparsd Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

georgestephanis commented Feb 17, 2026

Uh oh!

masteradhoc commented Feb 17, 2026

Uh oh!

dknauss commented Feb 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Comments

kasparsd commented Feb 17, 2026 •

edited

Loading

github-actions bot commented Feb 17, 2026 •

edited

Loading