| Summary: | Javascript used for tags handling wants access to CGISESSID cookie | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Galen Charlton <gmcharlt> |
| Component: | Architecture, internals, and plumbing | Assignee: | Galen Charlton <gmcharlt> |
| Status: | CLOSED FIXED | QA Contact: | Marcel de Rooy <m.de.rooy> |
| Severity: | minor | ||
| Priority: | P5 - low | CC: | m.de.rooy |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| GIT URL: | Initiative type: | --- | |
| Sponsorship status: | --- | Comma delimited list of Sponsors: | |
| Crowdfunding goal: | 0 | Patch complexity: | Small patch |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: | ||
| Circulation function: | |||
| Bug Depends on: | |||
| Bug Blocks: | 9102 | ||
| Attachments: |
bug 9401: remove direct reads of CGISESSID cookie by JavaScript
bug 9401: remove direct reads of CGISESSID cookie by JavaScript |
||
|
Description
Galen Charlton
2013-01-16 05:07:19 UTC
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. |