Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Standardise on int|WP_User input to the "for user" functions #535

Merged
merged 8 commits into from
Mar 16, 2023

Conversation

dd32
Copy link
Member

@dd32 dd32 commented Mar 8, 2023

Currently the various API-level functions accept two main inputs: ID or WP_User, and assumes that if not provided the function should operate on the current user.

Accepts WP_User:

  • Two_Factor_Core::get_enabled_providers_for_user()
  • Two_Factor_Core::get_available_providers_for_user()

Accepts ID:

  • Two_Factor_Core::get_primary_provider_for_user()
  • Two_Factor_Core::is_user_using_two_factor()

When a WP_User is passed in place of an ID, or an ID in place of a WP_User, the functions operate on the current user, which could lead to unexpected responses.

This PR standardises the above 4 functions to:

  • Accept int|WP_User
  • Only default to the current user when no parameters are passed / null is passed (The default value for the function param)
  • Uses the same user validation logic via a helper method, rather than duplicating logic.

For example, currently:

wp_set_current_user( 1 );
$incorrect = Two_Factor_Core::get_enabled_providers_for_user( 2 );
$correct = Two_Factor_Core::get_enabled_providers_for_user( get_user_by( 'id', 2 ) );

// $incorrect is for user_id 1, despite 2 being passed in.
// $correct is correct, because a WP_User was passed

After this PR, both $incorrect and $correct in the above would contain the details for user 2.

I'll note that is_user_api_login_enabled() has not been touched, as the $user_id is not used for any logic within the function, and the onus is on the filtering functions to check the value of $user_id passed.

@dd32 dd32 added Needs Code Review PHP Pull requests that update Php code Compatibility Compatibility with other plugins, Core, back-compat labels Mar 8, 2023
class-two-factor-core.php Show resolved Hide resolved
tests/class-two-factor-core.php Show resolved Hide resolved
@dd32 dd32 merged commit 3db2103 into WordPress:master Mar 16, 2023
@jeffpaul jeffpaul added this to the 0.8.0 milestone Mar 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Compatibility Compatibility with other plugins, Core, back-compat Needs Code Review PHP Pull requests that update Php code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants