Bug 9401

Summary: Javascript used for tags handling wants access to CGISESSID cookie
Product: Koha Reporter: Galen Charlton <gmcharlt>
Component: Architecture, internals, and plumbingAssignee: Galen Charlton <gmcharlt>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: minor    
Priority: P5 - low CC: m.de.rooy
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Who signed the patch off: Text to go in the release notes:
Version(s) released in:
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
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.
Comment 1 Galen Charlton 2013-01-16 05:55:10 UTC Comment hidden (obsolete)
Comment 2 Marcel de Rooy 2013-02-01 10:02:39 UTC
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>
Comment 3 Marcel de Rooy 2013-02-01 10:12:46 UTC
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
Comment 4 Jared Camins-Esakov 2013-02-01 16:08:19 UTC
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.
Comment 5 Marcel de Rooy 2013-02-03 17:56:58 UTC
(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.