Bug 36195 - CSRF - testing reports
Summary: CSRF - testing reports
Status: NEW
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low critical
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords: collector
: 36280 (view as bug list)
Depends on: 34478
Blocks: 36192 36349
  Show dependency treegraph
 
Reported: 2024-02-28 14:02 UTC by Nick Clemens (kidclamp)
Modified: 2024-11-04 22:42 UTC (History)
10 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 36195: serials/routing-preview.pl - Fix save and preview (4.61 KB, patch)
2024-03-01 14:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 36195: Replace POST batch_records_ajax.pl with GET (1.27 KB, patch)
2024-03-08 12:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 36195: Replace POST in itemsearch.tt with GET (998 bytes, patch)
2024-03-11 17:39 UTC, Brendan Lawlor
Details | Diff | Splinter Review
Bug 36195: Replace POST in itemsearch.tt with GET (1.24 KB, patch)
2024-03-11 17:42 UTC, Brendan Lawlor
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens (kidclamp) 2024-02-28 14:02:46 UTC
This bug is for tracking follow-ups missed in the initial review. Please see the "testing reports" section on the framapad here:
https://annuel.framapad.org/p/koha_34478_remaining

Additional entries can be added there, and should be crossed out when completed

Patches should be attached here

This bug can be closed when that section of the pad is empty
Comment 1 Jonathan Druart 2024-03-01 14:11:57 UTC
Created attachment 162668 [details] [review]
Bug 36195: serials/routing-preview.pl - Fix save and preview

No idea why we are passing issue_escaped instead of the id, but this
patch fixes the regression.
Comment 2 Caroline Cyr La Rose 2024-03-05 22:08:12 UTC
Hi, I'm not sure if this is where I should report this. I added it to the framapad, too...

I can't search for items using the item search. It returns a 403: Forbidden message.

To recreate:
1. Go to Item search
2. Click Search
   --> 403: Forbidden

plack-intranet-error.log says
[2024/03/05 22:06:45] [WARN] Programming error - op 'like' must start with 'cud-' for POST at /kohadevbox/koha/Koha/Middleware/CSRF.pm line 78.
Comment 3 David Cook 2024-03-05 22:50:20 UTC
(In reply to Caroline Cyr La Rose from comment #2)
> I can't search for items using the item search. It returns a 403: Forbidden
> message.

I've looked into this one a bit (as I noticed on framapad), and I think itemsearch.pl is going to require some refactoring to fix its usage of "op". 

Basically, the "op" in the GET is fine, but it gets used for the POST to the API too. Could maybe convert that second POST to a GET, although it would be a long URL...
Comment 4 Jonathan Druart 2024-03-08 12:54:54 UTC
Created attachment 162960 [details] [review]
Bug 36195: Replace POST batch_records_ajax.pl with GET

We retrieve a list of records for DT, it does not need to be a POST
request.

Test plan:
1. Stage a batch
2. When it's done, click on "view batch"
=> Without this patch the page loads but pops an alert that says "403: Forbidden" and table stays empty
Comment 5 Jonathan Druart 2024-03-08 12:54:54 UTC
*** Bug 36280 has been marked as a duplicate of this bug. ***
Comment 6 Brendan Lawlor 2024-03-11 17:39:02 UTC
Created attachment 163049 [details] [review]
Bug 36195: Replace POST in itemsearch.tt with GET
Comment 7 Brendan Lawlor 2024-03-11 17:42:26 UTC
Created attachment 163050 [details] [review]
Bug 36195: Replace POST in itemsearch.tt with GET

We retrieve a list of records for DT, it does not need to be a POST
request.

Test plan:

1. Try an item search
2. The page loads but pops an alert that says "403: Forbidden" and table stays empty
3. Apply patch
4. Try an item search again and the table loads results
Comment 8 Katrin Fischer 2024-03-12 15:35:03 UTC
Comment on attachment 163050 [details] [review]
Bug 36195: Replace POST in itemsearch.tt with GET

>From 28f445587ee8e7f70852367f2c9e37ae2c4ebcea Mon Sep 17 00:00:00 2001
>From: Brendan Lawlor <blawlor@clamsnet.org>
>Date: Mon, 11 Mar 2024 17:32:08 +0000
>Subject: [PATCH] Bug 36195: Replace POST in itemsearch.tt with GET
>
>We retrieve a list of records for DT, it does not need to be a POST
>request.
>
>Test plan:
>
>1. Try an item search
>2. The page loads but pops an alert that says "403: Forbidden" and table stays empty
>3. Apply patch
>4. Try an item search again and the table loads results
>---
> koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/itemsearch.tt | 1 -
> 1 file changed, 1 deletion(-)
>
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/itemsearch.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/itemsearch.tt
>index b3a89e508c..db703115b3 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/itemsearch.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/itemsearch.tt
>@@ -478,7 +478,6 @@
>                 "processing": true,
>                 "ajax": {
>                     url: '/cgi-bin/koha/catalogue/itemsearch.pl',
>-                    type: 'POST',
>                     data: function ( d ) {
>                         for (i in params) {
>                             d[params[i].name] = params[i].value;
>-- 
>2.30.2
Comment 9 Katrin Fischer 2024-03-12 15:36:22 UTC
Last 3 patches pushed to master and obsoleted here. Thanks!
Comment 10 Victor Grousset/tuxayo 2024-03-12 20:37:24 UTC
I reported the issue corresponding to Bug 36280 in the pad just a day before promulgation of directive «Please file individual bugs to deal with remaining issues per RM request»

Just writing here to be sure that report didn't slip under the radar due to being right before the change in strategy for CSRF related regressions.
Comment 11 Katrin Fischer 2024-03-12 20:46:46 UTC
(In reply to Victor Grousset/tuxayo from comment #10)
> I reported the issue corresponding to Bug 36280 in the pad just a day before
> promulgation of directive «Please file individual bugs to deal with
> remaining issues per RM request»
> 
> Just writing here to be sure that report didn't slip under the radar due to
> being right before the change in strategy for CSRF related regressions.

I pushed all patches on this and the other CSRF bug today. So this is just for future bugs.
Comment 12 Fridolin Somers 2024-03-13 06:56:56 UTC
Depends on Bug 34478 no ?
Comment 13 David Cook 2024-03-13 22:15:34 UTC
(In reply to Fridolin Somers from comment #12)
> Depends on Bug 34478 no ?

Yep. I've just added as dependency.
Comment 14 Victor Grousset/tuxayo 2024-03-23 03:51:39 UTC
I see on the pad «Make sure to file as blocker for 36192»

So I might have linked bug 36349 in the wrong way. Does it mean that other bugs need be listed here in "depends on"?
Like an omnibus.
Comment 15 Fridolin Somers 2024-04-17 08:53:24 UTC
Indeed, I'm looking at graph from Bug 34478 and this dependancy looks wrong
Comment 16 Manos PETRIDIS 2024-10-08 17:44:13 UTC
Could bug https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37978  be related?
Comment 17 Victor Grousset/tuxayo 2024-10-09 23:44:39 UTC
Almost sure not: bug 37978 comment 10
Comment 18 Martin Renvoize (ashimema) 2024-11-04 14:20:43 UTC
I'm not really seeing much use of this bug.. should we close it at this point as I'm not sure it's really serving any meaningful purpose.
Comment 19 David Cook 2024-11-04 22:42:45 UTC
(In reply to Martin Renvoize (ashimema) from comment #18)
> I'm not really seeing much use of this bug.. should we close it at this
> point as I'm not sure it's really serving any meaningful purpose.

Yeah I've been thinking that lately too