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

Jump to bottom

New INI setting "enable_internet_features" to disable outgoing network communications #11465

Merged
merged 4 commits into from Jun 19, 2017
Merged

New INI setting "enable_internet_features" to disable outgoing network communications #11465

merged 4 commits into from Jun 19, 2017

Conversation

ThaDafinser
Copy link
Contributor

First try for #6324

@ThaDafinser
Copy link
Contributor Author

Still outgoing currently is the Marketplace plugin
https://plugins.piwik.org/api/2.0/plugins/checkUpdates?plugins=..

@mattab mattab added the Needs Review PRs that need a code review label Mar 22, 2017
@mattab mattab added this to the 3.0.4 milestone Mar 22, 2017
@mattab
Copy link
Member

mattab commented Mar 22, 2017

Thanks for the PR @ThaDafinser - we will review this soon

@sgiehl
Copy link
Member

sgiehl commented Apr 9, 2017

Just had a rough look: SEO Widget might still do outgoing requests. Not sure if we maybe should disable this widget if outgoing connections are disabled.

sgiehl
sgiehl requested changes Apr 28, 2017
View reviewed changes
plugins/CoreAdminHome/templates/home.twig
<div piwik-widget-loader='{"module":"Marketplace","action":"getPremiumFeatures"}'></div>
{% endif %}
{% if hasNewPlugins and isMarketplaceEnabled %}
{% if hasNewPlugins and isMarketplaceEnabled and isInternetEnabled %}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add the check for the RssWidget some lines below as well? It also requires connection to the internet.

config/global.ini.php Outdated
@@ -810,7 +816,6 @@
Plugins[] = Insights
Plugins[] = Morpheus
Plugins[] = Contents
Plugins[] = TestRunner
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any purpose on changing that?

config/global.ini.php Outdated
@@ -552,13 +552,19 @@
; - "Email server settings"
enable_general_settings_admin = 1

; By setting this option to 0, all outgoing connections will be disabled
; It also disables enable_auto_update, enable_update_communication if enabled!
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually it doesn't disable those settings - it makes them being ignored in current use cases. Maybe it's better to write something like:
Disabling this will disable features like automatic updates for Piwik, its plugins and components like the GeoIP database, referrer spam blacklist or search engines and social network definitions

@mattab
Copy link
Member

mattab commented May 16, 2017

Hi @ThaDafinser

Stefan reviewed the pr and left a few comments. Do you think you could look at them over next few days?

Thanks and have a great week

@mattab mattab modified the milestones: 3.0.5, Priority Backlog (Help wanted) May 16, 2017
@ThaDafinser
Copy link
Contributor Author

@mattab @sgiehl i changed the requested parts

@sgiehl
Copy link
Member

sgiehl commented May 18, 2017

@ThaDafinser thanks for the update. Will check that in the coming days.

@sgiehl
Copy link
Member

sgiehl commented May 25, 2017

When disabling internet connection and trying to install a plugin via marketplace the error message currently says You cannot install or update the plugin directly as automatic updates are disabled in the config. To enable automatic updates set '[General]enable_auto_update=1' in 'config/config.ini.php'.
That might be a bit misleading. Not sure if we maybe should show another message in that case.

@sgiehl
Copy link
Member

sgiehl commented May 25, 2017

We maybe should hide the ´check for updates´ area in admin, as those check should not work.

@sgiehl
Copy link
Member

sgiehl commented May 25, 2017 •
edited

SEO Rankings and Piwik Blog Widgets won't work without internet as well. Should we remove those widgets from the list?

@sgiehl
Copy link
Member

sgiehl commented May 25, 2017

That's all I just can think off. Other changes now look good.

@sgiehl
Copy link
Member

sgiehl commented May 30, 2017

Note: this might also fix #11065

@sgiehl
Copy link
Member

sgiehl commented Jun 17, 2017

@ThaDafinser Could you adjust the other mentioned things? Or shall we merge this one, and make a new issue/PR for the remaining stuff?

@ThaDafinser
Copy link
Contributor Author

👍 for merging and making the rest in a new one

@sgiehl sgiehl mentioned this pull request Jun 19, 2017
Closed
@sgiehl sgiehl merged commit 22b4852 into matomo-org:3.x-dev Jun 19, 2017
@mattab mattab changed the title disable outgoing communication with one setting New INI setting "enable_internet_features" to disable outgoing network communications Sep 11, 2017
@mattab mattab added the c: Security For issues that make Matomo more secure. Please report issues through HackerOne and not in Github. label Sep 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sgiehl sgiehl sgiehl approved these changes

Assignees
No one assigned
Labels
c: Security For issues that make Matomo more secure. Please report issues through HackerOne and not in Github. Needs Review PRs that need a code review
Projects
None yet
Milestone
3.1.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@ThaDafinser @mattab @sgiehl