Bug 17215 - Cardnumbers of patrons created using LDAP Replication not using autoMemberNum if configured
Summary: Cardnumbers of patrons created using LDAP Replication not using autoMemberNum...
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Janik
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-08-29 12:21 UTC by Lee Jamison
Modified: 2022-07-24 11:51 UTC (History)
10 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Assigns numerical cardnumber if autoMemberNum is enabled (1.21 KB, patch)
2016-08-29 12:45 UTC, Lee Jamison
Details | Diff | Splinter Review
Implemented autoMemberNum in C4::Auth_with_ldap.pm (1.32 KB, patch)
2016-08-30 17:44 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17215 - converted tabs to whitespace (1.86 KB, patch)
2016-08-31 12:49 UTC, Lee Jamison
Details | Diff | Splinter Review
Bug 17215 - converted tabs to whitespace (1.91 KB, patch)
2016-09-01 13:29 UTC, Lee Jamison
Details | Diff | Splinter Review
Bug 17215 - Cardnumbers of patrons created using LDAP Replication not using autoMemberNum if configured (1.96 KB, patch)
2016-09-06 18:09 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Lee Jamison 2016-08-29 12:21:04 UTC
When using LDAP and leveraging the 'replicate' flag, patrons who are replicated have their cardnumber assigned equal to their LDAP username. This is an inconsistency when the 'autoMemberNum' System Preference is set to 'Do'. When 'autoMemberNum' is set to 'Don't' then replicate should act as it currently does.
Comment 1 Lee Jamison 2016-08-29 12:45:43 UTC Comment hidden (obsolete)
Comment 2 Nick Clemens 2016-08-30 17:44:04 UTC Comment hidden (obsolete)
Comment 3 Nick Clemens 2016-08-30 17:45:01 UTC
QA tools complaints:

   FAIL	  forbidden patterns
		forbidden pattern: tab char (line 245)
		forbidden pattern: tab char (line 247)
		forbidden pattern: tab char (line 242)
		forbidden pattern: tab char (line 243)
		forbidden pattern: tab char (line 241)
		forbidden pattern: tab char (line 248)
		forbidden pattern: tab char (line 244)
		forbidden pattern: tab char (line 246)


Tested using LDAP server here:
http://www.forumsys.com/en/tutorials/integration-how-to/ldap/online-ldap-test-server/
Comment 4 Nick Clemens 2016-08-30 18:13:29 UTC
Just pasting in from my koha-conf.xml to ease testing setup for anyone else:

  <useldapserver>1</useldapserver><!-- see C4::Auth_with_ldap for extra configs you must add if you want to turn this on -->
    <ldapserver id="ldapserver">
      <hostname>ldap://ldap.forumsys.com/</hostname>
      <port>389</port>
      <base>dc=example,dc=com</base>
      <user>cn=read-only-admin,dc=example,dc=com</user>          <!-- DN, if not anonymous -->
      <pass>password</pass>          <!-- password, if not anonymous -->
      <replicate>1</replicate>       <!-- add new users from LDAP to Koha database -->
      <update>0</update>             <!-- update existing users in Koha database -->
      <auth_by_bind>1</auth_by_bind> <!-- set to 1 to authenticate by binding instead of password comparison, e.g., to use Active Directory -->
      <principal_name>uid=%s,dc=example,dc=com</principal_name>
      <mapping>                  <!-- match koha SQL field names to your LDAP record field names -->
        <userid       is="uid" ></userid>
        <categorycode is="banana">YA</categorycode>
        <branchcode is="apple">LPL</branchcode>
 <!--       <password     is="userpassword"   ></password>-->
      </mapping>
    </ldapserver>
Comment 5 Lee Jamison 2016-08-31 12:49:28 UTC Comment hidden (obsolete)
Comment 6 Chris Cormack 2016-08-31 23:51:41 UTC
Hi Lee

Thanks for marking this signed off. Would you please be able to add a signoff to the patch

(git commit --amend -s)

And then obsolete the old ones.

Thanks heaps
Comment 7 Lee Jamison 2016-09-01 13:26:49 UTC
Sorry about that Chris! Still learning the procedures! I just signed off on it so I'll upload the signed off patch now!

