Generator-jhipster: Social login is acting anti-social and email isNull problem

Created on 21 Jan 2018 · 8Comments · Source: jhipster/generator-jhipster

Overview of the issue

Issues:

there are people who have the same firstname and lastname - how can we fix this? (just kidding - that is not an issue, but related :-)
with the current social login only the first person with an unique firstname_lastname combination will be able to signup - this behaviour is way too anti-social for a social login.
if login is changed via admin-panel in JHipster - social login for connected social users will be broken, too (i guess this feature will be dropped anyway)
email credential is not always provided by social connect providers (if its not verified e.g.) - thus can be null - this case is not handled
- social users are activated by default even with unverified email/null - normal jhipster users have to verify their email via link/email
- when social users with email null try to change their email in jhipster settings, they get an internal server error (cache evict on email will cause NullPointerException)

Related issues:

The jhi_users.login is used in some places of jhipster as fk - jhi_users.id should be used always
there is an open issue #6909 with pending pull request #6955 for checking the current password before changing the password - very good, but it will fail for users, who signed up with social account first, because they get random default passwords (thus not know their current password)

Cause:

if sub/provider from social login connection is not already stored to jhi_social_connection, SocialController signUp will be called.
then signUp will try to establish a connection between existing jhi_user and social account by looking up a jhi_user with the email credential from the social provider
if the social email credential is not already in use by a jhi_user, a new jhi_user will be created.
the new jhi_user is always created with a login generated this way:
- Twitter: username (the @ gets chopped off. Twitters usernames are unique at some point in time, but not stable over time)
- Google/Facebook: firstname + "_" + lastname (and those combinations are far from unique)
email retrieved from social providers can be null

Suggested Fix:

generate the login so that it could not fail to create a new user (perhaps count all users wich begin with the login and add this to login resulting in firstname_username_7 etc.)
add email null handling/error msg
use jhi_users.id as to join instead of jhi_users.login in jhi_social_connection.user_id
add testcases for signUp with same credentials/email null

Perhaps also good to know when working on the implementation:

currently the email_verified OAuth attribute is never checked: Google/Facebook/Twitter will just not provide email credential if it is not verified. So everything is ok regarding security on this side. But you will always get null as email, if its unverified.
if this thing should be easy extendible from everyone perhaps email_verified check should be added (perhaps do the check if provider is not google/facebook/twitter)
(before connecting to other api providers it must be checked if email credentials can be exposed unverified. If it will be exposed, checking the optional email_verified attribute is neccessary, or even better: just do not use the provider. Otherwise with the current and also a fixed implementation attackers will be able to impersonate all existing jhi_users including admins just by guessing or knowing their email - although: that would be really social as you share all your accounts.)

Tests/Reproduce the errors:
Testing and mocking social apis is causing physical pain and social disorder - so perhaps its better to explore the issues with real accounts or just follow the code hitting strg-b e.g..
I tested the issues with Twitter accounts (one of them i registered to an interesting email: system@localhost. This account is working and i can signup/login via Twitter to jhipster apps. Trivia: admin@localhost was already in use...lets hope its not verified either:-)).
I guess due to known issues with SpringSocialConnect regarding Twitter API email - i was not able to get the email credential pulled out the connection from an verified account in the short testing period, thus always resulting in null and new jhi account.
Maybe jhipsters Twitter integration is not working at all due to the error in social connect (thus @ NotNull for email could exclude all twitter users - so that could be another issue).
Do not have tested Facebook API.
Used two google accounts and name myself just admin in both.
SignIn in jhipster with the first account works as intended.
However signIn with the second account, the above mentioned errors occure.
On the frontend the second attemp was resulting in a success and error msg at the same time - Registration saved Login with no-provider...

help wanted

Source

RobinKamps

👍4

All 8 comments

