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: Show wp-admin login form if site has local users #39139

Merged
merged 2 commits into from
Sep 4, 2024
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: minor
Type: changed

SSO: Show wp-admin login form if site has local users
43 changes: 27 additions & 16 deletions projects/plugins/wpcomsh/wpcomsh.php
Original file line number Diff line number Diff line change
Expand Up @@ -217,26 +217,37 @@ function wpcomsh_jetpack_sso_auth_cookie_expiration( $seconds ) {
add_filter( 'jetpack_sso_auth_cookie_expiration', 'wpcomsh_jetpack_sso_auth_cookie_expiration' );

/**
* Determine if users who are already logged in to WordPress.com are automatically logged in to wp-admin.
* Determine if users should be enforced to log in with their WP.com account.
*
* Sites without local users:
* - WP.com login, always.
*
* Sites with local users:
* - If user comes from Calypso: WP.com login
* - Otherwise: Jetpack SSO login, so they can decide whether to use a WP.com account or a local account.
*/
function wpcomsh_bypass_jetpack_sso_login() {
/**
* Sites with the classic interface:
* - Automatic login if they come from Calypso.
* - Otherwise we display the login form, so they can decide whether to use a WP.com account or a local account.
*/
if ( 'wp-admin' === get_option( 'wpcom_admin_interface' ) ) {
$calypso_domains = array(
'https://wordpress.com/',
'https://horizon.wordpress.com/',
'https://wpcalypso.wordpress.com/',
'http://calypso.localhost:3000/',
'http://127.0.0.1:41050/', // Desktop App.
);
return in_array( wp_get_referer(), $calypso_domains, true );
$calypso_domains = array(
'https://wordpress.com/',
'https://horizon.wordpress.com/',
'https://wpcalypso.wordpress.com/',
'http://calypso.localhost:3000/',
'http://127.0.0.1:41050/', // Desktop App.
);
if ( in_array( wp_get_referer(), $calypso_domains, true ) ) {
return true;
}

if ( class_exists( '\Automattic\Jetpack\Connection\Manager' ) ) {
$connection_manager = new \Automattic\Jetpack\Connection\Manager( 'jetpack' );
$users = get_users( array( 'fields' => array( 'ID' ) ) );
foreach ( $users as $user ) {
if ( ! $connection_manager->is_user_connected( $user->ID ) ) {
Copy link
Member

Choose a reason for hiding this comment

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

How does this perform for large sites? > 10k users?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question! I have no idea 😅 I'll see if there is an easy way to populate a test site with 10k fake users and check

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd say though that if a site has 10k users, it's very likely that most of them will be non-connected, and we'll stop the loop when the first one is found.

Copy link
Member Author

Choose a reason for hiding this comment

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

Used FakerPress to create 5k users (not connected to WP.com) on a site:

Screenshot 2024-09-04 at 11 22 11

It seems that the performance is not largely affected in this scenario.

Before, it took 1.34s to show the Jetpack SSO login. Now, it takes 1.53s

Before After
Screenshot 2024-09-04 at 11 24 03 Screenshot 2024-09-04 at 11 22 51

Copy link
Contributor

Choose a reason for hiding this comment

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

Imo, that's just one scenario. If 4.9k users are connected there would be a higher performance penalty. This has an O(n) complexity.

return false;
}
}
}

// Users of sites with the default interface are always logged in automatically.
return true;
}
add_filter( 'jetpack_sso_bypass_login_forward_wpcom', 'wpcomsh_bypass_jetpack_sso_login' );
Expand Down