bp_core_catch_no_access() redirects to root and doesn't pass referer for login redirect

[ethitter]

If a user isn't logged in and tries to access a restricted section of BuddyPress, bp_core_catch_uri redirects to the root domain without passing the referer for redirection after login. A perfect example of this is in the "friend request" notification email; included is a link to review the requests, but one must be logged in for the link to be of any use.

The problem lies in the function's check for the existance of an appropriate template file, which relies on $bp_path. This variable isn't defined if the user isn't logged in, so the template file check automatically fails. As a result, the user is redirect to the root domain without any explanation.

The easiest solution would be to check if the user is logged in, and if not, redirect to wp-login.php, passing the requested uri as the redirect_to parameter. Post-login, bp_core_catch_uri would handle the template redirection as it does now.

[r-a-y]

Also see #2783 for additional discussion.

[boonebgorges]

The problem is sadly more complex than this.

For items that are added via bp_core_new_subnav_item() and protected with user_has_acces (like the friend request notification email), the item does not strictly speaking exist when the user is not logged in, so BP returns a 'page not found' message.

When an item is protected via a check inside of the function itself, it simply fails to load the template, or sometimes even returns false, which results in a redirect back to the home page.

The problem is actually quite serious from a usability point of view, but a real solution lies beyond the scope of BP 1.3. I'm hoping for something quite robust in BP 1.4.

[rocketpoprich]

Any temporary fix for this bug?

[rocketpoprich]

Ok so here is my work around. This fixes my particular grief with this defect, but is not particularly kosher (modifying core plugin files, be careful with your updates) so your mileage may vary.

In /wp-content/plugins/buddypress/bp-friends/bp-friends-notifications.php edit the variable $all_requests_link on line 14. This is the link sent in your friend requests e-mail that breaks if you are not logged in.

What you will attempt to do now is change this variable to include /wp-login.php?redirect_to=the_link_you_want_to_login_redirect to. reauth=1 is added in case the person is logged in, they will be redirected to the correct place also and not bypassed.

In the end your line 14 variable $all_requests_link should read:

$all_requests_link = "http://www.yourdomain.com/wp-login.php?reauth=1&redirect_to=".bp_core_get_user_domain( $friend_id ) . BP_FRIENDS_SLUG . '/requests/';

Please change yourdomain.com in this variable to your actual domain.

Good luck and lets hope this defect gets fixed the right way, soon.

[acustica]

Let me preface this by saying that after writing what's below, I took a look at the current version of bp-core-catchuri.php in version control (in the trunk, as of r4172) and it has been totally rewritten (and the bp_core_catch_no_access() function I mention below no longer exists). So perhaps this is just a waste of time. But I don't actually understand what version of BP the trunk represents and it's relationship to BP public releases, so in case this could be fixed before the next major version, I'll still post my notes and patch (sorry if it is now irrelevant):

This may require serious in-depth development and be beyond the scope of BP 1.3, as boonebgorges, but I am not yet convinced. The problem, at least in my case, lies in this one line:

// If the template file doesn't exist, redirect to the root domain.
if ( !bp_is_blog_page() && !file_exists( apply_filters( 'bp_located_template', locate_template( $filtered_templates, false ), $filtered_templates ) ) ) 
	bp_core_redirect( $bp->root_domain );

in the bp_core_catch_no_access() function in bp-core/bp-core-catchuri.php. To elaborate on this ticket's description, the point seems to be to ensure that BP actually has a real template file to use to handle the current request. However, in many cases, what it is really checking is whether a BP component has to have already called bp_core_load_template(), which never happens if the component decides the current user doesn't have permission to access it.

It seems to me that these problems can be remedied by just changing the order of the checks that happen in the bp_core_catch_no_access() function. If the above check comes after this check, which handles situations where $bp_path is not set like I described above:

if ( !$bp_path && !bp_is_blog_page() ) {
	if ( is_user_logged_in() ) {
		wp_redirect( $bp->root_domain );
	} else {
		wp_redirect( site_url( 'wp-login.php?redirect_to=' . site_url() . $_SERVER['REQUEST_URI'] ) );
	}
}

then it would actually make sense. As it is, the above check, which routes a non-logged in user to a login page with the proper redirect_to set, is pointless, because if !$bp_path, then obviously the file_exists() call will return false.

That's my interpretation, so please correct me if I'm wrong (and if it's still relevant). I've attached a patch implementing my proposed fix based on the r3233 version of this file.

And lastly, on my site, I've just added a 'bp_located_template' filter to bypass the file_exists() check and it currently works like a charm (hoping there aren't any unforeseen consequences), but it's obviously less than ideal to remove the check to confirm the template file exists. To see the code and description, please see my post here: http://buddypress.org/community/groups/how-to-and-troubleshooting/forum/topic/friend-request-wp-login-redirects/#post-98565

[r-a-y]

I'm about to propose a similar fix for v1.3 with a couple of tweaks, but the main part for v1.2 is I just removed:

	// Add .php to all options in $bp_path
	foreach( (array) $bp_path as $template )
		$filtered_templates[] = "$template.php";

	// If the template file doesn't exist, redirect to the root domain.
	if ( !bp_is_blog_page() && !file_exists( apply_filters( 'bp_located_template', locate_template( $filtered_templates, false ), $filtered_templates ) ) ) 
		bp_core_redirect( $bp->root_domain );

I believe this isn't necessary because the $bp_path global forms the $filtered_templates array. Need some feedback here from a core dev.

[boonebgorges]

I know that r-a-y has close to a working patch for this, so I'm going to bump it back to the 1.3 milestone so it doesn't get lost in the pile.

[DJPaul]

r-a-y, are you creating a patch for this for trunk? Happy to test it.

[r-a-y]

I'm going to create a new ticket for v1.3, since this ticket specifically refers to the 1.2-branch and I want to avoid confusion for others who might come across this ticket.

[acustica]

I’m attaching a new version of the patch that also fixes the line that redirects to the login page. It wasn’t URL-escaping the current request before using it as the redirect_to variable. So, I swapped out site_url( ... ) with wp_login_url( ... ), which automatically takes care of all that.

[DJPaul]

Fixed in r4465 (#3246)