Skip to content

Fix stale 2FA used-code records blocking valid authentication#24160

Merged
sgiehl merged 4 commits into5.x-devfrom
dev-19948
Mar 3, 2026
Merged

Fix stale 2FA used-code records blocking valid authentication#24160
sgiehl merged 4 commits into5.x-devfrom
dev-19948

Conversation

@sgiehl
Copy link
Copy Markdown
Member

@sgiehl sgiehl commented Mar 2, 2026

Summary

This PR fixes 2FA code reuse handling when an old twofa_codes_used_* record already exists in option.

Before this change:

  • wasTwoFaCodeUsedRecently() treated stale timestamps as “recent”, so old entries could still block login.
  • If inserting the used-code record failed with a duplicate key, validation failed immediately, even when the existing record was stale.

After this change:

  • The “recently used” check correctly blocks only codes used within BLOCK_TWOFA_CODE_MINUTES.
  • On duplicate insert, we now attempt an atomic UPDATE that refreshes the timestamp only if the existing record is stale.
  • If that update succeeds, auth continues; if not, auth is rejected (code is still within block window).

Why

A user could be incorrectly blocked from using a valid 2FA code until cleanup ran, due to stale option rows and duplicate-key behavior.

Additional changes

  • Added stricter internal typing/docblocks in TwoFactorAuthentication (private/internal methods), aligned with ongoing PHPStan cleanup.

Tests

Added integration coverage in TwoFactorAuthenticationTest for:

  • stale record can be reused and timestamp is refreshed
  • stale record can be reused without requiring cleanup first

Checklist

  • [✔] I have understood, reviewed, and tested all AI outputs before use
  • [✔] All AI instructions respect security, IP, and privacy rules

Review

@sgiehl sgiehl added this to the 5.9.0 milestone Mar 2, 2026
@sgiehl sgiehl added Bug For errors / faults / flaws / inconsistencies etc. c: Security For issues that make Matomo more secure. Please report issues through HackerOne and not in Github. labels Mar 2, 2026
@sgiehl sgiehl requested a review from a team March 2, 2026 14:39
@nathangavin nathangavin self-assigned this Mar 3, 2026
nathangavin
nathangavin previously approved these changes Mar 3, 2026
Comment thread plugins/TwoFactorAuth/TwoFactorAuthentication.php Outdated
Comment thread plugins/TwoFactorAuth/TwoFactorAuthentication.php Outdated
Comment thread plugins/TwoFactorAuth/TwoFactorAuthentication.php Outdated
@sgiehl sgiehl enabled auto-merge (squash) March 3, 2026 10:52
@sgiehl sgiehl merged commit 619bdcc into 5.x-dev Mar 3, 2026
29 of 30 checks passed
@sgiehl sgiehl deleted the dev-19948 branch March 3, 2026 11:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug For errors / faults / flaws / inconsistencies etc. c: Security For issues that make Matomo more secure. Please report issues through HackerOne and not in Github.

Development

Successfully merging this pull request may close these issues.

3 participants