fix(mfa): harden MFA against bypasses found in a security review

CHANGELOG.md

@@ -18,6 +18,20 @@
 ### Security
 
 - **Password-only sign-ins can't skip MFA** — in `plugin` mode, `wp-login.php` with `?fallback=1`, `wp_signon()`, headless mode and the login button refuse (`403 workos_mfa_step_unsupported`) a user who has a factor or must use MFA, since they can't ask for a code. Application passwords are exempt. Filter `workos_mfa_allow_password_fallback`.
+- **MFA hardening** — from a security review of the MFA work before release. In `plugin` mode:
+  - **Bearer tokens** from other apps on the same WorkOS environment no longer act for users who have or need MFA. REST accepts only tokens whose session signed in on this site (filters `workos_mfa_accept_bearer_token`, `workos_bearer_token_user`).
+  - **Your own account**: the admin routes (`PUT /users/{id}/mfa/required`, `POST …/mfa/reset`) and the user-edit tools no longer work on yourself. WordPress grants every user `edit_user` on themselves, so a hijacked session could reset its factors and clear the require flag without a code.
+  - **Adding a second authenticator** to an account that has one needs a current code from an existing one (`factor_id`, `code`; `403 workos_mfa_code_required`). This covers the account panel and the legacy `/auth/mfa/totp/enroll` and `/auth/mfa/sms/enroll` routes, which also follow `workos_mfa_can_manage` now.
+  - **Wrong codes are capped per account**, 5 per 15 minutes and 30 a day, across sign-ins and the account panel (`429 workos_mfa_too_many_codes`), under a per-account lock so parallel requests can't overspend it.
+  - **Sign-in codes**: `/auth/mfa/challenge` and `/auth/mfa/verify` need an `mfa_token`. WorkOS's own `mfa_challenge` is parked like the plugin's, so the browser never holds WorkOS's pending token and can't pair it with a challenge from another account. A code must come from the factor its challenge was made for.
+  - **Factors added outside WordPress** (the WorkOS dashboard, another app, before this version) are challenged: the policy reads the user's factors from WorkOS at every sign-in. Factors removed in WorkOS turn the user's MFA off in WordPress to match (activity-log event `mfa.removed_in_workos`, action `workos_mfa_removed_in_workos`, an email to the user); users who must use MFA enroll again.
+  - **Re-linking**: a user with MFA is never re-linked to a different WorkOS user by email, whatever the Re-link Protection setting (filter `workos_relink_protected`).
+  - **Application passwords** exempt only their own user from the password-only guard, so an XML-RPC `system.multicall` can't borrow one to sign in another user. The guard also follows a default Login Profile set to `always`, for users linked to WorkOS.
+  - **A parked sign-in** expires 10 minutes after the first factor however often it's touched, and its stored WorkOS response is encrypted with a key derived from the token. The hosted-AuthKit return URL carries a one-time resume code instead of the token.
+  - **Account changes**: adding or removing an authenticator emails the account and signs out its other sessions. An admin reset signs the user out everywhere and revokes their WorkOS session (filter `workos_mfa_notify_user`).
+  - **Headless mode** refuses an MFA user before storing their tokens or firing `workos_user_authenticated`.
+  - **The admin panel** applies its changes only after the profile saves, and shows a reset WorkOS refused.
+  - **Errors** from MFA routes no longer include WorkOS's response body.
 - **WordPress password could skip WorkOS MFA** — on the headless `wp-login.php` path, a WorkOS `mfa_challenge` or `mfa_enrollment` answer was treated like a wrong password, and WordPress's own password check then signed the user in without a second factor. That path now refuses with `403 workos_mfa_step_unsupported`, past WordPress's password handlers too. The login button's form gives the same message instead of WorkOS's raw error, and the AuthKit password route no longer re-syncs the password to WorkOS when only the second factor is missing.
 
 ### Fixed

docs/rate-limiting.md

@@ -57,13 +57,14 @@ Why the send policies layer: a single fixed window is burstable across its bound
 
 Why the request policies do not: a daily cap on sign-in attempts locks out legitimate users who mistype a password, and the abuse it would prevent is already covered by the send limits. Adding tiers there is a one-line edit if that changes.
 
