Login social: If a twitter account doesn't have an e-mail, JHipster sets the wrong user_id

[jodevan]

yo-rc.json.zip

Overview of the issue

If a twitter account doesn't have an e-mail, JHipster sets the wrong user_id in the jhi_social_user_connection table. This user_id belongs to the wrong user.

Motivation for or Use Case

Setting the user_id that belongs to another user is a really bad thing :-)

Reproduce the error

1. Have an existing non-social user in the database;
2. Login with a twitter user that doesn't have an e-mail set;
3. Once logged in, you'll see "You are logged in as user xxx.", where xxx is a pre-existing user.

Related issues

Not applicable

Suggest a Fix

The problem lays in the method createUserIfNotExist(UserProfile userProfile, String langKey, TipoUsuario tipoUsuario, String providerId), in class SocialService:

private User createUserIfNotExist(UserProfile userProfile, String langKey, TipoUsuario tipoUsuario, String providerId) {
    String email = userProfile.getEmail();
    String userName = userProfile.getUsername();
    if (!StringUtils.isBlank(userName)) {
        userName = userName.toLowerCase(Locale.ENGLISH);
    }
    if (StringUtils.isBlank(email) && StringUtils.isBlank(userName)) {
        log.error("Cannot create social user because email and login are null");
        throw new IllegalArgumentException("Email and login cannot be null");
    }
    if (StringUtils.isBlank(email) && userRepository.findOneByLogin(userName).isPresent()) {
        log.error("Cannot create social user because email is null and login already exist, login -> {}", userName);
        throw new IllegalArgumentException("Email cannot be null with an existing login");
    }

    // **********************************************************************************************************
    // If email is null or blank, this method will bring some arbitrary user from the database
    // **********************************************************************************************************
    Optional<User> user = userRepository.findOneByEmail(email);
    if (user.isPresent()) {
        log.info("User already exist associate the connection to this account");
        return user.get();
    }
// ...
}

Suggested fix: just test if the email address is not empty:

if (!StringUtils.isBlank(email)) {
    Optional<User> user = userRepository.findOneByEmail(email);
    if (user.isPresent()) {
        log.info("User already exist associate the connection to this account");
        return user.get();
    }
}

JHipster Version(s)

3.4.2

JHipster configuration, a .yo-rc.json file generated in the root folder

{
"generator-jhipster": {
"jhipsterVersion": "3.4.2",
"baseName": "tc",
"packageName": "com.tc",
"packageFolder": "com/tc",
"serverPort": "8080",
"authenticationType": "session",
"hibernateCache": "hazelcast",
"clusteredHttpSession": "hazelcast",
"websocket": "spring-websocket",
"databaseType": "sql",
"devDatabaseType": "mysql",
"prodDatabaseType": "mysql",
"searchEngine": "no",
"buildTool": "maven",
"enableSocialSignIn": true,
"rememberMeKey": "3f987da705ca593beefbc9c6ef079575463b6d01",
"useSass": false,
"applicationType": "monolith",
"testFrameworks": [
"protractor"
],
"jhiPrefix": "jhi",
"enableTranslation": false
}
}

Entity configuration(s) entityName.json files generated in the .jhipster directory

Not applicable

Browsers and Operating System

Not applicable

[jdubois]

Is this linked to #3511 ?

[jodevan]

It's related to it, but it's slightly different. #3511 claims to allow the e-mail address to be null.
This current bug is more like a consequence of allowing it, because in this scenario the line Optional<User> user = userRepository.findOneByEmail(email); may return some unpredictable user. The fix seems to be simple though and is already included in the bug description.
Sounds fair?

[ruddell]

In order to reproduce it without two Twitter accounts, you have to set admin's email to null. Then login with Twitter and it will say you logged in as admin! Definitely not good

[jdubois]

Yes good catch!