Conversation
… if disabled) for uninstall purposes
| * @return array List of provider keys and paths to class files. | ||
| */ | ||
| public static function get_providers() { | ||
| public static function get_providers_registered() { |
There was a problem hiding this comment.
Introducing dedicated methods for retrieving:
- all registered two-factor providers and their files.
- all registered two-factor providers and their classes without instantiating them (re-used by uninstall logic).
- all enabled providers (a subset of registered) and their classes instantiated (used as before).
| * | ||
| * @return array | ||
| */ | ||
| public static function get_providers() { |
There was a problem hiding this comment.
This is now much shorter as it can now get the class names for all enabled providers from the helpers.
| * | ||
| * @return array | ||
| */ | ||
| public static function uninstall_user_meta_keys() { |
There was a problem hiding this comment.
These are the two methods that providers can report their user meta key usage.
dd32
left a comment
dd32
left a comment
There was a problem hiding this comment.
General direction WFM.
See comment on the related ticket about deleting meta values by mid instead for performance.
| } | ||
|
|
||
| foreach ( $user_meta_keys as $meta_key ) { | ||
| delete_metadata( 'user', null, $meta_key, null, true ); |
There was a problem hiding this comment.
WP core uses this for deleting all post meta delete_post_meta_by_key() by key so we can re-use it as well.
|
It is fun to test this locally -- it purges the whole plugin directory along with all git history. I even tried to create another symlinked plugin instances hoping that WP would only delete the symlink but it still took everything 😅 |
|
Hi Folks, What would be considered sufficient DB cleanup in previous releases, just:
? This is also applicable for re-installations of the plugin (<0.10.0), for example in the completely hypothetical situation in which you accidentally reset your auth device without backing up the secrets and realize that your rarely opened 2FA recovery password manager DB had a racy sync at some point in the past. So if the only way in is deleting the plugin dir manually, then removing and re-installing, it would leave you in a spot in which you couldn't re-create a new secret (or even reuse the same) as you couldn't revalidate for the existing users. Completely hypothetical of course. Cheers, Valentin |
| * the value is the path to the file containing the class. | ||
| */ | ||
| $providers = apply_filters( 'two_factor_providers', $providers ); | ||
| $additional_providers = apply_filters( 'two_factor_providers', $providers ); |
There was a problem hiding this comment.
@kasparsd Using the same existing filter seems wrong here. Shouldn't this be a new filter so you only get the additional providers?
Right now the filter is called twice in get_providers(), see
two-factor/class-two-factor-core.php
Lines 264 to 275 in 6a95e7f
There was a problem hiding this comment.
@kasparsd This still seems wrong to me and is now released. Why running the filter twice?
There was a problem hiding this comment.
@ocean90 Sorry for missing your comment here. I'll create a patch to address this.
There was a problem hiding this comment.
Here is a pull request that removes the duplicate #651.
5 participants
What?
Fixes #630.
Why?
Delete all used user meta keys and options during plugin uninstall as expected.
How?
Register an uninstall hook for the plugin.
During uninstall, collect all meta keys and options used by two-factor providers through a known class helper. Delete them by looping through each user (a performance concern on larger sites).
Consider using a direct database query to purge the user meta by keys to avoid looping through every user on larger sites.
Testing Instructions
Screenshots or screencast
Changelog Entry
Added: register an uninstall hook for the plugin to delete all user meta data when the plugin is completely uninstalled.