[Bug 16492] New: Checkouts ( and possibly checkins and other actions ) will use the patron home branch as the logged in library
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16492 Bug ID: 16492 Summary: Checkouts ( and possibly checkins and other actions ) will use the patron home branch as the logged in library Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: critical Priority: P5 - low Component: SIP2 Assignee: koha-bugs@lists.koha-community.org Reporter: kyle@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org CC: colin.campbell@ptfs-europe.com Bug 14507 introduced the use of checkpw in C4::SIP::ILS::Patron so that non-Koha internal authentication processes would be able to function via SIP ( LDAP et al ). The problem is that checkpw changes the userenv to that of the patron! This is not usually an issue in Koha because most of the time that patron running through checkpw is the one to be logged in. Aside from SIP2 the only other area where this may be an issue is in SCO when using SelfCheckoutByLogin. -- 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=16492 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |kyle@bywatersolutions.com |ity.org | -- 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=16492 --- Comment #1 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 51390 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=51390&action=edit Bug 16492 - Checkouts ( and possibly checkins and other actions ) will use the patron home branch as the logged in library Bug 14507 introduced the use of checkpw in C4::SIP::ILS::Patron so that non-Koha internal authentication processes would be able to function via SIP ( LDAP et al ). The problem is that checkpw changes the userenv to that of the patron! This is not usually an issue in Koha because most of the time that patron running through checkpw is the one to be logged in. Aside from SIP2 the only other area where this may be an issue is in SCO when using SelfCheckoutByLogin. Test Plan: 1) On master, check out an item to a patron via SIP2 2) Note the checkout lists the item as having been checked out from the patron's home library not matter which library is was supposed to be checked out from. 3) Apply this patch 4) Re-checkout the item 5) The item should now be checked out as if it was checked out from the library as defined in the SIP configuration file. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16492 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #51390|0 |1 is obsolete| | --- Comment #2 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 51391 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=51391&action=edit Bug 16492 - Checkouts ( and possibly checkins and other actions ) will use the patron home branch as the logged in library Bug 14507 introduced the use of checkpw in C4::SIP::ILS::Patron so that non-Koha internal authentication processes would be able to function via SIP ( LDAP et al ). The problem is that checkpw changes the userenv to that of the patron! This is not usually an issue in Koha because most of the time that patron running through checkpw is the one to be logged in. Aside from SIP2 the only other area where this may be an issue is in SCO when using SelfCheckoutByLogin. Test Plan: 1) On master, check out an item to a patron via SIP2 2) Note the checkout lists the item as having been checked out from the patron's home library not matter which library is was supposed to be checked out from. 3) Apply this patch 4) Re-checkout the item 5) The item should now be checked out as if it was checked out from the library as defined in the SIP configuration file. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16492 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |14507 Status|NEW |Needs Signoff Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14507 [Bug 14507] SIP Authentication broken when LDAP Auth Enabled -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16492 Heather Braum <hbraum@nekls.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |hbraum@nekls.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16492 Srdjan Jankovic <srdjan@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |srdjan@catalyst.net.nz --- Comment #3 from Srdjan Jankovic <srdjan@catalyst.net.nz> --- I think that we need a test for this one. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16492 --- Comment #4 from Colin Campbell <colin.campbell@ptfs-europe.com> --- The call to checkpw in Patron.pm is weird and will probably make people think some magic is being invoked. the parameters should be more explicit i.e. : checkpw($dvh, $self->{userid}, $pwd, undef, undef, 1); with a comment to document -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16492 --- Comment #5 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 51744 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=51744&action=edit Bug 16492 [QA Followup] -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16492 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #51391|0 |1 is obsolete| | Attachment #51744|0 |1 is obsolete| | --- Comment #6 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 51745 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=51745&action=edit Bug 16492 - Checkouts ( and possibly checkins and other actions ) will use the patron home branch as the logged in library Bug 14507 introduced the use of checkpw in C4::SIP::ILS::Patron so that non-Koha internal authentication processes would be able to function via SIP ( LDAP et al ). The problem is that checkpw changes the userenv to that of the patron! This is not usually an issue in Koha because most of the time that patron running through checkpw is the one to be logged in. Aside from SIP2 the only other area where this may be an issue is in SCO when using SelfCheckoutByLogin. Test Plan: 1) On master, check out an item to a patron via SIP2 2) Note the checkout lists the item as having been checked out from the patron's home library not matter which library is was supposed to be checked out from. 3) Apply this patch 4) Re-checkout the item 5) The item should now be checked out as if it was checked out from the library as defined in the SIP configuration file. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16492 --- Comment #7 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Srdjan Jankovic from comment #3)
I think that we need a test for this one.
Unit test added! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16492 --- Comment #8 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Colin Campbell from comment #4)
The call to checkpw in Patron.pm is weird and will probably make people think some magic is being invoked. the parameters should be more explicit i.e. :
checkpw($dvh, $self->{userid}, $pwd, undef, undef, 1);
with a comment to document
Fixed in new patch! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16492 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16492 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #51745|0 |1 is obsolete| | --- Comment #9 from Chris Cormack <chris@bigballofwax.co.nz> --- Created attachment 51768 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=51768&action=edit Bug 16492 - Checkouts ( and possibly checkins and other actions ) will use the patron home branch as the logged in library Bug 14507 introduced the use of checkpw in C4::SIP::ILS::Patron so that non-Koha internal authentication processes would be able to function via SIP ( LDAP et al ). The problem is that checkpw changes the userenv to that of the patron! This is not usually an issue in Koha because most of the time that patron running through checkpw is the one to be logged in. Aside from SIP2 the only other area where this may be an issue is in SCO when using SelfCheckoutByLogin. Test Plan: 1) On master, check out an item to a patron via SIP2 2) Note the checkout lists the item as having been checked out from the patron's home library not matter which library is was supposed to be checked out from. 3) Apply this patch 4) Re-checkout the item 5) The item should now be checked out as if it was checked out from the library as defined in the SIP configuration file. Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16492 --- Comment #10 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 51797 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=51797&action=edit Bug 16492: Test that no_set_userenv will not set userenv if not exist yet -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16492 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #11 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Should not we just set a correct userenv instead of not creating at all? There are several places where an error could occur if not set. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16492 --- Comment #12 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #11)
Should not we just set a correct userenv instead of not creating at all? There are several places where an error could occur if not set.
That's a good idea. Would you be able to add that as a followup? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16492 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16492 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #51768|0 |1 is obsolete| | Attachment #51797|0 |1 is obsolete| | --- Comment #13 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 52188 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=52188&action=edit Bug 16492 - Checkouts ( and possibly checkins and other actions ) will use the patron home branch as the logged in library Bug 14507 introduced the use of checkpw in C4::SIP::ILS::Patron so that non-Koha internal authentication processes would be able to function via SIP ( LDAP et al ). The problem is that checkpw changes the userenv to that of the patron! This is not usually an issue in Koha because most of the time that patron running through checkpw is the one to be logged in. Aside from SIP2 the only other area where this may be an issue is in SCO when using SelfCheckoutByLogin. Test Plan: 1) On master, check out an item to a patron via SIP2 2) Note the checkout lists the item as having been checked out from the patron's home library not matter which library is was supposed to be checked out from. 3) Apply this patch 4) Re-checkout the item 5) The item should now be checked out as if it was checked out from the library as defined in the SIP configuration file. Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16492 --- Comment #14 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 52189 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=52189&action=edit Bug 16492: Test that no_set_userenv will not set userenv if not exist yet -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16492 --- Comment #15 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 52190 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=52190&action=edit Bug 16492: Add a FIXME to explain the possible problem with no_set_userenv -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16492 --- Comment #16 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Kyle M Hall from comment #12)
(In reply to Jonathan Druart from comment #11)
Should not we just set a correct userenv instead of not creating at all? There are several places where an error could occur if not set.
That's a good idea. Would you be able to add that as a followup?
I do not have the time right now. Let's see later, when we will need to reuse again this parameter. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16492 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #17 from Kyle M Hall <kyle@bywatersolutions.com> --- Pushed to master for 16.11, thanks for the followups Jonathan! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16492 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |frederic@tamil.fr --- Comment #18 from Frédéric Demians <frederic@tamil.fr> --- Pushed in 16.05. Will be in 16.05.01. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16492 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |julian.maurice@biblibre.com Status|Pushed to Master |Pushed to Stable --- Comment #19 from Julian Maurice <julian.maurice@biblibre.com> --- Patch pushed to 3.22.x, will be in 3.22.8 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org