Looks like Piwik always used the token auth and not the password: https://github.com/piwik/piwik/blob/0da8fa6a7f4f77412eb0f8cda966914416fad977/plugins/Login/Auth.php

Not sure if it confers any security benefits, going to investigate further...

Found initial use of token_auth for logging in: 62ff9b5

Before this, it used the 'password' as the credential column (for Zend's auth adapter).

My guess is that it was put in in preparation for API authentication. Since that uses only token auth, it may have been easier to have one code path instead of two.

I cannot think of any security benefits of using only the token auth. Someone could potentially brute force the password just as easily as when authenticating against the hashed password.

I can see a couple issues w/ merging this as is:

1. Can you make sure users cannot be added if there exists a user w/ the same name but different capitalization? Can you also make sure there is a test covering this? (Before posting this I checked for the answer to the first question, and it appears you cannot add a user w/ a similar name. I did not check for a test though.)
2. If there are users in current installs that are different based on case sensitivity, one of them will end up being ignored. To solve this, can you make Model::getUser() favor users w/ the exact same name as what is given? Ie, if the user's name is an exact match, use that user, otherwise use the first user that matches w/ case-insensitive compare. I'm not sure if it's currently possible for this to be the case, though, unless the user was added in the past or was added manually to the DB (or added through 3rd party code). We can change this behavior for 3.0, but I think for the LTS 2.15 version, we should try to maintain BC.

One more thing: can you make the case insensitivity explicit in Model::getUser()? Either by adding a comment or changing the code. It would be helpful to developers to see that case insensitivity is the expected behaviour when messing w/ the code. If you think a test is enough though, feel free to ignore this.

Forgot to mention it but for the record another motivation for this solution (i.e. login using the password, not always going through the auth token) is to help a tiny bit getting rid of md5() in the future. That's one less behavior coupled to md5 (because generating the auth_token from the login/password was using md5).

can you make the case insensitivity explicit in Model::getUser()? Either by adding a comment

Done. Comment + test should do it.

1. Can you make sure users cannot be added if there exists a user w/ the same name but different capitalization? Can you also make sure there is a test covering this?

Yep a test for that will be good. Will do it.

1. If there are users in current installs that are different based on case sensitivity, one of them will end up being ignored.

I hope that currently it's not possible to register with the same login and different case. Else this will be a mess. We'll see tomorrow ;)

@diosmosis It should be good with the last 2 commits. Check also b4c52c7, it's quite ugly but if you think that it's possible that some users could exist somewhere with same login/different case then…

As the initial request was re email address (which in another plugin may be used as login), does it maybe make sense to update methods like getUserByEmail(), userEmailExists() as well? Think they are already case insensitive though so please ignore it :)

Also wondering if there was a possible attack to create a new account with an existing username but different case, eg first letter uppercase, and then when logging in, I'm getting a different users identity or cannot log in anymore?

Eg there is a user with username test.tester. Now someone else is signing up with username Test.Tester. Now someone is trying to log in via Test.tester... it would probably always return the first user test.tester: https://github.com/piwik/piwik/pull/8610/files#diff-3a5ed5014067501a6c9488796d449cd5R161 so for the user Test.Tester it wouldn't be possible to log in case insensitive. It's an edge case I know.

Guess all I'm saying is can we prevent the creation of new users with the same username but different case? I think it's already the case and it is not even possible to have same username twice with different case so please ignore it all if someone can confirm this. userExists() looks like it's case insensitive

Yes this is what we discussed above, see the latest commits (e.g. 9244504, b4c52c7).