[Bug 28385] New: ldap server configuration broken since migration from XML::Simple
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28385 Bug ID: 28385 Summary: ldap server configuration broken since migration from XML::Simple Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: critical Priority: P5 - low Component: Authentication Assignee: koha-bugs@lists.koha-community.org Reporter: dpavlin@rot13.org QA Contact: testopia@bugs.koha-community.org CC: dpavlin@rot13.org -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28385 Dobrica Pavlinusic <dpavlin@rot13.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28385 --- Comment #1 from Dobrica Pavlinusic <dpavlin@rot13.org> --- Created attachment 121147 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121147&action=edit Bug 28385: ldap server configuration broken since migration from XML::Simple Bug 28278 removed usage of XML::Simple which had side-affect that C4::Context->config("ldapserver") returns one more level of hash based on id of element, so ldap configuration was no longer available generating error in log: No ldapserver "hostname" defined in KOHA_CONF: /etc/koha/sites/srvgit/koha-conf.xml which is very confusing since hostname is there, but we are loging at wrong place for it. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28385 Dobrica Pavlinusic <dpavlin@rot13.org> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=28278 -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28385 --- Comment #2 from Dobrica Pavlinusic <dpavlin@rot13.org> --- We don't have any other place in koha-conf.xml where we use same tag and id so I think that this is best place to put fix into as opposed to emulation of XML::Simple behavior. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28385 Dobrica Pavlinusic <dpavlin@rot13.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #121147|0 |1 is obsolete| | --- Comment #3 from Dobrica Pavlinusic <dpavlin@rot13.org> --- Created attachment 121148 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121148&action=edit Bug 28385: ldap server configuration broken since migration from XML::Simple Bug 28278 removed usage of XML::Simple which had side-affect that C4::Context->config("ldapserver") returns one more level of hash based on id of element, so ldap configuration was no longer available generating error in log: No ldapserver "hostname" defined in KOHA_CONF: /etc/koha/sites/srvgit/koha-conf.xml which is very confusing since hostname is there, but we are looking at wrong place for it. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28385 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also|https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=28278 | Keywords| |rel_21_05_candidate CC| |jonathan.druart+koha@gmail. | |com Assignee|koha-bugs@lists.koha-commun |dpavlin@rot13.org |ity.org | Depends on| |28278 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28278 [Bug 28278] Improve $KOHA_CONF parsing speed by using XML::LibXML -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28385 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |victor@tuxayo.net --- Comment #4 from Victor Grousset/tuxayo <victor@tuxayo.net> --- What would be the test plan? I would like to try to test this even though I don't know much about LDAP. No need for it to be an "explain like I'm five" test plan though ^^ It can looks like this: 1. Step to prepare some stuff 2. Another step 3. Do something else that exposes the current issue 4. Apply the patch 5. Redo some of the above steps 6. See that the issue is gone -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28385 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au --- Comment #5 from David Cook <dcook@prosentient.com.au> --- (In reply to Dobrica Pavlinusic from comment #2)
We don't have any other place in koha-conf.xml where we use same tag and id so I think that this is best place to put fix into as opposed to emulation of XML::Simple behavior.
Agreed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28385 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com --- Comment #6 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- This works as expected.. however I wonder if we ought to support both <ldapserver> and <ldapserver id="ldapserver"> syntax and perhaps start recommending the less verbose <ldapserver> option? Also, nice to see you still around Dobrica :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28385 --- Comment #7 from Dobrica Pavlinusic <dpavlin@rot13.org> --- I was wondering about dropping id and there is also ticket about multiple ldap servers which would have different ids, but we need to support id="ldapserver" for upgraded instances, so I decided to start here. I also considered adding something like: foreach my $ldap_id ( keys %$ldap ) { if ( exists($ldap->{$ldap_id}->{hostname}) ) { $ldap = $ldap->{$ldap_id}; last; } } but it seemed like too much code. It would support <ldapserver> without any id and different id from ldapserver, but since we suggest id="ldapserver" is included in documentation and koha-conf.xml template I decided against it. Was I wrong about this? ;-) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28385 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #121148|0 |1 is obsolete| | --- Comment #8 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 121244 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121244&action=edit Bug 28385: ldap server configuration broken since migration from XML::Simple Bug 28278 removed usage of XML::Simple which had side-affect that C4::Context->config("ldapserver") returns one more level of hash based on id of element, so ldap configuration was no longer available generating error in log: No ldapserver "hostname" defined in KOHA_CONF: /etc/koha/sites/srvgit/koha-conf.xml which is very confusing since hostname is there, but we are looking at wrong place for it. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28385 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #9 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- OK, I think I agree, this is the most pragmatic solution for now.. great to know you've thought it all through though Dobrica. It works as expected, so signing off. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28385 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #121244|0 |1 is obsolete| | --- Comment #10 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 121263 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121263&action=edit Bug 28385: ldap server configuration broken since migration from XML::Simple Bug 28278 removed usage of XML::Simple which had side-affect that C4::Context->config("ldapserver") returns one more level of hash based on id of element, so ldap configuration was no longer available generating error in log: No ldapserver "hostname" defined in KOHA_CONF: /etc/koha/sites/srvgit/koha-conf.xml which is very confusing since hostname is there, but we are looking at wrong place for it. == Test plan == 1. Setup an LDAP server with users or use a public one See https://wiki.koha-community.org/wiki/Ldap_testing 2. Configure Koha to use it, see example config above 3. Go to OPAC 4. Koha doesn't compile due to LDAP config not parsed correctly 5. Apply the patch 6. Koha compiles and page loads 7. Log in successfully with one of the LDAP users 8. Bonus: Try to log-in with a wrong username & a good password and vice-versa Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28385 --- Comment #11 from Victor Grousset/tuxayo <victor@tuxayo.net> --- It works :) Tested with a public LDAP server. See the added test plan for details. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28385 --- Comment #12 from Victor Grousset/tuxayo <victor@tuxayo.net> --- QA script reports missing POD (in-code Perl documentation) The relevant code is not even in a subroutine https://git.koha-community.org/Koha-community/Koha/src/branch/master/C4/Auth... so I don't know what should be done QA wise, if there is something to do. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28385 --- Comment #13 from David Cook <dcook@prosentient.com.au> --- (In reply to Martin Renvoize from comment #6)
This works as expected.. however I wonder if we ought to support both <ldapserver> and <ldapserver id="ldapserver"> syntax and perhaps start recommending the less verbose <ldapserver> option?
My hot take would be to look at migrating from koha-conf.xml to koha-conf.yml heh. Down with koha-conf.xml... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28385 --- Comment #14 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 121265 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121265&action=edit Bug 28385: Add tests This does not test bug 28385 directly but 28278 instead. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28385 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #121265|0 |1 is obsolete| | --- Comment #15 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 121279 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121279&action=edit Bug 28385: Add tests This does not test bug 28385 directly but 28278 instead. Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28385 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|ldap server configuration |LDAP server configuration |broken since migration from |broken since migration from |XML::Simple |XML::Simple -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28385 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #16 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Considering Victor's SO a QA stamp. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28385 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |21.05.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28385 --- Comment #17 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Pushed to master for 21.05, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28385 --- Comment #18 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 121361 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121361&action=edit Bug 28385: Use ldapserver only if exists Fix test t/db_dependent/Auth_with_ldap.t .. 1/4 # Failed test 'use C4::Auth_with_ldap;' # at t/db_dependent/Auth_with_ldap.t line 123. # Tried to use 'C4::Auth_with_ldap'. # Error: No ldapserver "id="ldapserver"" defined in KOHA_CONF: /etc/koha/sites/kohadev/koha-conf.xml at /kohadevbox/koha/C4/Auth_with_ldap.pm line 55. # Compilation failed in require at t/db_dependent/Auth_with_ldap.t line 123. # BEGIN failed--compilation aborted at t/db_dependent/Auth_with_ldap.t line 123. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28385 --- Comment #19 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- This last patch fixes failing tests in Auth_with_ldap.t and actually restores the previous behaviour (don't explode if no ldapserver entry defined). I am not saying it's the correct behaviour, but how it behave before this change. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28385 --- Comment #20 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Follow-up pushed to master. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28385 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #21 from Fridolin Somers <fridolin.somers@biblibre.com> --- Depends on Bug 28278 not in 20.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28385 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|rel_21_05_candidate | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28385 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |28587 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28587 [Bug 28587] ldap auth fails unless <ldapserver id="ldapserver"> -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org