Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions spp_dci_server/middleware/signature.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,12 +177,15 @@ async def verify_dci_signature(
error_code="err.signature.missing",
)

# Look up sender in registry
# Look up sender in registry. Require an ACTIVE record: a deactivated
# sender must not authenticate (and downstream consent resolution also
# requires active, so accepting an inactive sender here would desync).
# nosemgrep: odoo-sudo-without-context
sender_registry = env["spp.dci.sender.registry"].sudo().search([("sender_id", "=", sender_id)], limit=1)
SenderRegistry = env["spp.dci.sender.registry"].sudo()
sender_registry = SenderRegistry.search([("sender_id", "=", sender_id), ("active", "=", True)], limit=1)

if not sender_registry:
_logger.warning("Unknown sender_id in DCI request: %s", sender_id)
_logger.warning("Unknown sender_id (inactive or unregistered) in DCI request: %s", sender_id)
raise DCIHTTPException(
status_code=status.HTTP_401_UNAUTHORIZED,
error_message=f"Unknown sender_id: {sender_id}",
Expand Down
17 changes: 17 additions & 0 deletions spp_dci_server/models/transaction.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,23 @@ def process_async_search(self):
self.ensure_one()
self.state = "processing"

# Fail closed: a search with no resolved (or no longer active) sender
# would disengage consent filtering (DCIConsentAdapter has no sender)
# and return unscoped PII. A Many2one still dereferences a deactivated
# record, and this job runs asynchronously, so the sender may have been
# deactivated after the transaction was created -- check active too.
# Every caller that reaches this sink (sync search is guarded at the
# route; async/bulk reach here) must have an active sender resolved.
if not self.sender_id or not self.sender_id.active:
self.state = "rejected"
self.error_code = SearchStatusReasonCode.SEARCH_CRITERIA_INVALID.value
self.error_message = "No active registered sender resolved for this transaction; search refused."
_logger.warning(
"DCI async search refused: transaction %s has no resolved sender",
self.transaction_id,
)
return

try:
# Parse request
request_data = json.loads(self.request_payload)
Expand Down
21 changes: 16 additions & 5 deletions spp_dci_server/routers/async_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,10 +146,21 @@ async def async_search(
) from e

# SECURITY: Use verified_sender_id from signature verification
# instead of reading from envelope.header to prevent spoofing
# Use sudo() for API access - authentication is handled by signature verification
# instead of reading from envelope.header to prevent spoofing.
# Fail closed: require an ACTIVE registered sender. Persisting
# sender_id=False here would later run the search with no sender, which
# disengages consent filtering and returns unscoped PII.
# nosemgrep: odoo-sudo-without-context — DCI protocol handler with JWT/signature verification
sender = env["spp.dci.sender.registry"].sudo().search([("sender_id", "=", verified_sender_id)], limit=1)
sender = env["spp.dci.sender.registry"].sudo().get_by_sender_id(verified_sender_id)
if not sender:
_logger.warning(
"Async search rejected: sender %s is not an active registered sender",
verified_sender_id,
)
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
detail="Sender is not an active registered DCI sender",
)

