[drupal-devel] [bug] Incorrect access checking for username auto completion

[Morbus Iff]

Issue status update for 
http://drupal.org/node/24617
Post a follow up: 
http://drupal.org/project/comments/add/24617

 Project:      Drupal
 Version:      cvs
 Component:    node.module
 Category:     bug reports
 Priority:     normal
 Assigned to:  Anonymous
 Reported by:  drumm
 Updated by:   Morbus Iff
-Status:       patch (code needs review)
+Status:       patch (ready to be committed)
 Attachment:   http://drupal.org/files/issues/_p_24617_autocompleteperms.patch (793 bytes)

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




Morbus Iff



Previous comments:
------------------------------------------------------------------------

Thu, 09 Jun 2005 00:36:55 +0000 : drumm

The auto completion for user name on node edit pages checks
user_access('administer users') when it should be something more like 
node_access($node, 'edit').




------------------------------------------------------------------------

Fri, 10 Jun 2005 15:32:58 +0000 : Thox

-1


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




------------------------------------------------------------------------

Fri, 10 Jun 2005 15:35:53 +0000 : Thox

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.




------------------------------------------------------------------------

Fri, 10 Jun 2005 15:57:50 +0000 : killes at www.drop.org

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.




------------------------------------------------------------------------

Fri, 10 Jun 2005 19:40:57 +0000 : Thox

Attachment: http://drupal.org/files/issues/access.patch (1.55 KB)

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




------------------------------------------------------------------------

Tue, 05 Jul 2005 18:24:15 +0000 : Dries

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?




------------------------------------------------------------------------

Tue, 05 Jul 2005 18:55:17 +0000 : moshe weitzman

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).




------------------------------------------------------------------------

Thu, 07 Jul 2005 08:31:08 +0000 : Thox

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:



<?php
'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.




------------------------------------------------------------------------

Wed, 24 Aug 2005 19:21:39 +0000 : Morbus Iff

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?).