Attachment 144323 Details for Bug 30649 – Bug 30649: Vendor EDI account passwords should be encrypted in the database

[patch] Bug 30649: Vendor EDI account passwords should be encrypted in the database

Bug-30649-Vendor-EDI-account-passwords-should-be-e.patch (text/plain), 7.51 KB, created by Kyle M Hall (khall) on 2022-11-29 14:30:14 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Kyle M Hall (khall)

Created: 2022-11-29 14:30:14 UTC

Size: 7.51 KB

patch

obsolete

>From 3f1a8c76d594f59c3ee9dbff2570c8f200815d7d Mon Sep 17 00:00:00 2001
>From: Kyle Hall <kyle@bywatersolutions.com>
>Date: Tue, 18 Oct 2022 09:26:45 -0400
>Subject: [PATCH] Bug 30649: Vendor EDI account passwords should be encrypted
> in the database
>
>We are storing edi vendor acccount passwords in clear text in the
>database. Now that Koha has the Koha::Encryption module, we should
>use that to encrypt passwords for all existing and new EDI accounts.
>
>Test Plan:
>1) Apply this patch
>2) Create one or more EDI vendor accounts
>3) Run a report to view the account passwords, note they are in clear
>   text
>4) Run updatedatabase.pl
>5) Re-run the report, account passwords should be encrypted now
>6) Edit a vendor EDI account, note you can still view and update the
>   password for an account
>
>Signed-off-by: David Nind <david@davidnind.com>
>Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>---
> Koha/Edifact/Transport.pm                     | 23 +++++++++--------
> admin/edi_accounts.pl                         | 13 ++++++++--
> .../data/mysql/atomicupdate/bug_30649.pl      | 25 +++++++++++++++++++
> installer/data/mysql/kohastructure.sql        |  2 +-
> 4 files changed, 50 insertions(+), 13 deletions(-)
> create mode 100755 installer/data/mysql/atomicupdate/bug_30649.pl
>
>diff --git a/Koha/Edifact/Transport.pm b/Koha/Edifact/Transport.pm
>index 833c44e896..b423eebc37 100644
>--- a/Koha/Edifact/Transport.pm
>+++ b/Koha/Edifact/Transport.pm
>@@ -17,19 +17,22 @@ package Koha::Edifact::Transport;
> # You should have received a copy of the GNU General Public License
> # along with Koha; if not, see <http://www.gnu.org/licenses>.
> 
>-use strict;
>-use warnings;
>+use Modern::Perl;
>+
> use utf8;
>-use DateTime;
>+
> use Carp qw( carp );
>+use DateTime;
>+use Encode qw( from_to );
> use English qw{ -no_match_vars };
>+use File::Copy qw( copy move );
>+use File::Slurp qw( read_file );
> use Net::FTP;
> use Net::SFTP::Foreign;
>-use File::Slurp qw( read_file );
>-use File::Copy qw( copy move );
>+
> use Koha::Database;
> use Koha::DateUtils qw( dt_from_string );
>-use Encode qw( from_to );
>+use Koha::Encryption;
> 
> sub new {
>     my ( $class, $account_id ) = @_;
>@@ -136,7 +139,7 @@ sub sftp_download {
>     my $sftp = Net::SFTP::Foreign->new(
>         host     => $self->{account}->host,
>         user     => $self->{account}->username,
>-        password => $self->{account}->password,
>+        password => Koha::Encryption->new->decrypt_hex($self->{account}->password),
>         timeout  => 10,
>     );
>     if ( $sftp->error ) {
>@@ -221,7 +224,7 @@ sub ftp_download {
>       )
>       or return $self->_abort_download( undef,
>         "Cannot connect to $self->{account}->host: $EVAL_ERROR" );
>-    $ftp->login( $self->{account}->username, $self->{account}->password )
>+    $ftp->login( $self->{account}->username, Koha::Encryption->new->decrypt_hex($self->{account}->password) )
>       or return $self->_abort_download( $ftp, "Cannot login: $ftp->message()" );
>     $ftp->cwd( $self->{account}->download_directory )
>       or return $self->_abort_download( $ftp,
>@@ -262,7 +265,7 @@ sub ftp_upload {
>       )
>       or return $self->_abort_download( undef,
>         "Cannot connect to $self->{account}->host: $EVAL_ERROR" );
>-    $ftp->login( $self->{account}->username, $self->{account}->password )
>+    $ftp->login( $self->{account}->username, Koha::Encryption->new->decrypt_hex($self->{account}->password) )
>       or return $self->_abort_download( $ftp, "Cannot login: $ftp->message()" );
>     $ftp->cwd( $self->{account}->upload_directory )
>       or return $self->_abort_download( $ftp,
>@@ -293,7 +296,7 @@ sub sftp_upload {
>     my $sftp = Net::SFTP::Foreign->new(
>         host     => $self->{account}->host,
>         user     => $self->{account}->username,
>-        password => $self->{account}->password,
>+        password => Koha::Encryption->new->decrypt_hex($self->{account}->password),
>         timeout  => 10,
>     );
>     $sftp->die_on_error("Cannot ssh to $self->{account}->host");
>diff --git a/admin/edi_accounts.pl b/admin/edi_accounts.pl
>index 9f1be0bf20..5394eaf351 100755
>--- a/admin/edi_accounts.pl
>+++ b/admin/edi_accounts.pl
>@@ -18,10 +18,13 @@
> # along with Koha; if not, see <http://www.gnu.org/licenses>.
> 
> use Modern::Perl;
>+
> use CGI;
>+
> use C4::Auth qw( get_template_and_user );
> use C4::Output qw( output_html_with_http_headers );
> use Koha::Database;
>+use Koha::Encryption;
> use Koha::Plugins;
> 
> our $input = CGI->new();
>@@ -36,11 +39,13 @@ our ( $template, $loggedinuser, $cookie ) = get_template_and_user(
>     }
> );
> 
>+my $crypt = Koha::Encryption->new;
>+
> my $op = $input->param('op');
> $op ||= 'display';
> 
> if ( $op eq 'acct_form' ) {
>-    show_account();
>+    show_account($crypt);
>     $template->param( acct_form => 1 );
>     my @vendors = $schema->resultset('Aqbookseller')->search(
>         undef,
>@@ -67,11 +72,13 @@ else {
> 
>         # validate & display
>         my $id     = $input->param('id');
>+        my $password = scalar $input->param('password');
>+        $password = $crypt->encrypt_hex($password);
>         my $fields = {
>             description        => scalar $input->param('description'),
>             host               => scalar $input->param('host'),
>             username           => scalar $input->param('username'),
>-            password           => scalar $input->param('password'),
>+            password           => $password,
>             vendor_id          => scalar $input->param('vendor_id'),
>             upload_directory   => scalar $input->param('upload_directory'),
>             download_directory => scalar $input->param('download_directory'),
>@@ -153,9 +160,11 @@ sub get_account {
> }
> 
> sub show_account {
>+    my $crypt = shift;
>     my $acct_id = $input->param('id');
>     if ($acct_id) {
>         my $acct = $schema->resultset('VendorEdiAccount')->find($acct_id);
>+        $acct->password( $crypt->decrypt_hex($acct->password) );
>         if ($acct) {
>             $template->param( account => $acct );
>         }
>diff --git a/installer/data/mysql/atomicupdate/bug_30649.pl b/installer/data/mysql/atomicupdate/bug_30649.pl
>new file mode 100755
>index 0000000000..68c4e2df4e
>--- /dev/null
>+++ b/installer/data/mysql/atomicupdate/bug_30649.pl
>@@ -0,0 +1,25 @@
>+use Modern::Perl;
>+
>+return {
>+    bug_number => "30649",
>+    description => "Increase the vendor EDI account password field to 256 characters",
>+    up => sub {
>+        my ($args) = @_;
>+        my ( $dbh, $out ) = @$args{qw(dbh out)};
>+        $dbh->do(q{
>+            ALTER TABLE vendor_edi_accounts CHANGE COLUMN `password` `password` varchar(256) COLLATE utf8mb4_unicode_ci DEFAULT NULL
>+        });
>+
>+        require Koha::Encryption;
>+        my $e = Koha::Encryption->new;
>+
>+        my $schema = Koha::Database->new()->schema();
>+        my $rs     = $schema->resultset('VendorEdiAccount')->search();
>+        while ( my $a = $rs->next() ) {
>+            my $password = $a->password;
>+            $password = $e->encrypt_hex($password);
>+            $a->password($password);
>+            $a->update();
>+        }
>+    },
>+};
>diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql
>index d831006cd5..0c5e561233 100644
>--- a/installer/data/mysql/kohastructure.sql
>+++ b/installer/data/mysql/kohastructure.sql
>@@ -5850,7 +5850,7 @@ CREATE TABLE `vendor_edi_accounts` (
>   `description` mediumtext NOT NULL,
>   `host` varchar(40) DEFAULT NULL,
>   `username` varchar(40) DEFAULT NULL,
>-  `password` varchar(40) DEFAULT NULL,
>+  `password` varchar(256) DEFAULT NULL,
>   `last_activity` date DEFAULT NULL,
>   `vendor_id` int(11) DEFAULT NULL,
>   `download_directory` mediumtext DEFAULT NULL,
>-- 
>2.30.2

Actions: View | Diff | Splinter Review

Attachments on bug 30649: 134298 | 134300 | 134301 | 134302 | 142056 | 142057 | 142820 | 142989 | 144323 | 144324 | 144325 | 152097