https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20568 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |oleonard@myacpl.org --- Comment #37 from Tomás Cohen Arazi <tomascohen@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.