commonhealth-cloud-client feedback

[minrk]

I've tested the commonhealth cloud client and it seems to be working fine. I found these points of feedback in using it:

• API requests are authenticated with client id/secret. It would be nice if we could also authenticate with access tokens, too, so that the Hub could use client id/secret to issue access tokens that expire, and then Hub users would not need to be given the client id and secret in order to make requests of the CommonHealth Cloud API.
• Documenting the StorageDelegate API would be useful. I think I was able to figure out what's needed by inspecting the base CHStorageDelegate class, but since we will need a different delegate than the SQLite one, making it clear what's required would help. I believe I've made one that works storing the values in an AWS secret. Maybe we should move to a real KV store backend like DynamoDB, redis, etc.

Minor API refinements:

• the client really needs a url endpoint, but the Client constructor expects host, scheme, and port separately, with no default values for scheme or port (which will ~always be https, 443, I imagine). I think it probably makes more sense for this to accept a single URL argument, or at least have default values for scheme and port.
• passing both passphrase and salt to the KDF for the SQLiteDelegate seems a bit cumbersome and unnecessary. The docs ask for a passphrase with good entropy. If the input request is that the passphrase has good entropy, it could ask for the Fernet key itself (e.g. via secrets.token_urlsafe(32)). There is no added entropy by passing 32 random bytes through a KDF to get 32 bytes out - that only guards against lower entropy passphrases. Similarly, I don't think the use cases for salt apply here, because if the passphrase is generated and random, salt doesn't add any entropy or help protect the encrypted data. Adding salt for a KDF is mainly for memorized keys like a password, or otherwise aren't trusted to have good entropy. Asking for passphrase and salt from the same input source is really the same as asking for one actually random passphrase with no salt.
  • Suggestion: add option to provide the Fernet key directly, or
  • specify entropy requirement for input and reshape with a single hash instead of full KDF, or
  • make salt optional and default to ''

[minrk]

One more important one that I forgot to mention:

• default values from config file and/or environment variables

It would be super handy if we could launch user servers with some environment variables defined and the client objects would initialize with the right values.

We can work around this easily enough in the short term by initializing objects in startup files and/or providing custom subclasses that load defaults from our own known locations, but since these classes needs lots of input arguments that need to come from the Hub, providing it to users so they don't have to enter them all will be valuable.

[surfdoc]

Good point. We could provide default values that could be updated via passed in environment variables.

[minrk]

I also noticed that the Python package pins exact dependencies. This is generally fine for applications, but can be a problem for libraries that may share an env with other packages.

Usually, the best way to do this is to specify your loose version requirements in the package (e.g. cryptography>=24), and then freeze them to a lockfile to use separately when building a docker image / installing an application, e.g. with pip-compile.

This pinning is an impediment to packaging on conda-forge, where it would create conflicts. It's easier for pip to ignore dependencies, but conda tries harder to ensure requirements are satisfied.