Problem/Motivation

No way to display user created menus for anonymous users.

This is a followup for #1535868: Convert all blocks into plugins.

SystemMenuBlock has the following method:

  public function access(AccountInterface $account) {
    $derivative = $this->getDerivativeId();
    return ($account->isAuthenticated() || in_array($derivative, array('main', 'tools', 'footer')));
  }

Special-casing two particular menus that happen to be provided with the Standard profile is probably not best practice.

Proposed resolution

Get rid of this access check because menu links does this.

This issue should probably be resolved when block access is moved to access plugins for layouts; however, if that conversion does not happen, it should be fixed in another way.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

xjm’s picture

Issue tags: +Block plugins
xjm’s picture

Status: Postponed » Active
jibran’s picture

It should be fixed in another way

and what is that.

jibran’s picture

Issue summary: View changes

Removing myself from the author field so that I can unfollow. --xjm

andypost’s picture

Title: Fix access handling SystemMenuBlock::blockAccess() » Fix access handling SystemMenuBlock::access()
Priority: Normal » Major
Issue summary: View changes
Status: Active » Needs review
FileSize
916 bytes

This is a regression that user created menus only visible for authorized users only.

So better remove this ugly logic because menu link access system covers that

dawehner’s picture

It would be great to have some test coverage maybe for that?

andypost’s picture

Any idea for test? we have a lot tests that checks menu link access so I see no reason

Status: Needs review » Needs work

The last submitted patch, 4: 1874564-menu-block-access-4.patch, failed testing.

jibran’s picture

Status: Needs work » Needs review
jrockowitz’s picture

The patch works for me. It makes complete sense that a SystemMenuBlock::access() handler is not needed and the default block/menu link access controls is what should be used to limit access to a menu for anonymous users.

I am also unclear what tests needed to be created since this patch is removing a feature and relying the block modules access controls which I am assuming has test coverage.

benjy’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll

Needs a reroll

andypost’s picture

Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
777 bytes

Re-roll, I still see no reason for tests

benjy’s picture

Status: Needs review » Reviewed & tested by the community

I tested and this fixes the issue with anonymous users not been able to see user created menu's. Also the menu/menu link system does seem to have sufficient tests. Unless someone can suggest another test to add this should be RTBC.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Yeah, this looks fine. The only test I can think of would be specific to these particular blocks, but that doesn't make much sense.

Committed and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

xjm’s picture

Issue summary: View changes