Failed to export Submissions: Not allowed to write to file

[chriscroome]

Please use the 👍 reaction to show that you are affected by the same issue. Please don't comment if you have no relevant information to add!

Describe the bug

A Nextcloud instance that did have a working Forms app now generates an error on form submission.

To Reproduce Steps to reproduce the behaviour:

1. Go to the form
2. Complete it
3. Click "Submit"
4. The use has the error "There was an error submitting the form"

Expected behavior

A successfully submission.

Nextcloud (please complete the following information):

• Nextcloud-Version: 28.0.4.1
• Forms-Version: 4.2.0

Desktop (please complete the following information):

• OS: Debian
• Browser Firefox
• Version [e.g. 22]

Smartphone (please complete the following information):

• Device: [e.g. iPhone6]
• OS: [e.g. iOS8.1]
• Browser Firefox/
• Version 126.0

Browser log

[ERROR] forms: Error while submitting the form 
Object { app: "forms", level: 2, error: {…} }
NcSettingsSection-NSrCZ23G-6nKKlVz9.mjs:2:337806
    value https://cloud.example.org/apps/forms/js/NcSettingsSection-NSrCZ23G-6nKKlVz9.mjs:2
    value https://cloud.example.org/apps/forms/js/NcSettingsSection-NSrCZ23G-6nKKlVz9.mjs:2
    onConfirmedSubmit https://cloud.example.org/apps/forms/js/Submit-PS6lxYc-.mjs:2

Nextcloud log

The following is written to the Nextcloud log:

Failed to export Submissions: Not allowed to write to file

Server logs

There is nothing in the Apache or PHP-FPM logs.

TMPDIR

The tempdirectory is writable.

Additional context

The error that is produced appears to be here:

https://github.com/nextcloud/forms/blob/1ba3a1404c3a23eedd8010175b60f10c5a70f688/lib/Service/SubmissionService.php#L226

The change to the server (the forms were working prior to this change), was updating from Debian Bullseye to Debian Bookworm, the instance uses PHP-FPM 8.3.

I have tried to duplicate this on another server with the same configuration but haven't been able to.

[Chartman123]

Did you restart your php processes? Are you using php-fpm? We had lots of errors in the past after upgrades due to cached code...

[susnux]

Browser log

Could you please provide the full object from the log? Including the error message?

[chriscroome]

@Chartman123 PHP-FPM had been restarted.

@susnux here is a full log entry, all I have changed is the IP address and I also ran it through jq for formatting:

{
  "reqId": "oLRtH9XtF8rwtUiZidOT",
  "level": 2,
  "time": "2024-04-15T19:08:20+00:00",
  "remoteAddr": "X.X.X.X",
  "user": "--",
  "app": "forms",
  "method": "POST",
  "url": "/ocs/v2.php/apps/forms/api/v2.4/submission/insert",
  "message": "Failed to export Submissions: Not allowed to write to file",
  "userAgent": "Mozilla/5.0 (X11; Linux x86_64; rv:126.0) Gecko/20100101 Firefox/126.0",
  "version": "28.0.4.1",
  "data": {
    "app": "forms"
  }
}

[susnux]

Thank you, but @chriscroome what I meant is from you browser console there should be the network request that fails, could you please provide the response from the server? For me it works and then looks like this:

But for you there should be a more meaningful error message

[chriscroome]

This is what I have after exporting it and changing the domain name:

[ERROR] forms: Error while submitting the form 
Object { app: "forms", level: 2, error: {…} }
NcSettingsSection-NSrCZ23G-6nKKlVz9.mjs:2:337806
    value https://cloud.example.com/apps/forms/js/NcSettingsSection-NSrCZ23G-6nKKlVz9.mjs:2
    value https://cloud.example.com/apps/forms/js/NcSettingsSection-NSrCZ23G-6nKKlVz9.mjs:2
    onConfirmedSubmit https://cloud.example.com/apps/forms/js/Submit-PS6lxYc-.mjs:2

Is that what you need?

[chriscroome]

I have also found 500 errors in the Apache and PHP-FPM logs, this in from the PHP-FPM access log:

- -  15/Apr/2024:19:31:23 +0000 "POST /ocs/v2.php" 500 /home/cloud/sites/nextcloud/ocs/v2.php 616.068 10240 45.45%

And this is from the Apache error log:

X.X.X.X - - [15/Apr/2024:19:31:23 +0000] "POST /ocs/v2.php/apps/forms/api/v2.4/submission/insert HTTP/2.0" 500 855 "-" "Mozilla/5.0 (X11; Linux x86_64; rv:126.0) Gecko/20100101 Firefox/126.0"

[susnux]

I could reproduce this on the instance, but there is not much helpful information for further debugging. I guess some permission issues.

