#233: Add WP-CLI support with wp two-factor commands - #905
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: @mikeselander, @gurumark. 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. |
wp two-factor commands for per-user 2FA inspection and resetwp two-factor commands
|
Thanks for taking this on — I've been curious when WP-CLI might be supported. It should enable effective bulk provisioning and maybe eventually some policy controls. After scanning this branch with AI tools and a local test run to verify, a few key issues stand out:
To make the first point concrete, here's a small failing test against your branch that exercises the command directly. It needs a couple of WP-CLI runtime doubles since the plugin's PHPUnit run doesn't load WP-CLI:
|
Adds PHPUnit coverage for Two_Factor_CLI_Command (status, disable, enable, list-providers, backup-codes generate, unlock) and for the new Two_Factor_Core::clear_login_rate_limit() helper. Because the WP-CLI runtime is not loaded during PHPUnit, lightweight test doubles for WP_CLI, WP_CLI_Command, and the WP_CLI\Utils helpers are added under tests/cli/ to capture output and simulate the error()/confirm() exit behaviour. Also fixes the issues raised in review on PR WordPress#905: - backup-codes generate now enables the Two_Factor_Backup_Codes provider (mirroring rest_generate_codes()) so the codes are usable at login instead of being stored but never offered. - enable, disable, and the full reset now destroy the user's sessions when their enabled providers change, matching the profile-page behaviour. - The full reset no longer clears _two_factor_password_was_reset, so a rescued user still sees the notice explaining why their old password stopped working. - Removes the misleading reference to a non-existent "Phase 3" totp subcommand from the TOTP enable error message. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds PHPUnit coverage for Two_Factor_CLI_Command (status, disable, enable, list-providers, backup-codes generate, unlock) and for the new Two_Factor_Core::clear_login_rate_limit() helper. Because the WP-CLI runtime is not loaded during PHPUnit, lightweight test doubles for WP_CLI, WP_CLI_Command, and the WP_CLI\Utils helpers are added under tests/cli/ to capture output and simulate the error()/confirm() exit behaviour. Also fixes the issues raised in review on PR WordPress#905: backup-codes generate now enables the Two_Factor_Backup_Codes provider (mirroring rest_generate_codes()) so the codes are usable at login instead of being stored but never offered. enable, disable, and the full reset now destroy the user's sessions when their enabled providers change, matching the profile-page behaviour. The full reset no longer clears _two_factor_password_was_reset, so a rescued user still sees the notice explaining why their old password stopped working. Removes the misleading reference to a non-existent "Phase 3" totp subcommand from the TOTP enable error message.
Explicitly verify a confirmation prompt is emitted when --yes is omitted, in addition to the existing checks that the command aborts and leaves the provider enabled. Mirrors the prompt assertion used in the wordpress-ai Alt_Text_Command tests.
|
Huge props to @nimesh-xecurify for this work on the feedback and tests :) |
There was a problem hiding this comment.
Second pass after the test additions — this is close. I traced the paths I was unsure about last round: the CLI config changes destroy the user's sessions to force re-auth, matching what the profile-settings page already does, and disable_all_providers() deletes the raw enabled-providers meta before get_available_providers_for_user() re-checks, so the email fail-closed fallback can't force email back on during a full reset. Skipping capability checks is the right call for WP-CLI — shell access already implies trust. One thing to close before merge: the --count lower bound on backup-codes generate. Details in the inline note.
|
Thanks @dknauss and @nimesh-xecurify for your help on getting this tested and ready for a review. |
chubes4
left a comment
There was a problem hiding this comment.
Fuzzed this in an isolated WordPress/multisite runtime (seed 90520260723: 2,000 malformed invocations, 300 state sequences, 2,511 command dispatches). The general state/session/fail-closed behavior held, but three CLI safety gaps should be fixed before merge:
--countuses(int)coercion with no upper bound (CLI/class-two-factor-cli-command.php:545-561).--count=2garbagesilently generates 2 codes, and--count=101generates/stores/prints 101. Strictly validate a decimal integer and cap it at a documented safe maximum.- Regenerating backup codes immediately replaces every existing code without confirmation (
:556-592). This can silently invalidate offline recovery material; require confirmation/--yes, especially when codes already exist. - Single-provider TOTP disable only removes the enabled-provider flag (
:207-253) and leaves the TOTP secret in user meta. The provider REST delete path also callsdelete_user_totp_key()(providers/class-two-factor-totp.php:206-215). CLI disable should use provider-owned cleanup semantics so a later re-enable cannot resurrect the old secret.
No password/hash leakage, fail-open stale-provider state, session-invalidation failure, multisite user mix-up, or unexpected secret output was found. PHPStan and PHP compatibility passed. Happy to rerun the same seed after updates.
chubes4
left a comment
There was a problem hiding this comment.
Second-pass campaign against 66cb25d was substantially larger: 20,001 malformed invocations across three seeds, 5,000 state sequences, and 26,349 real commands over multisite plus PHP/WP compatibility smoke. The three prior findings are fixed in their normal paths, but harder state combinations exposed three remaining PR issues:
- Full reset skips residual security state when providers are already disabled (
CLI/class-two-factor-cli-command.php:280-291returns before cleanup at:312-351). With no enabled providers but stored recovery hashes, TOTP secret, throttle timestamp, seven failed attempts, and sessions,wp two-factor disable user --yesreports already disabled and leaves everything intact. Full reset should clean residual provider/rate/session state even when the enabled list is empty. - Backup hashes persist when provider enablement fails (
:617-637). With Backup Codes deregistered, generation exits 1 but leaves newly stored unknown hashes and no enabled provider/plaintext output. Validate enableability before generation or compensate the write on failure. - Provider typo is a false success (
:219-229).Two_Factor_TOTP(wrong case) exits 0/no changes while realTwo_Factor_Totpremains enabled with its secret. Validate requested provider registration and fail nonzero for unknown keys.
The current PHP 8.5 CI failure is test-only: set_user_totp_key() wraps update_user_meta(), whose successful insert returns integer meta ID 1315, not strict true; production replay confirms normal TOTP disable now deletes the secret. The test should assert non-false. There is also a PHPCS array-spacing error at the updated test around line 750.
Count parsing/cap, regeneration confirmation, enabled-TOTP cleanup, code replacement, fail-closed state, and secret isolation otherwise held. Six-worker concurrency was partially blocked by a WP Codebox temp-file collision, so no plugin race finding is claimed from corrupted worker starts. Happy to rerun seeds 9052301-9052303 after updates.
What?
Adds a
wp two-factorWP-CLI namespace with six subcommands for inspecting and managing two-factor authentication on a per-user basis.Fixes #233
Why?
The plugin ships no WP-CLI commands today. This PR adds support for it.
How?
cli/class-two-factor-cli-command.phpcontainingTwo_Factor_CLI_Command extends WP_CLI_Command. Registered underWP_CLI::add_command( 'two-factor', ... )in the plugin bootstrap behind adefined( 'WP_CLI' ) && WP_CLIguard.Two_Factor_Core::clear_login_rate_limit( $user )added toclass-two-factor-core.php. Both thedisable(full reset) andunlockcommands call this single method rather than deleting the rate-limit meta keys at each call site.Two_Factor_Coreand provider APIs — no raw SQL, no duplicated logic.Commands:
wp two-factor status <user>--formatwp two-factor disable <user> [<provider>]--yesskips promptwp two-factor list-providerswp two-factor enable <user> <provider>wp two-factor backup-codes generate <user> [--count=<n>]wp two-factor unlock <user>All commands accept user by ID, login, or email.
disable(full reset) assertsget_available_providers_for_user()is empty after clearing state, guarding against the fail-closed email fallback.Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Sonnet 4.6
Used for: Full implementation of the CLI class, helper method, and bootstrap wiring, based on the given specification from my side. All generated code was reviewed and tested manually before submission.
Testing Instructions
Setup: Activate the Two-Factor plugin. Create a test user and enable at least one 2FA provider (TOTP or Email) via their profile page.
Registration
wp help two-factor— confirm all six subcommands are listedwp help two-factor disable— confirm OPTIONS and EXAMPLES are shownUser resolution
wp two-factor status 1— resolves by numeric IDwp two-factor status <login>— resolves by loginwp two-factor status <email>— resolves by emailwp two-factor status nobody— printsError: User not found: nobodystatususing_2faisfalse--format=jsonreturns valid JSONlist-providersWP_DEBUGis off--format=jsonworksdisable— full resetwp two-factor disable <user>→ prompts for confirmationwp two-factor disable <user> --yes→ no prompt;statusshowsusing_2fa: false_two_factor_enabled_providersin the DB to a non-existent class name, then rundisable --yes— command should succeed and not leave email 2FA activedisable— single providerwp two-factor disable <user> Two_Factor_Totp→ only TOTP removed; backup codes still appear instatusenablewp two-factor enable <user> Two_Factor_Email→ success; appears instatuswp two-factor enable <user> Two_Factor_Totp→ error referencing profile pagewp two-factor enable <user> Two_Factor_Backup_Codes→ error pointing tobackup-codes generatewp two-factor enable <user> FakeClass→ error "Is it a registered provider?"backup-codes generatewp two-factor backup-codes generate <user>→ prints 10 codes--count=5→ prints exactly 5 codesunlockwp two-factor unlock <user>→ "Login throttle cleared"Screenshots or screencast
N/A — CLI only, no UI changes.
Changelog Entry
Added - WP-CLI
wp two-factorcommands for per-user 2FA status, disable, enable, backup-code generation, and login-throttle reset.