Closed (won't fix)
Project:
Drupal core
Version:
7.x-dev
Component:
user system
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
28 Oct 2008 at 17:27 UTC
Updated:
19 Oct 2014 at 10:53 UTC
Jump to comment: Most recent
Comments
Comment #1
dave reidBecause of the comment, this appears to be very much by design. I'll leave it up to more comments if this should be actually marked 'by design.'
Comment #2
David_Rothstein commentedI'm sure it does serve a purpose, but a function named "user_is_anonymous" should do what it says... otherwise, the API is broken and unusable. So I think this is definitely a bug ;)
I haven't looked at all the places in the code that call this function, but any of them that require this extra check could instead just call a wrapper function that checks
user_is_anonymous() || !empty($GLOBALS['menu_admin']);and that would solve the problem.Comment #3
Gurpartap Singh commenteduser_is_anonymous() is used in _menu_site_is_offline() as the api functions reference says. I am not exactly sure why menu administrators need to pass through the check.
!user_is_logged_in() can actually dictate the result correctly for user_is_anonymous(). Hence there is a chance to drop one of these functions, and merge the check into _menu_site_is_offline() itself, if at all required.
Comment #4
David_Rothstein commentedWell, the API website doesn't always show you all the references. I tried running a grep on the D7 codebase, and in addition to the reference you mention above, this function is also called in:
http://api.drupal.org/api/function/user_register_access/7
It also appears to be the access callback for three menu items in core:
user/login
user/password
openid/authenticate
(see http://api.drupal.org/api/function/user_menu/7 and http://api.drupal.org/api/function/openid_menu/7)
It's not clear which, if any, of the above might be broken as a result of this (the intention appears to have been to grant access to the menu items only).
It looks like #191914: You cannot add user/register to a menu was the issue which introduced this code.
Comment #5
alexanderpas commentedshould be renamed to menu_anonymous_access_check() or something like that...
_menu_site_is_offline() seems to be using the old school function, before the change, and thus should be changed to !user_is_logged_in()
Comment #6
damien tournoud commentedThis is nothing more than an alternative implementation, required by the menu system when managing permissions for the anonymous users. It could read:
Module writers *must* use it in access callbacks, even if it doesn't "technically" always returns the truth.
Because $GLOBALS['menu_admin'] is only set during a limited period of time, it shouldn't hurt to use that function outside the menu context.
This is not a bug. Lowering the priority, and reassigning as a task to study how we could do this is in a less hackish way for Drupal 7.
Comment #7
alexanderpas commentednot looking towards the menu issue... what's the difference between:
!user_is_logged_in()anduser_is_anonymous()actually nothing, and it should stay that way! (or even better, one as alias of the other.)
as
user_is_anonymous()might be used in access-checks or similar, i still consider this a critical bug, as it might restrict or allow when it explicitly shouldn't.I still favor at least the menu_anonymous_access_check() or something similar to that (what's in a name) which has a less global scope.
also, this needs a testcase.
Comment #8
damien tournoud commented@alexanderpass: actually you *should* and *have to* use
user_is_anonymous()in your menu access check. The behavior changes only in a very small period of time (when menu list is generated in the menu admin page), so it's *completely* safe to use this function outside of menu access checks.So please, this is clearly not a critical bug.
The only valid point I see is that
user_is_logged_in()should mimic the behavior to guarantee thatuser_is_logged_in() == !user_is_anonymoud.Comment #9
alexanderpas commentedmenu access check, yes... but what about other kind of access checks??? (like the one for the page offline? or another.)
and what about undefined variables...
Comment #10
damien tournoud commented?
I already told you it is totally safe to use user_is_anonymous() *anywhere*.
Comment #11
alexanderpas commentedso $GLOBALS['menu_admin'] is actually kind of a private global for the menu access check... doesn't sound very clean...
Comment #12
lotyrin commentedChanging the behavior and/or name of this is too late for D7.
Comment #13
tim.plunkettAlso #1197622: My account link / menu item.
Comment #14
chi commentedCannot find user_is_anonymous() in Drupal 8. Has it been removed?
Comment #15
star-szr@Chi indeed. In procedural code, \Drupal::currentUser()->isAnonymous(), in OO you would inject the current_user service (\Drupal\Core\Session\AccountInterface).
I would also add that the it's no longer a lie :) there's no special case for menu admin there any more.
Closing as won't fix because I agree that changing this for D7 is too late.