Bug 12497 - Make OPAC search history feature accessible when it should
Summary: Make OPAC search history feature accessible when it should
Status: Pushed to Master
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Victor Grousset/tuxayo
QA Contact: Marcel de Rooy
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-06-28 20:23 UTC by Katrin Fischer
Modified: 2017-12-07 14:26 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 12497: Fix search history non-accessible when OPAC was private (1.10 KB, patch)
2017-11-23 15:41 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 12497: Fix OPAC search history reachable by URL even when disabled (1.30 KB, patch)
2017-11-24 11:15 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 12497: Fix search history non-accessible when OPAC was private (1.16 KB, patch)
2017-11-24 20:26 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 12497: Fix OPAC search history reachable by URL even when disabled (1.36 KB, patch)
2017-11-24 20:27 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 12497: Fix search history non-accessible when OPAC was private (1.22 KB, patch)
2017-11-24 21:00 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 12497: Fix OPAC search history reachable by URL even when disabled (1.42 KB, patch)
2017-11-24 21:00 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 12497: Fix search history non-accessible when OPAC was private (1.32 KB, patch)
2017-11-27 13:31 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 12497: Fix OPAC search history reachable by URL even when disabled (1.51 KB, patch)
2017-11-27 13:32 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2014-06-28 20:23:34 UTC
The page for accessing the OPAC search history should not be accessible if the system preference EnableOpacSearchHistory is set to "Don't keep".

To test: 
- Turn off EnableOpacSearchHistory
- Try to access .../cgi-bin/koha/opac-search-history.pl
Comment 1 Martin Renvoize 2017-04-28 08:10:37 UTC
Is this still the case?
Comment 2 Marc Véron 2017-04-28 17:59:46 UTC
Still valid.

To reproduce:

- In staff client, Turn EnableOpacSearchHistory to 'Keep'
- Log in to the OPAC
- Do some seaches
- In OPAC, go to the Search history page (link top right)
- Verify that search history appears
- Copy link from browser address bar (.../cgi-bin/koha/opac-search-history.pl)
- Turn EnableOpacSearchHistory to "Don't keep"
- Log in again to OPAC
- Verify that link to search history does no longer appear
- Paste saved link in address bar
- Page with search history appears - should not.
Comment 3 Gaetan Boisson 2017-11-22 10:53:54 UTC
On additional issue is that if you have activated search history on the opac, and have an opac set to private via opacpublic, then patrons can only access their search history if they have the permission to manage borrowers.

To reproduce :
- set your opac to private via opacpublic
- activate EnableOpacSearchHistory
- log in your private opac with a user that has no permissions for the staff interface
- search for something in the catalog
- the link for search history is now visible, but if you click on it, you will get an access denied message

- log in to your private opac with a user that has the "borrowers" permission
- repeat above steps : you can now access your search history.

Since this issue and the fact that the url still responds when the syspref is turned off, when it should  display an access denied page, can be fixed in the same block of code, we will offer a patch for both issues at once.
Comment 4 Victor Grousset/tuxayo 2017-11-23 15:41:32 UTC Comment hidden (obsolete)
Comment 6 Victor Grousset/tuxayo 2017-11-24 11:15:48 UTC Comment hidden (obsolete)
Comment 7 David Bourgault 2017-11-24 20:26:49 UTC Comment hidden (obsolete)
Comment 8 David Bourgault 2017-11-24 20:27:00 UTC Comment hidden (obsolete)
Comment 9 Charles Farmer 2017-11-24 20:51:15 UTC
Is this intended behavior to return a 404 whenever a systempreference blocks a page in Koha? I feel like the template could simply state to users and admin alike that search history is disabled, unless that's deemed too verbose for the user's own good.

But it works, nonetheless, so I'm signing it off.
Comment 10 Charles Farmer 2017-11-24 21:00:33 UTC
Created attachment 69341 [details] [review]
Bug 12497: Fix search history non-accessible when OPAC was private

(when the user didn't have the "borrowers" permission)

Test plan:
1. Set syspref "OpacPublic" to false (Disable)
2. Log in as a user without any permission
3. Try to access the search history. The link is next to "Log out"
4. Then you should see "Access denied"
5. Apply this patch
6. Refresh the page
7. Then you should see the search history

Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Signed-off-by: Charles Farmer <charles.farmer@inLibro.com>
Comment 11 Charles Farmer 2017-11-24 21:00:37 UTC
Created attachment 69342 [details] [review]
Bug 12497: Fix OPAC search history reachable by URL even when disabled

Test plan:
1. Set EnableOpacSearchHistory syspref to "Keep"
2. Make a search in the OPAC
3. Go to /cgi-bin/koha/opac-search-history.pl
4. Set the EnableOpacSearchHistory syspref to "Don't keep"
5. Refresh the OPAC page to show that history is still accessible
6. Apply the patch
7. Refresh the OPAC page, you should end on the 404 page
8. Set EnableOpacSearchHistory syspref to "Keep"
9. Go to /cgi-bin/koha/opac-search-history.pl which should be reachable

Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Signed-off-by: Charles Farmer <charles.farmer@inLibro.com>
Comment 12 Marcel de Rooy 2017-11-27 13:29:43 UTC
(In reply to Charles Farmer from comment #9)
> Is this intended behavior to return a 404 whenever a systempreference blocks
> a page in Koha? I feel like the template could simply state to users and
> admin alike that search history is disabled, unless that's deemed too
> verbose for the user's own good.
> 
> But it works, nonetheless, so I'm signing it off.

You will find more examples like:

if ( ! C4::Context->preference('suggestion') ) {
    print $input->redirect("/cgi-bin/koha/errors/404.pl");
    exit;
}

But you are right, nice is something else :)
Comment 13 Marcel de Rooy 2017-11-27 13:31:59 UTC
Created attachment 69380 [details] [review]
Bug 12497: Fix search history non-accessible when OPAC was private

(when the user didn't have the "borrowers" permission)

Test plan:
1. Set syspref "OpacPublic" to false (Disable)
2. Log in as a user without any permission
3. Try to access the search history. The link is next to "Log out"
4. Then you should see "Access denied"
5. Apply this patch
6. Refresh the page
7. Then you should see the search history

Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Signed-off-by: Charles Farmer <charles.farmer@inLibro.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 14 Marcel de Rooy 2017-11-27 13:32:04 UTC
Created attachment 69381 [details] [review]
Bug 12497: Fix OPAC search history reachable by URL even when disabled

Test plan:
1. Set EnableOpacSearchHistory syspref to "Keep"
2. Make a search in the OPAC
3. Go to /cgi-bin/koha/opac-search-history.pl
4. Set the EnableOpacSearchHistory syspref to "Don't keep"
5. Refresh the OPAC page to show that history is still accessible
6. Apply the patch
7. Refresh the OPAC page, you should end on the 404 page
8. Set EnableOpacSearchHistory syspref to "Keep"
9. Go to /cgi-bin/koha/opac-search-history.pl which should be reachable

Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Signed-off-by: Charles Farmer <charles.farmer@inLibro.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 15 Jonathan Druart 2017-12-07 14:26:18 UTC
Patch has been pushed to master for 18.05.

Congratulations Victor for your first patch in!