Skip to content

Commit f22dea2

Browse files
committed
Only log to Insights if HMAC signature failed verification
This will help us filter out spam requests as they should have passed the API key test - otherwise malicious requests could send multiple requests and trigger a calculation of the signature as well as log to Insights.
1 parent 2abe19a commit f22dea2

File tree

4 files changed

+24
-10
lines changed

4 files changed

+24
-10
lines changed

manage_breast_screening/notifications/tests/test_views.py

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -70,11 +70,6 @@ def test_create_message_status_with_invalid_request(mock_insights_logger):
7070
assert response.status_code == expected_response.status_code
7171
assert response.text == expected_response.text
7272

73-
mock_insights_logger.assert_called_once_with(
74-
message="Signature does not match",
75-
event_name="create_message_status_validation_error",
76-
)
77-
7873

7974
def test_create_message_status_with_queue_configuration_error(mock_insights_logger):
8075
body = {"some": "data"}

manage_breast_screening/notifications/tests/validators/test_request_validator.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,20 @@ def test_valid_with_invalid_api_key(self):
7171
req = self.mock_request(headers)
7272
assert RequestValidator(req).valid() == (False, "Invalid API key")
7373

74+
def test_valid_invalid_signature(self, mock_insights_logger):
75+
"""Test that valid headers with invalid signature doesn't pass verification."""
76+
signature = "invalid_signature"
77+
headers = {
78+
RequestValidator.API_KEY_HEADER_NAME: "api_key",
79+
RequestValidator.SIGNATURE_HEADER_NAME: signature,
80+
}
81+
req = self.mock_request(headers)
82+
assert RequestValidator(req).valid() == (False, "Signature does not match")
83+
mock_insights_logger.assert_called_with(
84+
message="Signature does not match",
85+
event_name="create_message_status_validation_error",
86+
)
87+
7488
def test_valid(self):
7589
"""Test that valid API key and signature headers pass verification."""
7690
body = '{"this": "that"}'

manage_breast_screening/notifications/validators/request_validator.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@
22
import hmac
33
import os
44

5+
from manage_breast_screening.notifications.services.application_insights_logging import (
6+
ApplicationInsightsLogging,
7+
)
8+
59

610
class RequestValidator:
711
ENCODING = "ASCII"
@@ -18,7 +22,12 @@ def valid(self) -> tuple[bool, str]:
1822
return False, error_message
1923

2024
if not self.verify_signature():
21-
return False, "Signature does not match"
25+
error_message = "Signature does not match"
26+
ApplicationInsightsLogging().custom_event(
27+
message=error_message,
28+
event_name="create_message_status_validation_error",
29+
)
30+
return False, error_message
2231

2332
return True, ""
2433

manage_breast_screening/notifications/views.py

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,6 @@ def create_message_status(request):
2626
valid, message = RequestValidator(request).valid()
2727

2828
if not valid:
29-
ApplicationInsightsLogging().custom_event(
30-
message=message,
31-
event_name="create_message_status_validation_error",
32-
)
3329
return JsonResponse({"error": {"message": message}}, status=400)
3430

3531
try:

0 commit comments

Comments
 (0)