Commit Graph

55212 Commits

Author SHA1 Message Date
Jonathan Druart fac259d2b9
Bug 37905: Fix parameter name
It's named enqueued_date on the API side, not enqueued_on (which is the DB
column's name)

No behaviour change expected here.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-18 14:26:02 +02:00
Jonathan Druart caca6791ce
Bug 37905: Use correct RFC3339 formatted date to the server
This patch suggests to revert the previous attempt to fix this problem.
The date was using the client-side's tz and so the "last hour" was not
matching the one from the server.

With bug 37831 with simply need to pass a rfc3339-formatted date and the
filtering will be done as expected.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-18 14:26:02 +02:00
Jonathan Druart ed0c5af59e
Revert "Bug 36217: Fix background jobs page's include_last_hour filter"
This reverts commit f5c8393480.

https://bugs.koha-community.org/show_bug.cgi?id=37905
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-18 14:26:01 +02:00
Kyle M Hall 9d8b6c6ab8
Bug 37902: (QA follow-up) Tidy Query.pm & Object.t
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-18 14:26:00 +02:00
Jonathan Druart 2f4b72532d
Bug 37902: Apply exact match for datetime
We do not want to apply "like" and do a "contains" search if a correctly
formatted date is passed (ie. starting with "YYYY-MM-DD HH:MM:SS")

It causes underlying problems if we add '%' characters to this string as
it will then become an invalid date.

There are several ways of dealing with this problem. This patch is
suggesting the easiest path: Apply an exact search (ie. do not add '%')
if the value appears to be a datetime.

Certainly not the best looking patch but it seems to be quite effective:
  * no need to change the client
  * no need to rework build_query_params, merge_q_params, attributes_from_api

We could (to confirm) pass the result set, but it seems a lot of additional processing
(that is done later already, in attributes_from_api)

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-18 14:26:00 +02:00
Jonathan Druart ba342ed6e2
Bug 37902: Add tests
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-18 14:25:59 +02:00
Jonathan Druart 95e9ae46bd
Bug 37902: Do not convert a date if it has like markers
If we pass a datetime parameter we are adding "like" operator and % at
the beginning and ending of the attribute value.
For instance:
  attributes=2024-09-16 10:11:12
  attributes:{ like => '%2024-09-16 10:11:12%' }

We do not want to reach the fixup code and raise an exception.

However I don't think we should add the like for datetime attributes
actually. But can we modify this behaviour now?

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-18 14:25:59 +02:00
Jonathan Druart b63b8e19b3
Bug 37902: Adjust timezone
We didn't take into account the server's timezone.

This does not feel right, we are not supposed to deal with time zone
outside of Koha::DateUtils.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-18 14:25:58 +02:00
Jonathan Druart c7786b419c
Bug 37902: TODOs
There are still different structures we won't handle properly.
This patch adds conditionals to prevent failures or warnings.

Should be done, but later.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-18 14:25:58 +02:00
Jonathan Druart 101fd97cb7
Bug 37902: Make sure we loop over if a structure is passed
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-18 14:25:57 +02:00
Jonathan Druart e36db00fa1
Bug 37902: Make sure filtered_params are converted
Previously we only adjusted the attributes that were passed directly,
not the ones in 'q' (which is the recommended way now)

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-18 14:25:56 +02:00
Jonathan Druart aa70b472ff
Bug 37728: Adjust test to catch more missing 'op' in POSTed forms
The test is not catching missing op in forms that are not the first POST
form of the template.

Found when fixing the test for bug 37309. It was not catching the
missing op in the second form.

Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-18 14:25:56 +02:00
Phil Ringnalda adbbbb1d8c
Bug 37728: (follow-up) Tell the test that opac-illrequests.tt has an op
xt/find-missing-op-in-forms.t wants to see name="op" value="cud-..." in any
form with method="post", but opac-illrequests.tt inserts its op input by
passing around whole and unpack and repacking whole.keys which includes
the op. We just need to tell the test that it really exists.

At first, I thought of this approach as a joke, faking out the test, but
when I tried to be more responsible and put a whole <input type="hidden" etc.
in the comment, I realized that would be more likely to confuse someone
who thought that was uncommented and that they were making real changes than
it would be to future-proof against changes in the test's approach.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-18 14:25:55 +02:00
Phil Ringnalda 1f8b6d5daa
Bug 37786: members/cancel-charge.pl needs CSRF protection
members/cancel-charge.pl will take either a POST or a GET, and as long as the
accountline_id it is passed can be cancelled, will cancel it. That means any
link you click anywhere while logged in to Koha might cancel a charge. It also
takes a borrowernumber which isn't used for the cancelling, only to determine
what account to show after a charge is cancelled, letting a malicious link
show an account other than the one whose charge was just cancelled.

Test plan:
 1. Without the patch, Circulation - Checkout - search for the 'koha' patron
    you log in as
 2. Accounting - Create manual invoice - Make it a Manual fee of 100.00 and
    Save
 3. Pretending it's a well-disguised link in a spear-phishing email, load
    http://localhost:8081/cgi-bin/koha/members/cancel-charge.pl?borrowernumber=5&accountlines_id=1
 4. You are now looking at charges for the patron Acosta, Edna rather than for
    the patron koha, but if you look at the patron koha, its 100.00 charge
    has been cancelled.
 5. Apply patch and reset_all (or if you don't, you'll have to manually adjust
    the link to reflect the charge being accountlines_id 3 rather than 1)
 6. Circulation - Checkout - search for the 'koha' patron you log in as
 7. Accounting - Create manual invoice - Make it a Manual fee of 100.00 and
    Save
 8. Click the link http://localhost:8081/cgi-bin/koha/members/cancel-charge.pl?borrowernumber=5&accountlines_id=1
 9. You got a 403 because you didn't pass the op cud-cancel, but if you did
    pass that op, you would also get a 403 for having a cud- op in a GET (and
    if you POST, you won't have a csrf_token)
10. Checkout - search for koha - Accounting - Cancel charge
11. Having done it the right way, you're now on koha's list of transactions,
    where you can see you just cancelled it

Sponsored-by: Chetco Community Public Library

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-18 14:25:55 +02:00
Jonathan Druart 824d80b6f8
Bug 38190: Remove JS error on suggestion page
This code could appear several time as we include it in modals

Test plan:
Go on http://localhost:8081/cgi-bin/koha/suggestion/suggestion.pl
Open the console
Notice that without this patch you see a JS error
  Uncaught SyntaxError: redeclaration of const av_bsort1
With this patch applied the error is gone.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-18 14:25:54 +02:00
Katrin Fischer 55c6df46b7
Bug 37528: (QA follow-up) Slight rephrase of system preference and warning message
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-18 14:25:53 +02:00
Marcel de Rooy cfc953a411
Bug 37892: (QA follow-up) Fix condition in memberentry
Patron may be undefined. So the test may crash.
Also there is an issue with operator precedence.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-18 14:25:53 +02:00
Marcel de Rooy bc68a17c54
Bug 37892: (QA follow-up) Add tests for ->is_guarantor/ee
Test plan:
Run t/db_dependent/Koha/Patron.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-18 14:25:52 +02:00
Nick Clemens 391e150e07
Bug 37892: Fix guarantor restriction, add tests
[SQUASHED IN QA]

These patches will alter the checks for a patron that prevent a category with
'can_be_guarantee' from being a guarantor.  Two patrons in the same category should be
allowed to have a guarantee/guarantor relationship

The tests below assume you are using the KTD sample data. Update borrowernumbers if not.

To test:
 0 - Apply tests patch
 1 - Set the 'Patron' category as 'Can be a guarantee'
 2 - Add a relationship between two patrons of the same category
     This is restricted from the staff interface
     perl -e 'use Koha::Patrons; my $p = Koha::Patrons->find(5)->add_guarantor({ guarantor_id => 23, relationship => 'father'});'
 3 - Note there is no warning or exception. This should be allowed.
 4 - Checkout an item to Edna (borrowernumber 5)
 5 - Set 'TrackLastPatronActivityTriggers' to 'Checking in an item'
 6 - Try to check the item in, KABOOM
 7 - Set 'TrackLastPatronActivityTriggers' to 'Checking out an item'
 8 - Try to issue an item to Enda, KABOOM
 9 - prove -v t/db_dependent/Koha/Patron.t, fail
10 - Apply second patch
11 - prove -v t/db_dependent/Koha/Patron.t, one more test passes, but then fail
12 - Apply third patch
13 - prove -v t/db_dependent/Koha/Patron.t, pass!
14 - restart_all
15 - Checkout to Enda, OK!
16 - Checkin from Edna, OK!
17 - Find two more patrons in the category and attempt to link them
18 - 'Guarantor cannot be a guarantee'
19 - Apply fourth patch
20 - You can add a guarantor from the same category in interface
21 - Try to add a guarantor to the guarantor assigned in 20
22 - Confirm you cannot add a guarantor - "Guarantor cannot be a guarantee"

Signed-off-by: Olivier V <olivier.vezina@inLibro.com>
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>

Bug 37892: Fix patron updates

Signed-off-by: Olivier V <olivier.vezina@inLibro.com>
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>

Bug 37892: Fix patron creation

Signed-off-by: Olivier V <olivier.vezina@inLibro.com>
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>

Bug 37892: Fix memberentry.pl

Signed-off-by: Olivier V <olivier.vezina@inLibro.com>
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>

Bug 37892: (follow-up) Fix patron creation

This patch fixes the 22. of the test plan (22 - Confirm you cannot add a guarantor - "Guarantor cannot be a guarantee"
        )

Signed-off-by: Olivier V <olivier.vezina@inLibro.com>
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>

Bug 37892: (follow-up) Fix patron creation

TEST PLAN:
1 - Do the 22 parts of the test plan
2 - Add a guarantor to one patron not selected before (let's say A is
  the guarantee, B the guarantor)
3 - Try and add a guarantor to B -> you will success
4 - Remove B's guarantor
5 - Apply this patch
6 - Repeat 3 -> you will not be able to

Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>

Bug 37892: (follow-up) Tidyness

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
[EDIT]
Renamed a subtest to patron creation tests in Patron.t.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-18 14:25:52 +02:00
Baptiste Wojtkowski dd1bef0ea8
Bug 37528: (follow-up) Add help strings for blank borrowerrelationship
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-18 14:25:51 +02:00
Baptiste Wojtkowski ec5d2c9ff9
Bug 37528: (follow-up) Tidyness
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-18 14:25:51 +02:00
Johanna Räisä 9ea82b05d5
Bug 37528: check if selected relationship is valid
This patch checks if the selected relationship is valid before trying to save the patron record.
It takes the list of valid relationships from borrowerRelationships syspref and checks if the selected relationship is in the list.

Also this patch fixes relationship field required message when BorrowerMandatoryField is not set.
The required message is shown when adding the guarantee from guarantor's detail page.

Test plan:
1) Add at least one option to borrowerRelationships syspref.
2) Leave the relationship unchecked from BorrowerMandatoryField syspref.
3) Create a new guarantee patron.
4) Add a guarantor to the guarantee patron.
5) Leave the relationship field empty and try to save the patron record.
6) Notice the 500 error page.
7) Apply the patch.
8) Repeat steps 3-5.
9) Notice the error message "Guarantor relationship is invalid".

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Olivier V <olivier.vezina@inLibro.com>
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-18 14:25:50 +02:00
Lucas Gass 2d28aa403a
Bug 38183: Get the active tab number from data-attribute
To test:

1. Have more than 1 staff member who can manage suggestions
2. Create some suggestions
3. Move some suggestions to different statuses (Pending/Accepted/Rejected) so you have multiple tabs on the "Suggestions management" page.
4. Go to the first tab, check some suggestions, and click "Select manager" under "Update manager".
5. Pick a new manager, notice nothing changes on that tab.
6. Now look at the last tab in your list of tabs, see the suggestion manager has been set incorrectly to the last tab.
7. APPLY PATCH
8. Try 4-5 again. The manager should be selected correctly.
9. Try setting the manaager from every tab, making sure it works right.

Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-18 14:25:49 +02:00
Emily Lamancusa 4033699062
Bug 38156: Sort issues by borrowernumber before parallel chunking
When the automatic renewal cron job is using parallel processing, it
aims to process all of the renewals for any given patron together in one
chunk to avoid data conflicts. To accomplish this, it starts a new data
chunk each time it encounters a new patron. However, if a patron's
renewing checkouts aren't all consecutive in the database, that patron's
data ends up split across multiple chunks.

We need to sort the issues by borrowernumber before attempting to chunk
them in order to make sure they are chunked and processed correctly.

To test (using KTD default test data):
Setup:
1. Edit the default circulation rule:
    - Set Automatic renewal to "Yes"
    - Set No automatic renewal before to 3
2. Open the following patron accounts in separate tabs:
    - Floyd Delgado
    - Joyce Gaines
    - Edna Acosta
    - Mary Burton
3. Perform the following patron account edits for each of the above
   patrons (and keep the tabs open):
    - Enable automatic renewal notices, and set them to digests only
    - Add a value to the email field
4. Enter the kshell (ktd --shell)
5. Edit /etc/koha/sites/kohadev/koha-conf.xml, and add the following
   lines near the end, just above the </config> and </yazgfs> closing
   tags:
<auto_renew_cronjob>
    <parallel_loops_count>2</parallel_loops_count>
</auto_renew_cronjob>
6. restart_all

Reproducing the issue:
7. Apply the test patch only
8. Run perl generate_checkouts.pl to generate test data
9. perl misc/cronjobs/automatic_renewals.pl -v -c
--> The test patch added output that will show how the renewals were
    chunked into "chunk 0" and "chunk 1" for the two parallel loops.
    Note that the issues for each borrower are not processed nicely in
    one chunk, but are separated across multiple chunks and alternated
    with other borrowers.
10. Check the checkouts for each of the four patrons from above
--> All checkouts should have renewed
11. Check the notices tab for each of the four patrons
--> Notice errors in the automatic renewal digest notices. A patron's
    renewals may be split across multiple digests, a digest may be
    missing renewals, or a patron may not have received a digest at all

Testing the patch:
12. Apply the second patch
13. Reset the due dates on all checkouts so that they will all be
    eligible for automatic renewal again:
    - koha-mysql kohadev
    - UPDATE issues SET date_due=<two days from today>;
14. perl misc/cronjobs/automatic_renewals.pl -v -c
--> Note that the renewals are now correctly chunked by patron
15. Check the checkouts and notices tab for each of the four patrons
--> All checkouts should have renewed, and all patrons should have a
    single new Auto Renewals Digest notice that correctly lists all of
    their renewed items

Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Edit: tidied the code block inline (tcohen)
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-18 14:25:49 +02:00
Hammat Wele 7cccc3c0d9
Bug 13342: Not logged user can place a review/comment
We are able to comment a notice even when no user is connected

To test

1. Log in to OPAC.
2. Find a Biblio and open the comments tab.
3. Open another tab/window, and log out from the account.
4. Return to the first tab.
5. Click "Post your comments on this title."
 --> We are able to post a comment
6. Apply the patch
7. Repeat step 1, 2, 3, 4, 5
 --> You should be redirected to the login page.

Signed-off-by: David Cook <dcook@prosentient.com.au>

Bug 13342: Tidy

Signed-off-by: David Cook <dcook@prosentient.com.au>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-18 12:07:53 +02:00
Lucas Gass 2e7f51dcfc
Bug 37720: (follow-up) Adjust tests
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-18 12:07:52 +02:00
Jonathan Druart a05d1d9be4
Bug 37720: Prevent XSS in label creator
Because labels/label-edit-batch.pl fills a DataTable with things that include
a link created by C4/Creators/Lib.pm, it outputs them with the $raw filter,
so HTML in author/title/callnumber is executed in the label batch editor.
While we wait for a fix that moves the link creation into the template and
out of C4, encoding HTML in Lib.pm for the bits going into the link, and
switching from $raw to the html filter for the rest of the things, will at
least get rid of the XSS.

Test plan:
 1. Without this patch, but with the patch from bug 37654 so you don't get
    alert()s in batch import, download attachment 170675 [details]
 2. Cataloging - Stage records for import - browse to the downloaded file -
    Upload file - when the upload finishes Stage for import - when staging
    finishes View batch (get alert()s if you didn't apply bug 37654) - Import
    this batch into the catalog
 3. Once the import finishes, Cataloging - Manage staged records
 4. In the row for your import, in the # Items column, click "(Create label
    batch)"
 5. In the "Label batch #n created" message, click the link to the batch #
 6. Because the batch includes a call number with an open <script>, you'll
    get XSS alert()s and then one about something going wrong while loading
    the table, with only one of the two records showing in the batch editor
 7. Apply patch, restart_all
 8. Cataloging - Label creator - Manage Label batches
 9. In the row for your batch, click Edit
10. You will see both labels, with their attempts at XSS visible as text
    rather than being interpreted as HTML

Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-18 12:07:51 +02:00
David Cook 4b727f38f8
Bug 37681: Fix XSS in staff interface item URLs on detail page
This patch uses Javascript objects and safe sinks to prevent XSS
in the item URLs on the staff interface detail page.

It also makes sure those URLs don't get double-escaped. Yippee!

Test plan:
0. Apply the patch
1. Add/edit an item with the following URL:
http://prosentient.com.au?q=http%3A%2F%2Fprosentient.com.au
2. Add/edit a different item with the following URLs:
http://prosentient.com.au?q=http%3A%2F%2Fprosentient.com.au |
http://prosentient.com.au?q=http%3A%2F%2Fprosentient.com.au
3. Go to the staff interface detail page
4. Notice that the URLs are not double-encoded!
5. Try out a malicious payload (talk to QA/security about this)
6. Confirm that the malicious payload fails to execute the XSS
7. Celebrate!

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-18 12:07:51 +02:00
Victor Grousset/tuxayo 3972f5460a
Bug 36598: (QA follow-up): tidy up code
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-18 12:07:50 +02:00
Julian Maurice ab84ff6407
Bug 36598: Add comments asking to keep both CSRF checks in sync
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-18 12:07:50 +02:00
Julian Maurice 36858c8c57
Bug 36598: Prevent use of unsafe HTTP method with non-cud op parameter
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-18 12:07:49 +02:00
Julian Maurice 4325e607cf
Bug 36598: Improve documentation and error message in CSRF plugin
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-18 12:07:48 +02:00
Julian Maurice f36c071f5b
Bug 36598: Fix CSRF header name (underscore -> hyphen)
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-18 12:07:48 +02:00
Julian Maurice 30670cba57
Bug 36598: Prohibit CUD operations with safe HTTP methods (GET/HEAD/...)
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-18 12:07:47 +02:00
Julian Maurice e61b92c8dd
Bug 36598: Enable CSRF protection for Mojolicious apps
Test plan:
1. Run bin/opac daemon -l http://*:3001/
2. Go to http://localhost:3001/cgi-bin/koha/opac-user.pl
3. With browser devtools, locate csrf_token hidden input within the
   login form and remove it or modify it
4. Try to submit the form with correct credentials, it should fail
   ("Wrong CSRF token")
5. Reload the page, try to log in normally without modifying the DOM, it
   should succeed
6. Run bin/intranet daemon -l http://*:3002/
7. Go to http://localhost:3002/cgi-bin/koha/mainpage.pl
8. With browser devtools, locate csrf_token hidden input within the
   login form and remove it or modify it
9. Try to submit the form with correct credentials, it should fail
   ("Wrong CSRF token")
10. Reload the page, try to log in normally without modifying the DOM,
    it should succeed
11. Run prove t/db_dependent/mojo/csrf.t

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-18 12:07:47 +02:00
Phil Ringnalda 8269ac3d1e
Bug 37654: XSS in Batch record import for Citation column
Viewing a staged MARC record batch loads a DataTable from
/tools/batch_records_ajax.pl, and both batch_records_ajax.pl and the
DataTable just trust the author/title/isbn/issn to be free of HTML. They
shouldn't.

Test plan:
1. Without this patch applied, download attachment 170418, then Cataloging
   - Stage records for import - Select the downloaded file - Upload file -
   Stage for import
2. When the background job completes, View batch - you'll get three alert()s
   from the title, author, and ISSN, and the author and ISSN displayed huge
3. Apply patch, restart_all
4. Manage staged records - click HTMLescapingimporttestrecord.mrc - get zero
   alerts and no <h2> display

Sponsored-by: Chetco Community Public Library
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-18 12:07:46 +02:00
Phil Ringnalda 05a3781cf1
Bug 37656: XSS in Advanced editor from Z39.50 search results
The Advanced editor inserts data from Z39.50 results into the search results
page without escaping HTML. Whether it's German records with "<<A>> Title"
or someone with a compromised catalog or a book with the title "<em> for
emphasis" it shouldn't.

