Bug 37342

Summary: CSRF error - Cannot add new authorities from basic editor with 'Link authorities automatically'
Product: Koha Reporter: Nick Clemens (kidclamp) <nick>
Component: CatalogingAssignee: Nick Clemens (kidclamp) <nick>
Status: Needs documenting --- QA Contact: Emily Lamancusa (emlam) <emily.lamancusa>
Severity: normal    
Priority: P5 - low CC: dcook, emily.lamancusa, fridolin.somers, jake.deery, lucas, m.de.rooy, maxeinergl
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.11.00,24.05.04
Circulation function:
Attachments: Bug 37342: Use get for launching authority popup
Bug 37342: Use get for launching authority popup
Bug 37342: Use get for launching authority popup

Description Nick Clemens (kidclamp) 2024-07-12 12:39:48 UTC
To recreate:
1 - Go to cataloging -> new record
2 - If not in basic editor, switch
3 - Go to tab 100
4 - Enter a name that has no authority in your system: McNotExist, Falsy
5 - Click 'Link authorities automatically'
6 - subfield 9 should turn red, and have an X and a + button
7 - Click the green +
8 - Boom
Comment 1 Nick Clemens (kidclamp) 2024-07-12 12:42:19 UTC
Created attachment 168901 [details] [review]
Bug 37342: Use get for launching authority popup

This patch simply switches to a GET as the operation is not cud

To test:
 1 - Go to cataloging -> new record
 2 - If not in basic editor, switch
 3 - Go to tab 100
 4 - Enter a name that has no authority in your system: McNotExist, Falsy
 5 - Click 'Link authorities automatically'
 6 - subfield 9 should turn red, and have an X and a + button
 7 - Click the green +
 8 - Boom
 9 - Apply patch
10 - Reload page and repeat (clear cache so js reloads)
11 - Success! Authority window launches
Comment 2 Jake Deery 2024-07-12 13:06:13 UTC
Created attachment 168903 [details] [review]
Bug 37342: Use get for launching authority popup

This patch simply switches to a GET as the operation is not cud

To test:
 1 - Go to cataloging -> new record
 2 - If not in basic editor, switch
 3 - Go to tab 100
 4 - Enter a name that has no authority in your system: McNotExist, Falsy
 5 - Click 'Link authorities automatically'
 6 - subfield 9 should turn red, and have an X and a + button
 7 - Click the green +
 8 - Boom
 9 - Apply patch
10 - Reload page and repeat (clear cache so js reloads)
11 - Success! Authority window launches

Signed-off-by: Jake Deery <jake.deery@ptfs-europe.com>
Comment 3 Jake Deery 2024-07-12 13:06:56 UTC
Hi Nick,

Able to reproduce, and the patch has the expected outcome. Signed off with thanks!

Jake.
Comment 4 Jake Deery 2024-07-12 14:50:24 UTC
Hi Nick,

Looks good to me, happy to sign this one off and over to QA..! Signed off with thanks.

Jake.
Comment 5 Jake Deery 2024-07-12 14:51:01 UTC
Ugh, how come I refreshed that and couldn't see my own comment..! Sorry for the noise folks.
Comment 6 Emily Lamancusa (emlam) 2024-07-16 18:00:29 UTC
Created attachment 169055 [details] [review]
Bug 37342: Use get for launching authority popup

This patch simply switches to a GET as the operation is not cud

To test:
 1 - Go to cataloging -> new record
 2 - If not in basic editor, switch
 3 - Go to tab 100
 4 - Enter a name that has no authority in your system: McNotExist, Falsy
 5 - Click 'Link authorities automatically'
 6 - subfield 9 should turn red, and have an X and a + button
 7 - Click the green +
 8 - Boom
 9 - Apply patch
10 - Reload page and repeat (clear cache so js reloads)
11 - Success! Authority window launches

Signed-off-by: Jake Deery <jake.deery@ptfs-europe.com>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 7 Katrin Fischer 2024-07-18 14:09:49 UTC
Fixed this QA script complaint:

 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tt
   FAIL	  forbidden patterns
		forbidden pattern: simple-quote string (line 2461)

data-confirmation-msg="[% t('Are you sure you want to delete this report?') 

This is about translations. Some languages (French?) need to use the ' in their words, so they would break the JavaScript. There is an existing coding guideline to wrap all translatable strings into double quotes to avoid that issue.

So please don't ignore them :)
Comment 8 Katrin Fischer 2024-07-18 14:20:23 UTC
Hm, I fixed one occurence, but this confuses me too much, please provide a follow-up :)

            $("body").append('<div id="preview-sql-modal" tabindex="-1" role="dialog" aria-hidden="true" class="modal">\
                <div class="modal-dialog">\
                    <div class="modal-content">\
                        <div class="modal-header" style="min-height:40px;">\
                            <button type="button" class="closebtn" data-dismiss="modal" aria-label="Close">\
                                <span aria-hidden="true">×</span>\
                            </button>\
                            <h4 class="modal-title">' + title + '</h4>\
                        </div>\
                        <div class="modal-body"><textarea id="code' + reportid + '">' + message + '</textarea>\
                        </div>\
                        <div class="modal-footer">\
                            <a id="preview-modal-editreport" class="btn btn-default" href="/cgi-bin/koha/reports/guided_reports.pl?id=' + reportid + '&amp;op=edit_form"><i class="fa-solid fa-pencil" aria-hidden="true"></i> ' + _("Edit") + '</a>\
                            <a id="preview-modal-duplicate" class="btn btn-default" href="/cgi-bin/koha/reports/guided_reports.pl?op=duplicate&amp;id=' + reportid + '"><i class="fa fa-copy"></i> ' + _("Duplicate") + '</a>\
                            <a id="preview-modal-duplicate" class="btn btn-default" href="/cgi-bin/koha/tools/scheduler.pl?id=' + reportid + '"><i class="fa-solid fa-clock"></i> ' + _("Schedule") + '</a>\
                            <a class="btn btn-default submit-form-link" href="#" data-id="' + reportid + '" data-action="guided_reports.pl" data-method="post" data-op="cud-delete" data-confirmation-msg="' + _('Are you sure you want to delete this report?') + '"><i class="fa fa-trash-can"></i> Delete</a>\
                            <a id="preview-modal-runreport" class="btn btn-default" href="/cgi-bin/koha/reports/guided_reports.pl?id=' + reportid + '&amp;op=run"><i class="fa fa-play"></i> ' + _("Run report") + '</a>\
                            <a href="#" id="preview-sql-modal-cancel" data-dismiss="modal" class="btn btn-default"><i class="fa fa-times" aria-hidden="true"></i> ' + _("Close") + '</a>\
                        </div>\
                    </div>\
                </div>\
            </div>');
Comment 9 Emily Lamancusa (emlam) 2024-07-18 14:32:48 UTC
I think the above comments are meant for a different bug? This one doesn't touch guided_reports_start.tt or any translatable strings.
Comment 10 Katrin Fischer 2024-07-18 14:36:17 UTC
Thanks Emily, it's good I am on vacation next week (breaking all the things today...)
Comment 11 Emily Lamancusa (emlam) 2024-07-18 14:58:46 UTC
No problem! I was worried it was me who was breaking things and forgetting to run the qa script for a minute :)
Comment 12 Katrin Fischer 2024-07-18 15:57:36 UTC
Pushed for 24.11!

Well done everyone, thank you!
Comment 13 Lucas Gass (lukeg) 2024-09-03 14:44:07 UTC
Backported to 24.05.x for upcoming 24.05.04
Comment 14 Fridolin Somers 2024-09-17 13:49:41 UTC
Not for 23.11.x