From 4e355dc3964240d316e205401c4f260ff726e34c Mon Sep 17 00:00:00 2001
From: Jake Deery <jake.deery@ptfs-europe.com>
Date: Fri, 29 Nov 2024 16:31:53 +0000
Subject: [PATCH] Bug 35761: Integrate EDI into new FTP/SFTP management UI

This patch integrates the EDI Transport.pm module into the new FTP/SFTP
management UI by tweaking the existing methods to look at the new SftpServer
object, instead of its own.

Included also is a change to the database structure to make use of the new
sftp_servers table, as well as an atomic update to copy the schema changes,
and existing config to the new sftp_servers table.
---
 Koha/Edifact/Transport.pm                     | 113 ++++++++++++------
 Koha/Schema/Result/SftpServer.pm              |  36 +++++-
 Koha/Schema/Result/VendorEdiAccount.pm        |  90 +++++++-------
 admin/edi_accounts.pl                         |  67 +++++------
 ...38489-move_edi_SFTP_data_to_SFTP_tables.pl | 111 +++++++++++++++++
 installer/data/mysql/kohastructure.sql        |  12 +-
 .../prog/en/modules/admin/edi_accounts.tt     |  91 +++++++-------
 t/db_dependent/Koha/Edifact/Transport.t       |   4 +-
 8 files changed, 358 insertions(+), 166 deletions(-)
 create mode 100755 installer/data/mysql/atomicupdate/bug_38489-move_edi_SFTP_data_to_SFTP_tables.pl

diff --git a/Koha/Edifact/Transport.pm b/Koha/Edifact/Transport.pm
index ebc5d801c87..64bd0f8997e 100644
--- a/Koha/Edifact/Transport.pm
+++ b/Koha/Edifact/Transport.pm
@@ -32,7 +32,7 @@ use Net::SFTP::Foreign;
 
 use Koha::Database;
 use Koha::DateUtils qw( dt_from_string );
