Use bp_core_no_access() with group admin panels instead of 404

[boonebgorges]

Right now, if an unauthorized user (non-admin, or non-logged-in) tries to access a group admin panel (example.com/groups/group-name/admin/members, etc), it 404s. It should call up bp_core_no_access(). Note that the fix will probably have to be applied both to the core tabs and to BP_Group_Extension tabs.

Should be a fairly easy fix, so I'm putting it under 1.6. Is puntable though.

[sboisvert]

From my understanding of the code, this is because of:

if ( $this->visibility == 'public' || ( $this->visibility != 'public' && $bp->groups->current_group->user_has_access ) ) {
	if ( $this->enable_nav_item ) {
		bp_core_new_subnav_item( array( 'name' => ( !$this->nav_item_name ) ? $this->name : $this->nav_item_name, 'slug' => $this->slug, 'parent_slug' => $bp->groups->current_group->slug, 'parent_url' => bp_get_group_permalink( $bp->groups->current_group ), 'position' => $this->nav_item_position, 'item_css_id' => 'nav-' . $this->slug, 'screen_function' => array( &$this, '_display_hook' ), 'user_has_access' => $this->enable_nav_item ) );

line 1231 of v1.5 of bp-groups-classes.php

The problem seems to be that the
add_action( 'bp_screens', $screen_function );
actually takes place in bp_core_new_subnav_item() and that is also where permissions are checked. the bp_core_new_subnav_item() does take a user_has_access parameter. The problem is that there doesn't seem to be an easy way for plugins to latch on to that.

I don't know if someone can go over if my analysis is correct.
The solution I would propose would be to add a new var to BP_Group_Extension that would be user_has_access (default to true)
and then plugins instead of removing the nav item when they don't want the user to have access would instead use $this->user_has_access = false;

Now maybe its simpler to change the behavior of $this->enable_nav_item in BP_Group_Extension->_register() so that it calls bp_core_new_subnav_item() everytime, with the enable_nav_item being the parameter to user_has_access. from my reading of the code in bp_core_new_subnav_item() this would mean that it still is not displayed and if the asked screen is the one to which the user does not have access bp_core_no_access() is already called.

This would be much simpler and would mean the only change required is to remove
if ( $this->enable_nav_item ) {
from BP_Group_Extension->_register(), Sadly I don't know enough about buddypress to fully understand the repercussions on what I'm suggesting and therefore I might be making terrible suggestions. Hopefully this will help someone fix the problem.

[r-a-y]

This works for me.
Tested on 1.6-bleeding.

Boone, can you confirm?

[boonebgorges]

Ray, can you define "this"?

[sboisvert]

as a side note, i've been running 1.5.1 with the above lines reversed as I alluded to

bp_core_new_subnav_item( array( 'name' => ( !$this->nav_item_name ) ? $this->name : $this->nav_item_name, 'slug' => $this->slug, 'parent_slug' => $bp->groups->current_group->slug, 'parent_url' => bp_get_group_permalink( $bp->groups->current_group ), 'position' => $this->nav_item_position, 'item_css_id' => 'nav-' . $this->slug, 'screen_function' => array( &$this, '_display_hook' ), 'user_has_access' => $this->enable_nav_item ) );
if ( $this->enable_nav_item ) {

And it has the desired effect.
Ray is that what you are referring to?

[r-a-y]

Sorry for being extremely vague, Boone!

I mean that navigating to a group admin page while not being logged-in correctly prompts me to login.

sboisvert, I haven't looked at your code yet.

[boonebgorges]

I've confirmed sboisvert's original issue.

It's going to be tricky to fix it by reordering bp_core_new_subnav_item() and the like. Instead, since the problem is localized to admin pages only, I'm going to put a fix higher up the chain, near where we do access control for hidden and private groups in the group component loader.

[boonebgorges]

(In [5462]) When a non-admin of a group attempts to visit a group admin tab, redirect with bp_core_no_access() rather than 404. Fixes #3694