From e7e4ef11fd67124c14d7b56325d232fa60eda99c Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Fri, 30 Oct 2020 16:28:39 -0300 Subject: [PATCH] Bug 19133: (QA follow-up) More contemporary language Signed-off-by: Tomas Cohen Arazi Signed-off-by: Kyle M Hall --- Koha/Patron/Password/Recovery.pm | 8 ++++---- api/v1/swagger/paths/patrons.json | 2 +- ...133-2_Add_permission_get_password_reset_uuid.perl | 10 +++++++--- ...-Password-recovery-custom-link-allowed-hosts.perl | 12 ++++++++++++ ...9133-Password-recovery-custom-link-whitelist.perl | 11 ----------- installer/data/mysql/mandatory/sysprefs.sql | 2 +- .../prog/en/modules/admin/preferences/opac.pref | 2 +- t/db_dependent/api/v1/patrons_password_recovery.t | 6 +++--- 8 files changed, 29 insertions(+), 24 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/Bug-19133-Password-recovery-custom-link-allowed-hosts.perl delete mode 100644 installer/data/mysql/atomicupdate/Bug-19133-Password-recovery-custom-link-whitelist.perl diff --git a/Koha/Patron/Password/Recovery.pm b/Koha/Patron/Password/Recovery.pm index 753601378c..a17e55134e 100644 --- a/Koha/Patron/Password/Recovery.pm +++ b/Koha/Patron/Password/Recovery.pm @@ -156,10 +156,10 @@ sub SendPasswordRecoveryEmail { if (defined $params->{url}) { # Make sure host name exists in whitelist my $url = $params->{url}; - my $whitelist = C4::Context->preference( - 'OpacResetPasswordHostWhitelist' + my $allowed_list = C4::Context->preference( + 'OpacResetPasswordAllowedHosts' ); - my @allowed_hosts = split /[,\s\|]/, $whitelist if defined $whitelist; + my @allowed_hosts = split /[,\s\|]/, $allowed_list if defined $allowed_list; foreach my $allowed_host (@allowed_hosts) { if ($url =~ /^https?:\/\/$allowed_host/) { $url =~ s/{uuid}/$uuid_str/gi; @@ -168,7 +168,7 @@ sub SendPasswordRecoveryEmail { } } Koha::Exceptions::WrongParameter->throw( error => 'System preference' - . ' OpacResetPasswordHostWhitelist does' + . ' OpacResetPasswordAllowedHosts does' . " not contain the requested host of '$url'" ) if $uuidLink ne $params->{url}; } diff --git a/api/v1/swagger/paths/patrons.json b/api/v1/swagger/paths/patrons.json index 522b072d93..ccf0533cf0 100644 --- a/api/v1/swagger/paths/patrons.json +++ b/api/v1/swagger/paths/patrons.json @@ -731,7 +731,7 @@ "type": "string" }, "complete_url": { - "description": "The location where patron will complete their password recovery. This location will be added into the password recovery email. To avoid malicious use, whitelist certain hosts in OpacResetPasswordHostWhitelist system preference and forbid all other custom links. Use {uuid} for adding the password recovery uuid into your custom link. It will be replaced by the actual uuid in the email.", + "description": "The location where patron will complete their password recovery. This location will be added into the password recovery email. To avoid malicious use, allow certain hosts in OpacResetPasswordAllowedHosts system preference and forbid all other custom links. Use {uuid} for adding the password recovery uuid into your custom link. It will be replaced by the actual uuid in the email.", "type": "string" }, "skip_email": { diff --git a/installer/data/mysql/atomicupdate/Bug-19133-2_Add_permission_get_password_reset_uuid.perl b/installer/data/mysql/atomicupdate/Bug-19133-2_Add_permission_get_password_reset_uuid.perl index 924631ce35..5754d9d9c1 100644 --- a/installer/data/mysql/atomicupdate/Bug-19133-2_Add_permission_get_password_reset_uuid.perl +++ b/installer/data/mysql/atomicupdate/Bug-19133-2_Add_permission_get_password_reset_uuid.perl @@ -1,8 +1,12 @@ $DBversion = 'XXX'; # will be replaced by the RM if( CheckVersion( $DBversion ) ) { - $dbh->do( "INSERT INTO permissions (module_bit, code, description) VALUES ( 4, 'get_password_reset_uuid', 'Allow the user to get password reset uuid when recovering passwords. Useful for third party service integrations that wish to do something with the uuid, such as handle emails themselves instead of Koha.')" ); + $dbh->do(q{ + INSERT INTO permissions + (module_bit, code, description) + VALUES + ( 4, 'get_password_reset_uuid', 'Allow the user to get password reset uuid when recovering passwords. Useful for third party service integrations that wish to do something with the uuid, such as handle emails themselves instead of Koha.'); + }); - SetVersion( $DBversion ); - print "Upgrade to $DBversion done (Bug 19133 - Add permission get_password_reset_uuid.)\n"; + NewVersion( $DBversion, 19133, "Password recovery via REST API - Add permission get_password_reset_uuid"); } diff --git a/installer/data/mysql/atomicupdate/Bug-19133-Password-recovery-custom-link-allowed-hosts.perl b/installer/data/mysql/atomicupdate/Bug-19133-Password-recovery-custom-link-allowed-hosts.perl new file mode 100644 index 0000000000..24be48a37d --- /dev/null +++ b/installer/data/mysql/atomicupdate/Bug-19133-Password-recovery-custom-link-allowed-hosts.perl @@ -0,0 +1,12 @@ +$DBversion = 'XXX'; # will be replaced by the RM +if( CheckVersion( $DBversion ) ) { + + $dbh->do(q{ + INSERT INTO systempreferences + (variable, value, options, explanation, type) + VALUES + ('OpacResetPasswordAllowedHosts', '', '', 'Allowed external host names for password reset in third party service. Separate list by whitespace, comma or |', 'free'); + }); + + NewVersion( $DBversion, 19133, "Password recovery via REST API - Allowed hosts for custom links"); +} diff --git a/installer/data/mysql/atomicupdate/Bug-19133-Password-recovery-custom-link-whitelist.perl b/installer/data/mysql/atomicupdate/Bug-19133-Password-recovery-custom-link-whitelist.perl deleted file mode 100644 index fc1fe37519..0000000000 --- a/installer/data/mysql/atomicupdate/Bug-19133-Password-recovery-custom-link-whitelist.perl +++ /dev/null @@ -1,11 +0,0 @@ -$DBversion = 'XXX'; # will be replaced by the RM -if( CheckVersion( $DBversion ) ) { - - $dbh->do( - "INSERT INTO systempreferences (variable, value, options, explanation, type) VALUES ('OpacResetPasswordHostWhitelist', '', '', 'Whitelist external host names for password reset in third party service. Separate list by whitespace, comma or |', 'free')" - ); - - # Always end with this (adjust the bug info) - SetVersion( $DBversion ); - print "Upgrade to $DBversion done (Bug 19133 - Password recovery via REST API - whitelist custom links)\n"; -} diff --git a/installer/data/mysql/mandatory/sysprefs.sql b/installer/data/mysql/mandatory/sysprefs.sql index 02c170b903..ab5c233fd4 100644 --- a/installer/data/mysql/mandatory/sysprefs.sql +++ b/installer/data/mysql/mandatory/sysprefs.sql @@ -439,7 +439,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('OpacRenewalBranch','checkoutbranch','itemhomebranch|patronhomebranch|checkoutbranch|none','Choose how the branch for an OPAC renewal is recorded in statistics','Choice'), ('OPACReportProblem', 0, NULL, 'Allow patrons to submit problem reports for OPAC pages to the library or Koha Administrator', 'YesNo'), ('OpacResetPassword','0','','Shows the ''Forgot your password?'' link in the OPAC','YesNo'), -('OpacResetPasswordHostWhitelist','','','Whitelist external host names for password reset in third party service. Separate list by whitespace, comma or |','free'), +('OpacResetPasswordAllowedHosts','','','Allowed external host names for password reset in third party service. Separate list by whitespace, comma or |','free'), ('OPACResultsLibrary', 'homebranch', 'homebranch|holdingbranch', 'Defines whether the OPAC displays the holding or home branch in search results when using XSLT', 'Choice'), ('OPACResultsSidebar','','70|10','Define HTML to be included on the search results page, underneath the facets sidebar','Textarea'), ('OPACSearchForTitleIn','Other Libraries (WorldCat)\nOther Databases (Google Scholar)\nOnline Stores (Bookfinder.com)\nOpen Library (openlibrary.org)','70|10','Enter the HTML that will appear in the \'Search for this title in\' box on the detail page in the OPAC. Enter {TITLE}, {AUTHOR}, or {ISBN} in place of their respective variables in the URL. Leave blank to disable \'More Searches\' menu.','Textarea'), diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref index c72d409135..10120fe2c7 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref @@ -390,7 +390,7 @@ OPAC: - " to recover their password via e-mail in the OPAC." - - "Allow the following " - - pref: OpacResetPasswordHostWhitelist + - pref: OpacResetPasswordAllowedHosts type: textarea - " host names of third party services to be used as external password reset locations. Separate list elements by new line, whitespace, comma or |." - diff --git a/t/db_dependent/api/v1/patrons_password_recovery.t b/t/db_dependent/api/v1/patrons_password_recovery.t index 62cb6e2885..176429886d 100644 --- a/t/db_dependent/api/v1/patrons_password_recovery.t +++ b/t/db_dependent/api/v1/patrons_password_recovery.t @@ -197,7 +197,7 @@ subtest 'recovery() tests' => sub { plan tests => 5; t::lib::Mocks::mock_preference( - 'OpacResetPasswordHostWhitelist', '' + 'OpacResetPasswordAllowedHosts', '' ); $tx = $t->ua->build_tx(POST => $url => json => { @@ -211,7 +211,7 @@ subtest 'recovery() tests' => sub { ->status_is(400); t::lib::Mocks::mock_preference( - 'OpacResetPasswordHostWhitelist', 'allowed' + 'OpacResetPasswordAllowedHosts', 'allowed' ); $tx = $t->ua->build_tx(POST => $url => json => { @@ -249,7 +249,7 @@ subtest 'recovery() tests' => sub { plan tests => 10; t::lib::Mocks::mock_preference( - 'OpacResetPasswordHostWhitelist', 'anotherallowed' + 'OpacResetPasswordAllowedHosts', 'anotherallowed' ); my ($service_borrowernumber, $service_session) = create_user_and_session({ authorized => 1 -- 2.24.1 (Apple Git-126)