Skip to content

Commit b50385c

Browse files
committed
Reject a bundle with more entries than the configured maximum
1 parent df209c3 commit b50385c

13 files changed

Lines changed: 66 additions & 19 deletions

File tree

‎app.conf.example‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,3 +94,7 @@ expected_audiences = https://localhost:8501
9494
default_page_size = 50
9595
# Upper limit for `_count`, larger values are clamped to this value
9696
max_page_size = 100
97+
98+
[bundle]
99+
# Maximum number of entries in a transaction Bundle, larger bundles are rejected
100+
max_entries = 100

‎app/config.py‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,10 @@ def validate_page_sizes(self) -> Self:
107107
return self
108108

109109

110+
class ConfigBundle(BaseModel):
111+
max_entries: int = Field(default=100, ge=1)
112+
113+
110114
class ConfigLogging(GFConfigLogging):
111115
_split_console_streams = field_validator("console_streams", mode="before")(split_comma_separated())
112116

@@ -120,6 +124,7 @@ class Config(BaseModel):
120124
uvicorn: ConfigUvicorn
121125
authorization_headers: ConfigAuthorizationHeaders
122126
pagination: ConfigPagination = Field(default_factory=ConfigPagination)
127+
bundle: ConfigBundle = Field(default_factory=ConfigBundle)
123128

124129

125130
def read_ini_file(path: str) -> Any:

‎app/container.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ def container_config(binder: inject.Binder) -> None:
7070
)
7171
binder.bind(LocalizationListService, localization_list_service)
7272

73-
bundle_service = BundleService(localization_list_service)
73+
bundle_service = BundleService(localization_list_service, max_entries=config.bundle.max_entries)
7474
binder.bind(BundleService, bundle_service)
7575

7676
binder.bind(ConfigCryptoServiceApi, config.crypto_service_api)

‎app/errors/mapping.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
from app.models.fhir.resources.operation_outcome.resource import IssueType
66
from app.services.exceptions import (
7+
BundleTooLargeError,
78
ConflictError,
89
CryptoServiceError,
910
ForbiddenError,
@@ -43,6 +44,7 @@ class ErrorSpec(BaseModel):
4344
PseudonymError: ErrorSpec(severity="error", fhir_code="invalid", http_status=422, log_as="invalid_request"),
4445
PseudonymExchangeError: ErrorSpec(severity="error", fhir_code="invalid", http_status=400, log_as="failure"),
4546
CryptoServiceError: ErrorSpec(severity="error", fhir_code="transient", http_status=502, log_as="failure"),
47+
BundleTooLargeError: ErrorSpec(severity="error", fhir_code="too-costly", http_status=413, log_as="invalid_request"),
4648
}
4749

4850
# Applied to any exception without a mapped type (rendered as HTTP 500). Unmapped

‎app/routers/fhir/base.py‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
from app.models.fhir.resources.operation_outcome.resource import OperationOutcome
1111
from app.models.response import FHIRJSONResponse
1212
from app.routers.dependencies import get_auth_context
13-
from app.services.exceptions import InvalidModelError
1413
from app.services.fhir.bundle import BundleService
1514

