Bug 10952: Store anonymous search history in session

Storing search history into cookie can cause problems, due to the size
limitation of 4KB.

The solution here is to store search history into the CGI::Session
object, so there is no size limitation (but anonymous search history
still remember up to 15 requests max.)

Test plan:
- Go to OPAC in anonymous mode.
- Check that the "Search history" link is *not* shown in the top right
  corner of the page
- Make some searches on /cgi-bin/koha/opac-search.pl
- The "Search history" link should appear. Click.
- Your search history should be displayed.
- Try to log in with invalid username/password
- Go back to search history, it's still there
- Now log in with valid username/password
- Your anonymous search history should be saved into your own search
  history.

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Restoring original sign offs and comments below

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Work as described. No koha-qa errors

Well, search history saving is similar before and after patch.
i.e. anonmymous search is saved when user logs in, but cookie
KohaOpacRecentSearches is empty.
Shows current an previous session searches

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
All tests and QA script pass, works as described.

Signed-off-by: Charlene Criton <charlene.criton@univ-lyon2.fr>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>

diff --git a/C4/Auth.pm b/C4/Auth.pm
index 00eeeef..0d4daec 100644 (file)
--- a/C4/Auth.pm
+++ b/C4/Auth.pm
@@ -49,7 +49,7 @@ BEGIN {
     @EXPORT      = qw(&checkauth &get_template_and_user &haspermission &get_user_subpermissions);
     @EXPORT_OK   = qw(&check_api_auth &get_session &check_cookie_auth &checkpw &checkpw_internal &checkpw_hash
                       &get_all_subpermissions &get_user_subpermissions
-                      ParseSearchHistoryCookie
+                      ParseSearchHistorySession SetSearchHistorySession
                    );
     %EXPORT_TAGS = ( EditPermissions => [qw(get_all_subpermissions get_user_subpermissions)] );
     $ldap        = C4::Context->config('useldapserver') || 0;
@@ -262,7 +262,7 @@ sub get_template_and_user {
 
             # And if there's a cookie with searches performed when the user was not logged in,
             # we add them to the logged-in search history
-            my @recentSearches = ParseSearchHistoryCookie($in->{'query'});
+            my @recentSearches = ParseSearchHistorySession($in->{'query'});
             if (@recentSearches) {
                 my $sth = $dbh->prepare($SEARCH_HISTORY_INSERT_SQL);
                 $sth->execute( $borrowernumber,
@@ -273,14 +273,6 @@ sub get_template_and_user {
                            $_->{'time'},
                         ) foreach @recentSearches;
 
-                # And then, delete the cookie's content
-                my $newsearchcookie = $in->{'query'}->cookie(
-                                            -name => 'KohaOpacRecentSearches',
-                                            -value => encode_json([]),
-                                            -HttpOnly => 1,
-                                            -expires => ''
-                                         );
-                $cookie = [$cookie, $newsearchcookie];
             }
         }
     }
@@ -297,7 +289,7 @@ sub get_template_and_user {
      # Anonymous opac search history
      # If opac search history is enabled and at least one search has already been performed
      if (C4::Context->preference('EnableOpacSearchHistory')) {
-        my @recentSearches = ParseSearchHistoryCookie($in->{'query'}); 
+        my @recentSearches = ParseSearchHistorySession($in->{'query'});
         if (@recentSearches) {
             $template->param(ShowOpacRecentSearchLink => 1);
         }
@@ -647,6 +639,8 @@ sub checkauth {
     my ( $userid, $cookie, $sessionID, $flags, $barshelves, $pubshelves );
     my $logout = $query->param('logout.x');
 
+    my $anon_search_history;
+
     # This parameter is the name of the CAS server we want to authenticate against,
     # when using authentication against multiple CAS servers, as configured in Auth_cas_servers.yaml
     my $casparam = $query->param('cas');
@@ -695,7 +689,8 @@ sub checkauth {
             #if a user enters an id ne to the id in the current session, we need to log them in...
             #first we need to clear the anonymous session...
             $debug and warn "query id = $q_userid but session id = $s_userid";
-            $session->flush;      
+            $anon_search_history = $session->param('search_history');
+            $session->flush;
             $session->delete();
             C4::Context->_unset_userenv($sessionID);
             $sessionID = undef;
@@ -755,6 +750,14 @@ sub checkauth {
 
         #we initiate a session prior to checking for a username to allow for anonymous sessions...
         my $session = get_session("") or die "Auth ERROR: Cannot get_session()";
+
+        # Save anonymous search history in new session so it can be retrieved
+        # by get_template_and_user to store it in user's search history after
+        # a successful login.
+        if ($anon_search_history) {
+            $session->param('search_history', $anon_search_history);
+        }
+
         my $sessionID = $session->id;
         C4::Context->_new_userenv($sessionID);
         $cookie = $query->cookie(
@@ -966,6 +969,8 @@ sub checkauth {
                     $info{'invalid_username_or_password'} = 1;
                     C4::Context->_unset_userenv($sessionID);
                 }
+                $session->param('lasttime',time());
+                $session->param('ip',$session->remote_addr());
             }
         }    # END if ( $userid    = $query->param('userid') )
         elsif ($type eq "opac") {
@@ -1773,16 +1778,27 @@ sub getborrowernumber {
     return 0;
 }
 
-sub ParseSearchHistoryCookie {
-    my $input = shift;
-    my $search_cookie = $input->cookie('KohaOpacRecentSearches');
-    return () unless $search_cookie;
-    my $obj = eval { decode_json(uri_unescape($search_cookie)) };
+sub ParseSearchHistorySession {
+    my $cgi = shift;
+    my $sessionID = $cgi->cookie('CGISESSID');
+    return () unless $sessionID;
+    my $session = get_session($sessionID);
+    return () unless $session and $session->param('search_history');
+    my $obj = eval { decode_json(uri_unescape($session->param('search_history'))) };
     return () unless defined $obj;
     return () unless ref $obj eq 'ARRAY';
     return @{ $obj };
 }
 
+sub SetSearchHistorySession {
+    my ($cgi, $search_history) = @_;
+    my $sessionID = $cgi->cookie('CGISESSID');
+    return () unless $sessionID;
+    my $session = get_session($sessionID);
+    return () unless $session;
+    $session->param('search_history', uri_escape(encode_json($search_history)));
+}
+
 END { }    # module clean-up code here (global destructor)
 1;
 __END__

diff --git a/koha-tmpl/opac-tmpl/prog/en/modules/opac-search-history.tt b/koha-tmpl/opac-tmpl/prog/en/modules/opac-search-history.tt
index 9042543..4d0fa82 100644 (file)
--- a/koha-tmpl/opac-tmpl/prog/en/modules/opac-search-history.tt
+++ b/koha-tmpl/opac-tmpl/prog/en/modules/opac-search-history.tt
@@ -51,7 +51,7 @@
                <tbody>
                    [% FOREACH recentSearche IN recentSearches %]
                    <tr>
-            <td><span title="[% recentSearche.time %]">[% recentSearche.time |$KohaDates with_hours => 1 %]</span></td>
+            <td><span title="[% recentSearche.time %]">[% recentSearche.time %]</span></td>
                        <td><a href="/cgi-bin/koha/opac-search.pl?[% recentSearche.query_cgi |html %]">[% recentSearche.query_desc |html %]</a></td>
                        <td>[% recentSearche.total %]</td>
                    </tr>

diff --git a/opac/opac-search-history.pl b/opac/opac-search-history.pl
index dc1c2e3..4c02ba8 100755 (executable)
--- a/opac/opac-search-history.pl
+++ b/opac/opac-search-history.pl
@@ -20,7 +20,7 @@
 use strict;
 use warnings;
 
-use C4::Auth qw(:DEFAULT get_session ParseSearchHistoryCookie);
+use C4::Auth qw(:DEFAULT get_session ParseSearchHistorySession SetSearchHistorySession);
 use CGI;
 use JSON qw/decode_json encode_json/;
 use C4::Context;
@@ -49,22 +49,17 @@ if (!$loggedinuser) {
 
     # Deleting search history
     if ($cgi->param('action') && $cgi->param('action') eq 'delete') {
-       # Deleting cookie's content 
-       my $recentSearchesCookie = $cgi->cookie(
-           -name => 'KohaOpacRecentSearches',
-           -value => encode_json([]),
-           -expires => ''
-           );
-
-       # Redirecting to this same url with the cookie in the headers so it's deleted immediately
-       my $uri = $cgi->url();
-       print $cgi->redirect(-uri => $uri,
-                            -cookie => $recentSearchesCookie);
+        # Deleting cookie's content
+        SetSearchHistorySession($cgi, []);
+
+        # Redirecting to this same url with the cookie in the headers so it's deleted immediately
+        my $uri = $cgi->url();
+        print $cgi->redirect(-uri => $uri);
 
     # Showing search history
     } else {
 
-        my @recentSearches = ParseSearchHistoryCookie($cgi);
+        my @recentSearches = ParseSearchHistorySession($cgi);
            if (@recentSearches) {
 
                # As the dates are stored as unix timestamps, let's do some formatting

diff --git a/opac/opac-search.pl b/opac/opac-search.pl
index 8604d0f..00cd8e3 100755 (executable)
--- a/opac/opac-search.pl
+++ b/opac/opac-search.pl
@@ -42,7 +42,7 @@ for ( $searchengine ) {
 }
 
 use C4::Output;
-use C4::Auth qw(:DEFAULT get_session ParseSearchHistoryCookie);
+use C4::Auth qw(:DEFAULT get_session ParseSearchHistorySession SetSearchHistorySession);
 use C4::Languages qw(getAllLanguages);
 use C4::Search;
 use C4::Biblio;  # GetBiblioData
@@ -616,7 +616,7 @@ for (my $i=0;$i<@servers;$i++) {
         # Opac search history
         my $newsearchcookie;
         if (C4::Context->preference('EnableOpacSearchHistory')) {
-            my @recentSearches = ParseSearchHistoryCookie($cgi);
+            my @recentSearches = ParseSearchHistorySession($cgi);
 
             # Adding the new search if needed
             my $path_info = $cgi->url(-path_info=>1);
@@ -626,7 +626,7 @@ for (my $i=0;$i<@servers;$i++) {
             my $query_desc_history = join ", ", grep { defined $_ } $query_desc, $limit_desc;
 
             if (!$borrowernumber || $borrowernumber eq '') {
-                # To a cookie (the user is not logged in)
+                # To the session (the user is not logged in)
                 if (!$offset) {
                     push @recentSearches, {
                                 "query_desc" => Encode::decode_utf8($query_desc_history) || "unknown",
@@ -639,13 +639,7 @@ for (my $i=0;$i<@servers;$i++) {
 
                 shift @recentSearches if (@recentSearches > 15);
                 # Pushing the cookie back
-                $newsearchcookie = $cgi->cookie(
-                            -name => 'KohaOpacRecentSearches',
-                            # We uri_escape the whole serialized structure so we're sure we won't have any encoding problems
-                            -value => uri_escape( encode_json(\@recentSearches) ),
-                            -expires => ''
-                );
-                $cookie = [$cookie, $newsearchcookie];
+                SetSearchHistorySession($cgi, \@recentSearches);
             }
             else {
                 # To the session (the user is logged in)