-use Koha::Encryption;
+use Koha::SFTP::Servers;
 
 sub new {
     my ( $class, $account_id ) = @_;
@@ -62,33 +62,44 @@ sub download_messages {
     my ( $self, $message_type ) = @_;
     $self->{message_type} = $message_type;
 
+    my $sftp_server_id = $self->{account}->download_sftp_server_id;
+    my $sftp_server_transport;
+    $sftp_server_transport = Koha::SFTP::Servers->find($sftp_server_id)->transport
+        if ( $sftp_server_id );
+
     my @retrieved_files;
 
-    if ( $self->{account}->transport eq 'SFTP' ) {
-        @retrieved_files = $self->sftp_download();
-    }
-    elsif ( $self->{account}->transport eq 'FILE' ) {
-        @retrieved_files = $self->file_download();
-    }
-    else {    # assume FTP
-        @retrieved_files = $self->ftp_download();
+    if ($message_type) {
+        if ( !$sftp_server_transport ) {
+            @retrieved_files = $self->file_download();
+        } elsif ( $sftp_server_transport eq 'sftp' ) {
+            @retrieved_files = $self->sftp_download();
+        } elsif ( $sftp_server_transport eq 'ftp' ) {
+            @retrieved_files = $self->ftp_download();
+        }
     }
+
     return @retrieved_files;
 }
 
 sub upload_messages {
     my ( $self, @messages ) = @_;
+
+    my $sftp_server_id = $self->{account}->upload_sftp_server_id;
+    my $sftp_server_transport;
+    $sftp_server_transport = Koha::SFTP::Servers->find($sftp_server_id)->transport
+        if ( $sftp_server_id );
+
     if (@messages) {
-        if ( $self->{account}->transport eq 'SFTP' ) {
+        if ( !$sftp_server_transport ) {
+            $self->ftp_upload(@messages);
+        } elsif ( $sftp_server_transport eq 'sftp' ) {
             $self->sftp_upload(@messages);
-        }
-        elsif ( $self->{account}->transport eq 'FILE' ) {
+        } elsif ( $sftp_server_transport eq 'ftp' ) {
             $self->file_upload(@messages);
         }
-        else {    # assume FTP
-            $self->ftp_upload(@messages);
-        }
     }
+
     return;
 }
 
@@ -131,17 +142,23 @@ sub file_download {
 sub sftp_download {
     my $self = shift;
 
+    my $sftp_server_id = $self->{account}->download_sftp_server_id;
+    return
+        unless $sftp_server_id;
+    my $sftp_server = Koha::SFTP::Servers->find($sftp_server_id);
+    return
+        unless $sftp_server;
+
     my $file_ext = _get_file_ext( $self->{message_type} );
 
     # C = ready to retrieve E = Edifact
     my $msg_hash = $self->message_hash();
     my @downloaded_files;
-    my $port = $self->{account}->download_port ? $self->{account}->download_port : '22';
     my $sftp = Net::SFTP::Foreign->new(
-        host     => $self->{account}->host,
-        user     => $self->{account}->username,
-        password => Koha::Encryption->new->decrypt_hex($self->{account}->password),
-        port     => $port,
+        host     => $sftp_server->host,
+        user     => $sftp_server->user_name,
+        password => $sftp_server->plain_text_password,
+        port     => $sftp_server->port,
         timeout  => 10,
     );
     if ( $sftp->error ) {
@@ -213,24 +230,30 @@ sub ingest {
 sub ftp_download {
     my $self = shift;
 
+    my $sftp_server_id = $self->{account}->download_sftp_server_id;
+    return
+        unless $sftp_server_id;
+    my $sftp_server = Koha::SFTP::Servers->find($sftp_server_id);
+    return
+        unless $sftp_server;
+
     my $file_ext = _get_file_ext( $self->{message_type} );
 
     # C = ready to retrieve E = Edifact
 
     my $msg_hash = $self->message_hash();
     my @downloaded_files;
-    my $port = $self->{account}->download_port ? $self->{account}->download_port : '21';
     my $ftp  = Net::FTP->new(
-        $self->{account}->host,
-        Port    => $port,
+        $sftp_server->host,
+        Port    => $sftp_server->port,
         Timeout => 10,
-        Passive => 1
+        Passive => $sftp_server->passiv,
         )
         or return $self->_abort_download(
         undef,
-        "Cannot connect to $self->{account}->host: $EVAL_ERROR"
+        "Cannot connect to " . $sftp_server->host . ": " . $EVAL_ERROR
         );
-    $ftp->login( $self->{account}->username, Koha::Encryption->new->decrypt_hex($self->{account}->password) )
+    $ftp->login( $sftp_server->user_name, $sftp_server->plain_text_password )
       or return $self->_abort_download( $ftp, "Cannot login: $ftp->message()" );
     $ftp->cwd( $self->{account}->download_directory )
       or return $self->_abort_download( $ftp,
@@ -264,18 +287,25 @@ sub ftp_download {
 
 sub ftp_upload {
     my ( $self, @messages ) = @_;
-    my $port = $self->{account}->upload_port ? $self->{account}->upload_port : '21';
+
+    my $sftp_server_id = $self->{account}->upload_sftp_server_id;
+    return
+        unless $sftp_server_id;
+    my $sftp_server = Koha::SFTP::Servers->find($sftp_server_id);
+    return
+        unless $sftp_server;
+
     my $ftp  = Net::FTP->new(
-        $self->{account}->host,
-        Port    => $port,
+        $sftp_server->host,
+        Port    => $sftp_server->port,
         Timeout => 10,
-        Passive => 1
+        Passive => $sftp_server->passiv
         )
         or return $self->_abort_download(
         undef,
-        "Cannot connect to $self->{account}->host: $EVAL_ERROR"
+        "Cannot connect to " . $sftp_server->host . ": " . $EVAL_ERROR
         );
-    $ftp->login( $self->{account}->username, Koha::Encryption->new->decrypt_hex($self->{account}->password) )
+    $ftp->login( $sftp_server->user_name, $sftp_server->plain_text_password )
       or return $self->_abort_download( $ftp, "Cannot login: $ftp->message()" );
     $ftp->cwd( $self->{account}->upload_directory )
       or return $self->_abort_download( $ftp,
@@ -303,15 +333,22 @@ sub ftp_upload {
 
 sub sftp_upload {
     my ( $self, @messages ) = @_;
-    my $port = $self->{account}->upload_port ? $self->{account}->upload_port : '22';
+
+    my $sftp_server_id = $self->{account}->upload_sftp_server_id;
+    return
+        unless $sftp_server_id;
+    my $sftp_server = Koha::SFTP::Servers->find($sftp_server_id);
+    return
+        unless $sftp_server;
+
     my $sftp = Net::SFTP::Foreign->new(
-        host     => $self->{account}->host,
-        user     => $self->{account}->username,
-        password => Koha::Encryption->new->decrypt_hex($self->{account}->password),
-        port     => $port,
+        host     => $sftp_server->host,
+        user     => $sftp_server->user_name,
+        password => $sftp_server->plain_text_password,
+        port     => $sftp_server->port,
         timeout  => 10,
     );
-    $sftp->die_on_error("Cannot ssh to $self->{account}->host");
+    $sftp->die_on_error("Cannot ssh to " . $sftp_server->host);
     $sftp->setcwd( $self->{account}->upload_directory )
       or return $self->_abort_download( $sftp,
         "Cannot change remote dir : " . $sftp->error );
diff --git a/Koha/Schema/Result/SftpServer.pm b/Koha/Schema/Result/SftpServer.pm
index 33bb3e76f91..fc72090055b 100644
--- a/Koha/Schema/Result/SftpServer.pm
+++ b/Koha/Schema/Result/SftpServer.pm
@@ -146,9 +146,41 @@ __PACKAGE__->add_columns(
 
 __PACKAGE__->set_primary_key("id");
 
+=head1 RELATIONS
 
-# Created by DBIx::Class::Schema::Loader v0.07051 @ 2024-12-02 11:43:19
-# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:K1+15gYfWmqYKldm0NdgGQ
+=head2 vendor_edi_accounts_download_sftp_servers
+
+Type: has_many
+
+Related object: L<Koha::Schema::Result::VendorEdiAccount>
+
+=cut
+
+__PACKAGE__->has_many(
+  "vendor_edi_accounts_download_sftp_servers",
+  "Koha::Schema::Result::VendorEdiAccount",
+  { "foreign.download_sftp_server_id" => "self.id" },
+  { cascade_copy => 0, cascade_delete => 0 },
+);
+
+=head2 vendor_edi_accounts_upload_sftp_servers
+
+Type: has_many
+
+Related object: L<Koha::Schema::Result::VendorEdiAccount>
+
+=cut
+
+__PACKAGE__->has_many(
+  "vendor_edi_accounts_upload_sftp_servers",
+  "Koha::Schema::Result::VendorEdiAccount",
+  { "foreign.upload_sftp_server_id" => "self.id" },
+  { cascade_copy => 0, cascade_delete => 0 },
+);
+
+
+# Created by DBIx::Class::Schema::Loader v0.07051 @ 2024-12-02 15:04:20
+# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:obHhjrZvl6IUrRYeUaIn4A
 
 __PACKAGE__->add_columns(
     '+passiv'     => { is_boolean => 1 },
diff --git a/Koha/Schema/Result/VendorEdiAccount.pm b/Koha/Schema/Result/VendorEdiAccount.pm
index 27e52b8477c..6e57f0b1ca1 100644
--- a/Koha/Schema/Result/VendorEdiAccount.pm
+++ b/Koha/Schema/Result/VendorEdiAccount.pm
@@ -34,31 +34,16 @@ __PACKAGE__->table("vendor_edi_accounts");
   data_type: 'mediumtext'
   is_nullable: 0
 
-=head2 host
-
-  data_type: 'varchar'
-  is_nullable: 1
-  size: 40
-
-=head2 username
-
-  data_type: 'varchar'
-  is_nullable: 1
-  size: 40
-
-=head2 password
-
-  data_type: 'mediumtext'
-  is_nullable: 1
-
-=head2 upload_port
+=head2 download_sftp_server_id
 
   data_type: 'integer'
+  is_foreign_key: 1
   is_nullable: 1
 
-=head2 download_port
+=head2 upload_sftp_server_id
 
   data_type: 'integer'
+  is_foreign_key: 1
   is_nullable: 1
 
 =head2 last_activity
@@ -103,13 +88,6 @@ __PACKAGE__->table("vendor_edi_accounts");
   is_nullable: 1
   size: 3
 
-=head2 transport
-
-  data_type: 'varchar'
-  default_value: 'FTP'
-  is_nullable: 1
-  size: 6
-
 =head2 quotes_enabled
 
   data_type: 'tinyint'
@@ -160,16 +138,10 @@ __PACKAGE__->add_columns(
   { data_type => "integer", is_auto_increment => 1, is_nullable => 0 },
   "description",
   { data_type => "mediumtext", is_nullable => 0 },
-  "host",
-  { data_type => "varchar", is_nullable => 1, size => 40 },
-  "username",
-  { data_type => "varchar", is_nullable => 1, size => 40 },
-  "password",
-  { data_type => "mediumtext", is_nullable => 1 },
-  "upload_port",
-  { data_type => "integer", is_nullable => 1 },
-  "download_port",
-  { data_type => "integer", is_nullable => 1 },
+  "download_sftp_server_id",
+  { data_type => "integer", is_foreign_key => 1, is_nullable => 1 },
+  "upload_sftp_server_id",
+  { data_type => "integer", is_foreign_key => 1, is_nullable => 1 },
   "last_activity",
   { data_type => "date", datetime_undef_if_invalid => 1, is_nullable => 1 },
   "vendor_id",
@@ -184,8 +156,6 @@ __PACKAGE__->add_columns(
   { data_type => "varchar", default_value => "EUR", is_nullable => 1, size => 3 },
   "id_code_qualifier",
   { data_type => "varchar", default_value => 14, is_nullable => 1, size => 3 },
-  "transport",
-  { data_type => "varchar", default_value => "FTP", is_nullable => 1, size => 6 },
   "quotes_enabled",
   { data_type => "tinyint", default_value => 0, is_nullable => 0 },
   "invoices_enabled",
@@ -216,6 +186,26 @@ __PACKAGE__->set_primary_key("id");
 
 =head1 RELATIONS
 
+=head2 download_sftp_server
+
+Type: belongs_to
+
+Related object: L<Koha::Schema::Result::SftpServer>
+
+=cut
+
+__PACKAGE__->belongs_to(
+  "download_sftp_server",
+  "Koha::Schema::Result::SftpServer",
+  { id => "download_sftp_server_id" },
+  {
+    is_deferrable => 1,
+    join_type     => "LEFT",
+    on_delete     => "RESTRICT",
+    on_update     => "RESTRICT",
+  },
+);
+
 =head2 edifact_messages
 
 Type: has_many
@@ -251,6 +241,26 @@ __PACKAGE__->belongs_to(
   },
 );
 
+=head2 upload_sftp_server
+
+Type: belongs_to
+
+Related object: L<Koha::Schema::Result::SftpServer>
+
+=cut
+
+__PACKAGE__->belongs_to(
+  "upload_sftp_server",
+  "Koha::Schema::Result::SftpServer",
+  { id => "upload_sftp_server_id" },
+  {
+    is_deferrable => 1,
+    join_type     => "LEFT",
+    on_delete     => "RESTRICT",
+    on_update     => "RESTRICT",
+  },
+);
+
 =head2 vendor
 
 Type: belongs_to
@@ -272,8 +282,8 @@ __PACKAGE__->belongs_to(
 );
 
 
-# Created by DBIx::Class::Schema::Loader v0.07049 @ 2024-04-19 16:25:44
-# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:jenUF3Wx7J7F5270mLQMbw
+# Created by DBIx::Class::Schema::Loader v0.07051 @ 2024-12-02 15:04:20
+# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:rej2ARtjUUHS8AB4H6KuUw
 
 
 # You can replace this text with custom code or comments, and it will be preserved on regeneration
diff --git a/admin/edi_accounts.pl b/admin/edi_accounts.pl
index 4d7ae364f55..90514149d21 100755
--- a/admin/edi_accounts.pl
+++ b/admin/edi_accounts.pl
@@ -24,7 +24,6 @@ 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();
@@ -39,13 +38,11 @@ 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($crypt);
+    show_account();
     $template->param( acct_form => 1 );
     my @vendors = $schema->resultset('Aqbookseller')->search(
         undef,
@@ -56,6 +53,15 @@ if ( $op eq 'acct_form' ) {
     );
     $template->param( vendors => \@vendors );
 
+    my @sftp_servers = $schema->resultset('SftpServer')->search(
+        undef,
+        {
+            columns  => [ 'name', 'host', 'port', 'id' ],
+            order_by => { -asc => 'name' },
+        }
+    );
+    $template->param( sftp_servers => \@sftp_servers );
+
     if ( C4::Context->config("enable_plugins") ) {
         my @plugins = Koha::Plugins->new()->GetPlugins({
             method => 'edifact',
@@ -64,7 +70,7 @@ if ( $op eq 'acct_form' ) {
     }
 }
 elsif ( $op eq 'delete_confirm' ) {
-    show_account($crypt);
+    show_account();
     $template->param( delete_confirm => 1 );
 }
 else {
@@ -72,28 +78,22 @@ 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           => $password,
-            upload_port        => scalar $input->param('upload_port')   || 22,
-            download_port      => scalar $input->param('download_port') || 22,
-            vendor_id          => scalar $input->param('vendor_id'),
-            upload_directory   => scalar $input->param('upload_directory'),
-            download_directory => scalar $input->param('download_directory'),
-            san                => scalar $input->param('san'),
-            standard           => scalar $input->param('standard'),
-            transport          => scalar $input->param('transport'),
-            quotes_enabled     => $input->param('quotes_enabled') ? 1 : 0,
-            invoices_enabled   => $input->param('invoices_enabled') ? 1 : 0,
-            orders_enabled     => $input->param('orders_enabled') ? 1 : 0,
-            responses_enabled  => $input->param('responses_enabled') ? 1 : 0,
-            auto_orders        => $input->param('auto_orders') ? 1 : 0,
-            id_code_qualifier  => scalar $input->param('id_code_qualifier'),
-            plugin             => scalar $input->param('plugin'),
+            description             => scalar $input->param('description'),
+            upload_sftp_server_id   => $input->param('upload_sftp_server_id') || undef,
+            download_sftp_server_id => $input->param('download_sftp_server_id') || undef,
+            vendor_id               => scalar $input->param('vendor_id'),
+            upload_directory        => scalar $input->param('upload_directory'),
+            download_directory      => scalar $input->param('download_directory'),
+            san                     => scalar $input->param('san'),
+            standard                => scalar $input->param('standard'),
+            quotes_enabled          => $input->param('quotes_enabled') ? 1 : 0,
+            invoices_enabled        => $input->param('invoices_enabled') ? 1 : 0,
+            orders_enabled          => $input->param('orders_enabled') ? 1 : 0,
+            responses_enabled       => $input->param('responses_enabled') ? 1 : 0,
+            auto_orders             => $input->param('auto_orders') ? 1 : 0,
+            id_code_qualifier       => scalar $input->param('id_code_qualifier'),
+            plugin                  => scalar $input->param('plugin'),
         };
 
         if ($id) {
@@ -162,14 +162,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 );
-        }
-    }
-    return;
+    return unless $acct_id;
+
+    my $acct = $schema->resultset('VendorEdiAccount')->find($acct_id);
+    return unless $acct;
+    
+    return $template->param( account => $acct );
 }
diff --git a/installer/data/mysql/atomicupdate/bug_38489-move_edi_SFTP_data_to_SFTP_tables.pl b/installer/data/mysql/atomicupdate/bug_38489-move_edi_SFTP_data_to_SFTP_tables.pl
new file mode 100755
index 00000000000..f92bff903e0
--- /dev/null
+++ b/installer/data/mysql/atomicupdate/bug_38489-move_edi_SFTP_data_to_SFTP_tables.pl
@@ -0,0 +1,111 @@
+use Modern::Perl;
+
+return {
+    bug_number  => "38489",
+    description => "Move SFTP data from EDI module to FTP/SFTP table",
+    up          => sub {
+        my ($args)        = @_;
+        my ( $dbh, $out ) = @$args{qw(dbh out)};
+
+        if ( TableExists('sftp_servers') ) {
+            if ( column_exists( 'vendor_edi_accounts', 'host')
+            && column_exists( 'vendor_edi_accounts', 'download_port' )
+            && column_exists( 'vendor_edi_accounts', 'upload_port' )
+            && column_exists( 'vendor_edi_accounts', 'transport' )
+            && column_exists( 'vendor_edi_accounts', 'username' )
+            && column_exists( 'vendor_edi_accounts', 'password' ) )
+            {
+                ## copy edi vendor sftp servers to sftp_servers
+                $dbh->do(
+                    q{
+                        INSERT IGNORE INTO `sftp_servers` (name,host,port,transport,user_name,password)
+                        SELECT description,host,download_port,LOWER(transport),username,password
+                        FROM vendor_edi_accounts
+                        WHERE vendor_edi_accounts.download_port IS NOT NULL;
+                    }
+                );
+                say $out "Copied FTP/SFTP download servers from vendor_edi_accounts table";
+
+                $dbh->do(
+                    q{
+                        INSERT IGNORE INTO `sftp_servers` (name,host,port,transport,user_name,password)
+                        SELECT description,host,upload_port,LOWER(transport),username,password
+                        FROM vendor_edi_accounts
+                        WHERE vendor_edi_accounts.upload_port IS NOT NULL;
+                    }
+                );
+                say $out "Copied FTP/SFTP upload servers from vendor_edi_accounts table";
+
+                ## add foreign keys
+                unless ( column_exists( 'vendor_edi_accounts', 'download_sftp_server_id' )
+                    || column_exists( 'vendor_edi_accounts', 'upload_sftp_server_id' ) )
+                {
+                    $dbh->do(
+                        q{
+                            ALTER TABLE `vendor_edi_accounts`
+                                ADD COLUMN `download_sftp_server_id` int(11) DEFAULT NULL AFTER `description`,
+                                ADD COLUMN `upload_sftp_server_id` int(11) DEFAULT NULL AFTER `download_sftp_server_id`;
+                        }
+                    );
+                    say $out "Added download_sftp_server_id column in vendor_edi_accounts table";
+                    say $out "Added upload_sftp_server_id column in vendor_edi_accounts table";
+                }
+                unless ( foreign_key_exists( 'vendor_edi_accounts', 'vfk_download_sftp_server_id' )
+                    || foreign_key_exists( 'vendor_edi_accounts', 'vfk_upload_sftp_server_id' ) )
+                {
+                    $dbh->do(
+                        q{
+                            ALTER TABLE `vendor_edi_accounts`
+                                ADD CONSTRAINT `vfk_download_sftp_server_id` FOREIGN KEY (`download_sftp_server_id`) REFERENCES `sftp_servers` (`id`),
+                                ADD CONSTRAINT `vfk_upload_sftp_server_id` FOREIGN KEY (`upload_sftp_server_id`) REFERENCES `sftp_servers` (`id`);
+                        }
+                    );
+                    say $out "Added vfk_download_sftp_server_id constraint in vendor_edi_accounts table";
+                    say $out "Added vfk_upload_sftp_server_id constraint in vendor_edi_accounts table";
+                }
+
+                ## set matching sftp server ids in new foreign keys
+                $dbh->do(
+                    q{
+                        UPDATE IGNORE `vendor_edi_accounts`, `sftp_servers`
+                            SET vendor_edi_accounts.download_sftp_server_id = sftp_servers.id
+                            WHERE vendor_edi_accounts.host = sftp_servers.host
+                            AND vendor_edi_accounts.download_port = sftp_servers.port
+                            AND vendor_edi_accounts.transport = sftp_servers.transport;
+                    }
+                );
+                say $out "Matched sftp_server.id with vendor_edi_accounts.download_sftp_server_id";
+                $dbh->do(
+                    q{
+                        UPDATE IGNORE `vendor_edi_accounts`, `sftp_servers`
+                            SET vendor_edi_accounts.upload_sftp_server_id = sftp_servers.id
+                            WHERE vendor_edi_accounts.host = sftp_servers.host
+                            AND vendor_edi_accounts.upload_port = sftp_servers.port
+                            AND vendor_edi_accounts.transport = sftp_servers.transport;
+                    }
+                );
+                say $out "Matched sftp_server.id with vendor_edi_accounts.upload_sftp_server_id";
+
+
+                ## drop useless columns from edi vendors
+                $dbh->do(
+                    q{
+                        ALTER TABLE vendor_edi_accounts
+                            DROP COLUMN `host`,
+                            DROP COLUMN `username`,
+                            DROP COLUMN `password`,
+                            DROP COLUMN `download_port`,
+                            DROP COLUMN `upload_port`,
+                            DROP COLUMN `transport`;
+                    }
+                );
+                say $out "Dropped host column in vendor_edi_accounts table";
+                say $out "Dropped username column in vendor_edi_accounts table";
+                say $out "Dropped password column in vendor_edi_accounts table";
+                say $out "Dropped download_port column in vendor_edi_accounts table";
+                say $out "Dropped upload_port column in vendor_edi_accounts table";
+                say $out "Dropped transport column in vendor_edi_accounts table";
+            }
+        }
+    },
+};
diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql
index f805ba76079..bee77d7f5c0 100644
--- a/installer/data/mysql/kohastructure.sql
+++ b/installer/data/mysql/kohastructure.sql
@@ -6626,11 +6626,8 @@ DROP TABLE IF EXISTS `vendor_edi_accounts`;
 CREATE TABLE `vendor_edi_accounts` (
   `id` int(11) NOT NULL AUTO_INCREMENT,
   `description` mediumtext NOT NULL,
-  `host` varchar(40) DEFAULT NULL,
-  `username` varchar(40) DEFAULT NULL,
-  `password` mediumtext DEFAULT NULL,
-  `upload_port` int(11) DEFAULT NULL,
-  `download_port` int(11) DEFAULT NULL,
+  `download_sftp_server_id` int(11) DEFAULT NULL,
+  `upload_sftp_server_id` int(11) DEFAULT NULL,
   `last_activity` date DEFAULT NULL,
   `vendor_id` int(11) DEFAULT NULL,
   `download_directory` mediumtext DEFAULT NULL,
@@ -6638,7 +6635,6 @@ CREATE TABLE `vendor_edi_accounts` (
   `san` varchar(20) DEFAULT NULL,
   `standard` varchar(3) DEFAULT 'EUR',
   `id_code_qualifier` varchar(3) DEFAULT '14',
-  `transport` varchar(6) DEFAULT 'FTP',
   `quotes_enabled` tinyint(1) NOT NULL DEFAULT 0,
   `invoices_enabled` tinyint(1) NOT NULL DEFAULT 0,
   `orders_enabled` tinyint(1) NOT NULL DEFAULT 0,
@@ -6649,7 +6645,11 @@ CREATE TABLE `vendor_edi_accounts` (
   PRIMARY KEY (`id`),
   KEY `vendorid` (`vendor_id`),
   KEY `shipmentbudget` (`shipment_budget`),
+  KEY `vfk_download_sftp_server_id` (`download_sftp_server_id`),
+  KEY `vfk_upload_sftp_server_id` (`upload_sftp_server_id`),
+  CONSTRAINT `vfk_download_sftp_server_id` FOREIGN KEY (`download_sftp_server_id`) REFERENCES `sftp_servers` (`id`),
   CONSTRAINT `vfk_shipment_budget` FOREIGN KEY (`shipment_budget`) REFERENCES `aqbudgets` (`budget_id`),
+  CONSTRAINT `vfk_upload_sftp_server_id` FOREIGN KEY (`upload_sftp_server_id`) REFERENCES `sftp_servers` (`id`),
   CONSTRAINT `vfk_vendor_id` FOREIGN KEY (`vendor_id`) REFERENCES `aqbooksellers` (`id`)
 ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;
 /*!40101 SET character_set_client = @saved_cs_client */;
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 6068ebcfe56..8f7c1abeca5 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
@@ -135,40 +135,35 @@
      <input type="text" name="description" id="description" size="20" value="[% account.description | html %]" />
   </li>
   <li>
-     [% transport_types = [ 'FTP', 'SFTP', 'FILE' ] %]
-     <label for="transport">Transport: </label>
-     <select name="transport" title="valid types of transport are FTP and SFTP"
-      id="transport">
-      [% FOREACH transport_type IN transport_types %]
-           [% IF transport_type == account.transport %]
-              <option value="[% transport_type | html %]" selected="selected">[% transport_type | html %]</option>
-           [% ELSE %]
-              <option value="[% transport_type | html %]">[% transport_type | html %]</option>
-           [% END %]
-       [% END %]
-     </select>
-  </li>
-  <li>
-     <label for="host">Remote host: </label>
-     <input type="text" name="host" id="host" size="20" maxlength="40" value="[% account.host | html %]" />
-  </li>
-  <li>
-     <label for="username">Username: </label>
-     <input type="text" name="username" id="username" size="20" maxlength="40" value="[% account.username | html %]" />
-  </li>
-  <li>
-     <label for="password">Password: </label>
-     <input type="text" name="password" id="password" size="20" maxlength="40" value="[% account.password | html %]" />
-  </li>
-  <li>
-     <label for="upload_port">Upload port: </label>
-     <input type="text" name="upload_port" id="upload_port" size="20" maxlength="40" value="[% account.upload_port | html %]" />
-        <div class="hint">The upload port will default to 22 if not set.</div>
+     <label for="upload_sftp_server_id">Upload FTP/SFTP server: </label>
+     <select name="upload_sftp_server_id" id="upload_sftp_server_id">
+        <option value="">(do not use FTP/SFTP Server)</option>
+        [% FOREACH sftp_server IN sftp_servers %]
+        [% IF account.upload_sftp_server.id == sftp_server.id %]
+            <option value="[% sftp_server.id | html %]" selected="selected">
+        [% ELSE %]
+            <option value="[% sftp_server.id | html %]">
+        [% END %]
+            [% sftp_server.name | html %] ([% sftp_server.host | html %]:[% sftp_server.port | html %])
+        </option>
+        [% END %]
+    </select>
   </li>
   <li>
-     <label for="download_port">Download port: </label>
-     <input type="text" name="download_port" id="download_port" size="20" maxlength="40" value="[% account.download_port | html %]" />
-        <div class="hint">The download port will default to 22 if not set.</div>
+    <label for="download_sftp_server_id">Download FTP/SFTP server: </label>
+    <select name="download_sftp_server_id" id="download_sftp_server_id">
+        <option value="">(do not use FTP/SFTP Server)</option>
+        [% FOREACH sftp_server IN sftp_servers %]
+        [% IF account.download_sftp_server.id == sftp_server.id %]
+            <option value="[% sftp_server.id | html %]" selected="selected">
+        [% ELSE %]
+            <option value="[% sftp_server.id | html %]">
+        [% END %]
+            [% sftp_server.name | html %] ([% sftp_server.host | html %]:[% sftp_server.port | html %])
+        </option>
+        [% END %]
+    </select>
+        <div class="hint">Configure under <a href="/cgi-bin/koha/admin/sftp_servers.pl">FTP/SFTP Servers</a></div>
   </li>
   <li>
      <label for="download_directory">Download directory: </label>
@@ -313,12 +308,8 @@
        <th>ID</th>
        <th>Vendor</th>
        <th>Description</th>
-       <th>Transport</th>
-       <th>Remote host</th>
-       <th>Username</th>
-       <th>Password</th>
-       <th>Upload port</th>
-       <th>Download port</th>
+       <th>Upload FTP/SFTP server</th>
+       <th>Download FTP/SFTP server</th>
        <th>Download directory</th>
        <th>Upload directory</th>
        <th>Qualifier</th>
@@ -336,12 +327,24 @@
       <td>[% account.id | html %]</td>
       <td><a href="/cgi-bin/koha/acqui/supplier.pl?booksellerid=[% account.vendor_id | uri %]">[% account.vendor.name | html %]</a></td>
       <td>[% account.description | html %]</td>
-      <td>[% account.transport | html %]</td>
-      <td>[% account.host | html %]</td>
-      <td>[% account.username | html %]</td>
-      <td>[% IF account.password %]*****[% END %]</td>
-      <td>[% account.upload_port | html %]</td>
-      <td>[% account.download_port | html %]</td>
+      <td>
+        [% IF (account.upload_sftp_server) %]
+        <a href="/cgi-bin/koha/admin/sftp_servers.pl?op=edit_form&sftp_server_id=[% account.upload_sftp_server.id | html %]">
+            [% account.upload_sftp_server.name | html %]
+        </a>
+        [% ELSE %]
+        <em>None set</em>
+        [% END %]
+      </td>
+      <td>
+        [% IF (account.download_sftp_server) %]
+        <a href="/cgi-bin/koha/admin/sftp_servers.pl?op=edit_form&sftp_server_id=[% account.download_sftp_server.id | html %]">
+            [% account.download_sftp_server.name | html %]
+        </a>
+        [% ELSE %]
+        <em>None set</em>
+        [% END %]
+      </td>
       <td>[% account.download_directory | html %]</td>
       <td>[% account.upload_directory | html %]</td>
       <td>
diff --git a/t/db_dependent/Koha/Edifact/Transport.t b/t/db_dependent/Koha/Edifact/Transport.t
index 83f05afa68b..25212ef220b 100755
--- a/t/db_dependent/Koha/Edifact/Transport.t
+++ b/t/db_dependent/Koha/Edifact/Transport.t
@@ -20,7 +20,9 @@ my $account = $builder->build(
     {
         source => 'VendorEdiAccount',
         value  => {
-            description => 'test vendor', transport => 'FILE',
+            description             => 'test vendor',
+            upload_sftp_server_id   => undef,
+            download_sftp_server_id => undef,
         }
     }
 );
-- 
2.43.0