Add system check warnings for php-fpm and nginx if config files are accessible #18398
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bx80
added
not-in-changelog
sgiehl
requested changes
Nov 29, 2021
….ini.php access instead of config.ini.php
sgiehl
reviewed
Dec 2, 2021
There was a problem hiding this comment.
The code looks fine so far. But I'm not able to test that fully, as I don't have a nginx setup locally and no time to create one to only test that. I also haven't fully read the both issues this PR should solve, so not sure if that is the case. Maybe @Findus23 can have a quick look if this PR makes sense or not?
|
This issue is in "needs review" but there has been no activity for 7 days. ping @matomo-org/core-reviewers |
github-actions
bot
added
the
Stale
github-actions
bot
removed
the
Stale
justinvelluppillai
suggested changes
Dec 14, 2021
There was a problem hiding this comment.
Seems to work ok. Pending fixing @Findus23 changes this should be ok to merge, and then would be good to ask the users experiencing the problem originally if this resolves it for them.
justinvelluppillai
approved these changes
Dec 14, 2021
This was referenced Mar 31, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
Fixes #13589
Fixes #18132
Nginx doesn't use .htaccess files by default, therefore will not apply access restriction rules for the /config directory
Apache will ignore .htaccess files for any php files passed to php-fpm via fpm-fcgi, therefore will also not apply access restriction rules.
This PR adds additional checks to the system diagnostics page to detect these scenarios and provide informative warnings.
Accessibility of the /config/global.ini.php files is used a general test of whether the access rules are being applied. If the global config file is not accessible then no warnings will be shown even if running php-fpm or nginx, as we can assume that non-htaccess rules have been added to the server config and we don't want to show a warning once the issue has been corrected.
For the PHP-SAPI check:
If the config is accessible and fpm-fcgi is being used...
...if Apache then show a warning suggesting adding a ProxyPass rule
...if nginx then show a warning and link to the official nginx server configuration
...otherwise show a generic warning that an access rule should be added to prevent access to /config
For the server info check:
If the config is accessible and nginx is being used then show a warning and link to the official nginx server configuration
Review