foundertherapy / django-cryptographic-fields

A set of fields that wrap standard Django fields with encryption provided by the python cryptography library.
MIT License
29 stars 34 forks source link

Python Zen (and security) violation #2

Open levigross opened 9 years ago

levigross commented 9 years ago

The Python Zen states

Errors should never pass silently.
Unless explicitly silenced.

The following code

        if isinstance(value, basestring):
            try:
                value = decrypt_str(value)
            except cryptography.fernet.InvalidToken:
                pass

Is silent if the decryption process fails. It should not be as there is no good reason for the encryption process to fail (and if it does it is important to let the user know).

I would change this code to raise a SuspiciousOperation exception.

https://docs.djangoproject.com/en/1.8/ref/exceptions/#suspiciousoperation

LucasRoesler commented 8 years ago

The one nice thing of it failing silently, is that it allows you to change the field type to the encrypted field while still reading older unencrypted values. Raising an error might make it a little bit more difficult to migrate old columns.

levigross commented 8 years ago

But it is the right thing to do. The Pythonic way...