Thanks for the detailed analysis. Yes the social logging is not something
we actively work as it person that contributed the feature isn't active
anymore and most of the core team may not be using the feature. That said I
believe it is an important feature for people (though strangely our stat
doesn't reflect this)

I agree that this needs improvement and you are more than welcome to
improve it. We will help us much as we can. I'm on my mobile now so I'll
answer your questions in detail later.

On 21 Jan 2018 11:19 am, "Robin Kamps" notifications@github.com wrote:

Overview of the issue

Issues:

there are people who have the same firstname and lastname - how can
we fix this? (just kidding - that is not an issue, but related :-)

with the current social login only the first person with an unique
firstname_lastname combination will be able to signup - this behaviour is
way too anti-social for a social login.

if login is changed via admin-panel in JHipster - social login for
connected social users will be broken, too (i guess this feature will be
dropped anyway)

email credential is not always provided by social connect providers
(if its not verified e.g.) - thus can be null - this case is not handled

social users are activated by default even with unverified

email/null - normal jhipster users have to verify their email via link/email

when social users with email null try to change their email in

jhipster settings, they get an internal server error (cache evict on email

will cause NullPointerException)

Related issues:

The jhi_users.login is used in some places of jhipster as fk -
jhi_users.id should be used always

there is an open issue #6909
https://github.com/jhipster/generator-jhipster/issues/6909 with
pending pull request #6955
https://github.com/jhipster/generator-jhipster/pull/6955 for
checking the current password before changing the password - very good, but
it will fail for users, who signed up with social account first, because
they get random default passwords (thus not know their current password)

Cause:

if sub/provider from social login connection is not already stored
to jhi_social_connection, SocialController signUp will be called.

then signUp will try to establish a connection between existing
jhi_user and social account by looking up a jhi_user with the email
credential from the social provider

if the social email credential is not already in use by a jhi_user,
a new jhi_user will be created.

the new jhi_user is always created with a login generated this way:

Twitter: username (the @ gets chopped off. Twitters usernames are

unique at some point in time, but not stable over time)

Google/Facebook: firstname + "_" + lastname (and those

combinations are far from unique)

email retrieved from social providers can be null

Suggested Fix:

generate the login so that it could not fail to create a new user
(perhaps count all users wich begin with the login and add this to login
resulting in firstname_username_7 etc.)

add email null handling/error msg

use jhi_users.id as to join instead of jhi_users.login in
jhi_social_connection.user_id

add testcases for signUp with same credentials/email null

Perhaps also good to know when working on the implementation:

currently the email_verified OAuth attribute is never checked:
Google/Facebook/Twitter will just not provide email credential if it is not
verified. So everything is ok regarding security on this side. But you will
always get null as email, if its unverified.

if this thing should be easy extendible from everyone perhaps
email_verified check should be added (perhaps do the check if provider is
not google/facebook/twitter)
(before connecting to other api providers it must be checked if email
credentials can be exposed unverified. If it will be exposed, checking the
optional email_verified attribute is neccessary, or even better: just do
not use the provider. Otherwise with the current and also a fixed
implementation attackers will be able to impersonate all existing jhi_users
including admins just by guessing or knowing their email - although: that
would be really social as you share all your accounts.)

Tests/Reproduce the errors:
Testing and mocking social apis is causing physical pain and social
disorder - so perhaps its better to explore the issues with real accounts
or just follow the code hitting strg-b e.g..
I tested the issues with Twitter accounts (one of them i registered to an
interesting email: system@localhost. This account is working and i can
signup/login via Twitter to jhipster apps. Trivia: admin@localhost was
already in use...lets hope its not verified either:-)).
I guess due to known issues with SpringSocialConnect regarding Twitter API
email - i was not able to get the email credential pulled out the
connection from an verified account in the short testing period, thus
always resulting in null and new jhi account.
Maybe jhipsters Twitter integration is not working at all due to the error
in social connect (thus @ NotNull for email could exclude all twitter users

so that could be another issue).
Do not have tested Facebook API.
Used two google accounts and name myself just admin in both.
SignIn in jhipster with the first account works as intended.
However signIn with the second account, the above mentioned errors occure.
On the frontend the second attemp was resulting in a success and error msg
at the same time - Registration saved Login with no-provider...

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/jhipster/generator-jhipster/issues/7032, or mute the
thread
https://github.com/notifications/unsubscribe-auth/ABDlF09aHeH1WQersCxT0vBi4Z09JcYtks5tMw8ugaJpZM4Rlw9i
.

deepu105 on 21 Jan 2018

👍1

Yes this is a very good analysis - unfortunately we don't work a lot on that part as it doesn't look widely used.

2 more comments:

we rely on spring-social on the server side, and spring-social-facebook is broken as Facebook changed its API, and as spring-social-facebook hasn't been upgraded yet (which also makes me think this isn't widely used).
we currently have a "username" and an "email" -> in JHipster v5 we might remove the "username", as people now always log in using the email

@RobinKamps if you feel like it, you could become the "maintainer" of that part.

jdubois on 22 Jan 2018

What's the status on this? Would like to contribute.

dajaro on 21 Feb 2018

I don't think anyone is working on this so please do contribute

On 21 Feb 2018 3:19 am, "David" notifications@github.com wrote:

What's the status on this? Would like to contribute.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/jhipster/generator-jhipster/issues/7032#issuecomment-367191821,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABDlF6rzGVhGX12te1ex8A-8W87atGyGks5tW30bgaJpZM4Rlw9i
.

deepu105 on 21 Feb 2018

I still have issue with signup after i signin (facebook)
Cannot GET /social/signup

3.0.0.M1
updated webpack.dev.js proxy: [{
context: [
/* jhipster-needle-add-entity-to-webpack - JHipster will add entity api paths here */
'/api',
'/management',
'/swagger-resources',
'/v2/api-docs',
'/h2-console',
'/auth',
'/signin',
'/signup'
],