Bug 19908 - Password should not be mandatory
Summary: Password should not be mandatory
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: master
Hardware: All All
: P5 - low critical with 1 vote (vote)
Assignee: Josef Moravec
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on: 18298
Blocks: 19907
  Show dependency treegraph
 
Reported: 2018-01-03 09:48 UTC by Katrin Fischer
Modified: 2018-04-24 05:09 UTC (History)
16 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 19908: Password fields in edit/add patron form should respect BorrowerMandatoryField preference (2.33 KB, patch)
2018-01-03 11:05 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 18908: Do not use .optional in password match validation function (1.43 KB, patch)
2018-02-05 08:17 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 19908: Password fields in edit/add patron form should respect BorrowerMandatoryField preference (2.44 KB, patch)
2018-02-19 19:08 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 18908: Do not use .optional in password match validation function (1.43 KB, patch)
2018-02-19 19:08 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 18908: Do not use .optional in password match validation function (1.49 KB, patch)
2018-02-23 20:34 UTC, Roch D'Amour
Details | Diff | Splinter Review
Bug 18908: Do not use .optional in password match validation function (1.55 KB, patch)
2018-02-25 22:14 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 19908: Password fields in edit/add patron form should respect BorrowerMandatoryField preference (2.51 KB, patch)
2018-02-25 22:16 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 18908: Do not use .optional in password match validation function (1.55 KB, patch)
2018-02-25 22:16 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 19908: Password fields in edit/add patron form should respect BorrowerMandatoryField preference (2.50 KB, patch)
2018-03-07 15:42 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 19908: Do not use .optional in password match validation function (1.49 KB, patch)
2018-03-07 15:42 UTC, Josef Moravec
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2018-01-03 09:48:45 UTC
When password is not set as a mandatory patron field, it should not be required. 

To test:
- make sure BorrowerMandatoryField is empty and doesn't list password
- add a new patron, leave password fields empty
- try to save - you will be forced to enter a password with the field now being highlighted as required.

I have also tried changing  RequireStrongPassword and emptying minPasswordLength.

Often libraries will leave password and username empty for patrons that won't ever use the OPAC. For example when they are not using the OPAC functionalities in general or for internal users. Making the password mandatory is a regression to former behaviour.
Comment 1 Josef Moravec 2018-01-03 11:05:55 UTC
Created attachment 70242 [details] [review]
Bug 19908: Password fields in edit/add patron form should respect BorrowerMandatoryField preference

Test plan:
0) Do not apply the patch, note the password field is always required
1) Apply the patch
2) Try to add and edit patron with and without "password" in BorrowerMandatoryField, it should always respect this setting
3) Use "Change password" button in patron toolbar, the password field
should be never required here - when leaved blank, the password is
unchanged
4) Play with  minPasswordLength and  RequireStrongPassword preferences,
    to ensure they work as expected
Comment 2 Jonathan Druart 2018-01-03 15:22:42 UTC
On members/member-password.pl I can submit the form without confirming the password.
Comment 3 Josef Moravec 2018-02-05 08:17:44 UTC
Created attachment 71217 [details] [review]
Bug 18908: Do not use .optional in password match validation function

Test plan:
0) Aplly only first patch
1) You will be able to submit a from on member-password.pl even if the
passwords don't match
2) Apply this patch -> you can't be able to submit the form if the
paswords don't match, but you'll be able to submit the form when
password fields are blank
Comment 4 Josef Moravec 2018-02-05 08:20:53 UTC
(In reply to Jonathan Druart from comment #2)
> On members/member-password.pl I can submit the form without confirming the
> password.

Should be fixed by this second patch.

This is restoring the original behavior, but there is still room for imporvements:

It is not possible to change only userid from here.
When you submit the blank passwords, then you get the same form again - not very useful I think.

So probably adding the ability to change only userid from here would be nice enhancement here I think.
Comment 5 Claire Gravely 2018-02-08 15:05:51 UTC
Hi, 

I got as far as applying the first patch to test. 
BorrowerMandatoryField is empty

I was unable to save the patron record without entering a username. I get the following message:

The following fields are wrong. Please fix them.
 - Username/password already exists.

It will not let me save until I have entered a username for the patron. Previous behaviour was that one was automatically generated if left blank.
Comment 6 Claire Gravely 2018-02-08 15:45:14 UTC
(In reply to Claire Gravely from comment #5)
> Hi, 
> 
> I got as far as applying the first patch to test. 
> BorrowerMandatoryField is empty
> 
> I was unable to save the patron record without entering a username. I get
> the following message:
> 
> The following fields are wrong. Please fix them.
>  - Username/password already exists.
> 
> It will not let me save until I have entered a username for the patron.
> Previous behaviour was that one was automatically generated if left blank.

Sorry, should add this was for patron categories with type Org.
Comment 7 Josef Moravec 2018-02-19 19:07:49 UTC
(In reply to Claire Gravely from comment #5)
> Hi, 
> 
> I got as far as applying the first patch to test. 
> BorrowerMandatoryField is empty
> 
> I was unable to save the patron record without entering a username. I get
> the following message:
> 
> The following fields are wrong. Please fix them.
>  - Username/password already exists.
> 
> It will not let me save until I have entered a username for the patron.
> Previous behaviour was that one was automatically generated if left blank.

That is bug in master too and not related to this bug.

Other categories have username generated as "firstname.surname" if you leave it blank, but organization not and when you have any patron with userid=null in db, koha thinks its duplicate and you can't save it.

But out of scope of this bug report i think.
Comment 8 Josef Moravec 2018-02-19 19:08:29 UTC
Created attachment 71970 [details] [review]
Bug 19908: Password fields in edit/add patron form should respect BorrowerMandatoryField preference

Test plan:
0) Do not apply the patch, note the password field is always required
1) Apply the patch
2) Try to add and edit patron with and without "password" in BorrowerMandatoryField, it should always respect this setting
3) Use "Change password" button in patron toolbar, the password field
should be never required here - when leaved blank, the password is
unchanged
4) Play with  minPasswordLength and  RequireStrongPassword preferences,
    to ensure they work as expected
