From d07df7d51250bb5a40bb556aab48afb18a67a396 Mon Sep 17 00:00:00 2001 From: Julian Maurice Date: Thu, 26 Sep 2013 11:22:26 +0200 Subject: [PATCH] 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 Restoring original sign offs and comments below Signed-off-by: Bernardo Gonzalez Kriegel 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 All tests and QA script pass, works as described. Signed-off-by: Charlene Criton Signed-off-by: Kyle M Hall Signed-off-by: Galen Charlton --- C4/Auth.pm | 50 ++++++++++++------- .../prog/en/modules/opac-search-history.tt | 2 +- opac/opac-search-history.pl | 21 +++----- opac/opac-search.pl | 14 ++---- 4 files changed, 46 insertions(+), 41 deletions(-) diff --git a/C4/Auth.pm b/C4/Auth.pm index 00eeeef842..0d4daecfd4 100644 --- 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 904254336d..4d0fa8297d 100644 --- 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 @@ [% FOREACH recentSearche IN recentSearches %] - [% recentSearche.time |$KohaDates with_hours => 1 %] + [% recentSearche.time %] [% recentSearche.query_desc |html %] [% recentSearche.total %] diff --git a/opac/opac-search-history.pl b/opac/opac-search-history.pl index dc1c2e3412..4c02ba841f 100755 --- 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 8604d0f529..00cd8e3fb9 100755 --- 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) -- 2.39.5