Incorrect access checking for username auto completion

-1

The current "Authored by" field is only for users "administer nodes" permission.

Whoops, administer nodes != administer users. This makes things different.

The true permission should be administer nodes... which almost suggests that the autocomplete function should be part of node.module, not user.module. It depends where else the autocomplete is used in the future.

I think the function should stay in user.module, but node.module should get a menu callback that utilizes it. This is not a problem as user.module is a required module.

Attached patch moves the menu entry from user.module into node.module and fixes the permission check.

Say we wanted to make the Author-field on "edit comment" pages editable. I think the permissions would clash, and you'd be able to by-pass permissions if you have access to at least one (because they'd all share the same callback). So, I don't think this solution is sufficiently generic and possibly insecure. Not?

I think some are compounding two separate idea. In my mind, the username autocomplete callback needs a very minimal permission like 'view user profile'. And that callback belongs in user.module.

The decision about showing the author field and the responsibility for validating its contents belong to node.module and comment.module (in Dries' example).

If the menu item stays in user.module and the permission changed to 'access user profiles', then any users that are 'administer nodes' but not 'access user profiles' would get errors trying to use the autocomplete.

The following would work, but I'm not sure if it's the right way to go:

'access' => user_access('access user profiles') || user_access('administer nodes')

I think the 'access user profiles' permission is best to use, possibly even going far enough to change the autocomplete field to a plain text field if the user doesn't have the permission.

I'm not entirely sure what Dries is talking about in #5.

However, I tend to think that "access user profiles" is the right way to go - I've been testing changing $admin_access to $view_access on NHPR.org, and it seems to work fine. By itself, being able to access those URLs isn't any sort of security concern - the places that depend on it for editing (such as 'administer nodes') is controlled by a much stronger permission.

Thox's comment in #7 would be a problem, true, but I'm trying to figure out when someone would have 'administer nodes' permissions without having the ability to view a user profile (if they're able to edit the node of someone else, shouldn't they know as much public information about that user as possible?).

This is the currently running code I have over on NHPR.org.

The problem with Thox' patch is that it doesn't work when editing comments; some people might have 'adminster comments' permission, but not 'administer nodes' permission.

The problem with Morbus' patch is that everyone can access all usernames. (Example: http://nhpr.org/user/autocomplete/a)

Dries: I don't see that as a problem - a spider could do the same thing by just hitting /user/0, 1, 2, 3, 4, and so on.

Agreed. Committed to HEAD. Thanks.