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 <david@davidnind.com> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This commit is contained in:
parent
8316c01bac
commit
4dd0bfc107
|
|
@ -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
|
||||
|
|
|
|||
20
C4/Output.pm
20
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<Koha::Biblio> 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();
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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 } ) ) {
|
||||
|
|
|
|||
|
|
@ -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");
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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 ) {
|
||||
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -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 ) =
|
||||
|
|
|
|||
32
t/Context.t
32
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' );
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue