Skip to content
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

SSO: switch from module codebase to Connection package, part 3 #37153

Merged
merged 26 commits into from
May 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
51be1c0
SSO: remove references to Jetpack class
jeherve May 1, 2024
83a29ad
Switch to modern display of notice and add missing i18n wrapping
jeherve May 1, 2024
7054189
Load SSO feature from the Connection package
jeherve May 1, 2024
3b7fafa
Switch to Connection package for all SSO references
jeherve May 1, 2024
feb2bb4
Deprecate all module classes and methods
jeherve May 1, 2024
914d1d3
Remove module test
jeherve May 1, 2024
b544238
Remove test reference too
jeherve May 1, 2024
18dbc04
Remove one more test directory (multisite)
jeherve May 1, 2024
c17a41e
Update Phan config
jeherve May 1, 2024
d05c401
Move SSO callables to the Connection package
jeherve May 1, 2024
c2294ed
Add deprecated class back
jeherve May 1, 2024
4e3492f
Remove deprecated private methods
jeherve May 1, 2024
7745ce8
Remove private properties and methods
jeherve May 1, 2024
9ecff27
Merge remote-tracking branch 'origin/trunk' into update/sso-use-package
jeherve May 1, 2024
836745c
Merge remote-tracking branch 'origin/trunk' into update/sso-use-package
jeherve May 9, 2024
d1571de
Move user generation to Utils class
jeherve May 9, 2024
0ea30ae
Try fixing tests
jeherve May 9, 2024
6980c07
Revert "Try fixing tests"
jeherve May 10, 2024
65a375a
Merge remote-tracking branch 'origin/trunk' into update/sso-use-package
jeherve May 10, 2024
b204864
Bump versions
jeherve May 10, 2024
f4a1a59
Ensure the SSO module is used in tests
jeherve May 10, 2024
c562446
Merge remote-tracking branch 'origin/trunk' into update/sso-use-package
jeherve May 10, 2024
6c9ade7
Add new SSO XML-RPC method
jeherve May 10, 2024
5fc7741
Merge remote-tracking branch 'origin/trunk' into update/sso-use-package
jeherve May 10, 2024
1e7a3f6
Merge branch 'trunk' into update/sso-use-package
fgiannar May 14, 2024
a4402bc
Jetpack Connection: Bump package version
fgiannar May 14, 2024
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
4 changes: 4 additions & 0 deletions projects/packages/connection/changelog/update-sso-use-package
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: changed

SSO: do not rely on the Jetpack class anymore.
2 changes: 1 addition & 1 deletion projects/packages/connection/src/class-package-version.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
*/
class Package_Version {

const PACKAGE_VERSION = '2.8.0';
const PACKAGE_VERSION = '2.8.1-alpha';

const PACKAGE_SLUG = 'connection';

Expand Down
49 changes: 49 additions & 0 deletions projects/packages/connection/src/class-utils.php
Original file line number Diff line number Diff line change
Expand Up @@ -83,4 +83,53 @@ public static function filter_register_request_body( $properties ) {
)
);
}