Where is the file located you want to export submissions to? A groupfolder? Local storage? Is the file or parent folder shared?

[chriscroome]

I didn't set the form up so I don't know the answers to these questions I'm afraid but I have asked our client if they know the answers.

I think I have found the csv file on the file system and there doesn't apper to be any issue with the permissions or ownership -- the file is owned by the same user that PHP-FPM runs as and is 0644, the directory is 0775 and also owned by the same user that PHP-FPM runs as.

[mariomorvan]

The issue is still present on the instance mentioned by chriscroome.

I was able to make a new test form that leads to the same error message "There was an error submitting the form" whenever a submission is attempted by a user (except the owner of the form, for whom there is not such message). The error comes up after a result spreadsheet is saved in a folder shared with the form owner (with edit rights).

If no spreadsheet is created, or if it is created in the form owner's personal space, no such issue was noticed.

[Chartman123]

@mariomorvan thanks for sharing this information with us. Are those "normal" shared folders or group folders or something else?

[mariomorvan]

@mariomorvan thanks for sharing this information with us. Are those "normal" shared folders or group folders or something else?

@Chartman123 thanks for looking into this. Normal share as far as I understand the different kind of shares (we do not have installed group folders)

[mariomorvan]

the logs show the following message associated with today's submission errors:

Exception OC\Files\View::basicOperation(): Argument #2 ($path) must be of type string, null given, called in /home/cloud/sites/nextcloud/lib/private/Files/View.php on line 528 in file '/home/cloud/sites/nextcloud/lib/private/Files/View.php' line 1128
Exception thrown: Exception

[Chartman123]

@mariomorvan @susnux I could reproduce it this way:

• User A shares folder with user B with full permissions
• User B creates a form and links a spreadsheet in the shared folder
• User B fills the form: everything works fine
• If shared to all users of the instance: submission by logged in users works
• Someone fills and submits the form via public share link (not logged in): submission fails

What's strange to me is that I don't get the original warning/error message but an error 500. Perhaps something else changed in the meantime. Heres the server logfile attached that I could see:

Logfile

