Bug 9299 - for loop in Auth_with_ldap.pm requires an extended patron attribute to be set or LDAP logins fail
Summary: for loop in Auth_with_ldap.pm requires an extended patron attribute to be set...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: 3.12
Hardware: All All
: P3 major
Assignee: Nuño López Ansótegui
QA Contact: Jonathan Druart
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-12-17 22:17 UTC by Bryan Lakatos
Modified: 2014-05-26 21:04 UTC (History)
7 users (show)

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

Attachments
bug_9299: rework unique attributes check on ldap login (2.65 KB, patch)
2013-06-05 04:37 UTC, Srdjan Jankovic 		Details | Diff | Splinter Review
[SIGNED-OFF] bug_9299: rework unique attributes check on ldap login (2.82 KB, patch)
2013-07-23 08:06 UTC, Nuño López Ansótegui 		Details | Diff | Splinter Review
Bug 9299: rework unique attributes check on ldap login (3.08 KB, patch)
2013-09-20 15:07 UTC, Jonathan Druart 		Details | Diff | Splinter Review
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Bryan Lakatos 2012-12-17 22:17:24 UTC 
If ExtendedPatronAttributes is enabled but no extended attributes are defined, then the for loop starting on line 181 of Auth_with_ldap.pm causes the array mentioned there to be undefined. This causes an error which prohibits LDAP logins:

(the error returned is)
Can't use an undefined value as an ARRAY reference at /usr/share/koha/lib/C4/Auth_with_ldap.pm line 181, <DATA> line 522.

Disabling ExtendedPatronAttributes gets around the problem, but the loop ought to be made to do nothing if there are no attributes configured/defined.

The offending loop:

                for (my $i; $i< scalar(@$extended_patron_attributes)-1;$i++) {
                        my $attr=$extended_patron_attributes->[$i];
                        unless (C4::Members::Attributes::CheckUniqueness($attr->{code}, $attr->{value}, $borrowernumber)) {
                                unshift @errors, $i;
                                warn "ERROR_extended_unique_id_failed $attr->{code} $attr->{value}";
                        }
Comment 1 Robin Sheat 2013-05-29 22:27:42 UTC 
I've seen this happen when extended attributes are defined, but it's definitely a problem. It might be when none are assigned to the user, or something like that.
Comment 2 Srdjan Jankovic 2013-06-05 04:37:49 UTC Comment hidden (obsolete)
Comment 3 Srdjan Jankovic 2013-06-05 04:41:07 UTC 
To test:

Have ldap server set up in koha-conf.xml and  ExtendedPatronAttributes syspref enabled, with no extended attributes defined.
Successful login should not throw an exception.
Comment 4 Nuño López Ansótegui 2013-07-23 08:06:14 UTC Comment hidden (obsolete)
Comment 5 Bryan Lakatos 2013-09-19 18:52:07 UTC 
I verify that the submitted fix works. Can we move this on down the line into the package? Thanks!
Comment 6 Jonathan Druart 2013-09-20 15:07:45 UTC 
Created attachment 21279 [details] [review]
Bug 9299: rework unique attributes check on ldap login

commit 73e89ab1bf24e8b43560ac0218ac0144d81985df
Author: Srdjan <srdjan@catalyst.net.nz>
Date:   Wed Jun 5 16:34:29 2013 +1200

    Bug 9299: rework unique attributes check on ldap login
    
    To reproduce:
    - configure a ldap server and $KOHA_CONF, etc.
    - try to log in
    - you will got a software error:
    Can't use an undefined value as an ARRAY reference at
    /home/koha/src/C4/Auth_with_ldap.pm line 183.
    
    Signed-off-by: Nuño López Ansótegui <nunyo@masmedios.com>
    Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 7 Jonathan Druart 2013-09-20 15:08:25 UTC 
I reproduce the issue and I configure this patch fixes it.
Marked as Passed QA.
Comment 8 Jonathan Druart 2013-09-20 15:08:40 UTC 
s/configure/confirm...
Comment 9 Galen Charlton 2013-09-27 16:23:35 UTC 
Pushed to master.  Thanks, Srdjan!
Comment 10 Katrin Fischer 2013-11-05 09:42:10 UTC 
I think we have run into this problem in 3.12 - maybe a candidate for a backport? Not sure if there are dependencies on other features in master.
Comment 11 Robin Sheat 2013-11-05 20:01:28 UTC 
(In reply to Katrin Fischer from comment #10)
> I think we have run into this problem in 3.12 - maybe a candidate for a
> backport? Not sure if there are dependencies on other features in master.

It is a problem on 3.12, I'll change the version setting to indicate that.
Comment 12 Tomás Cohen Arazi (tcohen) 2013-11-13 17:32:43 UTC 
This patch has been pushed to 3.12.x, will be in 3.12.7.

Thanks Srdjan!