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

CANTINA-958: Add better filtering in wp vip two-factor report CLI command #4910

Merged
merged 3 commits into from
Oct 3, 2023

Conversation

rebeccahum
Copy link
Contributor

@rebeccahum rebeccahum commented Sep 29, 2023

Description

Adds filtering by whether 2fa is enabled, 2fa provider, user role, and user.

Example usages:
* wp vip two-factor report
* wp vip two-factor report --2fa-enabled=true
* wp vip two-factor report --2fa-provider=email
* wp vip two-factor report --role=administrator
* wp vip two-factor report --role=administrator --2fa-enabled=false
* wp vip two-factor report --role=administrator --2fa-enabled=true --2fa-provider=email
* wp vip two-factor report --user_login=wpvip

Changelog Description

WP-CLI Command Updated: wp vip two-factor report

Add filtering by whether 2fa is enabled, 2fa provider, user role, and user.

Pre-review checklist

Please make sure the items below have been covered before requesting a review:

  • This change works and has been tested locally (or has an appropriate fallback).
  • This change works and has been tested on a Go sandbox.
  • This change has relevant unit tests (if applicable).
  • This change uses a rollout method to ease with deployment (if applicable - especially for large scale actions that require writes).
  • This change has relevant documentation additions / updates (if applicable).
  • I've created a changelog description that aligns with the provided examples.

Pre-deploy checklist

  • VIP staff: Ensure any alerts added/updated conform to internal standards (see internal documentation).

Steps to Test

  1. Create users with different roles, administrative users with 2FA and administrative users without 2FA
  2. Do the examples in description and ensure it returns the expected results

@rebeccahum rebeccahum marked this pull request as ready for review September 29, 2023 18:59
@rebeccahum rebeccahum requested a review from a team as a code owner September 29, 2023 18:59
@codecov
Copy link

codecov bot commented Sep 29, 2023

Codecov Report

Merging #4910 (d56f402) into develop (8de9938) will decrease coverage by 0.07%.
The diff coverage is 0.00%.

@@              Coverage Diff              @@
##             develop    #4910      +/-   ##
=============================================
- Coverage      29.20%   29.14%   -0.07%     
- Complexity      4726     4740      +14     
=============================================
  Files            277      277              
  Lines          20830    20873      +43     
=============================================
  Hits            6083     6083              
- Misses         14747    14790      +43     
Files Coverage Δ
wp-cli/vip-two-factor.php 0.00% <0.00%> (ø)

@rebeccahum
Copy link
Contributor Author

@GaryJones Can you plz traumatize me with a code review thx

Copy link
Contributor

@GaryJones GaryJones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of minor bits, but overall ✅

Would be good to add some Behat tests to check the guard clauses you have in here, as well successful output. (example)

Do you have any thoughts about getting this into https://github.com/WordPress/two-factor instead?

wp-cli/vip-two-factor.php Outdated Show resolved Hide resolved
wp-cli/vip-two-factor.php Outdated Show resolved Hide resolved
Comment on lines +127 to 129
$fields = array( 'ID', 'display_name', 'roles' );
$fields[] = 'two_factor_enabled';
$fields[] = 'two_factor_providers';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$fields = array( 'ID', 'display_name', 'roles' );
$fields[] = 'two_factor_enabled';
$fields[] = 'two_factor_providers';
$fields = array( 'ID', 'display_name', 'roles', 'two_factor_enabled', 'two_factor_providers' );

Further, looking at the output from wp user list --format=table:

+----+------------------+-------------------+-----------------------------------------+---------------------+---------------+
| ID | user_login       | display_name      | user_email                              | user_registered     | roles         |
+----+------------------+-------------------+-----------------------------------------+---------------------+---------------+

...then maybe this report output should also include the user_login, after the ID?
(May be outside the scope of this PR)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nah, I don't think it's necessary given the user lookup can be login/id/email.

