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

(-)a/Koha/ApiKey.pm (-35 / +32 lines)
Lines 19-24 package Koha::ApiKey; Link Here
19
19
20
use Modern::Perl;
20
use Modern::Perl;
21
21
22
use Clone           qw( clone );
22
use C4::Log         qw( logaction );
23
use C4::Log         qw( logaction );
23
use Koha::AuthUtils qw(hash_password);
24
use Koha::AuthUtils qw(hash_password);
24
use Koha::Exceptions::ApiKey;
25
use Koha::Exceptions::ApiKey;
Lines 51-57 sub store { Link Here
51
52
52
    my $is_new = !$self->in_storage;
53
    my $is_new = !$self->in_storage;
53
54
54
    if ( $self->in_storage ) {
55
    if ( !$is_new ) {
55
        my %dirty_columns = $self->_result->get_dirty_columns;
56
        my %dirty_columns = $self->_result->get_dirty_columns;
56
57
57
        # only allow 'description' and 'active' to be updated
58
        # only allow 'description' and 'active' to be updated
Lines 70-88 sub store { Link Here
70
        );
71
        );
71
    }
72
    }
72
73
73
    my $result = $self->SUPER::store();
74
    my $log = !$params->{skip_log} && C4::Context->preference('ApiKeyLog');
75
    my $original;
74
76
75
    # Log the action unless explicitly skipped
77
    # Log the action unless explicitly skipped
76
    if ( !$params->{skip_log} && C4::Context->preference('ApiKeyLog') ) {
78
    $original = $is_new ? undef : $self->get_from_storage()
77
        if ($is_new) {
79
        if $log;
78
            logaction(
80
79
                'APIKEYS', 'CREATE', $self->patron_id,
81
    $self->SUPER::store();
80
                sprintf( "Client ID: %s, Description: %s", $self->client_id, $self->description )
82
81
            );
83
    if ( $log && $is_new ) {
82
        }
84
        logaction( 'APIKEYS', 'CREATE', $self->patron_id, $self, undef );
85
    } elsif ($log) {
86
        logaction( 'APIKEYS', 'MODIFY', $self->patron_id, $self, undef, $original );
83
    }
87
    }
84
88
85
    return $result;
89
    return $self;
86
}
90
}
87
91
88
=head3 validate_secret
92
=head3 validate_secret
Lines 132-149 Overloaded delete method to add logging. Link Here
132
sub delete {
136
sub delete {
133
    my ($self) = @_;
137
    my ($self) = @_;
134
138
135
    my $client_id   = $self->client_id;
139
    my $patron_id = $self->patron_id;
136
    my $description = $self->description;
140
    my $result    = $self->SUPER::delete();
137
    my $patron_id   = $self->patron_id;
138
139
    my $result = $self->SUPER::delete();
140
141
141
    if ( C4::Context->preference('ApiKeyLog') ) {
142
    logaction( 'APIKEYS', 'DELETE', $patron_id )
142
        logaction(
143
        if C4::Context->preference('ApiKeyLog');
143
            'APIKEYS', 'DELETE', $patron_id,
144
            sprintf( "Client ID: %s, Description: %s", $client_id, $description )
145
        );
146
    }
147
144
148
    return $result;
145
    return $result;
149
}
146
}
Lines 163-176 sub revoke { Link Here
163
    Koha::Exceptions::ApiKey::AlreadyRevoked->throw
160
    Koha::Exceptions::ApiKey::AlreadyRevoked->throw
164
        if !$self->active;
161
        if !$self->active;
165
162
166
    $self->active(0)->store( { skip_log => 1 } );
163
    my $log      = C4::Context->preference('ApiKeyLog');
164
    my $original = $log ? clone( $self->unblessed ) : undef;
167
165
168
    if ( C4::Context->preference('ApiKeyLog') ) {
166
    $self->active(0);
169
        logaction(
167
    $self->store( { skip_log => 1 } );
170
            'APIKEYS', 'REVOKE', $self->patron_id,
168
171
            sprintf( "Client ID: %s, Description: %s", $self->client_id, $self->description )
169
    logaction( 'APIKEYS', 'REVOKE', $self->patron_id, $self, undef, $original )
172
        );
170
        if $log;
173
    }
174
171
175
    return $self;
172
    return $self;
176
}
173
}
Lines 190-203 sub activate { Link Here
190
    Koha::Exceptions::ApiKey::AlreadyActive->throw
187
    Koha::Exceptions::ApiKey::AlreadyActive->throw
191
        if $self->active;
188
        if $self->active;
192
189
193
    $self->active(1)->store( { skip_log => 1 } );
190
    my $log      = C4::Context->preference('ApiKeyLog');
191
    my $original = $log ? clone( $self->unblessed ) : undef;
194
192
195
    if ( C4::Context->preference('ApiKeyLog') ) {
193
    $self->active(1);
196
        logaction(
194
    $self->store( { skip_log => 1 } );
197
            'APIKEYS', 'ACTIVATE', $self->patron_id,
195
198
            sprintf( "Client ID: %s, Description: %s", $self->client_id, $self->description )
196
    logaction( 'APIKEYS', 'ACTIVATE', $self->patron_id, $self, undef, $original )
199
        );
197
        if $log;
200
    }
201
198
202
    return $self;
199
    return $self;
203
}
200
}
(-)a/t/db_dependent/Koha/ApiKey.t (-8 / +6 lines)
Lines 154-160 subtest 'validate_secret() tests' => sub { Link Here
154
    )->first;
