From 4dd0bfc10750b2e7134d1a3614be42efd4f58ee9 Mon Sep 17 00:00:00 2001 From: Olivia Reynolds Date: Tue, 23 Jun 2026 09:21:59 +0000 Subject: [PATCH] Bug 39337: Add is_opac_suppressed helper to C4::Context Previously, OPAC suppression was applied inconsistently, with only some callers taking OpacSuppressionByIPRange into account. This patch adds a new helper to C4::Context to determine whether OPAC suppression should be active for this request, and switches to using it everywhere that was previously manually checking the system preferences. Test plan (specific URLs given for KTD, with thanks to David Cook): 1. Apply patch. 2. Navigate to http://localhost:8081/cgi-bin/koha/cataloguing/addbiblio.pl?biblionumber=2 3. Set 942$n to "Yes" and save the record. 4. Disable the system preference "OpacSuppression", and ensure "OpacSuppressionByIPRange" is blank. 5a. Verify you can successfully access http://localhost:8080/cgi-bin/koha/opac-detail.pl?biblionumber=2 5b. Verify you can successfully access http://localhost:8080/cgi-bin/koha/opac-MARCdetail.pl?biblionumber=2 5c. Verify you can successfully access http://localhost:8080/cgi-bin/koha/opac-ISBDdetail.pl?biblionumber=2 5d. Verify you can successfully access http://localhost:8080/cgi-bin/koha/opac-search.pl?idx=&q=The+ideas+behind&weight_search=1 5e. Verify you can successfully access http://localhost:8080/api/v1/public/biblios/2 5f. Verify you can successfully access http://localhost:8080/cgi-bin/koha/oai.pl?verb=GetRecord&metadataPrefix=oai_dc&identifier=KOHA-OAI-TEST:2 5g. Verify KOHA-OAI-TEST-2 appears on http://localhost:8080/cgi-bin/koha/oai.pl?verb=ListRecords&metadataPrefix=oai_dc 6. Enable the system preference "OpacSuppression", and ensure "OpacSuppressionByIPRange" is blank. 7. Repeat steps 5a-5g, verifying that: - the OPAC details pages return "Record blocked". - the OPAC search page returns "No results found". - the API returns "Bibliographic record not found". - the OAI GetRecord and ListRecords show "Record Status: deleted" for KOHA-OAI-TEST-2. 8. Keep "OpacSuppression" enabled, but set "OpacSuppressionByIPRange" to "." (to allow all IPs to bypass suppression). 9. Repeat steps 5a-5g, verifying that every page is successful again. 10. Run `prove t/Context.t`, and verify all tests are successful. Sponsored-by: Open Fifth Signed-off-by: David Nind Signed-off-by: David Cook Signed-off-by: David Nind Signed-off-by: Pedro Amorim --- C4/Context.pm | 20 ++++++++++++++++++++ C4/Output.pm | 20 ++++---------------- Koha/OAI/Server/Repository.pm | 5 +---- Koha/REST/V1/Biblios.pm | 2 +- opac/opac-ISBDdetail.pl | 3 +-- opac/opac-MARCdetail.pl | 3 +-- opac/opac-detail.pl | 3 +-- opac/opac-search.pl | 13 +------------ t/Context.t | 32 +++++++++++++++++++++++++++++++- 9 files changed, 61 insertions(+), 40 deletions(-) diff --git a/C4/Context.pm b/C4/Context.pm index db69cad9c8..ef91190a7b 100644 --- a/C4/Context.pm +++ b/C4/Context.pm @@ -983,6 +983,26 @@ sub is_internal_PSGI_request { return $is_internal; } +=head3 is_opac_suppressed + + return if C4::Context->is_opac_suppressed; + +is_opac_suppressed returns true if and only if OPAC suppression is enabled for +the current request, based on the sysprefs OpacSuppression and +OpacSuppressionByIPRange. + +=cut + +sub is_opac_suppressed { + return 0 unless C4::Context->preference('OpacSuppression'); + + my $IPAddress = $ENV{'REMOTE_ADDR'}; + my $IPRange = C4::Context->preference('OpacSuppressionByIPRange'); + return 1 unless $IPRange; + + return $IPAddress !~ /^$IPRange/; +} + __END__ =head1 ENVIRONMENT diff --git a/C4/Output.pm b/C4/Output.pm index 3b87674872..fa4943fd18 100644 --- a/C4/Output.pm +++ b/C4/Output.pm @@ -438,8 +438,7 @@ sub parametrized_url { =item redirect_if_opac_suppressed - redirect_if_opac_suppressed( $query, $biblio ) - if C4::Context->preference('OpacSuppression'); + redirect_if_opac_suppressed( $query, $biblio ); For a given I object, it handles redirection if it is suppressed from the OPAC. @@ -456,20 +455,9 @@ sub redirect_if_opac_suppressed { } else { $redirect_url = "/cgi-bin/koha/errors/404.pl"; } - if ( $biblio->opac_suppressed() ) { - - # if OPAC suppression by IP address - if ( C4::Context->preference('OpacSuppressionByIPRange') ) { - my $IPAddress = $ENV{'REMOTE_ADDR'}; - my $IPRange = C4::Context->preference('OpacSuppressionByIPRange'); - if ( $IPAddress !~ /^$IPRange/ ) { - print $query->redirect($redirect_url); - C4::Auth::safe_exit(); - } - } else { - print $query->redirect($redirect_url); - C4::Auth::safe_exit(); - } + if ( C4::Context->is_opac_suppressed && $biblio->opac_suppressed() ) { + print $query->redirect($redirect_url); + C4::Auth::safe_exit(); } } diff --git a/Koha/OAI/Server/Repository.pm b/Koha/OAI/Server/Repository.pm index 24a2cdb8c5..0df697310f 100644 --- a/Koha/OAI/Server/Repository.pm +++ b/Koha/OAI/Server/Repository.pm @@ -216,10 +216,7 @@ sub get_biblio_marcxml { } if ($record) { - # Check if the bibliographic record is suppressed in OPAC - if ( C4::Context->preference('OpacSuppression') && $biblio->opac_suppressed ) { - return; - } + return if C4::Context->is_opac_suppressed && $biblio->opac_suppressed; my $rules = C4::Context->yaml_preference('OpacHiddenItems') // {}; if ( $biblio->hidden_in_opac( { rules => $rules } ) ) { diff --git a/Koha/REST/V1/Biblios.pm b/Koha/REST/V1/Biblios.pm index f8e7713da9..5a95b5c94a 100644 --- a/Koha/REST/V1/Biblios.pm +++ b/Koha/REST/V1/Biblios.pm @@ -163,7 +163,7 @@ sub get_public { my $patron = $c->stash('koha.user'); # Check if the bibliographic record is suppressed in OPAC - if ( C4::Context->preference('OpacSuppression') && $biblio->opac_suppressed ) { + if ( C4::Context->is_opac_suppressed && $biblio->opac_suppressed ) { return $c->render_resource_not_found("Bibliographic record"); } diff --git a/opac/opac-ISBDdetail.pl b/opac/opac-ISBDdetail.pl index 34e32ec1d6..86cf48b390 100755 --- a/opac/opac-ISBDdetail.pl +++ b/opac/opac-ISBDdetail.pl @@ -73,8 +73,7 @@ if ( !$biblio ) { } # If record should be suppressed, handle it early -redirect_if_opac_suppressed( $query, $biblio ) - if C4::Context->preference('OpacSuppression'); +redirect_if_opac_suppressed( $query, $biblio ); #open template my ( $template, $loggedinuser, $cookie ) = get_template_and_user( diff --git a/opac/opac-MARCdetail.pl b/opac/opac-MARCdetail.pl index 6cd85287b2..a53e4053df 100755 --- a/opac/opac-MARCdetail.pl +++ b/opac/opac-MARCdetail.pl @@ -97,8 +97,7 @@ if ( !$biblio ) { } # If record should be suppressed, handle it early -redirect_if_opac_suppressed( $query, $biblio ) - if C4::Context->preference('OpacSuppression'); +redirect_if_opac_suppressed( $query, $biblio ); unless ( $patron and $patron->category->override_hidden_items ) { diff --git a/opac/opac-detail.pl b/opac/opac-detail.pl index ec9f2b7818..064eab3205 100755 --- a/opac/opac-detail.pl +++ b/opac/opac-detail.pl @@ -108,8 +108,7 @@ unless ( $biblio && $record ) { } # If record should be suppressed, handle it early -redirect_if_opac_suppressed( $query, $biblio ) - if C4::Context->preference('OpacSuppression'); +redirect_if_opac_suppressed( $query, $biblio ); my $items = $biblio->items->search_ordered; if ($specific_item) { diff --git a/opac/opac-search.pl b/opac/opac-search.pl index 69dafb2d4a..78cec11103 100755 --- a/opac/opac-search.pl +++ b/opac/opac-search.pl @@ -502,18 +502,7 @@ my $weight_search = $cgi->param('weight_search_submitted') # Define some global variables my ( $error, $query, $simple_query, $query_cgi, $query_desc, $limit, $limit_cgi, $limit_desc, $query_type ); -my $suppress = 0; -if ( C4::Context->preference('OpacSuppression') ) { - - # OPAC suppression by IP address - if ( C4::Context->preference('OpacSuppressionByIPRange') ) { - my $IPAddress = $ENV{'REMOTE_ADDR'}; - my $IPRange = C4::Context->preference('OpacSuppressionByIPRange'); - $suppress = ( $IPAddress !~ /^$IPRange/ ); - } else { - $suppress = 1; - } -} +my $suppress = C4::Context->is_opac_suppressed; ## I. BUILD THE QUERY ( $error, $query, $simple_query, $query_cgi, $query_desc, $limit, $limit_cgi, $limit_desc, $query_type ) = diff --git a/t/Context.t b/t/Context.t index 31b8a3104f..c2d2ff1fe6 100755 --- a/t/Context.t +++ b/t/Context.t @@ -19,7 +19,7 @@ use Modern::Perl; use DBI; use Test::NoWarnings; -use Test::More tests => 36; +use Test::More tests => 37; use Test::MockModule; use Test::Warn; use YAML::XS; @@ -271,3 +271,33 @@ subtest 'psgi_env and is_internal_PSGI_request' => sub { delete $ENV{'PSGI.UPPERCASE'}; ok( !C4::Context->is_internal_PSGI_request, 'api but no longer PSGI' ); }; + +subtest 'is_opac_suppressed' => sub { + plan tests => 6; + + $ENV{REMOTE_ADDR} = '200.16.23.1'; + + t::lib::Mocks::mock_preference( 'OpacSuppression', 0 ); + t::lib::Mocks::mock_preference( 'OpacSuppressionByIPRange', '' ); + ok( !C4::Context->is_opac_suppressed, 'suppression disabled' ); + + t::lib::Mocks::mock_preference( 'OpacSuppression', 0 ); + t::lib::Mocks::mock_preference( 'OpacSuppressionByIPRange', '127.' ); + ok( !C4::Context->is_opac_suppressed, 'suppression disabled with non-matching ip range' ); + + t::lib::Mocks::mock_preference( 'OpacSuppression', 1 ); + t::lib::Mocks::mock_preference( 'OpacSuppressionByIPRange', '200.' ); + ok( !C4::Context->is_opac_suppressed, 'suppression enabled with matching ip range' ); + + t::lib::Mocks::mock_preference( 'OpacSuppression', 1 ); + t::lib::Mocks::mock_preference( 'OpacSuppressionByIPRange', '' ); + ok( C4::Context->is_opac_suppressed, 'suppression enabled' ); + + t::lib::Mocks::mock_preference( 'OpacSuppression', 1 ); + t::lib::Mocks::mock_preference( 'OpacSuppressionByIPRange', '127.' ); + ok( C4::Context->is_opac_suppressed, 'suppression enabled with non-matching ip range' ); + + t::lib::Mocks::mock_preference( 'OpacSuppression', 1 ); + t::lib::Mocks::mock_preference( 'OpacSuppressionByIPRange', '200.' ); + ok( !C4::Context->is_opac_suppressed, 'suppression enabled with matching ip range' ); + }