Skip to:
Content

BuddyPress.org

Opened 17 months ago

Closed 17 months ago

Last modified 11 months ago

#7659 closed defect (bug) (fixed)

Subnav menu with 'user_has_access' set to false should redirect but doesn't

Reported by: r-a-y Owned by:
Milestone: 3.0 Priority: normal
Severity: normal Version: 2.6.0
Component: Navigation Keywords: has-patch commit
Cc:

Description

In #6534, we refactored the way we register menu items with the BP Navigation API.

Unfortunately, we broke functionality to redirect subnav menu items with user_has_access set to false.

Example code:

<?php
add_action( 'bp_setup_nav', function() {
        bp_core_new_nav_item( array(
                'name'                    => 'Secret Stash',
                'slug'                    => 'secret-stash',
                'screen_function'         => 'my_secret_stash_screen',
                'default_subnav_slug'     => 'my-stash',
                'show_for_displayed_user' => false,
        ) );

        bp_core_new_subnav_item( array(
                'name'            => 'My Stash',
                'slug'            => 'my-stash',
                'parent_url'      => trailingslashit( bp_displayed_user_domain() . 'secret-stash' ),
                'parent_slug'     => 'secret-stash',
                'user_has_access' => bp_is_my_profile(),
                'no_access_url'   => is_user_logged_in() ? bp_loggedin_user_domain() : '',
                'screen_function' => 'my_secret_stash_screen'
        ) );
} );

function my_secret_stash_screen() {
        add_action( 'bp_template_content', function() {
                echo "Ssh! It's a secret!";
        } ) ;
        bp_core_load_template( 'members/single/plugins' );
}

Expected behavior:
If you are logged in and you attempt to view another person's stash (example.com/members/ANOTHER-USER/secret-stash/), you should be redirected back to your profile page with an error message.

If you are logged out and you attempt to view the same URL, you should get redirected to login. On success, you can view your stash. If you login with another account, you get redirected back to your profile page with an error message.

Current behavior:
You get a 404 page when you view that URL.


The problem:

In #6534, we missed a portion of the refactor in bp_core_register_subnav_screen_function().

$parent_nav is an array that needs to use the reset() function so we grab the actual nav object.

See other areas in bp-core-buddybar.php that follows the same pattern:
https://buddypress.trac.wordpress.org/browser/tags/2.9.2/src/bp-core/bp-core-buddybar.php?marks=308-315,506-514,683-686#L308

Attached patch fixes this up.

Patch also fixes the redirect message to show up on frontend pages (the mode portion of the patch) and alters the strings to better reflect what is happening (that instead of this).

Attachments (1)

7659.01.patch (1.8 KB) - added by r-a-y 17 months ago.

Download all attachments as: .zip

Change History (9)

@r-a-y
17 months ago

#1 @r-a-y
17 months ago

  • Component changed from Core to Navigation
  • Keywords has-patch added

#2 @DJPaul
17 months ago

  • Keywords commit added

Sure, you've done more work here than I have, so if this the way to go, let's do it.

#3 @r-a-y
17 months ago

#7552 was marked as a duplicate.

Going to commit this now.

#4 @r-a-y
17 months ago

In 11814:

Navigation: Restricted subnav menu pages should redirect instead of throwing a 404.

Back in BuddyPress 2.6.0, we refactored the way we register menu items with
the BP Navigation API (see #6534).

The problem is we missed an instance during the refactor. This caused
restricted subnav pages (those set with the 'user_has_access' parameter
to false in bp_core_new_subnav_item()) to 404 instead of being redirected
away.

Commit fixes this problem.

See #7659.

#5 @r-a-y
17 months ago

In 11815:

Navigation: Show redirect message on restricted subnav menu pages.

Commit also alters the message strings to better reflect what is happening
("that page" instead of "this page").

See #7659.

#6 @r-a-y
17 months ago

  • Resolution set to fixed
  • Status changed from new to closed

#7 @boonebgorges
13 months ago

[11814] broke compatibilty in cases where a plugin registered a subnav item *before* its corresponding parent. In most cases this doesn't make much sense, but in this specific case - where the plugin was using bp_core_new_subnav_item() to add a *group* subnav item - it's a little less silly. Anyway, the plugin was doing something like this:

add_action( 'bp_setup_nav', function() {
    if ( ! bp_is_group() )
        return;

    bp_core_new_subnav_item( array(
        'parent_slug' => $current_group->slug, // etc
    ) );
} );

and because of WP's load order rules, this plugin happened to load before the Groups component was fully set up.

The solution for this plugin is pretty simple: move to a later priority, after core components have had a chance to set up. So I'm not sure this is a bug worth worrying about, which is why I'm reporting it here rather than in a new ticket. But I wanted to record it for posterity.

#8 @r-a-y
11 months ago

[11814] broke compatibilty in cases where a plugin registered a subnav item *before* its corresponding parent.

Good call! This issue was also raised in #7931. There's a patch in that ticket.

Can you test your previous plugin with that patch to see if it will work, @boonebgorges ?

Note: See TracTickets for help on using tickets.