/**
* Generate a new user from a SSO attempt.
*
* @param object $user_data WordPress.com user information.
*/
public static function generate_user( $user_data ) {
$username = $user_data->login;
/**
* Determines how many times the SSO module can attempt to randomly generate a user.
*
* @module sso
*
* @since jetpack-4.3.2
*
* @param int 5 By default, SSO will attempt to random generate a user up to 5 times.
*/
$num_tries = (int) apply_filters( 'jetpack_sso_allowed_username_generate_retries', 5 );

$exists = username_exists( $username );
$tries = 0;
while ( $exists && $tries++ < $num_tries ) {
$username = $user_data->login . '_' . $user_data->ID . '_' . wp_rand();
$exists = username_exists( $username );
}

if ( $exists ) {
return false;
}

$user = (object) array();
$user->user_pass = wp_generate_password( 20 );
$user->user_login = wp_slash( $username );
$user->user_email = wp_slash( $user_data->email );
$user->display_name = $user_data->display_name;
$user->first_name = $user_data->first_name;
$user->last_name = $user_data->last_name;
$user->url = $user_data->url;
$user->description = $user_data->description;

if ( isset( $user_data->role ) && $user_data->role ) {
$user->role = $user_data->role;
}

$created_user_id = wp_insert_user( $user );

update_user_meta( $created_user_id, 'wpcom_user_id', $user_data->ID );
return get_userdata( $created_user_id );
}
}
12 changes: 7 additions & 5 deletions projects/packages/connection/src/sso/class-force-2fa.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,7 @@ public function plugins_loaded() {
$this->role = apply_filters( 'jetpack_force_2fa_cap', 'manage_options' );

// Bail if Jetpack SSO is not active
if (
! class_exists( 'Jetpack' )
|| ! ( new Modules() )->is_active( 'sso' )
) {
if ( ! ( new Modules() )->is_active( 'sso' ) ) {
add_action( 'admin_notices', array( $this, 'admin_notice' ) );
return;
}
Expand All @@ -75,7 +72,12 @@ public function admin_notice() {
* @module SSO
*/
if ( apply_filters( 'jetpack_force_2fa_dependency_notice', true ) && current_user_can( $this->role ) ) {
printf( '<div class="%1$s"><p>%2$s</p></div>', 'notice notice-warning', 'Jetpack Force 2FA requires Jetpack and the Jetpack SSO module.' );
wp_admin_notice(
esc_html__( 'Jetpack Force 2FA requires Jetpack’s SSO feature.', 'jetpack-connection' ),
array(
'type' => 'warning',
)
);
}
}

Expand Down
49 changes: 0 additions & 49 deletions projects/packages/connection/src/sso/class-helpers.php
Original file line number Diff line number Diff line change
Expand Up @@ -214,55 +214,6 @@ public static function allowed_redirect_hosts( $hosts, $api_base = '' ) {
return array_unique( $hosts );
}

/**
* Generate a new user from a SSO attempt.
*
* @param object $user_data WordPress.com user information.
*/
public static function generate_user( $user_data ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We added the method very recently, but it's already been released.
Do you think it's worth the trouble to deprecate it first?

$username = $user_data->login;
/**
* Determines how many times the SSO module can attempt to randomly generate a user.
*
* @module sso
*
* @since jetpack-4.3.2
*
* @param int 5 By default, SSO will attempt to random generate a user up to 5 times.
*/
$num_tries = (int) apply_filters( 'jetpack_sso_allowed_username_generate_retries', 5 );

$exists = username_exists( $username );
$tries = 0;
while ( $exists && $tries++ < $num_tries ) {
$username = $user_data->login . '_' . $user_data->ID . '_' . wp_rand();
$exists = username_exists( $username );
}

if ( $exists ) {
return false;
}

$user = (object) array();
$user->user_pass = wp_generate_password( 20 );
$user->user_login = wp_slash( $username );
$user->user_email = wp_slash( $user_data->email );
$user->display_name = $user_data->display_name;
$user->first_name = $user_data->first_name;
$user->last_name = $user_data->last_name;
$user->url = $user_data->url;
$user->description = $user_data->description;

if ( isset( $user_data->role ) && $user_data->role ) {
$user->role = $user_data->role;
}

$created_user_id = wp_insert_user( $user );

update_user_meta( $created_user_id, 'wpcom_user_id', $user_data->ID );
return get_userdata( $created_user_id );
}

/**
* Determines how long the auth cookie is valid for when a user logs in with SSO.
*
Expand Down
34 changes: 25 additions & 9 deletions projects/packages/connection/src/sso/class-sso.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,6 @@ private function __construct() {

self::$instance = $this;

/*
* This feature currently relies on the Jetpack plugin.
* Bail if Jetpack isn't installed.
*/
if ( ! class_exists( 'Jetpack' ) ) {
return;
}

add_action( 'admin_init', array( $this, 'maybe_authorize_user_after_sso' ), 1 );
add_action( 'admin_init', array( $this, 'register_settings' ) );
add_action( 'login_init', array( $this, 'login_init' ) );
Expand All @@ -71,6 +63,9 @@ private function __construct() {

add_filter( 'wp_login_errors', array( $this, 'sso_reminder_logout_wpcom' ) );

// Synchronize SSO options with WordPress.com.
add_filter( 'jetpack_sync_callable_whitelist', array( $this, 'sync_sso_callables' ), 10, 1 );

/**
* Filter to include Force 2FA feature.
*
Expand Down Expand Up @@ -134,6 +129,27 @@ public static function get_instance() {
return self::$instance;
}

/**
* Add SSO callables to the sync whitelist.
*
* @since $$next-version$$
*
* @param array $callables list of callables.
*
* @return array list of callables.
*/
public function sync_sso_callables( $callables ) {
$sso_callables = array(
'sso_is_two_step_required' => array( Helpers::class, 'is_two_step_required' ),
'sso_should_hide_login_form' => array( Helpers::class, 'should_hide_login_form' ),
'sso_match_by_email' => array( Helpers::class, 'match_by_email' ),
'sso_new_user_override' => array( Helpers::class, 'new_user_override' ),
'sso_bypass_default_login_form' => array( Helpers::class, 'bypass_login_forward_wpcom' ),
);

return array_merge( $callables, $sso_callables );
}

/**
* Safety heads-up added to the logout messages when SSO is enabled.
* Some folks on a shared computer don't know that they need to log out of WordPress.com as well.
Expand Down Expand Up @@ -864,7 +880,7 @@ public function handle_login() {
$user_data->role = $new_user_override_role;
}

$user = Helpers::generate_user( $user_data );
$user = Utils::generate_user( $user_data );
if ( ! $user ) {
$tracking->record_user_event(
'sso_login_failed',
Expand Down
7 changes: 4 additions & 3 deletions projects/packages/connection/tests/php/sso/test_Helpers.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

namespace Automattic\Jetpack\Connection\SSO;

use Automattic\Jetpack\Connection\Utils;
use Automattic\Jetpack\Constants;
use WorDBless\BaseTestCase;

Expand Down Expand Up @@ -259,7 +260,7 @@ public function test_allowed_redirect_hosts_api_base_added_on_dev_version() {
* Test "generate_user_returns_user_when_username_not_exists".
*/
public function test_generate_user_returns_user_when_username_not_exists() {
$user = Helpers::generate_user( $this->user_data );
$user = Utils::generate_user( $this->user_data );
$this->assertIsObject( $user );
$this->assertInstanceOf( 'WP_User', $user );

Expand All @@ -273,7 +274,7 @@ public function test_generate_user_returns_user_if_username_exists_and_has_tries
add_filter( 'jetpack_sso_allowed_username_generate_retries', array( $this, 'return_one' ) );
wp_insert_user( $this->user_data );

$user = Helpers::generate_user( $this->user_data );
$user = Utils::generate_user( $this->user_data );

$this->assertIsObject( $user );
$this->assertInstanceOf( 'WP_User', $user );
Expand All @@ -289,7 +290,7 @@ public function test_generate_user_returns_user_if_username_exists_and_has_tries
*/
public function test_generate_user_sets_user_role_when_provided() {
$this->user_data->role = 'administrator';
$user = Helpers::generate_user( $this->user_data );
$user = Utils::generate_user( $this->user_data );
$this->assertContains( 'administrator', get_userdata( $user->ID )->roles );
}

Expand Down
6 changes: 1 addition & 5 deletions projects/plugins/jetpack/.phan/baseline.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@
// PhanUndeclaredVariableDim : 6 occurrences
// PhanImpossibleCondition : 5 occurrences
// PhanNonClassMethodCall : 5 occurrences
// PhanPluginUnreachableCode : 5 occurrences
// PhanTypeMismatchDimAssignment : 5 occurrences
// PhanTypeSuspiciousStringExpression : 5 occurrences
// PhanUndeclaredStaticProperty : 5 occurrences
Expand All @@ -79,6 +78,7 @@
// PhanTypeInvalidRightOperandOfNumericOp : 4 occurrences
// PhanDeprecatedFunctionInternal : 3 occurrences
// PhanDeprecatedTrait : 3 occurrences
// PhanPluginUnreachableCode : 3 occurrences
// PhanStaticPropIsStaticType : 3 occurrences
// PhanTypeConversionFromArray : 3 occurrences
// PhanTypeMismatchArgumentReal : 3 occurrences
Expand Down Expand Up @@ -459,10 +459,6 @@
'modules/sitemaps/sitemap-librarian.php' => ['PhanTypeMismatchArgument', 'PhanTypeMismatchReturnProbablyReal'],
'modules/sitemaps/sitemap-logger.php' => ['PhanTypeMismatchProperty'],
'modules/sitemaps/sitemaps.php' => ['PhanNoopNew', 'PhanTypeMismatchArgument'],
'modules/sso.php' => ['PhanNoopNew', 'PhanRedundantCondition', 'PhanTypeMismatchArgument', 'PhanTypeMismatchArgumentProbablyReal', 'PhanTypeMismatchReturn', 'PhanTypeMismatchReturnProbablyReal'],
'modules/sso/class-jetpack-force-2fa.php' => ['PhanDeprecatedFunction'],
'modules/sso/class.jetpack-sso-helpers.php' => ['PhanTypeMismatchArgumentProbablyReal', 'PhanTypeMismatchReturn'],
'modules/sso/class.jetpack-sso-user-admin.php' => ['PhanPluginUnreachableCode', 'PhanTypeArraySuspiciousNullable', 'PhanTypeMismatchArgument', 'PhanTypeMismatchArgumentInternal'],
'modules/stats.php' => ['PhanDeprecatedFunction', 'PhanPossiblyUndeclaredVariable', 'PhanRedundantCondition', 'PhanSuspiciousMagicConstant', 'PhanTypeMismatchArgument', 'PhanTypeMismatchArgumentProbablyReal', 'PhanTypeMismatchReturn', 'PhanTypeMismatchReturnNullable', 'PhanTypeMismatchReturnProbablyReal', 'PhanTypeMissingReturn'],
'modules/subscriptions.php' => ['PhanPossiblyUndeclaredVariable', 'PhanTypeMismatchArgument', 'PhanTypeMismatchArgumentInternal', 'PhanTypeMismatchArgumentProbablyReal', 'PhanTypeMismatchDefault', 'PhanTypeMismatchReturnProbablyReal', 'PhanTypeSuspiciousNonTraversableForeach'],
'modules/subscriptions/subscribe-modal/class-jetpack-subscribe-modal.php' => ['PhanTypeMismatchReturnNullable'],
Expand Down
4 changes: 4 additions & 0 deletions projects/plugins/jetpack/changelog/update-sso-use-package
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: other

SSO: switch to loading feature from the Connection package.
17 changes: 1 addition & 16 deletions projects/plugins/jetpack/class.jetpack.php
Original file line number Diff line number Diff line change
Expand Up @@ -1079,28 +1079,13 @@ public function point_edit_comment_links_to_calypso( $url ) {
* @return array list of callables.
*/
public function filter_sync_callable_whitelist( $callables ) {

// Jetpack Functions.
$jetpack_callables = array(
'single_user_site' => array( 'Jetpack', 'is_single_user_site' ),
'updates' => array( 'Jetpack', 'get_updates' ),
'available_jetpack_blocks' => array( 'Jetpack_Gutenberg', 'get_availability' ), // Includes both Gutenberg blocks *and* plugins.
);
$callables = array_merge( $callables, $jetpack_callables );

// Jetpack_SSO_Helpers.
if ( include_once JETPACK__PLUGIN_DIR . 'modules/sso/class.jetpack-sso-helpers.php' ) {
$sso_helpers = array(
'sso_is_two_step_required' => array( 'Jetpack_SSO_Helpers', 'is_two_step_required' ),
'sso_should_hide_login_form' => array( 'Jetpack_SSO_Helpers', 'should_hide_login_form' ),
'sso_match_by_email' => array( 'Jetpack_SSO_Helpers', 'match_by_email' ),
'sso_new_user_override' => array( 'Jetpack_SSO_Helpers', 'new_user_override' ),
'sso_bypass_default_login_form' => array( 'Jetpack_SSO_Helpers', 'bypass_login_forward_wpcom' ),
);
$callables = array_merge( $callables, $sso_helpers );
}

return $callables;
return array_merge( $callables, $jetpack_callables );
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<?php // phpcs:ignore WordPress.Files.FileName.InvalidClassFileName

use Automattic\Jetpack\Connection\Utils;
use Automattic\Jetpack\Constants;

/**
Expand Down Expand Up @@ -58,7 +59,6 @@ public function validate_input( $object ) {
* @return object|false
*/
public function create_or_get_user() {
require_once JETPACK__PLUGIN_DIR . 'modules/sso/class.jetpack-sso-helpers.php';
// Check for an existing user
$user = get_user_by( 'email', $this->user_data['email'] );
$roles = (array) $this->user_data['roles'];
Expand All @@ -76,7 +76,7 @@ public function create_or_get_user() {
$this->user_data->url = isset( $this->user_data->URL ) ? $this->user_data->URL : '';
$this->user_data->display_name = $this->user_data->name;
$this->user_data->description = '';
$user = Jetpack_SSO_Helpers::generate_user( $this->user_data );
$user = Utils::generate_user( $this->user_data );
}

if ( is_multisite() ) {
Expand Down
Loading
Loading