From 731b55d783f49ebb9c9139c62dac3a7695fea150 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Wed, 22 Jan 2025 14:58:29 +0000 Subject: [PATCH] Bug 39190: Add key handling to SFTP Transports We add triggers to store in the base class and then use them in the SFTP transport class to write out the key file to the filesystem for later use in connect. Signed-off-by: Kyle M Hall --- Koha/File/Transport.pm | 7 +- Koha/File/Transport/SFTP.pm | 132 ++++++++++++++++++++++ t/db_dependent/Koha/File/Transport/SFTP.t | 62 +++++++++- 3 files changed, 196 insertions(+), 5 deletions(-) diff --git a/Koha/File/Transport.pm b/Koha/File/Transport.pm index 7bbcf1e18f4..de13dd73781 100644 --- a/Koha/File/Transport.pm +++ b/Koha/File/Transport.pm @@ -17,10 +17,9 @@ package Koha::File::Transport; use Modern::Perl; use constant { - DEFAULT_TIMEOUT => 10, - TEST_FILE_NAME => '.koha_test_file', - TEST_FILE_CONTENT => "Hello, world!\n", - KEY_FILE_PERMISSIONS => '0600', + DEFAULT_TIMEOUT => 10, + TEST_FILE_NAME => '.koha_test_file', + TEST_FILE_CONTENT => "Hello, world!\n", }; use Koha::Database; diff --git a/Koha/File/Transport/SFTP.pm b/Koha/File/Transport/SFTP.pm index 26efdd5df4e..3143c6c7bdd 100644 --- a/Koha/File/Transport/SFTP.pm +++ b/Koha/File/Transport/SFTP.pm @@ -19,6 +19,8 @@ use Modern::Perl; use Koha::Logger; +use File::Spec; +use IO::File; use Net::SFTP::Foreign; use Try::Tiny; @@ -28,6 +30,32 @@ use base qw(Koha::File::Transport); Koha::File::Transport::SFTP - SFTP implementation of file transport +=head2 Class methods + +=head3 store + + $server->store; + +Overloaded store method that ensures key_file also gets written to the filesystem. + +=cut + +sub store { + my ($self) = @_; + + $self->SUPER::store; + $self->discard_changes; + $self->_write_key_file; + + return $self; +} + +=head3 connect + + my $success = $self->connect; + +Start the SFTP transport connect, returns true on success or undefined on failure. + =cut sub connect { @@ -35,8 +63,10 @@ sub connect { $self->{connection} = Net::SFTP::Foreign->new( host => $self->host, + port => $self->port, user => $self->user_name, password => $self->plain_text_password, + key_path => $self->_locate_key_file, timeout => $self->DEFAULT_TIMEOUT, more => [qw(-o StrictHostKeyChecking=no)], ); @@ -55,6 +85,16 @@ sub connect { return 1; } +=head3 upload_file + + my $success = $transport->upload_file($fh); + +Passed a filehandle, this will upload the file to the current directory of the server connection. + +Returns true on success or undefined on failure. + +=cut + sub upload_file { my ( $self, $local_file, $remote_file ) = @_; @@ -73,6 +113,16 @@ sub upload_file { return 1; } +=head3 download_file + + my $success = $transport->download_file($filename); + +Passed a filename, this will download the file from the current directory of the server connection. + +Returns true on success or undefined on failure. + +=cut + sub download_file { my ( $self, $remote_file, $local_file ) = @_; @@ -89,6 +139,16 @@ sub download_file { return 1; } +=head3 change_directory + + my $success = $server->change_directory($directory); + +Passed a directory name, this will change the current directory of the server connection. + +Returns true on success or undefined on failure. + +=cut + sub change_directory { my ( $self, $remote_directory ) = @_; @@ -105,6 +165,14 @@ sub change_directory { return 1; } +=head3 list_files + + my @files = $server->list_files; + +Returns an array of filenames found in the current directory of the server connection. + +=cut + sub list_files { my ($self) = @_; @@ -121,6 +189,70 @@ sub list_files { return $file_list; } +=head2 Internal methods + +=head3 _write_key_file + + my $success = $server->_write_key_file; + +Writes the keyfile from the db into a file. + +Returns 1 on success, undef on failure. + +=cut + +sub _write_key_file { + my ($self) = @_; + + my $upload_path = C4::Context->config('upload_path') or return; + my $logger = Koha::Logger->get; + my $key_path = File::Spec->catdir( $upload_path, 'ssh_keys' ); + my $key_file = File::Spec->catfile( $key_path, 'id_ssh_' . $self->id ); + + mkdir $key_path unless -d $key_path; + unlink $key_file if -f $key_file; + + my $fh = IO::File->new( $key_file, 'w' ) or return; + + try { + chmod 0600, $key_file if -f $key_file; + print $fh $self->plain_text_key; + close $fh or $logger->warn("Failed to close key file: $!"); + return 1; + } catch { + $logger->warn("Error writing key file: $_"); + close $fh; + return; + }; +} + +=head3 _locate_key_file + + my $path = $server->_locate_key_file; + +Returns the keyfile's path if it exists, undef otherwise. + +=cut + +sub _locate_key_file { + my ($self) = @_; + + my $upload_path = C4::Context->config('upload_path') or return; + my $key_file = File::Spec->catfile( + $upload_path, + 'ssh_keys', + 'id_ssh_' . $self->id + ); + + return ( -f $key_file ) ? $key_file : undef; +} + +=head3 _abort_operation + +Helper method to abort the current operation and return. + +=cut + sub _abort_operation { my ($self) = @_; diff --git a/t/db_dependent/Koha/File/Transport/SFTP.t b/t/db_dependent/Koha/File/Transport/SFTP.t index d3293f4242e..42a24f08dd3 100755 --- a/t/db_dependent/Koha/File/Transport/SFTP.t +++ b/t/db_dependent/Koha/File/Transport/SFTP.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 5; +use Test::More tests => 7; use Test::Exception; use Test::Warn; @@ -29,6 +29,66 @@ use t::lib::Mocks; my $schema = Koha::Database->new->schema; my $builder = t::lib::TestBuilder->new; +subtest 'store() and _post_store_trigger() tests' => sub { + plan tests => 2; + + my $transport = $builder->build_object( + { + class => 'Koha::File::Transports', + value => { transport => 'sftp', password => 'testpass' } + } + ); + + my $post_store_called = 0; + no warnings 'redefine'; + local *Koha::File::Transport::SFTP::_post_store_trigger = sub { + $post_store_called = 1; + }; + + lives_ok { $transport->store } 'store() should complete without error'; + is( $post_store_called, 1, '_post_store_trigger() should be called' ); +}; + +subtest '_write_key_file() tests' => sub { + + plan tests => 3; + + $schema->storage->txn_begin; + + my $transport = $builder->build_object( + { + class => 'Koha::File::Transports', + value => { + transport => 'sftp', + password => undef, + key_file => undef, + } + } + ); + $transport->set( { key_file => '321test' } )->store(); + $transport->discard_changes; + + my $path = '/tmp/kohadev_test'; + t::lib::Mocks::mock_config( 'upload_path', $path ); + mkdir $path if !-d $path; + + my $first_test = $transport->_write_key_file; + + my $file = $transport->_locate_key_file; + my $second_test = ( -f $file ); + + open( my $fh, '<', $transport->_locate_key_file ); + my $third_test = <$fh>; + + is( $first_test, 1, 'Writing key file should return 1' ); + is( $second_test, 1, 'Written key file should exist' ); + is( $third_test, "321test\n", 'The contents of the key file should be 321test\n' ); + + unlink $file; + + $schema->storage->txn_rollback; +}; + subtest 'connect() tests' => sub { plan tests => 2; -- 2.49.0