Skip to content

Commit 358416b

Browse files
Make attestations field required when purchasing privileges (#518)
Now that the frontend has been updated to pass in attestations when purchasing privileges, we are making the field required both on the API endpoint as well as the privilege schema. ### Requirements List - 🚨🚨**BREAKING CHANGE**🚨🚨This change is incompatible with old privilege records that do not have a list of attestations, any invalid privilege records will need to be deleted or updated to include an 'attestations' field. A migration script has been added as part of this PR which devs can run to fix their environments. ### Description List - Update privilege schema to make attestations field required - Update POST purchase API endpoint request schema to make attestations required - Remove flag gating enforcement of attestation versions ### Testing List - For API configuration changes: CDK tests added/updated in `backend/compact-connect/tests/unit/test_api.py` - Code review Closes #425
1 parent 755d83f commit 358416b

File tree

6 files changed

+101
-56
lines changed

6 files changed

+101
-56
lines changed

backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/record.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ class PrivilegeRecordSchema(CalculatedStatusRecordSchema):
4545
# the id of the transaction that was made when the user purchased the privilege
4646
compactTransactionId = String(required=False, allow_none=False)
4747
# list of attestations that were accepted when purchasing this privilege
48-
attestations = List(Nested(AttestationVersionRecordSchema()), required=False, allow_none=False)
48+
attestations = List(Nested(AttestationVersionRecordSchema()), required=True, allow_none=False)
4949
# the human-friendly identifier for this privilege
5050
privilegeId = String(required=True, allow_none=False)
5151

backend/compact-connect/lambdas/python/purchases/handlers/privileges.py

Lines changed: 24 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import json
2-
import os
32
from datetime import date
43

54
from aws_lambda_powertools.utilities.typing import LambdaContext
@@ -125,35 +124,30 @@ def _validate_attestations(compact: str, attestations: list[dict], has_active_mi
125124
# Get all latest attestations for this compact
126125
latest_attestations = config.compact_configuration_client.get_attestations_by_locale(compact=compact)
127126

128-
# TODO: We were asked not to enforce attestations until the frontend is updated # noqa: FIX002
129-
# to pass them in the request body. For now, we will simply check whatever
130-
# attestation is passed in to make sure it is the latest version. This conditional
131-
# should be removed once the frontend is updated to pass in all required attestations.
132-
if os.environ.get('ENFORCE_ATTESTATIONS') == 'true':
133-
# Build list of required attestations
134-
required_ids = REQUIRED_ATTESTATION_IDS.copy()
135-
if has_active_military_affiliation:
136-
required_ids.append(MILITARY_ATTESTATION_ID)
137-
138-
# Validate investigation attestations - exactly one must be provided
139-
investigation_attestations = [a for a in attestations if a['attestationId'] in INVESTIGATION_ATTESTATION_IDS]
140-
if len(investigation_attestations) != 1:
141-
raise CCInvalidRequestException(
142-
'Exactly one investigation attestation must be provided '
143-
f'(either {INVESTIGATION_ATTESTATION_IDS[0]} or {INVESTIGATION_ATTESTATION_IDS[1]})'
144-
)
145-
required_ids.append(investigation_attestations[0]['attestationId'])
146-
147-
# Check that all required attestations are present
148-
provided_ids = {a['attestationId'] for a in attestations}
149-
missing_ids = set(required_ids) - provided_ids
150-
if missing_ids:
151-
raise CCInvalidRequestException(f'Missing required attestations: {", ".join(missing_ids)}')
152-
153-
# Check for any invalid attestation IDs
154-
invalid_ids = provided_ids - set(required_ids)
155-
if invalid_ids:
156-
raise CCInvalidRequestException(f'Invalid attestations provided: {", ".join(invalid_ids)}')
127+
# Build list of required attestations
128+
required_ids = REQUIRED_ATTESTATION_IDS.copy()
129+
if has_active_military_affiliation:
130+
required_ids.append(MILITARY_ATTESTATION_ID)
131+
132+
# Validate investigation attestations - exactly one must be provided
133+
investigation_attestations = [a for a in attestations if a['attestationId'] in INVESTIGATION_ATTESTATION_IDS]
134+
if len(investigation_attestations) != 1:
135+
raise CCInvalidRequestException(
136+
'Exactly one investigation attestation must be provided '
137+
f'(either {INVESTIGATION_ATTESTATION_IDS[0]} or {INVESTIGATION_ATTESTATION_IDS[1]})'
138+
)
139+
required_ids.append(investigation_attestations[0]['attestationId'])
140+
141+
# Check that all required attestations are present
142+
provided_ids = {a['attestationId'] for a in attestations}
143+
missing_ids = set(required_ids) - provided_ids
144+
if missing_ids:
145+
raise CCInvalidRequestException(f'Missing required attestations: {", ".join(missing_ids)}')
146+
147+
# Check for any invalid attestation IDs
148+
invalid_ids = provided_ids - set(required_ids)
149+
if invalid_ids:
150+
raise CCInvalidRequestException(f'Invalid attestations provided: {", ".join(invalid_ids)}')
157151

158152
# Verify all provided attestations are the latest version
159153
for attestation in attestations:

backend/compact-connect/lambdas/python/purchases/tests/function/test_handlers/test_purchase_privileges.py

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import json
2-
import os
32
from datetime import UTC, date, datetime
43
from unittest.mock import MagicMock, patch
54

@@ -83,9 +82,6 @@ def setUp(self):
8382
from cc_common.data_model.schema.attestation import AttestationRecordSchema
8483

8584
super().setUp()
86-
# set the feature flag to enable the attestation validation feature
87-
# this should be removed when the feature is enabled by default
88-
os.environ.update({'ENFORCE_ATTESTATIONS': 'true'})
8985
# Load test attestation data
9086
with open('../common/tests/resources/dynamo/attestation.json') as f:
9187
test_attestation = json.load(f)
@@ -615,22 +611,3 @@ def test_post_purchase_privileges_validates_military_attestation(self, mock_purc
615611

616612
resp = post_purchase_privileges(event, self.mock_context)
617613
self.assertEqual(200, resp['statusCode'], resp['body'])
618-
619-
# TODO - remove this test once the feature flag is removed # noqa: FIX002
620-
@patch('handlers.privileges.PurchaseClient')
621-
def test_post_purchase_privileges_should_not_validate_attestations_if_flag_not_set(
622-
self, mock_purchase_client_constructor
623-
):
624-
"""Test that military attestation is required when user has active military affiliation."""
625-
from handlers.privileges import post_purchase_privileges
626-
627-
os.environ.update({'ENFORCE_ATTESTATIONS': 'false'})
628-
629-
self._when_purchase_client_successfully_processes_request(mock_purchase_client_constructor)
630-
self._load_military_affiliation_record_data(status='active')
631-
632-
event = self._when_testing_provider_user_event_with_custom_claims()
633-
event['body'] = _generate_test_request_body(attestations=[])
634-
635-
resp = post_purchase_privileges(event, self.mock_context)
636-
self.assertEqual(200, resp['statusCode'], resp['body'])
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
# ruff: noqa: T201 we use print statements for migration scripts
2+
#!/usr/bin/env python3
3+
import argparse
4+
5+
import boto3
6+
7+
# This migration script updates privilege records that don't have an attestations field
8+
# by adding an empty attestations list to them.
9+
10+
11+
def update_privileges_missing_attestations(table_name: str, dry_run: bool = False) -> None:
12+
"""
13+
Scans the provider table for privilege records missing the attestations field
14+
and adds an empty attestations list to them.
15+
16+
:param table_name: The name of the DynamoDB table to update
17+
:param dry_run: If True, only print what would be updated without making changes
18+
"""
19+
dynamodb = boto3.resource('dynamodb')
20+
table = dynamodb.Table(table_name)
21+
22+
# Get all privilege records
23+
scan_kwargs = {
24+
'FilterExpression': 'contains(#type_attr, :type_value)',
25+
'ExpressionAttributeNames': {'#type_attr': 'type'},
26+
'ExpressionAttributeValues': {':type_value': 'privilege'},
27+
}
28+
29+
updated_count = 0
30+
try:
31+
done = False
32+
start_key = None
33+
while not done:
34+
if start_key:
35+
scan_kwargs['ExclusiveStartKey'] = start_key
36+
response = table.scan(**scan_kwargs)
37+
items = response.get('Items', [])
38+
39+
for item in items:
40+
# Check if attestations field is missing
41+
if 'attestations' not in item:
42+
print(f"{'Would update' if dry_run else 'Updating'} record with pk={item['pk']}, sk={item['sk']}")
43+
44+
if not dry_run:
45+
# Update the item with an empty attestations list
46+
table.update_item(
47+
Key={'pk': item['pk'], 'sk': item['sk']},
48+
UpdateExpression='SET attestations = :empty_list',
49+
ExpressionAttributeValues={':empty_list': []},
50+
)
51+
updated_count += 1
52+
53+
start_key = response.get('LastEvaluatedKey')
54+
done = start_key is None
55+
56+
print(f"{'Would have updated' if dry_run else 'Successfully updated'} {updated_count} privilege records")
57+
58+
except Exception as e:
59+
print(f'Error updating privileges: {str(e)}')
60+
raise
61+
62+
63+
if __name__ == '__main__':
64+
parser = argparse.ArgumentParser(
65+
description='Update privilege records to include empty attestations list if missing'
66+
)
67+
parser.add_argument('table_name', help='The name of the DynamoDB table to update')
68+
parser.add_argument('-d', '--dry-run', action='store_true', help='Perform a dry run without making any changes')
69+
70+
args = parser.parse_args()
71+
72+
print(f"Starting migration on table: {args.table_name} {'(DRY RUN)' if args.dry_run else ''}")
73+
update_privileges_missing_attestations(args.table_name, args.dry_run)

backend/compact-connect/stacks/api_stack/v1_api/api_model.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -480,7 +480,7 @@ def post_purchase_privileges_request_model(self) -> Model:
480480
description='Post purchase privileges request model',
481481
schema=JsonSchema(
482482
type=JsonSchemaType.OBJECT,
483-
required=['selectedJurisdictions', 'orderInformation'],
483+
required=['selectedJurisdictions', 'orderInformation', 'attestations'],
484484
properties={
485485
'selectedJurisdictions': JsonSchema(
486486
type=JsonSchemaType.ARRAY,

backend/compact-connect/tests/resources/snapshots/PURCHASE_PRIVILEGE_REQUEST_SCHEMA.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,8 @@
174174
},
175175
"required": [
176176
"selectedJurisdictions",
177-
"orderInformation"
177+
"orderInformation",
178+
"attestations"
178179
],
179180
"type": "object",
180181
"$schema": "http://json-schema.org/draft-04/schema#"

0 commit comments

Comments
 (0)