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
EZP-19123: Make ezuser.login case in-sensitive across databases #1561
Conversation
42650cd
to
cc5ca78
Compare
Looks clean to me. Is the other approach better in some aspects ? |
it was just easier, this one will need to deal with upgrade script. But this one is more correct, it makes all db's behave closer to how mysql did, which is what most people use. |
If it is more correct then i'd rather go for it... |
@@ -148,8 +148,8 @@ public function loadAnonymousUser(); | |||
/** | |||
* Loads a user for the given login and password. | |||
* | |||
* This method is case sensitive in regards to $login and $password as a comparison of password hash is done | |||
* which contains both parameters. | |||
* Since 6.1 $login is handled case in-sensitive across all storage engines and database backends, however if login |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick: I believe it's case insensitive
(no hyphen)
43ee5f3
to
3d0ac28
Compare
3d0ac28
to
bca681f
Compare
bca681f
to
69609f4
Compare
review ping @glye @joaoinacio Note: Was previously attempting to normalize the characters, however did not find anything that does that for us easily and it's not within scope. |
0e305f3
to
166d044
Compare
@@ -164,7 +164,8 @@ public function loadUserByCredentials($login, $password); | |||
/** | |||
* Loads a user for the given login. | |||
* | |||
* Note: This method loads user by $login where $login might be case in-sensitive on certain storage engines! | |||
* Since 6.1 $login is handled case insensitive across all storage engines and database backends, like was the case |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can be simplified to "login is case-insensitive".
Just a bit unsure about the "double wording": both "case insensitive" and "normalized" are used. Could we try to use only one of those ? That aside, +1 on the implementation (but reviewed on mobile). |
+1, esp. after the ADD COLUMN was added :) |
@@ -0,0 +1,9 @@ | |||
-- Set storage engine schema version number | |||
UPDATE ezsite_data SET value='6.1.0' WHERE name='ezpublish-version'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
6.2 :)
https://jira.ez.no/browse/EZP-19123
Replaces #1527, needed for #1388 (addition-of/switch-to bcrypt user password hashing).
Todo: