Attachment 134302 Details for Bug 30649 – Bug 30649 - Add ability to encrypt data in Koha, use for EDI vendor accounts

[patch] Bug 30649 - Add ability to encrypt data in Koha, use for EDI vendor accounts

Bug-30649---Add-ability-to-encrypt-data-in-Koha-us.patch (text/plain), 12.11 KB, created by Kyle M Hall (khall) on 2022-04-28 18:47:07 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Kyle M Hall (khall)

Created: 2022-04-28 18:47:07 UTC

Size: 12.11 KB

patch

obsolete

>From 18b789ea29aac9928d2b8e13821844a8311bfe48 Mon Sep 17 00:00:00 2001
>From: Kyle Hall <kyle@bywatersolutions.com>
>Date: Thu, 28 Apr 2022 14:29:29 -0400
>Subject: [PATCH] Bug 30649 - Add ability to encrypt data in Koha, use for EDI
> vendor accounts
>
>Right now, we store passwords for external services in plain text in the database. It would be good to have the option to encrypt those passwords.
>
>Test Plan:
>1) prove t/Crypt.t
>2) Create a new vendor edi account
>3) Note the password is encrypted
>4) Update the password for an existing EDI vendor account
>5) Note the password is now encrypted in the database
>6) Stretch goal, set up an FTP server, send an EDI order to the FTP
>   server where the vendor password is encrypted.
>---
> Koha/Crypt.pm                                 | 82 +++++++++++++++++++
> Koha/Edifact/Transport.pm                     | 25 +++---
> admin/edi_accounts.pl                         | 19 +++--
> debian/templates/koha-conf-site.xml.in        |  4 +
> etc/koha-conf.xml                             |  5 ++
> .../data/mysql/atomicupdate/bug_30649.pl      | 11 +++
> installer/data/mysql/kohastructure.sql        |  2 +-
> .../prog/en/modules/admin/edi_accounts.tt     |  2 +-
> t/Crypt.t                                     | 54 ++++++++++++
> t/secretfile                                  |  1 +
> 10 files changed, 188 insertions(+), 17 deletions(-)
> create mode 100644 Koha/Crypt.pm
> create mode 100755 installer/data/mysql/atomicupdate/bug_30649.pl
> create mode 100755 t/Crypt.t
> create mode 100644 t/secretfile
>
>diff --git a/Koha/Crypt.pm b/Koha/Crypt.pm
>new file mode 100644
>index 0000000000..08678accf5
>--- /dev/null
>+++ b/Koha/Crypt.pm
>@@ -0,0 +1,82 @@
>+package Koha::Crypt;
>+
>+# Copyright 2018, 2022 Koha Development Team
>+#
>+# This file is part of Koha.
>+#
>+# Koha is free software; you can redistribute it and/or modify it
>+# under the terms of the GNU General Public License as published by
>+# the Free Software Foundation; either version 3 of the License, or
>+# (at your option) any later version.
>+#
>+# Koha is distributed in the hope that it will be useful, but
>+# WITHOUT ANY WARRANTY; without even the implied warranty of
>+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>+# GNU General Public License for more details.
>+#
>+# You should have received a copy of the GNU General Public License
>+# along with Koha; if not, see <http://www.gnu.org/licenses>.
>+
>+use Modern::Perl;
>+
>+use C4::Context;
>+use Crypt::Simple passfile => C4::Context->config('secret');
>+
>+=head1 NAME
>+
>+Koha::Crypt - Class for encrypting data in Koha
>+
>+=head1 Crypts
>+
>+=head2 Koha::Crypt
>+
>+Simple encryption
>+
>+=head1 Class methods
>+
>+=head2 encrypt_if_needed
>+
>+$data = Koha::Crypt->encrypt_if_needed( $data )
>+
>+If path to a secret file is set in the Koha conf,
>+the data will be returned encrypted, otherwise the
>+data will be returned as is.
>+
>+Encrypted data is prefixed with 3 tab characters to denote
>+that it is encrypted.
>+
>+=cut
>+
>+sub encrypt_if_needed {
>+    my ( $class, $data ) = @_;
>+
>+    if ( C4::Context->config('secret') ) {
>+        return "\t\t\t" . encrypt($data);
>+    }
>+    else {
>+        return $data;
>+    }
>+}
>+
>+=head2 decrypt_if_needed
>+
>+$data = Koha::Crypt->decrypt_if_needed( $data )
>+
>+If path to a secret file is set in the Koha conf,
>+the data will be returned decrypted, otherwise the
>+data will be returned as is.
>+
>+=cut
>+
>+sub decrypt_if_needed {
>+    my ( $class, $data ) = @_;
>+
>+    if ( C4::Context->config('secret') && $data =~ /^\t\t\t/ ) {
>+        return decrypt($data);
>+    }
>+    else {
>+        return $data;
>+    }
>+}
>+
>+1;
>diff --git a/Koha/Edifact/Transport.pm b/Koha/Edifact/Transport.pm
>index 833c44e896..3da140ee06 100644
>--- a/Koha/Edifact/Transport.pm
>+++ b/Koha/Edifact/Transport.pm
>@@ -20,16 +20,21 @@ package Koha::Edifact::Transport;
> use strict;
> use warnings;
> use utf8;
>-use DateTime;
>+
>+use C4::Context;
>+use Koha::Database;
>+use Koha::DateUtils qw( dt_from_string );
>+use Koha::Crypt;
>+
> use Carp qw( carp );
>+use Crypt::Simple passfile => C4::Context->config('secret');
>+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 );
> 
> sub new {
>     my ( $class, $account_id ) = @_;
>@@ -136,7 +141,7 @@ sub sftp_download {
>     my $sftp = Net::SFTP::Foreign->new(
>         host     => $self->{account}->host,
>         user     => $self->{account}->username,
>-        password => $self->{account}->password,
>+        password => Koha::Crypt->decrypt_if_needed($self->{account}->password),
>         timeout  => 10,
>     );
>     if ( $sftp->error ) {
>@@ -221,7 +226,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::Crypt->decrypt_if_needed($self->{account}->password)i )
>       or return $self->_abort_download( $ftp, "Cannot login: $ftp->message()" );
>     $ftp->cwd( $self->{account}->download_directory )
>       or return $self->_abort_download( $ftp,
>@@ -262,7 +267,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::Crypt->decrypt_if_needed($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 +298,7 @@ sub sftp_upload {
>     my $sftp = Net::SFTP::Foreign->new(
>         host     => $self->{account}->host,
>         user     => $self->{account}->username,
>-        password => $self->{account}->password,
>+        password => Koha::Crypt->decrypt_if_needed($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..72e48bcb98 100755
>--- a/admin/edi_accounts.pl
>+++ b/admin/edi_accounts.pl
>@@ -21,6 +21,7 @@ use Modern::Perl;
> use CGI;
> use C4::Auth qw( get_template_and_user );
> use C4::Output qw( output_html_with_http_headers );
>+use Koha::Crypt;
> use Koha::Database;
> use Koha::Plugins;
> 
>@@ -88,13 +89,21 @@ else {
>         };
> 
>         if ($id) {
>-            $schema->resultset('VendorEdiAccount')->search(
>-                {
>-                    id => $id,
>-                }
>-            )->update_all($fields);
>+            my $account = $schema->resultset('VendorEdiAccount')->find($id);
>+
>+           # New password needs encrypted,
>+           # old password is already encrypted and should not be encrypted again
>+            if ( $account->password ne $fields->{password} ) {
>+                $fields->{password} =
>+                  Koha::Crypt->encrypt_if_needed( $fields->{password} );
>+            }
>+
>+            $account->update($fields);
>         }
>         else {    # new record
>+            $fields->{password} =
>+              Koha::Crypt->encrypt_if_needed( $fields->{password} );
>+
>             $schema->resultset('VendorEdiAccount')->create($fields);
>         }
>     }
>diff --git a/debian/templates/koha-conf-site.xml.in b/debian/templates/koha-conf-site.xml.in
>index 20d217222d..f63b5d4e53 100644
>--- a/debian/templates/koha-conf-site.xml.in
>+++ b/debian/templates/koha-conf-site.xml.in
>@@ -455,5 +455,9 @@ __END_SRU_PUBLICSERVER__
>      <do_not_remove_cookie>KohaOpacLanguage</do_not_remove_cookie>
>  -->
> 
>+ 
>+
> </config>
> </yazgfs>
>diff --git a/etc/koha-conf.xml b/etc/koha-conf.xml
>index 2c95e2c5eb..74c3849532 100644
>--- a/etc/koha-conf.xml
>+++ b/etc/koha-conf.xml
>@@ -271,5 +271,10 @@
>      <do_not_remove_cookie>KohaOpacLanguage</do_not_remove_cookie>
>  -->
> 
>+  
>+
>+
> </config>
> </yazgfs>
>diff --git a/installer/data/mysql/atomicupdate/bug_30649.pl b/installer/data/mysql/atomicupdate/bug_30649.pl
>new file mode 100755
>index 0000000000..e4443fcd30
>--- /dev/null
>+++ b/installer/data/mysql/atomicupdate/bug_30649.pl
>@@ -0,0 +1,11 @@
>+use Modern::Perl;
>+
>+return {
>+    bug_number => "30649",
>+    description => "Add ability to encrypt data in Koha, use for EDI vendor accounts",
>+    up => sub {
>+        my ($args) = @_;
>+        my ($dbh, $out) = @$args{qw(dbh out)};
>+        $dbh->do(q{ALTER TABLE vendor_edi_accounts CHANGE COLUMN `password` `password` varchar(128) NULL DEFAULT NULL});
>+    },
>+};
>diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql
>index 8f5f7b438f..571b980235 100644
>--- a/installer/data/mysql/kohastructure.sql
>+++ b/installer/data/mysql/kohastructure.sql
>@@ -5262,7 +5262,7 @@ CREATE TABLE `vendor_edi_accounts` (
>   `description` mediumtext COLLATE utf8mb4_unicode_ci NOT NULL,
>   `host` varchar(40) COLLATE utf8mb4_unicode_ci DEFAULT NULL,
>   `username` varchar(40) COLLATE utf8mb4_unicode_ci DEFAULT NULL,
>-  `password` varchar(40) COLLATE utf8mb4_unicode_ci DEFAULT NULL,
>+  `password` varchar(128) COLLATE utf8mb4_unicode_ci DEFAULT NULL,
>   `last_activity` date DEFAULT NULL,
>   `vendor_id` int(11) DEFAULT NULL,
>   `download_directory` mediumtext COLLATE utf8mb4_unicode_ci DEFAULT NULL,
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/edi_accounts.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/edi_accounts.tt
>index 4e37967413..96cea1237f 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/edi_accounts.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/edi_accounts.tt
>@@ -163,7 +163,7 @@ EDI accounts &rsaquo; Administration &rsaquo; Koha
>   </li>
>   <li>
>      <label for="password">Password: </label>
>-     <input type="text" name="password" id="password" size="20" maxlength="40" value="[% account.password | html %]" />
>+     <input type="password" name="password" id="password" size="20" maxlength="40" value="[% account.password | html %]" />
>   </li>
>   <li>
>      <label for="download_directory">Download directory: </label>
>diff --git a/t/Crypt.t b/t/Crypt.t
>new file mode 100755
>index 0000000000..6b973db333
>--- /dev/null
>+++ b/t/Crypt.t
>@@ -0,0 +1,54 @@
>+#!/usr/bin/perl
>+
>+# This file is part of Koha.
>+#
>+# Koha is free software; you can redistribute it and/or modify it
>+# under the terms of the GNU General Public License as published by
>+# the Free Software Foundation; either version 3 of the License, or
>+# (at your option) any later version.
>+#
>+# Koha is distributed in the hope that it will be useful, but
>+# WITHOUT ANY WARRANTY; without even the implied warranty of
>+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>+# GNU General Public License for more details.
>+#
>+# You should have received a copy of the GNU General Public License
>+# along with Koha; if not, see <http://www.gnu.org/licenses>.
>+
>+use Modern::Perl;
>+
>+use t::lib::Mocks;
>+
>+use Test::More tests => 6;
>+
>+use Cwd qw(abs_path);
>+use File::Basename;
>+my $dirname = dirname(__FILE__);
>+my $secret_path = abs_path("$dirname/secretfile");
>+
>+t::lib::Mocks::mock_config('secret', $secret_path);
>+
>+use_ok('Koha::Crypt');
>+
>+my $data = "some password";
>+my $encrypted = Koha::Crypt->encrypt_if_needed($data);
>+
>+isnt($data, $encrypted, "Data appears to be encrypted");
>+
>+my $decrypted = Koha::Crypt->decrypt_if_needed($encrypted);
>+
>+is( $decrypted, $data, "Data was decrypted" );
>+
>+# Test with decryption disabled
>+t::lib::Mocks::mock_config('secret', "");
>+
>+use_ok('Koha::Crypt');
>+
>+$data = "some other password";
>+$encrypted = Koha::Crypt->encrypt_if_needed($data);
>+
>+is($data, $encrypted, "Data was not encrypted");
>+
>+$decrypted = Koha::Crypt->decrypt_if_needed($encrypted);
>+
>+is( $decrypted, $data, "Data was never encrypted" );
>diff --git a/t/secretfile b/t/secretfile
>new file mode 100644
>index 0000000000..d97c5eada5
>--- /dev/null
>+++ b/t/secretfile
>@@ -0,0 +1 @@
>+secret
>-- 
>2.30.2

Actions: View | Diff | Splinter Review

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