Skip to content
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

Incorporate a model's identity also in its ekey #8

Merged
merged 4 commits into from
Jun 12, 2017

Conversation

jatinderjit
Copy link
Collaborator

This will ensure that instances with same pk, but belonging to different
models, won't share an ekey.

Also change key length from 24 to 32 (AES-196 to AES-256).

This reverts commit 2776210.

The chained lookup works only when the base model being queried also
inherits from the EncryptedIDModel.

For example, if Foo inherits from EncryptedIDModel, and another model
Bar has a foreign key to Foo. Then Bar.objects.get(foo__ekey='xxx')
works only if Bar also inherits from EncryptedIDModel.
@jatinderjit jatinderjit force-pushed the model_ekey branch 5 times, most recently from 8dc93e0 to 75b9b34 Compare June 11, 2017 20:51
This will ensure that model instances belonging to different models, but
having the same pk, shouldn't share an ekey.
@Mystic-Mirage
Copy link
Contributor

Hi! Why support for related field query reverted?

@amitu amitu merged commit bce5b45 into amitu:master Jun 12, 2017
@amitu
Copy link
Owner

amitu commented Jun 12, 2017

@jatinderjit, please explain to @Mystic-Mirage.

Hey @Mystic-Mirage, we are facing some problem at work, we will resolve it and put it back. Do

@jatinderjit
Copy link
Collaborator Author

@Mystic-Mirage During a chained lookup - Foo.objects.filter(bar__fieldname=value), only Foo's manager is used. Bar's manager or queryset functions are not called. When you do split by __, that happens in Foo's manager, even though it is Bar's field.

The tests were working for Foo.objects.filter(bar__ekey=value) because Foo and Bar shared the same manager. So Foo's manager knows that it has to decode and convert to id when it sees ekey.

But if we have a third model Baz with the default manager, then Baz.objects.filter(bar__ekey=value) will raise an error, because the column "ekey" doesn't exist. So this was not a generic solution. And with current API, a generic solution is probably not possible.

We've made another change to make ekey unique for different models (even for same id). This makes calling the decode function contextual - it is not model independent anymore. So decoding Bar's ekey from Foo's model is not that trivial anymore. We've to infer bar's model from Foo's queryset (though it could easily be implemented for a simple case when bar is a ForeignKey in Foo to Bar). But then again, it won't be a generic solution.

@Mystic-Mirage
Copy link
Contributor

@jatinderjit, I see. Thanks for details!

@amitu
Copy link
Owner

amitu commented Jun 12, 2017

@jatinderjit, it would be helpful if you can create a branch with a test that fails that me and @Mystic-Mirage, and probably others can take a shot at fixing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants