Are you sure it is affected by this line? It doesn't do anything with sites. I tried to reproduce without a site but couldn't. Can you maybe describe step by step?

I can reproduce it after clicking on the activation link

Problem is after clicking on the activation link it wants to redirect to another URL here: https://github.com/piwik/piwik/blob/master/plugins/Login/Controller.php#L282

There it wants to create a URL to redirect but as no website is defined and it fails here: https://github.com/piwik/piwik/blob/master/core/Plugin/Menu.php#L191

In this case $websiteId = 0. We could check whether a site exists at all and if not, we ignore the exception but not sure about side effects. Also Piwik is currently designed that there is at least one site. To make the login work we could change the login controller to build the URL ourselves maybe? I don't see why it has to use URL::redirectToUrl in this case as only module & action is needed. (Obviously it is to remove duplicated code but in this case we could maybe redirect to the module directly)

@diosmosis I committed a patch see f1ce174

I'm not sure with the latest login refactoring that was happening a couple of months ago. I presume this code should work as other Login plugins are supposed to extend the Login controller? And if they don't extend it, the change is still ok as it would never go into this method of the original Login controller (instead into the confirmResetPassword method of the custom Login plugin)?

@tsteur Login plugins have to extend from the controller as a practical reality, but I don't think they have to (or should). Ideally, the Login controller should be always activated, and other plugins should provide Auth implementations, PasswordResetters, etc. I don't think there are many plugins in the marketplace, though, so maybe we can do a quick check and see if it won't break anything.

True, they should ideally only provide auth implementation. I will have a look at the Marketplace but I can't think of any problem, it should be even better this way as we get rid of the doAsSuperUser(). BTW: We do not only have to care about Marketplace plugins. I presume there are many plugins out there used in companies etc that are not public.

• If a plugin implements it's own controller with a custom confirmResetPassword method and does not extend Piwik's Login Controller it does not matter anyway as our code will never be executed
• If a plugin implements it's own controller with a custom resetPasswordSuccess method and does not extend Piwik's Login Controller the reset password would not work anyway as it would never execute confirmResetPassword of our Login controller. It would require the plugin to also implement confirmResetPassword which makes everything fine again
• If a plugin extends our Login controller everything is fine anyway and nothing changes. They can overwrite the resetPasswordSuccess method if they want to but they don't have to. In any way it would work.
• Ideally they only provide a different auth adapter which is officially supported. Our controllers are not a public API so far unless they are tagged with @api. Nonetheless it would still work.