February 2022 - Koha-bugs - lists.koha-community.org

[Bug 29543] Self-checkout allows returning everybody's loans
by bugzilla-daemon＠bugs.koha-community.org 08 Feb '22

08 Feb '22

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29543 --- Comment #57 from Marcel de Rooy <m.de.rooy(a)rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #55) > (In reply to Marcel de Rooy from comment #50) > > Why dont you pass the session cookie? > > You could pass something like [ $cookie1, $cookie2 ] ? > > I think CGISESSID is in the CGI object already. Cookies need to be sent out. If they have changed, they will only be updated at client side when you sent them explicitly to the output subs. output_html_with_http_headers and friends actually assume that it is the authentication cookie; so you are cheating here. > (In reply to Marcel de Rooy from comment #53) > > Iiuc JWT is not encrypted. So this may be a bit more secure, but could be > > improved. > > It is encoded using a secret, see Koha::Token::_gen_jwt OK. It is bit hidden but I found it in the _default_params sub. -- You are receiving this mail because: You are watching all bug changes.

1 0

[Bug 30048] New: Koha::ArticleRequests->search_limited may be malformed.
by bugzilla-daemon＠bugs.koha-community.org 08 Feb '22

08 Feb '22

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30048 Bug ID: 30048 Summary: Koha::ArticleRequests->search_limited may be malformed. Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs(a)lists.koha-community.org Reporter: martin.renvoize(a)ptfs-europe.com QA Contact: testopia(a)bugs.koha-community.org Whilst digging through Koha::Suggestions with bug 29886 it highlighted to me that our 'search_limited' method differs somewhat between different Koha classes. My initial understanding, from Koha::Patrons->search_limited, was that the idea of search_limited was to protect patron data across branches. In actual fact, it's more about restricting the object class in questions data from being leaked across branches. As such, I think the construction used in ArticleRequests is incorrect. Instead of limiting based on the requesting patrons attached to the requests the currently logged in patron can see.. we should be limiting simply by which libraries the current patron is allowed to see... and the linked patron details should be getting obfuscated in their own right. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

1 4

[Bug 29543] Self-checkout allows returning everybody's loans
by bugzilla-daemon＠bugs.koha-community.org 08 Feb '22

08 Feb '22

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29543 --- Comment #56 from Jonathan Druart <jonathan.druart+koha(a)gmail.com> --- (In reply to Marcel de Rooy from comment #54) > $cookie = $query->cookie( > -name => 'JWT', > -value => $jwt // '', > -expires => $jwt ? '+1d' : '', > > Expiry should be shorter and in line with the expiry for sco. Yes, correct. That could be shorter. -- You are receiving this mail because: You are watching all bug changes.

1 0

[Bug 29543] Self-checkout allows returning everybody's loans
by bugzilla-daemon＠bugs.koha-community.org 08 Feb '22

08 Feb '22

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29543 --- Comment #55 from Jonathan Druart <jonathan.druart+koha(a)gmail.com> --- (In reply to Marcel de Rooy from comment #50) > Question when looking in sco-main > > my $jwt = $query->cookie('JWT'); > if ($op eq "logout") { > $template->param( loggedout => 1 ); > $query->param( patronlogin => undef, patronpw => undef ); > undef $jwt; > } > > Shouldnt we do some cleaning up here and just exit ? exit and what? Here we are going to display the login form, that's the expected behaviour (In reply to Marcel de Rooy from comment #51) > Hmm. Why are we doing this in sco-main: > > L47 > my ( $template, $loggedinuser, $cookie ) = get_template_and_user( > So the CGISESSID cookie is used here and we check permissions. We check authentication for the staff member (who has logged in, or the credential from the sysprefs) > L371 > $cookie = $query->cookie( -name => 'JWT', > We create a new cookie JWT. But ignore the session cookie. The JWT is used to authorised the OPAC user that has been authenticate in the previous step (cardnumber or login+password, depending on SelfCheckoutByLogin. > L381 > output_html_with_http_headers $query, $cookie, $template->output, undef, { > force_no_caching => 1 }; > We output now without the CGISESSID, only JWT. > Why dont you pass the session cookie? > You could pass something like [ $cookie1, $cookie2 ] ? I think CGISESSID is in the CGI object already. (In reply to Marcel de Rooy from comment #52) > L362 > csrf_token => Koha::Token->new->generate_csrf( { > session_id => scalar $query->cookie('CGISESSID') . $patron->cardnumber, id > => $patron->userid } ), > > Where is the corresponding csrf check? in sco-patron-image.pl (In reply to Marcel de Rooy from comment #53) > Iiuc JWT is not encrypted. So this may be a bit more secure, but could be > improved. It is encoded using a secret, see Koha::Token::_gen_jwt -- You are receiving this mail because: You are watching all bug changes.

