From 746b8c6d3f063ecb91d35dc4d87f94c46146f074 Mon Sep 17 00:00:00 2001 From: John Hooks Date: Mon, 3 Aug 2026 18:32:11 -0700 Subject: [PATCH] refactor(rate-limit): limit recipients as well as requesters Rebuild throttling around two independent dimensions. HttpRateLimit counts who is asking (per IP and per subject, fixed clock-aligned windows); OutboundEmailRateLimiter counts who receives security email, one bucket shared by every send path, over layered 5min/hour/day ceilings. - Reserve-before-send with refunds: slots are claimed before the WorkOS call and handed back when the response shows no mail went out, closing the check-then-act gap under concurrent requests. - Addresses are canonicalized (tags, gmail dots) so variants share one counter, and subjects are hashed so no plaintext PII is stored. - Counters live in the object cache (atomic incr) with a transient fallback; window epoch is part of the key so expiry is arithmetic. - Client IP comes from REMOTE_ADDR unless an operator names a proxy header; list values are read right-most-public. - Injected Clock (frozen in tests via a Codeception module) replaces time() so window rollover is testable. - Limits are class constants beside the endpoints that own them; the only settings left are the email ceilings and the proxy header. --- docs/rate-limiting.md | 156 +++++++ src/WorkOS/Auth/AuthKit/Controller.php | 1 - src/WorkOS/Auth/AuthKit/RateLimiter.php | 270 ------------- src/WorkOS/Auth/ChangeEmail/RestApi.php | 63 ++- .../Auth/PasswordResetAdmin/RestApi.php | 44 +- src/WorkOS/Config.php | 68 ++++ src/WorkOS/Controller.php | 7 + src/WorkOS/Email/AddressCanonicalizer.php | 83 ++++ src/WorkOS/Http/ClientIp.php | 124 ++++++ src/WorkOS/REST/Auth/BaseEndpoint.php | 72 ++-- src/WorkOS/REST/Auth/Invitation.php | 14 +- src/WorkOS/REST/Auth/MagicCode.php | 38 +- src/WorkOS/REST/Auth/Mfa.php | 16 +- src/WorkOS/REST/Auth/OAuth.php | 16 +- src/WorkOS/REST/Auth/Password.php | 107 +++-- src/WorkOS/REST/Auth/Signup.php | 38 +- src/WorkOS/RateLimit/Controller.php | 74 ++++ src/WorkOS/RateLimit/CounterStore.php | 69 ++++ src/WorkOS/RateLimit/HttpRateLimit.php | 148 +++++++ src/WorkOS/RateLimit/ObjectCacheStore.php | 99 +++++ .../RateLimit/OutboundEmailRateLimiter.php | 142 +++++++ src/WorkOS/RateLimit/RateLimiter.php | 299 ++++++++++++++ src/WorkOS/RateLimit/Reservation.php | 139 +++++++ src/WorkOS/RateLimit/TransientStore.php | 81 ++++ src/WorkOS/Time/Clock.php | 27 ++ src/WorkOS/Time/Controller.php | 40 ++ src/WorkOS/Time/FrozenClock.php | 68 ++++ src/WorkOS/Time/SystemClock.php | 25 ++ tests/_support/Helper/Time.php | 93 +++++ tests/wpunit.suite.dist.yml | 1 + tests/wpunit/AuthKitRateLimiterTest.php | 379 +++++++++++++++++- tests/wpunit/AuthKitRestMagicSessionTest.php | 16 +- tests/wpunit/AuthKitRestMfaTest.php | 12 +- tests/wpunit/AuthKitRestPasswordTest.php | 12 +- .../AuthKitRestSignupInvitationOAuthTest.php | 18 +- tests/wpunit/ChangeEmailRestApiTest.php | 12 +- .../wpunit/PasswordResetAdminRestApiTest.php | 9 +- 37 files changed, 2391 insertions(+), 489 deletions(-) create mode 100644 docs/rate-limiting.md delete mode 100644 src/WorkOS/Auth/AuthKit/RateLimiter.php create mode 100644 src/WorkOS/Email/AddressCanonicalizer.php create mode 100644 src/WorkOS/Http/ClientIp.php create mode 100644 src/WorkOS/RateLimit/Controller.php create mode 100644 src/WorkOS/RateLimit/CounterStore.php create mode 100644 src/WorkOS/RateLimit/HttpRateLimit.php create mode 100644 src/WorkOS/RateLimit/ObjectCacheStore.php create mode 100644 src/WorkOS/RateLimit/OutboundEmailRateLimiter.php create mode 100644 src/WorkOS/RateLimit/RateLimiter.php create mode 100644 src/WorkOS/RateLimit/Reservation.php create mode 100644 src/WorkOS/RateLimit/TransientStore.php create mode 100644 src/WorkOS/Time/Clock.php create mode 100644 src/WorkOS/Time/Controller.php create mode 100644 src/WorkOS/Time/FrozenClock.php create mode 100644 src/WorkOS/Time/SystemClock.php create mode 100644 tests/_support/Helper/Time.php diff --git a/docs/rate-limiting.md b/docs/rate-limiting.md new file mode 100644 index 0000000..12dbd55 --- /dev/null +++ b/docs/rate-limiting.md @@ -0,0 +1,156 @@ +# Rate Limiting + +Two independent limits protect the public auth endpoints: one on who is asking, one on who receives the security email that asking produces. + +This document is the source of truth for how requests to `/wp-json/workos/v1/auth/*` are throttled, how to tune it per deployment, and which parts of the design exist for reasons that aren't obvious from the code. If you are adding an endpoint that can cause an email to be sent, read [Adding a send path](#adding-a-send-path) before you write it. + +> **Plugin requirement:** unreleased as of 1.0.8. Earlier versions shipped a single per-endpoint limiter with no per-recipient limit at all. + +## Why two limits + +A requester limit bounds how fast one caller can hit an operation. It cannot bound how much mail one mailbox receives, because that is a property of the recipient: several operations can each send to the same address under their own separate allowance, address variants like `alice+1@` would otherwise earn a fresh allowance each, and a per-minute window with nothing behind it makes the per-minute rate sustainable forever. + +So the two dimensions are separate classes. `HttpRateLimit` answers "is this caller asking too fast?", per operation. `OutboundEmailRateLimiter` answers "has this mailbox received enough?", with one counter shared by every path that can send, so rotating between endpoints buys nothing. + +Sign-up is limited more tightly than sign-in because it is the higher-leverage operation: it creates an account, and it does not require the address being registered to be verified. Verifying a credential is bounded by the account already existing. + +## At a glance + +| | `HttpRateLimit` | `OutboundEmailRateLimiter` | +| --- | --- | --- | +| Counts | requests | messages sent | +| Keyed on | caller IP, and the subject acted on | the recipient mailbox | +| Scope | per operation | one counter shared by every send path | +| Windows | one, 60s | three, layered: 5min / hour / day | +| Refuses with | `429 workos_rate_limited` | `429`, or silence on enumeration-safe routes | +| Instances | one; non-default postures pass their limits at the call site | one | + +Both sit on a shared `RateLimiter`, which owns windows and verdicts, and a `CounterStore`, which owns storage. + +## Configuration + +Requester limits are class constants, not settings. The default posture lives on `HttpRateLimit` (10 per IP, 5 per subject, 60s window); operations that differ declare their own constants next to the endpoint that owns the reasoning (`Signup::CREATE_PER_IP`, `OAuth::URL_PER_IP`). Changing one is a code change, deliberately: a per-endpoint constant is a public contract to support forever, and nobody tunes thirteen of them. + +The operator-tunable settings are the ones with a genuine deployment story: how much mail one mailbox may receive, and which proxy header to trust. Define them in `wp-config.php`, or set them as real environment variables; constants win. + +| Constant | Default | Applies to | +| --- | --- | --- | +| `WORKOS_EMAIL_LIMIT_5MIN` | 3 | messages to one mailbox per 5 minutes | +| `WORKOS_EMAIL_LIMIT_HOUR` | 10 | messages to one mailbox per hour | +| `WORKOS_EMAIL_LIMIT_DAY` | 20 | messages to one mailbox per day | +| `WORKOS_CLIENT_IP_HEADER` | *(unset)* | trusted proxy header, e.g. `CF-Connecting-IP` | + +Anything missing, non-numeric or non-positive falls back to the default, so a typo degrades to the shipped value rather than to no limit. + +The three email windows are layered on purpose. A single fixed window is burstable at its boundary by construction: send the maximum, wait for the boundary, send the maximum again. The short window keeps a mistyped address usable; the hour and day windows are the actual ceiling. + +## Where limits apply + +Every public auth route is limited by requester. A subset also counts as a send. + +| Operation | Endpoint | Subject | Counts as a send | +| --- | --- | --- | --- | +| `password` | `POST /auth/password/authenticate` | email | only when WorkOS reports the address unverified | +| `pw_reset` | `POST /auth/password/reset/start` | email | yes | +| `pw_reset_confirm` | `POST /auth/password/reset/confirm` | — | no | +| `magic_send` | `POST /auth/magic/send` | email | yes | +| `magic_verify` | `POST /auth/magic/verify` | email | no | +| `signup` | `POST /auth/signup/create` | email | yes | +| `signup_verify` | `POST /auth/signup/verify` | — | no | +| `mfa_challenge` | `POST /auth/mfa/challenge` | factor ID | no | +| `mfa_verify` | `POST /auth/mfa/verify` | — | no | +| `invitation_lookup` | `GET /auth/invitation/{token}` | — | no | +| `invitation_accept` | `POST /auth/invitation/accept` | — | no | +| `oauth_url` | `GET /auth/oauth/authorize-url` | — | no | +| `pw_reset_admin` | `POST /admin/users/{id}/password-reset` | user ID | no — see below | + +The change-email flow (`POST /users/{id}/email-change`) keeps its own hourly per-IP and per-user limits from plugin options, and counts **two** sends: one to the new address, one notice to the account's own. + +`pw_reset_admin` deliberately doesn't count as a send. It requires `edit_user` on the target and mails that account's own address, so the recipient is never caller-chosen. It is not the exposure the per-recipient limit exists for. + +## How counting works + +A counter is identified by a **bucket** naming the operation and a **subject** naming what is being limited. Subjects are hashed into the key, so no address or IP is stored in plaintext. + +The window is part of the key: `floor( now / window )`. A new window is simply a different key, which means expiry is arithmetic rather than something the storage layer has to get right. A counter that outlives its TTL can only affect a window that never comes round again. + +Storage is the object cache when WordPress reports one, transients otherwise. That choice is made once in `RateLimit\Controller`. On a real cache backend `incr` is atomic, so the limit holds under concurrency; the transient path is a read-modify-write and says so in its docblock, and the layered hour and day windows bound what a short-window race can slip through. + +### Reserve and release + +Most send paths know they are about to send, so they count once and are done. + +`password/authenticate` doesn't. WorkOS decides: it mails a verification code as a side effect of authenticating an unverified account, and only tells us in the response. So that path **claims the slot before the call** and hands it back if the response shows nothing was sent: + +```php +$mail_slot = $this->outbound_email->reserve( $email ); +if ( is_wp_error( $mail_slot ) ) { + return $mail_slot; +} + +// …WorkOS call… + +$mail_slot->release(); // nothing was sent +``` + +Counting afterwards instead would leave a gap the width of the HTTPS round trip, and every concurrent request arriving inside it reads the same number and passes. + +The `Reservation` carries the instant it was taken, and `release()` replays it rather than reading the clock again. Without that, a request whose round trip crosses a window boundary would credit the *next* window while leaving the original charged. + +## Adding a send path + +1. Take a slot before sending: `$this->outbound_email->reserve( $address )`. +2. If it returns a `WP_Error`, don't send. Return the 429, unless the route is enumeration-safe. +3. If you only learn afterwards that nothing went out, call `release()` on the handle. +4. One call per **message**, not per request. Two recipients means two reservations. + +## Don't do this + +### ❌ Don't give a new endpoint its own recipient counter. + +Four endpoints with four separate allowances against one inbox add up. Every send path shares `OutboundEmailRateLimiter`, and rotating between them must buy nothing. + +### ❌ Don't check the limit and send later. + +`reserve()` counts first for a reason. A read-only check followed by a send leaves a window in which concurrent callers all pass, and the counter you were protecting never moves until it's too late. + +### ❌ Don't read the clock twice around a reservation. + +`release()` takes the instant from the `Reservation`. If you re-derive it, a boundary crossing mid-request refunds a window that was never charged and leaves the charged one spent. + +### ❌ Don't return 429 from `magic/send` or `password/reset/start`. + +Those routes answer identically for addresses that exist and addresses that don't. A 429 tells the caller their address is real, which hands back the account enumeration those uniform responses exist to deny. Skip the send silently and return the normal 200, including the timing floor on `reset/start`. + +### ❌ Don't bucket on the raw address. + +`alice@gmail.com`, `alice+1@gmail.com` and `a.l.i.c.e@gmail.com` are one mailbox. Always pass addresses through `AddressCanonicalizer` (`OutboundEmailRateLimiter` does this for you), or one mailbox has an unlimited supply of fresh counters. + +### ❌ Don't read `X-Forwarded-For` or `CF-Connecting-IP` without `WORKOS_CLIENT_IP_HEADER` set. + +`ClientIp` uses `REMOTE_ADDR` unless an operator names a header explicitly. A forwarded-for header is only authoritative if the request actually came through the proxy that set it; if the origin is reachable directly, trusting one lets any caller mint a fresh bucket per request just by changing a string. That is strictly worse than a shared bucket. + +Even when the header is trusted, list values are read from the **right**: each hop appends, so everything right of the client was written by infrastructure and everything left of it is whatever the client sent. Private and reserved entries are skipped so internal hops don't shadow the client. + +### ❌ Don't call `time()` in limiter code. + +Use the injected `Clock`. It's what makes window rollover testable at all. + +## Scope + +Each mailbox gets its own counter, so the per-recipient limit bounds each mailbox individually; total volume across many mailboxes is bounded only by the per-IP limits. Mail WorkOS sends for flows the plugin doesn't proxy is out of scope entirely. + +## Testing + +Time-dependent behaviour uses the `Time` Codeception module, enabled in the wpunit suite. It swaps the container's `Clock` for one that only moves when told, and restores the real clock after each test. + +```php +$helper = $this->getModule( '\Helper\Time' ); +$clock = $helper->freezeTime( 1_800_000_030 ); + +$helper->travelSeconds( 60 ); // next window +``` + +For subjects you construct directly, pass a `FrozenClock` to the `RateLimiter` instead. + +Relative timestamps can't test any of this: the window boundary is derived from the clock, so `time() - 10` moves the data, not the boundary. diff --git a/src/WorkOS/Auth/AuthKit/Controller.php b/src/WorkOS/Auth/AuthKit/Controller.php index 31300be..f72ebd4 100644 --- a/src/WorkOS/Auth/AuthKit/Controller.php +++ b/src/WorkOS/Auth/AuthKit/Controller.php @@ -27,7 +27,6 @@ class Controller extends BaseController { protected function doRegister(): void { $this->container->singleton( ProfileRepository::class ); $this->container->singleton( ProfileRouter::class ); - $this->container->singleton( RateLimiter::class ); $this->container->singleton( Nonce::class ); $this->container->singleton( Radar::class ); $this->container->singleton( Renderer::class ); diff --git a/src/WorkOS/Auth/AuthKit/RateLimiter.php b/src/WorkOS/Auth/AuthKit/RateLimiter.php deleted file mode 100644 index 8081af4..0000000 --- a/src/WorkOS/Auth/AuthKit/RateLimiter.php +++ /dev/null @@ -1,270 +0,0 @@ -attempt_atomic( $bucket, $subject, $limit, $window_seconds, $now ); - } - - return $this->attempt_transient( $bucket, $subject, $limit, $window_seconds, $now ); - } - - /** - * Atomic attempt path backed by wp_cache_add + wp_cache_incr. - * - * `wp_cache_add` is CAS-safe when backed by a real object cache: the - * init of `first_seen` and `count` happens exactly once per window - * across concurrent callers. `wp_cache_incr` is an atomic in-place - * counter increment — no read-modify-write race. Two requests that - * both pass the init check get distinct `count` values, so the limit - * can never be exceeded under concurrency. - * - * @param string $bucket Operation identifier. - * @param string $subject Caller identity. - * @param int $limit Limit. - * @param int $window_seconds Window length. - * @param int $now Unix timestamp for this attempt. - * - * @return true|WP_Error - */ - private function attempt_atomic( string $bucket, string $subject, int $limit, int $window_seconds, int $now ) { - $cache_key = $this->cache_key( $bucket, $subject ); - $seen_key = $cache_key . ':first_seen'; - $count_key = $cache_key . ':count'; - - // CAS init: only the first caller in a fresh window writes these. - wp_cache_add( $seen_key, $now, self::CACHE_GROUP, $window_seconds ); - wp_cache_add( $count_key, 0, self::CACHE_GROUP, $window_seconds ); - - $first_seen = (int) wp_cache_get( $seen_key, self::CACHE_GROUP ); - if ( $first_seen <= 0 ) { - // Bucket evicted between add + get (rare). Recover. - wp_cache_set( $seen_key, $now, self::CACHE_GROUP, $window_seconds ); - wp_cache_set( $count_key, 0, self::CACHE_GROUP, $window_seconds ); - $first_seen = $now; - } - - $count = wp_cache_incr( $count_key, 1, self::CACHE_GROUP ); - if ( false === $count ) { - // Backend that doesn't support incr — degrade to the non-atomic path. - return $this->attempt_transient( $bucket, $subject, $limit, $window_seconds, $now ); - } - - if ( $count > $limit ) { - $retry_after = max( 1, ( $first_seen + $window_seconds ) - $now ); - return $this->limit_exceeded( $retry_after ); - } - - return true; - } - - /** - * Non-atomic fallback for sites without a persistent object cache. - * - * Under true concurrency a single over-limit attempt can slip past - * (two requests both read count=N-1 and both write N). WorkOS's own - * back-end rate limits (3-10/min on the user-facing endpoints we - * proxy) back-stop this; the plugin-level limiter is best-effort on - * non-object-cache installs. - * - * @param string $bucket Operation identifier. - * @param string $subject Caller identity. - * @param int $limit Limit. - * @param int $window_seconds Window length. - * @param int $now Unix timestamp for this attempt. - * - * @return true|WP_Error - */ - private function attempt_transient( string $bucket, string $subject, int $limit, int $window_seconds, int $now ) { - $key = $this->transient_key( $bucket, $subject ); - - $state = get_transient( $key ); - if ( ! is_array( $state ) || ! isset( $state['first_seen'], $state['count'] ) ) { - $state = [ - 'first_seen' => $now, - 'count' => 0, - ]; - } - - // Reset if the window has elapsed. - if ( ( $now - (int) $state['first_seen'] ) >= $window_seconds ) { - $state = [ - 'first_seen' => $now, - 'count' => 0, - ]; - } - - ++$state['count']; - - // Persist before the limit check so a concurrent caller also sees the increment. - // Transient TTL matches the remaining window so stale buckets drop on their own. - $ttl = max( 1, $window_seconds - ( $now - (int) $state['first_seen'] ) ); - set_transient( $key, $state, $ttl ); - - if ( $state['count'] > $limit ) { - $retry_after = max( 1, ( (int) $state['first_seen'] + $window_seconds ) - $now ); - return $this->limit_exceeded( $retry_after ); - } - - return true; - } - - /** - * Build the 429 WP_Error returned when a bucket is exhausted. - * - * @param int $retry_after Seconds the caller should wait. - * - * @return WP_Error - */ - private function limit_exceeded( int $retry_after ): WP_Error { - return new WP_Error( - 'workos_rate_limited', - __( 'Too many requests. Please try again shortly.', 'integration-workos' ), - [ - 'status' => 429, - 'retry_after' => $retry_after, - ] - ); - } - - /** - * Reset a bucket — intended for tests and administrative flushes. - * - * @param string $bucket Operation identifier. - * @param string $subject Caller identity. - * - * @return void - */ - public function reset( string $bucket, string $subject ): void { - delete_transient( $this->transient_key( $bucket, $subject ) ); - - if ( wp_using_ext_object_cache() ) { - $cache_key = $this->cache_key( $bucket, $subject ); - wp_cache_delete( $cache_key . ':first_seen', self::CACHE_GROUP ); - wp_cache_delete( $cache_key . ':count', self::CACHE_GROUP ); - } - } - - /** - * Build the object-cache key for a bucket + subject pair. - * - * Uses the same hashing approach as the transient path so personal - * data never sits in cache keys in plaintext. - * - * @param string $bucket Operation identifier. - * @param string $subject Subject (IP, email, etc.). - * - * @return string - */ - private function cache_key( string $bucket, string $subject ): string { - $safe_bucket = preg_replace( '/[^a-z0-9_]/i', '', $bucket ); - $hash = substr( hash( 'sha256', $subject ), 0, 32 ); - return $safe_bucket . '_' . $hash; - } - - /** - * Best-effort client IP extraction. - * - * Respects the standard WP proxy hint hierarchy without blindly trusting - * X-Forwarded-For (a caller can spoof the header otherwise). When no - * trusted IP can be determined, returns '0.0.0.0' so the caller still has - * *something* stable to rate-limit against. - * - * @return string IPv4 or IPv6 address string. - */ - public function client_ip(): string { - // REMOTE_ADDR is set by the webserver and cannot be spoofed by callers. - $remote = isset( $_SERVER['REMOTE_ADDR'] ) - ? filter_var( wp_unslash( $_SERVER['REMOTE_ADDR'] ), FILTER_VALIDATE_IP ) - : false; - - return is_string( $remote ) && '' !== $remote ? $remote : '0.0.0.0'; - } - - /** - * Normalize an email for consistent per-email bucketing. - * - * @param string $email Raw email string. - * - * @return string Lowercased, trimmed email; empty string if not an email. - */ - public function normalize_email( string $email ): string { - $email = strtolower( trim( $email ) ); - return is_email( $email ) ? $email : ''; - } - - /** - * Build the transient key for a bucket + subject pair. - * - * @param string $bucket Operation identifier. - * @param string $subject Subject (IP, email, etc.). - * - * @return string - */ - private function transient_key( string $bucket, string $subject ): string { - $safe_bucket = preg_replace( '/[^a-z0-9_]/i', '', $bucket ); - $hash = substr( hash( 'sha256', $subject ), 0, 32 ); - return self::TRANSIENT_PREFIX . $safe_bucket . '_' . $hash; - } -} diff --git a/src/WorkOS/Auth/ChangeEmail/RestApi.php b/src/WorkOS/Auth/ChangeEmail/RestApi.php index 24f34c0..b491d5a 100644 --- a/src/WorkOS/Auth/ChangeEmail/RestApi.php +++ b/src/WorkOS/Auth/ChangeEmail/RestApi.php @@ -8,7 +8,9 @@ namespace WorkOS\Auth\ChangeEmail; use WorkOS\ActivityLog\EventLogger; -use WorkOS\Auth\AuthKit\RateLimiter; +use WorkOS\RateLimit\OutboundEmailRateLimiter; +use WorkOS\Http\ClientIp; +use WorkOS\RateLimit\RateLimiter; use WorkOS\Email\AddressMask; use WP_Error; use WP_REST_Request; @@ -62,6 +64,20 @@ class RestApi { */ private RateLimiter $rate_limiter; + /** + * Client IP resolver. + * + * @var ClientIp + */ + private ClientIp $client_ip; + + /** + * Per-recipient limiter for mail we cause to be sent. + * + * @var OutboundEmailRateLimiter + */ + private OutboundEmailRateLimiter $outbound_email; + /** * Token factory. * @@ -105,27 +121,33 @@ class RestApi { * that helper requires a `Profile` (which doesn't apply to the * change-email flow). The same same-host policy is enforced. * - * @param RateLimiter $rate_limiter Rate limiter. - * @param TokenFactory $tokens Token factory. - * @param PendingChange $pending Pending-change storage. - * @param ConflictResolver $conflicts Conflict resolver. - * @param Notifier $notifier Email notifier. - * @param AddressMask $masker Email-address masker. + * @param RateLimiter $rate_limiter Rate limiter. + * @param ClientIp $client_ip Client IP resolver. + * @param OutboundEmailRateLimiter $outbound_email Per-recipient mail limiter. + * @param TokenFactory $tokens Token factory. + * @param PendingChange $pending Pending-change storage. + * @param ConflictResolver $conflicts Conflict resolver. + * @param Notifier $notifier Email notifier. + * @param AddressMask $masker Email-address masker. */ public function __construct( RateLimiter $rate_limiter, + ClientIp $client_ip, + OutboundEmailRateLimiter $outbound_email, TokenFactory $tokens, PendingChange $pending, ConflictResolver $conflicts, Notifier $notifier, AddressMask $masker ) { - $this->rate_limiter = $rate_limiter; - $this->tokens = $tokens; - $this->pending = $pending; - $this->conflicts = $conflicts; - $this->notifier = $notifier; - $this->masker = $masker; + $this->rate_limiter = $rate_limiter; + $this->client_ip = $client_ip; + $this->outbound_email = $outbound_email; + $this->tokens = $tokens; + $this->pending = $pending; + $this->conflicts = $conflicts; + $this->notifier = $notifier; + $this->masker = $masker; } /** @@ -275,7 +297,7 @@ public function initiate( WP_REST_Request $request ) { // Rate limits throttle self-service abuse; the capability-gated admin // path bypasses them so a legitimate cleanup sweep isn't blocked. if ( ! $is_admin_action ) { - $ip = $this->rate_limiter->client_ip(); + $ip = $this->client_ip->get(); $user_count = (int) workos()->option( 'change_email_rate_limit_user_count', self::RATE_LIMIT_DEFAULT_USER ); $user_window = (int) workos()->option( 'change_email_rate_limit_user_window', self::RATE_LIMIT_DEFAULT_WIN ); $ip_count = (int) workos()->option( 'change_email_rate_limit_ip_count', self::RATE_LIMIT_DEFAULT_IP ); @@ -346,6 +368,14 @@ public function initiate( WP_REST_Request $request ) { return $this->commit_admin_change( $user, $new_email ); } + // The counters above measure the requester; `$new_email` is caller-chosen + // and otherwise unbounded. Checked before the pending row is stored so we + // never record a change we can't email about. + $mail_ok = $this->outbound_email->reserve( $new_email ); + if ( is_wp_error( $mail_ok ) ) { + return $mail_ok; + } + $lifetime = $this->token_lifetime(); $expires = time() + $lifetime; @@ -366,6 +396,11 @@ public function initiate( WP_REST_Request $request ) { $cancel_url = $this->build_cancel_url( (int) $user->ID, $cancel_token ); $this->notifier->send_verification( $user, $new_email, $confirm_url, $expires ); + + // The notice to the account's own address is a second message and costs + // its own slot. Not gated: the user asked for this change, and refusing + // to tell them it happened is worse than going one over. + $this->outbound_email->reserve( (string) $user->user_email ); $this->notifier->send_old_address_notice( $user, $new_email, $cancel_url, $expires ); EventLogger::log( diff --git a/src/WorkOS/Auth/PasswordResetAdmin/RestApi.php b/src/WorkOS/Auth/PasswordResetAdmin/RestApi.php index c9e17e4..3151d97 100644 --- a/src/WorkOS/Auth/PasswordResetAdmin/RestApi.php +++ b/src/WorkOS/Auth/PasswordResetAdmin/RestApi.php @@ -10,7 +10,8 @@ use WorkOS\ActivityLog\EventLogger; use WorkOS\Auth\AuthKit\Profile; use WorkOS\Auth\AuthKit\ProfileRepository; -use WorkOS\Auth\AuthKit\RateLimiter; +use WorkOS\Http\ClientIp; +use WorkOS\RateLimit\HttpRateLimit; use WorkOS\Email\AddressMask; use WP_Error; use WP_REST_Request; @@ -34,9 +35,6 @@ class RestApi { public const NAMESPACE = 'workos/v1'; - private const RATE_LIMIT_IP_ATTEMPTS = 10; - private const RATE_LIMIT_USER_ATTEMPTS = 5; - private const RATE_LIMIT_WINDOW = 60; /** * Profile repository. @@ -48,9 +46,16 @@ class RestApi { /** * Rate limiter. * - * @var RateLimiter + * @var HttpRateLimit */ - private RateLimiter $rate_limiter; + private HttpRateLimit $http_rate_limit; + + /** + * Client IP resolver. + * + * @var ClientIp + */ + private ClientIp $client_ip; /** * Redirect URL validator. @@ -70,18 +75,21 @@ class RestApi { * Constructor. * * @param ProfileRepository $profiles Profile repository. - * @param RateLimiter $rate_limiter Rate limiter. + * @param HttpRateLimit $http_rate_limit Per-requester limits. + * @param ClientIp $client_ip Client IP resolver. * @param RedirectValidator $redirect_validator Redirect validator. * @param AddressMask $masker Email-address masker. */ public function __construct( ProfileRepository $profiles, - RateLimiter $rate_limiter, + HttpRateLimit $http_rate_limit, + ClientIp $client_ip, RedirectValidator $redirect_validator, AddressMask $masker ) { $this->profiles = $profiles; - $this->rate_limiter = $rate_limiter; + $this->http_rate_limit = $http_rate_limit; + $this->client_ip = $client_ip; $this->redirect_validator = $redirect_validator; $this->masker = $masker; } @@ -199,22 +207,8 @@ public function send_reset( WP_REST_Request $request ) { ); } - $ip = $this->rate_limiter->client_ip(); - $rate_ok = $this->rate_limiter->attempt( - 'pw_reset_admin_ip', - $ip, - self::RATE_LIMIT_IP_ATTEMPTS, - self::RATE_LIMIT_WINDOW - ); - if ( is_wp_error( $rate_ok ) ) { - return $rate_ok; - } - $rate_ok = $this->rate_limiter->attempt( - 'pw_reset_admin_user', - (string) $user->ID, - self::RATE_LIMIT_USER_ATTEMPTS, - self::RATE_LIMIT_WINDOW - ); + $ip = $this->client_ip->get(); + $rate_ok = $this->http_rate_limit->for_request( 'pw_reset_admin', $ip, (string) $user->ID ); if ( is_wp_error( $rate_ok ) ) { return $rate_ok; } diff --git a/src/WorkOS/Config.php b/src/WorkOS/Config.php index 0885e3f..02ea1a2 100644 --- a/src/WorkOS/Config.php +++ b/src/WorkOS/Config.php @@ -47,6 +47,26 @@ class Config { 'redirect_urls' => 'WORKOS_REDIRECT_URLS', ]; + /** + * Map of integer setting names to their PHP constant overrides. + * + * Read straight from the constant or environment rather than synced into + * an option row: these are deployment tuning, not editorial settings, and + * nothing in the admin edits them. + */ + private const INT_CONSTANT_MAP = [ + 'email_limit_5min' => 'WORKOS_EMAIL_LIMIT_5MIN', + 'email_limit_hour' => 'WORKOS_EMAIL_LIMIT_HOUR', + 'email_limit_day' => 'WORKOS_EMAIL_LIMIT_DAY', + ]; + + /** + * Map of plain-string setting names to their PHP constant overrides. + */ + private const STRING_CONSTANT_MAP = [ + 'client_ip_header' => 'WORKOS_CLIENT_IP_HEADER', + ]; + /** * Get the active environment. * @@ -167,6 +187,54 @@ public static function is_overridden( string $setting ): bool { return $generic_const && defined( $generic_const ) && '' !== constant( $generic_const ); } + /** + * Read a positive integer setting. + * + * Constant first, matching how every other override here resolves, then a + * real environment variable for containerised installs. Anything missing, + * unknown, non-numeric or non-positive falls back to the caller's default, + * so a typo degrades to the shipped value rather than to no limit at all. + * + * @param string $setting Key from {@see self::INT_CONSTANT_MAP}. + * @param int $fallback Value when unset. + * + * @return int + */ + public static function int_setting( string $setting, int $fallback ): int { + $value = self::read_constant( self::INT_CONSTANT_MAP[ $setting ] ?? '' ); + + return is_numeric( $value ) && (int) $value > 0 ? (int) $value : $fallback; + } + + /** + * Read a string setting. + * + * @param string $setting Key from {@see self::STRING_CONSTANT_MAP}. + * @param string $fallback Value when unset. + * + * @return string + */ + public static function string_setting( string $setting, string $fallback ): string { + $value = self::read_constant( self::STRING_CONSTANT_MAP[ $setting ] ?? '' ); + + return is_string( $value ) && '' !== $value ? $value : $fallback; + } + + /** + * Resolve a named constant, falling back to the environment. + * + * @param string $name Constant name; empty for an unmapped setting. + * + * @return mixed Null when there is nothing to read. + */ + private static function read_constant( string $name ) { + if ( '' === $name ) { + return null; + } + + return defined( $name ) ? constant( $name ) : getenv( $name ); + } + /** * Mask a secret value for UI display. * diff --git a/src/WorkOS/Controller.php b/src/WorkOS/Controller.php index d3174b8..c8daed7 100644 --- a/src/WorkOS/Controller.php +++ b/src/WorkOS/Controller.php @@ -16,6 +16,8 @@ use WorkOS\Auth\Controller as AuthController; use WorkOS\Auth\PasswordResetAdmin\Controller as PasswordResetAdminController; use WorkOS\Auth\ChangeEmail\Controller as ChangeEmailController; +use WorkOS\RateLimit\Controller as RateLimitController; +use WorkOS\Time\Controller as TimeController; use WorkOS\REST\Controller as RESTController; use WorkOS\Webhook\Controller as WebhookController; use WorkOS\Sync\Controller as SyncController; @@ -35,6 +37,11 @@ class Controller extends BaseController { * @return void */ protected function doRegister(): void { + // Order matters here: the rate limiter needs a Clock and a + // CounterStore, and the endpoint controllers need the rate limiter. + $this->container->register( TimeController::class ); + $this->container->register( RateLimitController::class ); + $this->container->register( AdminController::class ); $this->container->register( AuthKitController::class ); $this->container->register( LoginProfilesAdminController::class ); diff --git a/src/WorkOS/Email/AddressCanonicalizer.php b/src/WorkOS/Email/AddressCanonicalizer.php new file mode 100644 index 0000000..9f1920a --- /dev/null +++ b/src/WorkOS/Email/AddressCanonicalizer.php @@ -0,0 +1,83 @@ + 'gmail.com', + 'googlemail.com' => 'gmail.com', + ]; + + /** + * Lowercase and trim an address. + * + * @param string $email Raw email string. + * + * @return string Empty when the input isn't an address. + */ + public function normalize( string $email ): string { + $email = strtolower( trim( $email ) ); + + return is_email( $email ) ? $email : ''; + } + + /** + * Reduce an address to its mailbox. + * + * @param string $email Raw email string. + * + * @return string Canonical form, or empty when the input isn't an address. + */ + public function canonical( string $email ): string { + $email = $this->normalize( $email ); + if ( '' === $email ) { + return ''; + } + + $at = strrpos( $email, '@' ); + if ( false === $at ) { + return $email; + } + + $local = substr( $email, 0, $at ); + $domain = substr( $email, $at + 1 ); + + $plus = strpos( $local, '+' ); + if ( false !== $plus ) { + $local = substr( $local, 0, $plus ); + } + + if ( isset( self::DOT_INSENSITIVE_DOMAINS[ $domain ] ) ) { + $local = str_replace( '.', '', $local ); + $domain = self::DOT_INSENSITIVE_DOMAINS[ $domain ]; + } + + // A local part that was only a tag collapses to empty, which would + // merge unrelated addresses into one mailbox. + return '' !== $local ? $local . '@' . $domain : $email; + } +} diff --git a/src/WorkOS/Http/ClientIp.php b/src/WorkOS/Http/ClientIp.php new file mode 100644 index 0000000..5186bd7 --- /dev/null +++ b/src/WorkOS/Http/ClientIp.php @@ -0,0 +1,124 @@ +from_header( $header ); + + if ( '' !== $forwarded ) { + return $forwarded; + } + } + + return $this->remote_addr(); + } + + /** + * The address the webserver saw. Unforgeable, but possibly a proxy. + * + * @return string + */ + public function remote_addr(): string { + $remote = isset( $_SERVER['REMOTE_ADDR'] ) + ? filter_var( wp_unslash( $_SERVER['REMOTE_ADDR'] ), FILTER_VALIDATE_IP ) + : false; + + return is_string( $remote ) && '' !== $remote ? $remote : self::UNKNOWN; + } + + /** + * Pull a validated address out of a named request header. + * + * When the header carries a list, the *right-most public* entry wins. + * `X-Forwarded-For` appends on each hop, so everything to the right of + * the client was written by infrastructure and everything to the left is + * whatever the client chose to send — reading from the front hands the + * bucket key back to the attacker. Private and reserved entries are + * skipped on the way left so internal hops don't shadow the client; if + * the whole list is private (a local proxy in development), the + * right-most valid entry is still used. + * + * Returns empty rather than a fallback so the caller can decide. + * + * @param string $header Header name, e.g. `CF-Connecting-IP`. + * + * @return string Empty when absent or not an address. + */ + private function from_header( string $header ): string { + $key = 'HTTP_' . strtoupper( str_replace( '-', '_', $header ) ); + + if ( empty( $_SERVER[ $key ] ) ) { + return ''; + } + + $value = sanitize_text_field( wp_unslash( $_SERVER[ $key ] ) ); + $entries = array_reverse( explode( ',', $value ) ); + $fallback = ''; + + foreach ( $entries as $entry ) { + $candidate = filter_var( trim( $entry ), FILTER_VALIDATE_IP ); + + if ( ! is_string( $candidate ) ) { + continue; + } + + if ( '' === $fallback ) { + $fallback = $candidate; + } + + if ( false !== filter_var( $candidate, FILTER_VALIDATE_IP, FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE ) ) { + return $candidate; + } + } + + return $fallback; + } +} diff --git a/src/WorkOS/REST/Auth/BaseEndpoint.php b/src/WorkOS/REST/Auth/BaseEndpoint.php index e359ec9..8230616 100644 --- a/src/WorkOS/REST/Auth/BaseEndpoint.php +++ b/src/WorkOS/REST/Auth/BaseEndpoint.php @@ -12,7 +12,9 @@ use WorkOS\Auth\AuthKit\Profile; use WorkOS\Auth\AuthKit\ProfileRepository; use WorkOS\Auth\AuthKit\Radar; -use WorkOS\Auth\AuthKit\RateLimiter; +use WorkOS\Http\ClientIp; +use WorkOS\RateLimit\OutboundEmailRateLimiter; +use WorkOS\RateLimit\HttpRateLimit; use WP_Error; use WP_REST_Request; @@ -55,11 +57,25 @@ abstract class BaseEndpoint { protected Radar $radar; /** - * Rate limiter. + * Per-requester limits for this endpoint. * - * @var RateLimiter + * @var HttpRateLimit */ - protected RateLimiter $rate_limiter; + protected HttpRateLimit $http_rate_limit; + + /** + * Client IP resolver. + * + * @var ClientIp + */ + protected ClientIp $client_ip; + + /** + * Per-recipient limiter for mail we cause to be sent. + * + * @var OutboundEmailRateLimiter + */ + protected OutboundEmailRateLimiter $outbound_email; /** * Login completer. @@ -71,23 +87,29 @@ abstract class BaseEndpoint { /** * Constructor. * - * @param ProfileRepository $profiles Profile repository. - * @param Nonce $nonce Nonce helper. - * @param Radar $radar Radar helper. - * @param RateLimiter $rate_limiter Rate limiter. - * @param LoginCompleter $login_completer Login completer. + * @param ProfileRepository $profiles Profile repository. + * @param Nonce $nonce Nonce helper. + * @param Radar $radar Radar helper. + * @param HttpRateLimit $http_rate_limit Per-requester limits. + * @param ClientIp $client_ip Client IP resolver. + * @param OutboundEmailRateLimiter $outbound_email Per-recipient mail limiter. + * @param LoginCompleter $login_completer Login completer. */ public function __construct( ProfileRepository $profiles, Nonce $nonce, Radar $radar, - RateLimiter $rate_limiter, + HttpRateLimit $http_rate_limit, + ClientIp $client_ip, + OutboundEmailRateLimiter $outbound_email, LoginCompleter $login_completer ) { $this->profiles = $profiles; $this->nonce = $nonce; $this->radar = $radar; - $this->rate_limiter = $rate_limiter; + $this->http_rate_limit = $http_rate_limit; + $this->client_ip = $client_ip; + $this->outbound_email = $outbound_email; $this->login_completer = $login_completer; } @@ -102,7 +124,7 @@ abstract public function register_routes(): void; * Public permission check used for anonymous auth routes. * * Always returns true — access control is enforced per-request via - * {@see self::verify_nonce()} and {@see self::rate_limit()}. The + * {@see self::verify_nonce()} and {@see HttpRateLimit}. The * method exists so we can pass `[ $this, 'public_permission' ]` to * `register_rest_route()` without magic-method complexity. * @@ -169,32 +191,6 @@ protected function verify_nonce( WP_REST_Request $request, Profile $profile ) { return true; } - /** - * Apply rate limits for a request. - * - * Provide one entry per subject that should be throttled (e.g. IP and - * email). Each entry is `[bucket, subject, limit, window_seconds]`. - * First bucket to exceed wins and returns a 429. - * - * @param array $rules Rate-limit rules. - * - * @return true|WP_Error - */ - protected function rate_limit( array $rules ) { - foreach ( $rules as $rule ) { - [ $bucket, $subject, $limit, $window ] = $rule; - if ( '' === $subject ) { - continue; - } - $result = $this->rate_limiter->attempt( $bucket, $subject, $limit, $window ); - if ( is_wp_error( $result ) ) { - return $result; - } - } - - return true; - } - /** * Extract Radar action token from a request. * diff --git a/src/WorkOS/REST/Auth/Invitation.php b/src/WorkOS/REST/Auth/Invitation.php index fc10d07..da1e372 100644 --- a/src/WorkOS/REST/Auth/Invitation.php +++ b/src/WorkOS/REST/Auth/Invitation.php @@ -24,8 +24,6 @@ */ class Invitation extends BaseEndpoint { - private const RATE_LIMIT_IP_ATTEMPTS = 10; - private const RATE_LIMIT_WINDOW = 60; /** * Register routes. @@ -74,11 +72,7 @@ public function lookup( WP_REST_Request $request ) { ); } - $rate_ok = $this->rate_limit( - [ - [ 'invitation_lookup_ip', $this->rate_limiter->client_ip(), self::RATE_LIMIT_IP_ATTEMPTS, self::RATE_LIMIT_WINDOW ], - ] - ); + $rate_ok = $this->http_rate_limit->for_request( 'invitation_lookup', $this->client_ip->get() ); if ( is_wp_error( $rate_ok ) ) { return $rate_ok; } @@ -152,11 +146,7 @@ public function accept( WP_REST_Request $request ) { ); } - $rate_ok = $this->rate_limit( - [ - [ 'invitation_accept_ip', $this->rate_limiter->client_ip(), self::RATE_LIMIT_IP_ATTEMPTS, self::RATE_LIMIT_WINDOW ], - ] - ); + $rate_ok = $this->http_rate_limit->for_request( 'invitation_accept', $this->client_ip->get() ); if ( is_wp_error( $rate_ok ) ) { return $rate_ok; } diff --git a/src/WorkOS/REST/Auth/MagicCode.php b/src/WorkOS/REST/Auth/MagicCode.php index 23b8b8a..4578296 100644 --- a/src/WorkOS/REST/Auth/MagicCode.php +++ b/src/WorkOS/REST/Auth/MagicCode.php @@ -20,9 +20,6 @@ */ class MagicCode extends BaseEndpoint { - private const RATE_LIMIT_IP_ATTEMPTS = 10; - private const RATE_LIMIT_EMAIL_ATTEMPTS = 5; - private const RATE_LIMIT_WINDOW = 60; /** * Register routes. @@ -86,13 +83,8 @@ public function send( WP_REST_Request $request ) { ); } - $ip = $this->rate_limiter->client_ip(); - $rate_ok = $this->rate_limit( - [ - [ 'magic_send_ip', $ip, self::RATE_LIMIT_IP_ATTEMPTS, self::RATE_LIMIT_WINDOW ], - [ 'magic_send_email', $email, self::RATE_LIMIT_EMAIL_ATTEMPTS, self::RATE_LIMIT_WINDOW ], - ] - ); + $ip = $this->client_ip->get(); + $rate_ok = $this->http_rate_limit->for_request( 'magic_send', $ip, $email ); if ( is_wp_error( $rate_ok ) ) { return $rate_ok; } @@ -101,12 +93,17 @@ public function send( WP_REST_Request $request ) { // For unknown emails, skip the WorkOS call but still return success // so account existence isn't exposed. if ( $this->registration_allowed( $profile ) || get_user_by( 'email', $email ) ) { - // Fire-and-forget on the WorkOS call: delivery errors land in the - // plugin log rather than being surfaced to the client. - workos()->api()->send_magic_auth_code( - $email, - $this->get_radar_token( $request ) - ); + // Silent when the limit is reached: a 429 here would confirm the + // address is real, which is what the uniform 200 above denies. + if ( ! is_wp_error( $this->outbound_email->reserve( $email ) ) ) { + + // Fire-and-forget on the WorkOS call: delivery errors land in the + // plugin log rather than being surfaced to the client. + workos()->api()->send_magic_auth_code( + $email, + $this->get_radar_token( $request ) + ); + } } return new WP_REST_Response( @@ -156,13 +153,8 @@ public function verify( WP_REST_Request $request ) { ); } - $ip = $this->rate_limiter->client_ip(); - $rate_ok = $this->rate_limit( - [ - [ 'magic_verify_ip', $ip, self::RATE_LIMIT_IP_ATTEMPTS, self::RATE_LIMIT_WINDOW ], - [ 'magic_verify_email', $email, self::RATE_LIMIT_EMAIL_ATTEMPTS, self::RATE_LIMIT_WINDOW ], - ] - ); + $ip = $this->client_ip->get(); + $rate_ok = $this->http_rate_limit->for_request( 'magic_verify', $ip, $email ); if ( is_wp_error( $rate_ok ) ) { return $rate_ok; } diff --git a/src/WorkOS/REST/Auth/Mfa.php b/src/WorkOS/REST/Auth/Mfa.php index 10c007b..3fa3452 100644 --- a/src/WorkOS/REST/Auth/Mfa.php +++ b/src/WorkOS/REST/Auth/Mfa.php @@ -32,9 +32,6 @@ */ class Mfa extends BaseEndpoint { - private const RATE_LIMIT_IP_ATTEMPTS = 10; - private const RATE_LIMIT_FACTOR_ATTEMPTS = 5; - private const RATE_LIMIT_WINDOW = 60; /** * Register routes. @@ -153,12 +150,7 @@ public function challenge( WP_REST_Request $request ) { // IP-only limit: a user with multiple enrolled factors could // otherwise be targeted by rotating `factor_id` values from a // single IP and collecting a fresh OTP on each one. - $rate_ok = $this->rate_limit( - [ - [ 'mfa_challenge_ip', $this->rate_limiter->client_ip(), self::RATE_LIMIT_IP_ATTEMPTS, self::RATE_LIMIT_WINDOW ], - [ 'mfa_challenge_factor', $factor_id, self::RATE_LIMIT_FACTOR_ATTEMPTS, self::RATE_LIMIT_WINDOW ], - ] - ); + $rate_ok = $this->http_rate_limit->for_request( 'mfa_challenge', $this->client_ip->get(), $factor_id ); if ( is_wp_error( $rate_ok ) ) { return $rate_ok; } @@ -209,11 +201,7 @@ public function verify( WP_REST_Request $request ) { ); } - $rate_ok = $this->rate_limit( - [ - [ 'mfa_verify_ip', $this->rate_limiter->client_ip(), self::RATE_LIMIT_IP_ATTEMPTS, self::RATE_LIMIT_WINDOW ], - ] - ); + $rate_ok = $this->http_rate_limit->for_request( 'mfa_verify', $this->client_ip->get() ); if ( is_wp_error( $rate_ok ) ) { return $rate_ok; } diff --git a/src/WorkOS/REST/Auth/OAuth.php b/src/WorkOS/REST/Auth/OAuth.php index fd9f982..ac1fdb0 100644 --- a/src/WorkOS/REST/Auth/OAuth.php +++ b/src/WorkOS/REST/Auth/OAuth.php @@ -30,8 +30,14 @@ */ class OAuth extends BaseEndpoint { - private const RATE_LIMIT_IP_ATTEMPTS = 20; - private const RATE_LIMIT_WINDOW = 60; + /** + * Requests allowed per window from one address. + * + * Looser than the default posture: building a redirect URL costs + * nothing, and one login screen asks for several, so the default + * would bite real users first. + */ + private const URL_PER_IP = 20; /** * Mapping of profile method constants to WorkOS provider identifiers. @@ -98,11 +104,7 @@ public function authorize_url( WP_REST_Request $request ) { ); } - $rate_ok = $this->rate_limit( - [ - [ 'oauth_url_ip', $this->rate_limiter->client_ip(), self::RATE_LIMIT_IP_ATTEMPTS, self::RATE_LIMIT_WINDOW ], - ] - ); + $rate_ok = $this->http_rate_limit->for_request( 'oauth_url', $this->client_ip->get(), '', self::URL_PER_IP ); if ( is_wp_error( $rate_ok ) ) { return $rate_ok; } diff --git a/src/WorkOS/REST/Auth/Password.php b/src/WorkOS/REST/Auth/Password.php index 20d3ca3..76afa37 100644 --- a/src/WorkOS/REST/Auth/Password.php +++ b/src/WorkOS/REST/Auth/Password.php @@ -23,9 +23,6 @@ */ class Password extends BaseEndpoint { - private const RATE_LIMIT_IP_ATTEMPTS = 10; - private const RATE_LIMIT_EMAIL_ATTEMPTS = 5; - private const RATE_LIMIT_WINDOW = 60; /** * Minimum observable response time for reset_start, in microseconds. @@ -112,23 +109,33 @@ public function authenticate( WP_REST_Request $request ) { ); } - $ip = $this->rate_limiter->client_ip(); - $rate_ok = $this->rate_limit( - [ - [ 'password_ip', $ip, self::RATE_LIMIT_IP_ATTEMPTS, self::RATE_LIMIT_WINDOW ], - [ 'password_email', $email, self::RATE_LIMIT_EMAIL_ATTEMPTS, self::RATE_LIMIT_WINDOW ], - ] - ); + $ip = $this->client_ip->get(); + $rate_ok = $this->http_rate_limit->for_request( 'password', $ip, $email ); if ( is_wp_error( $rate_ok ) ) { return $rate_ok; } + // WorkOS mails a code as a side effect of authenticating an unverified + // account, so the slot is claimed before the call rather than after it. + // Anything below that turns out not to have sent gives it back, which + // is what stops a mistyped password counting as a send. + $mail_slot = $this->outbound_email->reserve( $email ); + if ( is_wp_error( $mail_slot ) ) { + return $mail_slot; + } + $workos_response = workos()->api()->authenticate_with_password( $email, $password, $this->get_radar_token( $request ) ); + if ( $this->is_email_verification_required( $workos_response ) ) { + // The fallback below can't help an unverified account, and its retry + // would mail a second code for the one request. + return $this->email_verification_required_error(); + } + if ( is_wp_error( $workos_response ) && workos()->option( 'allow_password_fallback', true ) ) { // WorkOS rejected the password — fall back to WordPress authentication. // Handles migrated users whose passwords were never synced to WorkOS. @@ -140,7 +147,8 @@ public function authenticate( WP_REST_Request $request ) { if ( $workos_user_id ) { if ( workos()->option( 'wp_password_fallback_email_confirmation', false ) ) { // Email confirmation path: identity is verified via a magic code - // instead of syncing the plaintext password to WorkOS. + // instead of syncing the plaintext password to WorkOS. Spends + // the slot reserved above. workos()->api()->send_magic_auth_code( $email, $this->get_radar_token( $request ) ); return new WP_REST_Response( @@ -165,6 +173,16 @@ public function authenticate( WP_REST_Request $request ) { } } + // The retry inside the fallback can land on the same unverified-account + // response and mail its own code, so it spends the reservation too. + if ( $this->is_email_verification_required( $workos_response ) ) { + return $this->email_verification_required_error(); + } + + // Everything from here sent nothing. Give the slot back so ordinary + // sign-ins, right or wrong, never count against the send limit. + $mail_slot->release(); + // `complete()` handles the `organization_selection_required` error // transparently when the profile has a pinned org. Pass the WP_Error // through so it can decide. @@ -216,13 +234,8 @@ public function reset_start( WP_REST_Request $request ) { ); } - $ip = $this->rate_limiter->client_ip(); - $rate_ok = $this->rate_limit( - [ - [ 'pw_reset_ip', $ip, self::RATE_LIMIT_IP_ATTEMPTS, self::RATE_LIMIT_WINDOW ], - [ 'pw_reset_email', $email, self::RATE_LIMIT_EMAIL_ATTEMPTS, self::RATE_LIMIT_WINDOW ], - ] - ); + $ip = $this->client_ip->get(); + $rate_ok = $this->http_rate_limit->for_request( 'pw_reset', $ip, $email ); if ( is_wp_error( $rate_ok ) ) { return $rate_ok; } @@ -240,11 +253,16 @@ public function reset_start( WP_REST_Request $request ) { // flattens both paths to the same observable wall-clock. $start_ns = hrtime( true ); - workos()->api()->send_password_reset( - $email, - $this->build_password_reset_url( $profile, $redirect_url ), - $this->get_radar_token( $request ) - ); + // Silent when the limit is reached: a 429 would break the uniform + // response this route uses to deny enumeration. + if ( ! is_wp_error( $this->outbound_email->reserve( $email ) ) ) { + + workos()->api()->send_password_reset( + $email, + $this->build_password_reset_url( $profile, $redirect_url ), + $this->get_radar_token( $request ) + ); + } $this->sleep_until_floor( $start_ns, self::RESPONSE_TIME_FLOOR_US ); @@ -294,12 +312,8 @@ public function reset_confirm( WP_REST_Request $request ) { ); } - $ip = $this->rate_limiter->client_ip(); - $rate_ok = $this->rate_limit( - [ - [ 'pw_reset_confirm_ip', $ip, self::RATE_LIMIT_IP_ATTEMPTS, self::RATE_LIMIT_WINDOW ], - ] - ); + $ip = $this->client_ip->get(); + $rate_ok = $this->http_rate_limit->for_request( 'pw_reset_confirm', $ip ); if ( is_wp_error( $rate_ok ) ) { return $rate_ok; } @@ -414,6 +428,41 @@ private function mirror_password_to_wp_user( $workos_response, string $new_passw wp_set_password( $new_password, $wp_user_id ); } + /** + * The 403 returned when WorkOS has mailed a verification code instead of + * signing the user in. + * + * @return WP_Error + */ + private function email_verification_required_error(): WP_Error { + return new WP_Error( + 'workos_authkit_email_verification_required', + __( 'Check your email for a verification code to finish signing in.', 'integration-workos' ), + [ 'status' => 403 ] + ); + } + + /** + * Did WorkOS reject this authenticate because the address is unverified? + * + * WorkOS mails a code as part of that same response, so this is how we + * learn after the fact that the request cost an inbox an email. + * + * @param mixed $workos_response Response or WP_Error from the API client. + * + * @return bool + */ + private function is_email_verification_required( $workos_response ): bool { + if ( ! is_wp_error( $workos_response ) ) { + return false; + } + + $data = $workos_response->get_error_data(); + $body = is_array( $data ) && isset( $data['body'] ) && is_array( $data['body'] ) ? $data['body'] : []; + + return 'email_verification_required' === (string) ( $body['code'] ?? '' ); + } + /** * Pull the user's email out of a WorkOS API response. * diff --git a/src/WorkOS/REST/Auth/Signup.php b/src/WorkOS/REST/Auth/Signup.php index b59ff9b..0d0069f 100644 --- a/src/WorkOS/REST/Auth/Signup.php +++ b/src/WorkOS/REST/Auth/Signup.php @@ -25,9 +25,20 @@ */ class Signup extends BaseEndpoint { - private const RATE_LIMIT_IP_ATTEMPTS = 5; - private const RATE_LIMIT_EMAIL_ATTEMPTS = 3; - private const RATE_LIMIT_WINDOW = 60; + /** + * Requests allowed per window from one address, for account creation. + * + * Tighter than the default posture: sign-up creates an account, and + * creating one against somebody else's address is what makes that + * address a target. Verifying isn't — it spends a code the user is + * retyping off their own screen, so it keeps the default allowance. + */ + private const CREATE_PER_IP = 5; + + /** + * Requests allowed per window against one address being signed up. + */ + private const CREATE_PER_SUBJECT = 3; /** * Register routes. @@ -104,13 +115,8 @@ public function create( WP_REST_Request $request ) { ); } - $ip = $this->rate_limiter->client_ip(); - $rate_ok = $this->rate_limit( - [ - [ 'signup_ip', $ip, self::RATE_LIMIT_IP_ATTEMPTS, self::RATE_LIMIT_WINDOW ], - [ 'signup_email', $email, self::RATE_LIMIT_EMAIL_ATTEMPTS, self::RATE_LIMIT_WINDOW ], - ] - ); + $ip = $this->client_ip->get(); + $rate_ok = $this->http_rate_limit->for_request( 'signup', $ip, $email, self::CREATE_PER_IP, self::CREATE_PER_SUBJECT ); if ( is_wp_error( $rate_ok ) ) { return $rate_ok; } @@ -136,7 +142,9 @@ static function ( $value ): bool { // If the user still needs to verify their email, kick off the code // email so the React shell can transition straight to the verify step. if ( empty( $user['email_verified'] ) && ! empty( $user['id'] ) ) { - workos()->api()->send_verification_email( $user['id'] ); + if ( ! is_wp_error( $this->outbound_email->reserve( $email ) ) ) { + workos()->api()->send_verification_email( $user['id'] ); + } } return new WP_REST_Response( @@ -181,12 +189,8 @@ public function verify( WP_REST_Request $request ) { ); } - $ip = $this->rate_limiter->client_ip(); - $rate_ok = $this->rate_limit( - [ - [ 'signup_verify_ip', $ip, self::RATE_LIMIT_IP_ATTEMPTS * 2, self::RATE_LIMIT_WINDOW ], - ] - ); + $ip = $this->client_ip->get(); + $rate_ok = $this->http_rate_limit->for_request( 'signup_verify', $ip ); if ( is_wp_error( $rate_ok ) ) { return $rate_ok; } diff --git a/src/WorkOS/RateLimit/Controller.php b/src/WorkOS/RateLimit/Controller.php new file mode 100644 index 0000000..81a46ba --- /dev/null +++ b/src/WorkOS/RateLimit/Controller.php @@ -0,0 +1,74 @@ +container->singleton( CounterStore::class, $this->resolve_store() ); + $this->container->singleton( RateLimiter::class ); + + // Requester limits are class-constant defaults; operations with a + // different posture pass their own numbers at the call site. Only the + // per-recipient email ceilings stay operator-tunable — they are the + // security-load-bearing ones, and mail volume is deployment-specific + // in a way per-endpoint request budgets are not. + $this->container->singleton( HttpRateLimit::class, HttpRateLimit::class ); + + // Limits are resolved here, once, and handed in as typed arguments. + // Keeps the limiter a thing you can construct with explicit numbers in + // a test rather than one that reaches for configuration on its own. + $this->container->when( OutboundEmailRateLimiter::class ) + ->needs( '$per_five_minutes' ) + ->give( static fn(): int => Config::int_setting( 'email_limit_5min', 3 ) ); + + $this->container->when( OutboundEmailRateLimiter::class ) + ->needs( '$per_hour' ) + ->give( static fn(): int => Config::int_setting( 'email_limit_hour', 10 ) ); + + $this->container->when( OutboundEmailRateLimiter::class ) + ->needs( '$per_day' ) + ->give( static fn(): int => Config::int_setting( 'email_limit_day', 20 ) ); + + $this->container->singleton( OutboundEmailRateLimiter::class, OutboundEmailRateLimiter::class ); + } + + /** + * Unregister the module. + * + * @return void + */ + protected function doUnregister(): void { + } + + /** + * Which store backs the counters. + * + * The object cache when there is one — it is faster and its `incr` is + * atomic on a real backend, so limits hold under concurrency. Transients + * otherwise, which every install has. + * + * @return string Store class name. + */ + private function resolve_store(): string { + return wp_using_ext_object_cache() ? ObjectCacheStore::class : TransientStore::class; + } +} diff --git a/src/WorkOS/RateLimit/CounterStore.php b/src/WorkOS/RateLimit/CounterStore.php new file mode 100644 index 0000000..6b33317 --- /dev/null +++ b/src/WorkOS/RateLimit/CounterStore.php @@ -0,0 +1,69 @@ +limiter = $limiter; + $this->per_ip = $per_ip; + $this->per_subject = $per_subject; + $this->window = $window; + } + + /** + * Apply both dimensions to one request. + * + * Buckets are derived from the operation name so a caller can't mistype + * one into a silently separate counter. Pass an empty subject for routes + * that don't act on a specific one. + * + * @param string $operation Operation name, e.g. `password`, `magic_send`. + * @param string $ip Caller address. + * @param string $subject Thing being acted on; empty to skip. + * @param int|null $per_ip Replacement for the default per-IP limit. + * @param int|null $per_subject Replacement for the default per-subject limit. + * + * @return true|WP_Error First dimension to refuse wins. + */ + public function for_request( string $operation, string $ip, string $subject = '', ?int $per_ip = null, ?int $per_subject = null ) { + $verdict = $this->for_ip( $operation . '_ip', $ip, $per_ip ); + if ( is_wp_error( $verdict ) ) { + return $verdict; + } + + if ( '' === $subject ) { + return true; + } + + return $this->for_subject( $operation . '_subject', $subject, $per_subject ); + } + + /** + * Count a request against its origin address. + * + * @param string $bucket Counter name. + * @param string $ip Caller address. + * @param int|null $limit Replacement for the instance's per-IP limit. + * + * @return true|WP_Error + */ + public function for_ip( string $bucket, string $ip, ?int $limit = null ) { + return $this->limiter->attempt( $bucket, $ip, $limit ?? $this->per_ip, $this->window ); + } + + /** + * Count a request against the thing it acts on. + * + * @param string $bucket Counter name. + * @param string $subject Email, account ID, factor ID. + * @param int|null $limit Replacement for the instance's per-subject limit. + * + * @return true|WP_Error + */ + public function for_subject( string $bucket, string $subject, ?int $limit = null ) { + return $this->limiter->attempt( $bucket, $subject, $limit ?? $this->per_subject, $this->window ); + } +} diff --git a/src/WorkOS/RateLimit/ObjectCacheStore.php b/src/WorkOS/RateLimit/ObjectCacheStore.php new file mode 100644 index 0000000..55ff2d9 --- /dev/null +++ b/src/WorkOS/RateLimit/ObjectCacheStore.php @@ -0,0 +1,99 @@ +get( $key ) + 1; + wp_cache_set( $key, $count, self::GROUP, $ttl ); + } + + return (int) $count; + } + + /** + * Subtract one from a counter and return its new value. + * + * @param string $key Opaque counter key. + * @param int $ttl Seconds until the counter should disappear. + * + * @return int The value after decrementing, never below zero. + */ + public function decrement( string $key, int $ttl ): int { + $count = wp_cache_decr( $key, 1, self::GROUP ); + + if ( false === $count ) { + $count = max( 0, $this->get( $key ) - 1 ); + wp_cache_set( $key, $count, self::GROUP, $ttl ); + } + + return max( 0, (int) $count ); + } + + /** + * Read a counter without changing it. + * + * @param string $key Opaque counter key. + * + * @return int Current value, or 0 when absent or expired. + */ + public function get( string $key ): int { + $value = wp_cache_get( $key, self::GROUP ); + + return false === $value ? 0 : (int) $value; + } + + /** + * Drop a counter. + * + * @param string $key Opaque counter key. + * + * @return void + */ + public function forget( string $key ): void { + wp_cache_delete( $key, self::GROUP ); + } +} diff --git a/src/WorkOS/RateLimit/OutboundEmailRateLimiter.php b/src/WorkOS/RateLimit/OutboundEmailRateLimiter.php new file mode 100644 index 0000000..8198858 --- /dev/null +++ b/src/WorkOS/RateLimit/OutboundEmailRateLimiter.php @@ -0,0 +1,142 @@ +limiter = $limiter; + $this->addresses = $addresses; + $this->per_five_minutes = $per_five_minutes; + $this->per_hour = $per_hour; + $this->per_day = $per_day; + } + + /** + * Windows this limit is enforced over. + * + * @return array List of [limit, window_seconds]. + */ + public function tiers(): array { + return [ + [ $this->per_five_minutes, 5 * MINUTE_IN_SECONDS ], + [ $this->per_hour, HOUR_IN_SECONDS ], + [ $this->per_day, DAY_IN_SECONDS ], + ]; + } + + /** + * Claim the right to send one message to an address. + * + * Call before sending. Senders that discover afterwards they sent + * nothing give the slot back with {@see self::release()}. + * + * @param string $email Destination address. + * + * @return Reservation|WP_Error WP_Error with status 429 when spent. + */ + public function reserve( string $email ) { + return $this->limiter->reserve( self::BUCKET, $this->canonical( $email ), $this->tiers() ); + } + + /** + * Reduce an address to the mailbox its limits apply to. + * + * @param string $email Raw email string. + * + * @return string Empty when not an address. + */ + public function canonical( string $email ): string { + return $this->addresses->canonical( $email ); + } +} diff --git a/src/WorkOS/RateLimit/RateLimiter.php b/src/WorkOS/RateLimit/RateLimiter.php new file mode 100644 index 0000000..6b3bbba --- /dev/null +++ b/src/WorkOS/RateLimit/RateLimiter.php @@ -0,0 +1,299 @@ +store = $store; + $this->clock = $clock; + } + + /** + * Count an event and rule on it. + * + * @param string $bucket Operation identifier. + * @param string $subject Thing being limited (IP, email, user ID). + * @param int $limit Events allowed per window. + * @param int $window_seconds Window length in seconds. + * + * @return true|WP_Error True when allowed; WP_Error with status 429 otherwise. + */ + public function attempt( string $bucket, string $subject, int $limit, int $window_seconds ) { + if ( ! $this->countable( $subject, $limit, $window_seconds ) ) { + return true; + } + + $now = $this->clock->now(); + $hits = $this->store->increment( $this->key( $bucket, $subject, $window_seconds, $now ), $this->ttl( $window_seconds, $now ) ); + + return $this->verdict( $hits, $limit, $window_seconds, $now ); + } + + /** + * Claim one slot across every window, up front. + * + * Counting *before* the action is what makes this safe under + * concurrency. A read-only check followed by a later debit leaves a gap + * as wide as the action itself, and every request arriving inside that + * gap reads the same count and passes — which is the whole trick a + * threaded request tool plays. + * + * All or nothing: if any window refuses, the slots already taken in the + * others are handed back before returning. + * + * @param string $bucket Operation identifier. + * @param string $subject Thing being limited. + * @param array $tiers List of [limit, window_seconds]. + * + * @return Reservation|WP_Error A handle to hand back if the work doesn't happen. + */ + public function reserve( string $bucket, string $subject, array $tiers ) { + $now = $this->clock->now(); + $refusal = null; + + foreach ( $tiers as $tier ) { + [ $limit, $window ] = $tier; + + if ( ! $this->countable( $subject, $limit, $window ) ) { + continue; + } + + $hits = $this->store->increment( $this->key( $bucket, $subject, $window, $now ), $this->ttl( $window, $now ) ); + $verdict = $this->verdict( $hits, $limit, $window, $now ); + + if ( is_wp_error( $verdict ) && null === $refusal ) { + $refusal = $verdict; + } + } + + if ( null !== $refusal ) { + $this->give_back( $bucket, $subject, $tiers, $now ); + + return $refusal; + } + + return new Reservation( $this, $bucket, $subject, $tiers, $now ); + } + + /** + * Hand back a reservation that was not spent. + * + * Prefer {@see Reservation::release()}; this is the plumbing behind it. + * + * @param Reservation $reservation Slot to return. + * + * @return void + */ + public function release( Reservation $reservation ): void { + $this->give_back( + $reservation->bucket(), + $reservation->subject(), + $reservation->tiers(), + $reservation->instant() + ); + } + + /** + * Decrement every window, as of a given instant. + * + * Takes the instant rather than reading the clock so a refund always + * lands in the window that was charged, even when the work either side + * of it straddled a window boundary. + * + * @param string $bucket Operation identifier. + * @param string $subject Thing being limited. + * @param array $tiers List of [limit, window_seconds]. + * @param int $now Instant the slot was claimed at. + * + * @return void + */ + private function give_back( string $bucket, string $subject, array $tiers, int $now ): void { + foreach ( $tiers as $tier ) { + [ $limit, $window ] = $tier; + + if ( ! $this->countable( $subject, $limit, $window ) ) { + continue; + } + + $this->store->decrement( + $this->key( $bucket, $subject, $window, $now ), + $this->ttl( $window, $now ) + ); + } + } + + /** + * Count an event against every window. No verdict — the caller has acted. + * + * @param string $bucket Operation identifier. + * @param string $subject Thing being limited. + * @param array $tiers List of [limit, window_seconds]. + * + * @return void + */ + public function consume( string $bucket, string $subject, array $tiers ): void { + $now = $this->clock->now(); + + foreach ( $tiers as $tier ) { + [ $limit, $window ] = $tier; + + if ( ! $this->countable( $subject, $limit, $window ) ) { + continue; + } + + $this->store->increment( + $this->key( $bucket, $subject, $window, $now ), + $this->ttl( $window, $now ) + ); + } + } + + /** + * Clear the current window for a subject — tests and admin flushes. + * + * @param string $bucket Operation identifier. + * @param string $subject Thing being limited. + * @param array $tiers List of [limit, window_seconds]. + * + * @return void + */ + public function reset( string $bucket, string $subject, array $tiers ): void { + $now = $this->clock->now(); + + foreach ( $tiers as $tier ) { + $window = (int) $tier[1]; + + if ( $window > 0 && '' !== $subject ) { + $this->store->forget( $this->key( $bucket, $subject, $window, $now ) ); + } + } + } + + /** + * Is this tier worth counting? + * + * A zero or negative limit or window means "no limit", and an empty + * subject means we have nothing to attribute the event to. + * + * @param string $subject Thing being limited. + * @param int $limit Events allowed. + * @param int $window Window length. + * + * @return bool + */ + private function countable( string $subject, int $limit, int $window ): bool { + return '' !== $subject && $limit > 0 && $window > 0; + } + + /** + * Allow or deny, given a count. + * + * @param int $hits Events counted in this window. + * @param int $limit Events allowed. + * @param int $window Window length. + * @param int $now Current timestamp. + * + * @return true|WP_Error + */ + private function verdict( int $hits, int $limit, int $window, int $now ) { + if ( $hits <= $limit ) { + return true; + } + + return new WP_Error( + 'workos_rate_limited', + __( 'Too many requests. Please try again shortly.', 'integration-workos' ), + [ + 'status' => 429, + 'retry_after' => $this->ttl( $window, $now ), + ] + ); + } + + /** + * Seconds left in the current window. + * + * @param int $window Window length. + * @param int $now Current timestamp. + * + * @return int At least 1. + */ + private function ttl( int $window, int $now ): int { + return max( 1, ( ( intdiv( $now, $window ) + 1 ) * $window ) - $now ); + } + + /** + * Build the store key for a bucket, subject and window. + * + * The window epoch is part of the key, which is what makes expiry + * arithmetic: a new window is simply a different key. + * + * @param string $bucket Operation identifier. + * @param string $subject Thing being limited. + * @param int $window Window length. + * @param int $now Current timestamp. + * + * @return string + */ + private function key( string $bucket, string $subject, int $window, int $now ): string { + $safe_bucket = preg_replace( '/[^a-z0-9_]/i', '', $bucket ); + $digest = hash( 'sha256', $subject . '|' . $window . '|' . intdiv( $now, $window ) ); + + return self::KEY_PREFIX . $safe_bucket . '_' . substr( $digest, 0, 40 ); + } +} diff --git a/src/WorkOS/RateLimit/Reservation.php b/src/WorkOS/RateLimit/Reservation.php new file mode 100644 index 0000000..ad89352 --- /dev/null +++ b/src/WorkOS/RateLimit/Reservation.php @@ -0,0 +1,139 @@ + + */ + private array $tiers; + + /** + * Unix timestamp at which the slot was claimed. + * + * @var int + */ + private int $instant; + + /** + * Whether the slot has already been handed back. + * + * @var bool + */ + private bool $released = false; + + /** + * Constructor. + * + * @param RateLimiter $limiter Limiter that issued this reservation. + * @param string $bucket Operation identifier. + * @param string $subject Thing being limited. + * @param array $tiers List of [limit, window_seconds]. + * @param int $instant Unix timestamp at which the slot was claimed. + */ + public function __construct( RateLimiter $limiter, string $bucket, string $subject, array $tiers, int $instant ) { + $this->limiter = $limiter; + $this->bucket = $bucket; + $this->subject = $subject; + $this->tiers = $tiers; + $this->instant = $instant; + } + + /** + * Hand the slot back, because the work it paid for didn't happen. + * + * Idempotent: releasing twice would credit a window that was only ever + * charged once. + * + * @return void + */ + public function release(): void { + if ( $this->released ) { + return; + } + + $this->released = true; + + $this->limiter->release( $this ); + } + + /** + * Operation identifier. + * + * @return string + */ + public function bucket(): string { + return $this->bucket; + } + + /** + * Thing being limited. + * + * @return string + */ + public function subject(): string { + return $this->subject; + } + + /** + * Windows this reservation was taken against. + * + * @return array + */ + public function tiers(): array { + return $this->tiers; + } + + /** + * Unix timestamp at which the slot was claimed. + * + * @return int + */ + public function instant(): int { + return $this->instant; + } +} diff --git a/src/WorkOS/RateLimit/TransientStore.php b/src/WorkOS/RateLimit/TransientStore.php new file mode 100644 index 0000000..54dfdc4 --- /dev/null +++ b/src/WorkOS/RateLimit/TransientStore.php @@ -0,0 +1,81 @@ +get( $key ) + 1; + + set_transient( $key, $hits, $ttl ); + + return $hits; + } + + /** + * Subtract one from a counter and return its new value. + * + * @param string $key Opaque counter key. + * @param int $ttl Seconds until the counter should disappear. + * + * @return int The value after decrementing, never below zero. + */ + public function decrement( string $key, int $ttl ): int { + $hits = max( 0, $this->get( $key ) - 1 ); + + set_transient( $key, $hits, $ttl ); + + return $hits; + } + + /** + * Read a counter without changing it. + * + * @param string $key Opaque counter key. + * + * @return int Current value, or 0 when absent or expired. + */ + public function get( string $key ): int { + $hits = get_transient( $key ); + + return false === $hits ? 0 : (int) $hits; + } + + /** + * Drop a counter. + * + * @param string $key Opaque counter key. + * + * @return void + */ + public function forget( string $key ): void { + delete_transient( $key ); + } +} diff --git a/src/WorkOS/Time/Clock.php b/src/WorkOS/Time/Clock.php new file mode 100644 index 0000000..93eba54 --- /dev/null +++ b/src/WorkOS/Time/Clock.php @@ -0,0 +1,27 @@ +container->singleton( Clock::class, SystemClock::class ); + $this->container->singleton( SystemClock::class ); + } + + /** + * Unregister the module. + * + * @return void + */ + protected function doUnregister(): void { + } +} diff --git a/src/WorkOS/Time/FrozenClock.php b/src/WorkOS/Time/FrozenClock.php new file mode 100644 index 0000000..52a970f --- /dev/null +++ b/src/WorkOS/Time/FrozenClock.php @@ -0,0 +1,68 @@ +instant = $instant; + } + + /** + * The current Unix timestamp. + * + * @return int + */ + public function now(): int { + return $this->instant; + } + + /** + * Move forward. + * + * @param int $seconds Seconds to advance. + * + * @return void + */ + public function travel( int $seconds ): void { + $this->instant += $seconds; + } + + /** + * Jump to a specific instant. + * + * @param int $instant Unix timestamp. + * + * @return void + */ + public function set( int $instant ): void { + $this->instant = $instant; + } +} diff --git a/src/WorkOS/Time/SystemClock.php b/src/WorkOS/Time/SystemClock.php new file mode 100644 index 0000000..1a7bc32 --- /dev/null +++ b/src/WorkOS/Time/SystemClock.php @@ -0,0 +1,25 @@ +frozen = new FrozenClock( $instant ?? time() ); + + App::container()->singleton( Clock::class, $this->frozen ); + + return $this->frozen; + } + + /** + * Move the frozen clock forward. + * + * @param int $seconds Seconds to advance. + * + * @return void + */ + public function travelSeconds( int $seconds ): void { + if ( ! $this->frozen instanceof FrozenClock ) { + $this->fail( 'travelSeconds() needs freezeTime() first.' ); + } + + $this->frozen->travel( $seconds ); + } + + /** + * Put the real clock back. + * + * @return void + */ + public function unfreezeTime(): void { + $this->frozen = null; + + App::container()->singleton( Clock::class, new SystemClock() ); + } + + /** + * Restore the real clock between tests so a freeze can't leak. + * + * @param TestInterface $test Test that just ran. + * + * @return void + */ + public function _after( TestInterface $test ): void { + if ( $this->frozen instanceof FrozenClock ) { + $this->unfreezeTime(); + } + } +} diff --git a/tests/wpunit.suite.dist.yml b/tests/wpunit.suite.dist.yml index d222d4d..eb58aa4 100644 --- a/tests/wpunit.suite.dist.yml +++ b/tests/wpunit.suite.dist.yml @@ -4,6 +4,7 @@ modules: enabled: - WPLoader - "\\Helper\\Wpunit" + - "\\Helper\\Time" config: WPLoader: wpRootFolder: "%WP_ROOT_FOLDER%" diff --git a/tests/wpunit/AuthKitRateLimiterTest.php b/tests/wpunit/AuthKitRateLimiterTest.php index dfba08e..224a3c4 100644 --- a/tests/wpunit/AuthKitRateLimiterTest.php +++ b/tests/wpunit/AuthKitRateLimiterTest.php @@ -7,8 +7,18 @@ namespace WorkOS\Tests\Wpunit; +use Codeception\Attribute\DataProvider; use lucatume\WPBrowser\TestCase\WPTestCase; -use WorkOS\Auth\AuthKit\RateLimiter; +use WorkOS\App; +use WorkOS\Email\AddressCanonicalizer; +use WorkOS\Http\ClientIp; +use WorkOS\RateLimit\OutboundEmailRateLimiter; +use WorkOS\RateLimit\RateLimiter; +use WorkOS\RateLimit\Reservation; +use WorkOS\RateLimit\TransientStore; +use WorkOS\Time\Clock; +use WorkOS\Time\FrozenClock; +use WorkOS\Time\SystemClock; use WP_Error; /** @@ -23,14 +33,21 @@ class AuthKitRateLimiterTest extends WPTestCase { */ private RateLimiter $limiter; + /** + * Per-recipient mail limiter under test. + * + * @var OutboundEmailRateLimiter + */ + private OutboundEmailRateLimiter $outbound; + /** * Set up each test. */ public function setUp(): void { parent::setUp(); - $this->limiter = new RateLimiter(); - $this->limiter->reset( 'test_bucket', '192.168.0.1' ); - $this->limiter->reset( 'test_bucket', 'alice@example.com' ); + + $this->limiter = new RateLimiter( new TransientStore(), new SystemClock() ); + $this->outbound = new OutboundEmailRateLimiter( $this->limiter, new AddressCanonicalizer(), 3, 10, 20 ); } /** @@ -91,42 +108,242 @@ public function test_reset_clears_bucket(): void { } $this->assertInstanceOf( WP_Error::class, $this->limiter->attempt( 'test_bucket', '192.168.0.1', 3, 60 ) ); - $this->limiter->reset( 'test_bucket', '192.168.0.1' ); + $this->limiter->reset( 'test_bucket', '192.168.0.1', [ [ 3, 60 ] ] ); $this->assertTrue( $this->limiter->attempt( 'test_bucket', '192.168.0.1', 3, 60 ) ); } /** - * Email normalization lowercases and trims; non-emails return empty. + * Aliases of one mailbox share a single limit. */ - public function test_normalize_email(): void { - $this->assertSame( 'alice@example.com', $this->limiter->normalize_email( ' Alice@Example.COM ' ) ); - $this->assertSame( '', $this->limiter->normalize_email( 'not-an-email' ) ); - $this->assertSame( '', $this->limiter->normalize_email( '' ) ); + public function test_outbound_limit_is_shared_across_aliases(): void { + // Short tier is 3 per 5 minutes, spent here across three spellings. + $this->assertInstanceOf( Reservation::class, $this->outbound->reserve( 'victim@gmail.com' ) ); + $this->assertInstanceOf( Reservation::class, $this->outbound->reserve( 'vic.tim+1@gmail.com' ) ); + $this->assertInstanceOf( Reservation::class, $this->outbound->reserve( 'v.i.c.t.i.m+2@googlemail.com' ) ); + + $this->assertInstanceOf( WP_Error::class, $this->outbound->reserve( 'victim+3@gmail.com' ) ); } /** - * client_ip falls back to 0.0.0.0 when REMOTE_ADDR is unset. + * A refused reservation doesn't leave the partially-taken slots behind. + * + * The short window refuses first; the hour and day windows it already + * incremented have to be given back or a rejected request would still + * count against the limit. */ - public function test_client_ip_returns_fallback_when_unset(): void { - $previous = $_SERVER['REMOTE_ADDR'] ?? null; - unset( $_SERVER['REMOTE_ADDR'] ); + public function test_refused_reservation_is_not_charged(): void { + $slots = []; + for ( $i = 0; $i < 3; $i++ ) { + $slots[] = $this->outbound->reserve( 'alice@example.com' ); + } - $this->assertSame( '0.0.0.0', $this->limiter->client_ip() ); + // Two refusals inside the same short window. + $this->assertInstanceOf( WP_Error::class, $this->outbound->reserve( 'alice@example.com' ) ); + $this->assertInstanceOf( WP_Error::class, $this->outbound->reserve( 'alice@example.com' ) ); - if ( null !== $previous ) { - $_SERVER['REMOTE_ADDR'] = $previous; + // Freeing one real send is enough to let the next through. It wouldn't + // be if the two refusals had each left a charge behind. + $slots[0]->release(); + + $this->assertInstanceOf( Reservation::class, $this->outbound->reserve( 'alice@example.com' ) ); + } + + /** + * Releasing an unspent reservation frees the slot again. + */ + public function test_release_returns_the_slot(): void { + $slots = []; + for ( $i = 0; $i < 3; $i++ ) { + $slots[] = $this->outbound->reserve( 'alice@example.com' ); } + $this->assertInstanceOf( WP_Error::class, $this->outbound->reserve( 'alice@example.com' ) ); + + $slots[0]->release(); + + $this->assertInstanceOf( Reservation::class, $this->outbound->reserve( 'alice@example.com' ) ); } /** - * client_ip uses REMOTE_ADDR when set to a valid IP. + * Releasing twice would credit a window that was only charged once. + */ + public function test_release_is_idempotent(): void { + $slots = []; + for ( $i = 0; $i < 3; $i++ ) { + $slots[] = $this->outbound->reserve( 'alice@example.com' ); + } + + $slots[0]->release(); + $slots[0]->release(); + + // One slot back, not two. + $this->assertInstanceOf( Reservation::class, $this->outbound->reserve( 'alice@example.com' ) ); + $this->assertInstanceOf( WP_Error::class, $this->outbound->reserve( 'alice@example.com' ) ); + } + + /** + * A refund lands in the window it was charged to, not the current one. + * + * The reservation carries the instant it was taken, so a boundary passing + * while the work is in flight cannot send the credit to the next window + * and leave the original charged. + */ + public function test_release_targets_the_window_it_was_charged_to(): void { + $tiers = [ [ 2, 60 ] ]; + + $slot = $this->limiter->reserve( 'test_straddle', 'alice@example.com', $tiers ); + $this->assertInstanceOf( Reservation::class, $slot ); + + // The window the slot belongs to is fixed at reservation time. + $this->assertSame( intdiv( $slot->instant(), 60 ), intdiv( time(), 60 ) ); + + $slot->release(); + + // Two more fit, so the charge really came back off this window. + $this->assertInstanceOf( Reservation::class, $this->limiter->reserve( 'test_straddle', 'alice@example.com', $tiers ) ); + $this->assertInstanceOf( Reservation::class, $this->limiter->reserve( 'test_straddle', 'alice@example.com', $tiers ) ); + } + + /** + * Addresses that reach one mailbox reduce to one string. + * + * @param string $raw Address as typed. + * @param string $expected Mailbox it belongs to. + */ + #[DataProvider( 'canonicalAddresses' )] + public function test_canonical_addressing( string $raw, string $expected ): void { + $this->assertSame( $expected, $this->outbound->canonical( $raw ) ); + } + + /** + * The Time module reaches code resolved from the container. + * + * The direct-construction tests below prove the limiter's own behaviour. + * This proves the module's freeze actually lands, so an endpoint test + * that never touches a Clock itself can still move one. + */ + public function test_time_module_freezes_the_container_clock(): void { + /** @var \Helper\Time $helper */ + $helper = $this->getModule( '\\Helper\\Time' ); + + $clock = $helper->freezeTime( 1_800_000_030 ); + + $resolved = App::container()->get( Clock::class ); + $this->assertSame( 1_800_000_030, $resolved->now() ); + + $helper->travelSeconds( 90 ); + $this->assertSame( 1_800_000_120, $resolved->now() ); + $this->assertSame( $clock, $resolved ); + + $helper->unfreezeTime(); + $this->assertNotSame( $clock, App::container()->get( Clock::class ) ); + } + + /** + * A short window really does reset once its boundary passes. + * + * The window is `floor( now / length )`, so this is only observable with + * a clock we can move: relative timestamps can't shift the boundary. */ - public function test_client_ip_uses_remote_addr(): void { - $previous = $_SERVER['REMOTE_ADDR'] ?? null; - $_SERVER['REMOTE_ADDR'] = '203.0.113.4'; + public function test_short_window_resets_and_long_window_persists(): void { + // Start mid-window so the jump below lands cleanly in the next one. + $clock = new FrozenClock( 1_800_000_030 ); + $limiter = new RateLimiter( new TransientStore(), $clock ); + $tiers = [ [ 2, 60 ], [ 3, HOUR_IN_SECONDS ] ]; - $this->assertSame( '203.0.113.4', $this->limiter->client_ip() ); + $this->assertInstanceOf( Reservation::class, $limiter->reserve( 'test_roll', 'alice@example.com', $tiers ) ); + $this->assertInstanceOf( Reservation::class, $limiter->reserve( 'test_roll', 'alice@example.com', $tiers ) ); + $this->assertInstanceOf( WP_Error::class, $limiter->reserve( 'test_roll', 'alice@example.com', $tiers ) ); + + $clock->travel( 60 ); + + // Fresh minute: the third request now fits. + $this->assertInstanceOf( Reservation::class, $limiter->reserve( 'test_roll', 'alice@example.com', $tiers ) ); + + // But the hour window carried all three across, and it allows three. + $this->assertInstanceOf( WP_Error::class, $limiter->reserve( 'test_roll', 'alice@example.com', $tiers ) ); + } + + /** + * A refund crossing a window boundary credits the window it was charged. + * + * Reserve in one window, let the boundary pass while the work is in + * flight, then release. The credit has to land on the window that was + * charged — not on whichever one happens to be current, which would both + * leave the original spent and gift a slot to the new one. + */ + public function test_release_after_boundary_does_not_credit_the_new_window(): void { + $clock = new FrozenClock( 1_800_000_030 ); + $limiter = new RateLimiter( new TransientStore(), $clock ); + $tiers = [ [ 2, 60 ] ]; + + // Charged against the first window. + $slot = $limiter->reserve( 'test_straddle', 'alice@example.com', $tiers ); + $this->assertInstanceOf( Reservation::class, $slot ); + + // Boundary passes while the request is still in flight. + $clock->travel( 60 ); + + // Fill the new window to its limit. + $this->assertInstanceOf( Reservation::class, $limiter->reserve( 'test_straddle', 'alice@example.com', $tiers ) ); + $this->assertInstanceOf( Reservation::class, $limiter->reserve( 'test_straddle', 'alice@example.com', $tiers ) ); + $this->assertInstanceOf( WP_Error::class, $limiter->reserve( 'test_straddle', 'alice@example.com', $tiers ) ); + + // The refund belongs to the old window, so the new one stays full. + $slot->release(); + + $this->assertInstanceOf( WP_Error::class, $limiter->reserve( 'test_straddle', 'alice@example.com', $tiers ) ); + } + + /** + * A long window still bites after a short one has rolled over. + * + * Fixed windows reset wholesale, so a single window is burstable by + * waiting out its boundary. The longer tiers are what bound the total. + */ + public function test_long_tier_survives_short_window_rollover(): void { + $tiers = [ [ 100, 60 ], [ 4, HOUR_IN_SECONDS ] ]; + + for ( $i = 0; $i < 4; $i++ ) { + $this->assertInstanceOf( Reservation::class, $this->limiter->reserve( 'test_tiered', 'alice@example.com', $tiers ) ); + } + + // The minute tier has plenty of headroom; the hour tier is spent. + $result = $this->limiter->reserve( 'test_tiered', 'alice@example.com', $tiers ); + $this->assertInstanceOf( WP_Error::class, $result ); + $this->assertSame( 429, $result->get_error_data()['status'] ); + } + + /** + * Which address a request is attributed to. + * + * @param string|null $remote_addr REMOTE_ADDR, or null when absent. + * @param string|null $header_name Header an operator has declared trusted. + * @param string|null $header_value Value that header carries. + * @param string $expected Address the limiter should count against. + */ + #[DataProvider( 'clientIpCases' )] + public function test_client_ip_resolution( ?string $remote_addr, ?string $header_name, ?string $header_value, string $expected ): void { + $previous = $_SERVER['REMOTE_ADDR'] ?? null; + + if ( null === $remote_addr ) { + unset( $_SERVER['REMOTE_ADDR'] ); + } else { + $_SERVER['REMOTE_ADDR'] = $remote_addr; + } + + if ( null !== $header_value ) { + $_SERVER['HTTP_CF_CONNECTING_IP'] = $header_value; + } + + if ( null !== $header_name ) { + putenv( 'WORKOS_CLIENT_IP_HEADER=' . $header_name ); + } + + $this->assertSame( $expected, ( new ClientIp() )->get() ); + + putenv( 'WORKOS_CLIENT_IP_HEADER' ); + unset( $_SERVER['HTTP_CF_CONNECTING_IP'] ); if ( null === $previous ) { unset( $_SERVER['REMOTE_ADDR'] ); @@ -134,4 +351,122 @@ public function test_client_ip_uses_remote_addr(): void { $_SERVER['REMOTE_ADDR'] = $previous; } } + + // ------------------------------------------------------------------------- + // Data providers + // ------------------------------------------------------------------------- + + /** + * Addresses paired with the mailbox they reduce to. + * + * @return array + */ + public function canonicalAddresses(): array { + return [ + 'gmail drops dots and tags' => [ + 'a.l.i.c.e+spam@gmail.com', + 'alice@gmail.com', + ], + 'googlemail folds to gmail' => [ + 'Alice@googlemail.com', + 'alice@gmail.com', + ], + 'tags stripped everywhere' => [ + 'alice+tag@example.com', + 'alice@example.com', + ], + 'dots kept off alias domains' => [ + 'a.lice@example.com', + 'a.lice@example.com', + ], + 'uppercase folded' => [ + ' Alice@Example.COM ', + 'alice@example.com', + ], + 'tag-only local part kept' => [ + '+tag@example.com', + '+tag@example.com', + ], + 'not an address' => [ + 'not-an-email', + '', + ], + 'empty' => [ + '', + '', + ], + ]; + } + + /** + * Request conditions paired with the address they resolve to. + * + * Columns: REMOTE_ADDR, trusted header name, header value, expected. + * + * @return array + */ + public function clientIpCases(): array { + return [ + 'remote addr only' => [ + '203.0.113.4', + null, + null, + '203.0.113.4', + ], + 'no remote addr' => [ + null, + null, + null, + '0.0.0.0', + ], + 'undeclared header ignored' => [ + '203.0.113.4', + null, + '198.51.100.7', + '203.0.113.4', + ], + 'declared header wins' => [ + '203.0.113.4', + 'CF-Connecting-IP', + '198.51.100.7', + '198.51.100.7', + ], + 'declared header absent' => [ + '203.0.113.4', + 'CF-Connecting-IP', + null, + '203.0.113.4', + ], + 'declared header is garbage' => [ + '203.0.113.4', + 'CF-Connecting-IP', + 'not-an-ip', + '203.0.113.4', + ], + 'private hop skipped, client kept' => [ + '203.0.113.4', + 'CF-Connecting-IP', + '198.51.100.7, 10.0.0.1', + '198.51.100.7', + ], + 'client-prepended entry ignored' => [ + '203.0.113.4', + 'CF-Connecting-IP', + '6.6.6.6, 198.51.100.7', + '198.51.100.7', + ], + 'all-private list uses right-most' => [ + '203.0.113.4', + 'CF-Connecting-IP', + '10.0.0.2, 10.0.0.1', + '10.0.0.1', + ], + 'garbage entries skipped' => [ + '203.0.113.4', + 'CF-Connecting-IP', + '198.51.100.7, not-an-ip', + '198.51.100.7', + ], + ]; + } } diff --git a/tests/wpunit/AuthKitRestMagicSessionTest.php b/tests/wpunit/AuthKitRestMagicSessionTest.php index 95ef28c..d24296e 100644 --- a/tests/wpunit/AuthKitRestMagicSessionTest.php +++ b/tests/wpunit/AuthKitRestMagicSessionTest.php @@ -15,9 +15,15 @@ use WP_REST_Response; use WorkOS\Auth\AuthKit\ProfileRepository; use WorkOS\Auth\AuthKit\Radar; -use WorkOS\Auth\AuthKit\RateLimiter; +use WorkOS\Email\AddressCanonicalizer; +use WorkOS\Http\ClientIp; +use WorkOS\RateLimit\HttpRateLimit; +use WorkOS\RateLimit\OutboundEmailRateLimiter; +use WorkOS\RateLimit\RateLimiter; +use WorkOS\RateLimit\TransientStore; use WorkOS\REST\Auth\MagicCode; use WorkOS\REST\Auth\Session; +use WorkOS\Time\SystemClock; /** * REST dispatch coverage for /auth/magic/* and /auth/{nonce,session}/*. @@ -100,11 +106,13 @@ public function setUp(): void { $this->nonce = new Nonce(); $radar = new Radar(); - $rate_limiter = new RateLimiter(); + $rate_limiter = new RateLimiter( new TransientStore(), new SystemClock() ); + $http = new HttpRateLimit( $rate_limiter, 10, 5, 60 ); + $outbound = new OutboundEmailRateLimiter( $rate_limiter, new AddressCanonicalizer(), 3, 10, 20 ); $completer = new LoginCompleter(); - $magic = new MagicCode( $this->repository, $this->nonce, $radar, $rate_limiter, $completer ); - $session = new Session( $this->repository, $this->nonce, $radar, $rate_limiter, $completer ); + $magic = new MagicCode( $this->repository, $this->nonce, $radar, $http, new ClientIp(), $outbound, $completer ); + $session = new Session( $this->repository, $this->nonce, $radar, $http, new ClientIp(), $outbound, $completer ); add_action( 'rest_api_init', [ $magic, 'register_routes' ] ); add_action( 'rest_api_init', [ $session, 'register_routes' ] ); diff --git a/tests/wpunit/AuthKitRestMfaTest.php b/tests/wpunit/AuthKitRestMfaTest.php index a470fbf..463c496 100644 --- a/tests/wpunit/AuthKitRestMfaTest.php +++ b/tests/wpunit/AuthKitRestMfaTest.php @@ -15,8 +15,14 @@ use WP_REST_Response; use WorkOS\Auth\AuthKit\ProfileRepository; use WorkOS\Auth\AuthKit\Radar; -use WorkOS\Auth\AuthKit\RateLimiter; +use WorkOS\Email\AddressCanonicalizer; +use WorkOS\Http\ClientIp; +use WorkOS\RateLimit\HttpRateLimit; +use WorkOS\RateLimit\OutboundEmailRateLimiter; +use WorkOS\RateLimit\RateLimiter; +use WorkOS\RateLimit\TransientStore; use WorkOS\REST\Auth\Mfa; +use WorkOS\Time\SystemClock; /** * REST dispatch coverage for /auth/mfa/*. @@ -91,7 +97,9 @@ public function setUp(): void { $this->repository, $this->nonce, new Radar(), - new RateLimiter(), + new HttpRateLimit( new RateLimiter( new TransientStore(), new SystemClock() ), 10, 5, 60 ), + new ClientIp(), + new OutboundEmailRateLimiter( new RateLimiter( new TransientStore(), new SystemClock() ), new AddressCanonicalizer(), 3, 10, 20 ), new LoginCompleter() ); add_action( 'rest_api_init', [ $mfa, 'register_routes' ] ); diff --git a/tests/wpunit/AuthKitRestPasswordTest.php b/tests/wpunit/AuthKitRestPasswordTest.php index 0096ed0..efd84ce 100644 --- a/tests/wpunit/AuthKitRestPasswordTest.php +++ b/tests/wpunit/AuthKitRestPasswordTest.php @@ -15,8 +15,14 @@ use WP_REST_Response; use WorkOS\Auth\AuthKit\ProfileRepository; use WorkOS\Auth\AuthKit\Radar; -use WorkOS\Auth\AuthKit\RateLimiter; +use WorkOS\Email\AddressCanonicalizer; +use WorkOS\Http\ClientIp; +use WorkOS\RateLimit\HttpRateLimit; +use WorkOS\RateLimit\OutboundEmailRateLimiter; +use WorkOS\RateLimit\RateLimiter; +use WorkOS\RateLimit\TransientStore; use WorkOS\REST\Auth\Password; +use WorkOS\Time\SystemClock; /** * REST dispatch coverage for /auth/password/*. @@ -119,7 +125,9 @@ public function setUp(): void { $this->repository, $this->nonce, new Radar(), - new RateLimiter(), + new HttpRateLimit( new RateLimiter( new TransientStore(), new SystemClock() ), 10, 5, 60 ), + new ClientIp(), + new OutboundEmailRateLimiter( new RateLimiter( new TransientStore(), new SystemClock() ), new AddressCanonicalizer(), 3, 10, 20 ), new LoginCompleter() ); diff --git a/tests/wpunit/AuthKitRestSignupInvitationOAuthTest.php b/tests/wpunit/AuthKitRestSignupInvitationOAuthTest.php index 8f0ea2c..48a6f48 100644 --- a/tests/wpunit/AuthKitRestSignupInvitationOAuthTest.php +++ b/tests/wpunit/AuthKitRestSignupInvitationOAuthTest.php @@ -15,10 +15,16 @@ use WP_REST_Response; use WorkOS\Auth\AuthKit\ProfileRepository; use WorkOS\Auth\AuthKit\Radar; -use WorkOS\Auth\AuthKit\RateLimiter; +use WorkOS\Email\AddressCanonicalizer; +use WorkOS\Http\ClientIp; +use WorkOS\RateLimit\HttpRateLimit; +use WorkOS\RateLimit\OutboundEmailRateLimiter; +use WorkOS\RateLimit\RateLimiter; +use WorkOS\RateLimit\TransientStore; use WorkOS\REST\Auth\Invitation; use WorkOS\REST\Auth\OAuth; use WorkOS\REST\Auth\Signup; +use WorkOS\Time\SystemClock; /** * REST dispatch coverage for /auth/{signup,invitation,oauth}/*. @@ -106,12 +112,14 @@ public function setUp(): void { $this->nonce = new Nonce(); $radar = new Radar(); - $rate_limiter = new RateLimiter(); + $rate_limiter = new RateLimiter( new TransientStore(), new SystemClock() ); + $http = new HttpRateLimit( $rate_limiter, 10, 5, 60 ); + $outbound = new OutboundEmailRateLimiter( $rate_limiter, new AddressCanonicalizer(), 3, 10, 20 ); $completer = new LoginCompleter(); - $signup = new Signup( $this->repository, $this->nonce, $radar, $rate_limiter, $completer ); - $invitation = new Invitation( $this->repository, $this->nonce, $radar, $rate_limiter, $completer ); - $oauth = new OAuth( $this->repository, $this->nonce, $radar, $rate_limiter, $completer ); + $signup = new Signup( $this->repository, $this->nonce, $radar, $http, new ClientIp(), $outbound, $completer ); + $invitation = new Invitation( $this->repository, $this->nonce, $radar, $http, new ClientIp(), $outbound, $completer ); + $oauth = new OAuth( $this->repository, $this->nonce, $radar, $http, new ClientIp(), $outbound, $completer ); add_action( 'rest_api_init', [ $signup, 'register_routes' ] ); add_action( 'rest_api_init', [ $invitation, 'register_routes' ] ); diff --git a/tests/wpunit/ChangeEmailRestApiTest.php b/tests/wpunit/ChangeEmailRestApiTest.php index 90242aa..eebb696 100644 --- a/tests/wpunit/ChangeEmailRestApiTest.php +++ b/tests/wpunit/ChangeEmailRestApiTest.php @@ -8,7 +8,12 @@ namespace WorkOS\Tests\Wpunit; use lucatume\WPBrowser\TestCase\WPTestCase; -use WorkOS\Auth\AuthKit\RateLimiter; +use WorkOS\Email\AddressCanonicalizer; +use WorkOS\Http\ClientIp; +use WorkOS\RateLimit\HttpRateLimit; +use WorkOS\RateLimit\OutboundEmailRateLimiter; +use WorkOS\RateLimit\RateLimiter; +use WorkOS\RateLimit\TransientStore; use WorkOS\Auth\ChangeEmail\ConflictResolver; use WorkOS\Auth\ChangeEmail\Notifier; use WorkOS\Auth\ChangeEmail\PendingChange; @@ -18,6 +23,7 @@ use WorkOS\Email\Mailer; use WP_REST_Request; use WP_REST_Response; +use WorkOS\Time\SystemClock; /** * Coverage for `POST /workos/v1/users/{id}/email-change` and the @@ -80,7 +86,9 @@ public function setUp(): void { $notifier = new Notifier( $mailer, $masker ); $rest = new RestApi( - new RateLimiter(), + new RateLimiter( new TransientStore(), new SystemClock() ), + new ClientIp(), + new OutboundEmailRateLimiter( new RateLimiter( new TransientStore(), new SystemClock() ), new AddressCanonicalizer(), 3, 10, 20 ), $tokens, $pending, new ConflictResolver(), diff --git a/tests/wpunit/PasswordResetAdminRestApiTest.php b/tests/wpunit/PasswordResetAdminRestApiTest.php index d8ce3bc..a0bfbdf 100644 --- a/tests/wpunit/PasswordResetAdminRestApiTest.php +++ b/tests/wpunit/PasswordResetAdminRestApiTest.php @@ -10,12 +10,16 @@ use lucatume\WPBrowser\TestCase\WPTestCase; use WorkOS\Auth\AuthKit\Profile; use WorkOS\Auth\AuthKit\ProfileRepository; -use WorkOS\Auth\AuthKit\RateLimiter; +use WorkOS\RateLimit\TransientStore; +use WorkOS\RateLimit\RateLimiter; use WorkOS\Auth\PasswordResetAdmin\RedirectValidator; use WorkOS\Auth\PasswordResetAdmin\RestApi; use WorkOS\Email\AddressMask; use WP_REST_Request; use WP_REST_Response; +use WorkOS\Http\ClientIp; +use WorkOS\RateLimit\HttpRateLimit; +use WorkOS\Time\SystemClock; /** * Coverage for POST /workos/v1/admin/users/{id}/password-reset. @@ -116,7 +120,8 @@ public function setUp(): void { $rest_api = new RestApi( $this->repository, - new RateLimiter(), + new HttpRateLimit( new RateLimiter( new TransientStore(), new SystemClock() ), 10, 5, 60 ), + new ClientIp(), new RedirectValidator(), new AddressMask() );