matomo-org / matomo

Empowering People Ethically with the leading open source alternative to Google Analytics that gives you full control over your data. Matomo lets you easily collect data from websites & apps and visualise this data and extract insights. Privacy is built-in. Liberating Web Analytics. Star us on Github? +1. And we love Pull Requests!
https://matomo.org/
GNU General Public License v3.0
19.91k stars 2.65k forks source link

require TwoFA to be verified before updating code base #14251

Closed paulrudy closed 5 years ago

paulrudy commented 5 years ago

When visiting self-hosted Matomo installation (with two-factor authentication enabled):

  1. Matomo prompted for login/pass, but did not require 2 factor authentication
  2. Matomo prompted for upgrade
  3. I initiated upgrade and Matomo completed it
  4. An error page—referencing something like a loop. I didn't copy it, sorry.
  5. Browsed back
  6. Matomo showed upgrade successfully completed page
  7. Matomo finally asks for 2 factor authentication

It seems to me 2 factor authentication should be successfully completed before prompting for upgrade and before permitting initiation of upgrade.

tsteur commented 5 years ago

What do you mean by "prompted for upgrade"? We screen to complete the upgrade by executing the updates is shown to anyone AFAIK, even to not logged in users if I remember correctly

paulrudy commented 5 years ago

Yes, by "prompted for upgrade", I meant the screen showing that an upgrade is available.

Matomo initiated the upgrade once I logged in with password, but it didn't require 2FA. It seems to me that it ought to, if 2FA is enabled, no?

tsteur commented 5 years ago

@paulrudy I can't reproduce it. What I would expect is that it shows the "Please update the database screen". We would show this even to a logged out user if you just updated the codebase. This is done in https://github.com/matomo-org/matomo/pull/13796

After logging in, you can access the update screen though directly by opening eg the URL https://matomo.example.com/index.php?module=CoreUpdater&action=newVersionAvailable . This we could possibly disallow I think by adjusting the condition to this: image

paulrudy commented 5 years ago

Sorry for the late reply. Glad my comment was useful, even if I couldn't quite remember accurately.