1 0

[Bug 29543] Self-checkout allows returning everybody's loans
by bugzilla-daemon＠bugs.koha-community.org 08 Feb '22

08 Feb '22

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29543 --- Comment #54 from Marcel de Rooy <m.de.rooy(a)rijksmuseum.nl> --- $cookie = $query->cookie( -name => 'JWT', -value => $jwt // '', -expires => $jwt ? '+1d' : '', Expiry should be shorter and in line with the expiry for sco. -- You are receiving this mail because: You are watching all bug changes.

1 0

[Bug 29543] Self-checkout allows returning everybody's loans
by bugzilla-daemon＠bugs.koha-community.org 08 Feb '22

08 Feb '22

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29543 --- Comment #53 from Marcel de Rooy <m.de.rooy(a)rijksmuseum.nl> --- Iiuc JWT is not encrypted. So this may be a bit more secure, but could be improved. -- You are receiving this mail because: You are watching all bug changes.

1 0

[Bug 29543] Self-checkout allows returning everybody's loans
by bugzilla-daemon＠bugs.koha-community.org 08 Feb '22

08 Feb '22

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29543 --- Comment #52 from Marcel de Rooy <m.de.rooy(a)rijksmuseum.nl> --- L362 csrf_token => Koha::Token->new->generate_csrf( { session_id => scalar $query->cookie('CGISESSID') . $patron->cardnumber, id => $patron->userid } ), Where is the corresponding csrf check? -- You are receiving this mail because: You are watching all bug changes.

1 0

[Bug 29543] Self-checkout allows returning everybody's loans
by bugzilla-daemon＠bugs.koha-community.org 08 Feb '22

08 Feb '22

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29543 --- Comment #51 from Marcel de Rooy <m.de.rooy(a)rijksmuseum.nl> --- Hmm. Why are we doing this in sco-main: L47 my ( $template, $loggedinuser, $cookie ) = get_template_and_user( So the CGISESSID cookie is used here and we check permissions. L371 $cookie = $query->cookie( -name => 'JWT', We create a new cookie JWT. But ignore the session cookie. L381 output_html_with_http_headers $query, $cookie, $template->output, undef, { force_no_caching => 1 }; We output now without the CGISESSID, only JWT. Why dont you pass the session cookie? You could pass something like [ $cookie1, $cookie2 ] ? -- You are receiving this mail because: You are watching all bug changes.

1 0

[Bug 29543] Self-checkout allows returning everybody's loans
by bugzilla-daemon＠bugs.koha-community.org 08 Feb '22

08 Feb '22

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29543 --- Comment #50 from Marcel de Rooy <m.de.rooy(a)rijksmuseum.nl> --- Question when looking in sco-main my $jwt = $query->cookie('JWT'); if ($op eq "logout") { $template->param( loggedout => 1 ); $query->param( patronlogin => undef, patronpw => undef ); undef $jwt; } Shouldnt we do some cleaning up here and just exit ? -- You are receiving this mail because: You are watching all bug changes.

1 0

[Bug 29543] Self-checkout allows returning everybody's loans
by bugzilla-daemon＠bugs.koha-community.org 08 Feb '22

08 Feb '22

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29543 Jonathan Druart <jonathan.druart+koha(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |30045 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30045 [Bug 30045] SCO print slip is broken -- You are receiving this mail because: You are watching all bug changes.

1 0