diff --git a/CHANGELOG.md b/CHANGELOG.md index b4bface..5fc88a3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,17 @@ # Changelog +## [Unreleased] + +### Security + +- **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 + +- **MFA challenges from WorkOS never reached the sign-in form** — WorkOS reports a required second factor as an `mfa_challenge` error, and the sign-in flows only looked for it in a successful response, so users saw WorkOS's error message instead of the code prompt. The challenge is now read from the error, and the profile's factor allowlist applies to it. `mfa_enrollment` returns `403 workos_authkit_mfa_enrollment_required` until the sign-in form can enroll a factor. +- **MFA challenge, verify and factor delete called endpoints WorkOS doesn't have** — they went to `/user_management/auth_factors/…` and `/user_management/auth_challenges/…`. Factors enrolled through User Management are MFA API factors, so the calls now go to `/auth/factors/{id}/challenge`, `/auth/challenges/{id}/verify` and `/auth/factors/{id}`. `POST /auth/mfa/totp/enroll` also read the factor from the wrong level of WorkOS's response, so the QR code and secret came back empty. It now reads `authentication_factor` and also returns the `challenge_id` WorkOS creates with it. And a request with an empty body (a challenge with no SMS template) went out as `[]`, which WorkOS rejects with 422 "Validation failed"; empty bodies now go out as `{}`. +- **`enforce = always` refused every MFA sign-in** — a login profile set to always require MFA rejected the response from a correct code, because that response carries no pending token. `POST /auth/mfa/verify` now marks its result as having passed MFA. + ## [1.0.10] - 2026-10-14 ### Security diff --git a/src/WorkOS/Api/Client.php b/src/WorkOS/Api/Client.php index 63328ff..bbd8328 100644 --- a/src/WorkOS/Api/Client.php +++ b/src/WorkOS/Api/Client.php @@ -540,7 +540,7 @@ public function list_auth_factors( string $user_id ) { * @param string $totp_issuer Human-readable issuer displayed by the authenticator (e.g. site name). * @param string $totp_user Human-readable account label (usually the user's email). * - * @return array|\WP_Error Factor data including `totp.qr_code` and `totp.secret`. + * @return array|\WP_Error `authentication_factor` (with `totp.qr_code`, `totp.secret`, `totp.uri`) and `authentication_challenge`. */ public function enroll_totp_factor( string $user_id, string $totp_issuer, string $totp_user ) { return $this->post( @@ -574,12 +574,15 @@ public function enroll_sms_factor( string $user_id, string $phone_number ) { /** * Delete (un-enroll) an authentication factor. * + * Factors enrolled through User Management are MFA API factors, so + * delete, challenge and verify go to `/auth/...`, not `/user_management/...`. + * * @param string $factor_id Factor ID. * * @return array|\WP_Error */ public function delete_auth_factor( string $factor_id ) { - return $this->delete( "/user_management/auth_factors/{$factor_id}" ); + return $this->delete( "/auth/factors/{$factor_id}" ); } /** @@ -599,12 +602,15 @@ public function challenge_auth_factor( string $factor_id, ?string $sms_template $body['sms_template'] = $sms_template; } - return $this->post( "/user_management/auth_factors/{$factor_id}/challenge", $body ); + return $this->post( "/auth/factors/{$factor_id}/challenge", $body ); } /** * Verify an authentication challenge with a user-provided code. * + * Answers `{ challenge, valid }`: check `valid`, since a wrong code + * is not guaranteed to come back as an error. + * * @param string $challenge_id Challenge ID. * @param string $code Code entered by the user. * @param string|null $radar_action_token Optional Radar action token. @@ -617,7 +623,7 @@ public function verify_auth_challenge( ?string $radar_action_token = null ) { return $this->post( - "/user_management/auth_challenges/{$challenge_id}/verify", + "/auth/challenges/{$challenge_id}/verify", [ 'code' => $code ], $this->radar_headers( $radar_action_token ) ); @@ -1034,6 +1040,21 @@ private function get( string $path, array $params = [], array $extra_headers = [ return $this->parse_response( $response ); } + /** + * Encode a request body as a JSON object. + * + * `wp_json_encode( [] )` is `[]`, a JSON array, and WorkOS answers 422 + * "Validation failed" to it (e.g. a challenge with no SMS template). + * An empty body goes out as `{}`. + * + * @param array $data Request body. + * + * @return string + */ + private static function json_body( array $data ): string { + return (string) wp_json_encode( empty( $data ) ? new \stdClass() : $data ); + } + /** * Send a POST request. * @@ -1048,7 +1069,7 @@ private function post( string $path, array $data = [], array $extra_headers = [] self::BASE_URL . $path, [ 'headers' => $this->headers( true, $extra_headers ), - 'body' => wp_json_encode( $data ), + 'body' => self::json_body( $data ), 'timeout' => 15, ] ); @@ -1071,7 +1092,7 @@ private function put( string $path, array $data = [], array $extra_headers = [] [ 'method' => 'PUT', 'headers' => $this->headers( true, $extra_headers ), - 'body' => wp_json_encode( $data ), + 'body' => self::json_body( $data ), 'timeout' => 15, ] ); diff --git a/src/WorkOS/Auth/AuthKit/LoginCompleter.php b/src/WorkOS/Auth/AuthKit/LoginCompleter.php index e76c478..f77fc86 100644 --- a/src/WorkOS/Auth/AuthKit/LoginCompleter.php +++ b/src/WorkOS/Auth/AuthKit/LoginCompleter.php @@ -60,6 +60,7 @@ public function __construct() { * @param Profile $profile Active Login Profile. * @param string $redirect_to Client-requested redirect (validated). * @param bool $honor_profile_redirect Whether the profile's `post_login_redirect` should override `$redirect_to`. AuthKit-REST callers leave this `true`; the legacy hosted-AuthKit callback passes `false` so the state-supplied URL still wins for that flow. + * @param bool $mfa_satisfied Whether this response came from a completed second factor. `POST /auth/mfa/verify` passes `true` so an `enforce = always` profile accepts the login it just challenged. * * @return array|WP_Error * On success: { @@ -77,13 +78,23 @@ public function __construct() { * 'email' => string, * } */ - public function complete( $workos_response, Profile $profile, string $redirect_to = '', bool $honor_profile_redirect = true ) { + public function complete( $workos_response, Profile $profile, string $redirect_to = '', bool $honor_profile_redirect = true, bool $mfa_satisfied = false ) { // WorkOS returns `organization_selection_required` when the // authenticate call resolves to a user that belongs to multiple // orgs without an org context. If this profile (or the global // setting) has an org pinned, complete transparently via the // organization-selection grant. With no pinned org we bail with a // clear error — there is no in-shell org picker by design. + if ( is_wp_error( $workos_response ) ) { + $mfa = $this->mfa_step( $workos_response ); + if ( is_wp_error( $mfa ) ) { + return $mfa; + } + if ( null !== $mfa ) { + $workos_response = $mfa; + } + } + if ( is_wp_error( $workos_response ) ) { $verification = $this->email_verification_step( $workos_response ); if ( null !== $verification ) { @@ -133,8 +144,10 @@ public function complete( $workos_response, Profile $profile, string $redirect_t // not return a pending factor (i.e., the user has nothing enrolled // or WorkOS is not enforcing org-level MFA), we must refuse to // complete the login. Silently letting the user through would - // quietly break the admin's "MFA required" guarantee. - if ( Profile::MFA_ENFORCE_ALWAYS === ( $profile->get_mfa()['enforce'] ?? '' ) ) { + // quietly break the admin's "MFA required" guarantee. A response + // from a verified second factor has no pending token by design, + // so it is the one single-step success that may pass. + if ( ! $mfa_satisfied && Profile::MFA_ENFORCE_ALWAYS === ( $profile->get_mfa()['enforce'] ?? '' ) ) { return new WP_Error( 'workos_authkit_mfa_required', __( 'This login requires multi-factor authentication, but no factor is enrolled on your account. Please enroll a factor and try again.', 'integration-workos' ), @@ -242,6 +255,45 @@ private function extract_factors( array $workos_response ): array { return []; } + /** + * 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` is refused for now: the shell has no enrollment + * step yet, and handing back a token it can't use would strand the + * user on a challenge with no factor. + * + * @param WP_Error $error Error from the WorkOS authenticate call. + * + * @return array|WP_Error|null Pending-token response, a refusal, or null for any other error. + */ + private function mfa_step( WP_Error $error ) { + $code = MfaStep::code( $error ); + if ( '' === $code ) { + return null; + } + + $body = MfaStep::body( $error ); + $pending_token = (string) ( $body['pending_authentication_token'] ?? '' ); + if ( '' === $pending_token ) { + return null; + } + + if ( MfaStep::ENROLLMENT === $code ) { + return new WP_Error( + 'workos_authkit_mfa_enrollment_required', + __( 'This account must set up multi-factor authentication before it can sign in. Contact the site administrator for help.', 'integration-workos' ), + [ 'status' => 403 ] + ); + } + + return [ + 'pending_authentication_token' => $pending_token, + 'authentication_factors' => isset( $body['authentication_factors'] ) && is_array( $body['authentication_factors'] ) ? $body['authentication_factors'] : [], + ]; + } + /** * The email-verification step, when WorkOS asks for one. * diff --git a/src/WorkOS/Auth/AuthKit/MfaStep.php b/src/WorkOS/Auth/AuthKit/MfaStep.php new file mode 100644 index 0000000..caae15f --- /dev/null +++ b/src/WorkOS/Auth/AuthKit/MfaStep.php @@ -0,0 +1,101 @@ +get_error_data(); + + return is_array( $data ) && isset( $data['body'] ) && is_array( $data['body'] ) ? $data['body'] : []; + } + + /** + * Error for sign-in paths that have no way to collect a second factor. + * + * Used by the wp-login.php headless filter and the login-button AJAX + * handler, which take an email and password only. + * + * @return WP_Error + */ + public static function unsupported_error(): WP_Error { + return new WP_Error( + 'workos_mfa_step_unsupported', + __( 'This account uses multi-factor authentication, which this sign-in form can’t complete. Contact the site administrator for help signing in.', 'integration-workos' ), + [ 'status' => 403 ] + ); + } +} diff --git a/src/WorkOS/Auth/Login.php b/src/WorkOS/Auth/Login.php index 8c637f2..10bcef9 100644 --- a/src/WorkOS/Auth/Login.php +++ b/src/WorkOS/Auth/Login.php @@ -8,9 +8,11 @@ namespace WorkOS\Auth; use WorkOS\Auth\AuthKit\LoginCompleter; +use WorkOS\Auth\AuthKit\MfaStep; use WorkOS\Auth\AuthKit\Profile; use WorkOS\Auth\AuthKit\ProfileRepository; use WorkOS\Vendor\StellarWP\SuperGlobals\SuperGlobals; +use WP_Error; defined( 'ABSPATH' ) || exit; @@ -19,6 +21,13 @@ */ class Login { + /** + * Refusal the headless filter raised for the current `authenticate` run. + * + * @var WP_Error|null + */ + private ?WP_Error $refusal = null; + /** * Constructor — registers hooks. */ @@ -29,6 +38,9 @@ public function __construct() { // Headless mode: authenticate filter. add_filter( 'authenticate', [ $this, 'authenticate_headless' ], 10, 3 ); + // Re-assert a headless refusal after WP's own password handlers. + add_filter( 'authenticate', [ $this, 'enforce_refusal' ], 99 ); + // Register callback rewrite. add_action( 'init', [ self::class, 'register_rewrite' ] ); add_action( 'template_redirect', [ $this, 'handle_callback' ] ); @@ -145,6 +157,15 @@ public function authenticate_headless( $user, $username, $password ) { $result = workos()->api()->authenticate_with_password( $email, $password ); + // WorkOS accepted the password but wants a second factor this form + // can't collect. Refuse, and keep refusing past WP's own password + // check at priority 20: it ignores an earlier WP_Error, so letting + // it run would sign the user in without MFA. + if ( MfaStep::is_mfa_error( $result ) ) { + $this->refusal = MfaStep::unsupported_error(); + return $this->refusal; + } + if ( is_wp_error( $result ) ) { // Don't block fallback — just return $user to let WP try next. return $user; @@ -174,6 +195,31 @@ public function authenticate_headless( $user, $username, $password ) { return $wp_user; } + /** + * Re-apply a refusal raised by `authenticate_headless()`. + * + * Core's `wp_authenticate_username_password()` and + * `wp_authenticate_email_password()` only short-circuit on a WP_User, + * so a WP_Error returned at priority 10 doesn't stop them from + * checking the WordPress password. Running last puts the refusal back. + * The refusal is cleared once used, so it never leaks into a later + * `authenticate` run in the same request. + * + * @param \WP_User|\WP_Error|null $user Result so far. + * + * @return \WP_User|\WP_Error|null + */ + public function enforce_refusal( $user ) { + if ( null === $this->refusal ) { + return $user; + } + + $refusal = $this->refusal; + $this->refusal = null; + + return $refusal; + } + /** * Register the /workos/callback rewrite rule. */ diff --git a/src/WorkOS/REST/Auth/Mfa.php b/src/WorkOS/REST/Auth/Mfa.php index a629daf..3c389d8 100644 --- a/src/WorkOS/REST/Auth/Mfa.php +++ b/src/WorkOS/REST/Auth/Mfa.php @@ -224,11 +224,14 @@ public function verify( WP_REST_Request $request ) { // `complete()` handles the `organization_selection_required` error // transparently when the profile has a pinned org. Pass the WP_Error - // through so it can decide. + // through so it can decide. A wrong code comes back as an error, so + // any success reaching the profile's `always` check has passed MFA. $result = $this->login_completer->complete( $workos_response, $profile, - $this->sanitize_redirect( (string) $request->get_param( 'redirect_to' ) ) + $this->sanitize_redirect( (string) $request->get_param( 'redirect_to' ) ), + true, + true ); if ( is_wp_error( $result ) ) { @@ -318,14 +321,19 @@ public function enroll_totp( WP_REST_Request $request ) { return $result; } - $totp = $result['totp'] ?? []; + // WorkOS nests the new factor under `authentication_factor` and adds + // a ready challenge for it under `authentication_challenge`. + $factor = isset( $result['authentication_factor'] ) && is_array( $result['authentication_factor'] ) ? $result['authentication_factor'] : $result; + $challenge = isset( $result['authentication_challenge'] ) && is_array( $result['authentication_challenge'] ) ? $result['authentication_challenge'] : []; + $totp = $factor['totp'] ?? []; return new WP_REST_Response( [ - 'factor_id' => (string) ( $result['id'] ?? '' ), - 'qr_code' => (string) ( $totp['qr_code'] ?? '' ), - 'secret' => (string) ( $totp['secret'] ?? '' ), - 'otpauth_uri' => (string) ( $totp['uri'] ?? '' ), + 'factor_id' => (string) ( $factor['id'] ?? '' ), + 'challenge_id' => (string) ( $challenge['id'] ?? '' ), + 'qr_code' => (string) ( $totp['qr_code'] ?? '' ), + 'secret' => (string) ( $totp['secret'] ?? '' ), + 'otpauth_uri' => (string) ( $totp['uri'] ?? '' ), ], 201 ); diff --git a/src/WorkOS/REST/Auth/Password.php b/src/WorkOS/REST/Auth/Password.php index 85a0520..1eba7c8 100644 --- a/src/WorkOS/REST/Auth/Password.php +++ b/src/WorkOS/REST/Auth/Password.php @@ -8,6 +8,7 @@ namespace WorkOS\REST\Auth; use WorkOS\Auth\AuthKit\FrontendRoute; +use WorkOS\Auth\AuthKit\MfaStep; use WorkOS\Auth\AuthKit\Profile; use WorkOS\RateLimit\TieredRateLimiter; use WorkOS\Auth\PasswordResetAdmin\RedirectValidator; @@ -149,8 +150,10 @@ public function authenticate( WP_REST_Request $request ) { ); // An unverified account already passed the password check; the - // fallback would only sync it again and mail a second code. - if ( is_wp_error( $workos_response ) && ! $this->triggered_verification_email( $workos_response ) && workos()->option( 'allow_password_fallback', true ) ) { + // fallback would only sync it again and mail a second code. The + // same goes for an MFA step: the password was right, and only the + // second factor is missing. + if ( is_wp_error( $workos_response ) && ! $this->triggered_verification_email( $workos_response ) && ! MfaStep::is_mfa_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. $wp_user = wp_authenticate( $email, $password ); diff --git a/src/WorkOS/UI/Ajax.php b/src/WorkOS/UI/Ajax.php index 7dbf44e..e2548aa 100644 --- a/src/WorkOS/UI/Ajax.php +++ b/src/WorkOS/UI/Ajax.php @@ -7,6 +7,7 @@ namespace WorkOS\UI; +use WorkOS\Auth\AuthKit\MfaStep; use WorkOS\Auth\Deactivation; use WorkOS\Auth\Login; use WorkOS\Sync\UserSync; @@ -46,6 +47,12 @@ public function handle(): void { $result = workos()->api()->authenticate_with_password( $email, $password ); + // The login button only takes an email and password, so it can't + // collect a second factor. Say so instead of echoing WorkOS's error. + if ( MfaStep::is_mfa_error( $result ) ) { + wp_send_json_error( [ 'message' => MfaStep::unsupported_error()->get_error_message() ], 403 ); + } + if ( is_wp_error( $result ) ) { wp_send_json_error( [ 'message' => $result->get_error_message() ] ); } diff --git a/tests/wpunit/ApiClientAuthKitTest.php b/tests/wpunit/ApiClientAuthKitTest.php index b0e2775..a8bf719 100644 --- a/tests/wpunit/ApiClientAuthKitTest.php +++ b/tests/wpunit/ApiClientAuthKitTest.php @@ -315,7 +315,8 @@ public function test_delete_auth_factor_issues_delete(): void { $request = $this->last_request(); $this->assertSame( 'DELETE', $request['method'] ); - $this->assertStringContainsString( '/user_management/auth_factors/factor_abc', $request['url'] ); + $this->assertStringContainsString( '/auth/factors/factor_abc', $request['url'] ); + $this->assertStringNotContainsString( '/user_management/', $request['url'] ); } public function test_challenge_auth_factor_posts_empty_when_no_template(): void { @@ -323,8 +324,10 @@ public function test_challenge_auth_factor_posts_empty_when_no_template(): void $request = $this->last_request(); $this->assertSame( 'POST', $request['method'] ); - $this->assertStringContainsString( '/user_management/auth_factors/factor_abc/challenge', $request['url'] ); - $this->assertSame( [], $this->last_body_json() ); + $this->assertStringContainsString( '/auth/factors/factor_abc/challenge', $request['url'] ); + $this->assertStringNotContainsString( '/user_management/', $request['url'] ); + // A JSON object: WorkOS answers 422 to an empty array. + $this->assertSame( '{}', $request['body'] ); } public function test_challenge_auth_factor_includes_sms_template(): void { @@ -337,7 +340,8 @@ public function test_verify_auth_challenge_posts_code(): void { $this->client->verify_auth_challenge( 'challenge_abc', '111222', 'radar_vc' ); $request = $this->last_request(); - $this->assertStringContainsString( '/user_management/auth_challenges/challenge_abc/verify', $request['url'] ); + $this->assertStringContainsString( '/auth/challenges/challenge_abc/verify', $request['url'] ); + $this->assertStringNotContainsString( '/user_management/', $request['url'] ); $this->assertSame( '111222', $this->last_body_json()['code'] ); $this->assertSame( 'radar_vc', $request['headers']['x-workos-radar-action-token'] ); } diff --git a/tests/wpunit/AuthKitRestMfaTest.php b/tests/wpunit/AuthKitRestMfaTest.php index 82b8a19..11fe976 100644 --- a/tests/wpunit/AuthKitRestMfaTest.php +++ b/tests/wpunit/AuthKitRestMfaTest.php @@ -176,7 +176,7 @@ public function test_challenge_requires_factor_id(): void { public function test_challenge_returns_challenge_id_on_success(): void { $this->queue_response( - '/user_management/auth_factors/', + '/auth/factors/factor_123/challenge', [ 'id' => 'challenge_abc', 'expires_at' => '2030-01-01T00:00:00Z' ] ); @@ -239,6 +239,88 @@ public function test_verify_completes_login_on_success(): void { $this->assertSame( 'urn:workos:oauth:grant-type:mfa-totp', $body['grant_type'] ); } + /** + * A verified second factor completes on an `enforce = always` profile. + * + * The TOTP grant answers without a pending token, which the `always` + * check used to read as "no factor enrolled" and refuse. + */ + public function test_verify_completes_login_on_enforce_always_profile(): void { + $this->save_always_profile(); + + $this->queue_response( + '/user_management/authenticate', + [ + 'access_token' => 'eyJhbGciOi.eyJzdWIiOiJ1c2VyX2Fsd2F5cyIsImV4cCI6OTk5OTk5OTk5OX0.sig', + 'refresh_token' => 'rt_always', + 'user' => [ 'id' => 'user_always', 'email' => 'always@example.com' ], + ] + ); + + $response = $this->dispatch_with_nonce( + 'POST', + '/workos/v1/auth/mfa/verify', + [ + 'profile' => 'mfa-always', + 'pending_authentication_token' => 'pat_always', + 'authentication_challenge_id' => 'chal_always', + 'code' => '123456', + ], + 'mfa-always' + ); + + $this->assertSame( 200, $response->get_status() ); + $this->assertSame( 'always@example.com', $response->get_data()['user']['email'] ); + } + + /** + * A wrong code on an `always` profile still fails: the satisfied flag + * only matters once WorkOS has accepted the code. + */ + public function test_verify_wrong_code_on_enforce_always_profile_fails(): void { + $this->save_always_profile(); + + $this->queue_response( + '/user_management/authenticate', + [ + 'code' => 'authentication_challenge_failed', + 'message' => 'The code is incorrect.', + ], + 400 + ); + + $response = $this->dispatch_with_nonce( + 'POST', + '/workos/v1/auth/mfa/verify', + [ + 'profile' => 'mfa-always', + 'pending_authentication_token' => 'pat_always', + 'authentication_challenge_id' => 'chal_always', + 'code' => '000000', + ], + 'mfa-always' + ); + + $this->assertSame( 400, $response->get_status() ); + $this->assertSame( 0, get_current_user_id() ); + } + + private function save_always_profile(): void { + $this->repository->save( + Profile::from_array( + [ + 'slug' => 'mfa-always', + 'title' => 'MFA Always', + 'methods' => [ Profile::METHOD_PASSWORD ], + 'mfa' => [ + 'enforce' => Profile::MFA_ENFORCE_ALWAYS, + 'factors' => [ Profile::FACTOR_TOTP ], + ], + ] + ) + ); + } + // --------------------------- /auth/mfa/factors --------------------------- public function test_list_factors_rejects_unauthenticated(): void { @@ -293,11 +375,20 @@ public function test_totp_enroll_returns_qr_and_secret(): void { $this->queue_response( '/user_management/users/user_mfa_2/auth_factors', [ - 'id' => 'factor_totp', - 'totp' => [ - 'qr_code' => 'data:image/png;base64,AAA', - 'secret' => 'SECRETBASE32', - 'uri' => 'otpauth://totp/Acme:alice@example.com?secret=SECRETBASE32', + 'authentication_factor' => [ + 'object' => 'authentication_factor', + 'id' => 'factor_totp', + 'type' => 'totp', + 'totp' => [ + 'qr_code' => 'data:image/png;base64,AAA', + 'secret' => 'SECRETBASE32', + 'uri' => 'otpauth://totp/Acme:alice@example.com?secret=SECRETBASE32', + ], + ], + 'authentication_challenge' => [ + 'object' => 'authentication_challenge', + 'id' => 'auth_challenge_enroll', + 'authentication_factor_id' => 'factor_totp', ], ], 201 @@ -313,6 +404,8 @@ public function test_totp_enroll_returns_qr_and_secret(): void { $this->assertSame( 201, $response->get_status() ); $data = $response->get_data(); $this->assertSame( 'factor_totp', $data['factor_id'] ); + $this->assertSame( 'auth_challenge_enroll', $data['challenge_id'] ); + $this->assertSame( 'data:image/png;base64,AAA', $data['qr_code'] ); $this->assertSame( 'SECRETBASE32', $data['secret'] ); $this->assertStringStartsWith( 'otpauth://', $data['otpauth_uri'] ); } @@ -372,7 +465,7 @@ public function test_delete_factor_removes_owned_factor(): void { ], ] ); - $this->queue_response( '/user_management/auth_factors/factor_x', [] ); + $this->queue_response( '/auth/factors/factor_x', [] ); $response = $this->dispatch_with_nonce( 'POST', diff --git a/tests/wpunit/AuthKitRestPasswordTest.php b/tests/wpunit/AuthKitRestPasswordTest.php index 387d853..ff02cd2 100644 --- a/tests/wpunit/AuthKitRestPasswordTest.php +++ b/tests/wpunit/AuthKitRestPasswordTest.php @@ -405,6 +405,79 @@ public function test_authenticate_email_verification_skips_wp_fallback(): void { $this->assertCount( 1, $this->captured, 'Only the first authenticate call may reach WorkOS.' ); } + /** + * WorkOS's real `mfa_challenge` rejection becomes the MFA step. + * + * When the environment requires MFA, WorkOS answers the password grant + * with an error, not a success, so the step has to be read off the + * error body. + */ + public function test_authenticate_mfa_challenge_error_is_surfaced(): void { + $this->response_queue = [ $this->mfa_challenge() ]; + + $response = $this->dispatch_with_nonce( + 'POST', + '/workos/v1/auth/password/authenticate', + [ + 'profile' => 'members', + 'email' => 'alice@example.com', + 'password' => 'hunter2', + ] + ); + + $this->assertSame( 200, $response->get_status() ); + $data = $response->get_data(); + $this->assertTrue( $data['mfa_required'] ); + $this->assertSame( 'pat_mfa', $data['pending_authentication_token'] ); + $this->assertSame( 'auth_factor_1', $data['factors'][0]['id'] ); + } + + /** + * The password was accepted and only the second factor is missing, so + * the WP fallback must not sync the password and try again. + */ + public function test_authenticate_mfa_challenge_skips_wp_fallback(): void { + update_user_meta( $this->wp_fallback_user_id, '_workos_user_id', 'user_wos_fallback' ); + $this->response_queue = [ $this->mfa_challenge() ]; + + $response = $this->dispatch_with_nonce( + 'POST', + '/workos/v1/auth/password/authenticate', + [ + 'profile' => 'members', + 'email' => 'fallback@example.com', + 'password' => 'wp_password_123', + ] + ); + + $this->assertSame( 200, $response->get_status() ); + $this->assertTrue( $response->get_data()['mfa_required'] ); + $this->assertCount( 1, $this->captured, 'Only the first authenticate call may reach WorkOS.' ); + $this->assertSame( 0, get_current_user_id() ); + } + + /** + * WorkOS's `mfa_challenge` rejection for an account with a TOTP factor. + * + * @return array HTTP response for the interceptor. + */ + private function mfa_challenge(): array { + return [ + 'response' => [ 'code' => 403, 'message' => 'Forbidden' ], + 'body' => wp_json_encode( + [ + 'code' => 'mfa_challenge', + 'message' => 'The user must complete an MFA challenge to finish authenticating.', + 'pending_authentication_token' => 'pat_mfa', + 'authentication_factors' => [ + [ 'id' => 'auth_factor_1', 'type' => 'totp' ], + ], + 'user' => [ 'id' => 'user_wos_fallback', 'email' => 'fallback@example.com' ], + ] + ), + ]; + } + /** * WorkOS's `email_verification_required` rejection for an account. * diff --git a/tests/wpunit/MfaLoginErrorsTest.php b/tests/wpunit/MfaLoginErrorsTest.php new file mode 100644 index 0000000..ea67e3c --- /dev/null +++ b/tests/wpunit/MfaLoginErrorsTest.php @@ -0,0 +1,252 @@ + 'sk_test_fake', + 'client_id' => 'client_fake', + 'environment_id' => 'environment_test', + 'login_mode' => 'headless', + 'allow_password_fallback' => true, + ] + ); + \WorkOS\App::container()->get( \WorkOS\Options\Production::class )->reset(); + + add_filter( 'pre_http_request', [ $this, 'intercept_http' ], 10, 3 ); + + $this->user_id = (int) wp_create_user( 'mfa_user', 'wp_password_123', 'mfa-user@example.com' ); + } + + public function tearDown(): void { + remove_filter( 'pre_http_request', [ $this, 'intercept_http' ], 10 ); + $this->next_workos = null; + + if ( $this->user_id ) { + wp_delete_user( $this->user_id ); + } + + wp_set_current_user( 0 ); + delete_option( 'workos_production' ); + \WorkOS\Config::set_active_environment( 'staging' ); + \WorkOS\App::container()->get( \WorkOS\Options\Production::class )->reset(); + + parent::tearDown(); + } + + public function intercept_http( $preempt, array $args, string $url ) { + if ( null === $this->next_workos || ! str_contains( $url, 'api.workos.com' ) ) { + return $preempt; + } + + return $this->next_workos; + } + + // ------------------------------- MfaStep ------------------------------- + + public function test_code_reads_challenge_and_enrollment(): void { + $this->assertSame( MfaStep::CHALLENGE, MfaStep::code( $this->api_error( $this->challenge_body() ) ) ); + $this->assertSame( MfaStep::ENROLLMENT, MfaStep::code( $this->api_error( $this->enrollment_body() ) ) ); + } + + public function test_code_ignores_other_errors_and_successes(): void { + $this->assertSame( '', MfaStep::code( $this->api_error( [ 'code' => 'invalid_credentials' ] ) ) ); + $this->assertSame( '', MfaStep::code( new WP_Error( 'http_request_failed', 'Timeout' ) ) ); + $this->assertSame( '', MfaStep::code( [ 'user' => [ 'id' => 'user_1' ] ] ) ); + } + + // ---------------------------- LoginCompleter ---------------------------- + + public function test_completer_turns_challenge_error_into_mfa_step(): void { + $result = ( new LoginCompleter() )->complete( $this->api_error( $this->challenge_body() ), $this->profile() ); + + $this->assertIsArray( $result ); + $this->assertTrue( $result['mfa_required'] ); + $this->assertSame( 'pat_mfa', $result['pending_authentication_token'] ); + $this->assertSame( [ [ 'id' => 'auth_factor_1', 'type' => 'totp' ] ], $result['factors'] ); + $this->assertSame( 0, get_current_user_id() ); + } + + public function test_completer_applies_factor_allowlist_to_challenge_error(): void { + $body = $this->challenge_body(); + $body['authentication_factors'] = [ [ 'id' => 'auth_factor_sms', 'type' => 'sms' ] ]; + + $result = ( new LoginCompleter() )->complete( $this->api_error( $body ), $this->profile() ); + + $this->assertInstanceOf( WP_Error::class, $result ); + $this->assertSame( 'workos_authkit_factor_not_allowed', $result->get_error_code() ); + } + + public function test_completer_refuses_enrollment_error_without_leaking_token(): void { + $result = ( new LoginCompleter() )->complete( $this->api_error( $this->enrollment_body() ), $this->profile() ); + + $this->assertInstanceOf( WP_Error::class, $result ); + $this->assertSame( 'workos_authkit_mfa_enrollment_required', $result->get_error_code() ); + $this->assertSame( 403, $result->get_error_data()['status'] ); + $this->assertStringNotContainsString( 'pat_enroll', wp_json_encode( $result->get_error_data() ) ); + $this->assertSame( 0, get_current_user_id() ); + } + + public function test_completer_accepts_satisfied_success_on_always_profile(): void { + $profile = $this->profile( Profile::MFA_ENFORCE_ALWAYS ); + $success = [ + 'access_token' => 'eyJ.eyJzdWIiOiJ1c2VyX3NhdCJ9.sig', + 'refresh_token' => 'rt_sat', + 'user' => [ 'id' => 'user_sat', 'email' => 'satisfied@example.com' ], + ]; + + $refused = ( new LoginCompleter() )->complete( $success, $profile ); + $this->assertInstanceOf( WP_Error::class, $refused ); + $this->assertSame( 'workos_authkit_mfa_required', $refused->get_error_code() ); + + $result = ( new LoginCompleter() )->complete( $success, $profile, '', true, true ); + $this->assertIsArray( $result ); + $this->assertSame( 'satisfied@example.com', $result['user']['email'] ); + } + + // ------------------------- Headless wp-login.php ------------------------- + + /** + * The WorkOS password was right but MFA is pending. WP's own password + * check at priority 20 must not sign the user in with the same password. + */ + public function test_headless_mfa_challenge_blocks_wp_password_fallback(): void { + $this->next_workos = $this->http( 403, $this->challenge_body() ); + + $result = wp_authenticate( 'mfa-user@example.com', 'wp_password_123' ); + + $this->assertInstanceOf( WP_Error::class, $result ); + $this->assertSame( 'workos_mfa_step_unsupported', $result->get_error_code() ); + } + + public function test_headless_mfa_enrollment_blocks_wp_password_fallback(): void { + $this->next_workos = $this->http( 403, $this->enrollment_body() ); + + $result = wp_authenticate( 'mfa_user', 'wp_password_123' ); + + $this->assertInstanceOf( WP_Error::class, $result ); + $this->assertSame( 'workos_mfa_step_unsupported', $result->get_error_code() ); + } + + /** + * Wrong WorkOS password still falls back to WordPress, as before. + */ + public function test_headless_invalid_credentials_still_falls_back(): void { + $this->next_workos = $this->http( + 400, + [ + 'code' => 'invalid_credentials', + 'message' => 'Invalid credentials.', + ] + ); + + $result = wp_authenticate( 'mfa-user@example.com', 'wp_password_123' ); + + $this->assertInstanceOf( \WP_User::class, $result ); + $this->assertSame( $this->user_id, $result->ID ); + } + + /** + * A refusal is used once: the next `authenticate` run in the same + * request starts clean. + */ + public function test_headless_refusal_does_not_leak_into_next_attempt(): void { + $this->next_workos = $this->http( 403, $this->challenge_body() ); + $this->assertInstanceOf( WP_Error::class, wp_authenticate( 'mfa-user@example.com', 'wp_password_123' ) ); + + $this->next_workos = $this->http( 400, [ 'code' => 'invalid_credentials' ] ); + $this->assertInstanceOf( \WP_User::class, wp_authenticate( 'mfa-user@example.com', 'wp_password_123' ) ); + } + + // -------------------------------- Helpers -------------------------------- + + private function profile( string $enforce = Profile::MFA_ENFORCE_IF_REQUIRED ): Profile { + return Profile::from_array( + [ + 'slug' => 'members', + 'title' => 'Members', + 'mfa' => [ + 'enforce' => $enforce, + 'factors' => [ Profile::FACTOR_TOTP ], + ], + ] + ); + } + + private function challenge_body(): array { + return [ + 'code' => 'mfa_challenge', + 'message' => 'The user must complete an MFA challenge to finish authenticating.', + 'pending_authentication_token' => 'pat_mfa', + 'authentication_factors' => [ [ 'id' => 'auth_factor_1', 'type' => 'totp' ] ], + 'user' => [ 'id' => 'user_mfa', 'email' => 'mfa-user@example.com' ], + ]; + } + + private function enrollment_body(): array { + return [ + 'code' => 'mfa_enrollment', + 'message' => 'The user must enroll in MFA to finish authenticating.', + 'pending_authentication_token' => 'pat_enroll', + 'user' => [ 'id' => 'user_mfa', 'email' => 'mfa-user@example.com' ], + ]; + } + + /** + * The WP_Error `Api\Client::parse_response()` builds for a 4xx. + */ + private function api_error( array $body, int $status = 403 ): WP_Error { + return new WP_Error( + 'workos_api_error', + (string) ( $body['message'] ?? 'Error' ), + [ + 'status' => $status, + 'body' => $body, + ] + ); + } + + private function http( int $code, array $body ): array { + return [ + 'response' => [ 'code' => $code, 'message' => 'Error' ], + 'body' => wp_json_encode( $body ), + ]; + } +}