There is a bug in the fboauth.fboauth.inc file that prevents a user from creating an account using facebook if there are more than 2 accounts beginning with the same username. For example, if the drupal site has the users: christianblaze & christianblaze1, and if you try to register/logon using facebook Oauth and my facebook username is christianblaze, the module will first try to search for the username christianblaze, and because it finds it exists, it will append a '1' to the username and try again...this is where the module fails, because there is a mistake in the fboauth.fboauth.inc file... this is the current code at line 155 and 156:

$query = "SELECT uid FROM {users} WHERE name = ':name'";
$uid = db_query("SELECT uid FROM {users} WHERE name = :name", array(':name' => $username))->fetchField();

Should read as follows:

$query = "SELECT uid FROM {users} WHERE name = :name";
$uid = db_query($query, array(':name' => $username))->fetchField();

The main culprit were the 'quotes' around ':name' on line 155. Also, because line 156 wasn't using the $query variable, the first username to be appended a '1' would not fail, but those appended a 2 and beyond would, because following these lines we would enter a loop that relied on the $query variable.

Hope this solves some problems out there!

CommentFileSizeAuthor
#10 fboauth_duplicate.patch638 bytesquicksketch

Comments

christianblaze’s picture

Title: Unused $query variable » registration fails due to duplicate username
Priority: Minor » Critical
Status: Active » Fixed

Not yet fixed

quicksketch’s picture

Title: registration fails due to duplicate username » Unused $query variable
Priority: Critical » Minor

Thanks for the report. I'm not sure why this was marked "fixed", as we didn't do anything to change the code yet. Upon inspecting the problem I see that we've got the two lines you mentioned in our code base:

  $query = "SELECT uid FROM {users} WHERE name = ':name'";
  $uid = db_query("SELECT uid FROM {users} WHERE name = :name", array(':name' => $username))->fetchField();

So there are a couple problems here. 1) The quotes around ':name' and 2) the fact that the $query variable isn't used at all. However problem #2 makes the problem in #1 a non-issue, since the query with the quotes is never used. Seems like the end-result of this report is just that we should delete the first line since it's unnecessary. Everything else seems to work fine.

christianblaze’s picture

Title: registration fails due to duplicate username » Unused $query variable
Priority: Critical » Minor
Status: Fixed » Active

Actually, if you look at the code that follows, you will see that the current code ONLY works for the first 2 users ie: christianblaze and christianblaze1... but if we were to already have a christianblaze and a christianblaze1, christianblaze2 is nevere created, and an error ensues... to reproduce, create 2 users using drupals registration form: ie: facebookuser and facebookuser1, then try and log on with your facebook user, and we expect that facebookuser2 should be created, but instead, an error is thrown because with the current code, it tries to create facebookuser1, not facebookuser2, because later on in the code we DO use the $query variable for that...

quicksketch’s picture

Title: Unused $query variable » Duplicate usernames cause SQL error and registration to fail
Priority: Minor » Normal

because later on in the code we DO use the $query variable for that...

Ah, okay thanks for the clarification. So yep sounds like your fixes would indeed solve the problem. :) I'll review and try this out when I get the chance.

christianblaze’s picture

Also, I'm kind of new to posting on this site, I moded this module so that it works with rules... do you think anyone would be interested in that functionality? If so, what do I do to post it so you guys can try it out and consider the changes? :)

quicksketch’s picture

Hi @christianblaze, welcome to drupal.org :)

I personally don't use Rules on any of my sites, so I'm not particularly interested in maintaining Rules support in FBOAuth. The way the module is written you shouldn't need to modify the module though. We've provided hooks enough that you should be able to write a separate module the implements the hooks provided by FBOAuth to trigger your rules. I've been saying similar things over in #1486984: Add hook_fboauth_properties_alter(), allowing other modules to extend mappable field types, where we're working on adding a new hook that fires on user login through FBOAuth.

christianblaze’s picture

Gotcha...

Too bad I don't really know how to use hooks! They confuse the hell out of me!

christianblaze’s picture

What I wish I could do with hooks is fire rules when a user signups using facebook, connects an account to facebook, logsin using facebook, or disconnects their facebook account.... how would I do something like that with hooks in a new module...I checked out a custom module on this site which said it did just this with your module, but all he did was hook and re-write almost the whole module to suit his needs... http://drupal.org/project/facebook_rules

and he added stuff that I dont need like posting to facebook, which requires additional permissions on facebook...

How could I make it simpler with hooks of my own?

christianblaze’s picture

PS: this is a great module...well done, I tried many others, including facebook connect, and drupal for facebook, and they are sooo buggy right now, especially when all you want is to log in using facebook and import a few fields! Thanks for creating it!

quicksketch’s picture

Status: Active » Fixed
StatusFileSize
new638 bytes

I've committed this patch to the project. This problem only affected the D7 branch.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

Issue summary: View changes

spelling