Problem/Motivation / Steps to reproduce

-We are using the groups module.
-We have nodes that reference groups.
-We don't ever want users to be able to navigate to and see a group entity, so anonymous users do not have the permission to view groups.
-We have a SOLR based view, with a facet for "organization" that allows filtering what organizations (groups) a node has been associated with via a field on the node.
-View shows all nodes, no access checks, just "is the node published" filters.
-Anonymous users see no facet values, as they lack permissions to view the groups.

For may use cases... it may potentially be pointless to perform access checks at this point for facets, and thus not terribly performant. If the referenced entities used as facets are still not visible, but can be used to trim down results... who cares that the referenced entities themselves cannot be viewed?

Another bit of motivation for this is that on the site we're working on we really don't care that much about permissions checks in many many locations. If it's published... just show it. We can skip permissions on many views all together... and we do... but facets does not have the ability it seems to take the same liberty.

Proposed resolution

In facets/src/Plugin/facets/processor/TranslateEntityProcessor.php Make $this->checkEntitiesAccess($entities, $facet, $access); Optional via config form on the facet processor.

Issue fork facets-3561530

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

jnicola created an issue. See original summary.

jnicola’s picture

Issue summary: View changes
jnicola’s picture

Issue summary: View changes
Status: Active » Needs work
Issue tags: +Needs tests

I've got something in place that...

  • Adds a facet config option on "TranslateEntityProcessor" to bypass entity access checks
  • Is disabled by default so no functionality will change for anyone.
  • When enabled skips calling $this->checkEntitiesAccess($entities, $facet, $access); which iterates over the entities array (which is passed by reference) and removes entities without access.

Pretty darn straightforward.

Needs tests, which I'll focus on as soon as I get this all in place for my day job.

jnicola’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests

Okay, I added two tests.

One for ensuring entities are filtered out based on access checking (aka: confirm and default to original behavior)
One for ensuring entities are not filtered out when the choice to bypass is present (aka: Test new behavior)

I would content this not only adds functionality, but adds test coverage where there currently isn't any as there isn't any test coverage relating to access currently, despite the filtering based on access being in the code.

lamp5’s picture

We use reference field_language to the languages configured in our system, and there is no permission related to this to allow other roles than admins to display language entities, so our facets are empty for anonymous users. This patch works amazingly.

paweldxm’s picture

I confirm, the patch fixes the problem.

jnicola’s picture

Status: Needs review » Reviewed & tested by the community

Moving this to reviewed and tested by the community. We've got three different entities all confirming this works, one of which is the US DOJ so not exactly a light use case, especially lately. This also has tests, it doesn't get much more polished than this!

lamp5’s picture

1 thing. We should add missing schema for a new config option.

mkalkbrenner’s picture

Status: Reviewed & tested by the community » Needs work

TranslateEntityAggregatedFieldProcessor needs to be covered as well.

mkalkbrenner’s picture

I think that the logic should be implemented in function checkEntitiesAccess() itself within the ProcessorPluginBase.
The setting should become available for every facet, so custom facets will get the feature as well.

mkalkbrenner’s picture

Version: 3.0.2 » 3.0.x-dev
Status: Needs work » Needs review
mkalkbrenner’s picture

The remaining failing tests are unrelated.

mkalkbrenner’s picture

Status: Needs review » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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