Summary: | Usernames should be case sensitive | ||
---|---|---|---|
Product: | Koha | Reporter: | Chris Cormack <chris> |
Component: | Patrons | Assignee: | Chris Cormack <chris> |
Status: | Failed QA --- | QA Contact: | Testopia <testopia> |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | gmcharlt, jdemuth, jonathan.druart, kyle.m.hall, mtompset, nick, veron |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
See Also: |
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18424 https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17829 |
||
GIT URL: | Change sponsored?: | --- | |
Patch complexity: | --- | Documentation contact: | |
Documentation submission: | Text to go in the release notes: | ||
Version(s) released in: | Circulation function: | ||
Attachments: |
Bug 16282 : Setting up the database for case sensitive usernames
Bug 16282 : Make check_pw_internal case sensitve for user names Bug 16282 : Making GetMember be case sensitive for userid |
Description
Chris Cormack
2016-04-18 03:05:57 UTC
This only changes for the internal methods of Koha So not Shibboleth, LDAP etc, they all function the same as they were. Just anything that uses Koha's internal login method is affected. For upgrades, behaviour will stay the same, the syspref will go in off. But for new installs they will have case sensitive usernames. Created attachment 50442 [details] [review] Bug 16282 : Setting up the database for case sensitive usernames Just adding a syspref For upgrade, current behaviour stays the same For new installs, usernames are now case sensitive Created attachment 50443 [details] [review] Bug 16282 : Make check_pw_internal case sensitve for user names To test: 1/ Make a user with a username like Fish 2/ Login to OPAC and staff side, test you can login with username fish, FISH, FIsh etc 3/ Apply patch, set UsernamesCaseSensitive to No 4/ Login again to OPAC and staff side, test you can still use fish, FISH 5/ Set syspref to Yes 6/ Try logging in with fish, note you cant 7/ Login with Fish successfully Created attachment 50447 [details] [review] Bug 16282 : Making GetMember be case sensitive for userid The main place this changes is SIP GetMember with a username is only used in 3 places C4/SIP/ILS/Patron.pm: $kp = GetMember(cardnumber=>$patron_id) || GetMember(userid=>$patron_id); misc/load_testing/benchmark_staff.pl: unless defined C4::Members::GetMember(userid => $user); tools/import_borrowers.pl: $member = GetMember( 'userid' => $borrower{'userid'} ); To Test SIP 1/ Apply patches, set syspref to No (it should be No already but just in case) 2/ Restart the SIP server 3/ Login to SIP with a userid and password combo 4/ Notice no change, try with different case for userid 5/ Switch on the syspref 6/ Restart the SIP server 7/ Notice the userid/username case sensitive now The benchmarking script appears faulty anyway, but if you can get it to run, it should now care about case sensitive usernames if you have the syspref on. To test borrower import 1/ Apply patch, check syspref is No 2/ Import borrowers with the userid being the matchpoint notice case doesn't matter 3/ Switch syspref to Yes 4/ Import borrowers again, now they will only match if the userid(username) matches exactly (Case sensitive) Just to note ILSDI AuthenticPatron, svc/authenticate will all respect this syspref also I assume the REST api will too, but it's all woo woo magic so hard to actually tell how it is authenticating. I tried testing this patch, it works well for regular sign in to koha. Import borrowers I couldn't test (but I think 15840 has an issue) SIP - can this be tested using the sip_cli_emulator? If so I couldn't figure a method
> SIP - can this be tested using the sip_cli_emulator? If so I couldn't figure
> a method
Perhaps, I tested this with an EzProxy set up that was connected to Koha via SIP. I'll see if I can figure out how to get the emulator to do it. I think it's just using cardnumber though. But probably can extend it
Comment on attachment 50443 [details] [review] Bug 16282 : Make check_pw_internal case sensitve for user names Review of attachment 50443 [details] [review]: ----------------------------------------------------------------- ::: C4/Auth.pm @@ +1793,5 @@ > + my $sth; > + if ( C4::Context->preference('UsernamesCaseSensitive') ){ > + # This query does a case sensitive query, but it first does a insensitive one so that the index can still be used > + # This will mean we don't get a nasty performance hit > + $sth = $dbh->prepare("SELECT * FROM (SELECT password,cardnumber,borrowernumber,userid,firstname,surname,borrowers.branchcode,branches.branchname,flags FROM borrowers JOIN branches ON borrowers.branchcode=branches.branchcode WHERE userid= ?) AS firstpass WHERE BINARY userid = ?"); WHERE BINARY? This is a MySQL-ism, is it not? (In reply to M. Tompsett from comment #8) > WHERE BINARY? This is a MySQL-ism, is it not? Is it? Anyone able to point to documentation? (In reply to Owen Leonard from comment #9) > (In reply to M. Tompsett from comment #8) > > > WHERE BINARY? This is a MySQL-ism, is it not? > > Is it? Anyone able to point to documentation? https://dev.mysql.com/doc/refman/5.7/en/cast-functions.html I can't find such a casting name for postgres sqlite or other SQLs. Though, I could be wrong. Which is why I was asking for confirmation. (In reply to M. Tompsett from comment #10) > (In reply to Owen Leonard from comment #9) > > (In reply to M. Tompsett from comment #8) > > > > > WHERE BINARY? This is a MySQL-ism, is it not? > > > > Is it? Anyone able to point to documentation? > > https://dev.mysql.com/doc/refman/5.7/en/cast-functions.html > I can't find such a casting name for postgres sqlite or other SQLs. Though, > I could be wrong. Which is why I was asking for confirmation. WHERE BINARY does not exist e.g. for Oracle: http://stackoverflow.com/questions/14819724/where-binary-in-oracle > WHERE BINARY does not exist e.g. for Oracle:
> http://stackoverflow.com/questions/14819724/where-binary-in-oracle
So this should be Failed QA? Please correct the status if not.
Note that I am going to remove GetMember on bug 17829. |