Skip to content

Commit 43c2afc

Browse files
obenlandclaude
obenland
and
claude
committed
Handle WP_Error from get_available_providers_for_user() in all call sites
- require_ordinary_provider(): Clean up raw user meta when stripping all providers, preventing the WP_Error that blocks users from logging in. - REST API 2fa_available_providers field: Guard against WP_Error return. - after_provider_deactivated(): Guard against WP_Error return (previous commit). Fixes the backwards-incompatible change introduced in WordPress/two-factor#586. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 4f24ab1 commit 43c2afc

4 files changed

Lines changed: 73 additions & 7 deletions

File tree

‎settings/rest-api.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,9 @@ function register_user_fields(): void {
252252
'2fa_available_providers',
253253
[
254254
'get_callback' => function( $user ) {
255-
return array_keys( Two_Factor_Core::get_available_providers_for_user( get_userdata( $user['id'] ) ) );
255+
$providers = Two_Factor_Core::get_available_providers_for_user( get_userdata( $user['id'] ) );
256+
257+
return is_wp_error( $providers ) ? array() : array_keys( $providers );
256258
},
257259
'schema' => [
258260
'type' => 'array',

‎tests/settings/test-rest-api.php‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,4 +77,37 @@ public function test_totp_setup_requires_authorization() : void {
7777
$this->assertSame( 'rest_forbidden', $actual['code'] );
7878
$this->assertSame( 403, $actual['data']['status'] );
7979
}
80+
81+
/**
82+
* Verify that the 2fa_available_providers REST field doesn't fatal when
83+
* get_available_providers_for_user() returns a WP_Error.
84+
*
85+
* @covers WordPressdotorg\Two_Factor\Settings\register_rest_fields
86+
*/
87+
public function test_available_providers_field_handles_wp_error() {
88+
wp_set_current_user( self::$regular_user->ID, self::$regular_user->user_login );
89+
90+
/*
91+
* Set enabled providers to a non-existent provider so
92+
* get_available_providers_for_user() returns a WP_Error.
93+
*/
94+
update_user_meta(
95+
self::$regular_user->ID,
96+
Two_Factor_Core::ENABLED_PROVIDERS_USER_META_KEY,
97+
[ 'Non_Existent_Provider' ]
98+
);
99+
100+
$request = new WP_REST_Request( 'GET', '/wp/v2/users/' . self::$regular_user->ID );
101+
$request->set_query_params( [ 'context' => 'edit' ] );
102+
$response = rest_do_request( $request );
103+
104+
$this->assertNotEquals( 500, $response->get_status() );
105+
106+
$data = rest_get_server()->response_to_data( $response, false );
107+
108+
if ( isset( $data['2fa_available_providers'] ) ) {
109+
$this->assertIsArray( $data['2fa_available_providers'] );
110+
$this->assertEmpty( $data['2fa_available_providers'] );
111+
}
112+
}
80113
}

‎tests/test-wporg-two-factor.php‎

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -261,17 +261,37 @@ public function test_not_redirected_when_2fa_not_needed() {
261261

262262
/**
263263
* @covers WordPressdotorg\Two_Factor\set_primary_provider_for_user
264+
* @covers WordPressdotorg\Two_Factor\require_ordinary_provider
264265
*/
265266
public function test_set_primary_provider_for_user_without_ordinary_provider() {
266267
// Backup codes alone (without an ordinary provider) are stripped by require_ordinary_provider(),
267-
// so get_primary_provider_for_user() will wp_die() since enabled providers no longer exist.
268+
// which also cleans up the raw meta so get_available_providers_for_user() returns an empty
269+
// array instead of a WP_Error. get_primary_provider_for_user() should then return null.
268270
$backup_codes_provider = Two_Factor_Backup_Codes::get_instance();
269271
$backup_codes_provider->generate_codes( self::$regular_user );
270272
$enabled = Two_Factor_Core::enable_provider_for_user( self::$regular_user->ID, 'Two_Factor_Backup_Codes' );
271273
$this->assertTrue( $enabled );
272274

273-
$this->expectException( WPDieException::class );
274-
Two_Factor_Core::get_primary_provider_for_user( self::$regular_user->ID );
275+
$this->assertNull( Two_Factor_Core::get_primary_provider_for_user( self::$regular_user->ID ) );
276+
}
277+
278+
/**
279+
* Verify that require_ordinary_provider() cleans up the raw user meta when
280+
* no ordinary provider is enabled, preventing get_available_providers_for_user()
281+
* from returning a WP_Error.
282+
*
283+
* @covers WordPressdotorg\Two_Factor\require_ordinary_provider
284+
*/
285+
public function test_require_ordinary_provider_cleans_meta_without_ordinary_provider() {
286+
$backup_codes_provider = Two_Factor_Backup_Codes::get_instance();
287+
$backup_codes_provider->generate_codes( self::$regular_user );
288+
Two_Factor_Core::enable_provider_for_user( self::$regular_user->ID, 'Two_Factor_Backup_Codes' );
289+
290+
// After require_ordinary_provider runs (via the filter), available providers
291+
// should be an empty array, not a WP_Error.
292+
$available = Two_Factor_Core::get_available_providers_for_user( self::$regular_user );
293+
$this->assertIsArray( $available );
294+
$this->assertEmpty( $available );
275295
}
276296

277297
/**
@@ -334,15 +354,20 @@ public function test_after_provider_deactivated_handles_wp_error() {
334354
] );
335355
$this->assertNotFalse( Two_Factor_Core::is_current_user_session_two_factor() );
336356

337-
// Set enabled providers to a non-existent provider so
338-
// get_available_providers_for_user() returns a WP_Error.
357+
/*
358+
* Temporarily remove require_ordinary_provider() and set a bogus enabled
359+
* provider in meta, so get_available_providers_for_user() returns a WP_Error.
360+
*/
361+
$require_ordinary = 'WordPressdotorg\Two_Factor\require_ordinary_provider';
362+
remove_filter( 'two_factor_enabled_providers_for_user', $require_ordinary, 99 );
339363
update_user_meta( $user_id, Two_Factor_Core::ENABLED_PROVIDERS_USER_META_KEY, [ 'Non_Existent_Provider' ] );
340364

341365
$this->assertInstanceOf( WP_Error::class, Two_Factor_Core::get_available_providers_for_user( $user_id ) );
342366

343-
// This should not fatal.
344367
after_provider_deactivated( $user_id );
345368

369+
add_filter( 'two_factor_enabled_providers_for_user', $require_ordinary, 99, 2 );
370+
346371
// The session meta should be cleared.
347372
$this->assertFalse( Two_Factor_Core::is_current_user_session_two_factor() );
348373
}

‎wporg-two-factor.php‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,12 @@ function require_ordinary_provider( array $enabled_providers, int $user_id ) : a
9999
}
100100
} else {
101101
$enabled_providers = array();
102+
103+
/*
104+
* Clean up the raw meta so get_available_providers_for_user() doesn't
105+
* see stale providers and return a WP_Error.
106+
*/
107+
delete_user_meta( $user_id, Two_Factor_Core::ENABLED_PROVIDERS_USER_META_KEY );
102108
}
103109

104110
return $enabled_providers;

0 commit comments

Comments
 (0)