154
    )->first;
155
    ok( $log_entry, 'CREATE log entry found' );
155
    ok( $log_entry, 'CREATE log entry found' );
156
    is( $log_entry->object, $test_patron->id, 'Log object is patron_id' );
156
    is( $log_entry->object, $test_patron->id, 'Log object is patron_id' );
157
    like( $log_entry->info, qr/Client ID: .+, Description: logged/, 'Log info contains client_id and description' );
157
    like( $log_entry->info, qr/logged/, 'Log info contains description' );
158
158
159
    # Test no CREATE logging when ApiKeyLog is disabled
159
    # Test no CREATE logging when ApiKeyLog is disabled
160
    t::lib::Mocks::mock_preference( 'ApiKeyLog', 0 );
160
    t::lib::Mocks::mock_preference( 'ApiKeyLog', 0 );
Lines 178-187 subtest 'validate_secret() tests' => sub { Link Here
178
    )->first;
178
    )->first;
179
    ok( $delete_log, 'DELETE log entry found' );
179
    ok( $delete_log, 'DELETE log entry found' );
180
    is( $delete_log->object, $test_patron->id, 'DELETE log object is patron_id' );
180
    is( $delete_log->object, $test_patron->id, 'DELETE log object is patron_id' );
181
    like(
181
182
        $delete_log->info, qr/Client ID: $client_id_to_delete, Description: logged/,
182
    # DELETE logs pass undef for info field, object is in diff field
183
        'DELETE log info contains client_id and description'
183
    is( $delete_log->info, undef, 'DELETE log info is undef as expected' );
184
    );
185
184
186
    # Test no DELETE logging when ApiKeyLog is disabled
185
    # Test no DELETE logging when ApiKeyLog is disabled
187
    t::lib::Mocks::mock_preference( 'ApiKeyLog', 0 );
186
    t::lib::Mocks::mock_preference( 'ApiKeyLog', 0 );
Lines 247-253 subtest 'revoke() tests' => sub { Link Here
247
    ok( $revoke_log, 'REVOKE log entry found' );
246
    ok( $revoke_log, 'REVOKE log entry found' );
248
    is( $revoke_log->object, $patron->id, 'REVOKE log object is patron_id' );
247
    is( $revoke_log->object, $patron->id, 'REVOKE log object is patron_id' );
249
    like(
248
    like(
250
        $revoke_log->info, qr/Client ID: $client_id_to_revoke, Description: test2/,
249
        $revoke_log->info, qr/$client_id_to_revoke.*test2/s,
251
        'REVOKE log info contains client_id and description'
250
        'REVOKE log info contains client_id and description'
252
    );
251
    );
253
252
Lines 301-307 subtest 'activate() tests' => sub { Link Here
301
    ok( $activate_log, 'ACTIVATE log entry found' );
300
    ok( $activate_log, 'ACTIVATE log entry found' );
302
    is( $activate_log->object, $patron->id, 'ACTIVATE log object is patron_id' );
301
    is( $activate_log->object, $patron->id, 'ACTIVATE log object is patron_id' );
303
    like(
302
    like(
304
        $activate_log->info, qr/Client ID: $client_id_to_activate, Description: test2/,
303
        $activate_log->info, qr/$client_id_to_activate.*test2/s,
305
        'ACTIVATE log info contains client_id and description'
304
        'ACTIVATE log info contains client_id and description'
306
    );
305
    );
307
306
308
- 

Return to bug 20638