patch causes access denied registering or requesting new password

adding to my queue. :)

Tested out your patch, hunmonk, but it wasn't giving the desired results for me. Attached is an updated patch that produced what you described as the 'intended results' on my test bed.

I had to remove this line of code from the !$may_cache part of hook_user(), because it was stopping user_view() from ever being reached in a few cases, and because this is a duplicate of an access check that already exists in user_view():

$view_access &= $account->status || $admin_access;

Doing that makes the 'page not found' (instead of 'access denied') for non-existent users work. However, typing in 'user/0' or 'user/randomstring' currently redirects to 'user', which is also wrong, and which should also give a 'page not found' response. Updated patch also caters to that issue.

I implemented the patch provided by jaza (user_access_2.patch) and it worked like a charm but then the site would give out access denied error whenever a visitor wanted to register or whenever a user requested a new password. Not knowing what exactly was causing this error, I unistalled all installed modules but that didn't help. So I looked at the differences between the two user.module files I have for two different drupal sites I've installed. I saw that the only differences between these two files was this patch. So I removed the codes and uploaded the file and voila. My problem with registering and requesting new password erroring in access denied was resolved. Of course, the problem with users not being able to view profiles of users never logged in still exist. But I'd rather have this problem than not be able to have new users register or users not being able to login because they have forgotten their password. Maybe someone could review the code.

Register and Request new password access denied

@desiree: thanks for your testing. New patch fixes the problem with access denied for 'user/register' and 'user/password', by first checking that the menu item 'user/'. arg(1) does NOT exist before adding a dynamic callback for it.

Apart from that, the patch remains the same, and still seems to apply and to fix the problem correctly.

Hi Jaza, the menu_get_item() call is a bit peculiar and is not something we do elsewhere. It makes me believe that we are not fixing the problem in the way we should (maybe we need dedicated callbacks)? I haven't actually investigated this too closely so I might be wrong. I'll do a more in-depth review is necessary, but maybe you could have a quick look already?

Dries: the call to menu_get_item() was only introduced with the latest version of the patch (user_access_3.patch). Here is a quick explanation of why the function call is needed:

1. This patch changes the behaviour of the dynamic menu item 'user/'. arg(1), so that it is generated for ANY value of arg(1), rather than just a numeric value.
2. The reason for this is that the callback function for this menu item, user_view(), contains the logic for correctly determining whether to display a 'page not found' or an 'access denied' message, when users try to access a user/* path that they can't or shouldn't be accessing. Therefore, it is important that this callback is fired for user/*, not just for user/[0-9]*.
3. However, this callback should NOT be fired for user/* paths that have already been registered as menu items within the $may_cache part of user_menu(). The easiest way to check that the menu item doesn't already exist, is to call menu_get_item().

The only other way to implement this patch, would be to declare global $_menu, and to check isset($_menu['path index']['user/'. arg(1)]). But IMO, using menu_get_item() is cleaner than checking the global menu tree directly.

Moving out of the "x.y.z" queue to a real queue.

According to the date, the patch would be most relevant with the 5.x-dev queue.

The menu cache is per-user, why not just not define user/(password|login|...) only if the user has a uid?

I noticed that lots of people wish a better solution for access denied. Changing this issue version.

Same thing discussed in http://drupal.org/node/118498.

This isn't happening for me in D8 or D7. Can't confirm D6 at the moment.