Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use PIWIK_DOCUMENT_ROOT instead of PIWIK_USER_PATH #11661

Merged
merged 1 commit into from Jul 25, 2017

Conversation

florianjacob
Copy link
Contributor

@florianjacob florianjacob commented May 2, 2017

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

@mattab
Copy link
Member

mattab commented May 8, 2017

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

@mattab mattab added this to the 3.0.5 milestone May 8, 2017
in UIAssetFetcher and StylesheetUIAssetMerger.
Resolves matomo-org#11654, makes PIWIK_USER_PATH work again.
Presumably. the paths were just mixed up.
@florianjacob florianjacob changed the title Correct PIWIK_USER_PATH to PIWIK_INCLUDE_PATH Correct PIWIK_USER_PATH to PIWIK_DOCUMENT_ROOT May 13, 2017
@florianjacob
Copy link
Contributor Author

florianjacob commented May 13, 2017

@mattab thanks for your interest! 😄

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');
  1. 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 #11654 (comment), 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
Copy link
Member

sgiehl commented Jul 19, 2017

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
Copy link
Member

mattab commented Jul 24, 2017

@sgiehl could you quickly test this use case?

@florianjacob
Copy link
Contributor Author

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
Copy link
Member

sgiehl commented Jul 25, 2017

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.

@mattab mattab merged commit af84e77 into matomo-org:3.x-dev Jul 25, 2017
@florianjacob florianjacob deleted the fix-piwik_user_path branch July 25, 2017 22:59
@florianjacob
Copy link
Contributor Author

Thanks!

@mattab mattab changed the title Correct PIWIK_USER_PATH to PIWIK_DOCUMENT_ROOT Use PIWIK_DOCUMENT_ROOT instead of PIWIK_USER_PATH Sep 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants