Needless JSON generation in case of passing a non-None client_data_hash

[cquintana92]

Hi! First I'd like to thank the developers of this library for your contribution to the open-source space :)

I've found a section in the source code that I'd like to know more about. In the register and authenticate methods of the Client struct, there are some sections that generate the client_data_json for calculating the client_data_json_hash. In these cases, the generated JSON is only used if the client_data_hash parameter is None. As this is known beforehand, the client_data_json could be generated only if the client_data_hash parameter is None, otherwise the struct construction and its serialization to JSON are not needed.

Is this behaviour intended in order to try to achieve the maximum "constant-time" operations (which wouldn't be, as the hashing only happens in the unwrap_or_else)? Or is it an overlook?

I'm willing to create the PR myself in case it's the latter.

Thanks!

[Progdrasil]

Hi @cquintana92, this isn't really a constant-time thing, or even an overlook, it's a side effect of wanting API correctness.

If you look at the response in both authenticate and register, you'll notice that the AuthenticatorResponse contains a field for the client_data_json which is non-optional.

On mobile platforms, the credential manager api will provide the client_data_hash ahead of time, and replace the client_data_json with the one that was used to create the hash provided to us. Hence the optional client_data_hash as a parameter. This meant that what-ever is in the response doesn't really mean much since it will be replaced, but I did not want to encode it as optional to prevent cases where is should be present and its not.

Granted we could probably just throw an empty map in there when client_data_hash is Some but we figured it doesn't hurt to have that present in a usable form. 🤷

[cquintana92]

Fair enough, sounds like a sane choice for the API. Thanks for answering :+1: