Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 41 additions & 9 deletions docs/rate-limiting.md
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,12 @@ Every public auth route is throttled by requester. A subset also counts as a sen

| Endpoint | Request subject | Request policy | Send policy |
|---|---|---|---|
| `POST /auth/password/authenticate` | email | shared | yes, reserved — see [Reserve and refund](#reserve-and-refund) |
| `POST /auth/password/reset/start` | email | shared | yes (`otp_send`) |
| `POST /auth/password/authenticate` | email, exact | shared | yes, reserved — see [Reserve and refund](#reserve-and-refund) |
| `POST /auth/password/reset/start` | mailbox | shared | yes (`otp_send`) |
| `POST /auth/password/reset/confirm` | — | shared (IP only) | no |
| `POST /auth/magic/send` | email | shared | yes (`otp_send`) |
| `POST /auth/magic/verify` | email | shared | no |
| `POST /auth/signup/create` | email | `Signup` constants | no — see below |
| `POST /auth/magic/send` | mailbox | shared | yes (`otp_send`) |
| `POST /auth/magic/verify` | email, exact | shared | no |
| `POST /auth/signup/create` | mailbox | `Signup` constants | no — see below |
| `POST /auth/signup/verify` | — | `Signup` constants (IP only) | no |
| `POST /auth/mfa/challenge` | factor ID | shared | no |
| `POST /auth/mfa/verify` | — | shared (IP only) | no |
Expand All @@ -88,7 +88,7 @@ Every public auth route is throttled by requester. A subset also counts as a sen
| `POST /users/{id}/email-change` | user ID | — | yes |
| `POST /admin/users/{id}/password-reset` | user ID | shared | no |

"Shared" means `REQUEST_IP` plus, where there is a subject, `REQUEST_SUBJECT`.
"Shared" means `REQUEST_IP` plus, where there is a subject, `REQUEST_SUBJECT`. "Exact" and "mailbox" are the two email keyings described under [Which mailbox](#which-mailbox).

`magic_send` and `pw_reset` share the `otp_send` bucket prefix, so rotating between those two endpoints buys an attacker nothing. `password` uses its own `password_otp` prefix: it reserves before it knows whether a send happens, and sharing would let heavy magic-code use lock an account out of password sign-in.

Expand All @@ -105,10 +105,11 @@ WorkOS decides: authenticating an account whose email is unverified makes it mai
So that path claims the slot before the call and hands it back when the outcome shows nothing was sent:

```php
$mailbox = $this->canonicalizer->canonical( $email );
$reserved = $this->rate_limit(
[
[ self::BUCKET_PASSWORD_OTP_IP, $ip, TieredRateLimiter::SEND_IP ],
[ self::BUCKET_PASSWORD_OTP_RECIPIENT, $email, TieredRateLimiter::SEND_RECIPIENT ],
[ self::BUCKET_PASSWORD_OTP_RECIPIENT, $mailbox, TieredRateLimiter::SEND_RECIPIENT ],
]
);
if ( is_wp_error( $reserved ) ) {
Expand All @@ -119,7 +120,7 @@ $workos_response = workos()->api()->authenticate_with_password( /* … */ );

if ( ! $this->triggered_verification_email( $workos_response ) ) {
$this->rate_limiter->refund( self::BUCKET_PASSWORD_OTP_IP, $ip, TieredRateLimiter::SEND_IP );
$this->rate_limiter->refund( self::BUCKET_PASSWORD_OTP_RECIPIENT, $email, TieredRateLimiter::SEND_RECIPIENT );
$this->rate_limiter->refund( self::BUCKET_PASSWORD_OTP_RECIPIENT, $mailbox, TieredRateLimiter::SEND_RECIPIENT );
}
```

Expand Down Expand Up @@ -153,6 +154,34 @@ A real environment variable of the same name works too. When the header holds a

When nothing usable is found, resolution returns `0.0.0.0` so a bucket still has a stable subject.

### Which mailbox

A per-recipient limit is only as good as the key it counts against. `alice@gmail.com`, `alice+1@gmail.com`, and `a.l.i.c.e@gmail.com` are three strings and one inbox, so email subjects are reduced to a mailbox by `WorkOS\Email\AddressCanonicalizer` before they become a bucket key. The canonical form is a key only; the address passed to WorkOS is always what the caller typed.

Both rewrites are opt-in per provider, and the reason is the asymmetry of guessing wrong:

- Collapsing two real mailboxes into one bucket lets either exhaust the other's allowance. On a corporate or self-hosted domain `business+brian@corp.com` is routinely its own mailbox with its own owner, so a blanket rule turns a spam control into a way to lock a colleague out.
- Splitting one real mailbox across several buckets lets a caller earn an allowance per tag, but the per-IP send limits still cap the total mail one caller can cause.

The second is recoverable; the first denies service to someone who did nothing wrong. So `+` is stripped only on domains known to treat it as a tag (Google, Outlook and its consumer domains, Fastmail), and dots are ignored only on Google. This is not over-caution: [RFC 5233](https://www.rfc-editor.org/rfc/rfc5233.html) states that the encoding of detailed addresses is "site and/or implementation specific", and RFC 5321 leaves local-part semantics to the destination host. Among big providers the convention is genuinely not universal, with Yahoo and iCloud offering disposable addresses instead of tags and Proton using a different separator.

#### Mailbox or account

Two different questions get keyed on an address, and they don't want the same answer:

- **Guessing limits** (`password_email`, `magic_verify_email`) bound attempts to guess a password or a code for one account. The right subject is the account, and WorkOS keys accounts on the exact address.
- **Mail-causing limits** (both `*_recipient` buckets, plus `magic_send_email`, `pw_reset_email`, and `signup_email`) bound how much mail an inbox receives. The right subject is the mailbox, so the canonical form is correct.

The split is not "request limits versus send limits". `pw_reset_email`, `magic_send_email`, and `signup_email` are per-request limits on routes whose whole effect is mail, so they belong with the mailbox group even though they sit next to the guessing limits in code.

The two groups therefore use different subjects, even where they sit side by side in the same `rate_limit()` call. Nothing stops someone registering `alice@gmail.com` and `alice+work@gmail.com` as separate accounts, since WorkOS treats them as distinct users, while Gmail delivers both to one inbox. Keying guessing on the exact address means an attacker hammering one cannot spend the other's sign-in budget; keying mail on the mailbox means they cannot double the volume aimed at that single inbox.

Guessing limits use the address as typed, which needs no account lookup, because the exact address is already the identifier WorkOS resolves. If you touch this, the neighbouring buckets look like the same kind of thing and are not: `signup_email` would let `alice+1@`, `alice+2@` and `alice+3@` create three accounts that all mail one inbox, and `pw_reset_email` and `magic_send_email` sit on routes that exist to send. All three stay canonical.

Conditioning on whether the address is already known to us was considered and rejected. It would cost a user lookup on every unauthenticated request, ahead of the limit that exists to avoid work; it would make the bucket key depend on whether an account exists; and pre-authentication we can only see WordPress users, so an account that exists only in WorkOS would key differently depending on sync state. Recognising an address as registered also says nothing about whether its inbox is shared, which is the only thing the send limit cares about.

The lists are fixed, not configurable. A self-hosted server that does treat `+` as a tag is invisible to us and will get a counter per tag, which is the milder of the two failures and is bounded by the per-IP send limits. Adding a domain is a code change, in `AddressCanonicalizer`.

### Storage

`ObjectCacheRateLimiter` is used when WordPress reports a persistent object cache. On a real backend `wp_cache_incr` is an atomic in-place increment, so two concurrent requests get distinct counts and a limit cannot be overspent.
Expand All @@ -165,9 +194,11 @@ To check which one a site is using:
wp eval 'var_dump( wp_using_ext_object_cache() );'
```

This is worth checking per environment rather than assuming. A site can ship an object-cache plugin and still run without one: Object Cache Pro is inert unless the `object-cache.php` drop-in is in place and `WP_REDIS_DISABLED` is not set. When it is inert, the transient path is what runs, and the concurrent-burst gap described above is open.

## Adding a send path

1. Take the slot before sending, with `SEND_IP` and `SEND_RECIPIENT`.
1. Take the slot before sending, with `SEND_IP` and `SEND_RECIPIENT`, keyed on the canonical mailbox rather than the address as typed.
2. Reuse `BaseEndpoint::BUCKET_OTP_SEND_IP` / `_RECIPIENT` unless the path has a reason not to share the allowance. Bucket names are constants so the sharing survives a rename.
3. Charge it before any "should we actually send?" branch, so a `429` never depends on whether the account exists.
4. If you only learn afterwards that nothing went out, refund both buckets.
Expand Down Expand Up @@ -206,5 +237,6 @@ That is what was removed. One policy, one place, applied to every path that mail
- `src/WorkOS/RateLimit/` — the module.
- `src/WorkOS/REST/Auth/BaseEndpoint.php` — the `rate_limit()` helper endpoints call.
- `src/WorkOS/Http/ClientIp.php` — caller identity, shared with the logs.
- `src/WorkOS/Email/AddressCanonicalizer.php` — which strings mean one mailbox.
- [`docs/change-email.md`](change-email.md) — the change-email flow's use of the send policy.
- [`docs/password-reset.md`](password-reset.md) — the reset endpoints.
2 changes: 2 additions & 0 deletions src/WorkOS/Controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
use WorkOS\REST\Controller as RESTController;
use WorkOS\Webhook\Controller as WebhookController;
use WorkOS\Sync\Controller as SyncController;
use WorkOS\Email\Controller as EmailController;
use WorkOS\Http\Controller as HttpController;
use WorkOS\Organization\Controller as OrganizationController;
use WorkOS\RateLimit\Controller as RateLimitController;
Expand All @@ -39,6 +40,7 @@ class Controller extends BaseController {
protected function doRegister(): void {
// Ahead of the features that resolve these at construction.
$this->container->register( HttpController::class );
$this->container->register( EmailController::class );
$this->container->register( RateLimitController::class );

$this->container->register( AdminController::class );
Expand Down
118 changes: 118 additions & 0 deletions src/WorkOS/Email/AddressCanonicalizer.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
<?php
/**
* Email-address canonicalization.
*
* @package WorkOS\Email
*/

namespace WorkOS\Email;

defined( 'ABSPATH' ) || exit;

/**
* Reduces an address to the mailbox it actually lands in.
*
* `alice@gmail.com`, `alice+1@gmail.com` and `a.l.i.c.e@gmail.com` are three
* strings and one inbox. Anything that counts per-mailbox has to agree on
* which string represents it, or a caller gets a fresh allowance for every
* variant they can type.
*
* Both rewrites are opt-in per provider, because both are conventions rather
* than rules. RFC 5233 is explicit that the encoding of detailed addresses
* is "site and/or implementation specific", and RFC 5321 leaves local-part
* semantics to the destination host, so neither `+` nor `.` can be assumed
* to be inert. Guessing wrong is not a wash: collapsing two real mailboxes
* into one bucket lets either of them exhaust the other's allowance, which
* turns a spam control into a way to lock a colleague out.
*
* The failure in the other direction is bounded, which is why the lists are
* conservative and fixed. An address on a provider we don't list gets a
* counter per tag, but the per-IP send limits still cap how much mail one
* caller can cause in total.
*/
class AddressCanonicalizer {

/**
* Providers that ignore dots in the local part, mapped to the domain
* their variants collapse to.
*/
private const DOT_INSENSITIVE_DOMAINS = [
'gmail.com' => 'gmail.com',
'googlemail.com' => 'gmail.com',
];

/**
* Providers known to treat `+` as a tag separator and deliver to the
* address on its left.
*
* Deliberately not "everyone". `+` is a legal local-part character, and
* on corporate or self-hosted mail `business+brian@corp.com` is
* routinely a real mailbox with its own owner. Consumer providers also
* disagree: Yahoo and iCloud offer disposable addresses instead of
* tags, and Proton uses a different separator entirely.
*/
private const PLUS_TAG_DOMAINS = [
'gmail.com',
'googlemail.com',
'outlook.com',
'hotmail.com',
'live.com',
'msn.com',
'fastmail.com',
'fastmail.fm',
];

/**
* 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.
*
* Use this for anything keyed per-mailbox. Never use it as the address
* to send to: the canonical form is a bucket key, not a destination.
*
* @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 );

if ( in_array( $domain, self::PLUS_TAG_DOMAINS, true ) ) {
$plus = strpos( $local, '+' );
if ( false !== $plus ) {
$local = substr( $local, 0, $plus );
}
}
Comment on lines +102 to +107

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You were right @redscar thanks for calling it out. #38 (comment)

There are still some edge cases, though they are not inadvertently modified, but come through without modification, possibly targeting multiple email addresses but not on the common platforms.


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;
}
}
37 changes: 37 additions & 0 deletions src/WorkOS/Email/Controller.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<?php
/**
* Controller for shared email-address services.
*
* @package WorkOS\Email
*/

namespace WorkOS\Email;

use WorkOS\Contracts\Controller as BaseController;

/**
* Binds address helpers used across features.
*
* Registered early: the auth endpoints resolve the canonicalizer at
* construction to key their rate limits on a mailbox rather than on the
* particular string a caller typed.
*/
class Controller extends BaseController {

/**
* Register email services.
*
* @return void
*/
protected function doRegister(): void {
$this->container->singleton( AddressCanonicalizer::class );
}

/**
* Nothing to unhook: this module only provides container bindings.
*
* @return void
*/
protected function doUnregister(): void {
}
}
23 changes: 17 additions & 6 deletions src/WorkOS/REST/Auth/BaseEndpoint.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use WorkOS\Auth\AuthKit\Profile;
use WorkOS\Auth\AuthKit\ProfileRepository;
use WorkOS\Auth\AuthKit\Radar;
use WorkOS\Email\AddressCanonicalizer;
use WorkOS\Http\ClientIp;
use WorkOS\RateLimit\TieredRateLimiter;
use WP_Error;
Expand Down Expand Up @@ -79,6 +80,13 @@ abstract class BaseEndpoint {
*/
protected ClientIp $client_ip;

/**
* Mailbox canonicalizer for rate-limit subjects.
*
* @var AddressCanonicalizer
*/
protected AddressCanonicalizer $canonicalizer;

/**
* Login completer.
*
Expand All @@ -89,26 +97,29 @@ abstract class BaseEndpoint {
/**
* Constructor.
*
* @param ProfileRepository $profiles Profile repository.
* @param Nonce $nonce Nonce helper.
* @param Radar $radar Radar helper.
* @param TieredRateLimiter $rate_limiter Rate limiter.
* @param ClientIp $client_ip Client IP resolver.
* @param LoginCompleter $login_completer Login completer.
* @param ProfileRepository $profiles Profile repository.
* @param Nonce $nonce Nonce helper.
* @param Radar $radar Radar helper.
* @param TieredRateLimiter $rate_limiter Rate limiter.
* @param ClientIp $client_ip Client IP resolver.
* @param AddressCanonicalizer $canonicalizer Mailbox canonicalizer.
* @param LoginCompleter $login_completer Login completer.
*/
public function __construct(
ProfileRepository $profiles,
Nonce $nonce,
Radar $radar,
TieredRateLimiter $rate_limiter,
ClientIp $client_ip,
AddressCanonicalizer $canonicalizer,
LoginCompleter $login_completer
) {
$this->profiles = $profiles;
$this->nonce = $nonce;
$this->radar = $radar;
$this->rate_limiter = $rate_limiter;
$this->client_ip = $client_ip;
$this->canonicalizer = $canonicalizer;
$this->login_completer = $login_completer;
}

Expand Down
16 changes: 12 additions & 4 deletions src/WorkOS/REST/Auth/MagicCode.php
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,15 @@ public function send( WP_REST_Request $request ) {
);
}

// Count against the mailbox, not the string typed: tag and dot
// variants of one inbox would otherwise each get an allowance.
$mailbox = $this->canonicalizer->canonical( $email );

$ip = $this->client_ip->get();
$rate_ok = $this->rate_limit(
[
[ 'magic_send_ip', $ip, TieredRateLimiter::REQUEST_IP ],
[ 'magic_send_email', $email, TieredRateLimiter::REQUEST_SUBJECT ],
[ 'magic_send_email', $mailbox, TieredRateLimiter::REQUEST_SUBJECT ],
]
);
if ( is_wp_error( $rate_ok ) ) {
Expand All @@ -99,7 +103,7 @@ public function send( WP_REST_Request $request ) {
$send_ok = $this->rate_limit(
[
[ self::BUCKET_OTP_SEND_IP, $ip, TieredRateLimiter::SEND_IP ],
[ self::BUCKET_OTP_SEND_RECIPIENT, $email, TieredRateLimiter::SEND_RECIPIENT ],
[ self::BUCKET_OTP_SEND_RECIPIENT, $mailbox, TieredRateLimiter::SEND_RECIPIENT ],
]
);
if ( is_wp_error( $send_ok ) ) {
Expand Down Expand Up @@ -157,10 +161,10 @@ public function verify( WP_REST_Request $request ) {
$code = (string) $request->get_param( 'code' );
$pending_auth_token = (string) $request->get_param( 'pending_authentication_token' );

if ( '' === $email || '' === $code ) {
if ( '' === $email || ! is_email( $email ) || '' === $code ) {
return new WP_Error(
'workos_authkit_invalid_input',
__( 'An email and code are required.', 'integration-workos' ),
__( 'A valid email and code are required.', 'integration-workos' ),
[ 'status' => 400 ]
);
}
Expand All @@ -169,6 +173,10 @@ public function verify( WP_REST_Request $request ) {
$rate_ok = $this->rate_limit(
[
[ 'magic_verify_ip', $ip, TieredRateLimiter::REQUEST_IP ],
// Code guessing is counted against the address as typed, not
// the mailbox: this route sends nothing, and the thing being
// guessed belongs to whichever account WorkOS resolves, which
// it keys on the exact address.
[ 'magic_verify_email', $email, TieredRateLimiter::REQUEST_SUBJECT ],
]
);
Expand Down
Loading