Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.This is related to http://drupal.org/node/208888.
When an external auth module approves a new user, user_save() is called. After the user is created, user_module_invoke() is called with the insert op and the new $user.
profile_user() catches this and calls profile_save_profile() which calls _profile_get_fields() which calls user_access(). The call to user_access() fails because the new $user object is passed along the call chain until _profile_get_fields(), when it is lost.
So _profile_get_fields() calls user_access() without passing the new $user. user_access() has no choice but to run the access check on the global $user...which is the wrong user!
This patch passes the new $user along to _profile_get_fields() so that when user_access() is called, the new user account is the one being checked.
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | profile_auth_access_5.x-dev.patch | 1.22 KB | joshk |
| profauthaccessjv.patch | 1.38 KB | jvandyk | |
Comments
Comment #1
moshe weitzman CreditAttribution: moshe weitzman commentedlooks good. needs testing.
Comment #2
birdmanx35 CreditAttribution: birdmanx35 commentedThis still works cleanly against HEAD.
Comment #3
moshe weitzman CreditAttribution: moshe weitzman commented@birdmanx - if you tested and it works, please set to rtbc.
Comment #4
birdmanx35 CreditAttribution: birdmanx35 commented@moshe weitzman. I wasn't sure, because I really have no idea what this patch is doing, or if it's good, or if it is written well. All I know is that it applies cleanly to HEAD.
Comment #5
Gábor HojtsyThe fact that it applies cleanly does not make it ready. Needs testing.
Comment #6
birdmanx35 CreditAttribution: birdmanx35 commentedYep, my apologies.
Comment #7
joshk CreditAttribution: joshk commentedI'm dealing with this in a backport sense. It works, but I wonder if it doesn't also "break" the user_access. The question here is not supposed to be whether or not the user being modivied has the "admin users" pem -- which is what this patch sets the user_access hook to check -- but rather whether or not the user doing the modifying has that perm (which, outside this edge case, is stored in global $user).
I worry that this patch will actually break the ability for profile.module to use admin-only fields.
Comment #8
joshk CreditAttribution: joshk commentedI've tested this and even though intuitively I still have a feeling that passing the $user object of the account being edited to user_access() should be causing issues with hidden/admin-only fields, it seems to work fine.
Comment #9
joshk CreditAttribution: joshk commented5.x-dev backport
Comment #10
jvandyk CreditAttribution: jvandyk commented"user warning: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ')' at line 1 query: SELECT p.perm FROM role r INNER JOIN permission p ON p.rid = r.rid WHERE r.rid IN () /drupal/modules/user/user.module on line 500."
Tested using pubcookie as the external auth module, the error above is brought on by the fact that the wrong user is being checked (see explanation in #1). Here it manifests itself as a role error because we are halfway through user_save() when the 'insert' op is called. At this point the global $user has no roles; the passed $user (which will shortly become the global $user) does; thus we see this error.
Applying the patch results in no error.
Comment #11
dpearcefl CreditAttribution: dpearcefl commentedIs this still an issue in current D6?