Statuses in simplenews issue table are not changed on cron run for any nodes that the anonymous user cannot access

CommentFileSizeAuthor
#8 simplenews.cron-access.2892623-8.patch449 bytesadamps
#2 simplenews-status_change_on_cron-2892623-2.patch804 bytesRaSpirit

Comments

RaSpirit created an issue. See original summary.

RaSpirit’s picture

RaSpirit commented
StatusFileSize
new simplenews-status_change_on_cron-2892623-2.patch804 bytes

The issue is in entity query, as cron runs in context of anonymous user, so possible fix is attached in patch.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Needs work

That would only make a difference if you actually have query alters doing something but then this could happen, yes.

However, this should use \Drupal\Core\Entity\Query\QueryInterface::accessCheck() then.

VitaliyDemchuk’s picture

VitaliyDemchuk commented

Working without patch

adamps’s picture

adamps commented
Priority: Major » Normal
Status: Needs work » Closed (works as designed)
Issue tags: -Entity query, -cron problem

Issue summary does not clearly describe a sequence of events

Patch implies that QueryInterface will restrict the result based on permissions of the current user. Is there any evidence to support this? Closing based on #4.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Closed (works as designed) » Active

Yes, entity queries respect node access by default, there's a way to opt out of that, which would be the better solution for this.

adamps’s picture

adamps commented
Status: Active » Needs work
Issue tags: +Needs tests, +Needs issue summary update

Thanks @Berdir. So looks like this needs work:

  • Issue summary does not clearly describe a sequence of events
  • Missing tests
  • Comment from #2/#6 has not been applied.
adamps’s picture

adamps commented
Version: 8.x-1.0-alpha3 » 3.x-dev
Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -Needs tests, -Needs issue summary update
StatusFileSize
new simplenews.cron-access.2892623-8.patch449 bytes
adamps’s picture

adamps commented
Title: \Drupal\simplenews\Mail\Mailer::updateSendStatus does not change status on cron » Incorrect send status for nodes with restricted access
Status: Needs review » Fixed

  • AdamPS committed a2f7e7f on 3.x 
    Issue #2892623 by AdamPS, RaSpirit: Incorrect send status for nodes with...

Status: Fixed » Closed (fixed)

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