The Travis test is failing as follows:

  1) UIIntegrationTest should load the plugins admin page correctly:
     Processed screenshot does not match expected for UIIntegrationTest_admin_plugins.png (image magick error: compare: image widths or heights differ `/home/travis/build/piwik/piwik/tests/lib/screenshot-testing/../../../tests/UI/./expected-ui-screenshots/UIIntegrationTest_admin_plugins.png' @ error/compare.c/CompareImageCommand/957.)

       Url to reproduce: http://localhost/tests/PHPUnit/proxy/?idSite=1&period=year&date=2012-08-09&module=CorePluginsAdmin&action=plugins

       Generated screenshot: /home/travis/build/piwik/piwik/tests/UI/processed-ui-screenshots/UIIntegrationTest_admin_plugins.png

       Expected screenshot: /home/travis/build/piwik/piwik/tests/UI/expected-ui-screenshots/UIIntegrationTest_admin_plugins.png

Generating diff file

Failures encountered. View all diffs at: /home/travis/build/piwik/piwik/tests/lib/screenshot-testing/../../../tests/UI/./screenshot-diffs/diffviewer.html

If processed screenshots are correct, you can copy the generated screenshots to the expected screenshot folder.

*** IMPORTANT *** In your commit message, explain the cause of the difference in rendering so other Piwik developers will be aware of it.

Which just means that the expected screenshot needs to be updated to be equal to the one created on the test. That is because, now that the UsersManager plugin can be deactivated, it also appears in the plugins list, which makes the list appear different than expected. (Plugins that cannot be deactivated do not appear there.)

The build artefacts at https://builds-artifacts.piwik.org/piwik/piwik/master/15402/ says that "No screenshots were uploaded by the Travis build." I guess that is because it is only uploaded for Piwik branches, and I'm not a Piwik dev to have one. So that it seems that the screenshot replacement would need to be done by the Piwik dev merging this pull request, as described in https://github.com/piwik/piwik/blob/master/tests/README.screenshots.md.

Do you mind roughly explaining what the plugin does and what is different to the UsersManager in core etc? Would be nice to understand the use case

Do you mind roughly explaining what the plugin does and what is different to the UsersManager in core etc? Would be nice to understand the use case

I will! But I have realised in the meantime that more changes might be required. Reason: I would first need to implement a (current) password confirmation on (non-admin) user settings in the above mentioned https://github.com/Joey3000/piwik-UsersManagerEncrypted plugin prototype. Without that, the use case is tiny. Because token_auth can completely bypass the session, making password encryption irrelevant. A (current) password confirmation would at least prevent user account take-over for non-admin users. Details: http://forum.piwik.org/read.php?15,129194,page=1#msg-129445.

I'm closing this PR in the meantime. And will reopen it when ready, and provide more explanation and the use case.

okay thx :)

The work on this PR is complete, so here come the details. Note: I'm going to call "UsersManagerEncrypted" the new plugin to extend the core UsersManager.

Use Case

LoginEncrypted and UsersManagerEncrypted plugins are designed for Piwik installations on which implementation of HTTPS is not possible and Piwik is therefore used over plain HTTP. (E.g. lack of support by the host, etc.) The goal of the two plugins is to work together to prevent a passive communication logger (e.g. on a public WiFi network) from, retrospectively, gathering enough information to be able to modify a Piwik user's account settings, thereby possibly taking over the account and locking out the legitimate user.

Only non-super users will be protected. (See Notes below for explanation.)

For additional details on the use case, please refer to the first point in the Important Notes part of the LoginEncrypted project's README.md.

Description

As can be seen in the current description of the LoginEncrypted project, my original assumption when developing it, was that Piwik uses only the standard PHP session management for user authentication, like many other applications. And that token_auth is used only for some additional tasks by external tools - similar to, e.g. an access token on Github. And that token_auth is not exposed to the client unless one navigates to the Piwik configuration page where it can be found.

Also, originally, the UsersManagerEncrypted plugin was only meant as an additional defense, encrypting passwords handled by the UsersManager - namely in a user's account settings, as well as in users management by a super-user. Similarly to the LoginEncrypted plugin's password encryption on login.

Unfortunately, the above assumptions with respect to token_auth are not correct, as token_auth can completely bypass a password authorization of the affected user. The token_auth, additionally, is not handled as a secret by Piwik and is continuously exchanged between the client and the server. (No matter if as an HTTP request parameter, a header or in the content.) It can, therefore, be learned on logging Piwik communication.

Therefore, to at least provide some level of protection for non-super users of Piwik, the UsersManagerEncrypted plugin now additionally implements a current password submission requirement on change of user's account settings. Which leads to the use case described above.

The password request is used, because knowledge of the current password is what, in the above use case, distinguishes an attacker from a legitimate user. While the above plugins ensure that all sent passwords are encrypted.

As the above shows, both plugins need to be used together, as password encryption by LoginEncrypted alone provides hardly any added value, due to the possibility of a bypass using a token_auth.

Notes

• As far as a super-user is concerned, the above plugins cannot protect them. Because, armed with a token_auth of a super-user, an attacker can simply disable the above plugins before any further action. That is also the reason why the current password request is not implemented on the users management administration page in UsersManagementEncrypted - that page is only accessible by a super user. Implementing an effective current password request for super-users would require "extension" of the CorePluginsAdmin plugin, which, I think, would go too far.
• As far as protection of other Piwik configuration is concerned (e.g. tracked sites): That would also require an "extension" of respective core plugins, which I have no plans for either.
• The above use case is the maximum of what's possible "extending" the core Login and UsersManager plugins.
• The above plugins are already almost feature-complete (a different branch of the LoginEncrypted project is used). They can be tried out on Piwik 2.15.0, if desired (with this PR applied). The missing but crucial feature is "salting" passwords before encryption. The salt does not need to be secret, it only needs to have a defined limited validity time, and to be known to both, the client and the server. So that the login page "nonce" can be used in LoginEncrypted. While for UsersManagementEncrypted, the PHP session ID would fit, but can't be accessed from client-side JavaScript (and it shouldn't). I would need to find a work-around for UsersManagementEncrypted. Like sending a nonce or a strong hash of the session ID to the page, or something.)
  P.S.: The reason for the need to salt is that, as of now, encrypted passwords can be used in replay attacks. Similarly to a pass-the-hash attack, but passing the encrypted password.

Please don't hesitate to ask for any additional clarification, or to provide any criticism or suggestions.

P.S.: Please think "reusable" whenever you read "extendable" in this PR. My terminology is terrible. :)

@Joey3000 nice description! I could simply merge as it all looks good but I'm worried re breaking your plugin at some point. To avoid this we'd need to mark it as API but this prevents us from making too many changes to it in the future. We wanna refactor the users manager at some point (there are some issues re usability in Admin with this plugin etc). From what I've seen eg in controller you only overwrite recordUserSettings so far ( https://github.com/Joey3000/piwik-UsersManagerEncrypted/blob/master/Controller.php ) . Would it be possible to change the visibility (private/protected/public) of the methods you need for now and also add an annotation @api everywhere you need it? Eg I doubt we will anytime soon change recordUserSettings so this will give us the possibility to still change other parts in the future. Similar for visibility changes in API.php, I believe they are not needed for your plugin yet.

Alternative suggestion: We could merge it now with all the visibility changes you made so you're free to use them when needed and when you want to extend your plugin but we'd only flag the methods you actually need with @api for now. You can work with this and reuse stuff but there's a certain risk that we break things at some point. Once you use/overwrite a new method we could add a new @api annotation once needed on demand to protect your plugin from breaking it.

Had no idea it would be that easy to overwrite a whole plugin with templates etc well done :) There's a certain risk that we change the names of the templates at some point too but unfortunately we cannot easily provide an API yet to add more form fields / settings to a user. If you want we could possibly post an event in userSettings.twig template ( https://github.com/piwik/piwik/blob/master/plugins/UsersManager/templates/userSettings.twig#L109 ) that would allow you to add custom form fields without having to redeclare all the templates etc. I'm sure other plugins could benefit from this event as well at some point. Eg we have such an event already in https://github.com/piwik/piwik/blob/master/plugins/CoreHome/templates/_topBar.twig#L1 and here is an example of someone listening to such an event: https://github.com/piwik/piwik/blob/2.15.0-b13/plugins/LanguagesManager/LanguagesManager.php#L42 . Would that maybe help?

BTW: You could avoid having to overwrite the API::addUser method by listening to the event API.UsersManager.addUser and overwriting the password there similar to: http://developer.piwik.org/api-reference/events#apipluginnamemethodname
but you would still need to overwrite updateUser() manually since you added a new parameter to the API method which is not possible with this event (I think). On the other side this might lead to errors when using PHP in strict mode: https://github.com/Joey3000/piwik-UsersManagerEncrypted/blob/master/API.php#L34-L35 see eg http://stackoverflow.com/questions/13423494/why-is-overriding-method-parameters-a-violation-of-strict-standards-in-php . The child method must have the same parameters as the parent method in strict mode and it will most likely fail on some installations. Maybe you could try to use an event there as well and instead of accessing $directCall you can maybe try Common::getRequestVar('directCall', false, 'int'). Do you know what I mean? So the updateUser method might fail on some installations, and I'd give it a try to use the events instead to change the password.

Thank you very much for the great feedback with all the detailed suggestions! I agree with all of them.

But after thinking a bit more about the two above plugins, I've come to the conclusion that their very limited use case does not justify the impact the introduction of UsersManagerEncrypted would have on the Piwik project, especially with developers needing to be considerate of UsersManagerEncrypted on updates of the core UsersManager plugin. Neither does it justify the effort to maintain the two plugins. I'm going to try to explain that conclusion.

To summarize their use case, the plugins:

• Protect against account take-overs. While an attacker can still freely roam the affected account, and "vandalize" the account in other ways if desired.
• Do not provide any protection where it matters most: super-user accounts.

On the other hand, under the assumption I had when I started working on those plugins - namely that token_auth is transmitted only when a user explicitly decides to transmit it - the use case looked very different for users not disclosing their token_auth:

• Impossibility for an attacker to access the affected account after the session logged by the attacker expires (e.g. the user logs out)
• Super-user accounts protected like any other account

That would have been worth the trouble. But, unfortunately, that original assumption was wrong. After having realized that, and seeing that those plugins have no use case worth mentioning, I tried to provide at least some protection by introducing the current password confirmation, which lead to the current use case. But I no longer consider that use case worth the hassle for everyone involved.

Note: A use case not worth mentioning is a "dumb attacker" scenario, involving an attacker who knows nothing of the token_auth usage by Piwik and gives up after seeing encrypted passwords and an expired session. It's a typical "security by obscurity" approach, which is a weak but valid additional security layer, but can't be the only layer existing.

So, it looks like I have come a full circle back to the starting point: That, as with any other web application:

• If security/privacy matters, HTTPS should be used.
• Privileged accounts should only be used on administrative tasks requiring them, with "view" accounts used for the daily viewing.

That last point provides almost as much protection for HTTP users of Piwik as the two plugins of mine. And if a "view" user's account gets hijacked they can ask a super-user to restore it, or (hopefully on a more trustworthy connection) login as a super-user to do it themselves. Whereby it should be noted that hijacking a "view" account would be counter-productive for an attacker, as it would make the user aware of its account abuse. It's much better for the attacker to just quietly use the account for "viewing" without the user's knowing.

Therefore, I'm stopping the development of the two plugins involved and going to pull the one already published (LoginEncrypted) from the Marketplace. I don't think the effort has been for nothing, though: I dipped my toes into web development and some minor core Login plugin related clean-ups were made in Piwik (hard-coded Login plugin references fixes, reusability improvements). That will benefit the many existing login related third-party plugins.

I might come back to this topic in some distant future, in case there are changes to the current token_auth mechanism in Piwik - see, e.g., decoupling token_auth from user's password in #5728 (comment), which would allow it's regeneration by UsersManagerEncrypted on session expiration for users not explicitly using token_auth. Or if there is an even more radical change of making token_auth a Github-style token, whose existence is completely optional to the user and whose access rights can be configured per token, like in #5703. (See also #8816 (comment) Solution Discussion point B for details, but please ignore the point A. :) )

On the other hand, I sincerely hope that due to growing adoption of HTTPS I won't need to come back to this topic in the future. Initiatives like Let's Encrypt, with its free and easy-to-obtain certificates getting ready, will help with that.

Thank you for your time and effort!

P.S.: I'm not signing off as a Piwik user and occasional contributer, though. :)

Thx for this detailed explanation. Feel free to comment in other issues or create issues on how to improve core and feel always free to create a PR. We're aware of some of the issues but they don't really have high priority for us right now. This may change any time. As you described HTTPS should be ideally used and super user only when needed. Maybe we can also do something to get this into user's mind. Especially re super user since many are possibly convenient not thinking about it.