Test plan:
 1. Not a dependency, but you'll avoid getting even more alerts while batch
    importing by starting with the patch from bug 37654
 2. Without this patch applied, download attachment 170421
 3. Administration - set the preference EnableAdvancedCatalogingEditor to
    Enable
 4. Cataloging - Stage records for import - browse to the downloaded file -
    Upload file - Stage for import
 5. Once the background job finishes, View batch (getting alerts if you
    didn't apply the patch from bug 37654) - Import this batch into the
    catalog
 6. When the import finishes, Search the catalog for script, on the imported
    record Edit record (if you wind up in the basic editor, Settings - Switch
    to Advanced editor)
 7. In the left sidebar below the search inputs, click Advanced », check
    the checkbox for Local catalog and uncheck any others, then search for
    the Title script
 8. You'll get five alerts, and the word "edition" displayed in huge text
 9. Close the search popup, apply patch, shift+reload the advanced editor
    page to clear your cache
10. Repeat step 7, but this time you won't get any alerts, and you'll see
    the title and the other <script> inclusions.

Sponsored-by: Chetco Community Public Library
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-18 12:07:46 +02:00
Phil Ringnalda ea1d3f7a01
Bug 37655: Basic editor needs to HTML-escape the bib record title used as a heading
We stick the title of a bib record you are editing in the basic editor into
an <h1> without escaping any HTML it might contain. We should instead escape
it.

Test plan:

1. Without the patch, search for any record in the catalog and click Edit
   record (if you are in the advanced editor, switch to the basic one)
2. Tab 2, Field 245, Subfield a, paste <script>alert('boo ❤')</script><h2>
   at the end of the subfield
3. Save, then from the record detail page select Edit - Edit record
4. You will have gotten an alert(), and the entire form will be the size
   of an <h2>. That's ugly, so go back to the detail page.
5. Apply patch, restart_all
6. Edit - Edit record
7. Now you should not get an alert, the whole title inluding the <script>
   should display in italics, and the "(Record number nnn)" after it should
   not be italicized.

Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-18 12:07:45 +02:00
wainuiwitikapark bbd644444f
Bug 37838: Fix broken remove button in course reserves
To test:
1) Go to staff client -> Course reserves
2) Create a course or go to an existing course
3) Add reserves to the course (will need at least 21 items to be able to go to a second page of results)
4) Go to the second page of results on the course details page
5) Click the Remove button next to a result
6) Notice how the button just makes the page move to the top
7) Apply patch
8) Repeat steps 1-5
9) Notice how the remove button is working as expected

Sponsored-by: Toi Ohomai Institute of Technology
Signed-off-by: Sam Sowanick <sam.sowanick@corvallisoregon.gov>
Signed-off-by: Laura_Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-16 17:34:48 +02:00
Emily Lamancusa ad4ce38551
Bug 38112: (QA follow-up) Hide description when search is cleared
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-16 14:36:00 +02:00
Jonathan Druart 913052d23c
Bug 38112: Restore description of patrons search
Certainly caused by bug 35329.

We used to display a description of the patrons search, but it is no longer displayed.

Test plan:
On the main patrons search select some values in the form on the left
and search. You should see a "Patrons found for: " h3 that will be
displayed and is supposed to describe the current search.
I don't think it's working very well to be honest, maybe a candidate for
candidate, especially if nobody else noticed its disappearance.

Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-16 14:36:00 +02:00
Lucas Gass dd4822e400
Bug 38162: Pass the rota_id correctly when deleting
To test:

1. Enable StockRotation
2. Got to Cataloging -> Stock rotation
3. Create a new Rota
4. Try to delete it
5. See the error "Can't call method "delete" on an undefined value at /kohadevbox/koha/tools/stockrotation.pl line 231"
6. APPLY PATCH
7. Try again, the rota should be deleted properly.

Signed-off-by: Laura ONeil <laura@bywatersolutions.com>
Signed-off-by: Alyssa <alyssa.drake@bywatersolutions.com>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-16 14:35:59 +02:00
Emily Lamancusa d60057b429
Bug 38146: Display full datetime of datelastseen in item holdings
To test:
1. Check in an item
2. Look at the item holdings table for the bib record that item is on
--> Note the date last seen column displays dates, but not the time
3. Apply patch and restart_all
4. Refresh the page
--> Date last seen column now shows the time you checked the item in

Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-16 14:35:59 +02:00
Lucas Gass 0f3d1f3daf
Bug 38005: (follow-up) Add/adjust tests
NC amended patch, tidied
Bug 38005: (QA follow-up) Tidy

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-16 14:35:58 +02:00
Nick Clemens 4ed5e6c248
Bug 38126: Skip allocated holds when filling from transport cost matrix
This patch simply weeds out allocated holds before filling from the matrix

