-
Notifications
You must be signed in to change notification settings - Fork 20
INTPYTHON-527 Add queryable encryption support #319
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
base: main
Are you sure you want to change the base?
Changes from all commits
a58edbc
09fd8ec
ea923dc
eff8f8b
7d5e39b
ef49313
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
import copy | ||
import os | ||
import time | ||
|
||
import django | ||
|
@@ -8,6 +9,7 @@ | |
from django.utils.functional import SimpleLazyObject | ||
from django.utils.text import format_lazy | ||
from django.utils.version import get_version_tuple | ||
from pymongo.encryption_options import AutoEncryptionOpts | ||
from pymongo.uri_parser import parse_uri as pymongo_parse_uri | ||
|
||
|
||
|
@@ -28,7 +30,19 @@ def check_django_compatability(): | |
) | ||
|
||
|
||
def parse_uri(uri, *, db_name=None, test=None): | ||
def get_auto_encryption_options(crypt_shared_lib_path=None): | ||
key_vault_database_name = "encryption" | ||
key_vault_collection_name = "__keyVault" | ||
key_vault_namespace = f"{key_vault_database_name}.{key_vault_collection_name}" | ||
kms_providers = {"local": {"key": os.urandom(96)}} | ||
return AutoEncryptionOpts( | ||
key_vault_namespace=key_vault_namespace, | ||
kms_providers=kms_providers, | ||
crypt_shared_lib_path=crypt_shared_lib_path, | ||
) | ||
Comment on lines
+33
to
+42
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's still obvious to me from the design doc to what extent it's appropriate for this library to provide helpers to generate |
||
|
||
|
||
def parse_uri(uri, *, auto_encryption_options=None, db_name=None, test=None): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd rather than |
||
""" | ||
Convert the given uri into a dictionary suitable for Django's DATABASES | ||
setting. | ||
|
@@ -48,14 +62,17 @@ def parse_uri(uri, *, db_name=None, test=None): | |
db_name = db_name or uri["database"] | ||
if not db_name: | ||
raise ImproperlyConfigured("You must provide the db_name parameter.") | ||
options = uri.get("options") | ||
if auto_encryption_options: | ||
options = {**uri.get("options"), "auto_encryption_options": auto_encryption_options} | ||
settings_dict = { | ||
"ENGINE": "django_mongodb_backend", | ||
"NAME": db_name, | ||
"HOST": host, | ||
"PORT": port, | ||
"USER": uri.get("username"), | ||
"PASSWORD": uri.get("password"), | ||
"OPTIONS": uri.get("options"), | ||
"OPTIONS": options, | ||
} | ||
if "authSource" not in settings_dict["OPTIONS"] and uri["database"]: | ||
settings_dict["OPTIONS"]["authSource"] = uri["database"] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,9 +2,9 @@ | |
|
||
import pymongo | ||
from django.core.exceptions import ImproperlyConfigured | ||
from django.test import SimpleTestCase | ||
from django.test import SimpleTestCase, TestCase, skipUnlessDBFeature | ||
|
||
from django_mongodb_backend import parse_uri | ||
from django_mongodb_backend import get_auto_encryption_options, parse_uri | ||
|
||
|
||
class ParseURITests(SimpleTestCase): | ||
|
@@ -94,3 +94,11 @@ def test_invalid_credentials(self): | |
def test_no_scheme(self): | ||
with self.assertRaisesMessage(pymongo.errors.InvalidURI, "Invalid URI scheme"): | ||
parse_uri("cluster0.example.mongodb.net") | ||
|
||
|
||
# TODO: This can be moved to `test_features` once transaction support is merged. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
class ParseUriOptionsTests(TestCase): | ||
@skipUnlessDBFeature("supports_queryable_encryption") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably we just make installing |
||
def test_auto_encryption_options(self): | ||
auto_encryption_options = get_auto_encryption_options(crypt_shared_lib_path="/path/to/lib") | ||
parse_uri("mongodb://localhost/db", auto_encryption_options=auto_encryption_options) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
from django.test import TestCase | ||
|
||
from .models import EncryptedData | ||
|
||
|
||
class ModelTests(TestCase): | ||
def test_save_load(self): | ||
EncryptedData.objects.create() |
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.
It's inappropriate to check for this package here. Instead the ImportError should be surfaced to the user if they try to use a feature that requires it. I can imagine a couple of ways to do so, but I think this concern should be deferred until the implementation is ironed out.