From 1769e15fbd5256ef7a1e72bd8cdd1e4ecec7cc52 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Mon, 26 Oct 2026 16:43:41 +0100 Subject: [PATCH] docs(19.6): record code review disposition and the WR-05 todo --- .../19.6-REVIEW-DISPOSITION.md | 16 ++++++++++++++++ .../2026-10-26-mfa-factor-changes-need-reauth.md | 16 ++++++++++++++++ 2 files changed, 32 insertions(+) create mode 100644 .planning/phases/19.6-two-factor-auth-and-device-flow-in-sm-user-plugin/19.6-REVIEW-DISPOSITION.md create mode 100644 .planning/todos/pending/2026-10-26-mfa-factor-changes-need-reauth.md diff --git a/.planning/phases/19.6-two-factor-auth-and-device-flow-in-sm-user-plugin/19.6-REVIEW-DISPOSITION.md b/.planning/phases/19.6-two-factor-auth-and-device-flow-in-sm-user-plugin/19.6-REVIEW-DISPOSITION.md new file mode 100644 index 00000000..03adeab5 --- /dev/null +++ b/.planning/phases/19.6-two-factor-auth-and-device-flow-in-sm-user-plugin/19.6-REVIEW-DISPOSITION.md @@ -0,0 +1,16 @@ +# Phase 19.6 code review disposition + +Review: 19.6-REVIEW.md (1 critical, 8 warning, 10 info). Fixes: 19.6-REVIEW-FIX.md. User decision 2026-10-26: fix the critical and all warnings, release, defer what needs a contract change. + +| ID | Severity | Status | Note | +|---|---|---|---| +| CR-01 | critical | fixed | sm-user-plugin cc3636c, released v0.1.13 | +| WR-01 | warning | fixed | sm-user-plugin 07196a4, released v0.1.13 (auth_url default and User-Agent-bound poll in release notes) | +| WR-02 | warning | fixed | sm-mfa-plugin eb555e8, released v0.1.2 | +| WR-03 | warning | fixed | sm-user-plugin 557c03c, released v0.1.13 | +| WR-04 | warning | fixed | sm-user-plugin 20e7b17, released v0.1.13 | +| WR-05 | warning | deferred | partial mitigation sm-mfa-plugin 463c750 (v0.1.2); full fix needs an API change: todo 2026-10-26-mfa-factor-changes-need-reauth.md, g15office krt3vrts | +| WR-06 | warning | fixed | sm-mfa-plugin 0a08070, released v0.1.2 | +| WR-07 | warning | fixed | sm-mfa-plugin f71d35d, released v0.1.2 | +| WR-08 | warning | fixed | sm-sociallogin-plugin ecfb8a7, released v0.1.5 | +| IN-01..IN-10 | info | open | outside the fix scope the user chose; listed in 19.6-REVIEW.md | diff --git a/.planning/todos/pending/2026-10-26-mfa-factor-changes-need-reauth.md b/.planning/todos/pending/2026-10-26-mfa-factor-changes-need-reauth.md new file mode 100644 index 00000000..3b591bdb --- /dev/null +++ b/.planning/todos/pending/2026-10-26-mfa-factor-changes-need-reauth.md @@ -0,0 +1,16 @@ +--- +title: Require re-authentication to change two-factor factors (review finding WR-05) +date: 2026-10-26 +priority: normal +area: sm-mfa-plugin +g15office: krt3vrts +--- + +Phase 19.6 code review WR-05 (19.6-REVIEW.md, 19.6-REVIEW-FIX.md): changing second factors needs only a session JWT. PHP and the frontends send no password on `DELETE /2fa/totp`, `DELETE /2fa/email`, `DELETE /2fa/webauthn/{id}`, `POST /2fa/totp/confirm` (when TOTP is already on) and `POST /2fa/webauthn/register`, so a stolen session can change factors while it lasts. + +Partial mitigation shipped in sm-mfa-plugin v0.1.2 (463c750): a password change demotes passkeys to second-factor keys, so a passkey registered from a stolen session does not outlive the victim's password change. + +Full fix (a contract change, user deferred it on 2026-10-26): +- Require a `password` field (or a current factor) on those routes, refused with the existing 403 `{"error":"Password confirmation required."}` and counted in the login throttle like the recovery-code routes (WR-06, 0a08070). +- Update the frontends that call them (sm-demo-app's vue-demo-app `pages/account/security.vue` and any app frontend) to ask for the password. +- Release sm-mfa-plugin as a minor/patch together with the frontend change, and bump the apps in separate commits.