To test:
 1 - Enable RealTimeHoldsQueue
 2 - Enable UseTransportCostMatrix
 3 - Enable LocalHoldsPriority Give/Home/Home
 4 - Administration - Transport cost matrix - enable transfers to/from Centreville and other libraries, add a cost, and save
 5 - Find a bib with a Centerville item
 6 - Place a hold for a centerville patron
 7 - Circulation - Holds queue - All libraries
 8 - Note hold is entered twice
 9 - On command line:
    perl misc/cronjobs/holds/build_holds_queue.pl --force
10 - Run holds queue again, still there twice
11 - Apply patch
12 - On command line:
    perl misc/cronjobs/holds/build_holds_queue.pl --force
    Confirm allocated only once
13 - Delete hold, place again from bib record (to test real time allocation)
14 - Confirm allocated only once

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-16 14:35:57 +02:00
Aleisha Amohia a11d62975f
Bug 38016: Rename booking circulation rules
This enhancement renames the bookings circulation rules to better describe what they do.

To test:

1. Go to Koha Administration -> Circulation and fines rules
2. Scroll to the 'Default holds and bookings policies by item type' section
3. Notice the bookings rules in this table:
  - Booking preparation period
  - Booking precaution period
4. Apply the patch and refresh the page
5. Confirm the bookings have been renamed to:
  - Booking pre-processing (days)
  - Booking post-processing (days)
6. Confirm these names make sense and it is clear what the rules are used for and how they are applied
7. Put values in the input fields for these rules and confirm saving a rule works as expected

Sponsored-by: Catalyst IT
Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-11 12:21:35 +02:00
Jonathan Druart 40f01df518
Bug 38129: Display note regarding permissions in suggestion manager search pop-up modal
1. In the staff interface, go to More > Suggestions > New purchase
   suggestion
2. Click 'Select manager'
   => With this patch you see a note regarding permissions
3. Search for 'alford' (for example)
   => User is not returned, they do not have the suggestion permission

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-11 12:21:34 +02:00
Owen Leonard 59afb14724
Bug 36742: (follow-up) Add hidden branchcode input for OPAC article requests
This patch adds a hidden input containing the logged-in user's
branchcode to the article request form when there is only one public
branch.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-11 12:21:34 +02:00
Owen Leonard 99acd9c3fd
Bug 36742: Do not show library selection options if there is one or fewer public libraries
If a system has only one library or only one library which is public we
don't need to show library-selection menus. It doesn't make sense to
show a dropdown with only one choice.

This patch updates instances of Branches.all to add a "public => 1"
parameter.

To test, apply the patch and start with a set of multiple public
libraries in your system (where public means the entry in Administration
-> Libraries for that library has the "Public" option set to "Yes").

Log in to the OPAC test these pages, in each case confirming that the
the library dropdown appears correctly.

- OPAC home page (with OpacAddMastheadLibraryPulldown enabled)
- OPAC news section (with existing news items and OpacNewsLibrarySelect
  enabled)
- Advanced search (Location and availability section)
- The "Most popular" page (with OpacTopissue enabled)
- The suggestion entry form (with suggestion enabled)
- The article request entry form (with ArticleRequests enabled and
  circulation rules configured to allow requests)

Test again with only one library or only one public library.
Test again with no public libraries.

Sponsored-by: Athens County Public Libraries
Signed-off-by: Jake Deery <jake.deery@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-11 12:21:33 +02:00
Andrew Fuerste Henry 01ebb27b5e
Bug 38097: Add class to not_retuned message
To test:
1: Enable ShowAllCheckins
2: Check in an item that was not checked out
3: Find "Item was not checked in" message in checkin table, confirm its only class is "problem"
4: apply patch, restart_all
5: repeat 2
6: confirm "Item was not checked in" now has the class "not_returned"

Signed-off-by: Eric Phetteplace <phette23@gmail.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
2024-10-11 12:21:33 +02:00