``` { "reqId": "xavjYSv75SOGCm3N4niG", "level": 3, "time": "2024-05-15T08:43:50+00:00", "remoteAddr": "62.225.12.220", "user": "--", "app": "no app in context", "method": "POST", "url": "/ocs/v2.php/apps/forms/api/v2.4/submission/insert", "message": "OC\\Files\\View::basicOperation(): Argument #2 ($path) must be of type string, null given, called in /var/www/html/lib/private/Files/View.php on line 530 in file '/var/www/html/lib/private/Files/View.php' line 1132", "userAgent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:109.0) Gecko/20100101 Firefox/115.0", "version": "29.0.0.19", "exception": { "Exception": "Exception", "Message": "OC\\Files\\View::basicOperation(): Argument #2 ($path) must be of type string, null given, called in /var/www/html/lib/private/Files/View.php on line 530 in file '/var/www/html/lib/private/Files/View.php' line 1132", "Code": 0, "Trace": [ { "file": "/var/www/html/lib/private/AppFramework/App.php", "line": 184, "function": "dispatch", "class": "OC\\AppFramework\\Http\\Dispatcher", "type": "->", "args": [ [ "OCA\\Forms\\Controller\\ApiController" ], "insertSubmission" ] }, { "file": "/var/www/html/lib/private/Route/Router.php", "line": 338, "function": "main", "class": "OC\\AppFramework\\App", "type": "::", "args": [ "OCA\\Forms\\Controller\\ApiController", "insertSubmission", [ "OC\\AppFramework\\DependencyInjection\\DIContainer" ], [ "v2.4", "ocs.forms.api.insertsubmission" ] ] }, { "file": "/var/www/html/ocs/v1.php", "line": 66, "function": "match", "class": "OC\\Route\\Router", "type": "->", "args": [ "/ocsapp/apps/forms/api/v2.4/submission/insert" ] }, { "file": "/var/www/html/ocs/v2.php", "line": 23, "args": [ "/var/www/html/ocs/v1.php" ], "function": "require_once" } ], "File": "/var/www/html/lib/private/AppFramework/Http/Dispatcher.php", "Line": 170, "Previous": { "Exception": "TypeError", "Message": "OC\\Files\\View::basicOperation(): Argument #2 ($path) must be of type string, null given, called in /var/www/html/lib/private/Files/View.php on line 530", "Code": 0, "Trace": [ { "file": "/var/www/html/lib/private/Files/View.php", "line": 530, "function": "basicOperation", "class": "OC\\Files\\View", "type": "->", "args": [ "file_exists", null ] }, { "file": "/var/www/html/lib/private/Files/Filesystem.php", "line": 546, "function": "file_exists", "class": "OC\\Files\\View", "type": "->", "args": [ null ] }, { "file": "/var/www/html/apps/files_versions/lib/Storage.php", "line": 190, "function": "file_exists", "class": "OC\\Files\\Filesystem", "type": "::", "args": [ null ] }, { "file": "/var/www/html/apps/files_versions/lib/Listener/FileEventsListener.php", "line": 197, "function": "store", "class": "OCA\\Files_Versions\\Storage", "type": "::", "args": [ null ] }, { "file": "/var/www/html/apps/files_versions/lib/Listener/FileEventsListener.php", "line": 103, "function": "write_hook", "class": "OCA\\Files_Versions\\Listener\\FileEventsListener", "type": "->", "args": [ [ "OC\\Files\\Node\\File" ] ] }, { "file": "/var/www/html/lib/private/EventDispatcher/ServiceEventListener.php", "line": 86, "function": "handle", "class": "OCA\\Files_Versions\\Listener\\FileEventsListener", "type": "->", "args": [ [ "OCP\\Files\\Events\\Node\\BeforeNodeWrittenEvent" ] ] }, { "file": "/var/www/html/3rdparty/symfony/event-dispatcher/EventDispatcher.php", "line": 230, "function": "__invoke", "class": "OC\\EventDispatcher\\ServiceEventListener", "type": "->", "args": [ [ "OCP\\Files\\Events\\Node\\BeforeNodeWrittenEvent" ], "OCP\\Files\\Events\\Node\\BeforeNodeWrittenEvent", [ "Symfony\\Component\\EventDispatcher\\EventDispatcher" ] ] }, { "file": "/var/www/html/3rdparty/symfony/event-dispatcher/EventDispatcher.php", "line": 59, "function": "callListeners", "class": "Symfony\\Component\\EventDispatcher\\EventDispatcher", "type": "->", "args": [ [ [ "Closure" ], [ "Closure" ] ], "OCP\\Files\\Events\\Node\\BeforeNodeWrittenEvent", [ "OCP\\Files\\Events\\Node\\BeforeNodeWrittenEvent" ] ] }, { "file": "/var/www/html/lib/private/EventDispatcher/EventDispatcher.php", "line": 86, "function": "dispatch", "class": "Symfony\\Component\\EventDispatcher\\EventDispatcher", "type": "->", "args": [ [ "OCP\\Files\\Events\\Node\\BeforeNodeWrittenEvent" ], "OCP\\Files\\Events\\Node\\BeforeNodeWrittenEvent" ] }, { "file": "/var/www/html/lib/private/EventDispatcher/EventDispatcher.php", "line": 98, "function": "dispatch", "class": "OC\\EventDispatcher\\EventDispatcher", "type": "->", "args": [ "OCP\\Files\\Events\\Node\\BeforeNodeWrittenEvent", [ "OCP\\Files\\Events\\Node\\BeforeNodeWrittenEvent" ] ] }, { "file": "/var/www/html/lib/private/Files/Node/HookConnector.php", "line": 93, "function": "dispatchTyped", "class": "OC\\EventDispatcher\\EventDispatcher", "type": "->", "args": [ [ "OCP\\Files\\Events\\Node\\BeforeNodeWrittenEvent" ] ] }, { "file": "/var/www/html/lib/private/legacy/OC_Hook.php", "line": 105, "function": "write", "class": "OC\\Files\\Node\\HookConnector", "type": "->", "args": [ [ true, "/Test_Forms/Test (Antworten).xlsx" ] ] }, { "file": "/var/www/html/lib/private/Files/View.php", "line": 1276, "function": "emit", "class": "OC_Hook", "type": "::", "args": [ "OC_Filesystem", "write", [ true, "/Test_Forms/Test (Antworten).xlsx" ] ] }, { "file": "/var/www/html/lib/private/Files/View.php", "line": 1148, "function": "runHooks", "class": "OC\\Files\\View", "type": "->", "args": [ [ "update", "write" ], "/Test_Forms/Test (Antworten).xlsx" ] }, { "file": "/var/www/html/lib/private/Files/View.php", "line": 682, "function": "basicOperation", "class": "OC\\Files\\View", "type": "->", "args": [ "file_put_contents", "/user.b/files/Test_Forms/Test (Antworten).xlsx", [ "update", "write" ], null ] }, { "file": "/var/www/html/lib/private/Files/Node/File.php", "line": 73, "function": "file_put_contents", "class": "OC\\Files\\View", "type": "->", "args": [ "/user.b/files/Test_Forms/Test (Antworten).xlsx", null ] }, { "file": "/var/www/html/custom_apps/forms/lib/Service/SubmissionService.php", "line": 224, "function": "putContent", "class": "OC\\Files\\Node\\File", "type": "->", "args": [ null ] }, { "file": "/var/www/html/custom_apps/forms/lib/Controller/ApiController.php", "line": 1145, "function": "writeFileToCloud", "class": "OCA\\Forms\\Service\\SubmissionService", "type": "->", "args": [ [ "OCA\\Forms\\Db\\Form", 42 ], "/Test_Forms/Test (Antworten).xlsx", "xlsx", "user.b" ] }, { "file": "/var/www/html/lib/private/AppFramework/Http/Dispatcher.php", "line": 232, "function": "insertSubmission", "class": "OCA\\Forms\\Controller\\ApiController", "type": "->", "args": [ 42, [ [ "b" ] ], "ZadmE4kBLEXE4gSxTdmceJAg" ] }, { "file": "/var/www/html/lib/private/AppFramework/Http/Dispatcher.php", "line": 138, "function": "executeController", "class": "OC\\AppFramework\\Http\\Dispatcher", "type": "->", "args": [ [ "OCA\\Forms\\Controller\\ApiController" ], "insertSubmission" ] }, { "file": "/var/www/html/lib/private/AppFramework/App.php", "line": 184, "function": "dispatch", "class": "OC\\AppFramework\\Http\\Dispatcher", "type": "->", "args": [ [ "OCA\\Forms\\Controller\\ApiController" ], "insertSubmission" ] }, { "file": "/var/www/html/lib/private/Route/Router.php", "line": 338, "function": "main", "class": "OC\\AppFramework\\App", "type": "::", "args": [ "OCA\\Forms\\Controller\\ApiController", "insertSubmission", [ "OC\\AppFramework\\DependencyInjection\\DIContainer" ], [ "v2.4", "ocs.forms.api.insertsubmission" ] ] }, { "file": "/var/www/html/ocs/v1.php", "line": 66, "function": "match", "class": "OC\\Route\\Router", "type": "->", "args": [ "/ocsapp/apps/forms/api/v2.4/submission/insert" ] }, { "file": "/var/www/html/ocs/v2.php", "line": 23, "args": [ "/var/www/html/ocs/v1.php" ], "function": "require_once" } ], "File": "/var/www/html/lib/private/Files/View.php", "Line": 1132 }, "message": "OC\\Files\\View::basicOperation(): Argument #2 ($path) must be of type string, null given, called in /var/www/html/lib/private/Files/View.php on line 530 in file '/var/www/html/lib/private/Files/View.php' line 1132", "exception": [], "CustomMessage": "OC\\Files\\View::basicOperation(): Argument #2 ($path) must be of type string, null given, called in /var/www/html/lib/private/Files/View.php on line 530 in file '/var/www/html/lib/private/Files/View.php' line 1132" }, "id": "664475f19288c" } ```

