Skip to content

Commit cd01ca8

Browse files
ayeshalshukri1-nhsKarthikeyannhsrobbailiff2
authored
eli-540 lambda to support hashed nhsnum (#483)
* secret handling * Fixed some integration tests. * Fixed integration test hashing serivce. * Stub for hashing service and secret repo. * Fixed integration tests. * Added logic for current and previous hash checks. * Added global const for secret * added previous secret testing. * Update logic with nhs fallback. * Fixed linting. * pyright errors fixed. * more info on error message. * Added extra tests for person repo function. * Added tests and linting. * Added hashing service tests. * Added testing and linting. * refined tests. * Updated logic as per PR feedback. * Refactored integration tests hashing service factory. * Refactored persisted person factory. * Added parma test for scenarios. * Updated scenarios for key tests. * updated description. * linting. * Added docstring for secret key scenario tests * Added more person repo test scenarios and updated docstring --------- Co-authored-by: karthikeyannhs <[email protected]> Co-authored-by: Robert <[email protected]>
1 parent 2260954 commit cd01ca8

File tree

13 files changed

+992
-38
lines changed

13 files changed

+992
-38
lines changed

src/eligibility_signposting_api/config/config.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
from yarl import URL
77

8+
from eligibility_signposting_api.processors.hashing_service import HashSecretName
89
from eligibility_signposting_api.repos.campaign_repo import BucketName
910
from eligibility_signposting_api.repos.person_repo import TableName
1011

@@ -22,6 +23,7 @@ def config() -> dict[str, Any]:
2223
person_table_name = TableName(os.getenv("PERSON_TABLE_NAME", "test_eligibility_datastore"))
2324
rules_bucket_name = BucketName(os.getenv("RULES_BUCKET_NAME", "test-rules-bucket"))
2425
audit_bucket_name = BucketName(os.getenv("AUDIT_BUCKET_NAME", "test-audit-bucket"))
26+
hashing_secret_name = HashSecretName(os.getenv("HASHING_SECRET_NAME", "test_secret"))
2527
aws_default_region = AwsRegion(os.getenv("AWS_DEFAULT_REGION", "eu-west-1"))
2628
enable_xray_patching = bool(os.getenv("ENABLE_XRAY_PATCHING", "false"))
2729
kinesis_audit_stream_to_s3 = AwsKinesisFirehoseStreamName(
@@ -42,6 +44,8 @@ def config() -> dict[str, Any]:
4244
"firehose_endpoint": None,
4345
"kinesis_audit_stream_to_s3": kinesis_audit_stream_to_s3,
4446
"enable_xray_patching": enable_xray_patching,
47+
"secretsmanager_endpoint": None,
48+
"hashing_secret_name": hashing_secret_name,
4549
"log_level": log_level,
4650
}
4751

@@ -58,5 +62,7 @@ def config() -> dict[str, Any]:
5862
"firehose_endpoint": URL(os.getenv("FIREHOSE_ENDPOINT", local_stack_endpoint)),
5963
"kinesis_audit_stream_to_s3": kinesis_audit_stream_to_s3,
6064
"enable_xray_patching": enable_xray_patching,
65+
"secretsmanager_endpoint": URL(os.getenv("SECRET_MANAGER_ENDPOINT", local_stack_endpoint)),
66+
"hashing_secret_name": hashing_secret_name,
6167
"log_level": log_level,
6268
}

src/eligibility_signposting_api/processors/__init__.py

Whitespace-only changes.
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
import hashlib
2+
import hmac
3+
from typing import Annotated, NewType
4+
5+
from wireup import Inject, service
6+
7+
from eligibility_signposting_api.repos.secret_repo import SecretRepo
8+
9+
HashSecretName = NewType("HashSecretName", str)
10+
11+
12+
def _hash(nhs_number: str, secret_value: str | None) -> str | None:
13+
if not secret_value:
14+
return None
15+
16+
nhs_str = str(nhs_number)
17+
18+
return hmac.new(
19+
secret_value.encode("utf-8"),
20+
nhs_str.encode("utf-8"),
21+
hashlib.sha512,
22+
).hexdigest()
23+
24+
25+
@service
26+
class HashingService:
27+
def __init__(
28+
self,
29+
secret_repo: Annotated[SecretRepo, Inject()],
30+
hash_secret_name: Annotated[HashSecretName, Inject(param="hashing_secret_name")],
31+
) -> None:
32+
super().__init__()
33+
self.secret_repo = secret_repo
34+
self.hash_secret_name = hash_secret_name
35+
36+
def hash_with_current_secret(self, nhs_number: str) -> str | None:
37+
secret_value = self.secret_repo.get_secret_current(self.hash_secret_name).get("AWSCURRENT")
38+
return _hash(nhs_number, secret_value)
39+
40+
def hash_with_previous_secret(self, nhs_number: str) -> str | None:
41+
secret_value = self.secret_repo.get_secret_previous(self.hash_secret_name).get("AWSPREVIOUS")
42+
return _hash(nhs_number, secret_value)
Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from .campaign_repo import CampaignRepo
22
from .exceptions import NotFoundError
33
from .person_repo import PersonRepo
4+
from .secret_repo import SecretRepo
45

5-
__all__ = ["CampaignRepo", "NotFoundError", "PersonRepo"]
6+
__all__ = ["CampaignRepo", "NotFoundError", "PersonRepo", "SecretRepo"]

src/eligibility_signposting_api/repos/factory.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,3 +43,17 @@ def firehose_client_factory(
4343
) -> BaseClient:
4444
endpoint_url = str(firehose_endpoint) if firehose_endpoint is not None else None
4545
return session.client("firehose", endpoint_url=endpoint_url)
46+
47+
48+
@service(qualifier="secretsmanager")
49+
def secretsmanager_client_factory(
50+
session: Session,
51+
secretsmanager_endpoint: Annotated[URL, Inject(param="secretsmanager_endpoint")],
52+
aws_default_region: Annotated[AwsRegion, Inject(param="aws_default_region")],
53+
) -> BaseClient:
54+
endpoint_url = str(secretsmanager_endpoint) if secretsmanager_endpoint is not None else None
55+
return session.client(
56+
service_name="secretsmanager",
57+
endpoint_url=endpoint_url,
58+
region_name=aws_default_region,
59+
)

src/eligibility_signposting_api/repos/person_repo.py

Lines changed: 41 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
from eligibility_signposting_api.model.eligibility_status import NHSNumber
99
from eligibility_signposting_api.model.person import Person
10+
from eligibility_signposting_api.processors.hashing_service import HashingService
1011
from eligibility_signposting_api.repos.exceptions import NotFoundError
1112

1213
logger = logging.getLogger(__name__)
@@ -32,17 +33,50 @@ class PersonRepo:
3233
This data is held in a handful of records in a single Dynamodb table.
3334
"""
3435

35-
def __init__(self, table: Annotated[Any, Inject(qualifier="person_table")]) -> None:
36+
def __init__(
37+
self,
38+
table: Annotated[Any, Inject(qualifier="person_table")],
39+
hashing_service: Annotated[HashingService, Inject()],
40+
) -> None:
3641
super().__init__()
3742
self.table = table
43+
self._hashing_service = hashing_service
44+
45+
def get_person_record(self, nhs_hash: str | None) -> Any:
46+
if nhs_hash:
47+
response = self.table.query(KeyConditionExpression=Key("NHS_NUMBER").eq(nhs_hash))
48+
49+
items = response.get("Items", [])
50+
has_person = any(item.get("ATTRIBUTE_TYPE") == "PERSON" for item in items)
51+
52+
if has_person:
53+
return items
54+
55+
return None
3856

3957
def get_eligibility_data(self, nhs_number: NHSNumber) -> Person:
40-
response = self.table.query(KeyConditionExpression=Key("NHS_NUMBER").eq(nhs_number))
58+
# AWSCURRENT secret
59+
nhs_hash = self._hashing_service.hash_with_current_secret(nhs_number)
60+
items = self.get_person_record(nhs_hash)
61+
62+
if not items:
63+
logger.error("No person record found for hashed nhs_number using secret AWSCURRENT")
64+
65+
# AWSPREVIOUS secret
66+
nhs_hash = self._hashing_service.hash_with_previous_secret(nhs_number)
4167

42-
if not (items := response.get("Items")) or not next(
43-
(item for item in items if item.get("ATTRIBUTE_TYPE") == "PERSON"), None
44-
):
45-
message = f"Person not found with nhs_number {nhs_number}"
46-
raise NotFoundError(message)
68+
if nhs_hash is not None:
69+
items = self.get_person_record(nhs_hash)
70+
if not items:
71+
logger.error("No person record found for hashed nhs_number using secret AWSPREVIOUS")
72+
message = "Person not found after checking AWSCURRENT and AWSPREVIOUS."
73+
raise NotFoundError(message)
74+
else:
75+
# fallback not hashed NHS number
76+
items = self.get_person_record(nhs_number)
77+
if not items:
78+
logger.error("No person record found for not hashed nhs_number")
79+
message = "Person not found after checking AWSCURRENT, AWSPREVIOUS, and not hashed NHS numbers."
80+
raise NotFoundError(message)
4781

4882
return Person(data=items)
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
import logging
2+
from typing import Annotated, NewType
3+
4+
from botocore.client import BaseClient
5+
from botocore.exceptions import ClientError
6+
from wireup import Inject, service
7+
8+
logger = logging.getLogger(__name__)
9+
10+
SecretName = NewType("SecretName", str)
11+
12+
13+
@service
14+
class SecretRepo:
15+
def __init__(self, secret_manager: Annotated[BaseClient, Inject(qualifier="secretsmanager")]) -> None:
16+
super().__init__()
17+
self.secret_manager = secret_manager
18+
19+
def _get_secret_by_stage(self, secret_name: str, stage: str) -> dict[str, str]:
20+
"""Internal helper to fetch a secret by version stage."""
21+
try:
22+
response = self.secret_manager.get_secret_value(
23+
SecretId=secret_name,
24+
VersionStage=stage,
25+
)
26+
return {stage: response["SecretString"]}
27+
28+
except ClientError:
29+
logger.exception("Failed to get secret %s at stage %s", secret_name, stage)
30+
return {}
31+
32+
def get_secret_current(self, secret_name: str) -> dict[str, str]:
33+
return self._get_secret_by_stage(secret_name, "AWSCURRENT")
34+
35+
def get_secret_previous(self, secret_name: str) -> dict[str, str]:
36+
return self._get_secret_by_stage(secret_name, "AWSPREVIOUS")

0 commit comments

Comments
 (0)