Some Javascript used for processing bib record tags in the OPAC and staff interface wants to read the value of the CGISESSID cookie. If it actually needed to, that would be a blocker to making the session cookie be httpOnly (see bug 9102). Fortunately, as it doesn't actually need to know the session key, it suffices to remove the offending code.
Created attachment 14617 [details] [review] bug 9401: remove direct reads of CGISESSID cookie by JavaScript Having embedded JavaScript read the session cookie directly is unnecessary and prevents the CGISESSID cookie being marked httpOnly as a security measure. The only Koha JS attempting this was the AJAX tags code. To test: - In general, verify that there are no regression withs adding tags in the OPAC or reviewing them in the staff interface. - In specific, for the OPAC - log into the OPAC - retrieve a bib record - add a tag - refresh the bib details page to verify that the tag was added - make sure the TagsInputOnList syspref is on - perform a search - add a tag to more than one record from the search results page - repeat the preceding using the CCSR theme - And in the staff interface - Go to the review tags tool - Reject a tag - Refresh to verify that the tag was rejected Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Created attachment 14998 [details] [review] bug 9401: remove direct reads of CGISESSID cookie by JavaScript Having embedded JavaScript read the session cookie directly is unnecessary and prevents the CGISESSID cookie being marked httpOnly as a security measure. The only Koha JS attempting this was the AJAX tags code. To test: - In general, verify that there are no regression withs adding tags in the OPAC or reviewing them in the staff interface. - In specific, for the OPAC - log into the OPAC - retrieve a bib record - add a tag - refresh the bib details page to verify that the tag was added - make sure the TagsInputOnList syspref is on - perform a search - add a tag to more than one record from the search results page - repeat the preceding using the CCSR theme - And in the staff interface - Go to the review tags tool - Reject a tag - Refresh to verify that the tag was rejected Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
QA Comment: Welcome addition to the cookies HTTPOnly patch! Tested adding tags, accepting and rejecting (toggling TagsModeration as well). Works fine still. Note that the session parameter from the javascript already was a plan-B-measure in the perl code, but it should not be. This patch nicely removes that. Code looks good to me. Just one remark (had to find something :) Line 279 (and below) still mentions CGISESSID as ajax post parameter in comments.. (opac-tags.pl) Passed QA
This patch has been pushed to Master. Tagging worked for me both before and after applying this patch on top of the patch for bug 9102. I am not entirely sure why. However, since it worked after, I am pushing the patch anyway since it removes unnecessary cruft.
(In reply to comment #4) > This patch has been pushed to Master. Tagging worked for me both before and > after applying this patch on top of the patch for bug 9102. I am not > entirely sure why. However, since it worked after, I am pushing the patch > anyway since it removes unnecessary cruft. Hi Jared Tagging worked since the session id from javascript was only plan-B in the script.