nextcloud / nextcloud/tables

Activity email notifications fail in cron: "Cannot pre check the user id" — TablesProvider calls canAccessNodeById() without userId

Open Beginner friendly
#2,871 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

0. Needs triage bug
Dominant language
JavaScript
Stars
214
Forks
52
Avg merge
1d 10h
Merged PRs (30d)
82

Description

Steps to reproduce

1.Nextcloud 34.0.2.1, Tables 2.2.1, Activity + Notifications apps enabled, email notifications configured.
2. Share a table with users/groups; recipients have "email" enabled for Tables activity events.
3. Another user updates rows in the shared table.
4. Wait for the SendNotificationMails background job (cron.php).

Expected behavior

Recipients get activity notification emails for the row updates.

Actual behavior

No emails are sent for Tables events. Every cron run logs, once per affected
notification:

{"level":2,"app":"tables","message":"Cannot pre check the user id",
 "scriptName":"/var/www/html/nextcloud/cron.php","version":"34.0.2.1"}

followed by a consequential error from Activity:

{"level":3,"app":"activity","message":"Value provided for richSubject is not valid"}

Partial trace of the first error:

lib/Service/PermissionsService.php:723 (preCheckUserId)
#0 lib/Service/PermissionsService.php(771): PermissionsService->basisCheck()
#1 lib/Service/PermissionsService.php(700): PermissionsService->basisCheckById()
#2 lib/Service/PermissionsService.php(299): PermissionsService->checkPermissionById()
#3 lib/Service/PermissionsService.php(140): PermissionsService->canReadColumnsByTableId()
#4 lib/Activity/TablesProvider.php(183):  PermissionsService->canAccessNodeById()
#5 apps/activity/lib/NotificationGenerator.php(82):  TablesProvider->parse()
#6 apps/activity/lib/NotificationGenerator.php(118): NotificationGenerator->populateEvent()
#7 lib/private/Notification/Manager.php(351):        NotificationGenerator->prepare()
#8 apps/notifications/lib/MailNotifications.php(149): Manager->prepare()
#9 apps/notifications/lib/MailNotifications.php(119): MailNotifications->sendEmailToUser()
#10 apps/notifications/lib/BackgroundJob/SendNotificationMails.php
Root cause analysis

In lib/Activity/TablesProvider.php::parse() the permission checks are called
without a user id:

if (isset($subjectParameters['table']['id'])
    && !$this->permissionsService->canAccessNodeById(Application::NODE_TYPE_TABLE, (int)$subjectParameters['table']['id'])) {

canAccessNodeById(int $nodeType, int $nodeId, ?string $userId = null) then
falls back to the session user, but in cron/background jobs there is no user
session, so PermissionsService::preCheckUserId() throws
"Cannot pre check the user id". parse() aborts, which also produces the
follow-up "richSubject is not valid" error in the Activity app, and the whole
mail notification batch for the recipient fails.

In the web UI everything works, because a session user exists.

Suggested fix

Pass the notification recipient explicitly — it is available as
$event->getAffectedUser() and is the semantically correct user to check
access for:

$affectedUser = $event->getAffectedUser() !== '' ? $event->getAffectedUser() : null;

if (isset($subjectParameters['table']['id'])
    && !$this->permissionsService->canAccessNodeById(Application::NODE_TYPE_TABLE, (int)$subjectParameters['table']['id'], $affectedUser)) {
    throw new UnknownActivityException();
}
if (isset($subjectParameters['view']['id'])
    && !$this->permissionsService->canAccessNodeById(Application::NODE_TYPE_VIEW, (int)$subjectParameters['view']['id'], $affectedUser)) {
    throw new UnknownActivityException();
}

Optionally, wrapping the checks in try/catch and rethrowing
UnknownActivityException would prevent a single problematic event from
breaking the entire mail generation run.

We applied this patch locally on 34.0.2.1 / Tables 2.2.1 and the errors are
gone; emails are generated again.

Tables app version

2.2.1

Browser

Safari

Client operating system

Mac os x Tahoe

Operating system

Ubuntu Server 24

Web server

Apache

PHP engine version

PHP 8.3

Database

MariaDB

Additional info

No response

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in lib/Activity/TablesProvider.php::parse() and trace the canAccessNodeById() calls into lib/Service/PermissionsService.php, especially preCheckUserId(). Use the affected notification recipient for background-job permission checks, then verify that cron-generated Tables activity notifications no longer log the user-id or richSubject errors and that recipients receive emails.

Written by the indexing model from the issue text.

Assessment

Tech stack
php
Domain
authorization, backend
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.