feat(mfa): add idp:reset-2fa command by romanetar · Pull Request #171 · OpenStackweb/openstackid

romanetar · 2026-10-07T17:50:55Z

ref: https://app.clickup.com/t/86ba2zp7r

Stacked on #169 (base: feat/mfa-disable-two-factor-service), which is stacked on #166 and #125. Merge bottom-up: #125, #166, #169, then this one.

Summary

Second half of ClickUp ticket 12 (Admin Enforcement and Reset Commands): idp:reset-2fa {email} --reason="...", for operators with server access who need to recover a user that lost their second factor and recovery codes.

--reason is required: without it (or blank) the command exits non-zero before touching anything.
Fails with a clear message if the email does not exist.
Calls IRecoveryCodeService::disableTwoFactor($user, null, null) from feat(mfa): add IRecoveryCodeService::disableTwoFactor #169, which clears two_factor_enabled and two_factor_enforced_at, deletes the recovery codes and revokes the trusted devices in one transaction. The command does not reimplement any of that.
Emits the settings_changed audit event with {"reason": "<--reason>", "actor": "console", "operator": "<os user>@<hostname>"} and ip_address 127.0.0.1.
Prints a summary: unused recovery codes deleted and active trusted devices revoked (counted before the reset).
Group-enforced users stay challenged at the next login, since enforcement is derived from group membership. The output says so explicitly, and tells the operator the user has no codes and must regenerate them from the profile Security section after logging in with the email OTP.

Things to know

If the audit event cannot be recorded, the reset is already committed, so the command reports it and exits with a failure instead of hiding the missing trail.
"The next password login of a group-enforced admin still lands on the 2FA challenge; a self-enrolled user's does not" is checked through User::shouldRequire2FA() on the reloaded user, which is what the login uses, not through an HTTP login.
Together with feat(mfa): add idp:enforce-admin-2fa command #166 this covers the ticket's two commands. Ticket 14 can reuse disableTwoFactor as is.

Test plan

tests/unit/Console/Reset2FACommandTest.php: 6 tests (missing and blank reason, unknown user, success with audit metadata, enforced-group warning, audit failure).
tests/Reset2FACommandIntegrationTest.php: 3 tests against the DB (no reason changes nothing; self-enrolled user fully cleared with 1 settings_changed row; group-enforced admin still requires 2FA).
The four new test files of this stack run together: 25 tests, 135 assertions, 0 failures.
Full phpunit suite not run. tests/RecoveryCodeRegenerationTest.php already fails on the base branch (Class "Strategies\MFA\MFAChallengeStrategyFactory" not found), unrelated to this change.

coderabbitai · 2026-10-07T17:51:04Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: defaults
Review profile: CHILL
Plan: Advanced
Run ID: 53ec627f-069a-43fd-9bee-5232c1de3c86

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

Trigger review

Finishing Touches

Generate docstrings

Commit to this branch
Create a new PR

Generate unit tests (beta)

Commit to this branch
Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

github-actions · 2026-10-07T17:52:23Z

📘 OpenAPI / Swagger preview

➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-171/

This page is automatically updated on each push to this PR.

github-actions · 2026-11-04T13:22:56Z

📘 OpenAPI / Swagger preview

➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-171/

This page is automatically updated on each push to this PR.

github-actions · 2026-11-04T13:23:13Z

📘 OpenAPI / Swagger preview

➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-171/

This page is automatically updated on each push to this PR.

Copilot

🟡 Changes recommended

Using get_current_user() attributes 2FA resets to the PHP script owner rather than the executing operator, undermining audit-trail accountability.

1 open issue

Reset audit records script owner instead of executing operator · New

What changed in this PR

Adds idp:reset-2fa for operators recovering users who have lost their second factor and recovery codes, reusing the shared reset service from #169.

Changes:

Requires a nonblank reason, resolves the user by email, and delegates the reset to disableTwoFactor.
Records audit metadata, reports reset counts and audit failures, and warns when group enforcement still applies.
Adds command registration and tests for validation, reset effects, auditing, and continued group enforcement.

File	Description
`tests/unit/Console/Reset2FACommandTest.php`	Tests validation, service calls, audit metadata, warnings, and audit failure handling.
`tests/Reset2FACommandIntegrationTest.php`	Checks persisted reset effects and continued group enforcement.
`app/Console/Kernel.php`	Registers the reset command with Artisan.
`app/Console/Commands/Reset2FACommand.php`	Implements reset orchestration, auditing, and operator output.

_{Review effort: Balanced}

smarcet · 2026-11-04T17:47:00Z

+        // the service owns the side effects (flags, codes, devices) in one transaction
+        $recovery_code_service->disableTwoFactor($user, null, null);
+
+        $operator = sprintf('%s@%s', get_current_user(), gethostname());


@romanetar Confirmed: get_current_user() returns the owner of the artisan file, not the effective user of the process, so the operator in the settings_changed audit row can name the deploy account instead of whoever ran the command. Suggested fix: posix_getpwuid(posix_geteuid())['name'], falling back to get_current_user() when posix is unavailable. Note that inside a k8s pod (kubectl exec) neither identifies the person, so --reason should name the operator or the ticket, as in the SDS §4.11 example.

