From cca9401c5500ec91675704c0e09337c7cbdd8393 Mon Sep 17 00:00:00 2001 From: whytheplatypus Date: Sat, 7 Dec 2019 19:17:40 -0500 Subject: [PATCH 1/8] A Crosswalk user_id_hash should be set only once - also enforce uniqueness at db level --- .../migrations/0003_auto_20191208_0010.py | 18 ++++++++++++++++++ apps/fhir/bluebutton/models.py | 6 ++++-- 2 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 apps/fhir/bluebutton/migrations/0003_auto_20191208_0010.py diff --git a/apps/fhir/bluebutton/migrations/0003_auto_20191208_0010.py b/apps/fhir/bluebutton/migrations/0003_auto_20191208_0010.py new file mode 100644 index 000000000..4d28a311a --- /dev/null +++ b/apps/fhir/bluebutton/migrations/0003_auto_20191208_0010.py @@ -0,0 +1,18 @@ +# Generated by Django 2.1.11 on 2019-12-08 00:10 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('bluebutton', '0002_auto_20180127_2032'), + ] + + operations = [ + migrations.AlterField( + model_name='crosswalk', + name='user_id_hash', + field=models.CharField(db_index=True, max_length=64, unique=True, verbose_name='PBKDF2 of User ID'), + ), + ] diff --git a/apps/fhir/bluebutton/models.py b/apps/fhir/bluebutton/models.py index 03c72ee35..3e1111e12 100644 --- a/apps/fhir/bluebutton/models.py +++ b/apps/fhir/bluebutton/models.py @@ -2,6 +2,7 @@ from requests import Response from django.conf import settings from django.db import models +from django.core.exceptions import ValidationError from apps.accounts.models import get_user_id_salt from apps.fhir.server.models import ResourceRouter from django.utils.crypto import pbkdf2 @@ -61,9 +62,8 @@ class Crosswalk(models.Model): default=settings.USER_ID_TYPE_DEFAULT, choices=settings.USER_ID_TYPE_CHOICES) user_id_hash = models.CharField(max_length=64, - blank=True, - default="", verbose_name="PBKDF2 of User ID", + unique=True, db_index=True) objects = models.Manager() # Default manager @@ -74,6 +74,8 @@ def __str__(self): return '%s %s' % (self.user.first_name, self.user.last_name) def set_hicn(self, hicn): + if self.pk: + raise ValidationError("this value cannot be modified.") self.user_id_hash = binascii.hexlify(pbkdf2(hicn, get_user_id_salt(), settings.USER_ID_ITERATIONS)).decode("ascii") From ff2e2753c5e3937ff8f44e83206c009a28701db1 Mon Sep 17 00:00:00 2001 From: whytheplatypus Date: Tue, 17 Dec 2019 09:49:52 -0500 Subject: [PATCH 2/8] Required user_id_hash and first pass on no update --- .../bluebutton/fixtures/fhir_bluebutton_new_testdata.json | 6 +++--- apps/fhir/bluebutton/models.py | 8 +++++--- apps/fhir/bluebutton/tests/test_utils.py | 3 +++ apps/mymedicare_cb/tests/test_callback.py | 1 + apps/test.py | 1 + 5 files changed, 13 insertions(+), 6 deletions(-) diff --git a/apps/fhir/bluebutton/fixtures/fhir_bluebutton_new_testdata.json b/apps/fhir/bluebutton/fixtures/fhir_bluebutton_new_testdata.json index da55736a7..9a34cca77 100644 --- a/apps/fhir/bluebutton/fixtures/fhir_bluebutton_new_testdata.json +++ b/apps/fhir/bluebutton/fixtures/fhir_bluebutton_new_testdata.json @@ -49,7 +49,7 @@ "fhir_source": 1, "fhir_id": "Patient/9342511", "date_created": "2016-07-01T19:37:00.452Z", - "user_id_hash": "" + "user_id_hash": "123456" } }, { @@ -60,7 +60,7 @@ "fhir_source": 1, "fhir_id": "Patient/9342511", "date_created": "2016-08-25T15:55:10.338Z", - "user_id_hash": "" + "user_id_hash": "123457" } }, { @@ -71,7 +71,7 @@ "fhir_source": 1, "fhir_id": "Patient/9342703", "date_created": "2016-08-29T03:15:48.436Z", - "user_id_hash": "" + "user_id_hash": "123458" } } ] diff --git a/apps/fhir/bluebutton/models.py b/apps/fhir/bluebutton/models.py index 3e1111e12..38f9e9a6d 100644 --- a/apps/fhir/bluebutton/models.py +++ b/apps/fhir/bluebutton/models.py @@ -24,6 +24,10 @@ class SynthCrosswalkManager(models.Manager): def get_queryset(self): return super().get_queryset().filter(Q(fhir_id__startswith='-')) +def hash_hicn(hicn): + return binascii.hexlify(pbkdf2(hicn, + get_user_id_salt(), + settings.USER_ID_ITERATIONS)).decode("ascii") def hash_hicn(hicn): """ @@ -76,9 +80,7 @@ def __str__(self): def set_hicn(self, hicn): if self.pk: raise ValidationError("this value cannot be modified.") - self.user_id_hash = binascii.hexlify(pbkdf2(hicn, - get_user_id_salt(), - settings.USER_ID_ITERATIONS)).decode("ascii") + self.user_id_hash = hash_hicn(hicn) def get_fhir_patient_url(self): # Return the fhir server url and {Resource_name}/{id} diff --git a/apps/fhir/bluebutton/tests/test_utils.py b/apps/fhir/bluebutton/tests/test_utils.py index 5ee62b858..265c53c4f 100644 --- a/apps/fhir/bluebutton/tests/test_utils.py +++ b/apps/fhir/bluebutton/tests/test_utils.py @@ -1,4 +1,5 @@ import os +import uuid from django.conf import settings from django.contrib.auth.models import User from django.test import TestCase, RequestFactory @@ -405,6 +406,7 @@ def setUp(self): xwalk = Crosswalk() xwalk.user = self.user xwalk.fhir_id = "Patient/12345" + xwalk.set_hicn(uuid.uuid4()) xwalk.save() def test_crosswalk_fhir_id(self): @@ -422,6 +424,7 @@ def test_crosswalk_fhir_id(self): x = Crosswalk() x.user = u x.fhir_id = "Patient/23456" + x.set_hicn(uuid.uuid4()) x.save() result = crosswalk_patient_id(u) diff --git a/apps/mymedicare_cb/tests/test_callback.py b/apps/mymedicare_cb/tests/test_callback.py index c1f51c785..037723f98 100644 --- a/apps/mymedicare_cb/tests/test_callback.py +++ b/apps/mymedicare_cb/tests/test_callback.py @@ -158,6 +158,7 @@ def sls_user_info_mock(url, request): 'sub': '0123456789abcdefghijklmnopqrstuvwxyz', 'given_name': '', 'family_name': '', + 'hicn': 'test', 'email': 'bob@bobserver.bob', 'hicn': '1234567890A', }, diff --git a/apps/test.py b/apps/test.py index bcb410c81..7caa1de72 100644 --- a/apps/test.py +++ b/apps/test.py @@ -98,6 +98,7 @@ def create_token(self, first_name, last_name): email="%s@%s.net" % (first_name, last_name)) Crosswalk.objects.get_or_create(user=user, fhir_id=settings.DEFAULT_SAMPLE_FHIR_ID, + user_id_hash=user.username, fhir_source=get_resourcerouter()) # create a oauth2 application and add capabilities From bb90bd247ba8287ee5867981f43b738bacab90ab Mon Sep 17 00:00:00 2001 From: whytheplatypus Date: Tue, 17 Dec 2019 14:41:11 -0500 Subject: [PATCH 3/8] Provide setters for crosswalk identifier fields --- .../migrations/0004_auto_20191217_1935.py | 33 +++++++++++++++++ apps/fhir/bluebutton/models.py | 37 ++++++++++++++++--- apps/fhir/bluebutton/permissions.py | 2 +- apps/fhir/bluebutton/utils.py | 2 +- 4 files changed, 66 insertions(+), 8 deletions(-) create mode 100644 apps/fhir/bluebutton/migrations/0004_auto_20191217_1935.py diff --git a/apps/fhir/bluebutton/migrations/0004_auto_20191217_1935.py b/apps/fhir/bluebutton/migrations/0004_auto_20191217_1935.py new file mode 100644 index 000000000..a839e992e --- /dev/null +++ b/apps/fhir/bluebutton/migrations/0004_auto_20191217_1935.py @@ -0,0 +1,33 @@ +# Generated by Django 2.1.11 on 2019-12-17 19:35 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('bluebutton', '0003_auto_20191208_0010'), + ] + + operations = [ + migrations.RenameField( + model_name='crosswalk', + old_name='user_id_hash', + new_name='_user_id_hash', + ), + migrations.RenameField( + model_name='crosswalk', + old_name='fhir_id', + new_name='_fhir_id', + ), + migrations.AlterField( + model_name='crosswalk', + name='_fhir_id', + field=models.CharField(db_column='fhir_id', db_index=True, max_length=80, null=True), + ), + migrations.AlterField( + model_name='crosswalk', + name='_user_id_hash', + field=models.CharField(db_column='user_id_hash', db_index=True, max_length=64, unique=True, verbose_name='PBKDF2 of User ID'), + ), + ] diff --git a/apps/fhir/bluebutton/models.py b/apps/fhir/bluebutton/models.py index 38f9e9a6d..b66a955f2 100644 --- a/apps/fhir/bluebutton/models.py +++ b/apps/fhir/bluebutton/models.py @@ -58,17 +58,20 @@ class Crosswalk(models.Model): blank=True, null=True) # default=settings.FHIR_SERVER_DEFAULT) - fhir_id = models.CharField(max_length=80, - blank=True, default="", db_index=True) + _fhir_id = models.CharField(max_length=80, + null=True, + db_column="fhir_id", + db_index=True) date_created = models.DateTimeField(auto_now_add=True) user_id_type = models.CharField(max_length=1, default=settings.USER_ID_TYPE_DEFAULT, choices=settings.USER_ID_TYPE_CHOICES) - user_id_hash = models.CharField(max_length=64, - verbose_name="PBKDF2 of User ID", - unique=True, - db_index=True) + _user_id_hash = models.CharField(max_length=64, + verbose_name="PBKDF2 of User ID", + unique=True, + db_column="user_id_hash", + db_index=True) objects = models.Manager() # Default manager real_objects = RealCrosswalkManager() # Real bene manager @@ -77,6 +80,28 @@ class Crosswalk(models.Model): def __str__(self): return '%s %s' % (self.user.first_name, self.user.last_name) + @property + def fhir_id(self): + return self._fhir_id + + @fhir_id.setter + def fhir_id(self, value): + if self._fhir_id is not None: + raise ValidationError("this value cannot be modified.") + self._fhir_id = value + + @property + def user_id_hash(self): + return self._user_id_hash + + @user_id_hash.setter + def user_id_hash(self, value): + if self.pk is not None: + raise ValidationError("this value cannot be modified.") + if self._user_id_hash is not None: + raise ValidationError("this value cannot be modified.") + self._user_id_hash = value + def set_hicn(self, hicn): if self.pk: raise ValidationError("this value cannot be modified.") diff --git a/apps/fhir/bluebutton/permissions.py b/apps/fhir/bluebutton/permissions.py index 3734dea92..68e20608d 100644 --- a/apps/fhir/bluebutton/permissions.py +++ b/apps/fhir/bluebutton/permissions.py @@ -27,7 +27,7 @@ def has_permission(self, request, view): logger.info('Crosswalk for %s does not exist' % request.user) return False - if crosswalk.fhir_id == "": + if crosswalk.fhir_id is None: authenticate_crosswalk(crosswalk) request.crosswalk = crosswalk diff --git a/apps/fhir/bluebutton/utils.py b/apps/fhir/bluebutton/utils.py index 5b335db98..e0838e8b9 100644 --- a/apps/fhir/bluebutton/utils.py +++ b/apps/fhir/bluebutton/utils.py @@ -137,7 +137,7 @@ def generate_info_headers(request): crosswalk = get_crosswalk(user) if crosswalk: # we need to send the HicnHash or the fhir_id - if len(crosswalk.fhir_id) > 0: + if crosswalk.fhir_id is not None: result['BlueButton-BeneficiaryId'] = 'patientId:' + str(crosswalk.fhir_id) else: result['BlueButton-BeneficiaryId'] = 'hicnHash:' + str(crosswalk.user_id_hash) From 91612d69469178acc6d19cd7dab365410216d686 Mon Sep 17 00:00:00 2001 From: whytheplatypus Date: Tue, 17 Dec 2019 15:38:18 -0500 Subject: [PATCH 4/8] Require crosswalk identity fields --- ...217_1935.py => 0004_auto_20191217_2031.py} | 24 +++++++++---------- apps/fhir/bluebutton/models.py | 4 ---- 2 files changed, 12 insertions(+), 16 deletions(-) rename apps/fhir/bluebutton/migrations/{0004_auto_20191217_1935.py => 0004_auto_20191217_2031.py} (75%) diff --git a/apps/fhir/bluebutton/migrations/0004_auto_20191217_1935.py b/apps/fhir/bluebutton/migrations/0004_auto_20191217_2031.py similarity index 75% rename from apps/fhir/bluebutton/migrations/0004_auto_20191217_1935.py rename to apps/fhir/bluebutton/migrations/0004_auto_20191217_2031.py index a839e992e..07b3e146e 100644 --- a/apps/fhir/bluebutton/migrations/0004_auto_20191217_1935.py +++ b/apps/fhir/bluebutton/migrations/0004_auto_20191217_2031.py @@ -1,4 +1,4 @@ -# Generated by Django 2.1.11 on 2019-12-17 19:35 +# Generated by Django 2.1.11 on 2019-12-17 20:31 from django.db import migrations, models @@ -10,24 +10,24 @@ class Migration(migrations.Migration): ] operations = [ - migrations.RenameField( + migrations.AlterField( model_name='crosswalk', - old_name='user_id_hash', - new_name='_user_id_hash', + name='fhir_id', + field=models.CharField(db_column='fhir_id', db_index=True, default=None, max_length=80, null=True), + ), + migrations.AlterField( + model_name='crosswalk', + name='user_id_hash', + field=models.CharField(db_column='user_id_hash', db_index=True, default=None, max_length=64, unique=True, verbose_name='PBKDF2 of User ID'), ), migrations.RenameField( model_name='crosswalk', old_name='fhir_id', new_name='_fhir_id', ), - migrations.AlterField( - model_name='crosswalk', - name='_fhir_id', - field=models.CharField(db_column='fhir_id', db_index=True, max_length=80, null=True), - ), - migrations.AlterField( + migrations.RenameField( model_name='crosswalk', - name='_user_id_hash', - field=models.CharField(db_column='user_id_hash', db_index=True, max_length=64, unique=True, verbose_name='PBKDF2 of User ID'), + old_name='user_id_hash', + new_name='_user_id_hash', ), ] diff --git a/apps/fhir/bluebutton/models.py b/apps/fhir/bluebutton/models.py index b66a955f2..f4901f2b1 100644 --- a/apps/fhir/bluebutton/models.py +++ b/apps/fhir/bluebutton/models.py @@ -24,10 +24,6 @@ class SynthCrosswalkManager(models.Manager): def get_queryset(self): return super().get_queryset().filter(Q(fhir_id__startswith='-')) -def hash_hicn(hicn): - return binascii.hexlify(pbkdf2(hicn, - get_user_id_salt(), - settings.USER_ID_ITERATIONS)).decode("ascii") def hash_hicn(hicn): """ From 5ecf10fd72b583660cc4af075a6bd228d5a1c1ef Mon Sep 17 00:00:00 2001 From: whytheplatypus Date: Tue, 17 Dec 2019 15:55:01 -0500 Subject: [PATCH 5/8] Update tests to account for identity uniqueness --- .../fixtures/fhir_bluebutton_new_testdata.json | 12 ++++++------ .../fixtures/fhir_bluebutton_test_rt.json | 3 ++- apps/fhir/bluebutton/tests/test_models.py | 15 +++++++++++++-- apps/test.py | 6 +++--- .../commands/create_test_user_and_application.py | 3 ++- 5 files changed, 26 insertions(+), 13 deletions(-) diff --git a/apps/fhir/bluebutton/fixtures/fhir_bluebutton_new_testdata.json b/apps/fhir/bluebutton/fixtures/fhir_bluebutton_new_testdata.json index 9a34cca77..7f40bf9c6 100644 --- a/apps/fhir/bluebutton/fixtures/fhir_bluebutton_new_testdata.json +++ b/apps/fhir/bluebutton/fixtures/fhir_bluebutton_new_testdata.json @@ -47,9 +47,9 @@ "fields": { "user": 1, "fhir_source": 1, - "fhir_id": "Patient/9342511", + "_fhir_id": "Patient/9342511", "date_created": "2016-07-01T19:37:00.452Z", - "user_id_hash": "123456" + "_user_id_hash": "123456" } }, { @@ -58,9 +58,9 @@ "fields": { "user": 2, "fhir_source": 1, - "fhir_id": "Patient/9342511", + "_fhir_id": "Patient/9342511", "date_created": "2016-08-25T15:55:10.338Z", - "user_id_hash": "123457" + "_user_id_hash": "123457" } }, { @@ -69,9 +69,9 @@ "fields": { "user": 3, "fhir_source": 1, - "fhir_id": "Patient/9342703", + "_fhir_id": "Patient/9342703", "date_created": "2016-08-29T03:15:48.436Z", - "user_id_hash": "123458" + "_user_id_hash": "123458" } } ] diff --git a/apps/fhir/bluebutton/fixtures/fhir_bluebutton_test_rt.json b/apps/fhir/bluebutton/fixtures/fhir_bluebutton_test_rt.json index d3b86d718..fd5a191b2 100644 --- a/apps/fhir/bluebutton/fixtures/fhir_bluebutton_test_rt.json +++ b/apps/fhir/bluebutton/fixtures/fhir_bluebutton_test_rt.json @@ -69,7 +69,8 @@ "pk": 1, "fields": {"user": 1, "fhir_source": 1, - "fhir_id": "4995802", + "_fhir_id": "4995802", + "_user_id_hash": "badhash", "date_created": "2016-05-25T05:03:49.342Z"} }, { diff --git a/apps/fhir/bluebutton/tests/test_models.py b/apps/fhir/bluebutton/tests/test_models.py index 2616689c4..e6c504b20 100644 --- a/apps/fhir/bluebutton/tests/test_models.py +++ b/apps/fhir/bluebutton/tests/test_models.py @@ -1,3 +1,4 @@ +from django.db.utils import IntegretyError from apps.test import BaseApiTest from ..models import Crosswalk @@ -19,7 +20,8 @@ def test_get_full_url_good(self): cw = Crosswalk.objects.create(user=user, fhir_source=fs, - fhir_id="123456") + fhir_id="123456", + _user_id_hash="badhash") fhir = Crosswalk.objects.get(user=user.pk) @@ -43,7 +45,8 @@ def test_get_full_url_bad(self): Crosswalk.objects.create(user=user, fhir_source=fs, - fhir_id="123456") + fhir_id="123456", + _user_id_hash="badhash") fhir = Crosswalk.objects.get(user=user.pk) @@ -51,3 +54,11 @@ def test_get_full_url_bad(self): invalid_match = "http://localhost:8000/fhir/" + "Practitioner/123456" self.assertNotEqual(url_info, invalid_match) + + def test_require_user_id_hash(self): + user = self._create_user('john', 'password', + first_name='John', + last_name='Smith', + email='john@smith.net') + with self.assertRaises(IntegrityError): + Crosswalk.objects.create(user=user) diff --git a/apps/test.py b/apps/test.py index 7caa1de72..5857c7907 100644 --- a/apps/test.py +++ b/apps/test.py @@ -97,8 +97,8 @@ def create_token(self, first_name, last_name): last_name=last_name, email="%s@%s.net" % (first_name, last_name)) Crosswalk.objects.get_or_create(user=user, - fhir_id=settings.DEFAULT_SAMPLE_FHIR_ID, - user_id_hash=user.username, + _fhir_id=settings.DEFAULT_SAMPLE_FHIR_ID, + _user_id_hash=user.username, fhir_source=get_resourcerouter()) # create a oauth2 application and add capabilities @@ -117,7 +117,7 @@ def create_token_no_fhir(self, first_name, last_name): last_name=last_name, email="%s@%s.net" % (first_name, last_name)) Crosswalk.objects.get_or_create(user=user, - user_id_hash="139e178537ed3bc486e6a7195a47a82a2cd6f46e911660fe9775f6e0dd3f1130", + _user_id_hash="139e178537ed3bc486e6a7195a47a82a2cd6f46e911660fe9775f6e0dd3f1130", fhir_source=get_resourcerouter()) # create a oauth2 application and add capabilities diff --git a/apps/testclient/management/commands/create_test_user_and_application.py b/apps/testclient/management/commands/create_test_user_and_application.py index 93652d3db..4c505f247 100644 --- a/apps/testclient/management/commands/create_test_user_and_application.py +++ b/apps/testclient/management/commands/create_test_user_and_application.py @@ -45,7 +45,8 @@ def create_user(group): u.groups.add(group) c, g_o_c = Crosswalk.objects.get_or_create(user=u, - fhir_id=settings.DEFAULT_SAMPLE_FHIR_ID, + _fhir_id=settings.DEFAULT_SAMPLE_FHIR_ID, + _user_id_hash="badhash", fhir_source=get_resourcerouter()) return u From c75b5172381a73da7ad956e7f949a82e88b152e9 Mon Sep 17 00:00:00 2001 From: David Gage Date: Fri, 20 Dec 2019 13:04:44 -0500 Subject: [PATCH 6/8] update tests for immutable crosswalk - also assert that the a user_id_hash is generally of the form expected --- .../migrations/0003_auto_20191208_0010.py | 17 +++++++++- .../migrations/0004_auto_20191217_2031.py | 33 ------------------- apps/fhir/bluebutton/models.py | 3 ++ apps/fhir/bluebutton/tests/test_models.py | 2 +- apps/mymedicare_cb/models.py | 9 ++--- apps/mymedicare_cb/tests/test_callback.py | 1 - 6 files changed, 25 insertions(+), 40 deletions(-) delete mode 100644 apps/fhir/bluebutton/migrations/0004_auto_20191217_2031.py diff --git a/apps/fhir/bluebutton/migrations/0003_auto_20191208_0010.py b/apps/fhir/bluebutton/migrations/0003_auto_20191208_0010.py index 4d28a311a..df9aa3e69 100644 --- a/apps/fhir/bluebutton/migrations/0003_auto_20191208_0010.py +++ b/apps/fhir/bluebutton/migrations/0003_auto_20191208_0010.py @@ -10,9 +10,24 @@ class Migration(migrations.Migration): ] operations = [ + migrations.AlterField( + model_name='crosswalk', + name='fhir_id', + field=models.CharField(db_column='fhir_id', db_index=True, default=None, max_length=80, null=True), + ), migrations.AlterField( model_name='crosswalk', name='user_id_hash', - field=models.CharField(db_index=True, max_length=64, unique=True, verbose_name='PBKDF2 of User ID'), + field=models.CharField(db_column='user_id_hash', db_index=True, default=None, max_length=64, unique=True, verbose_name='PBKDF2 of User ID'), + ), + migrations.RenameField( + model_name='crosswalk', + old_name='fhir_id', + new_name='_fhir_id', + ), + migrations.RenameField( + model_name='crosswalk', + old_name='user_id_hash', + new_name='_user_id_hash', ), ] diff --git a/apps/fhir/bluebutton/migrations/0004_auto_20191217_2031.py b/apps/fhir/bluebutton/migrations/0004_auto_20191217_2031.py deleted file mode 100644 index 07b3e146e..000000000 --- a/apps/fhir/bluebutton/migrations/0004_auto_20191217_2031.py +++ /dev/null @@ -1,33 +0,0 @@ -# Generated by Django 2.1.11 on 2019-12-17 20:31 - -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ('bluebutton', '0003_auto_20191208_0010'), - ] - - operations = [ - migrations.AlterField( - model_name='crosswalk', - name='fhir_id', - field=models.CharField(db_column='fhir_id', db_index=True, default=None, max_length=80, null=True), - ), - migrations.AlterField( - model_name='crosswalk', - name='user_id_hash', - field=models.CharField(db_column='user_id_hash', db_index=True, default=None, max_length=64, unique=True, verbose_name='PBKDF2 of User ID'), - ), - migrations.RenameField( - model_name='crosswalk', - old_name='fhir_id', - new_name='_fhir_id', - ), - migrations.RenameField( - model_name='crosswalk', - old_name='user_id_hash', - new_name='_user_id_hash', - ), - ] diff --git a/apps/fhir/bluebutton/models.py b/apps/fhir/bluebutton/models.py index f4901f2b1..c118b0bd3 100644 --- a/apps/fhir/bluebutton/models.py +++ b/apps/fhir/bluebutton/models.py @@ -56,6 +56,7 @@ class Crosswalk(models.Model): # default=settings.FHIR_SERVER_DEFAULT) _fhir_id = models.CharField(max_length=80, null=True, + default=None, db_column="fhir_id", db_index=True) date_created = models.DateTimeField(auto_now_add=True) @@ -66,6 +67,8 @@ class Crosswalk(models.Model): _user_id_hash = models.CharField(max_length=64, verbose_name="PBKDF2 of User ID", unique=True, + null=False, + default=None, db_column="user_id_hash", db_index=True) diff --git a/apps/fhir/bluebutton/tests/test_models.py b/apps/fhir/bluebutton/tests/test_models.py index e6c504b20..c522e918e 100644 --- a/apps/fhir/bluebutton/tests/test_models.py +++ b/apps/fhir/bluebutton/tests/test_models.py @@ -1,4 +1,4 @@ -from django.db.utils import IntegretyError +from django.db.utils import IntegrityError from apps.test import BaseApiTest from ..models import Crosswalk diff --git a/apps/mymedicare_cb/models.py b/apps/mymedicare_cb/models.py index 78a78a2d3..3e46cd2c3 100644 --- a/apps/mymedicare_cb/models.py +++ b/apps/mymedicare_cb/models.py @@ -70,14 +70,15 @@ def create_beneficiary_record(username=None, assert username is not None assert username != "" assert user_id_hash is not None + assert len(user_id_hash) == 64, "incorrect user id hash format" if User.objects.filter(username=username).exists(): raise ValidationError("user already exists", username) - if Crosswalk.objects.filter(user_id_hash=user_id_hash).exists(): + if Crosswalk.objects.filter(_user_id_hash=user_id_hash).exists(): raise ValidationError("user_id_hash already exists", user_id_hash) - if fhir_id and Crosswalk.objects.filter(fhir_id=fhir_id).exists(): + if fhir_id and Crosswalk.objects.filter(_fhir_id=fhir_id).exists(): raise ValidationError("fhir_id already exists", fhir_id) with transaction.atomic(): @@ -89,8 +90,8 @@ def create_beneficiary_record(username=None, user.save() Crosswalk.objects.create(user=user, fhir_source=fhir_source, - user_id_hash=user_id_hash, - fhir_id=fhir_id) + _user_id_hash=user_id_hash, + _fhir_id=fhir_id) # Extra user information # TODO: remvoe the idea of UserProfile diff --git a/apps/mymedicare_cb/tests/test_callback.py b/apps/mymedicare_cb/tests/test_callback.py index 037723f98..c1f51c785 100644 --- a/apps/mymedicare_cb/tests/test_callback.py +++ b/apps/mymedicare_cb/tests/test_callback.py @@ -158,7 +158,6 @@ def sls_user_info_mock(url, request): 'sub': '0123456789abcdefghijklmnopqrstuvwxyz', 'given_name': '', 'family_name': '', - 'hicn': 'test', 'email': 'bob@bobserver.bob', 'hicn': '1234567890A', }, From be14c99f1c0955d90f8d5eb56db6f0051600dea4 Mon Sep 17 00:00:00 2001 From: David Gage Date: Fri, 20 Dec 2019 14:01:18 -0500 Subject: [PATCH 7/8] Improve tests around creating crosswalks --- .../fixtures/fhir_bluebutton_test_rt.json | 2 +- apps/fhir/bluebutton/models.py | 6 +-- apps/fhir/bluebutton/tests/test_models.py | 51 +++++++++++++++++-- apps/mymedicare_cb/models.py | 4 +- apps/mymedicare_cb/tests/test_callback.py | 2 +- .../create_test_user_and_application.py | 2 +- 6 files changed, 55 insertions(+), 12 deletions(-) diff --git a/apps/fhir/bluebutton/fixtures/fhir_bluebutton_test_rt.json b/apps/fhir/bluebutton/fixtures/fhir_bluebutton_test_rt.json index fd5a191b2..ae4cdbe5f 100644 --- a/apps/fhir/bluebutton/fixtures/fhir_bluebutton_test_rt.json +++ b/apps/fhir/bluebutton/fixtures/fhir_bluebutton_test_rt.json @@ -70,7 +70,7 @@ "fields": {"user": 1, "fhir_source": 1, "_fhir_id": "4995802", - "_user_id_hash": "badhash", + "_user_id_hash": "139e178537ed3bc486e6a7195a47a82a2cd6f46e911660fe9775f6e0dd3f1130", "date_created": "2016-05-25T05:03:49.342Z"} }, { diff --git a/apps/fhir/bluebutton/models.py b/apps/fhir/bluebutton/models.py index c118b0bd3..3ae5a7487 100644 --- a/apps/fhir/bluebutton/models.py +++ b/apps/fhir/bluebutton/models.py @@ -85,7 +85,7 @@ def fhir_id(self): @fhir_id.setter def fhir_id(self, value): - if self._fhir_id is not None: + if self._fhir_id: raise ValidationError("this value cannot be modified.") self._fhir_id = value @@ -95,9 +95,9 @@ def user_id_hash(self): @user_id_hash.setter def user_id_hash(self, value): - if self.pk is not None: + if self.pk: raise ValidationError("this value cannot be modified.") - if self._user_id_hash is not None: + if self._user_id_hash: raise ValidationError("this value cannot be modified.") self._user_id_hash = value diff --git a/apps/fhir/bluebutton/tests/test_models.py b/apps/fhir/bluebutton/tests/test_models.py index c522e918e..b7cda3be0 100644 --- a/apps/fhir/bluebutton/tests/test_models.py +++ b/apps/fhir/bluebutton/tests/test_models.py @@ -1,4 +1,5 @@ from django.db.utils import IntegrityError +from django.core.exceptions import ValidationError from apps.test import BaseApiTest from ..models import Crosswalk @@ -20,8 +21,8 @@ def test_get_full_url_good(self): cw = Crosswalk.objects.create(user=user, fhir_source=fs, - fhir_id="123456", - _user_id_hash="badhash") + fhir_id="-20000000000001", + user_id_hash="139e178537ed3bc486e6a7195a47a82a2cd6f46e911660fe9775f6e0dd3f1130") fhir = Crosswalk.objects.get(user=user.pk) @@ -45,8 +46,8 @@ def test_get_full_url_bad(self): Crosswalk.objects.create(user=user, fhir_source=fs, - fhir_id="123456", - _user_id_hash="badhash") + fhir_id="-20000000000001", + user_id_hash="139e178537ed3bc486e6a7195a47a82a2cd6f46e911660fe9775f6e0dd3f1130") fhir = Crosswalk.objects.get(user=user.pk) @@ -62,3 +63,45 @@ def test_require_user_id_hash(self): email='john@smith.net') with self.assertRaises(IntegrityError): Crosswalk.objects.create(user=user) + + def test_immutable_fhir_id(self): + user = self._create_user('john', 'password', + first_name='John', + last_name='Smith', + email='john@smith.net') + + # created a default user + fs = ResourceRouter.objects.create(name="Main Server", + fhir_url="http://localhost:8000/fhir/", + shard_by="Patient", + server_search_expiry=1800) + + Crosswalk.objects.create(user=user, + fhir_source=fs, + fhir_id="-20000000000001", + user_id_hash="139e178537ed3bc486e6a7195a47a82a2cd6f46e911660fe9775f6e0dd3f1130") + cw = Crosswalk.objects.get(_fhir_id="-20000000000001") + with self.assertRaises(ValidationError): + cw.fhir_id = "-20000000000002" + + + def test_immuatble_user_id_hash(self): + user = self._create_user('john', 'password', + first_name='John', + last_name='Smith', + email='john@smith.net') + + # created a default user + fs = ResourceRouter.objects.create(name="Main Server", + fhir_url="http://localhost:8000/fhir/", + shard_by="Patient", + server_search_expiry=1800) + + cw = Crosswalk.objects.create(user=user, + fhir_source=fs, + fhir_id="-20000000000001", + user_id_hash="139e178537ed3bc486e6a7195a47a82a2cd6f46e911660fe9775f6e0dd3f1130") + cw = Crosswalk.objects.get(_fhir_id="-20000000000001") + self.assertEqual(cw.user_id_hash, "139e178537ed3bc486e6a7195a47a82a2cd6f46e911660fe9775f6e0dd3f1130") + with self.assertRaises(ValidationError): + cw.user_id_hash = "239e178537ed3bc486e6a7195a47a82a2cd6f46e911660fe9775f6e0dd3f1130" diff --git a/apps/mymedicare_cb/models.py b/apps/mymedicare_cb/models.py index 3e46cd2c3..214cfa15d 100644 --- a/apps/mymedicare_cb/models.py +++ b/apps/mymedicare_cb/models.py @@ -90,8 +90,8 @@ def create_beneficiary_record(username=None, user.save() Crosswalk.objects.create(user=user, fhir_source=fhir_source, - _user_id_hash=user_id_hash, - _fhir_id=fhir_id) + user_id_hash=user_id_hash, + fhir_id=fhir_id) # Extra user information # TODO: remvoe the idea of UserProfile diff --git a/apps/mymedicare_cb/tests/test_callback.py b/apps/mymedicare_cb/tests/test_callback.py index c1f51c785..6172b7b7d 100644 --- a/apps/mymedicare_cb/tests/test_callback.py +++ b/apps/mymedicare_cb/tests/test_callback.py @@ -155,7 +155,7 @@ def sls_user_info_mock(url, request): return { 'status_code': 200, 'content': { - 'sub': '0123456789abcdefghijklmnopqrstuvwxyz', + 'sub': '00112233-4455-6677-8899-aabbccddeeff', 'given_name': '', 'family_name': '', 'email': 'bob@bobserver.bob', diff --git a/apps/testclient/management/commands/create_test_user_and_application.py b/apps/testclient/management/commands/create_test_user_and_application.py index 4c505f247..5adaf6aed 100644 --- a/apps/testclient/management/commands/create_test_user_and_application.py +++ b/apps/testclient/management/commands/create_test_user_and_application.py @@ -46,7 +46,7 @@ def create_user(group): u.groups.add(group) c, g_o_c = Crosswalk.objects.get_or_create(user=u, _fhir_id=settings.DEFAULT_SAMPLE_FHIR_ID, - _user_id_hash="badhash", + _user_id_hash="139e178537ed3bc486e6a7195a47a82a2cd6f46e911660fe9775f6e0dd3f1130", fhir_source=get_resourcerouter()) return u From 71a4b3da7b1b8be295044f9648fc424de56a9b23 Mon Sep 17 00:00:00 2001 From: David Gage Date: Fri, 20 Dec 2019 14:08:15 -0500 Subject: [PATCH 8/8] fixup --- apps/fhir/bluebutton/tests/test_models.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/apps/fhir/bluebutton/tests/test_models.py b/apps/fhir/bluebutton/tests/test_models.py index b7cda3be0..446828ce3 100644 --- a/apps/fhir/bluebutton/tests/test_models.py +++ b/apps/fhir/bluebutton/tests/test_models.py @@ -84,7 +84,6 @@ def test_immutable_fhir_id(self): with self.assertRaises(ValidationError): cw.fhir_id = "-20000000000002" - def test_immuatble_user_id_hash(self): user = self._create_user('john', 'password', first_name='John', @@ -98,9 +97,9 @@ def test_immuatble_user_id_hash(self): server_search_expiry=1800) cw = Crosswalk.objects.create(user=user, - fhir_source=fs, - fhir_id="-20000000000001", - user_id_hash="139e178537ed3bc486e6a7195a47a82a2cd6f46e911660fe9775f6e0dd3f1130") + fhir_source=fs, + fhir_id="-20000000000001", + user_id_hash="139e178537ed3bc486e6a7195a47a82a2cd6f46e911660fe9775f6e0dd3f1130") cw = Crosswalk.objects.get(_fhir_id="-20000000000001") self.assertEqual(cw.user_id_hash, "139e178537ed3bc486e6a7195a47a82a2cd6f46e911660fe9775f6e0dd3f1130") with self.assertRaises(ValidationError):