| Project: | Drupal core |
| Version: | 5.x-dev |
| Component: | user system |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
When an external user auth module gives a thumbs-up to a new user, user_external_login_register() finishes the job of creating the user by calling user_save() after initializing some values in $userinfo.
Because of this change, we have a problem...that commit added a call to user_access() inside user_save(). Well, we can't call user_access() because we don't actually have a finished user object to hand it -- we're still in the process of creating it.
But note that user_access() is only called if the 'access' key is not set. So to avoid this problem, we simply add the access key to those that are set while creating the $userinfo array. Now the conditional before the user_access() code successfully prevents the bogus call to user_access() from happening, because the user_access() call is only made if 'access' is empty.
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| extauthaccessjv.patch | 1.23 KB | Ignored: Check issue status. | None | None |
Comments
#1
#2
+1
This is also an issue in 5.x. Backport patch:
http://drupal.org/node/213417
#3
fixes the bug and improves readability a bit.
#4
Well, setting the access time on a user externally logging in is logical on its own right as well, so committed. Thanks.
#5
Automatically closed -- issue fixed for two weeks with no activity.
#6
As per neil's instruction, here's that patch again.
#7
I tested this patch today and it fixed a problem I encountered while writing my own authentication module which I describe in this issue: http://drupal.org/node/237327
I like this patch better than mine, and I will leave a comment to that effect on my own issue. I haven't noticed any problems with the patch yet.
#8
This is not a direct backport of the patch, which only added the 'access' element. With this patch, $userinfo["authname_$module"] is set unconditionally, instead of only when $server evaluates to TRUE. If this was intentional, please explain and set back to reviewed & tested by the community.
#9
I think there is some confusion here, as drumm said the original patch only adds the 'access' element, but that's because the original patch already sets "authname_$module" regardless of the state of $server. Joshk's patch (probably mistakenly) includes an additional change to match the code in the 6.x branch. Since that change is not part of this bug I've taken it out and I'm supplying a new patch that is a direct backport of only the modified lines from the original patch.
I've also tested the patch myself in the first iteration and now this second iteration, so I'm changing the patch status again to reviewed & tested.
#10
This is a direct backport from 6.x. Should be ready to go in.
#11
Manually applied and committed to 5.x.
#12
Automatically closed -- issue fixed for two weeks with no activity.