diff --git a/infrastructure/modules/lambda/lambda.tf b/infrastructure/modules/lambda/lambda.tf index f6d07bd0..889cd6d2 100644 --- a/infrastructure/modules/lambda/lambda.tf +++ b/infrastructure/modules/lambda/lambda.tf @@ -24,6 +24,7 @@ resource "aws_lambda_function" "eligibility_signposting_lambda" { LOG_LEVEL = var.log_level ENABLE_XRAY_PATCHING = var.enable_xray_patching API_DOMAIN_NAME = var.api_domain_name + HASHING_SECRET_NAME = var.hashing_secret_name } } diff --git a/infrastructure/modules/lambda/variables.tf b/infrastructure/modules/lambda/variables.tf index dd70823e..85b63986 100644 --- a/infrastructure/modules/lambda/variables.tf +++ b/infrastructure/modules/lambda/variables.tf @@ -78,3 +78,8 @@ variable "api_domain_name" { description = "api domain name - env variable for status endpoint response" type = string } + +variable "hashing_secret_name" { + description = "hashing secret name" + type = string +} diff --git a/infrastructure/modules/secrets_manager/outputs.tf b/infrastructure/modules/secrets_manager/outputs.tf new file mode 100644 index 00000000..c2cf721e --- /dev/null +++ b/infrastructure/modules/secrets_manager/outputs.tf @@ -0,0 +1,8 @@ +output "aws_hashing_secret_arn" { + value = aws_secretsmanager_secret.hashing_secret.arn +} + +output "aws_hashing_secret_name" { + value = aws_secretsmanager_secret.hashing_secret.name +} + diff --git a/infrastructure/modules/secrets_manager/secrets_manager.tf b/infrastructure/modules/secrets_manager/secrets_manager.tf index 2d14c7a4..6a216ad9 100644 --- a/infrastructure/modules/secrets_manager/secrets_manager.tf +++ b/infrastructure/modules/secrets_manager/secrets_manager.tf @@ -10,15 +10,6 @@ resource "aws_secretsmanager_secret" "hashing_secret" { } } -# Initial secrets -resource "aws_secretsmanager_secret_version" "hashing_secrets_test" { - secret_id = aws_secretsmanager_secret.hashing_secret.id - secret_string = "initial_secret" - lifecycle { - ignore_changes = [secret_string] - } -} - # Resource-based policy attached to the secret resource "aws_secretsmanager_secret_policy" "hashing_secret_policy" { secret_arn = aws_secretsmanager_secret.hashing_secret.arn diff --git a/infrastructure/modules/secrets_manager/variables.tf b/infrastructure/modules/secrets_manager/variables.tf index 86930013..11827f78 100644 --- a/infrastructure/modules/secrets_manager/variables.tf +++ b/infrastructure/modules/secrets_manager/variables.tf @@ -1,6 +1,6 @@ variable "external_write_access_role_arn" { - description = "Arn of the external write access role to provide secret manager access" - type = string + description = "List of ARNs for external write access roles" + type = list(string) } variable "eligibility_lambda_role_arn" { diff --git a/infrastructure/stacks/api-layer/iam_policies.tf b/infrastructure/stacks/api-layer/iam_policies.tf index f5ccfabb..4bcae520 100644 --- a/infrastructure/stacks/api-layer/iam_policies.tf +++ b/infrastructure/stacks/api-layer/iam_policies.tf @@ -530,3 +530,34 @@ resource "aws_iam_role_policy" "external_audit_kms_access_policy" { role = aws_iam_role.write_access_role[count.index].id policy = data.aws_iam_policy_document.external_role_s3_audit_kms_access_policy.json } + +# IAM policy document for Lambda secret access +data "aws_iam_policy_document" "secrets_access_policy" { + statement { + effect = "Allow" + + actions = [ + "secretsmanager:GetSecretValue", + "secretsmanager:DescribeSecret", + ] + + resources = [ + module.secrets_manager.aws_hashing_secret_arn + ] + } +} + +# Attach secret read policy to Lambda role +resource "aws_iam_role_policy" "lambda_secret_read_policy_attachment" { + name = "LambdaSecretReadAccess" + role = aws_iam_role.eligibility_lambda_role.id + policy = data.aws_iam_policy_document.secrets_access_policy.json +} + +# Attach secret read policy to external write role +resource "aws_iam_role_policy" "external_secret_read_policy_attachment" { + count = length(aws_iam_role.write_access_role) + name = "ExternalSecretReadAccess" + role = aws_iam_role.write_access_role[count.index].id + policy = data.aws_iam_policy_document.secrets_access_policy.json +} diff --git a/infrastructure/stacks/api-layer/lambda.tf b/infrastructure/stacks/api-layer/lambda.tf index 8576a19d..87e32a4f 100644 --- a/infrastructure/stacks/api-layer/lambda.tf +++ b/infrastructure/stacks/api-layer/lambda.tf @@ -25,6 +25,7 @@ module "eligibility_signposting_lambda_function" { eligibility_rules_bucket_name = module.s3_rules_bucket.storage_bucket_name eligibility_status_table_name = module.eligibility_status_table.table_name kinesis_audit_stream_to_s3_name = module.eligibility_audit_firehose_delivery_stream.firehose_stream_name + hashing_secret_name = module.secrets_manager.aws_hashing_secret_name lambda_insights_extension_version = 38 log_level = "INFO" enable_xray_patching = "true" diff --git a/infrastructure/stacks/api-layer/secrets_manager.tf b/infrastructure/stacks/api-layer/secrets_manager.tf index 9785228b..85570d70 100644 --- a/infrastructure/stacks/api-layer/secrets_manager.tf +++ b/infrastructure/stacks/api-layer/secrets_manager.tf @@ -1,9 +1,8 @@ module "secrets_manager" { source = "../../modules/secrets_manager" - count = length(aws_iam_role.write_access_role) - external_write_access_role_arn = aws_iam_role.write_access_role[count.index].arn - environment = var.environment - stack_name = local.stack_name - workspace = terraform.workspace - eligibility_lambda_role_arn = aws_iam_role.eligibility_lambda_role.arn + external_write_access_role_arn = aws_iam_role.write_access_role[*].arn + environment = var.environment + stack_name = local.stack_name + workspace = terraform.workspace + eligibility_lambda_role_arn = aws_iam_role.eligibility_lambda_role.arn } diff --git a/src/eligibility_signposting_api/repos/person_repo.py b/src/eligibility_signposting_api/repos/person_repo.py index db35540a..d0c7d041 100644 --- a/src/eligibility_signposting_api/repos/person_repo.py +++ b/src/eligibility_signposting_api/repos/person_repo.py @@ -55,28 +55,30 @@ def get_person_record(self, nhs_hash: str | None) -> Any: return None def get_eligibility_data(self, nhs_number: NHSNumber) -> Person: - # AWSCURRENT secret - nhs_hash = self._hashing_service.hash_with_current_secret(nhs_number) - items = self.get_person_record(nhs_hash) + # Hash using AWSCURRENT secret and fetch items + items = None + nhs_hashed_with_current = self._hashing_service.hash_with_current_secret(nhs_number) + if nhs_hashed_with_current: + items = self.get_person_record(nhs_hashed_with_current) + if not items: + logger.warning("The AWSCURRENT secret was tried, but no person record was found") if not items: - logger.error("No person record found for hashed nhs_number using secret AWSCURRENT") - - # AWSPREVIOUS secret - nhs_hash = self._hashing_service.hash_with_previous_secret(nhs_number) - - if nhs_hash is not None: - items = self.get_person_record(nhs_hash) + # Hash using AWSPREVIOUS secret and fetch items + nhs_hashed_with_previous = self._hashing_service.hash_with_previous_secret(nhs_number) + if nhs_hashed_with_previous: + items = self.get_person_record(nhs_hashed_with_previous) if not items: - logger.error("No person record found for hashed nhs_number using secret AWSPREVIOUS") + logger.error("The AWSPREVIOUS secret was also tried, but no person record was found") message = "Person not found after checking AWSCURRENT and AWSPREVIOUS." raise NotFoundError(message) else: - # fallback not hashed NHS number + # fallback : Fetch using Raw NHS number items = self.get_person_record(nhs_number) if not items: - logger.error("No person record found for not hashed nhs_number") + logger.error("The not hashed nhs number was also tried, but no person record was found") message = "Person not found after checking AWSCURRENT, AWSPREVIOUS, and not hashed NHS numbers." raise NotFoundError(message) + logger.info("Person record found") return Person(data=items) diff --git a/src/eligibility_signposting_api/repos/secret_repo.py b/src/eligibility_signposting_api/repos/secret_repo.py index 1567abc1..cf37ad9c 100644 --- a/src/eligibility_signposting_api/repos/secret_repo.py +++ b/src/eligibility_signposting_api/repos/secret_repo.py @@ -26,7 +26,7 @@ def _get_secret_by_stage(self, secret_name: str, stage: str) -> dict[str, str]: return {stage: response["SecretString"]} except ClientError: - logger.exception("Failed to get secret %s at stage %s", secret_name, stage) + logger.warning("Failed to get secret %s at stage %s", secret_name, stage) return {} def get_secret_current(self, secret_name: str) -> dict[str, str]: