Bug 37342 - CSRF error - Cannot add new authorities from basic editor with 'Link authorities automatically'
Summary: CSRF error - Cannot add new authorities from basic editor with 'Link authorit...
Status: Needs documenting
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Nick Clemens (kidclamp)
QA Contact: Emily Lamancusa (emlam)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-07-12 12:39 UTC by Nick Clemens (kidclamp)
Modified: 2024-10-08 18:19 UTC (History)
7 users (show)

See Also:
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 (1.55 KB, patch)
2024-07-12 12:42 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 37342: Use get for launching authority popup (1.62 KB, patch)
2024-07-12 13:06 UTC, Jake Deery
Details | Diff | Splinter Review
Bug 37342: Use get for launching authority popup (1.68 KB, patch)
2024-07-16 18:00 UTC, Emily Lamancusa (emlam)
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-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