Access denied instead of Page not found for anonymous

Try the attached patch, that should display 403 for *existing* threads.

I am not 100% sure if this is the right thing to do, since it does give someone the information that such a thread exists. However, that is imho the default behavior in drupal (nodes, users and so on) so I suppose it is ok.

This will probably not make its way into the soon to be released 1.0 version but maybe into a later bugfix release. I have to write some tests for this first, before I can add it.

Updated the patch and added some tests.

subscribing

subscribing

Fixed in 6.x-2.x-dev and 7.x-1.x-dev.

I am quite sure we initially changed to page not found on purpose in order to not even hint to a potential attacker that there is something there to attack...

I have no issues of it being reverted as it has been, but at the same time, private messages are private and I would like to think even the message id should not be exposed if possible.

Hi.

Private MSG - 6.x-2.x-dev

Myself and site users are getting the "page not found" on a regular basis due to being logged out.

One of the more... geeky users asked why it doesn't check if the users session is closed, and if it is closed it could send them to the login page instead of the page not found. While, if their session is still open it direct straight to the message.

The problem is a lot of users log out of the site when done surfing it. They get e-mail notifications of a message and they click the link in the message which takes them to a page not found. It's not very user friendly, and doesn't say "hey you need to login" it just looks like the system is broken.

Maybe implementing this would be better?

Confirmed issue explained by GeekyLass at #8. +1 for correcting this issue as it does seem weird from users' perspective.
Should a separate issue be opened or as it seems related should it be looked at here?
Changing status to 'needs work' for the moment, but please advise.

This does not have anything to do with Privatemsg, that's a generic Drupal issue.

- With the latest -dev that contains this patch, you shouldn't get a page not found but a access denied instead.

With that, you can use a module like http://drupal.org/project/r4032login. You can also set the 403 page yourself at admin/settings/error-reporting but you either need some custom code to handle users that are already logged in and that users are forwarded back to the page they wanted to look at. I assume that both is handled by the linked module.

I updated to the 6.x-2.x-dev 2009-Dec-29 version of the module.

I'm still getting page not found and not access is denied. I attached a .gif from today.

Have you rebuilt your menu with devel.module or by accessing admin/build/modules?

I've been to "admin/build/modules" several times since. It seems to be giving the proper access denied page now. I replicated the error and it's a-okay.

I see that this has been fixed in 6.x-2.x-dev and 7.x-1.x-dev. Do you plan too fix this in 6.x-1.x or do we need to wait for a stable release of 6.x-2.x ?

Thanks for a great module.

I decided against backporting this because I think it's a feature and a (small) change in how Privatemsg works.

The patch above should apply against 1.x as well, so you could try to apply that if you need this.

Hi Berdir,
Thank you for a quick reply, I will try the patch; however, I don't understand your reasoning.

The e-mail notifications of new messages is a feature of the Privatemsg module. Most users are not logged into the site when they get the e-mail and for them to receive a 404 after they follow the link provided in the notification just does not make sense.

As far as I am aware, Privatemsg is the only module that behaves this way, including sites not running Drupal. That said, you are the maintainer of this module and the decision is yours.

Thank you for your time.

When I try to apply the patch against the 6.x-1.1 version of Privatemsg I receive the following error:

Hunk #1 FAILED at 138.
Hunk #2 succeeded at 241 (offset -10 lines).
Hunk #3 succeeded at 272 (offset -10 lines).
Hunk #4 succeeded at 287 (offset -10 lines).
Hunk #5 succeeded at 387 (offset -10 lines).
1 out of 5 hunks FAILED -- saving rejects to file privatemsg.module.rej
can't find file to patch at input line 93
Perhaps you should have used the -p or --strip option?

This is the content of the privatemsg.module.rej file:

***************
*** 138,147 ****
    );
    $items['messages/view/%privatemsg_thread'] = array(
      'title'            => 'Read message',
      'page callback'    => 'privatemsg_view',
      'page arguments'   => array(2),
      'file'             => 'privatemsg.pages.inc',
      'access callback'  => 'privatemsg_view_access',
      'type'             => MENU_LOCAL_TASK,
      'weight'           => -5,
    );
--- 138,151 ----
    );
    $items['messages/view/%privatemsg_thread'] = array(
      'title'            => 'Read message',
+     // Set the third argument to TRUE so that we can show access denied instead
+     // of not found.
+     'load arguments'   => array(NULL, NULL, TRUE),
      'page callback'    => 'privatemsg_view',
      'page arguments'   => array(2),
      'file'             => 'privatemsg.pages.inc',
      'access callback'  => 'privatemsg_view_access',
+     'access arguments' => array(2),
      'type'             => MENU_LOCAL_TASK,
      'weight'           => -5,
    );

I am not programmer nor familiar with patches, so it is very likely I am doing something wrong here.

I would very much appreciate some suggestions on how to deal with this.

No, you're doing it correct, 2.x added a line there so the automated patching doesn't work.

Just look for the line " $items['messages/view/%privatemsg_thread'] = array(" in privatemsg.module and then add all lines that have a "+" before them and remove the + after doing so. Note that the 'file' line will be missing in your version but it should be the same apart of that.

Vielen Dank Berdir.
Thank you very much for your help.

Patch for version 6.x-1.2.

Hi. Has anyone tested the patch in #21 or should I move to the 6.x-2 dev? Thanks

I patched using #21, and it seems to fix the problem for me in 6.x.1.2.

Hi,

I tried to patch PrivateMSG 1.3 with #21 and it's working with latest stable.

I can also confirm that the patch in #21 works. Be sure to flush cache after uploading the new files.

This problem doesn't appear to be fixed at all. The patch from comment 21 does fix it though.

It is fixed in 6.x-2.x and will not be backported to 6.x-1.x.

Try this patch if you still need to fix it in 1.x, I think it's much simpler. It's for 6.x-1.5