-
Notifications
You must be signed in to change notification settings - Fork 86
Keyring base API #161
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Keyring base API #161
Conversation
I'm not sure why this is showing all the files added in #158 as new, but would you please rebase on top of |
03b3bf9
to
66b348f
Compare
Co-Authored-By: Matt Bullock <[email protected]>
""" | ||
raise NotImplementedError("Keyring does not implement on_encrypt function") | ||
|
||
def on_decrypt(self, decryption_materials): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we discussed offline, per discussion in #163 we need to change this interface. encrypted_data_keys
should be an iterable of EncryptedDataKey
.
def on_decrypt(self, decryption_materials): | |
def on_decrypt(self, decryption_materials, encrypted_data_keys): |
def on_decrypt(self, decryption_materials, encrypted_data_keys): | ||
"""Attempt to decrypt the encrypted data keys. | ||
|
||
:param decryption_materials: May contain verification key, algorithm, encryption context and keyring trace. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's be consistent with the encryption materials.
I'm inclined to say that we should punt any description of the contents of encryption/decryption materials to the definitions of those structures (which Sphinx will link to when the docs are generated). Maybe just say "encryption/decryption materials for the keyring to modify" and "optionally modified encryption/decryption materials"?
Co-Authored-By: Matt Bullock <[email protected]>
Co-Authored-By: Matt Bullock <[email protected]>
Co-Authored-By: Matt Bullock <[email protected]>
Co-Authored-By: Matt Bullock <[email protected]>
We still need tests for this, but this is simple enough that I'm ok leaving that for another PR since this is not going into |
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.