Bug 20568: Add mandatory description field for api keys
authorTomas Cohen Arazi <tomascohen@theke.io>
Thu, 12 Apr 2018 17:38:47 +0000 (14:38 -0300)
committerJonathan Druart <jonathan.druart@bugs.koha-community.org>
Wed, 9 May 2018 15:55:58 +0000 (12:55 -0300)
This patch changes the table structure adding fields usually found on
this kind of api management pages.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>

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

Koha/ApiKey.pm
Koha/ApiKeys.pm
Koha/Schema/Result/ApiKey.pm
Koha/Schema/Result/Borrower.pm
installer/data/mysql/atomicupdate/bug_20568_api_keys.perl [new file with mode: 0644]
installer/data/mysql/kohastructure.sql
koha-tmpl/intranet-tmpl/prog/en/includes/members-toolbar.inc
koha-tmpl/intranet-tmpl/prog/en/modules/members/apikeys.tt
members/apikeys.pl

index be4b76e..08b661b 100644 (file)
@@ -22,6 +22,9 @@ use Modern::Perl;
 use Carp;
 
 use Koha::Database;
+use Koha::Exceptions;
+
+use UUID;
 
 use base qw(Koha::Object);
 
@@ -31,16 +34,62 @@ Koha::ApiKey - Koha API Key Object class
 
 =head1 API
 
-=head2 Class Methods
+=head2 Class methods
+
+=head3 store
+
+    my $api_key = Koha::ApiKey->new({ patron_id => $patron_id })->store;
+
+Overloaded I<store> method.
+
+=cut
+
+sub store {
+    my ($self) = @_;
+
+    my ( $uuid, $uuidstring );
+
+    $self->client_id($self->_generate_unused_uuid('client_id'));
+    $self->secret($self->_generate_unused_uuid('secret'));
+
+    return $self->SUPER::store();
+}
+
+=head2 Internal methods
 
 =cut
 
-=head3 type
+=head3 _type
 
 =cut
 
-sub type {
+sub _type {
     return 'ApiKey';
 }
 
+=head3 _generate_unused_uuid
+
+    my $string = $self->_generate_unused_uuid($column);
+
+$column can be 'client_id' or 'secret'.
+
+=cut
+
+sub _generate_unused_uuid {
+    my ($self, $column) = @_;
+
+    my ( $uuid, $uuidstring );
+
+    UUID::generate($uuid);
+    UUID::unparse( $uuid, $uuidstring );
+
+    while ( Koha::ApiKeys->search({ $column => $uuidstring })->count > 0 ) {
+        # Make sure $secret is unique
+        UUID::generate($uuid);
+        UUID::unparse( $uuid, $uuidstring );
+    }
+
+    return $uuidstring;
+}
+
 1;
index 8b25820..b851477 100644 (file)
@@ -22,8 +22,7 @@ use Modern::Perl;
 use Carp;
 
 use Koha::Database;
-
-use Koha::Borrower;
+use Koha::ApiKey;
 
 use base qw(Koha::Objects);
 
@@ -33,18 +32,22 @@ Koha::ApiKeys - Koha API Keys Object class
 
 =head1 API
 
-=head2 Class Methods
+=head2 Internal methods
 
 =cut
 
-=head3 type
+=head3 _type
 
 =cut
 
-sub type {
+sub _type {
     return 'ApiKey';
 }
 
+=head3 object_class
+
+=cut
+
 sub object_class {
     return 'Koha::ApiKey';
 }
index 6767a56..3ff0570 100644 (file)
@@ -23,13 +23,31 @@ __PACKAGE__->table("api_keys");
 
 =head1 ACCESSORS
 
-=head2 borrowernumber
+=head2 id
+
+  data_type: 'integer'
+  is_auto_increment: 1
+  is_nullable: 0
+
+=head2 patron_id
 
   data_type: 'integer'
   is_foreign_key: 1
   is_nullable: 0
 
-=head2 api_key
+=head2 client_id
+
+  data_type: 'varchar'
+  is_nullable: 0
+  size: 191
+
+=head2 secret
+
+  data_type: 'varchar'
+  is_nullable: 0
+  size: 191
+
+=head2 description
 
   data_type: 'varchar'
   is_nullable: 0
@@ -37,38 +55,68 @@ __PACKAGE__->table("api_keys");
 
 =head2 active
 
-  data_type: 'integer'
+  data_type: 'tinyint'
   default_value: 1
-  is_nullable: 1
+  is_nullable: 0
 
 =cut
 
 __PACKAGE__->add_columns(
-  "borrowernumber",
+  "id",
+  { data_type => "integer", is_auto_increment => 1, is_nullable => 0 },
+  "patron_id",
   { data_type => "integer", is_foreign_key => 1, is_nullable => 0 },
-  "api_key",
+  "client_id",
+  { data_type => "varchar", is_nullable => 0, size => 191 },
+  "secret",
+  { data_type => "varchar", is_nullable => 0, size => 191 },
+  "description",
   { data_type => "varchar", is_nullable => 0, size => 255 },
   "active",
-  { data_type => "integer", default_value => 1, is_nullable => 1 },
+  { data_type => "tinyint", default_value => 1, is_nullable => 0 },
 );
 
 =head1 PRIMARY KEY
 
 =over 4
 
-=item * L</borrowernumber>
+=item * L</id>
+
+=back
+
+=cut
+
+__PACKAGE__->set_primary_key("id");
+
+=head1 UNIQUE CONSTRAINTS
+
+=head2 C<client_id>
+
+=over 4
+
+=item * L</client_id>
 
-=item * L</api_key>
+=back
+
+=cut
+
+__PACKAGE__->add_unique_constraint("client_id", ["client_id"]);
+
+=head2 C<secret>
+
+=over 4
+
+=item * L</secret>
 
 =back
 
 =cut
 
-__PACKAGE__->set_primary_key("borrowernumber", "api_key");
+__PACKAGE__->add_unique_constraint("secret", ["secret"]);
 
 =head1 RELATIONS
 
-=head2 borrowernumber
+=head2 patron
 
 Type: belongs_to
 
@@ -77,16 +125,19 @@ Related object: L<Koha::Schema::Result::Borrower>
 =cut
 
 __PACKAGE__->belongs_to(
-  "borrowernumber",
+  "patron",
   "Koha::Schema::Result::Borrower",
-  { borrowernumber => "borrowernumber" },
+  { borrowernumber => "patron_id" },
   { is_deferrable => 1, on_delete => "CASCADE", on_update => "CASCADE" },
 );
 
 
-# Created by DBIx::Class::Schema::Loader v0.07025 @ 2015-03-24 07:35:30
-# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:dvujXVM5Vfu3SA2UfiVPtw
+# Created by DBIx::Class::Schema::Loader v0.07042 @ 2018-04-14 00:56:23
+# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:b7AUAgl2SClXJ2lzPVV0FA
 
+__PACKAGE__->add_columns(
+    '+active' => { is_boolean => 1 }
+);
 
 # You can replace this text with custom code or comments, and it will be preserved on regeneration
 1;
index 18655cc..b739284 100644 (file)
@@ -710,6 +710,21 @@ __PACKAGE__->has_many(
   { cascade_copy => 0, cascade_delete => 0 },
 );
 
+=head2 api_keys
+
+Type: has_many
+
+Related object: L<Koha::Schema::Result::ApiKey>
+
+=cut
+
+__PACKAGE__->has_many(
+  "api_keys",
+  "Koha::Schema::Result::ApiKey",
+  { "foreign.patron_id" => "self.borrowernumber" },
+  { cascade_copy => 0, cascade_delete => 0 },
+);
+
 =head2 aqbasketusers
 
 Type: has_many
@@ -1386,8 +1401,8 @@ Composing rels: L</aqorder_users> -> ordernumber
 __PACKAGE__->many_to_many("ordernumbers", "aqorder_users", "ordernumber");
 
 
-# Created by DBIx::Class::Schema::Loader v0.07042 @ 2018-02-16 17:54:53
-# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:wM40W+toV8ca5LFwinkHxA
+# Created by DBIx::Class::Schema::Loader v0.07042 @ 2018-04-11 19:53:27
+# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:/vLIMxDv4RcJOqKj6Mfg6w
 
 __PACKAGE__->belongs_to(
     "guarantor",
diff --git a/installer/data/mysql/atomicupdate/bug_20568_api_keys.perl b/installer/data/mysql/atomicupdate/bug_20568_api_keys.perl
new file mode 100644 (file)
index 0000000..7467a10
--- /dev/null
@@ -0,0 +1,29 @@
+$DBversion = "XXX";
+if(CheckVersion($DBversion)) {
+
+    $dbh->do(q{
+        DROP TABLE IF EXISTS api_keys;
+    });
+
+    $dbh->do(q{
+        CREATE TABLE `api_keys` (
+            `id`          INT(11) NOT NULL AUTO_INCREMENT,
+            `patron_id`   INT(11) NOT NULL,
+            `client_id`   VARCHAR(191) NOT NULL,
+            `secret`      VARCHAR(191) NOT NULL,
+            `description` VARCHAR(255) NOT NULL,
+            `active`      TINYINT(1) DEFAULT 1 NOT NULL,
+            PRIMARY KEY (`id`),
+            UNIQUE KEY `client_id` (`client_id`),
+            UNIQUE KEY `secret` (`secret`),
+            KEY `patron_id` (`patron_id`),
+            CONSTRAINT `api_keys_fk_patron_id`
+              FOREIGN KEY (`patron_id`)
+              REFERENCES `borrowers` (`borrowernumber`)
+              ON DELETE CASCADE ON UPDATE CASCADE
+        ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;
+    });
+
+    print "Upgrade to $DBversion done (Bug 20568 - Add API key management interface for patrons)\n";
+    SetVersion($DBversion);
+}
index 7dad2e7..76d6dc2 100644 (file)
 /*!40111 SET @OLD_SQL_NOTES=@@SQL_NOTES, SQL_NOTES=0 */;
 
 --
--- Table structure for table api_keys
---
-
-DROP TABLE IF EXISTS api_keys;
-CREATE TABLE api_keys (
-    borrowernumber int(11) NOT NULL, -- foreign key to the borrowers table
-    api_key VARCHAR(255) NOT NULL, -- API key used for API authentication
-    active int(1) DEFAULT 1, -- 0 means this API key is revoked
-    PRIMARY KEY (borrowernumber, api_key),
-    CONSTRAINT api_keys_fk_borrowernumber
-      FOREIGN KEY (borrowernumber)
-      REFERENCES borrowers (borrowernumber)
-      ON DELETE CASCADE ON UPDATE CASCADE
-) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
-
---
 -- Table structure for table `auth_header`
 --
 
@@ -1730,6 +1714,28 @@ CREATE TABLE borrower_sync (
 ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;
 
 --
+-- Table structure for table api_keys
+--
+
+DROP TABLE IF EXISTS `api_keys`;
+CREATE TABLE `api_keys` (
+    `id`          INT(11) NOT NULL AUTO_INCREMENT, -- API key internal identifier
+    `patron_id`   INT(11) NOT NULL,                -- Foreign key to the borrowers table
+    `client_id`   VARCHAR(191) NOT NULL,           -- API client ID
+    `secret`      VARCHAR(191) NOT NULL,           -- API client secret used for API authentication
+    `description` VARCHAR(255) NOT NULL,           -- API client description
+    `active`      TINYINT(1) DEFAULT 1 NOT NULL,   -- 0 means this API key is revoked
+    PRIMARY KEY (`id`),
+    KEY `patron_id` (`patron_id`),
+    UNIQUE KEY `client_id` (`client_id`),
+    UNIQUE KEY `secret` (`secret`),
+    CONSTRAINT `api_keys_fk_patron_id`
+      FOREIGN KEY (`patron_id`)
+      REFERENCES `borrowers` (`borrowernumber`)
+      ON DELETE CASCADE ON UPDATE CASCADE
+) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;
+
+--
 -- Table structure for table `issues`
 --
 
index 138ef92..280368b 100644 (file)
                 [% IF CAN_user_borrowers_edit_borrowers && useDischarge %]
                     <li><a href="/cgi-bin/koha/members/discharge.pl?borrowernumber=[% patron.borrowernumber %]">Discharge</a></li>
                 [% END %]
-                [% IF CAN_user_borrowers_edit_borrowers %]
 
-                [% IF ( CAN_user_borrowers ) %]
-                    <li><a id="apikeys" href="/cgi-bin/koha/members/apikeys.pl?borrowernumber=[% borrowernumber %]">Manage API keys</a></li>
+                [% IF CAN_user_borrowers_edit_borrowers %]
+                    <li><a id="apikeys" href="/cgi-bin/koha/members/apikeys.pl?patron_id=[% borrowernumber %]">Manage API keys</a></li>
                 [% ELSE %]
                     <li class="disabled"><a data-toggle="tooltip" data-placement="left" title="You are not authorized to manage API keys" id="apikeys" href="#">Manage API keys</a></li>
                 [% END %]
index e5131fc..db28346 100644 (file)
         <h1>API keys for [% INCLUDE 'patron-title.inc' %]</h1>
         <div>
           <form action="/cgi-bin/koha/members/apikeys.pl" method="post">
-            <input type="hidden" name="borrowernumber" value="[% borrowernumber %]">
+            <input type="hidden" name="patron_id" value="[% patron.id %]">
             <input type="hidden" name="op" value="generate">
-            <input type="submit" value="Generate new key">
+            <label for="description">Description: </label>
+            <input type="text" name="description">
+            <button class="btn btn-default btn-sm" type="submit"><i class="fa fa-plus"></i> Generate a new key</span></button>
           </form>
         </div>
-        [% IF api_keys.size > 0 %]
+
+        <br/>
+
+        <div id="keys">
+        [% IF api_keys && api_keys.size > 0 %]
           <table>
             <thead>
               <tr>
+                <th>Description</th>
                 <th>Key</th>
                 <th>Active</th>
                 <th>Actions</th>
             <tbody>
               [% FOREACH key IN api_keys %]
                 <tr>
-                  <td>[% key.api_key %]</td>
+                  <td>[% key.description %]</td>
+                  <td>[% key.value %]</td>
                   <td>[% IF key.active %]Yes[% ELSE %]No[% END %]</td>
                   <td>
                     <form action="/cgi-bin/koha/members/apikeys.pl" method="post">
-                      <input type="hidden" name="borrowernumber" value="[% borrowernumber %]">
-                      <input type="hidden" name="key" value="[% key.api_key %]">
+                      <input type="hidden" name="patron_id" value="[% patron.id %]">
+                      <input type="hidden" name="key" value="[% key.value %]">
                       <input type="hidden" name="op" value="delete">
                       <input type="submit" value="Delete">
                     </form>
                     <form action="/cgi-bin/koha/members/apikeys.pl" method="post">
-                      <input type="hidden" name="borrowernumber" value="[% borrowernumber %]">
-                      <input type="hidden" name="key" value="[% key.api_key %]">
+                      <input type="hidden" name="patron_id" value="[% patron.id %]">
+                      <input type="hidden" name="key" value="[% key.value %]">
                       [% IF key.active %]
                         <input type="hidden" name="op" value="revoke">
                         <input type="submit" value="Revoke">
               [% END %]
             </tbody>
           </table>
+        [% ELSE %]
+            <span class="warn">No keys defined for the current patron.</span>
         [% END %]
+        </div>
       </div>
     </div>
     <div class="yui-b">
index 7893ad3..183483b 100755 (executable)
 #!/usr/bin/env perl
 
-# Copyright 2015 BibLibre
-#
 # This file is part of Koha.
 #
-# Koha is free software; you can redistribute it and/or modify it under the
-# terms of the GNU General Public License as published by the Free Software
-# Foundation; either version 2 of the License, or (at your option) any later
-# version.
+# Copyright 2015 BibLibre
+#
+# Koha is free software; you can redistribute it and/or modify it
+# under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
 #
-# Koha is distributed in the hope that it will be useful, but WITHOUT ANY
-# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
-# A PARTICULAR PURPOSE.  See the GNU General Public License for more details.
+# Koha is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
 #
-# You should have received a copy of the GNU General Public License along
-# with Koha; if not, write to the Free Software Foundation, Inc.,
-# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+# You should have received a copy of the GNU General Public License
+# along with Koha; if not, see <http://www.gnu.org/licenses>.
 
 use Modern::Perl;
 
 use CGI;
-use String::Random;
 
 use C4::Auth;
-use C4::Members;
 use C4::Output;
+
 use Koha::ApiKeys;
-use Koha::ApiKey;
+use Koha::Patrons;
 
 my $cgi = new CGI;
 
-my ($template, $loggedinuser, $cookie) = get_template_and_user({
-    template_name => 'members/apikeys.tt',
-    query => $cgi,
-    type => 'intranet',
-    authnotrequired => 0,
-    flagsrequired => {borrowers => 1},
-});
+my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
+    {   template_name   => 'members/apikeys.tt',
+        query           => $cgi,
+        type            => 'intranet',
+        authnotrequired => 0,
+        flagsrequired   => { borrowers => 1 },
+    }
+);
+
+my $patron;
+my $patron_id = $cgi->param('patron_id') // '';
+my $api_key   = $cgi->param('key')       // '';
+
+$patron = Koha::Patrons->find($patron_id) if $patron_id;
+
+if ( not defined $patron ) {
+
+    # patron_id invalid -> exit
+    print $cgi->redirect("/cgi-bin/koha/errors/404.pl"); # escape early
+    exit;
+}
 
-my $borrowernumber = $cgi->param('borrowernumber');
-my $borrower = C4::Members::GetMember(borrowernumber => $borrowernumber);
 my $op = $cgi->param('op');
 
 if ($op) {
-    if ($op eq 'generate') {
-        my $apikey = new Koha::ApiKey;
-        $apikey->borrowernumber($borrowernumber);
-        $apikey->api_key(String::Random->new->randregex('[a-zA-Z0-9]{32}'));
-        $apikey->store;
-        print $cgi->redirect('/cgi-bin/koha/members/apikeys.pl?borrowernumber=' . $borrowernumber);
+    if ( $op eq 'generate' ) {
+        my $description = $cgi->param('description') // '';
+        my $api_key = Koha::ApiKey->new(
+            {   patron_id   => $patron_id,
+                description => $description
+            }
+        );
+        $api_key->store;
+        print $cgi->redirect( '/cgi-bin/koha/members/apikeys.pl?patron_id=' . $patron_id );
         exit;
     }
 
-    if ($op eq 'delete') {
-        my $key = $cgi->param('key');
-        my $api_key = Koha::ApiKeys->find({borrowernumber => $borrowernumber, api_key => $key});
-        if ($api_key) {
-            $api_key->delete;
+    if ( $op eq 'delete' ) {
+        my $api_key = $cgi->param('key');
+        my $key = Koha::ApiKeys->find({ patron_id => $patron_id, value => $api_key });
+        if ($key) {
+            $key->delete;
         }
-        print $cgi->redirect('/cgi-bin/koha/members/apikeys.pl?borrowernumber=' . $borrowernumber);
+        print $cgi->redirect( '/cgi-bin/koha/members/apikeys.pl?patron_id=' . $patron_id );
         exit;
     }
 
-    if ($op eq 'revoke') {
-        my $key = $cgi->param('key');
-        my $api_key = Koha::ApiKeys->find({borrowernumber => $borrowernumber, api_key => $key});
-        if ($api_key) {
-            $api_key->active(0);
-            $api_key->store;
+    if ( $op eq 'revoke' ) {
+        my $api_key = $cgi->param('key');
+        my $key = Koha::ApiKeys->find({ patron_id => $patron_id, value => $api_key });
+        if ($key) {
+            $key->active(0);
+            $key->store;
         }
-        print $cgi->redirect('/cgi-bin/koha/members/apikeys.pl?borrowernumber=' . $borrowernumber);
+        print $cgi->redirect( '/cgi-bin/koha/members/apikeys.pl?patron_id=' . $patron_id );
         exit;
     }
 
-    if ($op eq 'activate') {
-        my $key = $cgi->param('key');
-        my $api_key = Koha::ApiKeys->find({borrowernumber => $borrowernumber, api_key => $key});
-        if ($api_key) {
-            $api_key->active(1);
-            $api_key->store;
+    if ( $op eq 'activate' ) {
+        my $api_key = $cgi->param('key');
+        my $key = Koha::ApiKeys->find({ patron_id => $patron_id, value => $api_key });
+        if ($key) {
+            $key->active(1);
+            $key->store;
         }
-        print $cgi->redirect('/cgi-bin/koha/members/apikeys.pl?borrowernumber=' . $borrowernumber);
+        print $cgi->redirect( '/cgi-bin/koha/members/apikeys.pl?patron_id=' . $patron_id );
         exit;
     }
 }
 
-my @api_keys = Koha::ApiKeys->search({borrowernumber => $borrowernumber});
+my @api_keys = Koha::ApiKeys->search({ patron_id => $patron_id });
 
 $template->param(
     api_keys => \@api_keys,
-    borrower => $borrower,
-    borrowernumber => $borrowernumber,
+    patron   => $patron
 );
 
 output_html_with_http_headers $cgi, $cookie, $template->output;