1615
logger = logging.getLogger(__name__)
@@ -24,9 +23,7 @@ def register(
2423
ctx: Annotated[AuthContext, Depends(get_auth_context)],
2524
) -> Any:
2625
results_bundle = Bundle[Any](entry=[])
27-
valid_bundle = bundle_service.validate_localization_bundle_structure(data)
28-
if not valid_bundle:
29-
raise InvalidModelError("Bundle.entry is invalid")
26+
bundle_service.validate_entry_count(data)
3027
for i, entry in enumerate(data.entry):
3128
result = bundle_service.process_entry(
3229
ctx=ctx,

‎app/services/exceptions.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,3 +85,7 @@ class PseudonymExchangeError(Exception):
8585

8686
class CryptoServiceError(Exception):
8787
pass
88+
89+
90+
class BundleTooLargeError(Exception):
91+
pass

‎app/services/fhir/bundle.py‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
from app.models.ura import UraNumber
1919
from app.services.auth.auth_context import AuthContextService
2020
from app.services.exceptions import (
21+
BundleTooLargeError,
22+
InvalidModelError,
2123
InvalidResourceError,
2224
UnauthorizedManagingRequestError,
2325
UnauthorizedScopeError,
@@ -48,11 +50,9 @@ def _entry_for_exception(index: int, exc: Exception) -> BundleEntry[Any]:
4850

4951

5052
class BundleService:
51-
def __init__(
52-
self,
53-
localization_list_service: LocalizationListService,
54-
) -> None:
53+
def __init__(self, localization_list_service: LocalizationListService, max_entries: int) -> None:
5554
self.localization_list_service = localization_list_service
55+
self.max_entries = max_entries
5656

5757
def process_entry(self, ctx: AuthContext, entry: BundleEntry[Any], index: int) -> BundleEntry[Any]:
5858
if entry.request is None:
@@ -258,9 +258,11 @@ def requires_managing_request(method: str) -> bool:
258258
"""Whether an entry is a managing operation and so needs a source_id"""
259259
return method in ("POST", "DELETE")
260260

261-
@staticmethod
262-
def validate_localization_bundle_structure(bundle: Bundle[Any]) -> bool:
263-
return len(bundle.entry) > 0
261+
def validate_entry_count(self, bundle: Bundle[Any]) -> None:
262+
if not bundle.entry:
263+
raise InvalidModelError("Bundle.entry is invalid")
264+
if len(bundle.entry) > self.max_entries:
265+
raise BundleTooLargeError(f"Bundle.entry holds more than the maximum of {self.max_entries} entries")
264266

265267
def resolve_request_url(self, url: str, index: int) -> EntryRequestDto | BundleEntry[LocalizationList]:
266268
try:

‎tests/errors/test_mapping.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
from app.models.auth.data import AuthorizationScope
55
from app.models.fhir.resources.operation_outcome.resource import IssueType
66
from app.services.exceptions import (
7+
BundleTooLargeError,
78
ConflictError,
89
CryptoServiceError,
910
ForbiddenError,
@@ -41,6 +42,9 @@ def _scope_error() -> UnauthorizedScopeError:
4142
pytest.param(ValueError("boom"), DEFAULT_SPEC, id="value-error-is-unexpected"),
4243
pytest.param(PseudonymExchangeError("rejected"), _spec("invalid", 400, "failure"), id="pseudonym-exchange"),
4344
pytest.param(CryptoServiceError("down"), _spec("transient", 502, "failure"), id="crypto-service"),
45+
pytest.param(
46+
BundleTooLargeError("too many"), _spec("too-costly", 413, "invalid_request"), id="bundle-too-large"
47+
),
4448
pytest.param(InvalidModelError("bad"), _spec("structure", 400, "invalid_request"), id="invalid-model"),
4549
pytest.param(_scope_error(), _spec("security", 403), id="unauthorized-scope-via-mro"),
4650
pytest.param(UnauthorizedManagingRequestError(), _spec("security", 403), id="unauthorized-managing-via-mro"),

‎tests/routers/conftest.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,9 @@ def make_test_client(
124124
app.dependency_overrides[get_key_info_service] = lambda: key_info_service
125125
app.dependency_overrides[get_pseudonym_referral_service] = lambda: pseudonym_referral_service
126126
app.dependency_overrides[get_localization_list_service] = lambda: localization_list_service
127-
app.dependency_overrides[get_bundle_service] = lambda: BundleService(localization_list_service)
127+
app.dependency_overrides[get_bundle_service] = lambda: BundleService(
128+
localization_list_service, max_entries=get_test_config().bundle.max_entries
129+
)
128130

129131
def override_auth_ctx(request: Request) -> AuthContext:
130132
request.state.auth = auth_context

‎tests/routers/test_fhir_base.py‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
from app.services.referral_service import ReferralService
1414
from tests.conftest import make_list_resource
1515
from tests.routers.conftest import TEST_URA, make_auth_context, make_test_client
16+
from tests.test_config import get_test_config
1617

1718
FHIR_JSON = {"Content-Type": "application/fhir+json"}
1819

@@ -47,6 +48,16 @@ def test_processes_a_post_entry(self, client: TestClient) -> None:
4748
assert entry["response"]["status"] == "201"
4849
assert entry["resource"]["id"] is not None
4950

51+
def test_rejects_a_bundle_over_the_entry_limit_without_processing_it(self, client: TestClient) -> None:
52+
entries = [_post_entry()] * (get_test_config().bundle.max_entries + 1)
53+
with capture_records("app") as records:
54+
response = client.post("/fhir", json=_bundle(*entries), headers=FHIR_JSON)
55+
56+
assert response.status_code == 413
57+
assert response.json()["issue"][0]["code"] == "too-costly"
58+
assert len(records.for_event(Log.INVALID_REQUEST)) == 1
59+
assert records.for_event(Log.REGISTERED_REFERRAL) == []
60+
5061
def test_rejects_an_empty_bundle(self, client: TestClient) -> None:
5162
response = client.post("/fhir", json=_bundle(), headers=FHIR_JSON)
5263

0 commit comments

Comments
 (0)