Thanks for the info!
Comment 8 Lee Jamison 2016-09-01 13:29:52 UTC Comment hidden (obsolete)
Comment 9 Lee Jamison 2016-09-01 17:09:33 UTC
I noticed that my last update didn't indicate that the patch was signed off on (still working on figuring out how to do that when I do the git commit/signoff - sorry!). So I'll add it in here manually for archival purposes.

Created attachment 55092 [details] [review]
Bug 17215 - converted tabs to whitespace

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17215

Signed-off-by: Lee Jamison <ldjamison@marywood.edu>
Comment 10 Kyle M Hall 2016-09-06 18:09:12 UTC
Created attachment 55264 [details] [review]
Bug 17215 - Cardnumbers of patrons created using LDAP Replication not using autoMemberNum if configured

To test:
1. Set the autoMemberNum System Preference to "Don't".
2. Login to the OPAC with an LDAP account that does not currently exist as a patron record.
3. Check the patron record's cardnumber.
4. the cardnumber should equal the patron's LDAP username.
5. Delete the patron record.
6. Apply the patch.
7. Set the autoMemberNum System Preference to "Do".
8. Login to the OPAC with an LDAP account that does not currently exist as a patron record.
9. Check the patron record's cardnumber.
10. The cardnumber should now be the next highest cardnumber based on autoMemberNum.

Signed-off-by: Lee Jamison <ldjamison@marywood.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 11 Jonathan Druart 2016-10-03 11:55:40 UTC
I have revived by 6782 yesterday and it would fix this bug.
Could someone confirm?
Comment 12 Lee Jamison 2016-10-03 12:12:02 UTC
If I may add my thoughts, 6782's scope is specific to autoMemberNum in regards to the patron creation form. However, 17215 bypasses the patron creation form altogether when Auth_with_ldap.pm triggers. Also, when reviewing the Diff of each bug's latest patch both bugs apply completely different changes. 

17215 ONLY modifies C4::Auth_with_ldap while 6782 modifies C4::Members, memberentrygen.tt, and memberentry.pl
Comment 13 Jonathan Druart 2016-10-03 12:42:41 UTC
The patch from bug 6782 moves the autoMemberNum check to the AddMember subroutine. That means that the cardnumber will be auto-generated at this point.
Comment 14 Jonathan Druart 2017-03-29 20:07:03 UTC
Could someone confirm this is still needed after bug 6782?
Comment 15 Katrin Fischer 2017-10-15 11:59:08 UTC
Patch still applies, sadly, can't test LDAP with my local setup. Maybe try sending an e-mail to the list about this?
Comment 16 Lee Jamison 2017-10-16 00:37:41 UTC
(In reply to Katrin Fischer from comment #15)
> Patch still applies, sadly, can't test LDAP with my local setup. Maybe try
> sending an e-mail to the list about this?

Are you able to try Nick's LDAP test in Comment 4?
Comment 17 Katrin Fischer 2018-03-30 14:39:07 UTC
Should this be Signed off? I see lots of lines in the patch!
Comment 18 Lee Jamison 2018-05-26 02:38:22 UTC
Can anyone confirm if this is still relevant?
Comment 19 Janik 2022-01-22 11:31:15 UTC
I have reopened the bug as the problem was not fixed. It is still the case that the userId is taken as the cardnumber should a patron be created via LDAP.
Comment 20 Janik 2022-02-20 17:26:41 UTC
The bug seems fixed with patch 29930:
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130926&action=diff

When creating a new user, an empty cardnumber is now used and thus the setting 'autoMemberNum' is also considered during patron creation

To test:
1. Set the autoMemberNum System Preference to "Don't".
2. Login to the OPAC with an LDAP account that does not currently exist as a patron record.
3. Check the patron record's cardnumber.
4. the cardnumber should equal the patron's LDAP username.
5. Delete the patron record.
6. Apply the patch.
7. Set the autoMemberNum System Preference to "Do".
8. Login to the OPAC with an LDAP account that does not currently exist as a patron record.
9. Check the patron record's cardnumber.
10. The cardnumber should now be the next highest cardnumber based on autoMemberNum.