[chriscroome]

Strangely I received a from @Chartman123 saying:

@chriscroome Could you please try to reproduce the error with the latest NC release? I tried it with NC30 RC2 and 29.0.5 and (if I didn't mix up things) for me it worked now...

However it doesn't appear above :shrug:

@mariomorvan is the admin on the Nextcloud instance where this was an issue -- @mariomorvan I have set off a job to update your Nextcloud to version 29.0.5, when it has completed could you test it again to see if this issue is resolved?

[Chartman123]

Yeah, I deleted my comment again after I found that in my test scenario the file wasn't linked at all... so of course no error message. It still happens in the latest NC versions

[susnux]

I think this is the same error as this one: https://github.com/nextcloud/forms/issues/2165 We need to externalize this spreadsheet writing in a background job.

Should be doable for 4.3

[Chartman123]

@susnux to me this looks like some problem with the permissions. What would be the difference if we put it into a background job?

[susnux]

What would be the difference if we put it into a background job?

We run on system permissions not the entering user's

[nikkilocke]

We have this issue on Nextcloud v28.0.9 and v29. The spreadsheet is in a group folder to which the logged in user filling in the form does not have access. (Our cloud is set up so people only have access to shared folders - their personal quota is 0.) If the user is not logged in, it works fine. You need to attempt to write to the spreadsheet as the user who owns the form, even if it is being filled in by another user.

The attempt to submit is a POST to https:///ocs/v2.php/apps/forms/api/v2.4/submission/insert

which returns 500

<?xml version="1.0"?>
<ocs>
 <meta>
  <status>failure</status>
  <statuscode>500</statuscode>
  <message>Internal Server Error
</message>
 </meta>
 <data/>
</ocs>

The Nextcloud log entry says:

[no app in context] Error: Exception thrown: Exception
    POST /ocs/v2.php/apps/forms/api/v2.4/submission/insert
    from 46.102.211.217 by ddat at 13 Sept 2024, 12:28:06

{"reqId":"jiqkTnhkQekTNabpQKLe","level":3,"time":"2024-09-13T11:28:06+00:00","remoteAddr":"46.102.211.217","user":"ddat","app":"no app in context","method":"POST","url":"/ocs/v2.php/apps/forms/api/v2.4/submission/insert","message":"Exception thrown: Exception","userAgent":"Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:130.0) Gecko/20100101 Firefox/130.0","version":"28.0.7.4","exception":{"Exception":"Exception","Message":"OC\\Files\\View::basicOperation(): Argument #2 ($path) must be of type string, null given, called in /var/www/html/lib/private/Files/View.php on line 529 in file '/var/www/html/lib/private/Files/View.php' line 1134","Code":0,"Trace":[{"file":"/var/www/html/lib/private/AppFramework/App.php","line":184,"function":"dispatch","class":"OC\\AppFramework\\Http\\Dispatcher","type":"->","args":[["OCA\\Forms\\Controller\\ApiController"],"insertSubmission"]},{"file":"/var/www/html/lib/private/Route/Router.php","line":315,"function":"main","class":"OC\\AppFramework\\App","type":"::","args":["OCA\\Forms\\Controller\\ApiController","insertSubmission",["OC\\AppFramework\\DependencyInjection\\DIContainer"],["v2.4","ocs.forms.api.insertSubmission"]]},{"file":"/var/www/html/ocs/v1.php","line":65,"function":"match","class":"OC\\Route\\Router","type":"->","args":["/ocsapp/apps/forms/api/v2.4/submission/insert"]},{"file":"/var/www/html/ocs/v2.php","line":23,"args":["/var/www/html/ocs/v1.php"],"function":"require_once"}],"File":"/var/www/html/lib/private/AppFramework/Http/Dispatcher.php","Line":169,"Previous":{"Exception":"TypeError","Message":"OC\\Files\\View::basicOperation(): Argument #2 ($path) must be of type string, null given, called in /var/www/html/lib/private/Files/View.php on line 529","Code":0,"Trace":[{"file":"/var/www/html/lib/private/Files/View.php","line":529,"function":"basicOperation","class":"OC\\Files\\View","type":"->","args":["file_exists",null]},{"file":"/var/www/html/lib/private/Files/Filesystem.php","line":545,"function":"file_exists","class":"OC\\Files\\View","type":"->","args":[null]},{"file":"/var/www/html/apps/files_versions/lib/Storage.php","line":190,"function":"file_exists","class":"OC\\Files\\Filesystem","type":"::","args":[null]},{"file":"/var/www/html/apps/files_versions/lib/Listener/FileEventsListener.php","line":196,"function":"store","class":"OCA\\Files_Versions\\Storage","type":"::","args":[null]},{"file":"/var/www/html/apps/files_versions/lib/Listener/FileEventsListener.php","line":102,"function":"write_hook","class":"OCA\\Files_Versions\\Listener\\FileEventsListener","type":"->","args":[["OC\\Files\\Node\\File"]]},{"file":"/var/www/html/lib/private/EventDispatcher/ServiceEventListener.php","line":86,"function":"handle","class":"OCA\\Files_Versions\\Listener\\FileEventsListener","type":"->","args":[["OCP\\Files\\Events\\Node\\BeforeNodeWrittenEvent"]]},{"file":"/var/www/html/3rdparty/symfony/event-dispatcher/EventDispatcher.php","line":230,"function":"__invoke","class":"OC\\EventDispatcher\\ServiceEventListener","type":"->","args":[["OCP\\Files\\Events\\Node\\BeforeNodeWrittenEvent"],"OCP\\Files\\Events\\Node\\BeforeNodeWrittenEvent",["Symfony\\Component\\EventDispatcher\\EventDispatcher"]]},{"file":"/var/www/html/3rdparty/symfony/event-dispatcher/EventDispatcher.php","line":59,"function":"callListeners","class":"Symfony\\Component\\EventDispatcher\\EventDispatcher","type":"->","args":[[["Closure"],["Closure"]],"OCP\\Files\\Events\\Node\\BeforeNodeWrittenEvent",["OCP\\Files\\Events\\Node\\BeforeNodeWrittenEvent"]]},{"file":"/var/www/html/lib/private/EventDispatcher/EventDispatcher.php","line":94,"function":"dispatch","class":"Symfony\\Component\\EventDispatcher\\EventDispatcher","type":"->","args":[["OCP\\Files\\Events\\Node\\BeforeNodeWrittenEvent"],"OCP\\Files\\Events\\Node\\BeforeNodeWrittenEvent"]},{"file":"/var/www/html/lib/private/EventDispatcher/EventDispatcher.php","line":106,"function":"dispatch","class":"OC\\EventDispatcher\\EventDispatcher","type":"->","args":["OCP\\Files\\Events\\Node\\BeforeNodeWrittenEvent",["OCP\\Files\\Events\\Node\\BeforeNodeWrittenEvent"]]},{"file":"/var/www/html/lib/private/Files/Node/HookConnector.php","line":100,"function":"dispatchTyped","class":"OC\\EventDispatcher\\EventDispatcher","type":"->","args":[["OCP\\Files\\Events\\Node\\BeforeNodeWrittenEvent"]]},{"file":"/var/www/html/lib/private/legacy/OC_Hook.php","line":105,"function":"write","class":"OC\\Files\\Node\\HookConnector","type":"->","args":[[true,"/Digital (UK Tech)/Digital Discussions Applications Team/Teams Working Documents/Test (responses).xlsx"]]},{"file":"/var/www/html/lib/private/Files/View.php","line":1278,"function":"emit","class":"OC_Hook","type":"::","args":["OC_Filesystem","write",[true,"/Digital (UK Tech)/Digital Discussions Applications Team/Teams Working Documents/Test (responses).xlsx"]]},{"file":"/var/www/html/lib/private/Files/View.php","line":1150,"function":"runHooks","class":"OC\\Files\\View","type":"->","args":[["update","write"],"/Digital (UK Tech)/Digital Discussions Applications Team/Teams Working Documents/Test (responses).xlsx"]},{"file":"/var/www/html/lib/private/Files/View.php","line":681,"function":"basicOperation","class":"OC\\Files\\View","type":"->","args":["file_put_contents","/nikkilocke/files/Digital (UK Tech)/Digital Discussions Applications Team/Teams Working Documents/Test (responses).xlsx",["update","write"],null]},{"file":"/var/www/html/lib/private/Files/Node/File.php","line":73,"function":"file_put_contents","class":"OC\\Files\\View","type":"->","args":["/nikkilocke/files/Digital (UK Tech)/Digital Discussions Applications Team/Teams Working Documents/Test (responses).xlsx",null]},{"file":"/var/www/html/custom_apps/forms/lib/Service/SubmissionService.php","line":224,"function":"putContent","class":"OC\\Files\\Node\\File","type":"->","args":[null]},{"file":"/var/www/html/custom_apps/forms/lib/Controller/ApiController.php","line":1145,"function":"writeFileToCloud","class":"OCA\\Forms\\Service\\SubmissionService","type":"->","args":[["OCA\\Forms\\Db\\Form",111],"/Digital (UK Tech)/Digital Discussions Applications Team/Teams Working Documents/Test (responses).xlsx","xlsx","nikkilocke"]},{"file":"/var/www/html/lib/private/AppFramework/Http/Dispatcher.php","line":230,"function":"insertSubmission","class":"OCA\\Forms\\Controller\\ApiController","type":"->","args":[111,[["457"],["1186"]],""]},{"file":"/var/www/html/lib/private/AppFramework/Http/Dispatcher.php","line":137,"function":"executeController","class":"OC\\AppFramework\\Http\\Dispatcher","type":"->","args":[["OCA\\Forms\\Controller\\ApiController"],"insertSubmission"]},{"file":"/var/www/html/lib/private/AppFramework/App.php","line":184,"function":"dispatch","class":"OC\\AppFramework\\Http\\Dispatcher","type":"->","args":[["OCA\\Forms\\Controller\\ApiController"],"insertSubmission"]},{"file":"/var/www/html/lib/private/Route/Router.php","line":315,"function":"main","class":"OC\\AppFramework\\App","type":"::","args":["OCA\\Forms\\Controller\\ApiController","insertSubmission",["OC\\AppFramework\\DependencyInjection\\DIContainer"],["v2.4","ocs.forms.api.insertSubmission"]]},{"file":"/var/www/html/ocs/v1.php","line":65,"function":"match","class":"OC\\Route\\Router","type":"->","args":["/ocsapp/apps/forms/api/v2.4/submission/insert"]},{"file":"/var/www/html/ocs/v2.php","line":23,"args":["/var/www/html/ocs/v1.php"],"function":"require_once"}],"File":"/var/www/html/lib/private/Files/View.php","Line":1134},"CustomMessage":"Exception thrown: Exception"},"id":"66e424be7a69f"}

[toad]

forms-bug.permissions-hack.clean.diff.txt After further debugging, we have a workaround (patch), but I'm not 100% sure whether it is safe.

What is actually happening here is an interaction between several apps:

• The forms service accepts a form submission, and calls file_put_contents in Files (to update the linked spreadsheet)
• That calls basicOperation('file_put_contents', path) (with the real path, including the actual owner)
• That emits a BeforeNodeWrittenEvent, for hooks, such as the versioning app
• The versioning app receives the event
• FileEventsListener::store() calls file_exists() to check whether the file exists before writing a new version record
• That ultimately calls back into FileEventsListener::getPathForNode(), which returns null, because the owner of the file is the same as the logged in user (since the file is on a share), and does not have access to the file

However, FileEventListener is part of files_versions; it's not doing any meaningful security check here. In particular, if the user is logged out, it checks the path for a username, and uses that.

So if we patch FileEventListener to do the same thing when $user == $owner, it will correctly look up the filename against its real owner, and file_exists() returns true.

As I understand it, permissions checks have already been done by the time we get here; files_versions is just hooking into that process, creating version records, and presumably preventing old versions being deleted. And the forms app has already bypassed the permissions checks somehow; as discussed above, it'd be better to update the spreadsheet on a background thread, but right now it works (and implementing that is more work than I'm prepared to do for a workaround since you'll be doing it next year).

So my guess is that my patch is safe (though there are some fishy things around it). But I'm not very sure about this! If we can use this, it'd solve some immediate problems and we would not have to wait until next year.

Thoughts?

[Chartman123]

@toad thank you very much for digging into this... If you want you can open a pull request on the server repository (where files_versions is developed) and see what the maintainers think about it. :)

[toad]

I've created a pull request for a shorter version of the patch: https://github.com/nextcloud/server/pull/48295

Note that this is against server, so I'm not sure whether I need a server ticket?

I haven't implemented tests yet; if my assumptions about the security implications are wrong then the whole thing is the wrong approach.

[Chartman123]

@come-nc if the form is filled via the public share link like in this comment, the if branch is used here and ownerId has the owner of the Form, not the user providing the share.

[come-nc]

@come-nc if the form is filled via the public share link like in this comment, the if branch is used here and ownerId has the owner of the Form, not the user providing the share.

That’s not incorrect I think, since this owner does have write access to the file. Can you output $node->getOwner() after the if block? Maybe also the result of $node->getPath() and $this->storage->getUserFolder($ownerId)->getRelativePath($node->getPath())? Could you also please rename the storage property to rootFolder for consistency with the rest of Nextcould code :face_in_clouds:

[toad]

Hmmm, I think this has evolved. Filling in from a public link does work for us - because of the owner guessing, I assume.

[toad]

According to my logging, in the failing case (where the user is logged in and doesn't have access to the spreadsheet which is on a group share), $node->getOwner() was the logged in user's ID.

I think this comes from getFileInfo() and ultimately from GroupFolderStorage::getOwner().

Meanwhile node->getPath() is the full path, from the original owner, i.e. "/matthewtoseland/files/uk tech/test/test3 folder/forms_answers/Yet another test form (responses).xlsx"

Which is why guessing the owner from the path works (both in this case with my patch, and in the logged out case).

[Chartman123]

Could you also please rename the storage property to rootFolder for consistency with the rest of Nextcould code

Addressed in #2337

[toad]

HookConnector::write() calls getNodeForPath(). The passed in path is relative at this point ("/uk tech/test/Yet another test form (responses).xlsx").

That calls Filesystem::getView()->getFileInfo() on path. Which calls Files::Storage::Wrapper::getOwner($path), and we eventually end up in GorupFolderStorage::getOwner(), which returns the current logged in user (since it's a group storage).

Which means the Node's passed in info sets the owner to the currently logged in user. Even though it has the correct absolute path which starts with the real owner.

[toad]

Confirmed. The node has path = "/matthewtoseland/files/uk tech/test/test3 folder/New form title (responses).xlsx" and owner = "matthew-test3".

[toad]

Is there a realistic alternative here? Maybe GroupFolderStorage could check whether the logged in user has access?

Or alternatively, is forms really doing something dodgy here? "The logged in user has access to any group share we interact with" seems like a reasonable invariant. Does it holds outside of forms?

That line of reasoning means you'd need the originally planned "write updates to the spreadsheet on another thread" change. :(

My main concern personally/professionally is whether the workaround patch is safe. We might just turn off versioning to get around it though.

[toad]

@come-nc if the form is filled via the public share link like in this comment, the if branch is used here and ownerId has the owner of the Form, not the user providing the share.

Only if the user is not logged in. If the user is logged in, the owner ID is the logged in user's ID, because it's on a group share. Which is why I consider my workaround basically reasonable (it's not doing anything more dangerous than a path that already exists), but I'm not confident enough to put it on a live system; we'll probably shut down versioning instead.

As for the logging you asked for see my other comments.

[toad]

In fact, if the user is not logged in, $node->getOwner()?->getUid() is null. Which is why we need to guess the ID from the full path.

[come-nc]

Why did you mention HookConnector, the code path goes through a hook? This may be the problem, if the node is serialized to a path which is then turned into a Node again it may lose owner information.

If the ownerId in https://github.com/nextcloud/forms/blob/main/lib/Service/SubmissionService.php#L148-L152 is the logged in user id and not the form owner then it’s a problem in forms. It should not try to write in the file from a user without write permission.

If not, then it’s either a problem with groupfolders or with the hook/event system.

My guess is groupfolders was designed without permissions and this owner situation was never triggered before but the problem is in groupfolders.

[Chartman123]

Please keep in mind that is not only related to groupfolders but also to normal shares. On my dev instance I don't use groupfolders at all.

[toad]

@Chartman123 Are you saying it breaks with normal shares? I don't see how it would: if it doesn't work with the user, it checks the owner, and for a normal share, that will work. (But I can't test this right now because all of our files are in group folders)

We may be conflating two different bugs with different causes here; the first case (user is logged out) appeared to have been fixed before we started getting the new error message (argument 2 must be of type string).

[toad]

@come-nc So you're saying forms needs to be rewritten to write spreadsheet updates on a background thread? That's a much bigger change, when can we realistically expect it?

If it's going to take months then I need to know whether my patch is relatively safe, or whether we just need to turn off either versioning or forms for now. :(

[toad]

So the fact that forms works when there is no user logged in (without forking a separate thread with a different logged in user) shows that it already abuses the permissions model?

[toad]

And yes, it goes through a hook. This is the hook that files_versions uses to create a new revision during a write callback. Which creates a node with the original user's full path, but the logged in user's UID. FileEventsListener then gets a dodgy node. But FileEventsListener already has to workaround odd-looking Node's, because the Node might have a full path but no owner (this happens in the "no user logged in" / public share case).

Hence the 1-line workaround to do the same thing in the case where the owner and user UIDs are the same due to it being a group share.

But if no other NextCloud code abuses the permissions system like Forms is doing, then the fault is in Forms I guess?

[toad]

What @susnux said above - "We run on system permissions" - suggests that what Forms is doing - writing to files that the logged in user does not have access to, without having to fork a worker thread and change the logged in user - is perfectly valid?

In which case AFAICS the options are:

• The 1-line workaround here (make files_versions more tolerant, if it isn't enforcing security constraints itself; if it is, it should return a proper error message!)
• Make GroupFolderStorage::getOwner() not return the logged in user if they don't have access to the group share (returning null would seem reasonable?)
• Or maybe there's a way to not create a new version when filling in a form?

[come-nc]

@toad

1. Forms is not abusing the permissions system, it’s valid to write in a file the user has no write access to, I’m pretty sure we have other occurences of that.
2. I think part of the bug is that it goes through a hook and loses owner information, it should be fixed by doing things the other way around, emit the hook from the event. But that’s a big project we are not sure when we’ll be digging in. We may simply ditch hooks before.
3. I would be interested in an investigation of the issue for shares, as it may show a larger problem since shares do not return current user like groupfolders do.

If the ownerId in https://github.com/nextcloud/forms/blob/main/lib/Service/SubmissionService.php#L148-L152 is the logged in user id and not the form owner then it’s a problem in forms. It should not try to write in the file from a user without write permission.

I did not get an answer to this I think. But it matters a bit less if the hook loses owner information anyway I guess?

[Chartman123]

If the ownerId in https://github.com/nextcloud/forms/blob/main/lib/Service/SubmissionService.php#L148-L152 is the logged in user id and not the form owner then it’s a problem in forms. It should not try to write in the file from a user without write permission.

I did not get an answer to this I think. But it matters a bit less if the hook loses owner information anyway I guess?

I'll get this piece of information this evening :)

[toad]

A relative path is passed into writeFileToCloud, but what it passes through is the correct full path including the owning user. So at least in the group share case it looks like the the owner must be correct at that point. But it's not passed on into the Node.

The Node owner is in the fileInfo, which HookConnector::getNodeForPath() gets from View::getFileInfo(), which gets it from Storage. The owner returned from GroupFolderStorage (always the logged in user) is clearly not helpful for system writes.

Is it possible to pass the ownership in as an argument to e.g. write()? We already pass the path in. But that would mean passing it through putContent() etc.

Or is it important to get it from Storage? Ownership is a property of the file, not the path. In which case, can we just make GroupFolderStorage return null if it's not a share the current logged in user has access to? Does getOwner() returning null cause a problem elsewhere? It already returns null if we're filling in a form from a public share, which works.