When user is not logged , in he/she clicks email link to view new messages gets 404 page when should get login page

[dkelm]

As described in this link: ​https://buddypress.org/support/topic/error-404-for-non-logged-in-users/

To test this scenario the user must not be logged in!

Then a use get an email saying they have a new message.

Example: website.com/members/melissaverton/messages/view/8/

The user clicks on the link and they are brought to a 404 page and not a login page... User must login -> You must log in to access the page you requested.

Last time this functionality was working in version 2.5.3

The issue is in file bp-core-catchuri.php

function bp_core_no_access

[dkelm]

Please be sure when you test that you are not logged into the system.
Click link from email -> You should be brought to login page with error message: "You must log in to access the page you requested."

[dkelm]

Not Logged In

[slaFFik]

There are so many ways to fix this, that I'm not really sure how, when and should we fix this.

Related: #6307, #7341 (closed by me as wontfix)

[boonebgorges]

I think we should fix this in a general way.

Currently, subnav items with the user_has_access=false flag are never actually added to the component nav. The way it *should* work (and the way that BP_Group_Extension works, to some extent) is:

• Separate the concept of 'access' (user can visit the tab) from the concept of 'visibility' (user can see the subnav item. user_has_access could maybe be a back-compat way of defining them both at the same time.
• When visibility=false, don't add the nav item (or add it, but don't render it - this is a harder job, but maybe better in the long run). *Do* continue to register the screen function.
• When access=false, the screen function should be bp_core_no_access() (or a wrapper with appropriate redirects).

[dcavins]

Route message inbox URLs through the login form.

[dcavins]

I've just added a patch that uses the strategy in the BP group email subscription plugin: Send all links to a message thread through the login form so that the user has an opportunity to log in if not logged in. If logged in, the user will be redirected to the thread directly.

I was surprised to find that links to the friend requests management pane work OK without modification, even though they also look like `ttp://bptest.local/members/three/friends/requests/. Huh.

[dcavins]

Replying to dcavins:

I was surprised to find that links to the friend requests management pane work OK without modification, even though they also look like ​http://bptest.local/members/three/friends/requests/. Huh.

A note for my future self: It appears the difference is the main_nav arguments in the component setup. Messages includes the parameter: 'show_for_displayed_user' => bp_core_can_edit_settings(), but the friends component does not. When I comment out that parameter, instead of a 404 I get a login prompt.

[dcavins]

If the user is already logged in, don't stop by the login screen.

[r-a-y]

ray.patch implements the following:

• New 'visibility' parameter in bp_core_new_nav_item(). If set to false, this allows for a nav item to be hidden, but the screen function to still be registered. I still need to add PHPDoc for this parameter.
• Adds the login redirector from BP Group Email Subscription as per 7349.2.patch. Also adds the auth parameter to bp_core_no_access() so we don't have to manually call it as in 7349.2.patch. We should maybe think about renaming the auth parameter to something else.
• For the message single thread screen, use bp_core_no_access() instead of redirecting to /members/X/messages/ if not authenticated. Also removes the redirect from messages_action_conversation() since this shouldn't be added in this function.

---

If we want to be more cautious, use ray.02.patch. It's the same as ray.patch, but removes the visibility parameter from bp_core_new_nav_item().

This still fixes viewing the message single thread if not logged in, but is a little more conservative.

For example, if you are not logged in and you visit example.com/members/X/messages, in ray.patch, you will get redirected to login; in ray.02.patch, it 404s as before.

[r-a-y]

ray.03.patch is even simpler.

If no objections, I will commit this to fix the immediate issue, but I'd want feedback on the rest of the mods in ray.patch to see if they can be added for 2.9.

[boonebgorges]

7349.ray.03.patch​ definitely seems appropriate.

The more general 'visibility' approach seems sound to me, though I haven't tested it. We should be clear on back compat: it could cause privacy issues if we start registering screen functions that previously weren't registered because of their access settings. I'm pretty sure that's what your patch does.

I mentioned here that it'd be nice to continue to add visibility=false items to the nav array, but exclude them at the time of rendering. This change would open up a lot of possibilities beyond the scope of this ticket, since it'd then be possible to modify nav in dynamic ways that require access to all possible nav items. I don't think anything in ray.patch excludes us from moving in this direction in the future, but I wanted to put it out there.

Thanks for working on this!

[r-a-y]

In 11537:

Messages: Fix redirection issue when attempting to view a valid private message thread.

Previously, attempting to go to a valid private message thread URL when
logged out would throw a 404.

This commit properly redirects non-authenticated users attempting to view
a private message thread to the login screen. If a user is authenticated,
but is attempting to view a private message thread without access rights,
we redirect the user back to their own "Messages" page with a notice.

See #7349.

[r-a-y]

In 11538:

Messages: Add better UX when attempting to view an invalid thread.

This commit adds a notice when attempting to view an invalid thread for
logged-in users.

See #7349.

[r-a-y]

Refreshed after recent commits.

[hnla]

@r-a-y Is this commit worthy, do we want to squeeze in so we can test between now a rc, or punt to 3.0?

[r-a-y]

In 11610:

Core: Redirect login links to their rightful spot if already authenticated.

Previously, if a logged-in user clicked on a login link with the
'redirect_to' parameter, the user would need to re-authenticate in order
to get redirected to the given URL.

This commit bypasses the re-authentication process for logged-in users,
which improves the user experience for those clicking on login links via
email. wp_safe_redirect() is used to avoid redirecting to external
links that are not whitelisted by WordPress.

See #7349.

[r-a-y]

Going to mark as fixed.

I've split the 'visibility' stuff into #7552.

Thanks for everyone's feedback on this ticket!