# Get callback URI from header
callback_uri = envelope.header.sender_uri
Expand All @@ -171,7 +182,7 @@ async def async_search(
"correlation_id": correlation_id,
"action": "search",
"reg_type": "SOCIAL_REGISTRY", # Default, could be derived from request
"sender_id": sender.id if sender else False,
"sender_id": sender.id,
"sender_uri": verified_sender_id,
"callback_uri": callback_uri,
"request_payload": json.dumps(envelope.model_dump(mode="json")),
Expand All @@ -183,7 +194,7 @@ async def async_search(
_logger.info(
"Created async search transaction %s for sender_id=%s",
transaction.transaction_id,
sender.id if sender else "unknown",
sender.id,
)

# Queue the search job with job_worker
Expand Down
26 changes: 19 additions & 7 deletions spp_dci_server/routers/search.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,19 +99,31 @@ async def search_registry(
len(search_request.search_request),
)

# Resolve the authenticated sender to an ACTIVE registered sender record
# before returning any data. The consent adapter disengages (no filtering)
# when sender is None, so we must fail closed here rather than run a
# consent-bearing search with sender=None and leak unscoped PII.
# sudo: technical lookup of an already-verified sender id; the endpoint
# user (often public) has no read access to the registry.
SenderRegistry = env["spp.dci.sender.registry"].sudo() # nosemgrep: odoo-sudo-without-context
sender_registry = SenderRegistry.get_by_sender_id(verified_sender_id)
if not sender_registry:
_logger.warning(
"DCI search rejected: sender %s is not an active registered sender",
verified_sender_id,
)
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
detail="Sender is not an active registered DCI sender",
)

# Execute search using DCISocialSearchService
try:
from odoo.addons.spp_dci_server_social.services.search_service import (
DCISocialSearchService,
)

# Hand the verified sender to the service so consent filtering
# engages - the consent adapter disengages when sender is None.
# sudo: technical lookup of an already-verified sender id; the
# endpoint user (often public) has no read access to the registry.
SenderRegistry = env["spp.dci.sender.registry"].sudo() # nosemgrep: odoo-sudo-without-context
sender_registry = SenderRegistry.get_by_sender_id(verified_sender_id)
search_service = DCISocialSearchService(env, sender_registry=sender_registry or None)
search_service = DCISocialSearchService(env, sender_registry=sender_registry)
search_response = search_service.execute_search(search_request)
_logger.info(
"DCI search completed - transaction_id: %s, items: %d",
Expand Down
58 changes: 58 additions & 0 deletions spp_dci_server/tests/test_async_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,64 @@ def test_unexpected_error_returns_500(self):
self._call(envelope)
self.assertEqual(ctx.exception.status_code, 500)

def test_unresolved_sender_rejected(self):
"""A verified sender that doesn't resolve to an active registry record
must be rejected (403), not queued with sender_id=False (which would run
the background search unscoped and leak unscoped PII to the callback)."""
envelope = self._build_envelope()
with self.assertRaises(HTTPException) as ctx:
_run(
self.async_router.async_search(
envelope,
self.env,
_bearer_token="t",
verified_sender_id="ghost.sender.unregistered",
_rate_limit_check=None,
response=Response(),
)
)
self.assertEqual(ctx.exception.status_code, 403)

def test_process_async_search_without_sender_is_refused(self):
"""The shared async sink must refuse a transaction with no resolved
sender rather than run a consent-disengaged (unscoped) search. This
protects every caller of the sink (async + bulk)."""
txn = self.Transaction.sudo().create(
{
"transaction_id": f"txn-nosender-{uuid.uuid4()}",
"message_id": f"msg-{uuid.uuid4()}",
"correlation_id": str(uuid.uuid4()),
"action": "search",
"reg_type": "SOCIAL_REGISTRY",
"sender_id": False,
"request_payload": json.dumps({"message": {"transaction_id": "t", "search_request": []}}),
"state": "received",
}
)
txn.process_async_search()
self.assertEqual(txn.state, "rejected")
self.assertIn("sender", (txn.error_message or "").lower())

def test_process_async_search_with_inactive_sender_is_refused(self):
"""A sender deactivated after the transaction was created must also be
refused: a Many2one still dereferences the deactivated record, so a
plain truthiness check would let the unscoped search through."""
txn = self.Transaction.sudo().create(
{
"transaction_id": f"txn-inactive-{uuid.uuid4()}",
"message_id": f"msg-{uuid.uuid4()}",
"correlation_id": str(uuid.uuid4()),
"action": "search",
"reg_type": "SOCIAL_REGISTRY",
"sender_id": self.test_sender.id,
"request_payload": json.dumps({"message": {"transaction_id": "t", "search_request": []}}),
"state": "received",
}
)
self.test_sender.active = False
txn.process_async_search()
self.assertEqual(txn.state, "rejected")


# =============================================================================
# /subscribe
Expand Down
47 changes: 42 additions & 5 deletions spp_dci_server/tests/test_search_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,10 @@ def _build_response(self, statuses=("succ",)):
search_response=items,
)

def _call(self, envelope, search_response, sender_id="external.test.gov"):
def _call(self, envelope, search_response, sender_id=None):
# Default to the active test sender so the route's fail-closed sender
# resolution passes; tests probing rejection pass an explicit bad id.
sender_id = sender_id or self.test_sender.sender_id
with patch(
"odoo.addons.spp_dci_server_social.services.search_service.DCISocialSearchService"
) as mock_service_cls:
Expand Down Expand Up @@ -212,6 +215,40 @@ def test_sync_search_sender_lookup_survives_low_privilege_endpoint_user(self):
)
self.assertEqual(result.header.status, "succ")

