Fix #34: multiple authentication records can be associated with one user

[ronan-gloo]

Fix multiple creation of the same provider authentication #34

Check if the row with user_id / uid / provider exists in authentication table records, and try to log with.

[huglester]

Hello.

I think this fix isn't good.. I'll try to explain why.

For example we allow old users to link existing accounts with twitter - OK no problem.

But what if one user links to twitterusername1 and the user2 also links to the same twitter user? We will have 1 valid login for two accounts.

I tried to take approach like this: https://gist.github.com/34a7e3e2198dc9a2f49a

1. it would get rid of old duplicates
2. if user1 is logged in via @twitter1, an then user2 wants to link via @twitter1 - it will remove user1 auth
3. It also runs the command after insert (so if the updates are commind, command is not run); it's only run after fresh rows are inserts

@philsturgeon hey. Maybe you have some comments on this? Or maybe others @juanmalleluu , @onema ? This issue is killing us :)

[huglester]

I thought maybe it's even an APP problem? I mean we should not allow similar things on app level... not sure here guys :)

[onema]

I agree with @huglester. I think he brings up very valid points.

1. User should not be automatically logged in when the system is attempting to create a record and the authentication already exist for a different user.
2. I think that authentications should be unique to a user account, and deleting older records if they exist is a sound option.

Yet Ninjauth has some odd behaviors that need to be accounted for:

If a user authentication access keys/tokens expire (in the case of facebook this will happen every 60 days) or is revoked, and a user attempts to log in, the service provider (facebook, twitter) will return new access keys/tokens. At this point a new authentication gets created. We now have two authentication records for the same account, one with valid access keys/tokens, the other one with invalid/revoked ones.

That is what we where trying to address here https://github.com/happyninjas/fuel-ninjauth/pull/36, but as @philsturgeon noted, this code is strategy specific. maybe the post_save is also a good way to deal with this issue.

Thoughts?

[ronan-gloo]

@huglester your scenario is different to the adressed pull-request. This one try to avoid multiple authentication records for the same user. Having one authentication for multiple users seem's a bit weird to me, but anyway, the context is not the same.

But @onema is right, in case of expired token, we need to recreate the authentication record, and remove the old one, but keep it attached to the same user. i'll try the openid strategy and see how we can manage this for all package strategies.