Only use provider and provider user IDs to find connection

[pib]

Seems overkill to use all the fields just to check if the given connection already exists...

[mattupstate]

How is this overkill exactly? You've just removed the user_id parameter, no?

[pib]

No, it's also removing all of the other fields, which includes full_name, display_name, secret, profile_url, etc.

[mattupstate]

I still don't understand why its overkill.

[pib]

Because the provider_id and the provider_user_id are all that's needed to uniquely identify a connection. It's a waste of cpu cycles and memory to require all the other fields to be compared and it should have the same end result.

Actually, one difference it might have is if someone changes their profile after linking, in which case it would show up as not linked despite having the same provider_user_id. So I take it back, it's not overkill, it's a bug.

If I link with my google account, then change my display name on that google account, then I would be able to link with that same google account again because it wouldn't match because the display name is different, even though it is literally the same account.

[mattupstate]

That makes sense to me. Can you provide steps to produce/reproduce this bug? And perhaps even write the required tests to verify the code changes?

[eriktaubeneck]

This is interesting, because if the access_token and/or secret change, this doesn't fail, nor do we get a second connection object. I'm working on a PR to take care of that situation, but it is interesting that it doesn't fail.

[pib]

Sure, I'll write some tests which reproduce/demonstrate the bug and then also show that the code changes fix it.

[mattupstate]

Thanks, Paul!

[eriktaubeneck]

@pib I just wrote a quick test to see what was going on, but it seems that I don't know enough about the mock library off the top of my head to get it working. The issue I was talking about above was actually only with respect to the login route, and not the connect route (which seems to only look at user_id.) Anyways, you can see the template of a test on this branch.

[eriktaubeneck]

The issue with my test seems to have to do with this if you're interested.