Bugzilla – Attachment 179648 Details for
Bug 39190
Rework new (S)FTP classes to be polymorphic classes
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 39190: Add key handling to SFTP Transports
Bug-39190-Add-key-handling-to-SFTP-Transports.patch (text/plain), 7.66 KB, created by
Martin Renvoize (ashimema)
on 2025-03-24 15:57:53 UTC
(
hide
)
Description:
Bug 39190: Add key handling to SFTP Transports
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2025-03-24 15:57:53 UTC
Size:
7.66 KB
patch
obsolete
>From 731b55d783f49ebb9c9139c62dac3a7695fea150 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@ptfs-europe.com> >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 <kyle@bywatersolutions.com> >--- > 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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 39190
:
178555
|
178556
|
178557
|
178558
|
178559
|
178560
|
178561
|
178562
|
178563
|
178564
|
178565
|
178566
|
178567
|
178568
|
178569
|
178570
|
178571
|
179252
|
179253
|
179254
|
179255
|
179256
|
179257
|
179258
|
179259
|
179260
|
179261
|
179262
|
179263
|
179264
|
179265
|
179266
|
179267
|
179268
|
179396
|
179397
|
179398
|
179399
|
179400
|
179401
|
179402
|
179403
|
179404
|
179405
|
179406
|
179407
|
179408
|
179409
|
179410
|
179411
|
179412
|
179413
|
179589
|
179590
|
179591
|
179592
|
179593
|
179594
|
179595
|
179596
|
179597
|
179598
|
179599
|
179600
|
179601
|
179602
|
179603
|
179604
|
179605
|
179606
|
179646
|
179647
|
179648
|
179649
|
179650
|
179651
|
179652
|
179653
|
179654
|
179655
|
179656
|
179657
|
179658
|
179659
|
179660
|
179661
|
179662
|
179663
|
179992
|
179993
|
179994
|
179995
|
179996
|
179997
|
179998
|
179999
|
180000
|
180001
|
180002
|
180003
|
180004
|
180005
|
180006
|
180010
|
180011
|
180012
|
180013
|
180014
|
180015
|
180016
|
180017
|
180018
|
180019
|
180020
|
180021
|
180022
|
180023
|
180024
|
180025