[Fixed...see PR] Incorrect / dummy email saved to DB after social login with github

[connecteev]

After (successful) auth with github, this is what I see gets stored in the database: 64816@github.local and social_id in user_social table has a value of 64816.

The email is clearly incorrect. Any ideas why the correct (real) email doesn't get stored?

[connecteev]

This issue does not arise with Google login, the correct email is stored.

[connecteev]

Same issue is seen with Linkedin login as well. Any ideas? This is what I get back: Fyb1sl37oe@linkedin.local

[connecteev]

Found the problem. I am really not sure what the reasoning is behind doing this (see the conditional logic below if $service != 'google'. Removing the condition makes both Github and Linkedin logins work and I get the correct, valid email address back. I'll submit a PR on this and hope we can shed light on why this existed in the first place..

    public function callback($service)
    {
...
...
        $email = $serviceUser->getEmail();
        if ($service != 'google') {
            $email = $serviceUser->getId() . '@' . $service . '.local';
        }

...
...
...

    public function getExistingUser($serviceUser, $email, $service)
    {
        if ($service == 'google') {
            return User::where('email', $email)->orWhereHas('social', function ($q) use ($serviceUser, $service) {
                $q->where('social_id', $serviceUser->getId())->where('service', $service);
            })->first();
        } else {
            $userSocial = UserSocial::where('social_id', $serviceUser->getId())->first();
            return $userSocial ? $userSocial->user : null;
        }
    }

[connecteev]

PR submitted: https://github.com/fritsvt/laravel-nuxt-authentication/pull/7

[fritsvt]

I did this strategy as security measure for services like Github and Facebook where you're not always sure the email is verified. I agree that it's a bit weird to store the service ID as email but I'm not sure if it's a good idea to store the email for all services since it might lead to high jacking of accounts.

Maybe we could look into a better solution where we don't login users with the existing account when they're registered using the local authentication strategy.

[connecteev]

@fritsvt that's interesting...but can I propose not making it our problem to mutate the email...put the onus on the consumer of the repo to decide. Besides, it's very weird to get a service ID back in the email field.

A solution could be simply doing what I'm doing here (sending the accurate email 100% of the time) + sending back (and storing the db) a "email_verified_by_provider" boolean. If a provider does not send back any data indicating the email has been verified or not as part of the oAuth response, then we should assume it is, and proceed as usual.

[fritsvt]

I agree that it's not good practice. I didn't intend for this to be fully ready to go boilerplate though but it would be cool to move in that direction and make it bulletproof.

Your solution sounds good but I don't seem to be able to find it in the PR. Also what would happen if the email_verified_by_provider is returned false?

[connecteev]

@fritsvt yes this isn't in the PR, I'm proposing it now :) It can be added later...I think storing the correct email is more important for now (thats the first thing I care about and look for as an app developer when I provide social login..."did I get the users email?")

Here are some scenarios. In user_social:

• add "email_verified_by_provider" with a default value of false or 0.
• If a provider doesnt send anything back, we could save 1 or 0 (up to you, there are pros and cons to each)
• If a provider sends back "verified=true" (or equivalent), save the value as true or 1. For example, Google sends back ["verified_email"]=> bool(true)
• If a provider sends back "verified=false" (or equivalent), keep the value as false or 0. It's up to the application developer to decide whether they should go ahead and treat that email as kosher. The onus should be on them

I also would love to make this code bulletproof, and provde a repo with perfect nuxt + laravel auth + laravel socialite functionality. Let me know if you're planning on advancing and maintaining this repo going forward and if issues are appropriate and I will put in some requirements that make sense for any laravel +nuxt project.

[fritsvt]

Yeah it would be good to make a bulletproof update. I think that should include a way to handle an unverified email though. Because I don't want people deploying sites with security vulnerabilities.