Login: Allow logging in with social accounts #14082

pento · 2017-05-16T04:45:40Z

This pull request addresses #14076 by adding a Google social login button, and associated login routines.

Testing instructions

Run git checkout add/14076-social-login and start your server, or open a live branch
Open the Login page
Login with a Google account.

Reviews

Code
Design
Product

matticbot · 2017-05-16T04:45:41Z

Test live: https://calypso.live/?branch=add/14076-social-login

pento · 2017-05-16T04:46:34Z

7e72fa3 mostly just adds a barely styled button, and enough code framework for things to run, but doesn't do much yet. 🙂

yurynix · 2017-05-17T06:39:32Z

client/blocks/login/social.jsx

+		}
+
+		this.props.loginSocialUser( 'google', response.Zi.id_token ).then( () => {
+			this.props.onSuccess( this.state );


Sorry for my confusion, but what is this.state ?

Good catch, some left over code from while I was messing around. I'll fix it.

yurynix · 2017-05-18T11:19:40Z

Code looks good to me 👍 Thank you for working on that.

I think we already removed the proxy check from the server side, so maybe we can drop that already?

Tug · 2017-05-18T13:49:45Z

LGTM 👍
I think we can come back to it later to improve the css for instance.

gziolo · 2017-05-18T14:56:58Z

client/blocks/login/social.jsx

+	constructor() {
+		super();
+
+		this.handleGoogleResponse = this.handleGoogleResponse.bind( this );


You can use arrow function style instead of binding.

handleGoogleResponse = ( response ) => { ... };

gziolo · 2017-05-18T15:00:56Z

client/state/action-types.js

@@ -669,6 +669,9 @@ export const SITE_VOUCHERS_REQUEST_SUCCESS = 'SITE_VOUCHERS_REQUEST_SUCCESS';
 export const SITE_WORDPRESS_UPDATE_REQUEST = 'SITE_WORDPRESS_UPDATE_REQUEST';
 export const SITE_WORDPRESS_UPDATE_REQUEST_FAILURE = 'SITE_WORDPRESS_UPDATE_REQUEST_FAILURE';
 export const SITE_WORDPRESS_UPDATE_REQUEST_SUCCESS = 'SITE_WORDPRESS_UPDATE_REQUEST_SUCCESS';
+export const SOCIAL_LOGIN_REQUEST = 'SOCIAL_LOGIN_REQUEST';


Do we plan to consume those action types? :) I assume, we do. I think that at least SOCIAL_LOGIN_REQUEST_FAILURE is going to be useful to display error message when something goes wrong.

gziolo · 2017-05-18T15:03:25Z

client/blocks/login/social.jsx

+
+export default connect(
+	( state ) => ( {
+		isRequesting: isRequesting( state ),


Those two props isRequesting and requestError are never consumed.

gziolo · 2017-05-18T15:04:52Z

@pento do you plan to continue to work on this one or merge it as it is?
This PR has already 150 lines and is still behind the feature flag so we can merge it as it is and address remaining issues in the follow up PR.

Tug · 2017-05-18T16:55:52Z

@gziolo I addressed your comments, should be good to go now.
I agree we should try to merge it first as it will help us test the new social login endpoint.

drewblaisdell · 2017-05-18T18:43:35Z

client/blocks/login/index.jsx

@@ -134,7 +134,7 @@ class Login extends Component {
 		}

 		return (
-			<LoginForm onSuccess={ this.handleValidUsernamePassword } />


We still need to call handleValidUsernamePassword to ensure that users with 2FA enabled are redirected to the 2FA pages, otherwise it isn't possible to log in with a 2FA account.

(Requires new endpoint on WP.com that is currently not committed.)

drewblaisdell · 2017-05-18T19:06:17Z

I added a commit to fix 2FA and rebased. If you're cool with that (very minor) change, this LGTM. 👍

stephanethomas · 2017-05-19T06:15:10Z

client/state/login/actions.js

@@ -33,6 +36,7 @@ const loginErrorMessages = {
 	unknown: translate( 'Invalid username or password.' ),
 	account_unactivated: translate( 'This account has not been activated. Please check your email for an activation link.' ),
 	sms_recovery_code_throttled: translate( 'You can only request a recovery code via SMS once per minute. Please wait and try again.' ),
+	forbidden_for_automattician: 'Cannot use social login with an Automattician account',


I'm wondering (maybe for no good reason) if we really have to return an error that specific (could be just social_login_forbidden with no mention about Automatticians).

I don't see why we should not show this error to our fellow automatticians

pento added Login [Type] Enhancement labels May 16, 2017

matticbot added the [Size] L Large sized issue label May 16, 2017

fabianapsimoes added [Type] Task [Status] In Progress and removed [Type] Enhancement labels May 16, 2017

pento self-assigned this May 17, 2017

pento force-pushed the add/14076-social-login branch from 5e61fae to 8d32aab Compare May 17, 2017 00:43

yurynix reviewed May 17, 2017

View reviewed changes

pento force-pushed the add/14076-social-login branch from 8498c7b to 4d27baa Compare May 18, 2017 01:54

gziolo reviewed May 18, 2017

View reviewed changes

gziolo added [Status] Needs Author Reply and removed [Status] In Progress labels May 18, 2017

Tug added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] Needs Author Reply labels May 18, 2017

Tug self-assigned this May 18, 2017

drewblaisdell reviewed May 18, 2017

View reviewed changes

pento added 5 commits May 18, 2017 12:01

Add a Google login button, and some initial code wireframes.

8961b8c

Fix a CSS class not being renamed correctly.

891de55

Fix a couple of SSR inconsistencies

16910bd

Add the ability to login with Google

c658d1b

(Requires new endpoint on WP.com that is currently not committed.)

Tidy up some left over code from local experiments.

c561c95

pento and others added 5 commits May 18, 2017 12:01

Remove an unused import

d55471b

Remove error message for proxy check

063008c

Use fat arrow syntax four component bounded function

ea79f2d

Remove unused props and fix errorNotice

0402050

Login: Use success handler that supports 2FA

bc6aa71

drewblaisdell force-pushed the add/14076-social-login branch from 856dda0 to bc6aa71 Compare May 18, 2017 19:01

drewblaisdell added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels May 18, 2017

stephanethomas reviewed May 19, 2017

View reviewed changes

Tug merged commit 8c0ca8b into master May 19, 2017

Tug deleted the add/14076-social-login branch May 19, 2017 08:53

matticbot removed the [Status] Ready to Merge label May 19, 2017

pento mentioned this pull request Jun 27, 2017

Login: Allow logging in with social accounts #14076

Closed

This was referenced May 30, 2021

[Snyk] Fix for 1 vulnerabilities ajesse11x/wp-calypso#52

Open

[Snyk] Security upgrade @wordpress/editor from 9.0.11 to 9.1.0 awaisab172/wp-calypso#79

Open

This was referenced Oct 24, 2021

[Snyk] Security upgrade @wordpress/editor from 9.0.11 to 9.1.0 awaisab172/wp-calypso#101

Open

[Snyk] Fix for 1 vulnerabilities ajesse11x/wp-calypso#65

Open

snyk-bot mentioned this pull request Nov 4, 2021

[Snyk] Fix for 1 vulnerabilities ajesse11x/wp-calypso#66

Open

ajesse11x mentioned this pull request Oct 20, 2023

[Snyk] Fix for 2 vulnerabilities ajesse11x/wp-calypso#93

Open

awaisab172 mentioned this pull request Oct 20, 2023

[Snyk] Security upgrade @wordpress/editor from 9.0.11 to 9.1.0 awaisab172/wp-calypso#162

Open

ajesse11x mentioned this pull request Nov 16, 2023

[Snyk] Fix for 2 vulnerabilities ajesse11x/wp-calypso#95

Open

awaisab172 mentioned this pull request Nov 17, 2023

[Snyk] Security upgrade @wordpress/editor from 9.0.11 to 9.1.0 awaisab172/wp-calypso#167

Open

awaisab172 mentioned this pull request Mar 27, 2024

[Snyk] Security upgrade @wordpress/editor from 9.0.11 to 9.1.0 awaisab172/wp-calypso#188

Open

ajesse11x mentioned this pull request Mar 28, 2024

[Snyk] Fix for 2 vulnerabilities ajesse11x/wp-calypso#102

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Login: Allow logging in with social accounts #14082

Login: Allow logging in with social accounts #14082

pento commented May 16, 2017 •

edited by Tug

Loading

matticbot commented May 16, 2017

pento commented May 16, 2017

yurynix May 17, 2017

pento May 17, 2017

yurynix commented May 18, 2017

Tug commented May 18, 2017

gziolo May 18, 2017

gziolo May 18, 2017

gziolo May 18, 2017 •

edited

Loading

gziolo commented May 18, 2017 •

edited

Loading

Tug commented May 18, 2017

drewblaisdell May 18, 2017

drewblaisdell commented May 18, 2017

stephanethomas May 19, 2017

Tug May 19, 2017

Login: Allow logging in with social accounts #14082

Login: Allow logging in with social accounts #14082

Conversation

pento commented May 16, 2017 • edited by Tug Loading

Testing instructions

Reviews

matticbot commented May 16, 2017

pento commented May 16, 2017

yurynix May 17, 2017

Choose a reason for hiding this comment

pento May 17, 2017

Choose a reason for hiding this comment

yurynix commented May 18, 2017

Tug commented May 18, 2017

gziolo May 18, 2017

Choose a reason for hiding this comment

gziolo May 18, 2017

Choose a reason for hiding this comment

gziolo May 18, 2017 • edited Loading

Choose a reason for hiding this comment

gziolo commented May 18, 2017 • edited Loading

Tug commented May 18, 2017

drewblaisdell May 18, 2017

Choose a reason for hiding this comment

drewblaisdell commented May 18, 2017

stephanethomas May 19, 2017

Choose a reason for hiding this comment

Tug May 19, 2017

Choose a reason for hiding this comment

pento commented May 16, 2017 •

edited by Tug

Loading

gziolo May 18, 2017 •

edited

Loading

gziolo commented May 18, 2017 •

edited

Loading