-Two endpoints declare their own request policy instead of using the shared ones, each next to the endpoint that owns the reasoning:
+Three places declare their own policy instead of using the shared ones, each next to the endpoint that owns the reasoning:
 
 | Constant | Tiers | Why |
 |---|---|---|
 | `Signup::RATE_LIMIT_CREATE_IP` / `_CREATE_EMAIL` | 5 / min, 3 / min | Tighter: signup creates an account for an address nobody has proven they own. |
 | `Signup::RATE_LIMIT_VERIFY_IP` | 10 / min | Verify is cheap and users retype codes. |
 | `OAuth::RATE_LIMIT_URL_IP` | 20 / min | Looser: building an authorize URL sends nothing and costs no WorkOS call. |
+| `Mfa\CodeGuard::POLICY` | 5 / 15 min, 30 / day | Wrong MFA codes per account (`mfa_code_account`), across sign-ins and the account panel. Charged before WorkOS checks the code and refunded when it's right, under a per-account lock so parallel requests can't overspend it. Someone with the password can start unlimited sign-ins, so a per-sign-in cap alone doesn't bound guessing. |
 
 These are class constants, not settings. There is no admin UI and no filter. Retuning is a code change, deliberately: the numbers are a support contract, and the earlier per-feature options (`change_email_rate_limit_*`, removed) were four knobs nobody tuned, where a stored `0` silently meant "unlimited".
 
@@ -84,7 +85,8 @@ Every public auth route is throttled by requester. A subset also counts as a sen
 | `POST /auth/signup/verify` | — | `Signup` constants (IP only) | no |
 | `POST /auth/mfa/enroll` | `mfa_token` (hashed) | shared | no |
 | `POST /auth/mfa/challenge` | factor ID | shared | no |
-| `POST /auth/mfa/verify` | `mfa_token` (hashed) when given, else IP only | shared | no |
+| `POST /auth/mfa/verify` | `mfa_token` (hashed), plus the account (`CodeGuard`) | shared | no |
+| `POST`/`DELETE /users/{id}/mfa/…` | user ID, plus the account (`CodeGuard`) for code checks | shared | no |
 | `GET /auth/invitation/{token}` | — | shared (IP only) | no |
 | `POST /auth/invitation/accept` | — | shared (IP only) | no |
 | `GET /auth/oauth/authorize-url` | — | `OAuth` constant | no |

src/WorkOS/ActivityLog/AdminPage.php