Comment 9 Josef Moravec 2018-02-19 19:08:34 UTC
Created attachment 71971 [details] [review]
Bug 18908: Do not use .optional in password match validation function

Test plan:
0) Aplly only first patch
1) You will be able to submit a from on member-password.pl even if the
passwords don't match
2) Apply this patch -> you can't be able to submit the form if the
paswords don't match, but you'll be able to submit the form when
password fields are blank
Comment 10 Josef Moravec 2018-02-19 19:08:47 UTC
rebased on master
Comment 11 Roch D'Amour 2018-02-23 20:34:38 UTC
Created attachment 72159 [details] [review]
Bug 18908: Do not use .optional in password match validation function

Test plan:
0) Aplly only first patch
1) You will be able to submit a from on member-password.pl even if the
passwords don't match
2) Apply this patch -> you can't be able to submit the form if the
paswords don't match, but you'll be able to submit the form when
password fields are blank

https://bugs.koha-community.org/show_bug.cgi?id=19908
Signed-off-by: Roch D'Amour <roch.damour@inlibro.com>
Comment 12 Katrin Fischer 2018-02-25 22:14:22 UTC
Created attachment 72188 [details] [review]
Bug 18908: Do not use .optional in password match validation function

Test plan:
0) Aplly only first patch
1) You will be able to submit a from on member-password.pl even if the
passwords don't match
2) Apply this patch -> you can't be able to submit the form if the
paswords don't match, but you'll be able to submit the form when
password fields are blank

https://bugs.koha-community.org/show_bug.cgi?id=19908
Signed-off-by: Roch D'Amour <roch.damour@inlibro.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 13 Katrin Fischer 2018-02-25 22:16:34 UTC
Created attachment 72189 [details] [review]
Bug 19908: Password fields in edit/add patron form should respect BorrowerMandatoryField preference

Test plan:
0) Do not apply the patch, note the password field is always required
1) Apply the patch
2) Try to add and edit patron with and without "password" in BorrowerMandatoryField, it should always respect this setting
3) Use "Change password" button in patron toolbar, the password field
should be never required here - when leaved blank, the password is
unchanged
4) Play with  minPasswordLength and  RequireStrongPassword preferences,
    to ensure they work as expected

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 14 Katrin Fischer 2018-02-25 22:16:37 UTC
Created attachment 72190 [details] [review]
Bug 18908: Do not use .optional in password match validation function

Test plan:
0) Aplly only first patch
1) You will be able to submit a from on member-password.pl even if the
passwords don't match
2) Apply this patch -> you can't be able to submit the form if the
paswords don't match, but you'll be able to submit the form when
password fields are blank

https://bugs.koha-community.org/show_bug.cgi?id=19908
Signed-off-by: Roch D'Amour <roch.damour@inlibro.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 15 Katrin Fischer 2018-02-25 22:22:04 UTC
I was about to file a new bug for the problem with organisations, but then found it: bug 13655.

I think this patch is important as a bugfix for stable releases so should not be blocked by old broken behavoiur.
Comment 16 Jonathan Druart 2018-02-26 15:11:12 UTC
We should apply the same fix at the OPAC, right?

Please fix bug number on the second patch.
Comment 17 Agnes Rivers-Moore 2018-03-01 17:22:44 UTC
Our Koha as of 17.11.03 does not generate default username for any patron type.
Josef said 'Other categories (than org) have username generated as "firstname.surname" if you leave it blank' - but that does not seem to be the case any more.
Previous behaviour also assigned a default password, and that also does not happen any more.
Result is staff have to add three fields, making them unique/matching, even if the patron does not want an online account.
Looking forward to this being non-mandatory! Thanks.
Comment 18 Josef Moravec 2018-03-07 15:42:47 UTC
Created attachment 72503 [details] [review]
Bug 19908: Password fields in edit/add patron form should respect BorrowerMandatoryField preference

