-
Notifications
You must be signed in to change notification settings - Fork 862
Connection: sso broker redirect #47492
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: trunk
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| Significance: minor | ||
| Type: added | ||
|
|
||
| SSO: Add support for external SSO broker URL, allowing CIAB stores to route authentication through the MSD instead of directly to WordPress.com. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -724,6 +724,8 @@ public static function clear_cookies_after_login() { | |
| true | ||
| ); | ||
| } | ||
|
|
||
| delete_transient( self::BROKER_URL_TRANSIENT ); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -735,8 +737,16 @@ public static function disconnect() { | |
| if ( ( new Manager() )->is_user_connected() ) { | ||
| Helpers::delete_connection_for_user( get_current_user_id() ); | ||
| } | ||
| delete_transient( self::BROKER_URL_TRANSIENT ); | ||
| } | ||
|
|
||
| /** | ||
| * Transient key for caching the SSO broker URL server-side. | ||
| * | ||
| * @var string | ||
| */ | ||
| const BROKER_URL_TRANSIENT = 'jetpack_sso_broker_url'; | ||
|
|
||
| /** | ||
| * Retrieves nonce used for SSO form. | ||
| * | ||
|
|
@@ -755,7 +765,25 @@ public static function request_initial_nonce() { | |
| return new WP_Error( $xml->getErrorCode(), $xml->getErrorMessage() ); | ||
| } | ||
|
|
||
| $nonce = sanitize_key( $xml->getResponse() ); | ||
| $response = $xml->getResponse(); | ||
|
|
||
| // The response may be a plain nonce string (default) or an associative | ||
| // array containing 'nonce' and 'broker_url' for sites that use an | ||
| // external SSO broker (e.g. CIAB stores). | ||
| if ( is_array( $response ) && isset( $response['nonce'] ) ) { | ||
| $nonce = sanitize_key( $response['nonce'] ); | ||
|
|
||
| if ( ! empty( $response['broker_url'] ) ) { | ||
| $broker_url = esc_url_raw( $response['broker_url'] ); | ||
| $url_parts = wp_parse_url( $broker_url ); | ||
|
|
||
| if ( $url_parts && 'https' === ( $url_parts['scheme'] ?? '' ) ) { | ||
| set_transient( self::BROKER_URL_TRANSIENT, $broker_url, 10 * MINUTE_IN_SECONDS ); | ||
| } | ||
| } | ||
| } else { | ||
| $nonce = sanitize_key( $response ); | ||
| } | ||
|
Comment on lines
+768
to
+786
|
||
|
|
||
| setcookie( | ||
| 'jetpack_sso_nonce', | ||
|
|
@@ -771,6 +799,31 @@ public static function request_initial_nonce() { | |
| return $nonce; | ||
| } | ||
|
|
||
| /** | ||
| * Retrieves the SSO broker URL if one has been set by WP.com for this site. | ||
| * | ||
| * The broker URL is returned by the jetpack.sso.requestNonce XML-RPC call | ||
| * for sites that use an external SSO broker (e.g. CIAB stores). It is cached | ||
| * in a server-side transient to prevent browser-side tampering. | ||
| * | ||
| * @return string|false The broker URL, or false if not set. | ||
| */ | ||
| public static function get_broker_url() { | ||
| $broker_url = get_transient( self::BROKER_URL_TRANSIENT ); | ||
|
|
||
| if ( ! $broker_url ) { | ||
| return false; | ||
| } | ||
|
|
||
| $url_parts = wp_parse_url( $broker_url ); | ||
| if ( ! $url_parts || 'https' !== ( $url_parts['scheme'] ?? '' ) ) { | ||
| delete_transient( self::BROKER_URL_TRANSIENT ); | ||
| return false; | ||
| } | ||
|
|
||
| return $broker_url; | ||
| } | ||
|
Comment on lines
+811
to
+825
|
||
|
|
||
| /** | ||
| * The function that actually handles the login! | ||
| */ | ||
|
|
@@ -1085,6 +1138,23 @@ public function get_sso_url_or_die( $reauth = false, $args = array() ) { | |
| return $sso_redirect; | ||
| } | ||
|
|
||
| /** | ||
| * Returns the base URL for SSO authentication. | ||
| * | ||
| * If a broker URL has been set by WP.com (e.g. for CIAB stores using an | ||
| * external SSO broker like the MSD), that URL is used. Otherwise falls back | ||
| * to the default WordPress.com login URL. | ||
| * | ||
| * @return string The base SSO URL. | ||
| */ | ||
| public static function get_sso_base_url() { | ||
| $broker_url = self::get_broker_url(); | ||
| if ( $broker_url ) { | ||
| return $broker_url; | ||
| } | ||
| return 'https://wordpress.com/wp-login.php'; | ||
| } | ||
|
|
||
| /** | ||
| * Build WordPress.com SSO URL with appropriate query parameters. | ||
| * | ||
|
|
@@ -1106,7 +1176,7 @@ public function build_sso_url( $args = array() ) { | |
| return $sso_nonce; | ||
| } | ||
|
|
||
| return add_query_arg( $args, 'https://wordpress.com/wp-login.php' ); | ||
| return add_query_arg( $args, self::get_sso_base_url() ); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -1145,7 +1215,7 @@ public function build_reauth_and_sso_url( $args = array() ) { | |
| return $args['sso_nonce']; | ||
| } | ||
|
|
||
| return add_query_arg( $args, 'https://wordpress.com/wp-login.php' ); | ||
| return add_query_arg( $args, self::get_sso_base_url() ); | ||
| } | ||
|
|
||
| /** | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,147 @@ | ||
| <?php | ||
| /** | ||
| * Tests for SSO broker URL functionality. | ||
| * | ||
| * @package automattic/jetpack-connection | ||
| */ | ||
|
|
||
| namespace Automattic\Jetpack\Connection; | ||
|
|
||
| use Automattic\Jetpack\Connection\SSO\Helpers; | ||
| use Automattic\Jetpack\Constants; | ||
| use WorDBless\BaseTestCase; | ||
|
|
||
| /** | ||
| * Tests for SSO broker URL support (CIAB stores). | ||
| */ | ||
| class SSO_Broker_Test extends BaseTestCase { | ||
|
|
||
| /** | ||
| * Clean up after each test. | ||
| */ | ||
| public function tear_down() { | ||
| delete_transient( SSO::BROKER_URL_TRANSIENT ); | ||
| Constants::clear_constants(); | ||
| parent::tear_down(); | ||
| } | ||
|
|
||
| /** | ||
| * Test get_broker_url returns false when no transient is set. | ||
| */ | ||
| public function test_get_broker_url_returns_false_when_no_transient() { | ||
| $this->assertFalse( SSO::get_broker_url() ); | ||
| } | ||
|
|
||
| /** | ||
| * Test get_broker_url returns the URL when a valid HTTPS transient is set. | ||
| */ | ||
| public function test_get_broker_url_returns_url_when_valid_https_transient() { | ||
| set_transient( SSO::BROKER_URL_TRANSIENT, 'https://my.woo.ai/sso', 600 ); | ||
| $this->assertSame( 'https://my.woo.ai/sso', SSO::get_broker_url() ); | ||
| } | ||
|
|
||
| /** | ||
| * Test get_broker_url rejects and deletes a non-HTTPS transient. | ||
| */ | ||
| public function test_get_broker_url_rejects_http_url() { | ||
| set_transient( SSO::BROKER_URL_TRANSIENT, 'http://my.woo.ai/sso', 600 ); | ||
| $this->assertFalse( SSO::get_broker_url() ); | ||
| $this->assertFalse( get_transient( SSO::BROKER_URL_TRANSIENT ) ); | ||
| } | ||
|
|
||
| /** | ||
| * Test get_broker_url rejects and deletes a malformed URL. | ||
| */ | ||
| public function test_get_broker_url_rejects_malformed_url() { | ||
| set_transient( SSO::BROKER_URL_TRANSIENT, 'not-a-url', 600 ); | ||
| $this->assertFalse( SSO::get_broker_url() ); | ||
| $this->assertFalse( get_transient( SSO::BROKER_URL_TRANSIENT ) ); | ||
| } | ||
|
|
||
| /** | ||
| * Test get_sso_base_url returns wordpress.com when no broker is set. | ||
| */ | ||
| public function test_get_sso_base_url_defaults_to_wpcom() { | ||
| $this->assertSame( 'https://wordpress.com/wp-login.php', SSO::get_sso_base_url() ); | ||
| } | ||
|
|
||
| /** | ||
| * Test get_sso_base_url returns broker URL when set. | ||
| */ | ||
| public function test_get_sso_base_url_returns_broker_when_set() { | ||
| set_transient( SSO::BROKER_URL_TRANSIENT, 'https://my.woo.ai/sso', 600 ); | ||
| $this->assertSame( 'https://my.woo.ai/sso', SSO::get_sso_base_url() ); | ||
| } | ||
|
|
||
| /** | ||
| * Test get_sso_base_url falls back to wordpress.com when broker URL is invalid. | ||
| */ | ||
| public function test_get_sso_base_url_falls_back_for_invalid_broker() { | ||
| set_transient( SSO::BROKER_URL_TRANSIENT, 'http://insecure.example.com', 600 ); | ||
| $this->assertSame( 'https://wordpress.com/wp-login.php', SSO::get_sso_base_url() ); | ||
| } | ||
|
|
||
| /** | ||
| * Test allowed_redirect_hosts includes broker host when broker URL is set. | ||
| */ | ||
| public function test_allowed_redirect_hosts_includes_broker_host() { | ||
| set_transient( SSO::BROKER_URL_TRANSIENT, 'https://my.woo.ai/sso', 600 ); | ||
| Constants::set_constant( 'JETPACK__API_BASE', 'https://jetpack.wordpress.com/jetpack.' ); | ||
|
|
||
| $hosts = Helpers::allowed_redirect_hosts( array() ); | ||
| $this->assertContains( 'my.woo.ai', $hosts ); | ||
| } | ||
|
|
||
| /** | ||
| * Test allowed_redirect_hosts does not add broker host when no broker is set. | ||
| */ | ||
| public function test_allowed_redirect_hosts_excludes_broker_when_not_set() { | ||
| Constants::set_constant( 'JETPACK__API_BASE', 'https://jetpack.wordpress.com/jetpack.' ); | ||
|
|
||
| $hosts = Helpers::allowed_redirect_hosts( array() ); | ||
| $this->assertNotContains( 'my.woo.ai', $hosts ); | ||
| } | ||
|
|
||
| /** | ||
| * Test allowed_redirect_hosts does not add broker host when URL is not HTTPS. | ||
| */ | ||
| public function test_allowed_redirect_hosts_excludes_insecure_broker() { | ||
| set_transient( SSO::BROKER_URL_TRANSIENT, 'http://insecure.example.com', 600 ); | ||
| Constants::set_constant( 'JETPACK__API_BASE', 'https://jetpack.wordpress.com/jetpack.' ); | ||
|
|
||
| $hosts = Helpers::allowed_redirect_hosts( array() ); | ||
| $this->assertNotContains( 'insecure.example.com', $hosts ); | ||
| } | ||
|
|
||
| /** | ||
| * Test allowed_redirect_hosts still includes default hosts when broker is set. | ||
| */ | ||
| public function test_allowed_redirect_hosts_preserves_defaults_with_broker() { | ||
| set_transient( SSO::BROKER_URL_TRANSIENT, 'https://my.woo.ai/sso', 600 ); | ||
| Constants::set_constant( 'JETPACK__API_BASE', 'https://jetpack.wordpress.com/jetpack.' ); | ||
|
|
||
| $hosts = Helpers::allowed_redirect_hosts( array( 'test.com' ) ); | ||
| $this->assertContains( 'test.com', $hosts ); | ||
| $this->assertContains( 'wordpress.com', $hosts ); | ||
| $this->assertContains( 'jetpack.wordpress.com', $hosts ); | ||
| $this->assertContains( 'my.woo.ai', $hosts ); | ||
| } | ||
|
|
||
| /** | ||
| * Test disconnect clears the broker URL transient. | ||
| */ | ||
| public function test_disconnect_clears_broker_transient() { | ||
| set_transient( SSO::BROKER_URL_TRANSIENT, 'https://my.woo.ai/sso', 600 ); | ||
| $this->assertNotFalse( get_transient( SSO::BROKER_URL_TRANSIENT ) ); | ||
|
|
||
| SSO::disconnect(); | ||
| $this->assertFalse( get_transient( SSO::BROKER_URL_TRANSIENT ) ); | ||
| } | ||
|
|
||
| /** | ||
| * Test the BROKER_URL_TRANSIENT constant value is stable. | ||
| */ | ||
| public function test_broker_url_transient_constant_value() { | ||
| $this->assertSame( 'jetpack_sso_broker_url', SSO::BROKER_URL_TRANSIENT ); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When
jetpack.sso.requestNoncereturns a plain string (non-CIAB), the broker transient is never cleared. That means an oldbroker_urlcan remain injetpack_sso_broker_urland continue to redirect users through the broker even when WP.com no longer returns one. Consider explicitlydelete_transient( self::BROKER_URL_TRANSIENT )when the response is not the{ nonce, broker_url }shape, and also whenbroker_urlis present but fails HTTPS/URL validation.