def test_unresolved_sender_is_rejected(self):
"""A verified sender_id that does not resolve to an active registry
record must be rejected (403), never run with sender=None (which would
disengage consent filtering and return unscoped PII)."""
envelope = self._build_envelope()
with self.assertRaises(HTTPException) as ctx:
_run(
self.search_registry(
envelope,
self.env,
_bearer_token="t",
verified_sender_id="ghost-sender-not-registered",
_rate_limit_check=None,
)
)
self.assertEqual(ctx.exception.status_code, 403)

def test_inactive_sender_is_rejected(self):
"""A deactivated sender (which can still pass signature lookup if that
lookup omits the active filter) must not get data via the search path."""
self.test_sender.active = False
envelope = self._build_envelope()
with self.assertRaises(HTTPException) as ctx:
_run(
self.search_registry(
envelope,
self.env,
_bearer_token="t",
verified_sender_id=self.test_sender.sender_id,
_rate_limit_check=None,
)
)
self.assertEqual(ctx.exception.status_code, 403)

# --- service errors -------------------------------------------------------

def test_search_service_exception_rejects_all_items(self):
Expand All @@ -225,7 +262,7 @@ def test_search_service_exception_rejects_all_items(self):
envelope,
self.env,
_bearer_token="t",
verified_sender_id="s",
verified_sender_id=self.test_sender.sender_id,
_rate_limit_check=None,
)
)
Expand All @@ -251,7 +288,7 @@ def test_social_module_import_error_falls_back_to_rjct(self):
envelope,
self.env,
_bearer_token="t",
verified_sender_id="s",
verified_sender_id=self.test_sender.sender_id,
_rate_limit_check=None,
)
)
Expand Down Expand Up @@ -291,7 +328,7 @@ def test_signing_failure_is_tolerated(self):
envelope,
self.env,
_bearer_token="t",
verified_sender_id="s",
verified_sender_id=self.test_sender.sender_id,
_rate_limit_check=None,
)
)
Expand Down Expand Up @@ -320,7 +357,7 @@ def test_unexpected_error_returns_500(self):
envelope,
self.env,
_bearer_token="t",
verified_sender_id="s",
verified_sender_id=self.test_sender.sender_id,
_rate_limit_check=None,
)
)
Expand Down
17 changes: 17 additions & 0 deletions spp_dci_server/tests/test_signature_middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,23 @@ def test_verify_missing_sender(self):
self.assertEqual(exc.status_code, 401, "Should return 401 Unauthorized")
self.assertIn("unknown sender", exc.detail.lower())

def test_verify_inactive_sender_rejected(self):
"""A deactivated sender must not authenticate, even with a valid
signature (the registry lookup requires active=True)."""
self.test_sender.active = False
envelope_data = self.create_signed_envelope(
sender_id=self.test_sender_id,
receiver_id="registry.test.gov",
action="search",
)

import asyncio

with self.assertRaises(HTTPException) as context:
asyncio.run(self._verify_signature(envelope_data))

self.assertEqual(context.exception.status_code, 401, "Inactive sender should be rejected")

