Bug 28772: Make Koha::ApiKey->store encrypt the secret

This patch refactors the Koha::ApiKey class so:
- It encrypts the generated secret
- Allows accessing the plain text secret only immediately after the key
  creation (this implies that it won't be accessible if the key is
  fetched from the DB).
- It implements an allow list for attributes, that are not read only.
  Changing any other of them will make ->store throw an exception.
- A method for validating plain text secrets against the encrypted one
  is added.
- A method for accessing the plain text secret is added. Returns undef
  if the object is not 'fresh'.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/ApiKey.t
=> SUCCESS: Tests pass! Expected behavior is confirmed
3. Sign off :-D

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Koha/ApiKey.pm

@@ -20,9 +20,10 @@ package Koha::ApiKey;
 use Modern::Perl;
 
 
-use Koha::Database;
-use Koha::Exceptions;
+use Koha::AuthUtils qw(hash_password);
+use Koha::Exceptions::Object;
 
+use List::MoreUtils qw(any);
 use UUID;
 
 use base qw(Koha::Object);
@@ -46,14 +47,62 @@ Overloaded I<store> method.
 sub store {
     my ($self) = @_;
 
-    $self->client_id($self->_generate_unused_uuid('client_id'))
-        unless $self->client_id;
-    $self->secret($self->_generate_unused_uuid('secret'))
-        unless $self->secret;
+    if ( $self->in_storage ) {
+        my %dirty_columns = $self->_result->get_dirty_columns;
+
+        # only allow 'description' and 'active' to be updated
+        for my $property ( keys %dirty_columns ) {
+            Koha::Exceptions::Object::ReadOnlyProperty->throw( property => $property )
+              if $property ne 'description' and $property ne 'active';
+        }
+    } else {
+        $self->{_plain_text_secret} = $self->_generate_unused_uuid('secret');
+        $self->set(
+            {   secret    => Koha::AuthUtils::hash_password( $self->{_plain_text_secret} ),
+                client_id => $self->_generate_unused_uuid('client_id'),
+            }
+        );
+    }
 
     return $self->SUPER::store();
 }
 
+=head3 validate_secret
+
+    if ( $api_key->validate_secret( $secret ) ) { ... }
+
+Returns a boolean that tells if the passed secret matches the one on the DB.
+
+=cut
+
+sub validate_secret {
+    my ( $self, $secret ) = @_;
+
+    my $digest = Koha::AuthUtils::hash_password( $secret, $self->secret );
+
+    return $self->secret eq $digest;
+}
+
+=head3 plain_text_secret
+
+    my $generated_secret = $api_key->store->plain_text_secret;
+
+Returns the generated I<secret> so it can be displayed to  the end user.
+This is only accessible when the object is new and has just been stored.
+
+Returns I<undef> if the object was retrieved from the database.
+
+=cut
+
+sub plain_text_secret {
+    my ($self) = @_;
+
+    return $self->{_plain_text_secret}
+        if $self->{_plain_text_secret};
+
+    return;
+}
+
 =head2 Internal methods
 
 =cut

t/db_dependent/Koha/ApiKey.t

@@ -17,7 +17,7 @@
 
 use Modern::Perl;
 
-use Test::More tests => 2;
+use Test::More tests => 4;
 use Test::MockModule;
 use Test::Exception;
 
@@ -30,12 +30,9 @@ BEGIN {
 my $schema  = Koha::Database->new->schema;
 my $builder = t::lib::TestBuilder->new;
 
-my $uuid_gen_client_id_counter = 0;
-my $uuid_gen_secret_counter    = 0;
-
 subtest 'store() tests' => sub {
 
-    plan tests => 13;
+    plan tests => 16;
 
     $schema->storage->txn_begin;
 
@@ -46,7 +43,13 @@ subtest 'store() tests' => sub {
 
     my $patron_1 = $builder->build_object( { class => 'Koha::Patrons' } );
     my $description = 'Coral API key';
-    my $api_key = Koha::ApiKey->new({ patron_id => $patron_1->id, description => $description })->store;
+
+    my $api_key = Koha::ApiKey->new(
+        {
+            patron_id   => $patron_1->id,
+            description => $description
+        }
+    )->store;
 
     # re-read from DB
     $api_key->discard_changes;
@@ -75,6 +78,21 @@ subtest 'store() tests' => sub {
     is( $api_key->patron_id, $original_api_key->{patron_id}, '->store() preserves the patron_id' );
     is( $api_key->active, 0, '->store() preserves the active value' );
 
+    $api_key->set({ client_id => 'NewID!' });
+
+    throws_ok {
+        $api_key->store
+    }
+    'Koha::Exceptions::Object::ReadOnlyProperty',
+        'Read-only attribute overwrite attempt raises exception';
+
+    is( $@->property, 'client_id', 'Correct attribute reported back' );
+
+    $api_key->discard_changes;
+    # set a writeable attribute
+    $api_key->set({ description => 'Hey' });
+    lives_ok { $api_key->store } 'Updating a writeable attribute works';
+
     my $patron_to_delete = $builder->build_object( { class => 'Koha::Patrons' } );
     my $deleted_id = $patron_to_delete->id;
     $patron_to_delete->delete;
@@ -96,3 +114,41 @@ subtest 'store() tests' => sub {
 
     $schema->storage->txn_rollback;
 };
+
+subtest 'validate_secret() tests' => sub {
+
+    plan tests => 2;
+
+    $schema->storage->txn_begin;
+
+    my $patron  = $builder->build_object( { class => 'Koha::Patrons' } );
+    my $api_key = Koha::ApiKey->new(
+        {   patron_id   => $patron->id,
+            description => 'The description'
+        }
+    )->store;
+
+    my $secret = $api_key->plain_text_secret;
+
+    ok( $api_key->validate_secret( $secret ), 'Valid secret returns true' );
+    ok( !$api_key->validate_secret( 'Wrong secret' ), 'Invalid secret returns true' );
+
+    $schema->storage->txn_rollback;
+};
+
+subtest 'plain_text_secret() tests' => sub {
+
+    plan tests => 2;
+
+    $schema->storage->txn_begin;
+
+    my $patron = $builder->build_object({ class => 'Koha::Patrons' });
+    # generate a fresh API key
+    my $api_key = Koha::ApiKey->new({ description => 'blah', patron_id => $patron->id })->store;
+    my $plain_text_secret = $api_key->plain_text_secret;
+
+    ok( defined $plain_text_secret, 'A fresh API key carries its plain text secret' );
+    ok( $plain_text_secret ne q{}, 'Plain text secret is not an empty string' );
+
+    $schema->storage->txn_rollback;
+};