Currently, the default connect action provided by fboauth doesn't respect the settings provided by core (admin/config/people/accounts) concerning user creation. For example, if your use case requires administrator approval for account creation, you won't be able to use fboauth out of the box.
We should be able to add a couple quick checks in the default connect action to respect these core settings.
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | fboauth_login_user-d6.patch | 2.68 KB | quicksketch |
| #8 | fboauth_login_user-d7.patch | 3.07 KB | quicksketch |
| #4 | fboauth_admin_approval-d6.patch | 2.05 KB | quicksketch |
| #4 | fboauth_admin_approval-d7.patch | 3.23 KB | quicksketch |
Comments
Comment #1
areynolds commentedThere are a couple parts to this problem.
For part three, I'm basically going to copy code from user_register_submit(). I guess there isn't anyway to avoid the duplication there, unless we used user_register_form programmatically to save the user before adding the picture in fboauth_create_user. The other steps wouldn't involve much duplicate code; they'd mostly be additional checks and slight alterations to the existing behavior in fboauth_create_user.
Any alternatives? What do other modules that create users do to solve this problem? I can't think of anything off the top of my head.
Comment #2
quicksketchHave you already tried using FBOAuth with account creation disabled? It already respects these settings as far as I know. If account creation is disabled, the user account is not created and a message is shown.
Comment #3
quicksketchI've confirmed FBOAuth does not allow a new account if account creation is disabled. The user gets this message:
However I found that the user is immediately logged in if the account setting is set to "Visitors, but administrator approval is required", which is a pretty big problem. We're already supporting direct account creation and disabled account creation, but not this middle case.
Comment #4
quicksketchThese patches fix the issues in both D6 and D7.
The only situation we needed to handle that we weren't already was setting the $user->status property to 0 if administrator approval was required. In order to ensure a smooth workflow, we also need to send the user the pending-account e-mail. Unfortunately to do that we have to call the "private" function _user_mail_notify(), but considering that function hasn't changed since Drupal 5, I don't think this is a terrible trade-off.
Comment #5
quicksketchOne last review and these look good to me. I'll be putting out the 1.2 version shortly.
Comment #6
steve.klingele commentedNow in 7.x-1.2 the changes made with these commits seem to cause the following error with account registration set to "visitors":
Fatal error: Call to undefined function user_external_login() in ... /fboauth/includes/fboauth.fboauth.inc on line 111
Comment #7
quicksketchThanks steve.klingele. You're right, don't know how I missed that.
Comment #8
quicksketchIt looks like a lot of our logic was flawed in the D7 upgrade, as calling user_login_submit() didn't return TRUE or FALSE based on success, so a lot of our messages and workflow was screwy. To prevent this problem in the future and to increase code-consistency between D6 and D7, I've introduced a wrapper function around user_external_login() in D6 and a replacement function in D7 (since user_external_login doesn't exist).
This fixed a bunch of things, like users are now given a welcome message when logging in for the first time, and we got to remove several of the weird fake $form_state['uid'] assignments. Yet another release coming soon.