Fixed in b727811. The operator now comes from posix_getpwuid(posix_geteuid()), falling back to get_current_user() when posix is unavailable (see processUser() in Reset2FACommand). Agreed that inside a k8s pod neither identifies the person, so --reason is the accountability anchor and should name the operator or ticket.

github-actions · 2026-11-04T17:44:11Z

📘 OpenAPI / Swagger preview

➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-171/

This page is automatically updated on each push to this PR.

github-actions · 2026-11-04T17:45:30Z

📘 OpenAPI / Swagger preview

➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-171/

This page is automatically updated on each push to this PR.

github-actions · 2026-11-10T16:22:25Z

📘 OpenAPI / Swagger preview

➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-171/

This page is automatically updated on each push to this PR.

github-actions · 2026-11-10T16:57:15Z

📘 OpenAPI / Swagger preview

➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-171/

This page is automatically updated on each push to this PR.

github-actions · 2026-11-10T17:01:04Z

📘 OpenAPI / Swagger preview

➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-171/

This page is automatically updated on each push to this PR.

github-actions · 2026-11-10T19:03:09Z

📘 OpenAPI / Swagger preview

➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-171/

This page is automatically updated on each push to this PR.

Resets 2FA for one user, for operators with server access: requires --reason (aborts without it), finds the user by email and calls IRecoveryCodeService::disableTwoFactor, which clears the enrollment, deletes the recovery codes and revokes the trusted devices in one transaction. Emits a settings_changed audit event with {reason, actor: "console", operator} and prints a summary with the number of codes deleted and devices revoked. Group-enforced users stay challenged at login because enforcement is derived from group membership; the output says so explicitly.

get_current_user() returns the owner of the artisan script, not the OS user running the command, so the audit operator was always the checkout owner.

github-actions · 2026-11-10T19:14:33Z

📘 OpenAPI / Swagger preview

➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-171/

This page is automatically updated on each push to this PR.

romanetar added this pull request to stack #170 October 7, 2026 17:51

smarcet force-pushed the feat/mfa-reset-2fa-command branch from d79aef8 to fe2914a Compare November 4, 2026 13:21

smarcet assigned romanetar Nov 4, 2026

smarcet requested a balanced review from Copilot November 4, 2026 13:24

Copilot started reviewing on behalf of smarcet November 4, 2026 13:24

View session

Copilot AI reviewed Nov 4, 2026

View reviewed changes

smarcet force-pushed the feat/mfa-reset-2fa-command branch from fe2914a to aeeb94f Compare November 4, 2026 17:42

smarcet mentioned this pull request Nov 4, 2026

feat(2fa): trusted devices management UI and revoke endpoints #160

Merged

This was referenced Nov 6, 2026

feat(2fa): recovery-code enrollment modal and batch acknowledgment #183

Open

feat(2fa): disableTwoFactor service method with enrollment_changed audit and group guard #186

Open

romanetar force-pushed the feat/mfa-reset-2fa-command branch from aeeb94f to 967399c Compare November 10, 2026 16:14

romanetar force-pushed the feat/mfa-reset-2fa-command branch from 967399c to ad297f6 Compare November 10, 2026 16:55

romanetar requested a review from smarcet November 10, 2026 16:56

romanetar force-pushed the feat/mfa-reset-2fa-command branch from 0c69b29 to 0231417 Compare November 10, 2026 19:01

romanetar added 2 commits November 10, 2026 20:12

fix(mfa): record the process user as the reset-2fa operator

b727811

get_current_user() returns the owner of the artisan script, not the OS user running the command, so the audit operator was always the checkout owner.

romanetar force-pushed the feat/mfa-reset-2fa-command branch from 1129da2 to b727811 Compare November 10, 2026 19:13

smarcet mentioned this pull request Nov 13, 2026

feat(mfa): add IRecoveryCodeService::disableTwoFactor #169

Open

2 of 3 tasks

Conversation

romanetar commented Oct 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Things to know

Test plan

Uh oh!

coderabbitai Bot commented Oct 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

github-actions Bot commented Oct 7, 2026

Uh oh!

github-actions Bot commented Nov 4, 2026

Uh oh!

github-actions Bot commented Nov 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

smarcet Nov 4, 2026

Choose a reason for hiding this comment

Uh oh!

romanetar Nov 11, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Nov 4, 2026

Uh oh!

github-actions Bot commented Nov 4, 2026

Uh oh!

github-actions Bot commented Nov 10, 2026

Uh oh!

github-actions Bot commented Nov 10, 2026

Uh oh!

github-actions Bot commented Nov 10, 2026

Uh oh!

github-actions Bot commented Nov 10, 2026

Uh oh!

github-actions Bot commented Nov 10, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

romanetar commented Oct 7, 2026 •

edited

Loading

coderabbitai Bot commented Oct 7, 2026 •

edited

Loading