| Summary: | Creating users with empty password | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Nahuel Angelinetti <nahuel.angelinetti> |
| Component: | Authentication | Assignee: | Fernando L. Canizo <conan> |
| Status: | CLOSED FIXED | QA Contact: | Bugs List <koha-bugs> |
| Severity: | normal | ||
| Priority: | PATCH-Sent (DO NOT USE) | CC: | chris, conan, dpavlin, f.demians, jwagner, koha.sekjal, nengard |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| Change sponsored?: | --- | Patch complexity: | --- |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: | ||
| Circulation function: | |||
| Bug Depends on: | 6218, 6224 | ||
| Bug Blocks: | |||
| Attachments: |
bugfix
reimplementation Patch reworked to avoid conflict with patch from 6218 Signed-off patch with improvement |
||
|
Description
Chris Cormack
2010-05-21 01:13:35 UTC
This has been reverted due to mismatch, the password was being md5'd then compared to a non md5'd one, so the account wasnt disabled, it in turn ended up being a valid login, with a weak password Created attachment 4006 [details] [review] reimplementation This re-implements the fix to provide disabled logins and also adds auto-generated logins when none provided. (In reply to comment #2) > Created attachment 4006 [details] [review] > reimplementation > > This re-implements the fix to provide disabled logins and also adds > auto-generated logins when none provided. (In reply to comment #2) > Created attachment 4006 [details] [review] > reimplementation > > This re-implements the fix to provide disabled logins and also adds > auto-generated logins when none provided. This patch contains code from your patch for bug 6224. So there will be a merge conflict. It would be great to base it on a branch containing already bug 6224 patch and note that it must be apply after 6224. Your patch solves the bug, ie put a correct value in password field, but there is no UI feedback telling the librarian what has been done internally. I think it's confusing. And it's also not clear what happens when password is modified and cleared for an existing borrower. So I sign-off this patch, since it fixes the describe bug but something has to be done to improve userid/password management. The merge conflict should be manageable by the RM... Created attachment 4307 [details] [review] Patch reworked to avoid conflict with patch from 6218 I understand the merge problem now that 6218 got pushed into master. Here there is a rework of the patch to avoid the conflicting line. (In reply to comment #4) > (In reply to comment #2) > Your patch solves the bug, ie put a correct value in password field, but > there is no UI feedback telling the librarian what has been done > internally. I think it's confusing. And it's also not clear what happens > when password is modified and cleared for an existing borrower. Regarding your concerns I'm not sure if informing the user would not add to more confusion. I think it's enough to document this behavior. I tested this for the 4 user creation cases: no login/no password, no login/yes password, yes login/no password and yes login/yes password. Where yes/no stand for "provided". All went ok. Also I tried modifying an already created user password, and it also went ok. Trying to clear the password by deleting '*' in form or by deleting login+password doesn't do anything, password and login remains. In fact there's no way to clear a password or mark a user as disabled, but that's material for another bug, we will have to change C4::Members::ModMember Created attachment 4309 [details] [review] Signed-off patch with improvement This patch does correctly set the password directly to '!', rather than the hash thereof. I'm not necessarily a fan of the autogenerated userid, but it's consistent with existing code, so I'm marking this as Passed QA Patch pushed, please test This seems to be working as expected: If I create a patron with no password, the password column in the database is set to "!", which will not match any password submission. |