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

Google Sign-in: Migrate the pop-up flow to the new Google Identity Services API #64957

Merged
merged 8 commits into from Jun 30, 2022

Conversation

zaguiini
Copy link
Contributor

@zaguiini zaguiini commented Jun 24, 2022

Proposed Changes

We are migrating to the new Google Identity Services API. This PR migrates the pop-up flow behind a feature flag so existing users won't be affected. The redirect flow will be handled by another PR.

Testing Instructions

In these instructions, we will link a WPCOM account to a Google account. This is the easier path to test as the others involve setting multiple Google accounts and that's less than desirable.

  • Create and activate a WPCOM account using an e-mail (https://10minutemail.net) and a password (do NOT Google login);
  • Run bin/calypso-secrets .config/secrets.php in your WordPress.com sandbox;
  • Copy the output of the script to config/secrets.json in your wp-calypso checkout;
  • Build this branch with NODE_ENV=production CALYPSO_ENV=production yarn run build;
  • Run these commands in the Calypso root folder so you can SSL the local version you just built:
openssl req \
    -newkey rsa:2048 \
    -x509 \
    -nodes \
    -keyout ./config/server/key.pem \
    -new \
    -out ./config/server/certificate.pem \
    -subj /CN=wordpress.com \
    -reqexts SAN \
    -extensions SAN \
    -config <(cat /System/Library/OpenSSL/openssl.cnf \
     <(printf '[SAN]\nsubjectAltName=DNS:wpcalypso.wordpress.com')) \
    -sha256 \
    -days 365
 
# Add it to the trusted certificates
sudo security add-trusted-cert -d -k $(security list-keychains | grep System | cut -d '"' -f 2 ) ./config/server/certificate.pem
  • Kick off the server with NODE_ENV=production HOST=wpcalypso.wordpress.com PORT=443 PROTOCOL=https CALYPSO_ENV=wpcalypso BUILD_TRANSLATION_CHUNKS=true node build/server.js;
  • Proxy wpcalypso.wordpress.com to 127.0.0.1;
  • Open the dev tools and paste document.cookie='G_AUTH2_MIGRATION=informational'.

Keep the DevTools open to assert a few details below...

Verify that the current flow still works

It should connect the account successfully. You should also be able to see that /me/social-login/connect was called successfully in the network tab in the DevTools.

Verify that the new flow works

It should connect the account successfully. The network tab in the DevTools should show that both the calls to /wp-login.php?action=exchange-social-auth-code and /me/social-login/connect were issued successfully.


Verifying that social sign-ups work

The new flow should work all across the application since they're using the same component. If you'd like to try a sign-up instead of a social connection, here's how you can do so:

You should be redirected to the /start/domains page. In the background, requests to /wp-login.php?action=exchange-social-auth-code and /users/social/new should've been successfully issued.

Related to #64788.

@zaguiini zaguiini requested a review from a team June 24, 2022 22:06
@zaguiini zaguiini self-assigned this Jun 24, 2022
@matticbot matticbot added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Jun 24, 2022
@matticbot
Copy link
Contributor

matticbot commented Jun 24, 2022

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

App Entrypoints (~465 bytes added 📈 [gzipped])

name         parsed_size           gzip_size
entry-login      +1588 B  (+0.2%)     +465 B  (+0.2%)

Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used.

Sections (~856 bytes added 📈 [gzipped])

name             parsed_size           gzip_size
jetpack-connect      +1611 B  (+0.2%)     +454 B  (+0.2%)
security             +1588 B  (+0.3%)     +402 B  (+0.3%)
accept-invite        +1588 B  (+0.4%)     +436 B  (+0.5%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Async-loaded Components (~456 bytes added 📈 [gzipped])

name                          parsed_size           gzip_size
async-load-signup-steps-user      +1588 B  (+0.9%)     +436 B  (+0.8%)
async-load-design-blocks          +1588 B  (+0.1%)     +443 B  (+0.1%)

React components that are loaded lazily, when a certain part of UI is displayed for the first time.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

@zaguiini zaguiini force-pushed the update/pop-up-sign-in-with-google-deprecation branch from 4694445 to 49e8f42 Compare June 24, 2022 22:22
@zaguiini zaguiini linked an issue Jun 27, 2022 that may be closed by this pull request
5 tasks
@zaguiini zaguiini force-pushed the update/pop-up-sign-in-with-google-deprecation branch from 49e8f42 to 8d3b779 Compare June 27, 2022 18:18
Copy link
Contributor Author

@zaguiini zaguiini left a comment

Choose a reason for hiding this comment

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

I did not see the consent screen when creating an account for a brand new Google account, which is good.

client/components/social-buttons/google.js Show resolved Hide resolved
config/development.json Show resolved Hide resolved
client/components/social-buttons/google.js Show resolved Hide resolved
@zaguiini zaguiini marked this pull request as ready for review June 27, 2022 19:18
@vykes-mac
Copy link
Contributor

I have tested the mentioned cases:

Verify that the current flow still works
✅ Check that the #64538 shows up in the console;
✅ Google account connect successfully
✅ You should also be able to see that /me/social-login/connect was called successfully in the network tab in the DevTools.

Verify that the new flow works
✅ Check that the feature was activated by the Config flag enabled via URL: migration/sign-in-with-google message in the console;
✅ Check that the deprecation notice is not there anymore;
✅ Google account connect successfully
✅ The network tab in the DevTools should show that both the calls to /wp-login.php?action=exchange-social-auth-code and /me/social-login/connect were issued successfully.

Verifying that social sign-ups work
✅ Check that the feature was activated by the Config flag enabled via URL: migration/sign-in-with-google message in the console;
✅ Check that the deprecation notice is not there anymore;
✅ Continue with google redirected to the /start/domains page.
✅ In the background, requests to /wp-login.php?action=exchange-social-auth-code and /users/social/new should've been successfully issued

Copy link
Contributor

@vykes-mac vykes-mac left a comment

Choose a reason for hiding this comment

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

code looks good and tests works as expected

@zaguiini zaguiini merged commit 8243368 into trunk Jun 30, 2022
@zaguiini zaguiini deleted the update/pop-up-sign-in-with-google-deprecation branch June 30, 2022 21:36
@github-actions github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jun 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Google Sign-in: Migrate the pop-up flow to the new Google Identity Services API
4 participants