This patch addresses CSV formula injection vulnerabilities by ensuring
all Text::CSV instances use formula protection.
Changes made:
* Add formula => 'empty' to all Text::CSV constructors in:
- Koha::Patrons::Import
- Koha::BackgroundJob::ImportKBARTFile
- Koha::ERM::EUsage::CounterFile
The 'empty' formula mode causes potentially dangerous formulas (starting
with =, +, -, @) to be treated as empty strings rather than executed,
preventing CSV injection attacks while maintaining data integrity.
Security impact:
* Prevents execution of malicious formulas in CSV imports
* Protects against data exfiltration via CSV formula injection
* Maintains backward compatibility for legitimate CSV data
To test:
1. Apply the previous patch (author test)
2. Run:
$ ktd --shell
k$ prove xt/author/Text_CSV_Various.t
=> FAIL: Should show 3 files without formula protection
3. Apply this patch
4. Repeat 2
=> SUCCESS: All tests should now pass
5. Try importing CSV with formula content (=cmd|"/c calc",test,data)
=> SUCCESS: Formula content is rejected/emptied, not executed
6. Import normal CSV data
=> SUCCESS: Normal data imports correctly
7. Sign off :-D
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
This patch addresses CSV formula injection vulnerabilities by ensuring
all Text::CSV instances use formula protection.
Changes made:
* Add formula => 'empty' to all Text::CSV constructors in:
- Koha::Patrons::Import
The 'empty' formula mode causes potentially dangerous formulas (starting
with =, +, -, @) to be treated as empty strings rather than executed,
preventing CSV injection attacks while maintaining data integrity.
Security impact:
* Prevents execution of malicious formulas in CSV imports
* Protects against data exfiltration via CSV formula injection
* Maintains backward compatibility for legitimate CSV data
To test:
1. Apply the previous patch (author test)
2. Run:
$ ktd --shell
k$ prove xt/author/Text_CSV_Various.t
=> FAIL: Should show 1 file without formula protection
3. Apply this patch
4. Repeat 2
=> SUCCESS: All tests should now pass
5. Try importing CSV with formula content (=cmd|"/c calc",test,data)
=> SUCCESS: Formula content is rejected/emptied, not executed
6. Import normal CSV data
=> SUCCESS: Normal data imports correctly
7. Sign off :-D
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: wainuiwitikapark <wainuiwitikapark@catalyst.net.nz>
This patch makes the `to_api()` method return an proper exception when
the `$self->biblioitem` accessor returns `undef`. This is a bad data
scenario and we could do better in terms of logging the situation.
The new exception will be correctly rendered in the logs by the
`unhandled_exception` Mojolicious helper and no behavior change on the
API.
I wasn't sure to keep the api tests I wrote. I originally added handling
for the specific exception in the controller, but it become obvious it
was not worth: too much code, the `GET /biblios` endpoint would need the
same treatment, etc. And the current `500 Internal server error` is good
enough as a response, and the logs explain the situation correctly.
To test:
1. On a fresh `KTD` pick a biblio (say biblionumber=1)
2. Use your favourite REST tool (Postman?) to access the record with
`Accept: application/json`. i.e.
```
GET kohadev.localhost/api/v1/biblios/1
Accept: application/json
```
=> SUCCESS: All good
3. Delete the related `biblioitems` row:
$ ktd --shell
k$ koha-mysql kohadev
> DELETE FROM biblioitems WHERE biblionumber=1; \q
4. Repeat 2
=> SUCCESS: You get a 500 error
5. Check the logs
=> FAIL: You get a cryptic message about accessing a method on an
undefined object:
```
2025/02/13 13:15:00] [ERROR] GET /api/v1/biblios: unhandled exception (Mojo::Exception)<<Can't call method "to_api" on an undefined value at /usr/share/koha/Koha/Biblio.pm line 1433.>>
```
6. Apply this patch
7. Restart things:
k$ restart_all
8. Repeat 2
=> SUCCESS: You get a 500 error
9. Check the logs
=> SUCCESS: You get a more meaningful exception:
```
[2025/02/13 13:20:00] [ERROR] GET /api/v1/biblios/1: unhandled exception (Koha::Exceptions::RelatedObjectNotFound)<<Exception 'Koha::Exceptions::RelatedObjectNotFound' thrown 'The requested related object does not exist' with accessor => biblioitem, class => Koha::Biblioitem>>
```
10. Run the tests:
k$ prove t/db_dependent/Koha/Biblio.t \
t/db_dependent/api/v1/biblios.t
=> SUCCESS: They pass!
11. Sign off :-D
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 03ae61e9f8)
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
prepare_cgi_additional_field_values needs to consider a field value as empty, because a MARC get field will always be submitted empty, and then picked up and handled by set_additional_fields accordingly.
But because we don't want to polute the database with tons of empty additional fields if many empty repeatable fields were submitted in the UI, the check for empty value still happens, but only after the MARC get check has happen, not before.
Signed-off-by: Baptiste <baptiste.wojtkowski@biblibre.com>
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit f9561c59c2)
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
Add a new hook elasticsearch_to_document in order to allow completing
the 'document' to be sent to Elasticsearch. Usage example: (1) add new
search fields, (2) normalize/sanitize existing fields, (3) use id fields
to retrieve info via web services in other repositories (say VIAF), (4)
download file referenced in a field, and index its content.
To test:
1. Apply the unit tests patch, and run it:
prove t/db_dependent/Koha/Plugins/Biblio_and_Items_plugin_hooks.t
=> FAIL: Tests fail! The hook is not implemented so the desired
2. Apply this patch, and then repeat 1:
=> SUCCESS: It works!
Signed-off-by: Shi Yao Wang <shi-yao.wang@inlibro.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit e0b829bd66)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This patch suggests to deal with connection issues at
Koha::BackgroundJob->connect level.
It incorrectly returned a Net::Stomp object when the connection failed.
With this patch, if the CONNECT does not return a CONNECTED frame then
Koha::BackgroundJob->connect returns nothing and the caller is not
fooled.
It also fixes the about page, we now have:
"Using SQL polling (Fallback, Error connecting to RabbitMQ)"
instead of
"Using RabbitMQ"
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit c407ce021e)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This change causes es_indexer_daemon.pl to die on ERROR frames instead
of spinning in an infinite loop. It also adds a content-type STOMP frame header
to help consumers know what data type is in the MESSAGE frame body.
To test ERROR handling for es_indexer_daemon.pl:
1. Switch to using Elasticsearch
2. Change your RabbitMQ connection details in koha-conf.xml or the hard-coded ones in Koha/BackgroundJob.pm
to include an invalid password. For example, if you don't have a message_broker already defined, use the following:
<message_broker>
<password>bad</password>
</message_broker>
3. sudo koha-es-indexer --restart kohadev
4. View the errors in the logs:
tail -f /var/log/koha/kohadev/es-indexer-output.log
tail -f /var/log/koha/kohadev/es-indexer-error.log
To test MESSAGE handling for es_indexer_daemon.pl:
1. Switch to using Elasticsearch (and ensure RabbitMQ connection details are correct)
2. Change the title of a bib record to something new and unique
3. Try to search for the bib using the new title
4. Note you can find it
5. Look in the logs just to make sure there are no messages there
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 2c1314ead6)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Bug 35655 needed to mock the config, but instead introduced new env variables.
Test plan:
prove t/db_dependent/Koha/BackgroundJob.t
should still return green
Bonus 1: no more warning
t/db_dependent/Koha/BackgroundJob.t .. 1/6 Cannot connect to broker Failed to connect: Error connecting to not_localhost:99999: Invalid argument at /usr/share/perl5/Net/Stomp.pm line 27.; giving up at /usr/share/perl5/Net/Stomp.pm line 27.
Bonus 2: tests are passing even if you have 'message_broker' in the
config
To test this, add the following entry to $KOHA_CONf then prove again
<message_broker>
<hostname>localhost</hostname>
<port>61613</port>
<username>guest</username>
<password>guest</password>
<vhost></vhost>
</message_broker>
=> Without this patch the tests fail because the config will overwrite
the env vars.
Signed-off-by: Magnus Enger <magnus@libriotech.no>
Warning goes away and prove works even with the message_broker config.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 15a272e1e1)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This patch adds a trigger into the anonymize function of
Koha::Old::Checkouts such that we pass along the anonymize action to
the renewals record.
Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 4f77ec8387)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This patch adds a trigger into the anonymize function of
Koha::Old::Checkout such that we pass along the anonymize action to the
renewals record.
Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit e6d0df1156)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
[EDIT] Replaced borrowers in the ->request POD by borrower
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit ff2fda90d4)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit f5f5fded2d)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
t/db_dependent/Letters.t .. 95/104 Use of uninitialized value in string eq at /kohadevbox/koha/Koha/Patron.pm line 389.
contactname can be NULL in DB.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 45493d0f06)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit 1f2dffb43f)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
t/db_dependent/Koha/Installer.t .. 1/4
Use of uninitialized value $db_version in numeric eq (==) at
/kohadevbox/koha/Koha/Installer.pm line 32.
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 1974dd9c91)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit 8e93b7c900)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Test plan:
1. perldoc Koha/Acquisition/Order/Claims.pm
2. Check that it mentions Koha::Acquisition::Order::Claims
3. perldoc Koha/CoverImages.pm
4. Check that it mentions Koha::CoverImages.pm
4. These files no longer mention 'Koha::Cities'
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit f72a235de0)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit 8e0832fd96)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Using perl -MCarp::Always:
Argument "" isn't numeric in numeric eq (==) at /usr/share/perl5/DBIx/Class/Row.pm line 1017, <$handle_1> line 2.
DBIx::Class::Row::_eq_column_values(Koha::Schema::Result::Borrower=HASH(0x57c409b9ec08), "autorenew_checkouts", "", 1) called at /usr/share/perl5/DBIx/Class/Row.pm line 945
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 3aa81fa654)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit dcc3748bdd)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Use of uninitialized value $csvkeycol{"dateexpiry"} in array element at /kohadevbox/koha/Koha/Patrons/Import.pm line 310, <$handle_2> line 2.
Use of uninitialized value $csvkeycol{"dateexpiry"} in array element at /kohadevbox/koha/Koha/Patrons/Import.pm line 310, <$handle_3> line 2.
Use of uninitialized value $csvkeycol{"dateexpiry"} in array element at /kohadevbox/koha/Koha/Patrons/Import.pm line 310, <$handle_4> line 2.
Use of uninitialized value $csvkeycol{"dateexpiry"} in array element at /kohadevbox/koha/Koha/Patrons/Import.pm line 310, <$fh> line 2.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit e21d6c04a9)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit 88ae9b2c1f)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Use of uninitialized value in string comparison (cmp) at /kohadevbox/koha/Koha/SearchEngine/Elasticsearch/Search.pm line 536.
I think this is the correct fix. We generate a random av category that
is used in the data, so $label get undef:
521 $label = $authorised_values{$t};
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit d6cbe0a25e)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit 9f6088d48b)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
02:28:36 koha_1 | WARNING: The key derivation method "opensslv1" is deprecated. Using -pbkdf=>'pbkdf2' would be better.
02:28:36 koha_1 | Pass -nodeprecate=>1 to inhibit this message.
02:28:36 koha_1 | at /kohadevbox/koha/Koha/Patron.pm line 3216.
We should deal with that on a separate bug report. For now we can remove
the warnings from the log/output.
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 12e4e92e53)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit d0727bab0d)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 4609ffd715)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This patch makes the method return an actual array instead of a
comma-separated string. The API spec is adjusted so it also expects an
array in the response.
Tests are adjusted for the new return value.
To test:
1. Apply this patch
2. Run:
$ ktd --shell
k$ yarn api:bundle
k$ prove t/db_dependent/Koha/Item.t
=> SUCCESS: Tests pass!
3. Sign off :-D
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit ffab355be0)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
When StripWhitespaceChars is enabled and the user by chance enters
some white spaces in the input field, Koha generates empty subfield
(or even entirely empty field).
Test plan:
==========
1. Enable StripWhitespaceChars system preference.
2. Edit an existing record, putting just some spaces into an empty
subfield. Save the record.
3. Go to bibliographic record Normal view and in modal "MARC preview"
notice that the subfield you entered spaces in exists as an empty
subfield. If it was the only subfield in a field, the field exists
with no data at all. This is wrong.
4. Apply the patch ; restart_all.
5. Repeat 2. and 3. You should not see any empty subfield now. If the
empty subbfield was the only subfield in a field, the field should
have been removed from the record.
Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit d0ce815600)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit f48cb7a0f0)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
The previous commit introduced a bug where the control number subfield 'w'
was being added to the MARC field even when undefined, causing test failures.
This patch adds a condition to only push the control number subfield when
the value is actually defined, matching the pattern used for other subfields
in the method.
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 32860c2778)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit ad7a9355a3)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This patch updates the use of a hash to store and then pass subfields to
the MARC::Field new method to an array with key/value pairs using fat comma's.
This allows us to maintain the integrity check that we're passing string
as the first arguament in each pair whilst also allowing us to ensure
fields are passed in the order they are found.
Sponsored-by: Open Fifth <https://openfifth.co.uk>
Signed-off-by: Heather Hernandez <flyingendpaper@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 3d9778b2cd)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit ac2ca31d87)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
When a hold's pickup location is changed we should rebuild the holds queue.
To test:
1 - Enable RealTimeHoldsQueue
2 - Enable LocalHoldsPriority where pickup library matches home library
3 - Place two biblio level holds on a bib with one item for two different patrons for delivery to a different library then the home library
4 - Check the queue - firstpatron should have priority
5 - Change the pickup location for the second hold to be picked up at the home library
6 - Check the queue, unchanged
7 - Change the pickup location back
8 - Apply patch, restart all
9 - Change the pickup location to the items home library again
10 - Check the queue
11 - Second patron has now been given priority
Signed-off-by: Michelle Spinney <mspinney@clamsnet.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 3664281d37)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit 87043c9f53)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Test plan:
Read patch.
Run t/db_dependent/Koha/Quotes.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 0ed2b7cfa8)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit 816b4537e9)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Test plan:
Hit cataloguing/addbiblio.pl?frameworkcode=FA
Without this patch you get warnings in the Koha log file:
Use of uninitialized value in string eq at /kohadevbox/koha/cataloguing/addbiblio.pl line 572.
Use of uninitialized value in substitution (s///) at /kohadevbox/koha/Koha/UI/Form/Builder/Biblio.pm line 55.
with this patch applied: no warning
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 0ba11f98f9)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit fd6552957e)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Test plan:
prove t/db_dependent/Koha/AdditionalContents.t
should return green
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 827ad21244)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit ef3ee7d69c)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Make it consider the possibility of a patron not existing.
This ensures that placing a request with partners does not error and works as expected even if the ILL request's borrowernumber is undef
To test:
1) Run the test file before and after applying this patch:
prove t/db_dependent/Koha/ILL/Requests.t
To test (ILL request is kept after borrower is deleted):
1) Apply all patches except this one
2) Enable ILLModule and create an ILL request.
3) Add a type, valid cardnumber and library.
4) Delete the associated borrower.
5) Confirm the ILL request is still listed and shown as expected
Continue test, confirm follow-up bug is fixed:
6) Click 'Place request with partners'
7) You get a ERR_TOO_MANY_REDIRECTS error
8) Apply this patch and restart plack. Repeat.
9) Confirm the place request with partners libraries page is shown correctly
Sponsored-by: UKHSA - UK Health Security Agency
Signed-off-by: Jeremy Evans <Jeremy.Evans@ukhsa.gov.uk>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit e95ec209c1)
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
Due to changes made by the commit 0472b56a67, when editing an
existing bibliographic record a default value is inserted while it
should not, according to ApplyFrameworkDefaults systempreference.
Test plan:
==========
1. Verify that ApplyFrameworkDefaults systempreference is set to
‘when cataloguing new records only’.
2. Take any existing bibliographic record, e.g. sn:1. Open it in
standard editor and identify a non used (empty) subfield, e.g. 024 $a.
Note the framework (Books, Booklets, Workbooks). Close the editor
(Cancel).
3. Modify the framework in question (Books, Booklets, Workbooks) by
setting a default value for the chosen subfield (e.g. 024 $a).
4. Open the record in the editor again. Note that the 024 $a subfield
has now an inserted default value while it should not, according to
the ApplyFrameworkDefaults setting. Do not save the record, chose
Cancel.
5. Apply the patch ; restart_all.
6. Repeat p. 4 -- there should be no default value inserted in the
existing record now.
Sponsored-by: Pontificia Università di San Tommaso d'Aquino (Angelicum)
Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit cfe3421891)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This fixes checkout fields being made available to the public API.
Note: This depends on the fix at Bug 39313, so make sure you apply that
first.
To test:
1. Enabled OpacTrustedCheckout system preference
2. Search for an item and get the barcode
3. Go to the OPAC
4. Click the Self-checkout button at the top
5. Enter the barcode and submit
6. Notice that the due date does not show in the modal, even though the
due date was correctly set
7. Apply the patch and restart services
8. Go to the staff interface and check in your barcode
9. Repeat steps 3-5
10. Confirm the due date does show correctly in the modal
11. Confirm tests pass t/db_dependent/Koha/Checkout.t
Sponsored-by: Reserve Bank of New Zealand
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 53c58f7cef)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This patch adds handling for the 264 field in precedence to the 260 for
the generate_marc_host_field method and also adds subfields n and p to
the 245 handling.
Test plan
1) The routine is currently only called during bundle linking so...
1a) Creating a new bundle
* Add a new bib record
* Mark the bib record as a 'collection' type by setting leader
position 7 to 'c'
* Add a new item to this bib record
* You should see a new 'Manage bundle' button available in the
'Actions' column of the Holdings table.
* Clicking 'Manage bundle' should expand the table to include a new
row directly beneath this one.
* Use the new 'Add to bundle' button that appears in this row to
trigger a modal that allows entering the barcode of items you wish
to add to the bundle
* Ensure the 'Add MARC link' checkbox is checked to create 773 links
and trigger the code above
2) Inspect the MARC record for the host biblio and confirm the 773 has
been built.
3) Run unit tests to confirm they now pass
Signed-off-by: Lin Wei Li <lin-wei.li@inLibro.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 300247d2c6)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit b48f5c432d)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Even if severity does not exist. Additionally, only add severity to message if it exists.
Test plan:
1) Apply tests patches. Run tests (they should fail):
prove t/db_dependent/Koha/BackgroundJobs/ErmSushiHarvester.t
2) Apply this patch and run tests again (they should pass this time).
UI test:
1) Create a new data provider linked to Gale Cengage, service url:
https://api.siqcloud.online/counterapi/r51
2) Add report release 5, customer id: '123', requestor id: '123', report types: 'TR_J1'
3) Run a report for this data provider, from Feb 2025 to Apr 2025. Notice that before applying patches, the job report results in a red bar with no error message.
4) Apply patches, repeat 3. Notice it now results in a nice error message.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 023f935360)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit b801888856)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Set these sysprefs:
- FinesMode: Calculate and Charge
- WhenLostForgiveFine: Forgive
- WhenLostChargeReplacementFee: Charge
In circ rules:
- Have a rule that charges fines
- Set Refund lost item replacement fee to "Refund lost item charge and charge new overdue fine"
1 - Have/create item with replacement price
2 - Check item out to patron 1, due date in past
3 - Run fines.pl
4 - confirm fines on patron 1
5 - mark item lost
6 - confirm replacement cost generated and fine forgiven on patron 1
7 - check item in
8 - confirm replacement cost forgiven and new fine generated on patron 1
9 - check item out to patron 2, due date in future
10 - check item in
11 - mark item lost (no replacement fee generated because item is not checked out)
12 - in database, update date_due for checkout to patron 2, set to a date in the past
13 - check item in
14 - confirm patron 2 now has a fine
15 - Apply patches, restart all
16 - Repeat test plan with another item and confirm no fine is added for patron 2
Note: At step 12 we're pushing the date_due of the old issue into the past to make date_due<curdate(). In production, this comes up when there are days between steps 10 and 13.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 67c95addeb)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit 9c81bdf529)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This patch alters the query for checking previous checkouts to use
IN instead of a list of OR for itemnumbers.
To test:
1 - On the command line:
export DBIC_TRACE=1
2 - perl -e 'use Koha::Patrons;my $patron = Koha::Patrons->find(5);my $item = Koha::Items->find(54);warn $item->itemnumber;$patron->do_check_for_previous_checkout( $item->unblessed );'
3 - Note the query
4 - Apply patch
5 - Repeat 2
6 - Note the shorter query
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 64ec35b2a5)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
(cherry picked from commit 8b044defb5)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This change turns off ABSOLUTE and EVAL_PERL for user-entered templates,
as these options can allow for arbitrary code execution and information
disclosure.
Test plan:
0. Apply the patch
1. koha-plack --restart kohadev
2. Create a Notice for Reports with a report like:
[% IF ( x == 1 ) %]
One
[% ELSE %]
Two
[% END %]
3. Run a report using this template
4. The report should show "Two"
5. Turn on CHECKOUT emails for a test patron
6. Perform a checkout
7. Look at the CHECKOUT email and note that the email body
is correctly generated
Signed-off-by: Magnus Enger <magnus@libriotech.no>
Would be good to have this fixed as the default.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
The test plan should include PERL blocks or absolute paths! Tested those
instead. Testing x==1 seems irrelevant.
I tried:[% PERL %]print 1;[% END %] and [% INCLUDE /app/Dockerfile_1 %]
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
To test:
1. Have a MRC file with price populated in the 020$c
2. Have nothing the MarcFieldsToOrder or MarcItemFieldsToOrder sys prefs
3. Create a new basket and import from a new file
4. The price is not imported.
5. APPLY PATCH and restart_all
6. Try again, this time the price should be populated correctly.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit da3d4b72dd)
This patch adds deleted_biblio to the patron holds api and adjsuts the display to
shwo the data for deleted items
To test:
- place a hold
- cancel the hold
- look in patron hold history, see the title
- delete the biblio
- reload patron hold history
- you see a line with no title or author
- apply patch, yarn build, restart_all
- reload patron hold history
- success, you see deleted title info
- enable OPACHoldsHistory preference
- sign in to OPAC as patron
- confirm holds history shows deleted title info
Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 8a628fd131)
To test:
1. In k-t-d try loading both member/moremember.pl and circ/circulation.pl for a patron with many guarantors.
2. Note the load times.
3. APPLY PATCH
4. Load times should now be reduced pretty significantly.
5. prove -v /kohadevbox/koha/t/db_dependent/Koha/Patron.t
6. No behavior should change.
Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 8bd72af1ff)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
This patch correct the call to filter the record
To test:
1 - Enable OAI
2 - Set OAI-PMH:ConfFile to '/var/lib/koha/kohadev/OAI.yaml'
3 - Edit/create the file above to have content:
format:
marcxml:
metadataPrefix: marcxml
metadataNamespace: http://www.loc.gov/MARC21/slimhttp://www.loc.gov/standards/marcxml/schema/MARC21slim
schema: http://www.loc.gov/MARC21/slimhttp://www.loc.gov/standards/marcxml/schema/MARC21slim.xsd
include_items: 1
expanded_avs: 1
4 - save and restart_all
5 - View some records:
http://localhost:8080/cgi-bin/koha/oai.pl?verb=ListRecords&metadataPrefix=marcxml
6 - Note the items show coded values
7 - Apply the regressions tests patch
8 - Run:
$ ktd --shell
k$ prove t/db_dependent/Koha/OAI/Server/Repository.t
=> FAIL: Tests fail!
9 - Apply this patch
10 - Restart all
11 - Refresh and confirm items now have descriptions
12 - Repeat 8
=> SUCCESS: Tests pass!
Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Edit (tcohen): Amended the test plan to include the added regression
tests run
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 6130a072d3)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
This patch swaps biblio for biblioitem attributes in the return value.
The effect is that attributes from the `biblio` table overwrite those
named the same (`timestamp` being the target).
To test:
1. Apply the regression tests patch
2. Run:
$ ktd --shell
k$ prove t/db_dependent/api/v1/biblios.t
=> FAIL: The biblioitems.timestamp is returned!
3. Apply this patch
4. Repeat 2
=> SUCCESS: Things go smooth now
5. Sign off :-D
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jesse Maseto <jesse@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit f69aab3735)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Koha::REST::V1::Auth checks the request URL path and do various things
depending on how it looks.
For instance, it allows everyone to access paths starting with
"/api/v1/oauth/"
But because of how Koha::REST::V1 was written, when using mojolicious
applications Koha::App::Intranet and Koha::App::Opac, paths had to add a
path prefix ("/api"), which means the final path as seen by
Koha::REST::V1::Auth looked like this: "/api/api/v1/oauth".
I said "had to", but actually there is another way that does not require
this path manipulation and that's what this patch does:
Koha::REST::V1 now accepts a configuration parameter that allows to
change the base path for API routes, which allows
Koha::App::Plugin::RESTV1 (used by Koha::App::Intranet and
Koha::App::Opac) to generate the right routes.
This configuration parameter defaults to "/api/v1", so when outside of
Koha::App::Intranet and Koha::App::Opac (when using
debian/templates/plack.psgi for instance), the behavior is unchanged.
Test plan:
1. Do not apply the patch yet
2. Run `bin/intranet daemon -l http://*:8080`
3. Run `curl -i -d{} http://127.0.0.1:8080/api/v1/oauth/token`
It should return a 403 error, with an error message "Authentication
failure".
4. Stop `bin/intranet daemon -l http://*:8080` by hitting Ctrl-C on the
terminal you started it
5. Apply the patch
6. Run `bin/intranet daemon -l http://*:8080` again
7. Run `curl -i -d{} http://127.0.0.1:8080/api/v1/oauth/token`
This time it should return a 400 error with an error message saying
the "grant_type" property is missing.
This error is normal as we did not send any data in the POST request
body, and seeing this means Koha allowed us to use that route because
it recognized '/api/v1/oauth/' at the start of the URL path
8. You can do the same test with `bin/opac`
Signed-off-by: Magnus Enger <magnus@libriotech.no>
Works as advertised. See Bugzilla for notes from testing.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 751391cc5c)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
This patch makes the `POST /patrons/:patron_id/account/credits` endpoint
return a valid `Location` header. Tests are added.
Bonus: the debits counterpart already returned the header, but pointed
to a non-existent endpoint (now added by bug 38931) and it gets tests
added.
To test:
1. Apply the regression tests patch
2. Run:
$ ktd --shell
k$ prove t/db_dependent/api/v1/patrons_accounts.t
=> FAIL: The behavior is not implemented for credits.
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 4d6aef19cc)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
This patch makes the POST endpoints missing the Location header return
it.
Some endpoints I skipped:
* /patrons/account/credits (doesn't have a GET route to point to)
* /patrons/account/debits (doesn't have a GET route to point to)
To test:
1. Apply the regression tests
2. Run:
$ ktd --shell
k$ prove t/db_dependent/api/v1/authorities.t \
t/db_dependent/api/v1/biblios.t \
t/db_dependent/api/v1/clubs_holds.t \
t/db_dependent/api/v1/holds.t \
t/db_dependent/api/v1/ill_batchstatuses.t \
t/db_dependent/api/v1/import_batch_profiles.t \
t/db_dependent/api/v1/transfer_limits.t
=> FAIL: Tests fail! The endpoints don't return the header!
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit eab06ab69b)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Tomas Cohen Arazi
Pedro Amorim
Pedro Amorim
Frédéric Demians
Jonathan Druart
David Cook
Martin Renvoize
Baptiste Wojtkowski
Doris Tam
Janusz Kaczmarek
Martin Renvoize
Nick Clemens
Marcel de Rooy
Laura_Escamilla
Katrin Fischer
Aleisha Amohia
Tadeusz „tadzik” Sośnierz
Lucas Gass
Julian Maurice