From 05101f0afa28d1de67d8d313963cb06cc5b3e1e1 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Thu, 12 Apr 2018 14:38:47 -0300 Subject: [PATCH] Bug 20568: Add mandatory description field for api keys This patch changes the table structure adding fields usually found on this kind of api management pages. Signed-off-by: Kyle M Hall Signed-off-by: Julian Maurice Signed-off-by: Jonathan Druart --- Koha/ApiKey.pm | 55 +++++++- Koha/ApiKeys.pm | 13 +- Koha/Schema/Result/ApiKey.pm | 81 +++++++++--- Koha/Schema/Result/Borrower.pm | 19 ++- .../atomicupdate/bug_20568_api_keys.perl | 29 +++++ installer/data/mysql/kohastructure.sql | 38 +++--- .../prog/en/includes/members-toolbar.inc | 5 +- .../prog/en/modules/members/apikeys.tt | 27 ++-- members/apikeys.pl | 119 ++++++++++-------- 9 files changed, 281 insertions(+), 105 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug_20568_api_keys.perl diff --git a/Koha/ApiKey.pm b/Koha/ApiKey.pm index be4b76e43c..08b661be02 100644 --- a/Koha/ApiKey.pm +++ b/Koha/ApiKey.pm @@ -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 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; diff --git a/Koha/ApiKeys.pm b/Koha/ApiKeys.pm index 8b25820034..b8514773da 100644 --- a/Koha/ApiKeys.pm +++ b/Koha/ApiKeys.pm @@ -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'; } diff --git a/Koha/Schema/Result/ApiKey.pm b/Koha/Schema/Result/ApiKey.pm index 6767a568aa..3ff057007e 100644 --- a/Koha/Schema/Result/ApiKey.pm +++ b/Koha/Schema/Result/ApiKey.pm @@ -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 +=item * L + +=back + +=cut + +__PACKAGE__->set_primary_key("id"); + +=head1 UNIQUE CONSTRAINTS + +=head2 C + +=over 4 + +=item * L -=item * L +=back + +=cut + +__PACKAGE__->add_unique_constraint("client_id", ["client_id"]); + +=head2 C + +=over 4 + +=item * L =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 =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; diff --git a/Koha/Schema/Result/Borrower.pm b/Koha/Schema/Result/Borrower.pm index 18655ccfe1..b739284068 100644 --- a/Koha/Schema/Result/Borrower.pm +++ b/Koha/Schema/Result/Borrower.pm @@ -710,6 +710,21 @@ __PACKAGE__->has_many( { cascade_copy => 0, cascade_delete => 0 }, ); +=head2 api_keys + +Type: has_many + +Related object: L + +=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 -> 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 index 0000000000..7467a10a53 --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_20568_api_keys.perl @@ -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); +} diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index 7dad2e778a..76d6dc26c5 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -15,22 +15,6 @@ /*!40101 SET @OLD_SQL_MODE=@@SQL_MODE, SQL_MODE='NO_AUTO_VALUE_ON_ZERO' */; /*!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` -- @@ -1729,6 +1713,28 @@ CREATE TABLE borrower_sync ( CONSTRAINT borrower_sync_ibfk_1 FOREIGN KEY (borrowernumber) REFERENCES borrowers (borrowernumber) ON DELETE CASCADE ON UPDATE CASCADE ) 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` -- diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/members-toolbar.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/members-toolbar.inc index 138ef92e79..280368bcb5 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/members-toolbar.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/members-toolbar.inc @@ -59,10 +59,9 @@ [% IF CAN_user_borrowers_edit_borrowers && useDischarge %]
  • Discharge
    • [% END %] - [% IF CAN_user_borrowers_edit_borrowers %] - [% IF ( CAN_user_borrowers ) %] -
  • Manage API keys
    • + [% IF CAN_user_borrowers_edit_borrowers %] +
  • Manage API keys
    • [% ELSE %]
  • Manage API keys
    • [% END %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/apikeys.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/apikeys.tt index e5131fc1aa..db283460e6 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/apikeys.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/apikeys.tt @@ -24,15 +24,22 @@

    API keys for [% INCLUDE 'patron-title.inc' %]

    - + - + + +
    - [% IF api_keys.size > 0 %] + +
    + +
    + [% IF api_keys && api_keys.size > 0 %] + @@ -41,18 +48,19 @@ [% FOREACH key IN api_keys %] - + +
    Description Key Active Actions
    [% key.api_key %][% key.description %][% key.value %] [% IF key.active %]Yes[% ELSE %]No[% END %]
    - - + +
    - - + + [% IF key.active %] @@ -66,7 +74,10 @@ [% END %]
    + [% ELSE %] + No keys defined for the current patron. [% END %] +
    diff --git a/members/apikeys.pl b/members/apikeys.pl index 7893ad317f..183483bbbb 100755 --- a/members/apikeys.pl +++ b/members/apikeys.pl @@ -1,96 +1,109 @@ #!/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 . 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; -- 2.39.5