[Koha-bugs] [Bug 20568] Add API key management interface for patrons

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20568

Tomás Cohen Arazi <tomascohen at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |oleonard at myacpl.org

--- Comment #37 from Tomás Cohen Arazi <tomascohen at gmail.com> ---
(In reply to Julian Maurice from comment #30)
> QA comments:
> 
> 1) The UUID generation happens in the store method. This means that every
> time I revoke/activate a key, client_id and secret are changed. I think that
> is not the desired behaviour.

Done. I provided both regression tests for this situation and the fix.

> 2) A CSRF attack can remove and revoke keys. This can be prevented by using
> Koha::Token.

Done. Good catch :-D

> 3) UUID::generate($uuid); UUID::unparse( $uuid, $uuidstring ); can be
> replaced by $uuidstring = uuid();

I've had issues with Jessie's version of UUID (0.05). I get:

Error: "uuid" is not exported buy the UUID module

> 4) In intranet, if I have no key and click twice on "Generate a new client
> id/secret pair" link, the form appear then disappear. Not really a problem,
> but it's an odd behaviour. I suggest to just show the button even if I have
> no keys, like in OPAC.

Right, I will think about it. Maybe Owen can answer about this.

> 5) I feel that api_keys.id is not useful as client_id can be used as a
> primary key

You are right! Removed!

> 6) Since we're using UUID, why not store the binary representation instead
> of the string representation ? I don't know how that would affect
> performance but that will save storage space. If we stick with the string
> representation I suggest that we change the data type of client_id and
> secret to a fixed-width string type (CHAR(36) ?)

I don't think this is necessary. For this client_id/secret pairs as are not
expected be be many. Also, VARCHAR only takes as much space as the size of the
content.

-- 
You are receiving this mail because:
You are watching all bug changes.