fix: Ensure two-factor methods are fully configured before enabling them#798
fix: Ensure two-factor methods are fully configured before enabling them#798
Conversation
|
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 Unlinked AccountsThe 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. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
There was a problem hiding this comment.
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 checkis_available_for_user()before enabling providers - Introduced
$profile_errorserror store withadd_error()andaction_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.
|
There's a couple oddities that Copilot flagged that I'm not sure on, I've not dug deep into this yet. |
|
@kasparsd just tested this and works great.
4) also shows on the respective part
Also tested the flow when the Authenticator App gets resetted / disabled. Works also then! LGTM! |
|
Works perfectly and looks great @kasparsd! |
georgestephanis
left a comment
There was a problem hiding this comment.
As soon as the copilot flags are addressed (either by dismissing or accepting) I'm fine seeing this merge. :)
|
@kasparsd any chance you can check the open Copilot Feedback? :) |
don’t error on non-string values such as arrays, etc.
|
@masteradhoc This is now updated to address the feedback. Could you please re-test the implementation? |
masteradhoc
left a comment
There was a problem hiding this comment.
LGTM! With the new changes the functionality i tested before still works great!




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?
Testing Instructions
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