@@ -91,7 +91,7 @@ public function render_page(): void {
 		// phpcs:ignore WordPress.Security.NonceVerification.Recommended
 		$cleared = ! empty( SuperGlobals::get_get_var( 'cleared' ) );
 
-		$event_types = [ 'login', 'logout', 'login_failed', 'login_denied', 'user_suspended', 'onboarding_sync', 'bypass_activated', 'link_refused', 'mfa.enrolled', 'mfa.removed', 'mfa.reset', 'mfa.required_set', 'mfa.required_cleared', 'mfa.challenge_succeeded', 'mfa.challenge_failed', 'mfa.blocked' ];
+		$event_types = [ 'login', 'logout', 'login_failed', 'login_denied', 'user_suspended', 'onboarding_sync', 'bypass_activated', 'link_refused', 'mfa.enrolled', 'mfa.removed', 'mfa.reset', 'mfa.removed_in_workos', 'mfa.required_set', 'mfa.required_cleared', 'mfa.challenge_succeeded', 'mfa.challenge_failed', 'mfa.blocked' ];
 
 		?>
 		<div class="wrap">

src/WorkOS/Auth/AuthKit/FrontendRoute.php

@@ -182,20 +182,26 @@ public function maybe_render(): void {
 
 		if ( null !== $mfa ) {
 			$context['mfa'] = $mfa;
+			nocache_headers();
+			header( 'Referrer-Policy: no-referrer' );
 		}
 
 		$this->renderer->render_full_page( $profile, $context );
 	}
 
 	/**
-	 * The MFA step to resume, when `?workos_mfa=` names a live parked sign-in.
+	 * The MFA step to resume, when `?workos_mfa=` carries a live resume code.
+	 *
+	 * The code works once (see PendingLogin::handoff()); the token it
+	 * stands for goes into the page, never the URL.
 	 *
 	 * @param Profile $profile Profile being rendered.
 	 *
 	 * @return array|null `{ mfa_required, mfa_token, enroll, factors }`, or null.
 	 */
 	private function parked_mfa( Profile $profile ): ?array {
-		$token   = sanitize_text_field( (string) ( SuperGlobals::get_get_var( 'workos_mfa' ) ?? '' ) );
+		$code    = sanitize_text_field( (string) ( SuperGlobals::get_get_var( 'workos_mfa' ) ?? '' ) );
+		$token   = PendingLogin::redeem( $code );
 		$pending = '' !== $token ? PendingLogin::get( $token, $profile->get_slug() ) : null;
 		if ( null === $pending ) {
 			return null;

src/WorkOS/Auth/AuthKit/LoginCompleter.php

@@ -17,6 +17,7 @@
 use WorkOS\Mfa\PendingLogin;
 use WorkOS\Mfa\Policy;
 use WorkOS\Mfa\Settings as MfaSettings;
+use WorkOS\Mfa\VerifiedSessions;
 use WorkOS\Organization\EntitlementGate;
 use WorkOS\Sync\UserSync;
 use WP_Error;
@@ -73,13 +74,8 @@ public function __construct() {
 	 *     'user'        => { id, email, display_name },
 	 *     'redirect_to' => string,
 	 *   }
-	 *   When WorkOS asks for a code: {
-	 *     'mfa_required'                 => true,
-	 *     'pending_authentication_token' => string,
-	 *     'factors'                      => array,
-	 *   }
-	 *   When the plugin's MFA policy asks for a code, or the user must
-	 *   enroll first (either mode): {
+	 *   When WorkOS or the plugin's MFA policy asks for a code, or the
+	 *   user must enroll first (either mode): {
 	 *     'mfa_required' => true,
 	 *     'mfa_token'    => string,
 	 *     'enroll'       => bool,
@@ -103,11 +99,8 @@ public function complete( $workos_response, Profile $profile, string $redirect_t
 			if ( is_wp_error( $mfa ) ) {
 				return $mfa;
 			}
-			if ( isset( $mfa['mfa_token'] ) ) {
-				return $mfa;
-			}
 			if ( null !== $mfa ) {
-				$workos_response = $mfa;
+				return $mfa;
 			}
 		}
 
@@ -125,35 +118,18 @@ public function complete( $workos_response, Profile $profile, string $redirect_t
 		}
 
 		// WorkOS signals a required second factor by returning a pending
-		// authentication token and/or an authentication_factor. Hand that
-		// state back to the React shell so it can run the MFA challenge
-		// step and then POST /auth/mfa/verify.
+		// authentication token and/or an authentication_factor. Park it and
+		// hand the React shell an `mfa_token`, so the challenge and verify
+		// routes only ever touch this sign-in's own factors.
 		if ( ! empty( $workos_response['pending_authentication_token'] ) ) {
-			$factors = $this->extract_factors( $workos_response );
-
-			// Enforce the profile's factor allowlist. If WorkOS surfaces a
-			// challenge for a factor type the admin has disabled on this
-			// profile, reject rather than completing via an unauthorized
-			// factor.
-			$allowed_types = $profile->get_mfa()['factors'] ?? [];
-			if ( ! empty( $allowed_types ) ) {
-				foreach ( $factors as $factor ) {
-					$type = (string) ( $factor['type'] ?? '' );
-					if ( '' !== $type && ! in_array( $type, $allowed_types, true ) ) {
-						return new WP_Error(
-							'workos_authkit_factor_not_allowed',
-							__( 'The multi-factor method returned is not permitted for this login.', 'integration-workos' ),
-							[ 'status' => 403 ]
-						);
-					}
-				}
-			}
-
-			return [
-				'mfa_required'                 => true,
-				'pending_authentication_token' => (string) $workos_response['pending_authentication_token'],
-				'factors'                      => $factors,
-			];
+			return $this->workos_challenge(
+				(string) $workos_response['pending_authentication_token'],
+				$this->extract_factors( $workos_response ),
+				(string) ( $workos_response['user']['id'] ?? '' ),
+				$profile,
+				$redirect_to,
+				$honor_profile_redirect
+			);
 		}
 
 		// When the profile insists MFA is always required and WorkOS did
@@ -223,6 +199,7 @@ public function complete( $workos_response, Profile $profile, string $redirect_t
 	 */
 	public function finish( array $workos_response, WP_User $wp_user, Profile $profile, string $redirect_to = '', bool $honor_profile_redirect = true ): array {
 		Login::store_tokens( $wp_user->ID, $workos_response );
+		VerifiedSessions::remember( (int) $wp_user->ID, $workos_response );
 
 		wp_set_auth_cookie( $wp_user->ID, true );
 		wp_set_current_user( $wp_user->ID );
@@ -328,21 +305,31 @@ private function plugin_mfa_step( WP_User $wp_user, array $workos_response, Prof
 		$decision = $policy->decide( $wp_user->ID, $profile );
 		$factors  = [];
 
-		if ( Policy::CHALLENGE === $decision ) {
+		// Ask WorkOS rather than trust the mirrored flag: a factor added
+		// outside WordPress (or before this version) must still be
+		// challenged, and one removed there no longer is. list_factors()
+		// updates the flag to match, so the policy is asked again.
+		if ( Policy::BLOCK !== $decision ) {
 			$factors = App::container()->get( FactorService::class )->list_factors( $wp_user->ID );
 			if ( is_wp_error( $factors ) ) {
 				return $factors;
 			}
 
-			// The mirrored flag said "enrolled", but WorkOS has no factor
-			// (removed outside WordPress). list_factors() just cleared the
-			// flag, so ask again; a challenge with nothing to challenge
-			// becomes an enrollment.
-			if ( empty( $factors ) ) {
-				$decision = $policy->decide( $wp_user->ID, $profile );
-				if ( Policy::CHALLENGE === $decision ) {
-					$decision = Policy::ENROLL;
-				}
+			$decision = $policy->decide( $wp_user->ID, $profile );
+
+			// Only factors this profile allows can answer its challenge.
+			$factors = array_values(
+				array_filter(
+					$factors,
+					static fn( array $factor ): bool => $profile->allows_factor( (string) $factor['type'] )
+				)
+			);
+
+			// The user has factors, but none this profile can use (SMS
+			// only, say). A challenge with nothing to challenge becomes an
+			// enrollment.
+			if ( Policy::CHALLENGE === $decision && empty( $factors ) ) {
+				$decision = Policy::ENROLL;
 			}
 		}
 
@@ -431,18 +418,17 @@ static function ( array $factor ): array {
 	/**
 	 * The MFA step, when WorkOS asks for one.
 	 *
-	 * `mfa_challenge` becomes the same pending-token shape a challenge has
-	 * always used here, so the factor allowlist below applies to it.
-	 * `mfa_enrollment` parks WorkOS's pending token and user, and hands the
-	 * browser an `mfa_token` for the enrollment step. Without a WorkOS user
-	 * in the error there's nothing to enroll, so it's refused.
+	 * Both steps park WorkOS's pending token and hand the browser an
+	 * `mfa_token`. `mfa_challenge` keeps the factors WorkOS named;
+	 * `mfa_enrollment` keeps the WorkOS user, to enroll. Without a WorkOS
+	 * user in the error there's nothing to enroll, so it's refused.
 	 *
 	 * @param WP_Error $error                  Error from the WorkOS authenticate call.
 	 * @param Profile  $profile                Active Login Profile.
 	 * @param string   $redirect_to            Client-requested redirect.
 	 * @param bool     $honor_profile_redirect Whether the profile's `post_login_redirect` wins.
 	 *
-	 * @return array|WP_Error|null Pending-token response, an enrollment step, a refusal, or null for any other error.
+	 * @return array|WP_Error|null The MFA step, a refusal, or null for any other error.
 	 */
 	private function mfa_step( WP_Error $error, Profile $profile, string $redirect_to, bool $honor_profile_redirect ) {
 		$code = MfaStep::code( $error );
@@ -487,9 +473,75 @@ private function mfa_step( WP_Error $error, Profile $profile, string $redirect_t
 			);
 		}
 
+		return $this->workos_challenge(
+			$pending_token,
+			isset( $body['authentication_factors'] ) && is_array( $body['authentication_factors'] ) ? $body['authentication_factors'] : [],
+			(string) ( $workos_user['id'] ?? '' ),
+			$profile,
+			$redirect_to,
+			$honor_profile_redirect
+		);
+	}
+
+	/**
+	 * Park a sign-in WorkOS wants a code for.
+	 *
+	 * The browser gets an `mfa_token`, never WorkOS's pending token, and
+	 * may challenge only the factors WorkOS named for this sign-in. Factors
+	 * of a type the profile doesn't allow refuse the sign-in rather than
+	 * completing through a method the admin turned off.
+	 *
+	 * @param string  $pending_token          WorkOS pending authentication token.
+	 * @param array   $factors                Factors WorkOS returned.
+	 * @param string  $workos_user_id         WorkOS user, when WorkOS named one (for the per-account code limit).
+	 * @param Profile $profile                Active Login Profile.
+	 * @param string  $redirect_to            Client-requested redirect.
+	 * @param bool    $honor_profile_redirect Whether the profile's `post_login_redirect` wins.
+	 *
+	 * @return array|WP_Error
+	 */
+	private function workos_challenge( string $pending_token, array $factors, string $workos_user_id, Profile $profile, string $redirect_to, bool $honor_profile_redirect ) {
+		$allowed = (array) ( $profile->get_mfa()['factors'] ?? [] );
+		$listed  = [];
+		foreach ( $factors as $factor ) {
+			if ( ! is_array( $factor ) || empty( $factor['id'] ) ) {
+				continue;
+			}
+
+			$type = (string) ( $factor['type'] ?? '' );
+			if ( ! empty( $allowed ) && '' !== $type && ! in_array( $type, $allowed, true ) ) {
+				return new WP_Error(
+					'workos_authkit_factor_not_allowed',
+					__( 'The multi-factor method returned is not permitted for this login.', 'integration-workos' ),
+					[ 'status' => 403 ]
+				);
+			}
+
+			$listed[] = [
+				'id'   => (string) $factor['id'],
+				'type' => $type,
+			];
+		}
+
+		$token = PendingLogin::park(
+			[
+				'kind'                         => PendingLogin::KIND_WORKOS,
+				'profile'                      => $profile->get_slug(),
+				'enroll'                       => false,
+				'redirect_to'                  => $redirect_to,
+				'honor_profile_redirect'       => $honor_profile_redirect,
+				'pending_authentication_token' => $pending_token,
+				'workos_user_id'               => $workos_user_id,
+				'factor_ids'                   => array_column( $listed, 'id' ),
+				'factors'                      => $listed,
+			]
+		);
+
 		return [
-			'pending_authentication_token' => $pending_token,
-			'authentication_factors'       => isset( $body['authentication_factors'] ) && is_array( $body['authentication_factors'] ) ? $body['authentication_factors'] : [],
+			'mfa_required' => true,
+			'mfa_token'    => $token,
+			'enroll'       => false,
+			'factors'      => $listed,
 		];
 	}
 

src/WorkOS/Auth/Login.php

@@ -12,6 +12,8 @@
 use WorkOS\Auth\AuthKit\MfaStep;
 use WorkOS\Auth\AuthKit\Profile;
 use WorkOS\Auth\AuthKit\ProfileRepository;
+use WorkOS\Mfa\PasswordGuard;
+use WorkOS\Mfa\PendingLogin;
 use WorkOS\Vendor\StellarWP\SuperGlobals\SuperGlobals;
 use WP_Error;
 
@@ -179,6 +181,16 @@ public function authenticate_headless( $user, $username, $password ) {
 			return $wp_user;
 		}
 
+		// A user who has or needs the plugin's MFA can't finish here, and
+		// this attempt mustn't overwrite their stored WorkOS session or
+		// look like a login to `workos_user_authenticated` listeners.
+		// PasswordGuard would refuse at priority 100 anyway; refusing now
+		// keeps the tokens and the hook out of it.
+		if ( \WorkOS\App::container()->get( PasswordGuard::class )->must_use_mfa( (int) $wp_user->ID ) ) {
+			$this->refusal = MfaStep::unsupported_error();
+			return $this->refusal;
+		}
+
 		// Store tokens.
 		self::store_tokens( $wp_user->ID, $result );
 
@@ -318,10 +330,12 @@ public function handle_callback(): void {
 
 		// The plugin's MFA policy wants a code (or an enrollment) the hosted
 		// page didn't ask for. The sign-in is parked; finish it on the
-		// profile's AuthKit page, which renders just the MFA step.
+		// profile's AuthKit page, which renders just the MFA step. The URL
+		// carries a one-time resume code, not the token itself.
 		if ( ! empty( $outcome['mfa_token'] ) ) {
 			nocache_headers();
-			wp_safe_redirect( FrontendRoute::url_for_profile( $profile, [ 'workos_mfa' => (string) $outcome['mfa_token'] ] ) );
+			header( 'Referrer-Policy: no-referrer' );
+			wp_safe_redirect( FrontendRoute::url_for_profile( $profile, [ 'workos_mfa' => PendingLogin::handoff( (string) $outcome['mfa_token'] ) ] ) );
 			exit;
 		}