Summary
Follow-up to PR #1431 (the @self.folder access-check work). Wilco's 4th-pass review noted that while the API surface for ?IUser $currentUser is correctly plumbed end-to-end (saveObject → ensureObjectFolder → FileService::assertObjectFolderAccessible), non-HTTP callers of saveObject don't actually pass an explicit user. They fall through to IUserSession::getUser() → null on CLI / event-listener / background-job paths → default-deny on any object that carries a numeric @self.folder binding.
PR #1431 wired the TransitionEngine site in a follow-up commit ($this->userSession->getUser() was already available there — a trivial 1-line forward). The remaining sites need design decisions (where does the acting user come from for CLI / cron / settings re-saves?) and are tracked here.
Impact
Scoped to objects bound via @self.folder (numeric _folder values on the ObjectEntity). Legacy objects without numeric bindings are unaffected — assertObjectFolderAccessible no-ops for null / '' / non-numeric values. So the blast radius scales with adoption of the @self.folder feature.
For an environment that adopts @self.folder and then runs any of the affected code paths, the bound objects silently skip with a generic "failed" count in the command output rather than a clear "access denied" surface.
This is a functional regression rather than a security regression — default-deny is the correct fail mode. But operators have no good signal that the denial is the cause.
Affected caller sites
Each line is one save-path that needs the acting user threaded through.
| File |
Line(s) |
Caller context |
Suggested resolution |
lib/Command/RematerialiseCalculationsCommand.php |
189 |
OCC CLI |
Add --user <uid> flag; resolve via IUserManager; system-user fallback when omitted |
lib/Service/Configuration/ImportHandler.php |
1911, 1941 |
Import flow (CLI / cron) |
Plumb the import job's --user flag (when invoked from CLI) or the requesting user (when invoked via the import endpoint) |
lib/Service/SettingsService.php |
1242, 1460 |
App-settings re-save loops (CLI) |
Resolve a system user; settings re-saves are admin-scoped operations |
| Mapper adapter / MCP / GraphQL surfaces |
various |
Various |
Audit each; forward the requesting user where available |
lib/Service/Lifecycle/TransitionEngine.php (line 155) was wired in PR #1431 itself — $this->userSession->getUser() was already injected, so the fix was a 1-line forward.
Design notes
Three patterns to standardise across the codebase:
- Resolve the acting user from a
--user CLI flag. Use IUserManager::get($uid) and pass the result. When the flag is absent, fall back to (2) or (3).
- System-user pattern. When a CLI command runs as part of system maintenance (settings re-save, lifecycle housekeeping), resolve a designated system user via a known UID convention (e.g.
system or the admin user the app was installed by). Document the chosen convention.
- Pass-through from the original requester. When an event listener / background job runs in response to a user-initiated action, forward the user that triggered the original request. This requires the dispatching code to carry the user through the event payload — sometimes a small refactor.
Pick one pattern per caller site, with the choice documented inline.
Acceptance
References
Summary
Follow-up to PR #1431 (the
@self.folderaccess-check work). Wilco's 4th-pass review noted that while the API surface for?IUser $currentUseris correctly plumbed end-to-end (saveObject→ensureObjectFolder→FileService::assertObjectFolderAccessible), non-HTTP callers ofsaveObjectdon't actually pass an explicit user. They fall through toIUserSession::getUser()→ null on CLI / event-listener / background-job paths → default-deny on any object that carries a numeric@self.folderbinding.PR #1431 wired the TransitionEngine site in a follow-up commit (
$this->userSession->getUser()was already available there — a trivial 1-line forward). The remaining sites need design decisions (where does the acting user come from for CLI / cron / settings re-saves?) and are tracked here.Impact
Scoped to objects bound via
@self.folder(numeric_foldervalues on the ObjectEntity). Legacy objects without numeric bindings are unaffected —assertObjectFolderAccessibleno-ops fornull/''/ non-numeric values. So the blast radius scales with adoption of the@self.folderfeature.For an environment that adopts
@self.folderand then runs any of the affected code paths, the bound objects silently skip with a generic "failed" count in the command output rather than a clear "access denied" surface.This is a functional regression rather than a security regression — default-deny is the correct fail mode. But operators have no good signal that the denial is the cause.
Affected caller sites
Each line is one save-path that needs the acting user threaded through.
lib/Command/RematerialiseCalculationsCommand.php--user <uid>flag; resolve viaIUserManager; system-user fallback when omittedlib/Service/Configuration/ImportHandler.php--userflag (when invoked from CLI) or the requesting user (when invoked via the import endpoint)lib/Service/SettingsService.phplib/Service/Lifecycle/TransitionEngine.php(line 155) was wired in PR #1431 itself —$this->userSession->getUser()was already injected, so the fix was a 1-line forward.Design notes
Three patterns to standardise across the codebase:
--userCLI flag. UseIUserManager::get($uid)and pass the result. When the flag is absent, fall back to (2) or (3).systemor the admin user the app was installed by). Document the chosen convention.Pick one pattern per caller site, with the choice documented inline.
Acceptance
RematerialiseCalculationsCommandaccepts--user <uid>and resolves to anIUser.ImportHandlerforwards the requesting user (when called from the HTTP path) or the--userflag (when called from CLI).SettingsServiceresolves a system user and passes it through.@self.folder-bound objects in CLI flows silently default-deny.References
validate-self-folder-accesscapability — adds the access check and the API surface this issue wires up.FileService::assertObjectFolderAccessible(the proxy with?IUser $currentUser=null)ObjectService::saveObject(the public surface with the?IUser $currentUser=nullparameter)