Fix #10

[codecov-commenter]

Codecov Report

Merging #11 (8c6563c) into main (89f3acf) will increase coverage by 4.10%. The diff coverage is 90.47%.

@@            Coverage Diff             @@
##             main      #11      +/-   ##
==========================================
+ Coverage   92.18%   96.29%   +4.10%     
==========================================
  Files           2        2              
  Lines          64       81      +17     
==========================================
+ Hits           59       78      +19     
+ Misses          5        3       -2     

Impacted Files	Coverage Δ
encrypted_fields/fields.py	96.25% <90.47%> (+4.18%)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 89f3acf...8c6563c. Read the comment docs.

[frgmt]

@StevenMapes @hendrikschneider What do you think about this approach? I have removed the validation for integer since this package is treating everything as a textfield. Also, I was getting errors during makemigrations and I don't know how to write this testcase...

[StevenMapes]

I'll have to have a more detailed look but if removing the validations means someone would pass a string into the field and have it be encrypted without raising a validation error then I'd say it needs another fix as you'd still want the input validation to run prior to encryption otherwise there's no point in having the different types of encrypted fields you may as well just have one.

I'm very busy today but will try to pull down and test Ali's example app along with your solution.

I've not had a need to run a testcase over migrations before myself let alone makemigrations but this could be a useful package for both https://github.com/wemake-services/django-test-migrations

[hendrikschneider]

I also think the validation should be kept. When checking the validation function I saw the validators are checking for the min and max value of the input. As this lib does not make a difference between the difference IntegerField types (SmallInt, Integer, BigInteger, ...) it could be enough to check only if the input is an int (not checking the boundaries).

Idea:

def is_integer(value):
    if type(value) != int:
        raise ValidationError(
            _('%(value)s is not an integer number'),
            params={'value': value},
        )

class EncryptedIntegerField(EncryptedFieldMixin, models.IntegerField):
  @cached_property
          def validators(self):
              return [*self.default_validators, *self._validators, is_integer]

@StevenMapes I have used the django-test-migrations package and it is doing a fine job. Pretty straight forward to use.

[Ali-Javanmardi]

@hendrikschneider , @StevenMapes , @frgmt Thank you all to investigating this problem so fast. Actually for an encryption solution I came to this project as I found it so clean and minimalist .

As this lib does not make a difference between the difference IntegerField types (SmallInt, Integer, BigInteger, ...) it could be enough to check only if the input is an int (not checking the boundaries).

But in reply to above paragraph, I should say maximum value is so important for me. In django models I have chosen positive small integer to keep my number 16bits long. If we don't have a fixed maximum length at least I will be in trouble. To make it clear, I want to use this number as a security value and so I want to do bit-wise operation on that. If the number is longer than what I expect, a ~ (bitwise not) will return me an unknown value. Although this is my personal case I think keeping that min max validation rule as an important feature is valuable.

Thanks,

[frgmt]

I tried different approach. But, I realized that every fields can go through text.. 🤣

[frgmt]

@Ali-Javanmardi I fixed min and max value validation. Please check it. https://github.com/frgmt/django-fernet-encrypted-fields/pull/11/commits/8c6563c0f3ce70ea1fbeb354f5f4b90fc732c034#diff-d814f2c8886a6f57dff41b7b53409ec35a8d72ef4b18ad735a0a2c2730abade9R92-R98

[Ali-Javanmardi]

@frgmt

I installed branch hotfix/EncryptedIntegerField

pip freeze | egrep fernet
django-fernet-encrypted-fields==0.1.2

and then : python ./make_migrations.py

Migrations for 'enc_field':
  enc_field/migrations/0001_initial.py
    - Create model EncFields

and finally runtests: python ./runtests.py

Found 1 test(s).
Creating test database for alias 'default' ('file:memorydb_default?mode=memory&cache=shared')...
Operations to perform:
  Synchronize unmigrated apps: encrypted_fields
  Apply all migrations: enc_field
Synchronizing apps without migrations:
  Creating tables...
    Running deferred SQL...
Running migrations:
  Applying enc_field.0001_initial... OK
System check identified no issues (0 silenced).
test_integer_field_encrypted (enc_field.tests.Test_EncFields) ... ok

----------------------------------------------------------------------
Ran 1 test in 0.068s

OK
Destroying test database for alias 'default' ('file:memorydb_default?mode=memory&cache=shared')...

So, it seems your changes are working.

But in my model I just used: default=0

I will add min and max limits in few minutes and try again above steps

[Ali-Javanmardi]

@frgmt I changed models.py:

from django.db import models
from django.core.validators import MaxValueValidator, MinValueValidator
from encrypted_fields.fields import EncryptedIntegerField, EncryptedCharField

class EncFields(models.Model):
  enc_int = EncryptedIntegerField(default=0, blank=True, validators=[MinValueValidator(1), MaxValueValidator(100)])

and make migrations again:

python ./make_migrations.py

Migrations for 'enc_field':
  enc_field/migrations/0002_alter_encfields_enc_int.py
    - Alter field enc_int on encfields

and ran test again: python ./runtests.py

Found 1 test(s).
Creating test database for alias 'default' ('file:memorydb_default?mode=memory&cache=shared')...
Operations to perform:
  Synchronize unmigrated apps: encrypted_fields
  Apply all migrations: enc_field
Synchronizing apps without migrations:
  Creating tables...
    Running deferred SQL...
Running migrations:
  Applying enc_field.0001_initial... OK
  Applying enc_field.0002_alter_encfields_enc_int... OK
System check identified no issues (0 silenced).
test_integer_field_encrypted (enc_field.tests.Test_EncFields) ... ok

----------------------------------------------------------------------
Ran 1 test in 0.070s

OK
Destroying test database for alias 'default' ('file:memorydb_default?mode=memory&cache=shared')...

So setting min and max validators also works well

Thanks for all your efforts.

[Ali-Javanmardi]

It seems MaxValueValidator does not through error with this changes:

As you can see in above comment I changed model.py to:

from django.db import models
from django.core.validators import MaxValueValidator, MinValueValidator
from encrypted_fields.fields import EncryptedIntegerField, EncryptedCharField

class EncFields(models.Model):
  enc_int = EncryptedIntegerField(default=0, blank=True, validators=[MinValueValidator(1), MaxValueValidator(100)])

so there is a max limit of 100 in the field.

but when I tried this test:

    def test_integer_field_max_limit_encrypted(self):
        plaintext = 142

        model = EncFields()
        model.enc_int = plaintext
        model.save()

        ciphertext = self.get_db_value("enc_int", model.id)

        self.assertNotEqual(plaintext, ciphertext)
        self.assertNotEqual(plaintext, str(ciphertext))

        fresh_model = EncFields.objects.get(id=model.id)
        self.assertEqual(fresh_model.enc_int, plaintext)

I got an OK result from my test:

Found 2 test(s).
Creating test database for alias 'default' ('file:memorydb_default?mode=memory&cache=shared')...
Operations to perform:
  Synchronize unmigrated apps: encrypted_fields
  Apply all migrations: enc_field
Synchronizing apps without migrations:
  Creating tables...
    Running deferred SQL...
Running migrations:
  Applying enc_field.0001_initial... OK
  Applying enc_field.0002_alter_encfields_enc_int... OK
System check identified no issues (0 silenced).
test_integer_field_encrypted (enc_field.tests.Test_EncFields) ... ok
test_integer_field_max_limit_encrypted (enc_field.tests.Test_EncFields) ... ok

----------------------------------------------------------------------
Ran 2 tests in 0.070s

OK
Destroying test database for alias 'default' ('file:memorydb_default?mode=memory&cache=shared')...

I expected to receive a validation error by that test as there was a limit of (100) in the model field and I assigned (142) in the test.

[frgmt]

@Ali-Javanmardi You need to call full_clean before save. https://stackoverflow.com/questions/849142/how-to-limit-the-maximum-value-of-a-numeric-field-in-a-django-model https://docs.djangoproject.com/en/4.0/ref/models/instances/#validating-objects-1

[Ali-Javanmardi]

@frgmt You are right, I forgot to do that.

Now test result after full_clean shows expected error:

Found 2 test(s).
Creating test database for alias 'default' ('file:memorydb_default?mode=memory&cache=shared')...
Operations to perform:
  Synchronize unmigrated apps: encrypted_fields
  Apply all migrations: enc_field
Synchronizing apps without migrations:
  Creating tables...
    Running deferred SQL...
Running migrations:
  Applying enc_field.0001_initial... OK
  Applying enc_field.0002_alter_encfields_enc_int... OK
System check identified no issues (0 silenced).
test_integer_field_encrypted (enc_field.tests.Test_EncFields) ... ok
test_integer_field_max_limit_encrypted (enc_field.tests.Test_EncFields) ... ERROR

======================================================================
ERROR: test_integer_field_max_limit_encrypted (enc_field.tests.Test_EncFields)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/al/dev/try_encrypted_fields/enc_field/tests.py", line 47, in test_integer_field_max_limit_encrypted
    model.full_clean()
  File "/home/al/dev/try_encrypted_fields/venv/lib/python3.10/site-packages/django/db/models/base.py", line 1405, in full_clean
    raise ValidationError(errors)
django.core.exceptions.ValidationError: {'enc_int': ['Ensure this value is less than or equal to 100.']}

----------------------------------------------------------------------
Ran 2 tests in 0.070s

FAILED (errors=1)
Destroying test database for alias 'default' ('file:memorydb_default?mode=memory&cache=shared')...

[frgmt]

I merged into main. Let me know if there are any problems.