First draft of token authentication with jwt for api access #962
Conversation
There was a problem hiding this comment.
I think, I understood what you are doing here (although some explanation of the intended auth logic would have helped).
To me it seems as indeed all core features are in place. What would need to be decided is, if we want to encrypt the JWTs like it is done in the DDS and if you want some specific claims in the token regarding endpoints for which it is valid.
Unless I am missing something, these are general purpose tokens, meaning that one can authenticate with them for all functionalities. You may optionally want to include the option to specify an audience and thus have more specificity.
| try: | ||
| token_obj = jwt.JWT(jwt=token) | ||
| header = token_obj.token.jose_header | ||
| key_id = header.get("kid") |
There was a problem hiding this comment.
You do not include the code to generate the token, but I don't think that a token header should have custom fields. Or is a KeyID part of the specification?
There was a problem hiding this comment.
kid an optional header parameter https://www.rfc-editor.org/rfc/rfc7515#section-4.1.4.
Token generation would be part of whichever script is calling the genstat APIs, so they are not included here.
| public_key = jwk.JWK.from_pem(keys[0]["value"]["public_key"].encode()) | ||
| owner = keys[0]["value"].get("owner", "Unknown") | ||
| try: | ||
| verified_payload = jwt.JWT(jwt=token, key=public_key) |
There was a problem hiding this comment.
Mind that this just verifies the payload so we know it has not been tampered with. But its contents are openly readable. This is why we in DDS encrypt the whole token after generating it (encryption, decryption).
I have no strong opinions about that, but if we embed sensitive information like KeyIDs in our database in the token, encrypting it may be a way to go?
There was a problem hiding this comment.
Good point, I wasn't sure how far to take it. Currently the token would be signed with the private key of user, maybe encryption would be better as you said. The public keys would be saved in our database and the keyid is just used to select which public key to retrieve.
Requires https://github.com/NationalGenomicsInfrastructure/StatusDB_views/pull/202