@florianjacob opened this Pull Request on May 2nd 2017 Contributor

in UIAssetFetcher and StylesheetUIAssetMerger.
Resolves #11654, makes PIWIK_USER_PATH work again.
Presumably. the paths were just mixed up.

@mattab commented on May 8th 2017 Owner

Hi @florianjacob

Thanks for the PR!
Could you please write here what you do to use this constant, ie. how do you create the public/ folder, which files you move there, etc. ? We would like to test that this works well and then will also create a FAQ or something to share this knowledge with others. Looking forward to it

@florianjacob commented on May 13th 2017 Contributor

@mattab thanks for your interest! :smile:

To test this PR (this assumes a Debian-like filesystem):

  1. extract piwik, let's say to /var/www/piwik, or reuse an existing installation
  2. create /var/lib/piwik and make it read-write for the php-executing user
  3. move /var/www/piwik/config to /var/lib/piwik/config. tmp can be moved as well, otherwise it will be created automatically.
  4. add this bootstrap.php:
    <?php
    define('PIWIK_USER_PATH', '/var/lib/piwik');
  5. optionally remove write access for the php user on /var/www/piwik

In my case, I actually use an environment variable to set PIWIK_USER_PATH without needing to modify bootstrap.php.

I assumed I'm the only one who doesn't know how to use this, but I just checked git history, and PIWIK_USER_PATH is probably broken since about 3 years. 😆

So I'll also write down what I could gather and understand about PIWIK_USER_PATH, PIWIK_INCLUDE_PATH, preventing access to php files and making most of a piwik installation read-only for php.

As @robocoder described at https://github.com/piwik/piwik/issues/11654#issuecomment-297730843, in an ideal world, piwik would have a public/ folder in which all files reside that need to be served via the webserver like index.php, piwik.js etc., and the user had to set the webserver's document root to /var/www/piwik/public instead of /var/www/pwik.

The next best thing we have for this is PIWIK_USER_PATH and PIWIK_INCLUDE_PATH, mentioned by https://piwik.org/docs/include-piwik-in-your-project/#bootstrap-php-execute-custom-code-before-piwik-runs

PIWIK_USER_PATH is intended to move the folders config and tmp to another directory that is not publicly accessible. Additionally, these are the only two directories that need to be writable from php during piwik installation and operation. The remaining piwik files can all be made read only to increase security, and to be shared between multiple piwik installations.

PIWIK_USER_PATH does not work. This is what this PR tries to fix, with the motivation to make most of piwik read-only and therefore easily packageable and updateable for Linux distributions.

As I understand it, PIWIK_INCLUDE_PATH is another private folder, intended to move the remaining php files that don't need to be publicly accessible to another location than PIWIK_DOCUMENT_ROOT, where everything is publicly served by default. This could then be used to move all publicly accessible files to a public/ folder when PIWIK_INCLUDE_PATH points to the parent directory where the private php files live. But I don't know which files need to be moved then, though, i.e. which files really need to be publicly served, and therefore could not find out how to use it. ☚ī¸

@sgiehl commented on July 19th 2017 Member

Imho this change sounds legit.
But I'm wondering if this change might possibly break an installation if those variables have been overwritten before and now change.

@mattab commented on July 24th 2017 Owner

@sgiehl could you quickly test this use case?

@florianjacob commented on July 24th 2017 Contributor

My thoughts on this:

  • As $PIWIK_USER_PATH defaults to $PIWIK_DOCUMENT_PATH, it doesn't matter if somebody overwrote $PIWIK_DOCUMENT_PATH.

  • If somebody overwrote $PIWIK_USER_PATH, they would need to have identified all assets from core and plugins that go through UIAssetFetcher.php and / or StylesheetUIAssetMerger.php, and copied or moved them from $PIWIK_DOCUMENT_ROOT to $PIWIK_USER_PATH (personally, I have no idea how to find all that assets). If they moved those files, their configuration would break as the assets can't be found anymore. I think it's quite unlikely somebody has done this, because it would be quite hard to identify all assets and then moving them but keeping their correct path, needed to be redone on every update, and it's clearly against what the documentation of $PIWIK_USER_PATH says:

    PIWIK_USER_PATH – override the default to relocate config and tmp files outside the web document root.

  • If somebody overwrote both, it's the same case if they only overwrote $PIWIK_USER_PATH.

@sgiehl What do you think, did I miss something? If you see any use case, please give it a test! I just can't think of one.

@sgiehl commented on July 25th 2017 Member

Haven't had some deeper thoughts on that before. Your possible use cases seem to be complete. Can't think of any other.
So guess it should be good to merge.

@florianjacob commented on July 25th 2017 Contributor

Thanks!

This Pull Request was closed on July 25th 2017
Powered by GitHub Issue Mirror