View | Details | Raw Unified | Return to bug 40811
Collapse All | Expand All

(-)a/Koha/File/Transport/FTP.pm (-1 / +1 lines)
Lines 46-52 sub _connect { Link Here
46
        Passive => $self->passive ? 1 : 0,
46
        Passive => $self->passive ? 1 : 0,
47
    ) or return $self->_abort_operation($operation);
47
    ) or return $self->_abort_operation($operation);
48
48
49
    $self->{connection}->login( $self->user_name, $self->plain_text_password )
49
    $self->{connection}->login( $self->user_name, scalar $self->plain_text_password )
50
        or return $self->_abort_operation($operation);
50
        or return $self->_abort_operation($operation);
51
51
52
    $self->add_message(
52
    $self->add_message(
(-)a/Koha/File/Transport/SFTP.pm (-5 / +8 lines)
Lines 23-28 use File::Spec; Link Here
23
use IO::File;
23
use IO::File;
24
use Net::SFTP::Foreign;
24
use Net::SFTP::Foreign;
25
use Try::Tiny;
25
use Try::Tiny;
26
use JSON qw( decode_json encode_json );
26
27
27
use base qw(Koha::File::Transport);
28
use base qw(Koha::File::Transport);
28
29
Lines 47-58 sub _connect { Link Here
47
    # String to capture STDERR output
48
    # String to capture STDERR output
48
    $self->{stderr_capture} = '';
49
    $self->{stderr_capture} = '';
49
    open my $stderr_fh, '>', \$self->{stderr_capture} or die "Can't open scalar as filehandle: $!";
50
    open my $stderr_fh, '>', \$self->{stderr_capture} or die "Can't open scalar as filehandle: $!";
51
52
    # Use key-based auth if available, otherwise password auth
53
    my $key_file = $self->_locate_key_file;
50
    $self->{connection} = Net::SFTP::Foreign->new(
54
    $self->{connection} = Net::SFTP::Foreign->new(
51
        host     => $self->host,
55
        host => $self->host,
52
        port     => $self->port,
56
        port => $self->port,
53
        user     => $self->user_name,
57
        user => $self->user_name,
54
        password => $self->plain_text_password,
58
        $key_file ? ( key_path => $key_file ) : ( password => scalar $self->plain_text_password ),
55
        $self->_locate_key_file ? ( key_path => $self->_locate_key_file ) : (),
56
        timeout   => $self->DEFAULT_TIMEOUT,
59
        timeout   => $self->DEFAULT_TIMEOUT,
57
        stderr_fh => $stderr_fh,
60
        stderr_fh => $stderr_fh,
58
        more      => [qw( -v -o StrictHostKeyChecking=no)],
61
        more      => [qw( -v -o StrictHostKeyChecking=no)],
(-)a/t/db_dependent/Koha/File/Transport/FTP.t (-1 / +57 lines)
Lines 17-26 Link Here
17
17
18
use Modern::Perl;
18
use Modern::Perl;
19
19
20
use Test::More tests => 6;
20
use Test::More tests => 7;
21
use Test::Exception;
21
use Test::Exception;
22
use Test::NoWarnings;
22
use Test::NoWarnings;
23
use Test::Warn;
23
use Test::Warn;
24
use Test::MockModule;
24
25
25
use Koha::File::Transports;
26
use Koha::File::Transports;
26
27
Lines 30-35 use t::lib::Mocks; Link Here
30
my $schema  = Koha::Database->new->schema;
31
my $schema  = Koha::Database->new->schema;
31
my $builder = t::lib::TestBuilder->new;
32
my $builder = t::lib::TestBuilder->new;
32
33
34
subtest 'scalar context in authentication tests' => sub {
35
    plan tests => 4;
36
37
    $schema->storage->txn_begin;
38
39
    # Test 1: Transport with no password - plain_text_password should return undef in scalar context
40
    my $transport_no_pass = $builder->build_object(
41
        {
42
            class => 'Koha::File::Transports',
43
            value => { transport => 'ftp', password => undef }
44
        }
45
    );
46
47
    # Mock Net::FTP to verify parameters passed to login()
48
    my $login_called = 0;
49
    my @login_params;
50
    my $mock_ftp = Test::MockModule->new('Net::FTP');
51
    $mock_ftp->mock(
52
        'new',
53
        sub {
54
            my $class = shift;
55
            return bless {}, $class;
56
        }
57
    );
58
    $mock_ftp->mock(
59
        'login',
60
        sub {
61
            $login_called = 1;
62
            @login_params = @_;
63
            return 1;
64
        }
65
    );
66
    $mock_ftp->mock( 'quit',    sub { return 1; } );
67
    $mock_ftp->mock( 'abort',   sub { return 1; } );
68
    $mock_ftp->mock( 'status',  sub { return 0; } );
69
    $mock_ftp->mock( 'message', sub { return ''; } );
70
71
    # Test that plain_text_password returns undef when no password set
72
    is(
73
        scalar $transport_no_pass->plain_text_password, undef,
74
        'plain_text_password returns undef in scalar context when no password'
75
    );
76
77
    # Test 2: Attempt connection with no password - should pass undef not empty list
78
    $transport_no_pass->connect;
79
    is( $login_called,        1,     'login was called' );
80
    is( scalar @login_params, 3,     'login called with correct number of parameters (self, user, password)' );
81
    is( $login_params[2],     undef, 'password parameter is undef, not empty list' );
82
83
    # Clean up connection to avoid warnings in DESTROY
84
    $transport_no_pass->{connection} = undef;
85
86
    $schema->storage->txn_rollback;
87
};
88
33
subtest 'connect() tests' => sub {
89
subtest 'connect() tests' => sub {
34
    plan tests => 1;
90
    plan tests => 1;
35
91
(-)a/t/db_dependent/Koha/File/Transport/SFTP.t (-2 / +108 lines)
Lines 17-27 Link Here
17
17
18
use Modern::Perl;
18
use Modern::Perl;
19
19
20
use Test::More tests => 8;
20
use Test::More tests => 9;
21
use Test::Exception;
21
use Test::Exception;
22
use Test::NoWarnings;
22
use Test::NoWarnings;
23
use Test::Warn;
23
use Test::Warn;
24
use Test::MockModule;
24
25
26
use Koha::Encryption;
25
use Koha::File::Transports;
27
use Koha::File::Transports;
26
28
27
use t::lib::TestBuilder;
29
use t::lib::TestBuilder;
Lines 30-35 use t::lib::Mocks; Link Here
30
my $schema  = Koha::Database->new->schema;
32
my $schema  = Koha::Database->new->schema;
31
my $builder = t::lib::TestBuilder->new;
33
my $builder = t::lib::TestBuilder->new;
32
34
35
subtest 'scalar context and mutually exclusive authentication tests' => sub {
36
    plan tests => 10;
37
38
    $schema->storage->txn_begin;
39
40
    # Setup mock upload path for key file operations
41
    my $path = '/tmp/kohadev_test';
42
    t::lib::Mocks::mock_config( 'upload_path', $path );
43
    mkdir $path if !-d $path;
44
45
    # Mock Net::SFTP::Foreign to capture constructor parameters
46
    my %sftp_new_params;
47
    my $mock_sftp = Test::MockModule->new('Net::SFTP::Foreign');
48
    $mock_sftp->mock(
49
        'new',
50
        sub {
51
            my $class = shift;
52
            %sftp_new_params = @_;
53
            my $obj = bless { status => 0, error => undef }, $class;
54
            return $obj;
55
        }
56
    );
57
    my $undef;
58
    $mock_sftp->mock( 'error',  sub { return $undef; } );
59
    $mock_sftp->mock( 'status', sub { return 0; } );
60
    $mock_sftp->mock( 'cwd',    sub { return '/'; } );
61
62
    # Test 1: Transport with no password and no key - plain_text_password returns undef
63
    my $transport_no_auth = $builder->build_object(
64
        {
65
            class => 'Koha::File::Transports',
66
            value => { transport => 'sftp', password => undef, key_file => undef }
67
        }
68
    );
69
70
    is(
71
        scalar $transport_no_auth->plain_text_password, undef,
72
        'plain_text_password returns undef in scalar context when no password'
73
    );
74
75
    # Test 2: Connection with no auth should pass password => undef (not empty list)
76
    $transport_no_auth->connect;
77
    ok( exists $sftp_new_params{password}, 'password key exists in connection params' );
78
    is( $sftp_new_params{password}, undef, 'password parameter is undef when no password set' );
79
    ok( !exists $sftp_new_params{key_path}, 'key_path is not set when no key file exists' );
80
81
    # Test 3: Transport with key only - should use key_path, not password
82
    # Must encrypt key_file since plain_text_key() will decrypt it
83
    my $encrypted_key      = Koha::Encryption->new->encrypt_hex('test_key_content');
84
    my $transport_key_only = $builder->build_object(
85
        {
86
            class => 'Koha::File::Transports',
87
            value => { transport => 'sftp', password => undef, key_file => $encrypted_key }
88
        }
89
    );
90
    $transport_key_only->store;    # Trigger key file write
91
    $transport_key_only->discard_changes;
92
93
    %sftp_new_params = ();         # Clear params
94
    $transport_key_only->connect;
95
    ok( exists $sftp_new_params{key_path},  'key_path exists in connection params when key is set' );
96
    ok( !exists $sftp_new_params{password}, 'password is not set when using key-based auth' );
97
98
    # Test 4: Transport with password only - should use password, not key_path
99
    # Must encrypt password since plain_text_password() will decrypt it
100
    my $encrypted_pass      = Koha::Encryption->new->encrypt_hex('testpass');
101
    my $transport_pass_only = $builder->build_object(
102
        {
103
            class => 'Koha::File::Transports',
104
            value => { transport => 'sftp', password => $encrypted_pass, key_file => undef }
105
        }
106
    );
107
108
    %sftp_new_params = ();    # Clear params
109
    $transport_pass_only->connect;
110
    ok( exists $sftp_new_params{password}, 'password exists in connection params when password is set' );
111
    is( $sftp_new_params{password}, 'testpass', 'password is correctly decrypted' );
112
    ok( !exists $sftp_new_params{key_path}, 'key_path is not set when using password auth' );
113
114
    # Test 5: Transport with both key and password - should prefer key
115
    my $encrypted_both_pass = Koha::Encryption->new->encrypt_hex('bothpass');
116
    my $encrypted_both_key  = Koha::Encryption->new->encrypt_hex('both_test_key');
117
    my $transport_both      = $builder->build_object(
118
        {
119
            class => 'Koha::File::Transports',
120
            value => { transport => 'sftp', password => $encrypted_both_pass, key_file => $encrypted_both_key }
121
        }
122
    );
123
    $transport_both->store;    # Trigger key file write
124
    $transport_both->discard_changes;
125
126
    %sftp_new_params = ();     # Clear params
127
    $transport_both->connect;
128
    ok(
129
        exists $sftp_new_params{key_path} && !exists $sftp_new_params{password},
130
        'When both key and password exist, key_path is used and password is excluded'
131
    );
132
133
    # Cleanup
134
    unlink $transport_key_only->_locate_key_file if $transport_key_only->_locate_key_file;
135
    unlink $transport_both->_locate_key_file     if $transport_both->_locate_key_file;
136
137
    $schema->storage->txn_rollback;
138
};
139
33
subtest 'store() and _post_store_trigger() tests' => sub {
140
subtest 'store() and _post_store_trigger() tests' => sub {
34
    plan tests => 2;
141
    plan tests => 2;
35
142
36
- 

Return to bug 40811