Test plan:
0) Do not apply the patch, note the password field is always required
1) Apply the patch
2) Try to add and edit patron with and without "password" in BorrowerMandatoryField, it should always respect this setting
3) Use "Change password" button in patron toolbar, the password field
should be never required here - when leaved blank, the password is
unchanged
4) Play with  minPasswordLength and  RequireStrongPassword preferences,
    to ensure they work as expected

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 19 Josef Moravec 2018-03-07 15:42:51 UTC
Created attachment 72504 [details] [review]
Bug 19908: Do not use .optional in password match validation function

Test plan:
0) Aplly only first patch
1) You will be able to submit a from on member-password.pl even if the
passwords don't match
2) Apply this patch -> you can't be able to submit the form if the
paswords don't match, but you'll be able to submit the form when
password fields are blank

Signed-off-by: Roch D'Amour <roch.damour@inlibro.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 20 Josef Moravec 2018-03-07 15:43:14 UTC
(In reply to Jonathan Druart from comment #16)
> We should apply the same fix at the OPAC, right?
> 
> Please fix bug number on the second patch.

Bug number fixed.

Where in OPAC do you think? 

On change password page it doesn't make sense for me and on self registration, the password field is not required a if leaved blanked, the password is generated, which is correct.
Comment 21 Josef Moravec 2018-03-07 15:45:21 UTC
(In reply to Agnes Rivers-Moore from comment #17)
> Our Koha as of 17.11.03 does not generate default username for any patron
> type.
> Josef said 'Other categories (than org) have username generated as
> "firstname.surname" if you leave it blank' - but that does not seem to be
> the case any more.
> Previous behaviour also assigned a default password, and that also does not
> happen any more.
> Result is staff have to add three fields, making them unique/matching, even
> if the patron does not want an online account.
> Looking forward to this being non-mandatory! Thanks.

Hi Agnes, please fill a bug report about user names not generated.
Comment 22 Agnes Rivers-Moore 2018-03-07 19:18:12 UTC
(In reply to Josef Moravec from comment #21)
> (In reply to Agnes Rivers-Moore from comment #17)
> > Our Koha as of 17.11.03 does not generate default username for any patron
> > type.
> > Josef said 'Other categories (than org) have username generated as
> > "firstname.surname" if you leave it blank' - but that does not seem to be
> > the case any more.
> > Previous behaviour also assigned a default password, and that also does not
> > happen any more.
> > Result is staff have to add three fields, making them unique/matching, even
> > if the patron does not want an online account.
> > Looking forward to this being non-mandatory! Thanks.
> 
> Hi Agnes, please fill a bug report about user names not generated.

Will test further and submit a new bug for Koha not auto-generating username & password.
Comment 23 Katrin Fischer 2018-03-09 07:20:09 UTC
Should this be assigned or another status?
Comment 24 Josef Moravec 2018-03-09 10:32:34 UTC
Switching back to Passed QA, see comment 20
Comment 25 Agnes Rivers-Moore 2018-03-13 01:32:56 UTC
Bug 20378 created for Koha does not auto-generate OPAC ID and password.
Comment 26 Jonathan Druart 2018-03-19 18:30:53 UTC
Pushed to master for 18.05, thanks to everybody involved!
Comment 27 Agnes Rivers-Moore 2018-03-19 20:27:26 UTC
Will patches be issued for 17.x?
Comment 28 Agnes Rivers-Moore 2018-03-19 20:32:44 UTC
(In reply to Agnes Rivers-Moore from comment #22)
> (In reply to Josef Moravec from comment #21)
> > (In reply to Agnes Rivers-Moore from comment #17)
> > > Our Koha as of 17.11.03 does not generate default username for any patron
> > > type.
> > > Josef said 'Other categories (than org) have username generated as
> > > "firstname.surname" if you leave it blank' - but that does not seem to be
> > > the case any more.
> > > Previous behaviour also assigned a default password, and that also does not
> > > happen any more.
> > > Result is staff have to add three fields, making them unique/matching, even
> > > if the patron does not want an online account.
> > > Looking forward to this being non-mandatory! Thanks.
> > 
> > Hi Agnes, please fill a bug report about user names not generated.
> 
> Will test further and submit a new bug for Koha not auto-generating username
> & password.

It seems that resolving bug 19908 will probably fix the auto-generating username and password issue.  They were generated when a new record was saved.  Mandatory password prevented the save process.  I believe this is why the username and password were never generated.  Using a quick add without the password fields does allow save, and on reviewing the record, the username and password have been inserted.
Comment 29 Nick Clemens 2018-04-24 01:30:45 UTC
Awesome work all, backported to stable for 17.11.05
Comment 30 Fridolin SOMERS 2018-04-24 05:09:52 UTC
Depending on Bug 18298 not in 17.05.x