@rebeccahum rebeccahum merged commit 2558033 into develop Oct 3, 2023
39 checks passed
@rebeccahum rebeccahum deleted the cli/2fa-updates branch October 3, 2023 15:30
rebeccahum added a commit that referenced this pull request Oct 4, 2023
* Add the governance plugin (#4907)

* Add the VIP Governance plugin to the integrations

* Update the config used for seeing if the governance plugin is loaded or not

* Add a test for the new vip-governance integration

* Leave only the single test that actually tests some functionality

* Add a test for the Block Data API as well

* Shift the import lines to be in the right order

* Add in a test to see if the isloaded method works correctly for both the integrations

* Add a code coverage ignore block to the registrations

* Remove the extra false loaded check

* Switch the isLoad check to not define any constant

* Initial rollout safeguard

* Search: Increase character search length to 510 characters (#4911)

* Search: Increase character search length to 510 characters

* Adjust test

Adjust test

* CANTINA-958: Add better filtering in `wp vip two-factor report` CLI command (#4910)

* CANTINA-958: Add better filtering in `wp vip two-factor report` CLI command

x

* Update wp-cli/vip-two-factor.php

Co-authored-by: Gary Jones <[email protected]>

* Update wp-cli/vip-two-factor.php

Co-authored-by: Gary Jones <[email protected]>

---------

Co-authored-by: Gary Jones <[email protected]>

* chore(deps): Bump preact in /search/search-dev-tools (#4913)

Bumps [preact](https://github.com/preactjs/preact) from 10.18.0 to 10.18.1.
- [Release notes](https://github.com/preactjs/preact/releases)
- [Commits](preactjs/preact@10.18.0...10.18.1)

---
updated-dependencies:
- dependency-name: preact
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): Bump react-select from 5.7.5 to 5.7.7

Bumps [react-select](https://github.com/JedWatson/react-select) from 5.7.5 to 5.7.7.
- [Release notes](https://github.com/JedWatson/react-select/releases)
- [Changelog](https://github.com/JedWatson/react-select/blob/master/docs/CHANGELOG.md)
- [Commits](https://github.com/JedWatson/react-select/compare/[email protected]@5.7.7)

---
updated-dependencies:
- dependency-name: react-select
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Gopal Krishnan <[email protected]>
Co-authored-by: Rebecca Hum <[email protected]>
Co-authored-by: Gary Jones <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
andrea-sdl pushed a commit that referenced this pull request Oct 19, 2023
…ommand (#4910)

* CANTINA-958: Add better filtering in `wp vip two-factor report` CLI command

x

* Update wp-cli/vip-two-factor.php

Co-authored-by: Gary Jones <[email protected]>

* Update wp-cli/vip-two-factor.php

Co-authored-by: Gary Jones <[email protected]>

---------

Co-authored-by: Gary Jones <[email protected]>
andrea-sdl pushed a commit that referenced this pull request Oct 19, 2023
* Add the governance plugin (#4907)

* Add the VIP Governance plugin to the integrations

* Update the config used for seeing if the governance plugin is loaded or not

* Add a test for the new vip-governance integration

* Leave only the single test that actually tests some functionality

* Add a test for the Block Data API as well

* Shift the import lines to be in the right order

* Add in a test to see if the isloaded method works correctly for both the integrations

* Add a code coverage ignore block to the registrations

* Remove the extra false loaded check

* Switch the isLoad check to not define any constant

* Initial rollout safeguard

* Search: Increase character search length to 510 characters (#4911)

* Search: Increase character search length to 510 characters

* Adjust test

Adjust test

* CANTINA-958: Add better filtering in `wp vip two-factor report` CLI command (#4910)

* CANTINA-958: Add better filtering in `wp vip two-factor report` CLI command

x

* Update wp-cli/vip-two-factor.php

Co-authored-by: Gary Jones <[email protected]>

* Update wp-cli/vip-two-factor.php

Co-authored-by: Gary Jones <[email protected]>

---------

Co-authored-by: Gary Jones <[email protected]>

* chore(deps): Bump preact in /search/search-dev-tools (#4913)

Bumps [preact](https://github.com/preactjs/preact) from 10.18.0 to 10.18.1.
- [Release notes](https://github.com/preactjs/preact/releases)
- [Commits](preactjs/preact@10.18.0...10.18.1)

---
updated-dependencies:
- dependency-name: preact
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): Bump react-select from 5.7.5 to 5.7.7

Bumps [react-select](https://github.com/JedWatson/react-select) from 5.7.5 to 5.7.7.
- [Release notes](https://github.com/JedWatson/react-select/releases)
- [Changelog](https://github.com/JedWatson/react-select/blob/master/docs/CHANGELOG.md)
- [Commits](https://github.com/JedWatson/react-select/compare/[email protected]@5.7.7)

---
updated-dependencies:
- dependency-name: react-select
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Gopal Krishnan <[email protected]>
Co-authored-by: Rebecca Hum <[email protected]>
Co-authored-by: Gary Jones <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants