Set up a php oc10 controller for reading the config.json from the config folder

[kulmann]

Description

When ownCloud Web is deployed as an app, we need to read the config from the config folder, instead of from within the app folder. Otherwise we'll have issues with the signing.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Technical debt
• Tests

Checklist:

• Code changes
• Unit tests added
• Acceptance tests added
• Documentation ticket raised:

[CLAassistant]

All committers have signed the CLA.

[kulmann]

FYI, I discovered that while the app is reachable from a route with apps-external in it, the controller endpoint is only reachable from <baseurl>/index.php/apps/web/config, not <baseurl>/index.php/apps-external/web/config. This is - thanks @IljaN for pointing that out - because the app itself is served directly from it's path by the webserver. Not by the oc10 app framework. We might run into issues with that. Another solution would be to have a controller in place that serves all the static files of the app. But that's very ugly as well.... needs discussion.

[LukasHirt]

I actually liked it when the config was directly copied as config.json. No need to rename/copy the original file then on the server. But it's not anything critical that I'd try to block the PR with. 😁 LGTM 🚀

[kulmann]

I actually liked it when the config was directly copied as config.json. No need to rename/copy the original file then on the server. But it's not anything critical that I'd try to block the PR with. 😁 LGTM 🚀

The reason why this is important in this PR is because we first try to load the config.json directly and only if it doesn't exist we try the new oc10 app endpoint. So we really need that it doesn't exist in the first place. 😁

One ugly thing here: for the app deployment we will always have a 404 error for the config.json in the javascript console. The 404 is expected and we react on it by trying the config endpoint. But we can't suppress the console output.

[micbar]

@C0rby This needs a security review. Can you take a look?

[kulmann]

I actually liked it when the config was directly copied as config.json. No need to rename/copy the original file then on the server. But it's not anything critical that I'd try to block the PR with. 😁 LGTM 🚀

The reason why this is important in this PR is because we first try to load the config.json directly and only if it doesn't exist we try the new oc10 app endpoint. So we really need that it doesn't exist in the first place. 😁

One ugly thing here: for the app deployment we will always have a 404 error for the config.json in the javascript console. The 404 is expected and we react on it by trying the config endpoint. But we can't suppress the console output.

This is not true anymore. Instead of relying on the webserver to serve the app, we now have a php controller in place that serves all the files (except for the config.json) of the app. This brings three improvements:

1. We were now able to change the route for the config, so that it's config.json again and accessed with a require('config.json') call again. No need to construct the php controller route in the js code. No knowledge of the php env in the js code anymore. 🎉 🙌
2. For configuring the web.baseUrl in config.php, the admin doesn't need to care anymore whether the web app lives in apps, apps-external or some other custom apps folder. The Url always points to apps, as the app framework within core takes care of merging all configured app locations into apps routes.
3. When the app is disabled, it (properly) doesn't work anymore. Before it was still served from the webserver, so disabling the app didn't have any effect. The only remaining thing is the nav item. So after disabling the app, one still has to remove the web.baseUrl from config.php to remove all traces of the app in the frontend.

[kulmann]

@C0rby This needs a security review. Can you take a look?

@C0rby
Some context and more detailed questions: we introduced a FilesController which takes care of serving all the files of the web app (js, css, html, ...). Within the controller we have some checks in place - but we would like to have a second opinion of whether this is sufficient:

1. we don't allow .. in any path so that directory traversals to parent folders outside the app should ™️ not be possible.
2. we only allow paths and files that start with or equal the content of the permittedPaths array.
3. we needed to allow inline script execution in CSP headers for the two files oidc-callback.html and oidc-silent-redirect.html, because they are redirecting to different urls via inline script execution. We ran some manual tests if those headers really only apply to those two files - looks like that is the case. But this needs your attention and opinion.
4. since we're serving the files from the controller, the rules from the .htaccess file don't apply. We applied the headers manually to the response headers.

cc @LukasHirt

[micbar]

Let us wait for @C0rby before we merge

[C0rby]

The code looks good so far but I wanted to try to bypass the path traversal checks.
Unfortunately I can't get it to run... I always get config.json not found but it doesn't seem like the ConfigController.php is called.

[kulmann]

The code looks good so far but I wanted to try to bypass the path traversal checks.
Unfortunately I can't get it to run... I always get config.json not found but it doesn't seem like the ConfigController.php is called.

Did you copy a config.json into the config path of oc10?

[C0rby]

Yes, but my error was that I forgot the index.php in the baseUrl... 🤦

[C0rby]

This looks good. 👍
You might want to consider what I wrote in the comment but it's not absolutely necessary.

[C0rby]

The checks look good.
There is one thing I would change though.

$basePath = \dirname(__DIR__,2);
$absolutePath = \realpath( $basePath . '/' . $path;
if ($absolutePath === false) {
  return new DataResponse(['error' => 'resource not found'], Http::STATUS_NOT_FOUND);
}
if (\strpos($absolutePath, $basePath) !== 0) {
  return new DataResponse(['error' => 'resource not found'], Http::STATUS_NOT_FOUND);
}

This way if somehow an attacker manages to do path traversal we still check if the resulting path is in the allowed directory.