Enable TOTP method when method is configured #643
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…s one of the enabled options
kasparsd
commented
Dec 2, 2024
| @@ -818,52 +818,52 @@ public function test_show_password_reset_error() { | |||
| public function test_enable_disable_provider_for_user() { | |||
| $user = self::factory()->user->create_and_get(); | |||
| $enabled_providers = Two_Factor_Core::get_enabled_providers_for_user( $user->ID ); | |||
| $this->assertEmpty( $enabled_providers ); | |||
| $this->assertEmpty( $enabled_providers, 'No providers are enabled by default' ); | |||
There was a problem hiding this comment.
These help us identify failing tests instead of just showing the test method name.
| @@ -151,6 +151,10 @@ public function rest_delete_totp( $request ) { | |||
|
|
|||
| $this->delete_user_totp_key( $user_id ); | |||
|
|
|||
| if ( ! Two_Factor_Core::disable_provider_for_user( $user_id, 'Two_Factor_Totp' ) ) { | |||
There was a problem hiding this comment.
Deleting the TOTP key now also disables the method because it is unusable without the key.
| if ( in_array( $new_provider, $enabled_providers ) ) { | ||
| return true; | ||
| } | ||
|
|
||
| $enabled_providers[] = $new_provider; | ||
| $enabled = update_user_meta( $user_id, self::ENABLED_PROVIDERS_USER_META_KEY, $enabled_providers ); | ||
|
|
||
| // Primary provider must be enabled. |
There was a problem hiding this comment.
The primary provider is determined during get_primary_provider_for_user() so there is no need update it here. If no primary is configured, we always default to the first enabled.
| @@ -386,8 +391,10 @@ public function user_two_factor_options( $user ) { | |||
|
|
|||
| $error.find('p').text( errorMessage ); | |||
|
|
|||
| $( '#enabled-Two_Factor_Totp' ).prop( 'checked', false ); | |||
There was a problem hiding this comment.
We now enable the TOTP method when the key has been registered by default.
kasparsd
commented
Jan 9, 2025
| @@ -375,6 +379,7 @@ public function user_two_factor_options( $user ) { | |||
| user_id: <?php echo wp_json_encode( $user->ID ); ?>, | |||
| key: key, | |||
| code: code, | |||
| enable_provider: true, | |||
There was a problem hiding this comment.
Instruct the validator to also enable the provider, if everything is a pass.
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Users expect a method to be enabled if they chose to configure it.
Why?
Fixes #556.
How?
Testing Instructions
Setup workflow:
Reset workflow:
Screenshots or screencast
totp-actions.mp4
Changelog Entry