def test_verify_invalid_signature(self):
"""Test that bad signature returns 401 Unauthorized."""
# Create a properly signed envelope
Expand Down
14 changes: 13 additions & 1 deletion spp_dci_server_social/services/search_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -218,8 +218,20 @@ def _process_search_item(self, search_req: SearchRequestItem) -> SearchResponseI
# Apply consent filtering if available
domain = self._apply_consent_filter(domain)

# Get pagination parameters
# Get pagination parameters. Cap page_size so a single request cannot
# pull/enumerate the whole registry (the schema only enforces > 0).
page_size = criteria.pagination.page_size if criteria.pagination else 20
# nosemgrep: odoo-sudo-without-context
config = self.env["ir.config_parameter"].sudo()
try:
max_page_size = int(config.get_param("dci.max_page_size", 100))
except (TypeError, ValueError):
max_page_size = 100
# A non-positive value is a misconfiguration, not "no limit": fall back
# to the default so the cap can never be silently disabled.
if max_page_size <= 0:
max_page_size = 100
page_size = min(page_size, max_page_size)
page_number = criteria.pagination.page_number if criteria.pagination else 1

# OpenSPP extension: cursor-based pagination
Expand Down
57 changes: 57 additions & 0 deletions spp_dci_server_social/tests/test_search_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,63 @@ def test_search_pagination(self):
self.assertLessEqual(len(response_item.data.reg_records), 2)
self.assertGreaterEqual(response_item.pagination.total_count, 3) # At least our 3 individuals

def test_search_caps_page_size(self):
"""page_size is clamped to dci.max_page_size so a single request cannot
enumerate the whole registry (the schema only enforces page_size > 0)."""
self.env["ir.config_parameter"].sudo().set_param("dci.max_page_size", "1")
criteria = SearchCriteria(
reg_type="SOCIAL_REGISTRY",
reg_event_type="ACTIVE",
query_type="expression",
query={"seq": []}, # match all registrants
pagination=PaginationRequest(page_size=1000, page_number=1),
)
search_req = SearchRequestItem(
reference_id="test-ref-cap",
timestamp=datetime.now(UTC),
search_criteria=criteria,
)
request = SearchRequest(transaction_id="test-txn-cap", search_request=[search_req])
self.env.user.write({"group_ids": [(4, self.env.ref("spp_registry.group_registry_viewer").id)]})

response = self.search_service.execute_search(request)

item = response.search_response[0]
self.assertEqual(item.status, "succ")
# Returned page and reported page_size are clamped to the cap (1),
# even though there are >= 3 matching registrants.
self.assertLessEqual(len(item.data.reg_records), 1)
self.assertEqual(item.pagination.page_size, 1)
self.assertGreaterEqual(item.pagination.total_count, 3)

def test_search_non_positive_cap_falls_back_to_default(self):
"""A non-positive dci.max_page_size (0 or negative) must NOT disable the
cap. Such a misconfiguration should fall back to the default (100), not
leave page_size unbounded at whatever the client requested."""
self.env["ir.config_parameter"].sudo().set_param("dci.max_page_size", "0")
criteria = SearchCriteria(
reg_type="SOCIAL_REGISTRY",
reg_event_type="ACTIVE",
query_type="expression",
query={"seq": []}, # match all registrants
pagination=PaginationRequest(page_size=1000, page_number=1),
)
search_req = SearchRequestItem(
reference_id="test-ref-cap0",
timestamp=datetime.now(UTC),
search_criteria=criteria,
)
request = SearchRequest(transaction_id="test-txn-cap0", search_request=[search_req])
self.env.user.write({"group_ids": [(4, self.env.ref("spp_registry.group_registry_viewer").id)]})

response = self.search_service.execute_search(request)

item = response.search_response[0]
self.assertEqual(item.status, "succ")
# The client asked for 1000; a 0 cap must clamp to the default 100,
# never honor the unbounded request.
self.assertEqual(item.pagination.page_size, 100)

def test_search_pagination_second_page(self):
"""Test retrieving second page of results."""
criteria = SearchCriteria(
Expand Down
Loading