feat: reusable attempt-lock primitive backed by user meta - #36
Open
bordoni wants to merge 8 commits into
Open
Conversation
Introduces a durable counter/cooldown primitive that flows can reuse to bound repeated requests against the same subject. The existing burst limiter is transient-backed and advisory by construction: its storage is evictable, its fallback path is a read-modify-write that concurrent requests interleave, and a fixed window lets a caller spend a full allowance either side of a boundary. This layer is the durable half — written to user meta or a non-autoloaded option row, measured in hours, and escalating across repeats. The two are complementary; neither replaces the other. - Lockable/Store contracts plus BaseLock, which carries the whole engine so a concrete lock is roughly thirty lines. - Strikes accumulate to a threshold, then a cooldown engages. The breach count outlives the cooldown, so each subsequent lockout is drawn from a later rung of the ladder; a quiet period forgives the subject entirely. - Storage resolves per subject: user meta when a WordPress user owns the identity, a hashed option row when none does. Accounts exist upstream before they exist locally, so the fallback covers ordinary traffic, not edge cases. - Six locks covering code delivery, password sign-in, reset delivery, MFA challenges, account creation and token redemption. - IP subjects get looser thresholds and a short cooldown ceiling. Behind a CDN one address can front an entire audience, and a long lock there would be worse than the behaviour it bounds. - Daily sweep prunes expired option rows and enforces a row ceiling. - Operator surfaces are server-rendered admin-post requests gated on edit_users, with no REST route and no script bundle.
Wires the primitive into every path where a repeated request costs something the caller does not pay for: a delivered message, a billed SMS, a durable record upstream, or a guess at a short secret. Gate before the expensive call, count after it. Failure-counting locks clear on success, so a person who mistypes their own password or code and then gets it right carries nothing forward. Two paths reached password authentication with no throttling at all and are now covered: the anonymous admin-ajax login handler, which never went through the REST base class, and the wp-login.php authenticate filter. The ajax handler also relayed the upstream failure message verbatim, which distinguished between unknown account, wrong password and unverified address; it now answers uniformly. The change-email confirm and cancel routes are public by design — the emailed token is the whole credential — but nothing bounded how many tokens could be tried. Both now count failures against the presented token, hashed. Where a route answers uniformly to avoid confirming whether an account exists, the attempt is counted whether or not the underlying send happened. Counting only real sends would make the lock engage sooner for addresses that have accounts, reintroducing the difference the uniform response exists to remove. On the reset route the lock is checked ahead of the fixed response-time floor: a caller who is already barred is told so outright, so there is no timing signal left to flatten, and holding a PHP worker for most of a second per rejected request would work against the throttle. client_ip() gains an opt-in trusted-proxy header. Forwarding headers are caller-supplied in general, but behind a CDN REMOTE_ADDR is the edge node and every visitor shares one bucket, which is the worse failure once locks are durable. Existing tests updated for the new constructor arguments. Suite is at parity with main: 570 tests, same 4 pre-existing failures, no new ones.
- wp workos lock list|status|clear|clear-user|gc, for support work over SSH and for checking whether a lock is what is actually turning someone away before changing anything else. - Lock engagements and releases reach the activity log through a listener on the two lifecycle actions, keeping the engine itself free of any opinion about logging. Subjects are recorded as a kind plus a truncated hash so the log does not become a second copy of everyone's address. Releases are only recorded when something was actually holding, since successful sign-ins clear locks routinely. - Uninstall now removes lock meta and option rows by prefix and drops the scheduled sweep. Took the opportunity to add _workos_first_login and _workos_pending_email_change, both of which were already being left behind.
63 new tests across four files. The delivery test is the load-bearing one: it drives a sustained run of code requests at a single address, asserts only the allowance is forwarded, then empties the transients and the object cache and asserts the refusal holds. That second half is what separates this layer from the burst limiter in front of it — with the locks filtered off, those assertions return 200 instead of 429. Two behaviours the tests pinned down, both fixed in the process: - Counters recorded against an address before a WordPress user existed became unreachable once one was created, because resolution moved to user meta and left the earlier row orphaned. The store now folds that record onto the user on first read, so counting is continuous across account creation and an operator view cannot report a lock that no longer applies. - A refused sign-in was counted against the code-delivery budget regardless of why it was refused, so eight wrong passwords produced a message about codes the person had never been sent. Only refusals that actually dispatch a code are counted now; the two ledgers are separate and tested as such. Time is moved through a filter rather than slept, which is the only practical way to exercise a ladder that tops out a day out.
docs/locks.md follows the house dual-audience style: at-a-glance table, why the layer exists alongside the burst limiter, the three-step calling convention, a sequence diagram of the ladder, the storage matrix, a recipe for writing a lock, the full hook reference, the operator surfaces, and a Don't-do-this section. Adds workos()->lock( $slug ) so callers outside a REST endpoint have the same one-liner accessor as ->api() and ->option(). README and AGENTS cross-linked, Key Files and the test tree updated, version bumped in the four places.
Same meaning, register consistent with the rest of the file.
An action callback must not return a value. run() hands back the row count, which is useful to the CLI and to tests but meaningless to do_action(), so the hook now points at a thin void wrapper. Caught by PHPStan level 5.
ConfigSyncConstantsTest defines the wp-config constant overrides it exists to exercise, and a constant cannot be undefined for the rest of the process. Every later test asserting behaviour when the plugin is unconfigured then sees those constants and fails, because Config prefers a constant over the database value. CI has always split the run to avoid this; the composer script did not, so the documented shortcut reported four failures that were not regressions. It now chains both passes, and AGENTS.md explains the split and names the four tests so the next person recognises the symptom instead of chasing it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a reusable primitive for bounding how often the same caller can repeat an action that costs something — a delivered message, a billed SMS, a record created upstream, or a guess at a short-lived secret — and applies it across the auth flows.
Full reference:
docs/locks.md.Why
The plugin already throttles via
AuthKit\RateLimiter, a fixed-window limiter wired into every public auth route. It is cheap, it runs first, and it stays. But it is advisory by construction, for three reasons its own docblock spells out:tpermits a full allowance att+59sand another att+61s.None of that matters much for smoothing a burst, which is the limiter's job. It matters when the repeated thing costs money or wears down a secret. So there are two layers now:
RateLimiterThe primitive
LockableandStorecontracts, withBaseLockcarrying the whole engine so a concrete lock is about thirty lines — a slug and a policy.Two counters, and the distinction is the design.
strikesaccumulate within a run and reset when a cooldown engages.breachesrecords how many times the subject has already been locked and survives the cooldown, which is what makes the ladder escalate (5 min → 15 min → 1 h → 6 h → 24 h) rather than repeat. A quiet period then forgives the subject entirely, measured from when the cooldown ends, so a short decay window can never lift a longer lock that is still holding.Calling convention is three steps — gate, act, count:
For anything where success is normal — a password, a code, a token —
record()on failure andrelease()on success, so someone who mistypes their own password and then gets it right carries nothing forward.Refusals reuse the existing
workos_rate_limited/ 429 /retry_aftershape, so clients and the login UI need no change.Storage
_workos_lock_{slug}workos_lock_{slug}_{sha256}, autoload offThe option fallback is the ordinary case rather than an edge case: accounts are created in WorkOS first and only mirrored into WordPress on a successful login, so plenty of real traffic references an address with no local user. A user-meta-only design would not see it at all. Subject values are hashed into keys, so no address sits in
wp_optionsin plaintext, and a daily sweep prunes expired rows and caps the total.When an address that already has counters gains a WordPress user, the record moves onto that user on the next read — counting stays continuous across account creation and no orphaned row is left to report a lock that no longer applies.
Locks and where they apply
otp_sendpassword_authauthenticatefilterpassword_resetmfa_challengesignuptoken_guessAlso in scope
Two paths reached password authentication with no throttling at all, because neither goes through the REST base class:
admin-ajax.phpheadless login handler. It also relayed the upstream failure message verbatim, which distinguished between an unknown account, a wrong password and an unverified address; it now answers uniformly.wp-login.phpauthenticatefilter.The email-change confirm and cancel routes are public by design — the emailed token is the whole credential — but nothing bounded how many tokens could be tried. Both now count failures against the presented token, hashed.
Judgement calls worth a look
Password::triggered_verification_email().trusted_proxy_headersetting, empty by default.admin-post.phprequest gated onedit_users— notedit_user( $id ), which passes for anyone editing their own profile and would let a locked-out user lift their own lock. No REST route, no script bundle, works with JavaScript off.Operator surfaces
"Clear login locks" on the Users list and the user-edit screen, shown only for users who hold a lock.
wp workos lock list|status|clear|clear-user|gc.lock.engaged/lock.released/lock.clearedin the activity log, with subjects recorded as a kind plus a truncated hash.Fifteen filters and two actions, each with a per-lock variant — thresholds, ladders, decay, subjects, storage and the clock are all tunable, and
workos_locks_enabledturns the layer off while leaving the burst limiter in place.Testing
633 tests, up from 570. The 63 new ones cover the engine, the three backends, the sweep, the admin gate, and the guarded flows.
The delivery test is the load-bearing one: it drives a sustained run at a single address, asserts only the allowance is forwarded, then empties the transients and the object cache and asserts the refusal holds. With the locks filtered off those assertions return 200 instead of 429, which is the point — that behaviour is exactly what the burst limiter alone cannot provide.
Full suite green,
phpcsclean,phpstanlevel 5 clean with no baseline.Note for reviewers: the suite must be run in two passes —
--skip-group constantsthen--group constants— becauseConfigSyncConstantsTestdefines the wp-config constant overrides it exists to exercise, and those cannot be undefined for the rest of the process. CI has always split the run;composer test:wpunitnow does too, and AGENTS.md documents the symptom.