Opened 8 years ago
Closed 7 years ago
#7349 closed defect (bug) (fixed)
When user is not logged , in he/she clicks email link to view new messages gets 404 page when should get login page
Reported by: | dkelm | Owned by: | |
---|---|---|---|
Milestone: | 2.9 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Core | Keywords: | has-patch |
Cc: |
Description
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
Attachments (4)
Change History (26)
This ticket was mentioned in Slack in #buddypress by dcavins. View the logs.
8 years ago
#3
@
8 years ago
- Summary changed from When user is not logged, he/she clicks email link to view new messages gets 404 page when should get login page to When user is not logged , in he/she clicks email link to view new messages gets 404 page when should get login page
Not Logged In
#4
@
8 years ago
- Keywords dev-feedback added; needs-patch needs-testing removed
- Milestone changed from 2.7.3 to Under Consideration
- Priority changed from highest to normal
- Severity changed from blocker to normal
- Version 2.7.2 deleted
#5
@
8 years ago
- Keywords needs-patch added; dev-feedback removed
- Milestone changed from Under Consideration to Future Release
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 bebp_core_no_access()
(or a wrapper with appropriate redirects).
This ticket was mentioned in Slack in #buddypress by dcavins. View the logs.
8 years ago
#7
follow-up:
↓ 8
@
8 years ago
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 http://bptest.local/members/three/friends/requests/. Huh.
#8
in reply to:
↑ 7
@
8 years ago
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.
This ticket was mentioned in Slack in #buddypress by dcavins. View the logs.
8 years ago
#10
@
8 years ago
- Keywords has-patch added; needs-patch removed
- Milestone changed from Future Release to 2.9
This ticket was mentioned in Slack in #buddypress by hnla. View the logs.
8 years ago
#13
@
8 years ago
ray.patch
implements the following:
- New
'visibility'
parameter inbp_core_new_nav_item()
. If set tofalse
, 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 theauth
parameter tobp_core_no_access()
so we don't have to manually call it as in7349.2.patch
. We should maybe think about renaming theauth
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 frommessages_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.
#14
@
8 years ago
- Keywords needs-testing removed
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.
#15
@
8 years ago
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!
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."