Out of the box consent form might be a security risk. about oidconnect

[fulup-bzh]

Thank you for the feedback. The security concern you describe is one of the primary reason for me not to use "Socialite". The fact that OidConnect module integrates a real federation model protect us from the risk you describe.

In Socialite, the email your receive from Fakebook, Gmail, etc.. is used to query user in localDB, If you find the email you log corresponding user, if you do not find it you create a new user with that email as primary login key.

In OidConnect you only take UserID as returned by Facebook, Paypal, etc ... Then you check if this external userID exits in corresponding Social Federation table. If yes you retrieve corresponding local user. If not you prompt the user to create a new account, but this new account remains map on federated UserID and not on email or pseudonym values. I only force email and pseudonym to be unique in order to avoid the same user to create multiple account by accident, but technically this would be possible.

How to check this behaviour ?

• 1. To start with something clean. If you already create a demo account from github, login and delete your user on oiddemo.breizhme.net.
• 2. Login and federated a new account on the demo with facebook and any other supported social network [except github, that we will use in next step]
• 3. Logout from demo to pretend you're a different user.
• 4. Log on github and change your public email to be the same as the one used in step-2
• 5. Log on demo with this hacked github account. Despite the fact that you present the same email address that step-2, the system recognizes you as a new user and prompt from consent.
• 6. In consent phase the system will refuse your email from github. This because the two users [step-2 & step-5] are different, even if they claim the same email address.

Now if do the same thing with Socialite, step-5 user is going to log as being step-2 user, which I agree is a huge security risk.

It would have been nice to show all those usecases in the video, but video are already too long. Which is the reason why I had to host them on DialyMotion and accept their nasty advertisements.

[MLenngren]

I agree. Even if this component is hooked in directly to an already existing architecture the ones doing the integration should be aware.

I like the code, easy to read and follow, so changing and understanding what needs to be done was a breeze. Once again, thank you for your work!

[fulup-bzh]

Thank you for the compliment, I still need to make a proper PHP component but I'm somehow lazy :)

Note that automatically linking of multiple social users to one single local user based on email address remains possible. If your consider that a given email coming from a trusted IDP is safe enough to be used for an automatic federation nothing prevent you from doing it. In fact this is a pretty common scenario. OidDemo was originally implementing it, but I removed it to keep things as simple as possible. Few IDPs provide certified email. In my demo all IDP are ranked with the same level of security, but everyone knows that reality is somehow more complex.

For example Google returns a flag with "verified email" in its social user profile. Application may conciser that in such a case, if the end-user does not change his email during consent phase {or if consent does not allow user to change his email]. The application can then automatically federate this new social user with an existing the local account that uses this "verified" email.

What is not implemented out of the box, is the capability to merge after creation two local accounts in one. Some applications may want to group existing account in one single one. This is more complex has you need to understand how to merge data attached with both local accounts. This would happen when an existing account try to change its email and use one that already exist. We could then imagine a consent with "this email is already used, would you like to merge those two account". It is a very interesting use case but I'm not sure on how it could be implemented in a generic component.