https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20568
Tomás Cohen Arazi <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |[email protected] --- Comment #37 from Tomás Cohen Arazi <[email protected]> --- (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. _______________________________________________ Koha-bugs mailing list [email protected] http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
