Skip to content

Commit 40fe14e

Browse files
BB2-4193: Remove mbi hash usage (#1402)
* BB2-4193: Initial work to remove references to unhashed_mbi * Fix failing unit tests * Remove remaining references to mbi hash, fix some tests * Fix CodeQL issue * Fix failing unit test, some cleanup, single quotes * More single quotes and some clean up * Ensure generate_random_mbi conforms with MBI structure and is synthetic * Add context to docstring * Modify generate_random_mbi and add TODOs
1 parent a25ab44 commit 40fe14e

28 files changed

+556
-687
lines changed

apps/authorization/tests/test_data_access_grant.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,7 @@ def test_delete_authenticated_user_grant(self):
151151
app_name="test_app1",
152152
app_username="devuser1",
153153
app_user_organization="org1",
154+
mbi=self._generate_random_mbi(),
154155
)
155156

156157
user, application_2, ac = self._create_user_app_token_grant(
@@ -161,6 +162,7 @@ def test_delete_authenticated_user_grant(self):
161162
app_name="test_app2",
162163
app_username="devuser2",
163164
app_user_organization="org2",
165+
mbi=self._generate_random_mbi(),
164166
)
165167

166168
# 2. verify grant creation - errors if DNE or more than one is found

apps/authorization/tests/test_data_access_grant_permissions.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,7 @@ def test_revoked_data_access_grant(self):
289289
app_name="test_app1",
290290
app_username="devuser1",
291291
app_user_organization="org1",
292+
mbi=self._generate_random_mbi(),
292293
app_data_access_type="RESEARCH_STUDY",
293294
)
294295

@@ -357,6 +358,7 @@ def test_research_study_app_type(self):
357358
app_name="test_app1",
358359
app_username="devuser1",
359360
app_user_organization="org1",
361+
mbi=self._generate_random_mbi(),
360362
app_data_access_type="RESEARCH_STUDY",
361363
)
362364
self.assertEqual(app.data_access_type, "RESEARCH_STUDY")
@@ -439,6 +441,7 @@ def test_one_time_app_type(self):
439441
app_name="test_app1",
440442
app_username="devuser1",
441443
app_user_organization="org1",
444+
mbi=self._generate_random_mbi(),
442445
app_data_access_type="ONE_TIME",
443446
)
444447

@@ -481,6 +484,7 @@ def test_thirteen_month_app_type(self):
481484
app_name="test_app1",
482485
app_username="devuser1",
483486
app_user_organization="org1",
487+
mbi=self._generate_random_mbi(),
484488
app_data_access_type="THIRTEEN_MONTH",
485489
)
486490

@@ -555,6 +559,7 @@ def test_thirteen_month_app_type(self):
555559
app_name="test_app1",
556560
app_username="devuser1",
557561
app_user_organization="org1",
562+
mbi=self._generate_random_mbi(),
558563
app_data_access_type="THIRTEEN_MONTH",
559564
)
560565

@@ -607,6 +612,7 @@ def test_data_access_grant_permissions_has_permission(self):
607612
app_name="test_app1",
608613
app_username="devuser1",
609614
app_user_organization="org1",
615+
mbi=self._generate_random_mbi(),
610616
)
611617

612618
# 2. Test grant obj created OK.

apps/bb2_tools/admin.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -333,9 +333,9 @@ class BeneficiaryDashboardAdmin(ReadOnlyAdmin):
333333
"date_created",
334334
)
335335
# BB2-4166-TODO: add support for v3
336-
search_fields = ("user__username", "fhir_id_v2", "_user_id_hash", "_user_mbi_hash")
337-
readonly_fields = ("date_created",)
338-
raw_id_fields = ("user",)
336+
search_fields = ('user__username', 'fhir_id_v2', '_user_id_hash', '_user_mbi')
337+
readonly_fields = ('date_created',)
338+
raw_id_fields = ('user',)
339339

340340
def get_queryset(self, request):
341341
qs = super(BeneficiaryDashboardAdmin, self).get_queryset(request)
@@ -363,8 +363,8 @@ def get_access_tokens(self, obj):
363363
def get_identities(self, obj):
364364
# BB2-4166-TODO: add support for v3
365365
return format_html(
366-
"<div><ul><li>FHIR_ID_V2:{}</li><li>HICN HASH:{}</li><li>MBI HASH:{}</li>".format(
367-
obj.fhir_id(2), obj.user_hicn_hash, obj.user_mbi_hash
366+
'<div><ul><li>FHIR_ID_V2:{}</li><li>HICN HASH:{}</li><li>MBI:{}</li>'.format(
367+
obj.fhir_id(2), obj.user_hicn_hash, obj.user_mbi
368368
)
369369
)
370370

apps/dot_ext/tests/test_views.py

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -535,31 +535,31 @@ def test_get_tokens_on_inactive_app(self):
535535
application.save()
536536

537537
def test_delete_token_success(self):
538-
anna = self._create_user(self.test_username, "123456", fhir_id_v2="19990000000002", fhir_id_v3="39990000000002")
538+
anna = self._create_user(self.test_username, '123456', fhir_id_v2='19990000000002', fhir_id_v3='39990000000002')
539539
bob = self._create_user(
540-
"bob",
541-
"123456",
542-
fhir_id_v2="19990000000001",
540+
'bob',
541+
'123456',
542+
fhir_id_v2='19990000000001',
543543
fhir_id_v3=None,
544-
user_hicn_hash="86228a57f37efea543f4f370f96f1dbf01c3e3129041dba3ea4367545507c6e7",
545-
user_mbi_hash="98765432137efea543f4f370f96f1dbf01c3e3129041dba3ea4367545507c6e7",
544+
user_hicn_hash='86228a57f37efea543f4f370f96f1dbf01c3e3129041dba3ea4367545507c6e7',
545+
user_mbi='1SA0A00AA02',
546546
)
547547
# create a couple of capabilities
548548
capability_a = self._create_capability(
549-
"token_management", [["DELETE", r"/v1/o/tokens/\d+/"]], default=False
549+
'token_management', [['DELETE', r'/v1/o/tokens/\d+/']], default=False
550550
)
551551
# create an application and add capabilities
552552
anna_application = self._create_application(
553-
"an app",
553+
'an app',
554554
grant_type=Application.GRANT_AUTHORIZATION_CODE,
555-
redirect_uris="http://example.it",
555+
redirect_uris='http://example.it',
556556
)
557557
anna_application.scope.add(capability_a)
558558
bob_application = self._create_application(
559-
"another app",
559+
'another app',
560560
grant_type=Application.GRANT_IMPLICIT,
561561
client_type=Application.CLIENT_PUBLIC,
562-
redirect_uris="http://example.it",
562+
redirect_uris='http://example.it',
563563
user=anna_application.user,
564564
)
565565
bob_application.scope.add(capability_a)
@@ -579,10 +579,10 @@ def test_delete_token_success(self):
579579
# Post Django 2.2: An OSError exception is expected when trying to reach the
580580
# backend FHIR server and proves authentication worked.
581581
with self.assertRaisesRegexp(
582-
OSError, "Could not find the TLS certificate file"
582+
OSError, 'Could not find the TLS certificate file'
583583
):
584584
response = self.client.get(
585-
"/v1/fhir/Patient", headers={"authorization": "Bearer " + anna_token.token}
585+
'/v1/fhir/Patient', headers={'authorization': 'Bearer ' + anna_token.token}
586586
)
587587

588588
bob_tkn = self._create_test_token(bob, bob_application)
@@ -594,12 +594,12 @@ def test_delete_token_success(self):
594594
)
595595

596596
response = self.client.get(
597-
reverse("token_management:token-list"),
597+
reverse('token_management:token-list'),
598598
headers={
599-
"authorization": self._create_authorization_header(
599+
'authorization': self._create_authorization_header(
600600
anna_application.client_id, anna_application.client_secret_plain
601601
),
602-
"x-authentication": self._create_authentication_header(self.test_uuid),
602+
'x-authentication': self._create_authentication_header(self.test_uuid),
603603
},
604604
)
605605
grant_list = response.json()
@@ -609,21 +609,21 @@ def test_delete_token_success(self):
609609
)
610610
http_authn = self._create_authentication_header(self.test_uuid)
611611
response = self.client.delete(
612-
reverse("token_management:token-detail", args=[grant_list[0]["id"]]),
613-
headers={"authorization": http_authz, "x-authentication": http_authn},
612+
reverse('token_management:token-detail', args=[grant_list[0]['id']]),
613+
headers={'authorization': http_authz, 'x-authentication': http_authn},
614614
)
615615
self.assertEqual(response.status_code, 204)
616-
url_1 = reverse("token_management:token-detail", args=[grant_list[0]["id"]])
616+
url_1 = reverse('token_management:token-detail', args=[grant_list[0]['id']])
617617
http_authz = self._create_authorization_header(
618618
anna_application.client_id, anna_application.client_secret_plain
619619
)
620620
http_authn = self._create_authentication_header(self.test_uuid)
621621
failed_response = self.client.delete(
622-
url_1, headers={"authorization": http_authz, "x-authentication": http_authn}
622+
url_1, headers={'authorization': http_authz, 'x-authentication': http_authn}
623623
)
624624
self.assertEqual(failed_response.status_code, 404)
625625
response = self.client.get(
626-
"/v1/fhir/Patient", headers={"authorization": "Bearer " + anna_token.token}
626+
'/v1/fhir/Patient', headers={'authorization': 'Bearer ' + anna_token.token}
627627
)
628628
self.assertEqual(response.status_code, 401)
629629

@@ -637,22 +637,22 @@ def test_delete_token_success(self):
637637
# Post Django 2.2: An OSError exception is expected when trying to reach the
638638
# backend FHIR server and proves authentication worked.
639639
with self.assertRaisesRegexp(
640-
OSError, "Could not find the TLS certificate file"
640+
OSError, 'Could not find the TLS certificate file'
641641
):
642642
response = self.client.get(
643-
"/v1/fhir/Patient", headers={"authorization": "Bearer " + bob_tkn.token}
643+
'/v1/fhir/Patient', headers={'authorization': 'Bearer ' + bob_tkn.token}
644644
)
645645

646646
next_tkn = self._create_test_token(anna, anna_application)
647647

648648
# Post Django 2.2: An OSError exception is expected when trying to reach the
649649
# backend FHIR server and proves authentication worked.
650650
with self.assertRaisesRegexp(
651-
OSError, "Could not find the TLS certificate file"
651+
OSError, 'Could not find the TLS certificate file'
652652
):
653653
response = self.client.get(
654-
"/v1/fhir/Patient",
655-
headers={"authorization": "Bearer " + next_tkn.token},
654+
'/v1/fhir/Patient',
655+
headers={'authorization': 'Bearer ' + next_tkn.token},
656656
)
657657

658658
# self.assertEqual(next_tkn.token, tkn.token)

apps/fhir/bluebutton/admin.py

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,13 @@
44

55
@admin.register(Crosswalk)
66
class CrosswalkAdmin(admin.ModelAdmin):
7-
list_display = ("get_user_username", "fhir_id_v2", "fhir_id_v3")
8-
search_fields = ("user__username", "fhir_id_v2", "fhir_id_v3")
9-
raw_id_fields = ("user",)
7+
list_display = ('get_user_username', 'fhir_id_v2', 'fhir_id_v3')
8+
search_fields = ('user__username', 'fhir_id_v2', 'fhir_id_v3')
9+
raw_id_fields = ('user',)
1010

1111
@admin.display(
12-
description="User Name",
13-
ordering="username",
12+
description='User Name',
13+
ordering='username',
1414
)
1515
def get_user_username(self, obj):
1616
return obj.user.username
@@ -19,12 +19,12 @@ def get_user_username(self, obj):
1919
@admin.register(ArchivedCrosswalk)
2020
class ArchivedCrosswalkAdmin(admin.ModelAdmin):
2121
list_display = (
22-
"archived_at",
23-
"username",
24-
"fhir_id_v2",
25-
"fhir_id_v3",
26-
"user_id_type",
27-
"_user_id_hash",
28-
"_user_mbi_hash",
22+
'archived_at',
23+
'username',
24+
'fhir_id_v2',
25+
'fhir_id_v3',
26+
'user_id_type',
27+
'_user_id_hash',
28+
'_user_mbi',
2929
)
30-
search_fields = ("fhir_id_v2", "fhir_id_v3", "username", "_user_id_hash", "_user_mbi_hash")
30+
search_fields = ('fhir_id_v2', 'fhir_id_v3', 'username', '_user_id_hash', '_user_mbi')

apps/fhir/bluebutton/management/commands/user_mbi_backfill.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,8 @@ def process_records(self, crosswalk_records: List[Crosswalk], execute: bool) ->
105105
user_mbi_hash = crosswalk.user_mbi_hash
106106
rf = RequestFactory()
107107
request = rf.get('/')
108-
print('fhir_id %s' % (crosswalk.fhir_id))
108+
# TODO - tied to BB2-4193, remove these references to user_mbi_hash as part
109+
# of the ticket to remove the user_mbi_hash column from the crosswalk table
109110
print('user_mbi_hash %s' % (user_mbi_hash))
110111
if not user_mbi_hash:
111112
print('can\'t update this record, no user_mbi_hash')

apps/fhir/bluebutton/models.py

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -67,18 +67,6 @@ def hash_hicn(hicn):
6767
return hash_id_value(hicn)
6868

6969

70-
def hash_mbi(mbi):
71-
# BB2-237: Replaces ASSERT with exception. We should never reach this condition.
72-
if mbi == "":
73-
raise BBFhirBluebuttonModelException("MBI cannot be the empty string")
74-
75-
# NOTE: mbi value can be None here.
76-
if mbi is None:
77-
return None
78-
else:
79-
return hash_id_value(mbi)
80-
81-
8270
class Crosswalk(models.Model):
8371
"""Represents a crosswalk between a Django user (auth_user) and their MBI/HICN/FHIR IDs
8472
@@ -302,7 +290,7 @@ def create(crosswalk):
302290
fhir_id_v3=crosswalk.fhir_id(3),
303291
user_id_type=crosswalk.user_id_type,
304292
_user_id_hash=crosswalk.user_hicn_hash,
305-
_user_mbi_hash=crosswalk.user_mbi_hash,
293+
_user_mbi=crosswalk._user_mbi,
306294
date_created=crosswalk.date_created,
307295
)
308296
acw.save()

0 commit comments

Comments
 (0)