From 0944b7ff837a0cd75731dc4fb3ff723699fa5cec Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 4 Jun 2026 17:50:48 +0800 Subject: [PATCH 01/22] fix(spp_dci_server_social): SR server correctness + safety (Phase A) Prepares the social module to serve as a standalone SR DCI server: - accept the namespaced reg_type DCI clients send (ns:org:RegistryType:Social) alongside the legacy bare form; previously every client search was rejected - idtype-value queries match the identifier type by namespace URI or vocabulary code (clients resolve identifiers as short codes); malformed queries missing type/value now reject with FILTER_INVALID - sync search route resolves the verified sender and passes it to the search service so consent filtering engages; it silently disengaged because the consent adapter no-ops without a sender - remove unmounted, unauthenticated routers (social_search, sr_alias) and their tests; the only search surface is spp_dci_server's authenticated route - delete notifications carry external identifiers snapshotted before unlink, never raw database ids; legacy queued jobs degrade to empty identifier lists - document deployment prerequisites (endpoint user group, queue_job worker/channels, auth surface, client base URL) Suites: spp_dci_server 306, spp_dci_server_social 64 (63 +5 new -4 removed with the dead routers). --- spp_dci_server/routers/search.py | 5 +- spp_dci_server/tests/test_search_router.py | 29 +++++++ spp_dci_server_social/README.rst | 20 +++++ spp_dci_server_social/__init__.py | 1 - .../models/res_partner_dci_notify.py | 52 +++++++++--- spp_dci_server_social/readme/DESCRIPTION.md | 16 ++++ spp_dci_server_social/routers/__init__.py | 4 - .../routers/social_search.py | 85 ------------------- spp_dci_server_social/routers/sr_alias.py | 53 ------------ .../services/search_service.py | 28 ++++-- .../static/description/index.html | 21 +++++ spp_dci_server_social/tests/__init__.py | 1 - .../tests/test_dci_notifications.py | 51 ++++++++++- .../tests/test_search_service.py | 79 +++++++++++++++++ .../tests/test_social_routers.py | 65 -------------- 15 files changed, 278 insertions(+), 232 deletions(-) delete mode 100644 spp_dci_server_social/routers/__init__.py delete mode 100644 spp_dci_server_social/routers/social_search.py delete mode 100644 spp_dci_server_social/routers/sr_alias.py delete mode 100644 spp_dci_server_social/tests/test_social_routers.py diff --git a/spp_dci_server/routers/search.py b/spp_dci_server/routers/search.py index 9081efda..1d9599b5 100644 --- a/spp_dci_server/routers/search.py +++ b/spp_dci_server/routers/search.py @@ -105,7 +105,10 @@ async def search_registry( DCISocialSearchService, ) - search_service = DCISocialSearchService(env) + # Hand the verified sender to the service so consent filtering + # engages - the consent adapter disengages when sender is None. + sender_registry = env["spp.dci.sender.registry"].get_by_sender_id(verified_sender_id) + search_service = DCISocialSearchService(env, sender_registry=sender_registry or None) search_response = search_service.execute_search(search_request) _logger.info( "DCI search completed - transaction_id: %s, items: %d", diff --git a/spp_dci_server/tests/test_search_router.py b/spp_dci_server/tests/test_search_router.py index 60a0f4ce..0d071160 100644 --- a/spp_dci_server/tests/test_search_router.py +++ b/spp_dci_server/tests/test_search_router.py @@ -156,6 +156,35 @@ def test_invalid_search_request_returns_400(self): ) self.assertEqual(ctx.exception.status_code, 400) + # --- consent wiring ------------------------------------------------------- + + def test_sync_search_passes_verified_sender_to_service(self): + """The sync path must resolve the verified sender registry entry and + hand it to the search service - otherwise the consent adapter sees no + sender and silently disengages consent filtering.""" + envelope = self._build_envelope() + response = self._build_response(statuses=("succ",)) + with patch("odoo.addons.spp_dci_server_social.services.search_service.DCISocialSearchService") as mock_cls: + mock_cls.return_value.execute_search.return_value = response + _run( + self.search_registry( + envelope, + self.env, + _bearer_token="t", + verified_sender_id=self.test_sender.sender_id, + _rate_limit_check=None, + ) + ) + args, kwargs = mock_cls.call_args + passed_sender = kwargs.get("sender_registry") + if passed_sender is None and len(args) > 1: + passed_sender = args[1] + self.assertEqual( + passed_sender, + self.test_sender, + "verified sender was not passed to the search service (consent bypass)", + ) + # --- service errors ------------------------------------------------------- def test_search_service_exception_rejects_all_items(self): diff --git a/spp_dci_server_social/README.rst b/spp_dci_server_social/README.rst index b969e8a8..bbe9fae4 100644 --- a/spp_dci_server_social/README.rst +++ b/spp_dci_server_social/README.rst @@ -98,6 +98,26 @@ After installing: 3. Ensure the DCI queue_job cron is active under **Settings > Technical > Scheduled Actions** +Deployment Prerequisites (read before exposing the server) +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +- **Endpoint user groups**: the Odoo user configured on the DCI + ``fastapi.endpoint`` must belong to + ``spp_registry.group_registry_viewer`` - every search fails with an + access error otherwise. +- **queue_job worker**: event notifications are delivered through + delayed jobs on channels ``root.dci`` and ``dci``. A running queue_job + worker with those channels configured is a hard requirement - without + it, notifications are enqueued and silently never sent. +- **Inbound auth**: searches go through ``spp_dci_server``'s + authenticated route (bearer token per ``dci.api_tokens`` plus DCI + envelope signature). This module deliberately ships no unauthenticated + routes. +- **Client base URL**: DCI clients post to + ``{base_url}/registry/sync/search``; point their data source base URL + at ``.../api/v1/social`` so requests land on this server's + ``/social/registry/sync/search`` mount. + UI Location ~~~~~~~~~~~ diff --git a/spp_dci_server_social/__init__.py b/spp_dci_server_social/__init__.py index d2f52960..896e8044 100644 --- a/spp_dci_server_social/__init__.py +++ b/spp_dci_server_social/__init__.py @@ -1,5 +1,4 @@ # Part of OpenSPP. See LICENSE file for full copyright and licensing details. from . import models -from . import routers from . import services diff --git a/spp_dci_server_social/models/res_partner_dci_notify.py b/spp_dci_server_social/models/res_partner_dci_notify.py index 455a42e8..f250d262 100644 --- a/spp_dci_server_social/models/res_partner_dci_notify.py +++ b/spp_dci_server_social/models/res_partner_dci_notify.py @@ -80,18 +80,42 @@ def write(self, vals): def unlink(self): """Override unlink to trigger DCI delete notifications.""" - # Capture registrant IDs before deletion - registrant_ids = self.filtered(lambda r: r.is_registrant).ids + # Snapshot identifiers before deletion - the records are gone by the + # time the notification job runs, and subscribers must receive + # external identifiers, never raw database ids. + registrants = self.filtered(lambda r: r.is_registrant) + registrant_ids = registrants.ids + delete_payloads = registrants._dci_delete_payloads() result = super().unlink() - # Queue delete notification (IDs only since records are gone) if registrant_ids: - self._schedule_dci_notification("delete", registrant_ids) + self._schedule_dci_notification("delete", registrant_ids, payloads=delete_payloads) return result - def _schedule_dci_notification(self, event_type, partner_ids): + def _dci_delete_payloads(self): + """Snapshot external identifiers for delete notifications. + + Returns one payload dict per registrant in ``self`` containing the + registrant's external identifiers (namespace URI preferred, falling + back to the vocabulary code). Raw database ids are deliberately not + included (api-design principle: never expose DB IDs). + """ + payloads = [] + for partner in self: + identifiers = [ + { + "identifier_type": reg_id.id_type_id.namespace_uri or reg_id.id_type_id.code, + "identifier_value": reg_id.value, + } + for reg_id in partner.reg_ids + if reg_id.value + ] + payloads.append({"identifiers": identifiers}) + return payloads + + def _schedule_dci_notification(self, event_type, partner_ids, payloads=None): """Schedule DCI notification via post-commit hook. Uses post-commit to ensure notification only fires after @@ -112,7 +136,7 @@ def _schedule_dci_notification(self, event_type, partner_ids): # Use post-commit hook to defer notification until transaction commits # This ensures we don't notify about rolled-back changes def notify_on_commit(): - self._queue_dci_notification_job(event_type, partner_ids) + self._queue_dci_notification_job(event_type, partner_ids, payloads=payloads) # Register post-commit callback self.env.cr.postcommit.add(notify_on_commit) @@ -134,7 +158,7 @@ def _dci_notifications_enabled(self): enabled = config.get_param("dci.notifications_enabled", "true").lower() == "true" return enabled - def _queue_dci_notification_job(self, event_type, partner_ids): + def _queue_dci_notification_job(self, event_type, partner_ids, payloads=None): """Queue the actual notification job with deduplication. Uses queue_job with identity_key to deduplicate multiple @@ -157,7 +181,7 @@ def _queue_dci_notification_job(self, event_type, partner_ids): channel="root.dci", description=f"DCI {event_type} notification ({len(partner_ids)} records)", identity_key=identity_key, - )._execute_dci_notification(event_type, partner_ids) + )._execute_dci_notification(event_type, partner_ids, payloads=payloads) _logger.debug( "Queued DCI %s notification job for partner IDs: %s", @@ -186,7 +210,7 @@ def _get_notification_identity_key(self, event_type, partner_ids): # Use hash to keep key length manageable return hashlib.sha256(key_data.encode()).hexdigest()[:32] - def _execute_dci_notification(self, event_type, partner_ids): + def _execute_dci_notification(self, event_type, partner_ids, payloads=None): """Execute the DCI notification (called by queue_job). Builds notification payload and calls subscription.notify_event(). @@ -194,6 +218,8 @@ def _execute_dci_notification(self, event_type, partner_ids): Args: event_type: One of 'registration', 'update', 'delete' partner_ids: List of partner IDs to notify about + payloads: For delete events, identifier payloads snapshotted + before the records were removed """ if not partner_ids: return @@ -213,10 +239,12 @@ def _execute_dci_notification(self, event_type, partner_ids): return Subscription = self.env["spp.dci.subscription"] - # For delete events, we only have IDs (records are gone) + # For delete events the records are gone - use the identifier + # payloads snapshotted in unlink(). Jobs queued before this field + # existed carry no payloads; emit empty identifier lists rather + # than leaking raw database ids. if event_type == "delete": - # Build minimal records with just identifiers - records = [{"id": pid} for pid in partner_ids] + records = payloads if payloads is not None else [{"identifiers": []} for _ in partner_ids] Subscription.notify_event(event_type, records, "SOCIAL_REGISTRY") return diff --git a/spp_dci_server_social/readme/DESCRIPTION.md b/spp_dci_server_social/readme/DESCRIPTION.md index 9d96e21f..fb515d1e 100644 --- a/spp_dci_server_social/readme/DESCRIPTION.md +++ b/spp_dci_server_social/readme/DESCRIPTION.md @@ -42,6 +42,22 @@ After installing: 2. Configure sender registries in **Social Protection > DCI > Sender Registries** 3. Ensure the DCI queue_job cron is active under **Settings > Technical > Scheduled Actions** +### Deployment Prerequisites (read before exposing the server) + +- **Endpoint user groups**: the Odoo user configured on the DCI + `fastapi.endpoint` must belong to `spp_registry.group_registry_viewer` - + every search fails with an access error otherwise. +- **queue_job worker**: event notifications are delivered through delayed + jobs on channels `root.dci` and `dci`. A running queue_job worker with + those channels configured is a hard requirement - without it, + notifications are enqueued and silently never sent. +- **Inbound auth**: searches go through `spp_dci_server`'s authenticated + route (bearer token per `dci.api_tokens` plus DCI envelope signature). + This module deliberately ships no unauthenticated routes. +- **Client base URL**: DCI clients post to `{base_url}/registry/sync/search`; + point their data source base URL at `.../api/v1/social` so requests land + on this server's `/social/registry/sync/search` mount. + ### UI Location No standalone UI. Search functionality accessed via DCI API endpoints. Notifications triggered automatically on registrant changes. diff --git a/spp_dci_server_social/routers/__init__.py b/spp_dci_server_social/routers/__init__.py deleted file mode 100644 index 9f69202d..00000000 --- a/spp_dci_server_social/routers/__init__.py +++ /dev/null @@ -1,4 +0,0 @@ -# Part of OpenSPP. See LICENSE file for full copyright and licensing details. - -from . import social_search -from . import sr_alias diff --git a/spp_dci_server_social/routers/social_search.py b/spp_dci_server_social/routers/social_search.py deleted file mode 100644 index 0ac9c0ee..00000000 --- a/spp_dci_server_social/routers/social_search.py +++ /dev/null @@ -1,85 +0,0 @@ -# Part of OpenSPP. See LICENSE file for full copyright and licensing details. -"""DCI Social Registry search endpoints""" - -import logging -from datetime import datetime -from typing import Annotated - -from odoo.api import Environment - -from odoo.addons.fastapi.dependencies import odoo_env -from odoo.addons.spp_dci.schemas.search import SearchRequest, SearchResponse - -from fastapi import APIRouter, Depends, HTTPException, status - -_logger = logging.getLogger(__name__) - -social_search_router = APIRouter(tags=["DCI Social Registry"], prefix="/registry/social") - - -@social_search_router.post( - "/sync/search", - response_model=SearchResponse, - response_model_exclude_none=True, -) -async def sync_search( - request: SearchRequest, - env: Annotated[Environment, Depends(odoo_env)], -): - """ - Synchronous search for Social Registry persons/groups. - - Implements DCI sync search pattern - returns immediate results. - - Request body contains: - - transaction_id: Unique transaction identifier - - search_request: List of search request items with criteria - - Each search criteria supports: - - query_type: "idtype-value" or "expression" - - reg_type: Should be "SOCIAL_REGISTRY" - - pagination: page_size, page_number - - sort: Optional sort specifications - - Returns SearchResponse with matching Person/Group records. - """ - from ..services.search_service import DCISocialSearchService - - service = DCISocialSearchService(env) - - try: - response = service.execute_search(request) - return response - except Exception as e: - _logger.exception("Error executing DCI social registry search: %s", str(e)) - raise HTTPException( - status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, - detail=f"Search failed: {str(e)}", - ) from e - - -@social_search_router.post( - "/sync/notify", - status_code=status.HTTP_501_NOT_IMPLEMENTED, -) -async def sync_notify( - env: Annotated[Environment, Depends(odoo_env)], -): - """ - Receive notification from external DCI client. - - This endpoint is not yet implemented. In a full implementation, this would: - 1. Validate the notification signature - 2. Parse the notification payload - 3. Process the event (create/update/delete records) - 4. Return acknowledgment - - Returns 501 Not Implemented with DCI rejection response. - """ - _logger.info("DCI Social Registry notification endpoint called but not implemented") - return { - "status": "rjct", - "status_reason_code": "rjct.search_criteria.invalid", - "status_reason_message": "Notification endpoint is not yet implemented", - "timestamp": datetime.utcnow().isoformat(), - } diff --git a/spp_dci_server_social/routers/sr_alias.py b/spp_dci_server_social/routers/sr_alias.py deleted file mode 100644 index bbcfed77..00000000 --- a/spp_dci_server_social/routers/sr_alias.py +++ /dev/null @@ -1,53 +0,0 @@ -# Part of OpenSPP. See LICENSE file for full copyright and licensing details. -"""SPDCI-compliant short-form Social Registry endpoint aliases (/sr/*). - -This module provides thin redirects from SPDCI short-form paths to our -main implementation paths. No business logic duplication. - -Endpoint redirects: -- /sr/sync/search -> uses social_search.sync_search -- /sr/search -> uses async_router.async_search -- /sr/subscribe -> uses async_router.subscribe -- /sr/unsubscribe -> uses async_router.unsubscribe -- /sr/txn/status -> uses async_router.txn_status -- /sr/sync/txn/status -> uses async_router.txn_status (sync mode) -""" - -import logging -from typing import Annotated - -from odoo.api import Environment - -from odoo.addons.fastapi.dependencies import odoo_env -from odoo.addons.spp_dci.schemas import ( - SearchRequest, - SearchResponse, -) - -from fastapi import APIRouter, Depends - -_logger = logging.getLogger(__name__) - -# Social Registry short-form router with /sr prefix (SPDCI compliance) -sr_alias_router = APIRouter(tags=["SPDCI Social Registry Aliases"], prefix="/sr") - - -@sr_alias_router.post( - "/sync/search", - response_model=SearchResponse, - response_model_exclude_none=True, -) -async def sr_sync_search( - request: SearchRequest, - env: Annotated[Environment, Depends(odoo_env)], -): - """Alias for /registry/social/sync/search.""" - from .social_search import sync_search - - return await sync_search(request, env) - - -# Note: For async endpoints (/sr/search, /sr/subscribe, etc.), these are -# handled by the base spp_dci_server async_router which already provides -# the correct paths. If you need /sr/* aliases for those, add them here -# by importing and calling the corresponding functions from async_router. diff --git a/spp_dci_server_social/services/search_service.py b/spp_dci_server_social/services/search_service.py index ef76fc90..4ebbf746 100644 --- a/spp_dci_server_social/services/search_service.py +++ b/spp_dci_server_social/services/search_service.py @@ -26,6 +26,14 @@ _logger = logging.getLogger(__name__) +# Accepted spellings of the Social Registry type (compared lowercase): the +# SPDCI namespaced value sent by DCI clients, plus legacy bare forms. +_ACCEPTED_SOCIAL_REG_TYPES = { + "ns:org:registrytype:social", + "social_registry", + "social", +} + class DCISocialSearchService: """Service for DCI Social Registry search operations. @@ -138,11 +146,11 @@ def _process_search_item(self, search_req: SearchRequestItem) -> SearchResponseI """ criteria = search_req.search_criteria - # Default reg_type to SOCIAL_REGISTRY if not provided (optional per SPDCI spec) - reg_type = criteria.reg_type or "SOCIAL_REGISTRY" - - # Validate registry type - if reg_type != "SOCIAL_REGISTRY": + # reg_type is optional per the SPDCI spec (defaults to social); + # accept both the namespaced spelling DCI clients send + # (ns:org:RegistryType:Social) and the legacy bare form. + reg_type = criteria.reg_type + if reg_type and str(reg_type).strip().lower() not in _ACCEPTED_SOCIAL_REG_TYPES: return SearchResponseItem( reference_id=search_req.reference_id, timestamp=datetime.now(UTC), @@ -289,11 +297,17 @@ def _build_domain(self, criteria) -> list: id_type = query.type id_value = query.value - # Match by identifier type and value - # Note: Assuming id_type is a namespace URI + if not id_type or not id_value: + raise ValueError("idtype-value query requires both 'type' and 'value'") + + # Match the identifier type by namespace URI or by the short + # vocabulary code - DCI clients resolve identifiers from their + # local registrant IDs as short codes (UIN, NATIONAL_ID, ...). domain.extend( [ + "|", ("reg_ids.id_type_id.namespace_uri", "=", id_type), + ("reg_ids.id_type_id.code", "=", id_type), ("reg_ids.value", "=", id_value), ] ) diff --git a/spp_dci_server_social/static/description/index.html b/spp_dci_server_social/static/description/index.html index 4ffc9667..ec18c75b 100644 --- a/spp_dci_server_social/static/description/index.html +++ b/spp_dci_server_social/static/description/index.html @@ -456,6 +456,27 @@

Configuration

> Scheduled Actions +
+

Deployment Prerequisites (read before exposing the server)

+ +

UI Location

No standalone UI. Search functionality accessed via DCI API endpoints. diff --git a/spp_dci_server_social/tests/__init__.py b/spp_dci_server_social/tests/__init__.py index ce9eaa98..28df45b1 100644 --- a/spp_dci_server_social/tests/__init__.py +++ b/spp_dci_server_social/tests/__init__.py @@ -3,5 +3,4 @@ from . import common from . import test_search_service from . import test_dci_notifications -from . import test_social_routers from . import test_search_service_internals diff --git a/spp_dci_server_social/tests/test_dci_notifications.py b/spp_dci_server_social/tests/test_dci_notifications.py index f65519af..2507a58e 100644 --- a/spp_dci_server_social/tests/test_dci_notifications.py +++ b/spp_dci_server_social/tests/test_dci_notifications.py @@ -131,6 +131,50 @@ def test_unlink_triggers_delete_notification(self): self.assertEqual(args[0], "delete") self.assertIn(individual_id, args[1]) + def test_delete_notification_payload_has_no_db_ids(self): + """Delete notifications must carry external identifiers, never raw + Odoo database ids (api-design principle: never expose DB IDs).""" + individual = self._create_test_individual( + { + "family_name": "DeleteLeak", + "given_name": "NoDbId", + }, + ) + self.env.cr.postcommit.clear() + + Subscription = type(self.env["spp.dci.subscription"]) + Partner = self.Partner.__class__ + with ( + patch.object(Subscription, "notify_event") as mock_notify, + patch.object(Partner, "with_delay", lambda records, **kw: records), + ): + individual.unlink() + self.env.cr.postcommit.run() + + mock_notify.assert_called_once() + event_type, records, reg_type = mock_notify.call_args[0] + self.assertEqual(event_type, "delete") + for record in records: + self.assertNotIn("id", record, f"delete payload leaks raw DB id: {record}") + self.assertIn("identifiers", record) + + def test_delete_payload_snapshots_identifiers(self): + """The identifier snapshot helper captures external identifiers from a + live registrant (before unlink), keyed the way subscribers expect.""" + individual = self._create_test_individual( + { + "family_name": "DeleteSnap", + "given_name": "WithIds", + }, + identifier_value="DEL-SNAP-001", + ) + payloads = individual._dci_delete_payloads() + self.assertEqual(len(payloads), 1) + identifiers = payloads[0]["identifiers"] + self.assertTrue(identifiers, "no identifiers captured from reg_ids") + self.assertIn("DEL-SNAP-001", [i["identifier_value"] for i in identifiers]) + self.assertNotIn("id", payloads[0]) + def test_non_registrant_no_notification(self): """Test that changes to non-registrants don't trigger notifications.""" # Create a non-registrant partner @@ -197,17 +241,18 @@ def test_execute_notification_calls_subscription(self): self.assertEqual(args[2], "SOCIAL_REGISTRY") # reg_type def test_execute_notification_delete_with_ids_only(self): - """Test that delete notifications work with just IDs (no records).""" + """A legacy delete job queued without identifier payloads must still + notify - with an empty identifier list, never the raw DB id.""" with patch.object(self.env["spp.dci.subscription"].__class__, "notify_event") as mock_notify: partner = self.Partner.browse(self.individual_1.id) - # Simulate delete notification where records no longer exist + # Simulate a legacy queued job (no payloads argument serialized) partner._execute_dci_notification("delete", [99999]) # Non-existent ID # Verify notify_event was still called mock_notify.assert_called_once() args = mock_notify.call_args[0] self.assertEqual(args[0], "delete") - self.assertEqual(args[1], [{"id": 99999}]) + self.assertEqual(args[1], [{"identifiers": []}]) def test_multiple_writes_same_transaction(self): """Test that multiple writes in same transaction are handled.""" diff --git a/spp_dci_server_social/tests/test_search_service.py b/spp_dci_server_social/tests/test_search_service.py index a304234d..7462eb15 100644 --- a/spp_dci_server_social/tests/test_search_service.py +++ b/spp_dci_server_social/tests/test_search_service.py @@ -8,6 +8,7 @@ from odoo.exceptions import AccessError, ValidationError from odoo.tests import tagged +from odoo.addons.spp_dci.schemas.constants import SearchStatusReasonCode from odoo.addons.spp_dci.schemas.search import ( PaginationRequest, SearchCriteria, @@ -48,6 +49,84 @@ def test_set_sender(self): service.set_sender(self.test_sender) self.assertEqual(service.sender, self.test_sender) + def test_search_accepts_namespaced_reg_type(self): + """The DCI client sends the namespaced RegistryType value from its data + source (ns:org:RegistryType:Social) - the service must accept it, not + only the bare legacy form.""" + criteria = SearchCriteria( + reg_type="ns:org:RegistryType:Social", + query_type="idtype-value", + query={"type": self.test_id_type.namespace_uri, "value": "NAT-001"}, + ) + search_req = SearchRequestItem( + reference_id="test-ref-ns", + timestamp=datetime.now(UTC), + search_criteria=criteria, + ) + request = SearchRequest(transaction_id="test-txn-ns", 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) + + response_item = response.search_response[0] + self.assertEqual( + response_item.status, + "succ", + f"namespaced reg_type rejected: {response_item.status_reason_message}", + ) + + def test_search_by_identifier_short_code(self): + """idtype-value must also match the vocabulary code, not only the + namespace URI - DCI clients resolve identifiers from their local + registrant IDs as short codes (UIN, NATIONAL_ID, ...).""" + criteria = SearchCriteria( + reg_type="SOCIAL_REGISTRY", + query_type="idtype-value", + query={"type": self.test_id_type.code, "value": "NAT-001"}, + ) + search_req = SearchRequestItem( + reference_id="test-ref-code", + timestamp=datetime.now(UTC), + search_criteria=criteria, + ) + request = SearchRequest(transaction_id="test-txn-code", 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) + + response_item = response.search_response[0] + self.assertEqual(response_item.status, "succ") + self.assertEqual( + len(response_item.data.reg_records), + 1, + "short-code identifier type did not match any record", + ) + + def test_search_by_identifier_missing_value_rejected(self): + """A malformed idtype-value query (missing value) must be rejected + with FILTER_INVALID instead of silently matching nothing.""" + criteria = SearchCriteria( + reg_type="SOCIAL_REGISTRY", + query_type="idtype-value", + query={"type": self.test_id_type.namespace_uri}, + ) + search_req = SearchRequestItem( + reference_id="test-ref-noval", + timestamp=datetime.now(UTC), + search_criteria=criteria, + ) + request = SearchRequest(transaction_id="test-txn-noval", 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) + + response_item = response.search_response[0] + self.assertEqual(response_item.status, "rjct") + self.assertEqual( + response_item.status_reason_code, + SearchStatusReasonCode.FILTER_INVALID.value, + ) + def test_search_by_identifier_success(self): """Test searching by identifier type and value.""" # Create search request diff --git a/spp_dci_server_social/tests/test_social_routers.py b/spp_dci_server_social/tests/test_social_routers.py deleted file mode 100644 index 89c4aa80..00000000 --- a/spp_dci_server_social/tests/test_social_routers.py +++ /dev/null @@ -1,65 +0,0 @@ -# Part of OpenSPP. See LICENSE file for full copyright and licensing details. -"""Tests for the social-search and SR-alias routers.""" - -import asyncio -from unittest.mock import MagicMock, patch - -from odoo.tests import tagged - -from fastapi import HTTPException - -from .common import DCISocialServerCommon - - -def _run(coro): - loop = asyncio.new_event_loop() - try: - return loop.run_until_complete(coro) - finally: - loop.close() - - -SERVICE = "odoo.addons.spp_dci_server_social.services.search_service.DCISocialSearchService" - - -@tagged("post_install", "-at_install") -class TestSocialSearchRouter(DCISocialServerCommon): - def setUp(self): - super().setUp() - from odoo.addons.spp_dci_server_social.routers import social_search, sr_alias - - self.social_search = social_search - self.sr_alias = sr_alias - - def _request(self): - # The router only forwards the request object to the service, which - # we mock; a bare sentinel is sufficient. - return MagicMock(name="SearchRequest") - - def test_sync_search_returns_service_response(self): - sentinel = MagicMock(name="SearchResponse") - with patch(SERVICE) as svc: - svc.return_value.execute_search.return_value = sentinel - result = _run(self.social_search.sync_search(self._request(), self.env)) - self.assertIs(result, sentinel) - - def test_sync_search_wraps_errors_as_500(self): - with patch(SERVICE) as svc: - svc.return_value.execute_search.side_effect = RuntimeError("kaboom") - with self.assertRaises(HTTPException) as ctx: - _run(self.social_search.sync_search(self._request(), self.env)) - self.assertEqual(ctx.exception.status_code, 500) - self.assertIn("kaboom", ctx.exception.detail) - - def test_sync_notify_returns_not_implemented_payload(self): - result = _run(self.social_search.sync_notify(self.env)) - self.assertEqual(result["status"], "rjct") - self.assertIn("not yet implemented", result["status_reason_message"]) - self.assertIn("T", result["timestamp"]) - - def test_sr_alias_delegates_to_sync_search(self): - sentinel = MagicMock(name="SearchResponse") - with patch(SERVICE) as svc: - svc.return_value.execute_search.return_value = sentinel - result = _run(self.sr_alias.sr_sync_search(self._request(), self.env)) - self.assertIs(result, sentinel) From b75d43f69bb80f4be9430deee8074195866d13b8 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 4 Jun 2026 18:13:49 +0800 Subject: [PATCH 02/22] feat(spp_dci_server_social): serve programme enrollments in person records (Phase B) - spp_dci: add ProgramEnrollment schema (programme_name, enrolment_status, enrolment_date; SPDCI social profile naming, no database ids) and an optional enrolled_programs field on Person - spp_dci_server_social: _to_dci_person emits active enrollments (enrolled/paused states; draft/exited/not_eligible/duplicated excluded) from program_membership_ids - explicit spp_programs dependency - the service now reads program memberships directly, so the previously transitive dependency is real Unblocks sr.dci.program_count and sr.dci.has_programs: the SR client already reads enrolled_programs from person records. Suites: spp_dci 20, spp_dci_server 306, spp_dci_server_social 66. --- spp_dci/schemas/__init__.py | 3 +- spp_dci/schemas/person.py | 31 ++++++++++ spp_dci_server_social/__manifest__.py | 2 +- .../services/search_service.py | 16 ++++- .../tests/test_search_service.py | 59 +++++++++++++++++++ 5 files changed, 108 insertions(+), 3 deletions(-) diff --git a/spp_dci/schemas/__init__.py b/spp_dci/schemas/__init__.py index a535facf..158fd8ba 100644 --- a/spp_dci/schemas/__init__.py +++ b/spp_dci/schemas/__init__.py @@ -15,7 +15,7 @@ Place, AdditionalAttribute, ) -from .person import Person, RelatedPerson, DisabilityInfo +from .person import Person, ProgramEnrollment, RelatedPerson, DisabilityInfo from .group import Group, Member from .search import ( SearchCriteria, @@ -76,6 +76,7 @@ "AdditionalAttribute", # Person "Person", + "ProgramEnrollment", "RelatedPerson", "DisabilityInfo", # Group diff --git a/spp_dci/schemas/person.py b/spp_dci/schemas/person.py index d80acf08..6de98b50 100644 --- a/spp_dci/schemas/person.py +++ b/spp_dci/schemas/person.py @@ -26,6 +26,33 @@ class DisabilityInfo(BaseModel): ) +class ProgramEnrollment(BaseModel): + """A person's enrollment in a social protection programme. + + Field names follow the SPDCI social profile (programme_name, + enrolment_status, ...). No internal database ids are carried. + """ + + model_config = ConfigDict(populate_by_name=True) + + programme_identifier: str | None = Field( + None, + description="External programme identifier, when one exists", + ) + programme_name: str = Field( + ..., + description="Human-readable programme name", + ) + enrolment_status: str | None = Field( + None, + description="Enrollment status (enrolled, paused, ...)", + ) + enrolment_date: date | None = Field( + None, + description="Date the person was enrolled", + ) + + class RelatedPerson(BaseModel): """DCI RelatedPerson schema - family relationship information.""" @@ -96,6 +123,10 @@ class Person(BaseModel): None, description="Date when person details were last updated", ) + enrolled_programs: list[ProgramEnrollment] | None = Field( + None, + description="Active social protection programme enrollments", + ) # Update forward references diff --git a/spp_dci_server_social/__manifest__.py b/spp_dci_server_social/__manifest__.py index 62c60c03..8c80dc51 100644 --- a/spp_dci_server_social/__manifest__.py +++ b/spp_dci_server_social/__manifest__.py @@ -8,7 +8,7 @@ "website": "https://github.com/OpenSPP/OpenSPP2", "license": "LGPL-3", "development_status": "Alpha", - "depends": ["spp_dci_server", "spp_registry", "spp_cel_domain"], + "depends": ["spp_dci_server", "spp_registry", "spp_cel_domain", "spp_programs"], "data": [ "security/ir.model.access.csv", ], diff --git a/spp_dci_server_social/services/search_service.py b/spp_dci_server_social/services/search_service.py index 4ebbf746..a38c7cd3 100644 --- a/spp_dci_server_social/services/search_service.py +++ b/spp_dci_server_social/services/search_service.py @@ -13,7 +13,7 @@ from odoo.addons.spp_dci.schemas.common import Address, Identifier, Name from odoo.addons.spp_dci.schemas.constants import SearchStatusReasonCode from odoo.addons.spp_dci.schemas.group import Group, Member -from odoo.addons.spp_dci.schemas.person import Person +from odoo.addons.spp_dci.schemas.person import Person, ProgramEnrollment from odoo.addons.spp_dci.schemas.search import ( Pagination, SearchRequest, @@ -597,6 +597,19 @@ def _to_dci_person(self, partner) -> Person: if partner.email: emails.append(partner.email) + # Active programme enrollments (SPDCI social profile). Paused + # memberships are still enrollments; draft/exited/not_eligible/ + # duplicated are not. + enrollments = [ + ProgramEnrollment( + programme_name=membership.program_id.name, + enrolment_status=membership.state, + enrolment_date=membership.enrollment_date.date() if membership.enrollment_date else None, + ) + for membership in partner.program_membership_ids + if membership.state in ("enrolled", "paused") + ] + return Person( identifier=identifiers, name=name, @@ -608,6 +621,7 @@ def _to_dci_person(self, partner) -> Person: email=emails if emails else None, registration_date=partner.create_date if partner.create_date else None, last_updated=partner.write_date if partner.write_date else None, + enrolled_programs=enrollments if enrollments else None, ) def _to_dci_group(self, partner) -> Group: diff --git a/spp_dci_server_social/tests/test_search_service.py b/spp_dci_server_social/tests/test_search_service.py index 7462eb15..a347bfea 100644 --- a/spp_dci_server_social/tests/test_search_service.py +++ b/spp_dci_server_social/tests/test_search_service.py @@ -127,6 +127,65 @@ def test_search_by_identifier_missing_value_rejected(self): SearchStatusReasonCode.FILTER_INVALID.value, ) + def _search_by_nat_001(self, reference_id="test-ref-prog"): + """Run an idtype-value search for the NAT-001 fixture individual.""" + criteria = SearchCriteria( + reg_type="SOCIAL_REGISTRY", + query_type="idtype-value", + query={"type": self.test_id_type.namespace_uri, "value": "NAT-001"}, + ) + search_req = SearchRequestItem( + reference_id=reference_id, + timestamp=datetime.now(UTC), + search_criteria=criteria, + ) + request = SearchRequest(transaction_id=f"txn-{reference_id}", search_request=[search_req]) + self.env.user.write({"group_ids": [(4, self.env.ref("spp_registry.group_registry_viewer").id)]}) + return self.search_service.execute_search(request).search_response[0] + + def test_person_record_includes_enrolled_programs(self): + """The served person record must list active programme enrollments - + DCI clients derive sr.dci.program_count / has_programs from it.""" + program = self.env["spp.program"].create({"name": "Cash Transfer Test", "target_type": "individual"}) + self.env["spp.program.membership"].create( + { + "partner_id": self.individual_1.id, + "program_id": program.id, + "state": "enrolled", + } + ) + + response_item = self._search_by_nat_001() + + self.assertEqual(response_item.status, "succ") + record = response_item.data.reg_records[0] + self.assertIn("enrolled_programs", record, "person record lacks enrolled_programs") + programs = record["enrolled_programs"] + self.assertEqual(len(programs), 1) + self.assertEqual(programs[0]["programme_name"], "Cash Transfer Test") + self.assertEqual(programs[0]["enrolment_status"], "enrolled") + + def test_person_record_excludes_inactive_enrollments(self): + """Draft/exited memberships are not enrollments - they must not + appear in the served record.""" + program = self.env["spp.program"].create({"name": "Exited Program Test", "target_type": "individual"}) + self.env["spp.program.membership"].create( + { + "partner_id": self.individual_1.id, + "program_id": program.id, + "state": "exited", + } + ) + + response_item = self._search_by_nat_001(reference_id="test-ref-noprog") + + self.assertEqual(response_item.status, "succ") + record = response_item.data.reg_records[0] + self.assertFalse( + record.get("enrolled_programs"), + f"inactive enrollment leaked into the record: {record.get('enrolled_programs')}", + ) + def test_search_by_identifier_success(self): """Test searching by identifier type and value.""" # Create search request From 5212445f4a4dbc327c27a4f17383f27eb833723f Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 4 Jun 2026 18:25:06 +0800 Subject: [PATCH 03/22] feat(spp_dci_server_social): household summary in person records (Phase C) - spp_dci: HouseholdInfo schema (household_size, is_household_head) as an optional Person field - an OpenSPP extension so one person search answers household-level questions without a second group query - spp_dci_server_social: _build_household_info from the person's first active group membership; size counts active members, headship via the seeded urn:openspp:vocab:group-membership-type 'head' code; omitted entirely for group-less persons Unblocks sr.dci.household_size, is_head_of_household and large_household. Suites: spp_dci 20, spp_dci_server 306, spp_dci_server_social 68. --- spp_dci/schemas/__init__.py | 3 +- spp_dci/schemas/person.py | 24 ++++++++++ .../services/search_service.py | 24 +++++++++- .../tests/test_search_service.py | 48 +++++++++++++++++++ 4 files changed, 97 insertions(+), 2 deletions(-) diff --git a/spp_dci/schemas/__init__.py b/spp_dci/schemas/__init__.py index 158fd8ba..abeddd72 100644 --- a/spp_dci/schemas/__init__.py +++ b/spp_dci/schemas/__init__.py @@ -15,7 +15,7 @@ Place, AdditionalAttribute, ) -from .person import Person, ProgramEnrollment, RelatedPerson, DisabilityInfo +from .person import HouseholdInfo, Person, ProgramEnrollment, RelatedPerson, DisabilityInfo from .group import Group, Member from .search import ( SearchCriteria, @@ -77,6 +77,7 @@ # Person "Person", "ProgramEnrollment", + "HouseholdInfo", "RelatedPerson", "DisabilityInfo", # Group diff --git a/spp_dci/schemas/person.py b/spp_dci/schemas/person.py index 6de98b50..4cfea5c5 100644 --- a/spp_dci/schemas/person.py +++ b/spp_dci/schemas/person.py @@ -53,6 +53,26 @@ class ProgramEnrollment(BaseModel): ) +class HouseholdInfo(BaseModel): + """Summary of the person's household (OpenSPP extension). + + Carried inside the person record so one search answers household-level + questions (size, headship) without a second group query. + """ + + model_config = ConfigDict(populate_by_name=True) + + household_size: int | None = Field( + None, + ge=1, + description="Number of active members in the person's household", + ) + is_household_head: bool | None = Field( + None, + description="Whether the person is the head of the household", + ) + + class RelatedPerson(BaseModel): """DCI RelatedPerson schema - family relationship information.""" @@ -127,6 +147,10 @@ class Person(BaseModel): None, description="Active social protection programme enrollments", ) + household_info: HouseholdInfo | None = Field( + None, + description="Summary of the person's household (OpenSPP extension)", + ) # Update forward references diff --git a/spp_dci_server_social/services/search_service.py b/spp_dci_server_social/services/search_service.py index a38c7cd3..7d1c652c 100644 --- a/spp_dci_server_social/services/search_service.py +++ b/spp_dci_server_social/services/search_service.py @@ -13,7 +13,7 @@ from odoo.addons.spp_dci.schemas.common import Address, Identifier, Name from odoo.addons.spp_dci.schemas.constants import SearchStatusReasonCode from odoo.addons.spp_dci.schemas.group import Group, Member -from odoo.addons.spp_dci.schemas.person import Person, ProgramEnrollment +from odoo.addons.spp_dci.schemas.person import HouseholdInfo, Person, ProgramEnrollment from odoo.addons.spp_dci.schemas.search import ( Pagination, SearchRequest, @@ -622,6 +622,28 @@ def _to_dci_person(self, partner) -> Person: registration_date=partner.create_date if partner.create_date else None, last_updated=partner.write_date if partner.write_date else None, enrolled_programs=enrollments if enrollments else None, + household_info=self._build_household_info(partner), + ) + + def _build_household_info(self, partner) -> HouseholdInfo | None: + """Summarize the person's household (OpenSPP extension). + + Uses the person's first active group membership. Returns None when + the person belongs to no active group, so the field is omitted from + the wire record entirely. + """ + active = partner.individual_membership_ids.filtered(lambda m: not m.is_ended) + if not active: + return None + membership = active[0] + group_members = membership.group.group_membership_ids.filtered(lambda m: not m.is_ended) + + head_code = self.env["spp.vocabulary.code"].get_code("urn:openspp:vocab:group-membership-type", "head") + is_head = bool(head_code) and head_code.id in membership.membership_type_ids.ids + + return HouseholdInfo( + household_size=len(group_members) or None, + is_household_head=is_head, ) def _to_dci_group(self, partner) -> Group: diff --git a/spp_dci_server_social/tests/test_search_service.py b/spp_dci_server_social/tests/test_search_service.py index a347bfea..e8d8d7d6 100644 --- a/spp_dci_server_social/tests/test_search_service.py +++ b/spp_dci_server_social/tests/test_search_service.py @@ -186,6 +186,54 @@ def test_person_record_excludes_inactive_enrollments(self): f"inactive enrollment leaked into the record: {record.get('enrolled_programs')}", ) + def test_person_record_includes_household_info(self): + """The served person record must carry a household summary - DCI + clients derive sr.dci.household_size / is_head_of_household / + large_household from it (one search covers all variables).""" + head_code = self.env["spp.vocabulary.code"].get_code("urn:openspp:vocab:group-membership-type", "head") + self.assertTrue(head_code, "seeded head membership-type code missing") + membership = self.env["spp.group.membership"].search( + [("individual", "=", self.individual_1.id), ("group", "=", self.group_1.id)], + limit=1, + ) + self.assertTrue(membership, "fixture membership missing") + membership.write({"membership_type_ids": [(4, head_code.id)]}) + + response_item = self._search_by_nat_001(reference_id="test-ref-hh") + + self.assertEqual(response_item.status, "succ") + record = response_item.data.reg_records[0] + self.assertIn("household_info", record, "person record lacks household_info") + info = record["household_info"] + self.assertEqual(info["household_size"], 2) + self.assertTrue(info["is_household_head"]) + + def test_person_without_group_has_no_household_info(self): + """A person with no active group membership carries no household + summary (field omitted, not zeroed).""" + self._create_test_individual( + {"family_name": "Solo", "given_name": "NoGroup"}, + identifier_value="NAT-SOLO-1", + ) + criteria = SearchCriteria( + reg_type="SOCIAL_REGISTRY", + query_type="idtype-value", + query={"type": self.test_id_type.namespace_uri, "value": "NAT-SOLO-1"}, + ) + search_req = SearchRequestItem( + reference_id="test-ref-solo", + timestamp=datetime.now(UTC), + search_criteria=criteria, + ) + request = SearchRequest(transaction_id="txn-solo", search_request=[search_req]) + self.env.user.write({"group_ids": [(4, self.env.ref("spp_registry.group_registry_viewer").id)]}) + + response_item = self.search_service.execute_search(request).search_response[0] + + self.assertEqual(response_item.status, "succ") + record = response_item.data.reg_records[0] + self.assertNotIn("household_info", record) + def test_search_by_identifier_success(self): """Test searching by identifier type and value.""" # Create search request From 3602bb76a18f5d29ec786e690788f45314a46997 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 4 Jun 2026 18:44:45 +0800 Subject: [PATCH 04/22] feat(spp_dci_indicators): SR fetch handlers for the six sr.dci.* variables (Phase D) One Social Registry person search feeds every metric: - sr.dci.is_registered: record exists (not-found is a meaningful False) - sr.dci.program_count / has_programs: from enrolled_programs - sr.dci.household_size / is_head_of_household / large_household (>5 per the seeded variable): from household_info; size is skipped when the person has no household summary, headship/large default to False Person-not-found skips every metric except is_registered (no cache row, expressions simply don't match). Suite: spp_dci_indicators 78 (65 + 13 new). --- spp_dci_indicators/models/dci_cel_fetcher.py | 57 ++++++++ spp_dci_indicators/tests/__init__.py | 1 + spp_dci_indicators/tests/test_dci_cel_sr.py | 134 +++++++++++++++++++ 3 files changed, 192 insertions(+) create mode 100644 spp_dci_indicators/tests/test_dci_cel_sr.py diff --git a/spp_dci_indicators/models/dci_cel_fetcher.py b/spp_dci_indicators/models/dci_cel_fetcher.py index eddf135d..a1edc559 100644 --- a/spp_dci_indicators/models/dci_cel_fetcher.py +++ b/spp_dci_indicators/models/dci_cel_fetcher.py @@ -219,6 +219,12 @@ def _dci_metric_handlers(self): "dr.dci.vision_severe": self._dr_vision_severe, "dr.dci.hearing_severe": self._dr_hearing_severe, "dr.dci.mobility_severe": self._dr_mobility_severe, + "sr.dci.is_registered": self._sr_is_registered, + "sr.dci.program_count": self._sr_program_count, + "sr.dci.has_programs": self._sr_has_programs, + "sr.dci.household_size": self._sr_household_size, + "sr.dci.is_head_of_household": self._sr_is_head_of_household, + "sr.dci.large_household": self._sr_large_household, } # ── CRVS handlers (identifier-based) ────────────────────────────────────── @@ -235,6 +241,57 @@ def _crvs_is_alive(self, data_source, partner, id_type, id_value): def _crvs_birth_verified(self, data_source, partner, id_type, id_value): return self._crvs_service(data_source).verify_birth(id_type, id_value) is not None + # ── SR handlers (identifier-based; one person record feeds all metrics) ──── + + # "more than 5 members" per the seeded dci.sr.large_household variable + _SR_LARGE_HOUSEHOLD_THRESHOLD = 5 + + def _sr_service(self, data_source): + from odoo.addons.spp_dci_client_sr.services import SRService + + return SRService(self.env, data_source.code) + + def _sr_person(self, data_source, id_type, id_value): + """Fetch the person record from the Social Registry, or None.""" + return self._sr_service(data_source).search_person(id_type, id_value) + + def _sr_is_registered(self, data_source, partner, id_type, id_value): + # Not found is a meaningful False, not missing data. + return self._sr_person(data_source, id_type, id_value) is not None + + def _sr_program_count(self, data_source, partner, id_type, id_value): + person = self._sr_person(data_source, id_type, id_value) + if person is None: + return None + return len(person.get("enrolled_programs") or []) + + def _sr_has_programs(self, data_source, partner, id_type, id_value): + person = self._sr_person(data_source, id_type, id_value) + if person is None: + return None + return bool(person.get("enrolled_programs")) + + def _sr_household_size(self, data_source, partner, id_type, id_value): + person = self._sr_person(data_source, id_type, id_value) + if person is None: + return None + # No household summary -> size unknown, skip (no cache row). + return (person.get("household_info") or {}).get("household_size") + + def _sr_is_head_of_household(self, data_source, partner, id_type, id_value): + person = self._sr_person(data_source, id_type, id_value) + if person is None: + return None + # Registered but household-less -> not a head. + return bool((person.get("household_info") or {}).get("is_household_head")) + + def _sr_large_household(self, data_source, partner, id_type, id_value): + person = self._sr_person(data_source, id_type, id_value) + if person is None: + return None + size = (person.get("household_info") or {}).get("household_size") or 0 + return size > self._SR_LARGE_HOUSEHOLD_THRESHOLD + # ── DR handlers (partner-based; the service resolves the identifier) ─────── def _dr_status(self, data_source, partner): diff --git a/spp_dci_indicators/tests/__init__.py b/spp_dci_indicators/tests/__init__.py index 21faa2db..15c85887 100644 --- a/spp_dci_indicators/tests/__init__.py +++ b/spp_dci_indicators/tests/__init__.py @@ -5,6 +5,7 @@ from . import test_data_provider_dci from . import test_dci_cel_fetcher from . import test_dci_cel_dr +from . import test_dci_cel_sr from . import test_dci_cel_end_to_end from . import test_dci_cel_params from . import test_dci_cel_methods diff --git a/spp_dci_indicators/tests/test_dci_cel_sr.py b/spp_dci_indicators/tests/test_dci_cel_sr.py new file mode 100644 index 00000000..2b2e5a3b --- /dev/null +++ b/spp_dci_indicators/tests/test_dci_cel_sr.py @@ -0,0 +1,134 @@ +# Part of OpenSPP. See LICENSE file for full copyright and licensing details. +"""Tests for the SR (Social Registry) DCI fetch handlers. + +All six sr.dci.* metrics derive from a single person record returned by +SRService.search_person (identifiers -> registration, enrolled_programs, +household_info). The service is mocked here. + +Semantics: +- person not found: is_registered is False (a meaningful value); + every other metric is skipped (no data, no cache row) +- registered but no household_info: household_size skipped, + is_head_of_household / large_household are False +""" + +from unittest.mock import patch + +from odoo.tests import TransactionCase, tagged + +from odoo.addons.spp_dci.schemas.constants import RegistryType + +SEARCH_PERSON = "odoo.addons.spp_dci_client_sr.services.sr_service.SRService.search_person" + + +@tagged("post_install", "-at_install") +class TestDCICelSRHandlers(TransactionCase): + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.Fetcher = cls.env["spp.dci.cel.fetcher"] + cls.sr_source = cls.env["spp.dci.data.source"].create( + { + "name": "National SR", + "code": "national_sr_t", + "base_url": "https://sr.example.org/api", + "registry_type": RegistryType.SOCIAL_REGISTRY.value, + "our_sender_id": "openspp.test", + "auth_type": "none", + "state": "active", + } + ) + cls.provider = cls.env["spp.data.provider"].create( + {"name": "SR", "code": "national_sr_prov", "dci_data_source_id": cls.sr_source.id} + ) + cls.id_code = cls.env.ref("spp_vocabulary.code_id_type_national_id") + cls.partner = cls.env["res.partner"].create({"name": "SR Person", "is_registrant": True, "is_group": False}) + cls.env["spp.registry.id"].create( + {"partner_id": cls.partner.id, "id_type_id": cls.id_code.id, "value": "NID-SR-1"} + ) + + def _var(self, accessor, value_type="boolean"): + # One variable per accessor: spp.cel.variable enforces UNIQUE + # (cel_accessor, applies_to), so repeat fetches reuse the record. + if not hasattr(self, "_vars"): + self._vars = {} + if accessor not in self._vars: + self._vars[accessor] = self.env["spp.cel.variable"].create( + { + "name": f"zz_{accessor}", + "label": accessor, + "cel_accessor": accessor, + "source_type": "external", + "value_type": value_type, + "external_provider_id": self.provider.id, + "cache_strategy": "ttl", + } + ) + return self._vars[accessor] + + def _fetch(self, accessor, person, value_type="boolean"): + var = self._var(accessor, value_type=value_type) + with patch(SEARCH_PERSON, return_value=person): + return self.Fetcher.fetch_values(var, [self.partner.id]) + + # -- is_registered --------------------------------------------------------- + + def test_sr_is_registered_true(self): + result = self._fetch("sr.dci.is_registered", {"id": "EXT-1", "name": "SR Person"}) + self.assertEqual(result, {self.partner.id: True}) + + def test_sr_is_registered_false_when_not_found(self): + result = self._fetch("sr.dci.is_registered", None) + self.assertEqual(result, {self.partner.id: False}) + + # -- programmes ------------------------------------------------------------ + + def test_sr_program_count(self): + person = {"enrolled_programs": [{"programme_name": "A"}, {"programme_name": "B"}]} + result = self._fetch("sr.dci.program_count", person, value_type="number") + self.assertEqual(result, {self.partner.id: 2}) + + def test_sr_program_count_zero_without_enrollments(self): + result = self._fetch("sr.dci.program_count", {"id": "EXT-1"}, value_type="number") + self.assertEqual(result, {self.partner.id: 0}) + + def test_sr_program_metrics_skipped_when_not_found(self): + result = self._fetch("sr.dci.program_count", None, value_type="number") + self.assertEqual(result, {}) + + def test_sr_has_programs(self): + person = {"enrolled_programs": [{"programme_name": "A"}]} + self.assertEqual(self._fetch("sr.dci.has_programs", person), {self.partner.id: True}) + self.assertEqual(self._fetch("sr.dci.has_programs", {"id": "X"}), {self.partner.id: False}) + + # -- household ------------------------------------------------------------- + + def test_sr_household_size(self): + person = {"household_info": {"household_size": 4, "is_household_head": False}} + result = self._fetch("sr.dci.household_size", person, value_type="number") + self.assertEqual(result, {self.partner.id: 4}) + + def test_sr_household_size_skipped_without_household(self): + result = self._fetch("sr.dci.household_size", {"id": "EXT-1"}, value_type="number") + self.assertEqual(result, {}) + + def test_sr_is_head_of_household(self): + person = {"household_info": {"household_size": 3, "is_household_head": True}} + self.assertEqual(self._fetch("sr.dci.is_head_of_household", person), {self.partner.id: True}) + + def test_sr_is_head_false_without_household(self): + result = self._fetch("sr.dci.is_head_of_household", {"id": "EXT-1"}) + self.assertEqual(result, {self.partner.id: False}) + + def test_sr_large_household_above_threshold(self): + person = {"household_info": {"household_size": 6}} + self.assertEqual(self._fetch("sr.dci.large_household", person), {self.partner.id: True}) + + def test_sr_large_household_at_threshold_is_false(self): + # The seeded variable documents "more than 5 members" + person = {"household_info": {"household_size": 5}} + self.assertEqual(self._fetch("sr.dci.large_household", person), {self.partner.id: False}) + + def test_sr_large_household_false_without_household(self): + result = self._fetch("sr.dci.large_household", {"id": "EXT-1"}) + self.assertEqual(result, {self.partner.id: False}) From dd6a4f4830bcd10d58a773f5afd7b3c5105c10a0 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 4 Jun 2026 20:54:43 +0800 Subject: [PATCH 05/22] fix(spp_dci_server): SPDCI-compliant /registry mount + endpoint-user fixes - mount registry routers at /registry/* relative to the deployment base URL per the SPDCI spec (registry type travels in the message reg_type, not in a URL segment); the /social/registry long form stays as a legacy alias for existing deployments - sender-registry lookup on the sync path uses a narrow sudo: the endpoint user (often low-privilege) cannot read the sender registry, which surfaced as AccessError in live smoke testing - regression test for a low-privilege endpoint user driving the search Live-verified over HTTP: bearer-authenticated idtype-value search at {base}/registry/sync/search returns the person with programme enrollments and household summary. --- spp_dci_server/models/fastapi_endpoint_dci.py | 16 +++++++++-- spp_dci_server/routers/search.py | 6 ++++- .../tests/test_fastapi_endpoint_dci.py | 21 +++++++++++++++ spp_dci_server/tests/test_search_router.py | 27 +++++++++++++++++++ 4 files changed, 67 insertions(+), 3 deletions(-) diff --git a/spp_dci_server/models/fastapi_endpoint_dci.py b/spp_dci_server/models/fastapi_endpoint_dci.py index 0bc8d41a..7669cef6 100644 --- a/spp_dci_server/models/fastapi_endpoint_dci.py +++ b/spp_dci_server/models/fastapi_endpoint_dci.py @@ -129,8 +129,20 @@ def _get_fastapi_routers(self) -> list[APIRouter]: # JWKS at root level (/.well-known/jwks.json) routers.append(jwks_router) - # Registry operations under /social/registry prefix (SPDCI-compliant) - social_router = APIRouter(prefix="/social/registry", tags=["Social Registry"]) + # SPDCI-compliant mount: the spec defines /registry/* relative to + # the deployment base URL; the registry type is discriminated by + # the message's reg_type, not by a URL segment. + registry_router = APIRouter(prefix="/registry", tags=["DCI Registry"]) + registry_router.include_router(dci_search_router) + registry_router.include_router(dci_async_router) + registry_router.include_router(dci_callback_router) + registry_router.include_router(dci_bulk_upload_router) + registry_router.include_router(dci_receipt_router) + routers.append(registry_router) + + # Legacy long-form mount kept for existing deployments that + # configured base URLs ending in /social. + social_router = APIRouter(prefix="/social/registry", tags=["Social Registry (legacy path)"]) social_router.include_router(dci_search_router) social_router.include_router(dci_async_router) social_router.include_router(dci_callback_router) diff --git a/spp_dci_server/routers/search.py b/spp_dci_server/routers/search.py index 1d9599b5..3e291f05 100644 --- a/spp_dci_server/routers/search.py +++ b/spp_dci_server/routers/search.py @@ -107,7 +107,11 @@ async def search_registry( # Hand the verified sender to the service so consent filtering # engages - the consent adapter disengages when sender is None. - sender_registry = env["spp.dci.sender.registry"].get_by_sender_id(verified_sender_id) + # sudo: technical lookup of an already-verified sender id; the + # endpoint user (often public) has no read access to the registry. + sender_registry = ( + env["spp.dci.sender.registry"].sudo().get_by_sender_id(verified_sender_id) + ) # nosemgrep: odoo-sudo-without-context search_service = DCISocialSearchService(env, sender_registry=sender_registry or None) search_response = search_service.execute_search(search_request) _logger.info( diff --git a/spp_dci_server/tests/test_fastapi_endpoint_dci.py b/spp_dci_server/tests/test_fastapi_endpoint_dci.py index 9047b1b9..e0359060 100644 --- a/spp_dci_server/tests/test_fastapi_endpoint_dci.py +++ b/spp_dci_server/tests/test_fastapi_endpoint_dci.py @@ -135,6 +135,27 @@ def test_dci_api_app_choice_is_registered(self): keys = [k for k, _ in field.selection] self.assertIn("dci_api", keys) + def test_registry_routes_mounted_at_spec_path(self): + """SPDCI defines /registry/sync/search relative to the deployment + base URL - the registry type travels in the message reg_type, not in + the URL. The non-spec /social/registry mount stays for backward + compatibility, but the spec path must exist.""" + endpoint = self.Endpoint.create( + { + "name": "test-dci-spec-paths", + "app": "dci_api", + "root_path": "/test-dci-spec", + } + ) + all_paths = [] + for router in endpoint._get_fastapi_routers(): + if isinstance(router, APIRouter): + for route in router.routes: + all_paths.append(getattr(route, "path", "")) + self.assertIn("/registry/sync/search", all_paths, f"spec path missing: {all_paths}") + # Backward-compatible long form is still served + self.assertIn("/social/registry/sync/search", all_paths) + def test_get_fastapi_routers_returns_dci_routers(self): """A DCI endpoint must include the JWKS and registry-aliases routers.""" endpoint = self.Endpoint.create( diff --git a/spp_dci_server/tests/test_search_router.py b/spp_dci_server/tests/test_search_router.py index 0d071160..c4ba7e52 100644 --- a/spp_dci_server/tests/test_search_router.py +++ b/spp_dci_server/tests/test_search_router.py @@ -185,6 +185,33 @@ def test_sync_search_passes_verified_sender_to_service(self): "verified sender was not passed to the search service (consent bypass)", ) + def test_sync_search_sender_lookup_survives_low_privilege_endpoint_user(self): + """The endpoint commonly runs as a low-privilege user (e.g. public) + with no read access to the sender registry - the verified-sender + lookup must not raise AccessError (live smoke test regression).""" + low_priv = self.env["res.users"].create( + { + "name": "DCI Endpoint Smoke", + "login": "dci_endpoint_smoke", + "group_ids": [(6, 0, [self.env.ref("base.group_public").id])], + } + ) + env_low = self.env(user=low_priv) + envelope = self._build_envelope() + response = self._build_response(statuses=("succ",)) + with patch("odoo.addons.spp_dci_server_social.services.search_service.DCISocialSearchService") as mock_cls: + mock_cls.return_value.execute_search.return_value = response + result = _run( + self.search_registry( + envelope, + env_low, + _bearer_token="t", + verified_sender_id=self.test_sender.sender_id, + _rate_limit_check=None, + ) + ) + self.assertEqual(result.header.status, "succ") + # --- service errors ------------------------------------------------------- def test_search_service_exception_rejects_all_items(self): From 25d966e843cf15e4c851a56b3c45a48952732ed3 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 4 Jun 2026 20:56:17 +0800 Subject: [PATCH 06/22] style(spp_dci_server): keep nosemgrep suppression on the sudo match line ruff-format had wrapped the sender-registry sudo call so the suppression comment landed outside the matched expression, re-triggering the semgrep finding. Assign the sudo'd model first so the comment stays inline. --- spp_dci_server/routers/search.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/spp_dci_server/routers/search.py b/spp_dci_server/routers/search.py index 3e291f05..1c031fb2 100644 --- a/spp_dci_server/routers/search.py +++ b/spp_dci_server/routers/search.py @@ -109,9 +109,8 @@ async def search_registry( # 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. - sender_registry = ( - env["spp.dci.sender.registry"].sudo().get_by_sender_id(verified_sender_id) - ) # nosemgrep: odoo-sudo-without-context + 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_response = search_service.execute_search(search_request) _logger.info( From d42ba426f0186f3c16897e5d33c320eaf3d3a366 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 4 Jun 2026 21:08:33 +0800 Subject: [PATCH 07/22] fix(spp_dci_client_sr): use the data source's registry type, not a hardcoded one search_person, search_household and check_connection overrode the registry type with the ad-hoc string 'ns:registry_type:social_registry', which spec-compliant servers reject - every SR search returned no results. Drop the overrides so the client defaults to the namespaced RegistryType configured on the data source. Found during the local client-server e2e: searches succeeded over curl but returned None through SRService. Suite: spp_dci_client_sr 134. --- spp_dci_client_sr/services/sr_service.py | 5 ----- spp_dci_client_sr/tests/test_sr_service.py | 26 ++++++++++++++++++++++ 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/spp_dci_client_sr/services/sr_service.py b/spp_dci_client_sr/services/sr_service.py index 915477c1..268113df 100644 --- a/spp_dci_client_sr/services/sr_service.py +++ b/spp_dci_client_sr/services/sr_service.py @@ -69,7 +69,6 @@ def check_connection(self) -> bool: self.client.search( query_type="idtype-value", query_value="test:connection-check", - registry_type="ns:registry_type:social_registry", ) return True except Exception as e: @@ -104,7 +103,6 @@ def search_person( response = self.client.search_async( query_type="idtype-value", query_value=query_value, - registry_type="ns:registry_type:social_registry", ) correlation_id = response.get("message", {}).get("correlation_id") return {"correlation_id": correlation_id} if correlation_id else None @@ -112,7 +110,6 @@ def search_person( response = self.client.search( query_type="idtype-value", query_value=query_value, - registry_type="ns:registry_type:social_registry", ) # Extract search results @@ -160,7 +157,6 @@ def search_household( response = self.client.search_async( query_type="idtype-value", query_value=f"HHID:{household_id}", - registry_type="ns:registry_type:social_registry", reg_sub_type="group", ) correlation_id = response.get("message", {}).get("correlation_id") @@ -169,7 +165,6 @@ def search_household( response = self.client.search( query_type="idtype-value", query_value=f"HHID:{household_id}", - registry_type="ns:registry_type:social_registry", reg_sub_type="group", ) diff --git a/spp_dci_client_sr/tests/test_sr_service.py b/spp_dci_client_sr/tests/test_sr_service.py index 3452f135..14e531a9 100644 --- a/spp_dci_client_sr/tests/test_sr_service.py +++ b/spp_dci_client_sr/tests/test_sr_service.py @@ -310,6 +310,32 @@ def test_sync_person_to_local_looks_up_partner_by_namespace_uri(self, mock_searc self.assertTrue(result) self.assertEqual(result.partner_id, self.test_partner) + @patch("odoo.addons.spp_dci_client.services.client.DCIClient.search") + def test_search_person_uses_data_source_registry_type(self, mock_search): + """The service must not override the registry type - the client + derives it from the data source (namespaced per SPDCI). A hardcoded + ad-hoc value made every search get rejected by compliant servers.""" + mock_search.return_value = _sync_search_envelope(reg_records=[]) + service = self._get_sr_service() + service.search_person("UIN", "RT-001") + kwargs = mock_search.call_args.kwargs + self.assertIsNone( + kwargs.get("registry_type"), + f"search_person must not override registry_type, got {kwargs.get('registry_type')!r}", + ) + + @patch("odoo.addons.spp_dci_client.services.client.DCIClient.search") + def test_search_household_uses_data_source_registry_type(self, mock_search): + """Same contract for household search.""" + mock_search.return_value = _sync_search_envelope(reg_records=[]) + service = self._get_sr_service() + service.search_household("HH-RT-002") + kwargs = mock_search.call_args.kwargs + self.assertIsNone( + kwargs.get("registry_type"), + f"search_household must not override registry_type, got {kwargs.get('registry_type')!r}", + ) + @patch("odoo.addons.spp_dci_client.services.client.DCIClient.search") def test_sync_person_not_found_raises(self, mock_search): """When the registry returns no records, sync raises UserError per docstring.""" From 87bb912a6982dff9404e5653ba1af022dd6abef4 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 4 Jun 2026 21:16:37 +0800 Subject: [PATCH 08/22] fix(spp_dci_indicators): SR handlers return values for every queried person The CEL SQL fast path requires a complete cache - a row per candidate subject. Skipping not-found/household-less persons left cache holes, so every sr.dci.* comparison except is_registered fell back to Python and matched nothing (found during the local client-server e2e: cache status 'incomplete', base=30 have=20). Not found in SR now yields semantic defaults: 0 programs, household size 0, not a head, not a large household. Suite: spp_dci_indicators 78. --- spp_dci_indicators/models/dci_cel_fetcher.py | 31 ++++++++------------ spp_dci_indicators/tests/test_dci_cel_sr.py | 19 ++++++------ 2 files changed, 22 insertions(+), 28 deletions(-) diff --git a/spp_dci_indicators/models/dci_cel_fetcher.py b/spp_dci_indicators/models/dci_cel_fetcher.py index a1edc559..40fe14e3 100644 --- a/spp_dci_indicators/models/dci_cel_fetcher.py +++ b/spp_dci_indicators/models/dci_cel_fetcher.py @@ -255,40 +255,33 @@ def _sr_person(self, data_source, id_type, id_value): """Fetch the person record from the Social Registry, or None.""" return self._sr_service(data_source).search_person(id_type, id_value) + # Every SR handler returns a value for every queried person - the CEL + # SQL fast path requires a complete cache (a row per candidate), so a + # person not found in the SR yields the semantic defaults (no programs, + # no household) rather than a missing row. + def _sr_is_registered(self, data_source, partner, id_type, id_value): - # Not found is a meaningful False, not missing data. return self._sr_person(data_source, id_type, id_value) is not None def _sr_program_count(self, data_source, partner, id_type, id_value): - person = self._sr_person(data_source, id_type, id_value) - if person is None: - return None + person = self._sr_person(data_source, id_type, id_value) or {} return len(person.get("enrolled_programs") or []) def _sr_has_programs(self, data_source, partner, id_type, id_value): - person = self._sr_person(data_source, id_type, id_value) - if person is None: - return None + person = self._sr_person(data_source, id_type, id_value) or {} return bool(person.get("enrolled_programs")) def _sr_household_size(self, data_source, partner, id_type, id_value): - person = self._sr_person(data_source, id_type, id_value) - if person is None: - return None - # No household summary -> size unknown, skip (no cache row). - return (person.get("household_info") or {}).get("household_size") + person = self._sr_person(data_source, id_type, id_value) or {} + # Not registered or household-less -> size 0. + return (person.get("household_info") or {}).get("household_size") or 0 def _sr_is_head_of_household(self, data_source, partner, id_type, id_value): - person = self._sr_person(data_source, id_type, id_value) - if person is None: - return None - # Registered but household-less -> not a head. + person = self._sr_person(data_source, id_type, id_value) or {} return bool((person.get("household_info") or {}).get("is_household_head")) def _sr_large_household(self, data_source, partner, id_type, id_value): - person = self._sr_person(data_source, id_type, id_value) - if person is None: - return None + person = self._sr_person(data_source, id_type, id_value) or {} size = (person.get("household_info") or {}).get("household_size") or 0 return size > self._SR_LARGE_HOUSEHOLD_THRESHOLD diff --git a/spp_dci_indicators/tests/test_dci_cel_sr.py b/spp_dci_indicators/tests/test_dci_cel_sr.py index 2b2e5a3b..d2a8b4bc 100644 --- a/spp_dci_indicators/tests/test_dci_cel_sr.py +++ b/spp_dci_indicators/tests/test_dci_cel_sr.py @@ -5,11 +5,10 @@ SRService.search_person (identifiers -> registration, enrolled_programs, household_info). The service is mocked here. -Semantics: -- person not found: is_registered is False (a meaningful value); - every other metric is skipped (no data, no cache row) -- registered but no household_info: household_size skipped, - is_head_of_household / large_household are False +Semantics: every metric returns a value for every queried person - the CEL +SQL fast path requires a complete cache (a row per candidate). A person not +found in the SR yields the semantic defaults: not registered, 0 programs, +household size 0, not a head, not a large household. """ from unittest.mock import patch @@ -92,9 +91,10 @@ def test_sr_program_count_zero_without_enrollments(self): result = self._fetch("sr.dci.program_count", {"id": "EXT-1"}, value_type="number") self.assertEqual(result, {self.partner.id: 0}) - def test_sr_program_metrics_skipped_when_not_found(self): + def test_sr_program_count_zero_when_not_found(self): + """Not found in SR -> 0 programs (a value, so the cache stays complete).""" result = self._fetch("sr.dci.program_count", None, value_type="number") - self.assertEqual(result, {}) + self.assertEqual(result, {self.partner.id: 0}) def test_sr_has_programs(self): person = {"enrolled_programs": [{"programme_name": "A"}]} @@ -108,9 +108,10 @@ def test_sr_household_size(self): result = self._fetch("sr.dci.household_size", person, value_type="number") self.assertEqual(result, {self.partner.id: 4}) - def test_sr_household_size_skipped_without_household(self): + def test_sr_household_size_zero_without_household(self): + """Registered but household-less -> size 0 (complete cache).""" result = self._fetch("sr.dci.household_size", {"id": "EXT-1"}, value_type="number") - self.assertEqual(result, {}) + self.assertEqual(result, {self.partner.id: 0}) def test_sr_is_head_of_household(self): person = {"household_info": {"household_size": 3, "is_household_head": True}} From 8f7a4d02dbf2c8acde11f2f32f4570594f3c7470 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 4 Jun 2026 21:48:45 +0800 Subject: [PATCH 09/22] fix(spp_cel_domain): metric comparisons compose correctly in and/or The fresh-cache SQL shortcut for a metric comparison returned no ids and stashed an override domain for compile_and_preview - valid only when the comparison IS the whole plan. Composed under and/or, the first metric's override replaced the entire result, silently dropping every other conjunct (live-found: metricA == true and metricB == true matched everyone matching metricA alone). The shortcut now applies only at the plan root (as_root); composed metric nodes materialize their matching ids so set composition stays correct. Side effects fixed for free: require_coverage can no longer be bypassed by the leaked override, and compile_for_batch / compile_count_only no longer receive empty ids for fresh-cached metrics. Suite: spp_cel_domain 576 (573 + 3 new composition tests). --- spp_cel_domain/models/cel_executor.py | 32 ++++++-- spp_cel_domain/tests/__init__.py | 1 + .../tests/test_cel_metric_conjunction.py | 73 +++++++++++++++++++ 3 files changed, 99 insertions(+), 7 deletions(-) create mode 100644 spp_cel_domain/tests/test_cel_metric_conjunction.py diff --git a/spp_cel_domain/models/cel_executor.py b/spp_cel_domain/models/cel_executor.py index 60571a36..b9fb3600 100644 --- a/spp_cel_domain/models/cel_executor.py +++ b/spp_cel_domain/models/cel_executor.py @@ -424,7 +424,7 @@ def compile_and_preview( expr, ) exec_self = self.with_context(cel_mode="preview", cel_request_id=request_id) - ids = exec_self._execute_plan(model, plan, metrics_info) + ids = exec_self._execute_plan(model, plan, metrics_info, as_root=True) # If a fast-path domain override was provided in metrics_info, use it instead of materializing ids override_domain: list[Any] | None = None for mi in metrics_info: @@ -709,7 +709,13 @@ def _ensure_domain_list(self, domain: list[Any]) -> list[Any]: return [domain] # Execute - def _execute_plan(self, model: str, plan: Any, metrics_info: list[dict[str, Any]] | None = None) -> list[int]: # noqa: C901 + def _execute_plan( + self, + model: str, + plan: Any, + metrics_info: list[dict[str, Any]] | None = None, + as_root: bool = False, + ) -> list[int]: # noqa: C901 if isinstance(plan, LeafDomain): return self.env[plan.model].search(plan.domain).ids if isinstance(plan, AND): @@ -744,7 +750,7 @@ def _execute_plan(self, model: str, plan: Any, metrics_info: list[dict[str, Any] if isinstance(plan, CountThrough): return self._exec_count(plan) if isinstance(plan, MetricCompare): - return self._exec_metric(model, plan, metrics_info) + return self._exec_metric(model, plan, metrics_info, as_root=as_root) if isinstance(plan, CoverageRequire): # Only support gating on MetricCompare results for now if not isinstance(plan.node, MetricCompare): @@ -1071,10 +1077,16 @@ def _exec_metric( model: str, p: MetricCompare, metrics_info: list[dict[str, Any]] | None = None, + as_root: bool = False, ) -> list[int]: """Evaluate metric comparison and return matching subject IDs for current model. Uses openspp.metrics service with mode=fallback. + + ``as_root`` is True only when this comparison IS the whole plan: in + that case the fresh-cache SQL shortcut may return no ids and stash an + ``override_domain`` for the caller. Composed inside and/or/not, the + ids must be materialized so set composition stays correct. """ # Check metrics availability self._check_metrics_available(p.metric) @@ -1139,13 +1151,19 @@ def _exec_metric( "metric": p.metric, "period_key": period_key, "path": path, - "override_domain": domain, } ) + if as_root: + mi["override_domain"] = domain metrics_info.append(mi) - # We return [] and let compile_and_preview use override_domain to avoid - # materializing ids into a huge 'in' list - return [] + if as_root: + # The comparison is the whole plan: return [] and let + # compile_and_preview use override_domain to avoid + # materializing ids into a huge 'in' list. + return [] + # Composed inside and/or/not: materialize the matching ids so + # set composition (intersection/union) stays correct. + return self.env[subject_model].search(self._and_domains(base_dom, domain)).ids # Preview mode behavior when not fresh cel_mode = self.env.context.get("cel_mode") preview_cache_only_mode = cel_mode == "preview" and preview_cache_only diff --git a/spp_cel_domain/tests/__init__.py b/spp_cel_domain/tests/__init__.py index 325ae510..069950d9 100644 --- a/spp_cel_domain/tests/__init__.py +++ b/spp_cel_domain/tests/__init__.py @@ -5,6 +5,7 @@ from . import test_cel_caching from . import test_cel_exceptions from . import test_cel_field_aggregations +from . import test_cel_metric_conjunction from . import test_cel_functions from . import test_cel_parser from . import test_cel_security diff --git a/spp_cel_domain/tests/test_cel_metric_conjunction.py b/spp_cel_domain/tests/test_cel_metric_conjunction.py new file mode 100644 index 00000000..fa088d58 --- /dev/null +++ b/spp_cel_domain/tests/test_cel_metric_conjunction.py @@ -0,0 +1,73 @@ +# Part of OpenSPP. See LICENSE file for full copyright and licensing details. +"""Composing multiple metric() comparisons with and/or. + +The fresh-cache SQL shortcut for a single metric comparison returns no ids +and stashes an override domain for the caller. That shortcut is only valid +when the comparison IS the whole plan: inside a conjunction or disjunction +the override of the first metric must not replace the composed result +(live-found: `metricA == true and metricB == true` matched everyone that +matched metricA alone). +""" + +from odoo.tests import TransactionCase, tagged + + +@tagged("post_install", "-at_install") +class TestCelMetricConjunction(TransactionCase): + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.svc = cls.env["spp.cel.service"] + Partner = cls.env["res.partner"] + cls.a = Partner.create({"name": "Conj A", "is_registrant": True, "is_group": False}) + cls.b = Partner.create({"name": "Conj B", "is_registrant": True, "is_group": False}) + cls.c = Partner.create({"name": "Conj C", "is_registrant": True, "is_group": False}) + # Complete, fresh cache for both metrics across all three subjects: + # m1: A=1, B=1, C=0 / m2: A=1, B=0, C=1 + rows = [] + for metric, values in ( + ("zz.conj.m1", {cls.a: 1, cls.b: 1, cls.c: 0}), + ("zz.conj.m2", {cls.a: 1, cls.b: 0, cls.c: 1}), + ): + for partner, value in values.items(): + rows.append( + { + "variable_name": metric, + "subject_model": "res.partner", + "subject_id": partner.id, + "period_key": "current", + "value_json": {"value": value}, + "value_type": "number", + "source_type": "external", + "ttl_seconds": 3600, + } + ) + cls.env["spp.data.value"].upsert_values(rows) + + def _match(self, expr): + r = self.svc.compile_expression( + expr, + profile="registry_individuals", + base_domain=[("id", "in", (self.a | self.b | self.c).ids)], + limit=0, + materialize_sql=True, + ) + self.assertTrue(r.get("valid"), r.get("error")) + return self.env["res.partner"].search(r["domain"]) + + def test_metric_and_metric_intersects(self): + matched = self._match("metric('zz.conj.m1', me) >= 1 and metric('zz.conj.m2', me) >= 1") + self.assertEqual(matched, self.a, f"AND must intersect both metrics, got {matched.mapped('name')}") + + def test_metric_or_metric_unions(self): + matched = self._match("metric('zz.conj.m1', me) >= 1 or metric('zz.conj.m2', me) >= 1") + self.assertEqual( + matched, + self.a | self.b | self.c, + f"OR must union both metrics, got {matched.mapped('name')}", + ) + + def test_single_metric_shortcut_still_works(self): + """The root-level single-metric shortcut keeps its behavior.""" + matched = self._match("metric('zz.conj.m1', me) >= 1") + self.assertEqual(matched, self.a | self.b) From c43c58324a9ffee976ff4e1891b52e2e0f05974d Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 4 Jun 2026 21:53:31 +0800 Subject: [PATCH 10/22] fix(spp_cel_event): align _execute_plan override with the as_root parameter The base executor gained as_root for correct metric composition; the event executor's override broke compile with an unexpected-kwarg error on stacks where spp_cel_event is installed (invisible to spp_cel_domain's own test DB - signature changes must be checked against overrides repo-wide). Suite: spp_cel_event 170. --- spp_cel_event/models/cel_event_executor.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/spp_cel_event/models/cel_event_executor.py b/spp_cel_event/models/cel_event_executor.py index 02b10776..234a7765 100644 --- a/spp_cel_event/models/cel_event_executor.py +++ b/spp_cel_event/models/cel_event_executor.py @@ -31,7 +31,13 @@ class CelEventExecutor(models.AbstractModel): # This prevents memory exhaustion from overly broad queries MAX_QUERY_RESULTS = 100000 - def _execute_plan(self, model: str, plan: Any, metrics_info: list[dict[str, Any]] | None = None) -> list[int]: + def _execute_plan( + self, + model: str, + plan: Any, + metrics_info: list[dict[str, Any]] | None = None, + as_root: bool = False, + ) -> list[int]: """Execute query plan with event data support. Extends base executor to handle EventValueCompare, EventExists, and EventsAggregate nodes. @@ -42,7 +48,7 @@ def _execute_plan(self, model: str, plan: Any, metrics_info: list[dict[str, Any] return self._exec_event_exists(model, plan) if isinstance(plan, EventsAggregate): return self._exec_event_aggregate(model, plan) - return super()._execute_plan(model, plan, metrics_info) + return super()._execute_plan(model, plan, metrics_info, as_root=as_root) # ══════════════════════════════════════════════════════════════════════════════ # EventValueCompare Execution From 328ca85ab8dee07fa10abc02858b985d19962232 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 4 Jun 2026 22:08:28 +0800 Subject: [PATCH 11/22] fix(spp_dci_server_social): defensive guards on required m2o dereferences Guard membership.program_id and reg_id.id_type_id before building wire payloads (gemini review). Both fields are required=True so the branches are unreachable through the ORM, but the guards are cheap and protect against pathological data per the False-vs-None defensive pattern. Suite: spp_dci_server_social 68. --- spp_dci_server_social/models/res_partner_dci_notify.py | 2 +- spp_dci_server_social/services/search_service.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/spp_dci_server_social/models/res_partner_dci_notify.py b/spp_dci_server_social/models/res_partner_dci_notify.py index f250d262..70273291 100644 --- a/spp_dci_server_social/models/res_partner_dci_notify.py +++ b/spp_dci_server_social/models/res_partner_dci_notify.py @@ -110,7 +110,7 @@ def _dci_delete_payloads(self): "identifier_value": reg_id.value, } for reg_id in partner.reg_ids - if reg_id.value + if reg_id.value and reg_id.id_type_id ] payloads.append({"identifiers": identifiers}) return payloads diff --git a/spp_dci_server_social/services/search_service.py b/spp_dci_server_social/services/search_service.py index 7d1c652c..456e63e2 100644 --- a/spp_dci_server_social/services/search_service.py +++ b/spp_dci_server_social/services/search_service.py @@ -607,7 +607,7 @@ def _to_dci_person(self, partner) -> Person: enrolment_date=membership.enrollment_date.date() if membership.enrollment_date else None, ) for membership in partner.program_membership_ids - if membership.state in ("enrolled", "paused") + if membership.state in ("enrolled", "paused") and membership.program_id ] return Person( From 7b5de7e86ba07fc8a001a1f552fcce9bb10845e5 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 4 Jun 2026 22:25:46 +0800 Subject: [PATCH 12/22] test(spp_cel_event): cover the _execute_plan passthrough Event tests early-return on event nodes and plain plans take the SQL path, so the override's super() passthrough (the as_root thread-through) was the one uncovered changed line on the PR. Drive a LeafDomain plan through the override both with and without as_root. Suite: spp_cel_event 171. --- .../tests/test_cel_event_integration.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/spp_cel_event/tests/test_cel_event_integration.py b/spp_cel_event/tests/test_cel_event_integration.py index a579941b..24941933 100644 --- a/spp_cel_event/tests/test_cel_event_integration.py +++ b/spp_cel_event/tests/test_cel_event_integration.py @@ -1258,3 +1258,20 @@ def test_compute_aggregation_dict_dispatch(self): self.assertEqual(executor._compute_aggregation(events, "count", None), 2) # Unknown agg type returns 0 self.assertEqual(executor._compute_aggregation(events, "unknown_agg", "score"), 0) + + +@tagged("post_install", "-at_install") +class TestEventExecutorPassthrough(TransactionCase): + """Non-event plan nodes fall through the event executor's _execute_plan + override to the base implementation (with the as_root parameter intact).""" + + def test_non_event_plan_falls_through_to_base(self): + from odoo.addons.spp_cel_domain.models.cel_queryplan import LeafDomain + + partner = self.env["res.partner"].create( + {"name": "Passthrough Person", "is_registrant": True, "is_group": False} + ) + plan = LeafDomain(model="res.partner", domain=[("id", "=", partner.id)]) + executor = self.env["spp.cel.executor"] + self.assertEqual(executor._execute_plan("res.partner", plan), [partner.id]) + self.assertEqual(executor._execute_plan("res.partner", plan, as_root=True), [partner.id]) From eca3a913820606f8153c80cf924a020c0bb03754 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Mon, 8 Jun 2026 15:15:52 +0800 Subject: [PATCH 13/22] fix(spp_dci_client): show bearer token field in data source form The data source model enforces a constraint requiring bearer_token when auth_type is 'bearer', but the form view only rendered the OAuth2 fields. Selecting 'Bearer Token' left no input to set the credential. Add the bearer_token field gated on auth_type == 'bearer', mirroring the OAuth2 field layout. --- spp_dci_client/views/data_source_views.xml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/spp_dci_client/views/data_source_views.xml b/spp_dci_client/views/data_source_views.xml index 1cb7129a..74277da1 100644 --- a/spp_dci_client/views/data_source_views.xml +++ b/spp_dci_client/views/data_source_views.xml @@ -120,6 +120,12 @@ name="auth_type" help="Authentication method to use with this data source" /> + Date: Mon, 8 Jun 2026 15:43:05 +0800 Subject: [PATCH 14/22] feat(spp_dci_server): settings page for API tokens and security flags Operators had to edit raw ir.config_parameter records to manage the DCI bearer-token allow-list and security flags. Add a 'DCI Server' section to Settings (and a Server Settings item under the DCI config menu) exposing: - API Authentication: dci.api_tokens, dci.api_tokens_required, dci.sender_id - Development/Insecure: allow_unsigned_requests, bypass_bearer_auth, allow_http_callbacks, allow_internal_callback_ips (clearly warned) The boolean flags are managed via get_values/set_values writing literal 'true'/'false' strings, because Odoo's config_parameter boolean machinery deletes the parameter when False (which would make the default-true dci.api_tokens_required impossible to disable) and misreads 'false' as truthy. The menu is gated to base.group_system. --- spp_dci_server/__manifest__.py | 1 + spp_dci_server/models/__init__.py | 1 + spp_dci_server/models/res_config_settings.py | 85 +++++++++++++++++ spp_dci_server/tests/__init__.py | 1 + .../tests/test_res_config_settings.py | 55 +++++++++++ .../views/res_config_settings_views.xml | 91 +++++++++++++++++++ 6 files changed, 234 insertions(+) create mode 100644 spp_dci_server/models/res_config_settings.py create mode 100644 spp_dci_server/tests/test_res_config_settings.py create mode 100644 spp_dci_server/views/res_config_settings_views.xml diff --git a/spp_dci_server/__manifest__.py b/spp_dci_server/__manifest__.py index be4388f1..9f64bde3 100644 --- a/spp_dci_server/__manifest__.py +++ b/spp_dci_server/__manifest__.py @@ -24,6 +24,7 @@ "views/sender_registry_views.xml", "views/transaction_views.xml", "views/subscription_views.xml", + "views/res_config_settings_views.xml", ], "installable": True, "application": False, diff --git a/spp_dci_server/models/__init__.py b/spp_dci_server/models/__init__.py index c3993853..b2736127 100644 --- a/spp_dci_server/models/__init__.py +++ b/spp_dci_server/models/__init__.py @@ -1,5 +1,6 @@ from . import ( fastapi_endpoint_dci, + res_config_settings, sender_registry, server_key, subscription, diff --git a/spp_dci_server/models/res_config_settings.py b/spp_dci_server/models/res_config_settings.py new file mode 100644 index 00000000..0dac3b0c --- /dev/null +++ b/spp_dci_server/models/res_config_settings.py @@ -0,0 +1,85 @@ +# Part of OpenSPP. See LICENSE file for full copyright and licensing details. + +from odoo import api, fields, models + + +class ResConfigSettings(models.TransientModel): + _inherit = "res.config.settings" + + # The DCI auth middleware stores every boolean flag as the literal string + # "true"/"false" (see middleware/signature.py:_read_security_flag) and falls + # back to an in-code default when the parameter is missing. Odoo's + # ``config_parameter`` boolean machinery is incompatible with that: it + # *deletes* the parameter when the field is False (so a default-true flag can + # never be turned off) and reads back ``bool("false")`` as True. We therefore + # manage these flags explicitly in get_values/set_values instead. + _DCI_FLAG_PARAMS = { + "dci_api_tokens_required": ("dci.api_tokens_required", True), + "dci_allow_unsigned_requests": ("dci.allow_unsigned_requests", False), + "dci_bypass_bearer_auth": ("dci.bypass_bearer_auth", False), + "dci_allow_http_callbacks": ("dci.allow_http_callbacks", False), + "dci_allow_internal_callback_ips": ("dci.allow_internal_callback_ips", False), + } + + # --- API authentication --- + dci_api_tokens = fields.Char( + string="DCI API Bearer Tokens", + config_parameter="dci.api_tokens", + help="Accepted bearer tokens for incoming DCI requests. " + "Comma-separated for multiple clients. Each token must match the " + "Bearer Token configured on the calling client's data source.", + ) + dci_sender_id = fields.Char( + string="DCI Server Sender ID", + config_parameter="dci.sender_id", + default="openspp", + help="This server's own DCI sender id, stamped on outgoing envelopes.", + ) + dci_api_tokens_required = fields.Boolean( + string="Require DCI API Tokens", + default=True, + help="When enabled and no tokens are configured, every request is " + "rejected (fail-closed). Disable only for development.", + ) + + # --- Development / insecure options (never enable in production) --- + dci_allow_unsigned_requests = fields.Boolean( + string="Allow Unsigned Requests", + default=False, + help="Development only. Accept DCI envelopes that carry no signature, " + "skipping signature verification. Never enable in production.", + ) + dci_bypass_bearer_auth = fields.Boolean( + string="Bypass Bearer Authentication", + default=False, + help="Development only. Skip the bearer-token check entirely. Never enable in production.", + ) + dci_allow_http_callbacks = fields.Boolean( + string="Allow HTTP Callbacks", + default=False, + help="Development only. Permit plain-http (non-TLS) callback URLs. Never enable in production.", + ) + dci_allow_internal_callback_ips = fields.Boolean( + string="Allow Internal Callback IPs", + default=False, + help="Development only. Permit callbacks to internal/private IP addresses. Never enable in production.", + ) + + @api.model + def get_values(self): + res = super().get_values() + # System parameters require sudo; this view is gated to base.group_system. + # nosemgrep: odoo-sudo-without-context + icp = self.env["ir.config_parameter"].sudo() + for field_name, (param, default) in self._DCI_FLAG_PARAMS.items(): + default_str = "true" if default else "false" + res[field_name] = icp.get_param(param, default_str).lower() == "true" + return res + + def set_values(self): + super().set_values() + # System parameters require sudo; this view is gated to base.group_system. + # nosemgrep: odoo-sudo-without-context + icp = self.env["ir.config_parameter"].sudo() + for field_name, (param, _default) in self._DCI_FLAG_PARAMS.items(): + icp.set_param(param, "true" if self[field_name] else "false") diff --git a/spp_dci_server/tests/__init__.py b/spp_dci_server/tests/__init__.py index 7cfe173a..4ff61cfe 100644 --- a/spp_dci_server/tests/__init__.py +++ b/spp_dci_server/tests/__init__.py @@ -12,6 +12,7 @@ from . import test_rate_limit_middleware from . import test_receipt from . import test_receipt_router +from . import test_res_config_settings from . import test_search_router from . import test_sender_registry from . import test_server_key diff --git a/spp_dci_server/tests/test_res_config_settings.py b/spp_dci_server/tests/test_res_config_settings.py new file mode 100644 index 00000000..368256c4 --- /dev/null +++ b/spp_dci_server/tests/test_res_config_settings.py @@ -0,0 +1,55 @@ +# Part of OpenSPP. See LICENSE file for full copyright and licensing details. + +from odoo.tests import TransactionCase, tagged + +from odoo.addons.spp_dci_server.middleware.signature import _read_security_flag + + +@tagged("post_install", "-at_install") +class TestDCIServerConfigSettings(TransactionCase): + """The DCI Server settings page must round-trip to ir.config_parameter + and feed the values the auth middleware reads.""" + + def setUp(self): + super().setUp() + self.Settings = self.env["res.config.settings"] + self.Param = self.env["ir.config_parameter"].sudo() + + def _save(self, values): + settings = self.Settings.create(values) + settings.execute() + return settings + + def test_api_tokens_round_trip(self): + """Bearer tokens entered in settings persist to dci.api_tokens.""" + self._save({"dci_api_tokens": "alpha,beta"}) + self.assertEqual(self.Param.get_param("dci.api_tokens"), "alpha,beta") + + # And reading settings back reflects the stored value. + reloaded = self.Settings.create({}) + self.assertEqual(reloaded.dci_api_tokens, "alpha,beta") + + def test_sender_id_round_trip(self): + self._save({"dci_sender_id": "openspp.test.server"}) + self.assertEqual(self.Param.get_param("dci.sender_id"), "openspp.test.server") + + def test_security_flags_feed_middleware(self): + """Toggling the dev flags is visible to the middleware's flag reader.""" + self._save( + { + "dci_allow_unsigned_requests": True, + "dci_api_tokens_required": False, + } + ) + self.assertEqual(_read_security_flag(self.env, "dci.allow_unsigned_requests"), "true") + self.assertEqual(_read_security_flag(self.env, "dci.api_tokens_required"), "false") + + # Flipping them back is honoured too. + self._save( + { + "dci_allow_unsigned_requests": False, + "dci_api_tokens_required": True, + } + ) + self.assertEqual(_read_security_flag(self.env, "dci.allow_unsigned_requests"), "false") + self.assertEqual(_read_security_flag(self.env, "dci.api_tokens_required"), "true") diff --git a/spp_dci_server/views/res_config_settings_views.xml b/spp_dci_server/views/res_config_settings_views.xml new file mode 100644 index 00000000..59eda5c3 --- /dev/null +++ b/spp_dci_server/views/res_config_settings_views.xml @@ -0,0 +1,91 @@ + + + + + res.config.settings.view.form.inherit.dci.server + + res.config.settings + + + + + + + + + + + + + + + + + +

+ These options weaken or + disable DCI security checks. Enable them only in + development — never in production. +
+ + + + + + + + + + + + + + + + + + + + Settings + ir.actions.act_window + res.config.settings + + form + current + {'module': 'spp_dci_server'} + + + + From fc4912a832f327448f7cdfde49ce4c9409ef3766 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Mon, 8 Jun 2026 15:51:27 +0800 Subject: [PATCH 15/22] ci(security): retry gitleaks download to survive transient 5xx The Install Gitleaks step piped curl directly into tar. A transient 504 from the GitHub release CDN truncated the stream and failed the security gate with exit 2, despite no actual secrets. Download to a file with curl --retry/--retry-all-errors first, then extract, so a flaky download recovers instead of blocking the PR. --- .github/workflows/security.yml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index 888e14cb..070018a5 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -30,8 +30,12 @@ jobs: run: | GITLEAKS_VERSION="8.21.2" mkdir -p "$HOME/.local/bin" - curl -sSfL "https://github.com/gitleaks/gitleaks/releases/download/v${GITLEAKS_VERSION}/gitleaks_${GITLEAKS_VERSION}_linux_x64.tar.gz" \ - | tar -xz -C "$HOME/.local/bin" gitleaks + # Download to a file (not a pipe) with retries so a transient 5xx + # from the release CDN doesn't fail the whole security gate. + curl -sSfL --retry 5 --retry-all-errors --retry-delay 3 \ + -o /tmp/gitleaks.tar.gz \ + "https://github.com/gitleaks/gitleaks/releases/download/v${GITLEAKS_VERSION}/gitleaks_${GITLEAKS_VERSION}_linux_x64.tar.gz" + tar -xz -C "$HOME/.local/bin" gitleaks < /tmp/gitleaks.tar.gz echo "$HOME/.local/bin" >> "$GITHUB_PATH" - name: Run Gitleaks From ae7961fcbe6da847b8f7ca192dc77c297476b228 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Mon, 8 Jun 2026 16:06:51 +0800 Subject: [PATCH 16/22] feat(spp_dci): Test Connection validates the bearer token Test Connection probed {base_url}/health, but no such endpoint exists, so the server returned 404 - which the client treated as success. The bearer token was attached but never validated (auth is a per-route dependency; a non-existent route runs none), so a blank or mismatched token still showed 'Connection Successful'. Server: add an authenticated GET /registry/ping that requires the bearer token (verify_bearer_token) but no DCI signature - 200 with a valid token, 401 otherwise. Client: point test_connection() at /registry/ping and interpret the result - 200 means reachable AND credentials accepted; 404/405 means reachable but no ping endpoint (credentials unverified, shown as a warning); 401/403/5xx fail. Make the 401 guidance auth-type-agnostic (bearer or OAuth2). --- spp_dci_client/models/data_source.py | 51 +++++++++++++++---- spp_dci_client/services/errors.py | 8 ++- spp_dci_client/tests/test_data_source_http.py | 35 +++++++++++++ spp_dci_server/models/fastapi_endpoint_dci.py | 3 ++ spp_dci_server/routers/ping.py | 42 +++++++++++++++ spp_dci_server/tests/__init__.py | 1 + spp_dci_server/tests/test_ping_router.py | 40 +++++++++++++++ 7 files changed, 167 insertions(+), 13 deletions(-) create mode 100644 spp_dci_server/routers/ping.py create mode 100644 spp_dci_server/tests/test_ping_router.py diff --git a/spp_dci_client/models/data_source.py b/spp_dci_client/models/data_source.py index 6f810b2c..8a21656b 100644 --- a/spp_dci_client/models/data_source.py +++ b/spp_dci_client/models/data_source.py @@ -508,22 +508,20 @@ def test_connection(self): try: headers = self.get_headers() - # Test connection with a simple request to base URL - # Most DCI APIs have a health or info endpoint at root - test_url = f"{self.base_url}/health" + # Probe the authenticated ping endpoint. A 200 confirms both + # reachability *and* that our credentials are accepted; a 401/403 + # means we reached the server but auth is misconfigured (and falls + # through to the HTTPStatusError branch below). + test_url = f"{self.base_url}/registry/ping" with httpx.Client(verify=self.verify_ssl, timeout=self.timeout) as client: response = client.get(test_url, headers=headers) - # Consider 200, 404, and 405 as "connection successful" - # (404/405 mean we reached the server, just wrong endpoint) - if response.status_code in (200, 404, 405): + if response.status_code == 200: _logger.info( - "Connection test successful for data source %s: HTTP %s", + "Connection test successful for data source %s: HTTP 200", self.code, - response.status_code, ) - # Update state to active and record test date self.write( { "state": "active", @@ -536,12 +534,43 @@ def test_connection(self): "tag": "display_notification", "params": { "title": _("Connection Successful"), - "message": _("Successfully connected to %s at %s (HTTP %s)") - % (self.name, self.base_url, response.status_code), + "message": _("Connected to %s at %s and credentials were accepted.") + % (self.name, self.base_url), "type": "success", "sticky": False, }, } + elif response.status_code in (404, 405): + # We reached the server, but it has no ping endpoint (e.g. a + # non-OpenSPP DCI server). Treat as reachable, but make clear + # the credentials were not verified. + _logger.info( + "Connection test reached data source %s but no ping endpoint (HTTP %s); " + "credentials not verified", + self.code, + response.status_code, + ) + self.write( + { + "state": "active", + "last_test_date": fields.Datetime.now(), + "last_error": False, + } + ) + return { + "type": "ir.actions.client", + "tag": "display_notification", + "params": { + "title": _("Server Reachable"), + "message": _( + "Reached %s at %s, but it has no ping endpoint (HTTP %s), " + "so the credentials could not be verified." + ) + % (self.name, self.base_url, response.status_code), + "type": "warning", + "sticky": False, + }, + } else: raise httpx.HTTPStatusError( f"Unexpected status code: {response.status_code}", diff --git a/spp_dci_client/services/errors.py b/spp_dci_client/services/errors.py index 12736eca..f8fe8f0e 100644 --- a/spp_dci_client/services/errors.py +++ b/spp_dci_client/services/errors.py @@ -6,7 +6,7 @@ # HTTP error code to user message mapping HTTP_ERROR_MESSAGES = { 400: _("The request was invalid. Please check your data and try again."), - 401: _("Authentication failed. Please verify the OAuth2 credentials in Data Source settings."), + 401: _("Authentication failed. Please verify the credentials in Data Source settings."), 403: _("Access denied. The external registry rejected our request."), 404: _("The requested resource was not found on the external registry."), 500: _("The external registry encountered an error. Please try again later."), @@ -30,7 +30,11 @@ def format_http_error(status_code: int, technical_detail: str = None) -> str: # Add helpful next steps if status_code == 401: - user_msg += _("\n\nPlease check:\n• OAuth2 Token URL\n• Client ID and Secret\n• OAuth2 Scope") + user_msg += _( + "\n\nPlease check:\n" + "• Bearer Token (must match the server's allow-list), or\n" + "• OAuth2 Token URL, Client ID, Secret and Scope" + ) elif status_code in (500, 502, 503): user_msg += _("\n\nIf the problem persists, contact the registry administrator.") diff --git a/spp_dci_client/tests/test_data_source_http.py b/spp_dci_client/tests/test_data_source_http.py index b573e95f..c8fcb591 100644 --- a/spp_dci_client/tests/test_data_source_http.py +++ b/spp_dci_client/tests/test_data_source_http.py @@ -173,6 +173,41 @@ def test_connection_success_activates(self): self.assertEqual(ds.state, "active") self.assertTrue(ds.last_test_date) + def test_connection_unauthorized_sets_error_state(self): + """A 401 from the ping endpoint means the credentials were rejected; + Test Connection must surface this as a failure, not a success.""" + ds = self.DataSource.create( + { + "name": "Conn Auth DS", + "code": "conn_auth_ds", + "base_url": "https://dci.example.org/api", + "auth_type": "none", + } + ) + resp = MagicMock(status_code=401, text="unauthorized", request=MagicMock()) + with patch(HTTPX_CLIENT, return_value=_client_cm(resp)): + result = ds.test_connection() + self.assertEqual(result["params"]["type"], "danger") + self.assertEqual(ds.state, "error") + self.assertTrue(ds.last_error) + + def test_connection_no_ping_endpoint_warns_but_reachable(self): + """A 404/405 means the server is reachable but has no ping endpoint, so + credentials are unverified: reachable (active) with a warning.""" + ds = self.DataSource.create( + { + "name": "Conn NoPing DS", + "code": "conn_noping_ds", + "base_url": "https://dci.example.org/api", + "auth_type": "none", + } + ) + resp = MagicMock(status_code=404) + with patch(HTTPX_CLIENT, return_value=_client_cm(resp)): + result = ds.test_connection() + self.assertEqual(result["params"]["type"], "warning") + self.assertEqual(ds.state, "active") + def test_connection_http_error_sets_error_state(self): ds = self.DataSource.create( { diff --git a/spp_dci_server/models/fastapi_endpoint_dci.py b/spp_dci_server/models/fastapi_endpoint_dci.py index 7669cef6..62b5e214 100644 --- a/spp_dci_server/models/fastapi_endpoint_dci.py +++ b/spp_dci_server/models/fastapi_endpoint_dci.py @@ -118,6 +118,7 @@ def _get_fastapi_routers(self) -> list[APIRouter]: from ..routers.bulk_upload import dci_bulk_upload_router from ..routers.callbacks import dci_callback_router from ..routers.jwks import jwks_router + from ..routers.ping import dci_ping_router from ..routers.receipt import dci_receipt_router from ..routers.registry_aliases import ( crvs_router, @@ -138,6 +139,7 @@ def _get_fastapi_routers(self) -> list[APIRouter]: registry_router.include_router(dci_callback_router) registry_router.include_router(dci_bulk_upload_router) registry_router.include_router(dci_receipt_router) + registry_router.include_router(dci_ping_router) routers.append(registry_router) # Legacy long-form mount kept for existing deployments that @@ -148,6 +150,7 @@ def _get_fastapi_routers(self) -> list[APIRouter]: social_router.include_router(dci_callback_router) social_router.include_router(dci_bulk_upload_router) social_router.include_router(dci_receipt_router) + social_router.include_router(dci_ping_router) routers.append(social_router) # Add SPDCI-compliant registry type endpoints (stub implementations) diff --git a/spp_dci_server/routers/ping.py b/spp_dci_server/routers/ping.py new file mode 100644 index 00000000..d042d833 --- /dev/null +++ b/spp_dci_server/routers/ping.py @@ -0,0 +1,42 @@ +# Part of OpenSPP. See LICENSE file for full copyright and licensing details. +"""DCI authenticated ping endpoint. + +A lightweight endpoint clients can call to verify both reachability *and* +their bearer-token configuration in one request. Unlike the search routes it +requires only a valid bearer token (no signed DCI envelope), so a plain GET is +enough — which is exactly what the client's "Test Connection" needs. +""" + +import logging +from typing import Annotated + +from odoo.api import Environment + +from odoo.addons.fastapi.dependencies import odoo_env + +from fastapi import APIRouter, Depends + +from ..middleware.signature import verify_bearer_token + +_logger = logging.getLogger(__name__) + +dci_ping_router = APIRouter(tags=["DCI Ping"]) + + +@dci_ping_router.get("/ping") +async def ping( + env: Annotated[Environment, Depends(odoo_env)], + _bearer_token: Annotated[str, Depends(verify_bearer_token)], +): + """Authenticated liveness/auth check. + + Returns 200 with the server's sender id when the bearer token is accepted. + The ``verify_bearer_token`` dependency raises 401 when the token is missing + or not in the configured ``dci.api_tokens`` allow-list, so a client can + distinguish a reachable-but-misconfigured server from a working one. + + **Authentication**: Bearer token only (no DCI signature required). + """ + # nosemgrep: odoo-sudo-without-context + sender_id = env["ir.config_parameter"].sudo().get_param("dci.sender_id", "openspp") + return {"status": "ok", "sender_id": sender_id} diff --git a/spp_dci_server/tests/__init__.py b/spp_dci_server/tests/__init__.py index 4ff61cfe..d5076d77 100644 --- a/spp_dci_server/tests/__init__.py +++ b/spp_dci_server/tests/__init__.py @@ -9,6 +9,7 @@ from . import test_consent_adapter from . import test_fastapi_endpoint_dci from . import test_jwks_router +from . import test_ping_router from . import test_rate_limit_middleware from . import test_receipt from . import test_receipt_router diff --git a/spp_dci_server/tests/test_ping_router.py b/spp_dci_server/tests/test_ping_router.py new file mode 100644 index 00000000..662ffb0c --- /dev/null +++ b/spp_dci_server/tests/test_ping_router.py @@ -0,0 +1,40 @@ +# Part of OpenSPP. See LICENSE file for full copyright and licensing details. +"""Tests for the authenticated DCI ping endpoint.""" + +import asyncio + +from odoo.tests import tagged + +from odoo.addons.spp_dci_server.middleware.signature import verify_bearer_token +from odoo.addons.spp_dci_server.routers.ping import dci_ping_router, ping + +from .common import DCIServerCommon + + +def _run(coro): + loop = asyncio.new_event_loop() + try: + return loop.run_until_complete(coro) + finally: + loop.close() + + +@tagged("post_install", "-at_install") +class TestPingRouter(DCIServerCommon): + def test_ping_returns_ok_with_sender_id(self): + """A reachable, authenticated ping returns the server's sender id.""" + self.env["ir.config_parameter"].sudo().set_param("dci.sender_id", "openspp.test.server") + result = _run(ping(self.env, "a-valid-token")) + self.assertEqual(result["status"], "ok") + self.assertEqual(result["sender_id"], "openspp.test.server") + + def test_ping_requires_bearer_token(self): + """The route must be gated by verify_bearer_token so a bad/missing + token yields 401 rather than a misleading success.""" + route = next(r for r in dci_ping_router.routes if getattr(r, "path", None) == "/ping") + dependency_calls = [dep.call for dep in route.dependant.dependencies] + self.assertIn( + verify_bearer_token, + dependency_calls, + "ping endpoint must depend on verify_bearer_token", + ) From ce0ddaab15c67537eb92442d74db0b1fde757e94 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Mon, 8 Jun 2026 17:45:20 +0800 Subject: [PATCH 17/22] fix(spp_dci_indicators): depend on spp_dci_client_sr The sr.dci.* fetch handlers lazy-import SRService from spp_dci_client_sr and write spp.dci.sr.record, but the module only declared the dr/crvs/ibr clients as dependencies. On an instance without spp_dci_client_sr installed, every SR metric sync fails. Add it as an explicit dependency, matching the other registry clients. --- spp_dci_indicators/__manifest__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spp_dci_indicators/__manifest__.py b/spp_dci_indicators/__manifest__.py index 552bc1a7..b17e34bf 100644 --- a/spp_dci_indicators/__manifest__.py +++ b/spp_dci_indicators/__manifest__.py @@ -2,7 +2,7 @@ { "name": "OpenSPP DCI Indicators", "summary": "DCI data integration with CEL eligibility expressions", - "version": "19.0.1.0.0", + "version": "19.0.1.0.1", "category": "OpenSPP/Integration", "author": "OpenSPP.org", "website": "https://github.com/OpenSPP/OpenSPP2", @@ -13,6 +13,7 @@ "spp_dci_client_dr", "spp_dci_client_crvs", "spp_dci_client_ibr", + "spp_dci_client_sr", # SRService for sr.dci.* fetch handlers "spp_cel_domain", # Unified variable system "spp_studio", # For variable label and UI fields ], From 800189b3891773eb2fc1108989577c28995363fe Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Mon, 8 Jun 2026 18:28:14 +0800 Subject: [PATCH 18/22] refactor(spp_dci_indicators): rename CEL accessors to r.dci.. Adopt the new DCI accessor convention r.dci..: - r -> the target is registrants - dci -> evaluated against a remote DCI server - -> the registry type (sr, crvs, dr, ibr) Renames every variable's cel_accessor from .dci. to r.dci.. (e.g. sr.dci.is_registered -> r.dci.sr.is_registered) across the seed data, the fetcher handler registry, DCI_METHOD_ACCESSORS, the parameterized-method comparisons, docs and tests. The internal variable name (dci..) is unchanged. The resolver is data-driven (matches any registered cel_accessor with a dot), so no detection logic changed. Note: cel_accessor is the spp.data.value cache key; existing instances must re-sync (TTL caches self-heal) and update any saved eligibility expressions. --- spp_dci_indicators/README.md | 34 ++++++------ spp_dci_indicators/README.rst | 24 ++++----- spp_dci_indicators/USAGE.md | 52 +++++++++---------- spp_dci_indicators/__manifest__.py | 4 +- spp_dci_indicators/data/indicator_data.xml | 46 ++++++++-------- .../models/cel_variable_resolver_dci.py | 6 +-- .../models/data_cache_manager_dci.py | 2 +- spp_dci_indicators/models/dci_cel_fetcher.py | 40 +++++++------- spp_dci_indicators/readme/DESCRIPTION.md | 20 +++---- .../static/description/index.html | 24 ++++----- spp_dci_indicators/tests/test_dci_cel_dr.py | 12 ++--- .../tests/test_dci_cel_end_to_end.py | 14 ++--- .../tests/test_dci_cel_fetcher.py | 8 +-- .../tests/test_dci_cel_fetcher_errors.py | 20 +++---- .../tests/test_dci_cel_methods.py | 18 +++---- spp_dci_indicators/tests/test_dci_cel_sr.py | 30 +++++------ .../tests/test_indicator_data.py | 4 +- 17 files changed, 179 insertions(+), 179 deletions(-) diff --git a/spp_dci_indicators/README.md b/spp_dci_indicators/README.md index 01a766ce..07ea43ef 100644 --- a/spp_dci_indicators/README.md +++ b/spp_dci_indicators/README.md @@ -11,7 +11,7 @@ DCI Data Source (connection: URL, OAuth2, registry type) ▲ linked via the "DCI Integration" tab Data Provider (becomes "DCI-backed") ▲ External Source on the variable -Studio Variable (e.g. crvs.dci.is_alive) +Studio Variable (e.g. r.dci.crvs.is_alive) │ │ Sync DCI Values (action / cron) — real DCI calls, results cached ▼ @@ -29,42 +29,42 @@ variable's TTL. ```python # Check if person is alive (no death record in CRVS) -crvs.dci.is_alive == true +r.dci.crvs.is_alive == true # Check if birth was registered -crvs.dci.birth_verified == true +r.dci.crvs.birth_verified == true # Parameterized: check for a specific CRVS event -crvs.dci.has_event('death') == true +r.dci.crvs.has_event('death') == true # Check disability status -dr.dci.has_disability == true +r.dci.dr.has_disability == true # Parameterized: functional severity score for a disability type -dr.dci.severity('Vision') >= 3 +r.dci.dr.severity('Vision') >= 3 # Combined eligibility criteria -crvs.dci.is_alive == true and dr.dci.has_disability == true and age_years(me.birthdate) >= 18 +r.dci.crvs.is_alive == true and r.dci.dr.has_disability == true and age_years(me.birthdate) >= 18 ``` ## Available variables -| Accessor | Type | Meaning | -| ------------------------------------------------------------- | ------ | ------------------------------- | -| `crvs.dci.is_alive` | bool | no death event recorded in CRVS | -| `crvs.dci.birth_verified` | bool | birth registration exists | -| `crvs.dci.has_event('birth'\|'death')` | bool | parameterized event check | -| `dr.dci.has_disability` | bool | disability registered in DR | -| `dr.dci.assessed` | bool | functional assessment exists | -| `dr.dci.vision_severe` / `hearing_severe` / `mobility_severe` | bool | functional score ≥ 3 | -| `dr.dci.severity('Vision'\|'Hearing'\|'Mobility')` | number | functional score (0–4) | +| Accessor | Type | Meaning | +| --------------------------------------------------------------- | ------ | ------------------------------- | +| `r.dci.crvs.is_alive` | bool | no death event recorded in CRVS | +| `r.dci.crvs.birth_verified` | bool | birth registration exists | +| `r.dci.crvs.has_event('birth'\|'death')` | bool | parameterized event check | +| `r.dci.dr.has_disability` | bool | disability registered in DR | +| `r.dci.dr.assessed` | bool | functional assessment exists | +| `r.dci.dr.vision_severe` / `hearing_severe` / `mobility_severe` | bool | functional score ≥ 3 | +| `r.dci.dr.severity('Vision'\|'Hearing'\|'Mobility')` | number | functional score (0–4) | Parameterized methods take arguments from a **fixed, pre-synced set**: each (person, argument) pair is cached as its own row, keyed by the argument (`params_hash`). Arbitrary/dynamic arguments are not supported. > **Planned, not yet wired:** IBR and Social Registry variables, and -> `crvs.dci.is_married` (no outbound CRVS marriage query). The corresponding variable +> `r.dci.crvs.is_married` (no outbound CRVS marriage query). The corresponding variable > records exist but return no data until their fetch handlers are implemented. ## Setup diff --git a/spp_dci_indicators/README.rst b/spp_dci_indicators/README.rst index 956c28c1..5a6c3655 100644 --- a/spp_dci_indicators/README.rst +++ b/spp_dci_indicators/README.rst @@ -36,13 +36,13 @@ Key Capabilities Integration** tab, making the provider "DCI-backed" - Fetch and cache registry values per registrant via the **Sync DCI Values** action (or the disabled-by-default daily cron) -- Query Civil Registration via ``crvs.dci.is_alive``, - ``crvs.dci.birth_verified``, and the parameterized - ``crvs.dci.has_event('birth'|'death')`` -- Query the Disability Registry via ``dr.dci.has_disability``, - ``dr.dci.assessed``, - ``dr.dci.vision_severe``/``hearing_severe``/``mobility_severe``, and - the parameterized ``dr.dci.severity('Vision'|'Hearing'|'Mobility')`` +- Query Civil Registration via ``r.dci.crvs.is_alive``, + ``r.dci.crvs.birth_verified``, and the parameterized + ``r.dci.crvs.has_event('birth'|'death')`` +- Query the Disability Registry via ``r.dci.dr.has_disability``, + ``r.dci.dr.assessed``, + ``r.dci.dr.vision_severe``/``hearing_severe``/``mobility_severe``, and + the parameterized ``r.dci.dr.severity('Vision'|'Hearing'|'Mobility')`` - Parameterized methods cache one value per (registrant, argument), keyed via ``params_hash``; arguments come from a fixed, pre-synced set @@ -98,7 +98,7 @@ Extension Points (``_dci_metric_handlers`` for simple metrics, ``DCI_METHOD_ACCESSORS`` + ``_compute_method_values`` for parameterized ones) - Create the matching ``spp.cel.variable`` record (external source type, - ``ttl`` cache strategy, ``.dci.`` accessor) + ``ttl`` cache strategy, ``r.dci..`` accessor) CEL Expression Examples ~~~~~~~~~~~~~~~~~~~~~~~ @@ -106,16 +106,16 @@ CEL Expression Examples .. code:: python # Vital statistics verification - crvs.dci.is_alive == true and crvs.dci.birth_verified == true + r.dci.crvs.is_alive == true and r.dci.crvs.birth_verified == true # Parameterized event check - crvs.dci.has_event('death') == true + r.dci.crvs.has_event('death') == true # Disability-based eligibility - dr.dci.has_disability == true and dr.dci.severity('Mobility') >= 3 + r.dci.dr.has_disability == true and r.dci.dr.severity('Mobility') >= 3 # Multi-registry combined criteria - crvs.dci.is_alive == true and dr.dci.has_disability == true and age_years(me.birthdate) >= 18 + r.dci.crvs.is_alive == true and r.dci.dr.has_disability == true and age_years(me.birthdate) >= 18 Dependencies ~~~~~~~~~~~~ diff --git a/spp_dci_indicators/USAGE.md b/spp_dci_indicators/USAGE.md index f870bf5a..52137081 100644 --- a/spp_dci_indicators/USAGE.md +++ b/spp_dci_indicators/USAGE.md @@ -63,8 +63,8 @@ variable in eligibility: Must be alive, severe mobility disability, 18+: ```python -crvs.dci.is_alive == true and -dr.dci.severity('Mobility') >= 3 and +r.dci.crvs.is_alive == true and +r.dci.dr.severity('Mobility') >= 3 and age_years(me.birthdate) >= 18 ``` @@ -73,7 +73,7 @@ age_years(me.birthdate) >= 18 Birth verified, under 5: ```python -crvs.dci.birth_verified == true and +r.dci.crvs.birth_verified == true and age_years(me.birthdate) < 5 ``` @@ -82,28 +82,28 @@ age_years(me.birthdate) < 5 Alive, and either disabled or elderly: ```python -crvs.dci.is_alive == true and -(dr.dci.has_disability == true or age_years(me.birthdate) >= 60) +r.dci.crvs.is_alive == true and +(r.dci.dr.has_disability == true or age_years(me.birthdate) >= 60) ``` ## Variable Reference ### CRVS (Civil Registration and Vital Statistics) -| Accessor | Type | Description | Example | -| -------------------------------------- | ---- | ------------------------- | ------------------------------------- | -| `crvs.dci.is_alive` | bool | no death event in CRVS | `crvs.dci.is_alive == true` | -| `crvs.dci.birth_verified` | bool | birth registration exists | `crvs.dci.birth_verified == true` | -| `crvs.dci.has_event('birth'\|'death')` | bool | parameterized event check | `crvs.dci.has_event('death') == true` | +| Accessor | Type | Description | Example | +| ---------------------------------------- | ---- | ------------------------- | --------------------------------------- | +| `r.dci.crvs.is_alive` | bool | no death event in CRVS | `r.dci.crvs.is_alive == true` | +| `r.dci.crvs.birth_verified` | bool | birth registration exists | `r.dci.crvs.birth_verified == true` | +| `r.dci.crvs.has_event('birth'\|'death')` | bool | parameterized event check | `r.dci.crvs.has_event('death') == true` | ### DR (Disability Registry) -| Accessor | Type | Description | Example | -| ------------------------------------------------------------- | ------ | ---------------------------- | -------------------------------- | -| `dr.dci.has_disability` | bool | any registered disability | `dr.dci.has_disability == true` | -| `dr.dci.assessed` | bool | functional assessment exists | `dr.dci.assessed == true` | -| `dr.dci.vision_severe` / `hearing_severe` / `mobility_severe` | bool | score ≥ 3 | `dr.dci.vision_severe == true` | -| `dr.dci.severity('Vision'\|'Hearing'\|'Mobility')` | number | functional score (0–4) | `dr.dci.severity('Vision') >= 3` | +| Accessor | Type | Description | Example | +| --------------------------------------------------------------- | ------ | ---------------------------- | ---------------------------------- | +| `r.dci.dr.has_disability` | bool | any registered disability | `r.dci.dr.has_disability == true` | +| `r.dci.dr.assessed` | bool | functional assessment exists | `r.dci.dr.assessed == true` | +| `r.dci.dr.vision_severe` / `hearing_severe` / `mobility_severe` | bool | score ≥ 3 | `r.dci.dr.vision_severe == true` | +| `r.dci.dr.severity('Vision'\|'Hearing'\|'Mobility')` | number | functional score (0–4) | `r.dci.dr.severity('Vision') >= 3` | **Severity levels:** 1 no difficulty · 2 some difficulty · 3 a lot of difficulty · 4 cannot do. (Scores depend on the registry returning functional assessment data.) @@ -116,27 +116,27 @@ argument outside the enumerated set simply matches nothing. ### Planned (not yet wired) -`crvs.dci.is_married` and the IBR / Social Registry variables (`ibr.dci.*`, `sr.dci.*`) -exist as variable records but have no fetch handlers yet — they return no data until -implemented. +`r.dci.crvs.is_married` and the IBR / Social Registry variables (`r.dci.ibr.*`, +`r.dci.sr.*`) exist as variable records but have no fetch handlers yet — they return no +data until implemented. ## Common Patterns ```python # Alive and birth-verified -crvs.dci.is_alive == true and crvs.dci.birth_verified == true +r.dci.crvs.is_alive == true and r.dci.crvs.birth_verified == true # Any severe disability -dr.dci.vision_severe == true or dr.dci.hearing_severe == true or dr.dci.mobility_severe == true +r.dci.dr.vision_severe == true or r.dci.dr.hearing_severe == true or r.dci.dr.mobility_severe == true # Same, with explicit thresholds -dr.dci.severity('Vision') >= 3 or dr.dci.severity('Hearing') >= 3 +r.dci.dr.severity('Vision') >= 3 or r.dci.dr.severity('Hearing') >= 3 # Elderly OR disabled -age_years(me.birthdate) >= 60 or dr.dci.has_disability == true +age_years(me.birthdate) >= 60 or r.dci.dr.has_disability == true # Child with verified birth -age_years(me.birthdate) < 18 and crvs.dci.birth_verified == true +age_years(me.birthdate) < 18 and r.dci.crvs.birth_verified == true ``` ## Troubleshooting @@ -150,7 +150,7 @@ DCI variables only match registrants with a **fresh cached value**: 2. Did you run **Sync DCI Values** for those registrants? 3. Has the value's **TTL expired**? Re-sync. 4. Inspect the cache: **Settings → Technical → CEL Domain → Data Management → Data - Values**, filter by Variable Name (e.g. `crvs.dci.is_alive`). + Values**, filter by Variable Name (e.g. `r.dci.crvs.is_alive`). ### Some registrants never get a value @@ -180,7 +180,7 @@ variable exists and is active. (`_dci_metric_handlers` for simple metrics; `DCI_METHOD_ACCESSORS` + `_compute_method_values` for parameterized ones). 2. Add the `spp.cel.variable` record (external source type, `ttl` cache strategy, the - `.dci.` accessor) in `data/indicator_data.xml`. + `r.dci..` accessor) in `data/indicator_data.xml`. 3. Link the variable to a DCI-backed provider and sync. ### How values are stored diff --git a/spp_dci_indicators/__manifest__.py b/spp_dci_indicators/__manifest__.py index b17e34bf..ba87cee2 100644 --- a/spp_dci_indicators/__manifest__.py +++ b/spp_dci_indicators/__manifest__.py @@ -2,7 +2,7 @@ { "name": "OpenSPP DCI Indicators", "summary": "DCI data integration with CEL eligibility expressions", - "version": "19.0.1.0.1", + "version": "19.0.1.0.2", "category": "OpenSPP/Integration", "author": "OpenSPP.org", "website": "https://github.com/OpenSPP/OpenSPP2", @@ -13,7 +13,7 @@ "spp_dci_client_dr", "spp_dci_client_crvs", "spp_dci_client_ibr", - "spp_dci_client_sr", # SRService for sr.dci.* fetch handlers + "spp_dci_client_sr", # SRService for r.dci.sr.* fetch handlers "spp_cel_domain", # Unified variable system "spp_studio", # For variable label and UI fields ], diff --git a/spp_dci_indicators/data/indicator_data.xml b/spp_dci_indicators/data/indicator_data.xml index c8b6e352..b909cff1 100644 --- a/spp_dci_indicators/data/indicator_data.xml +++ b/spp_dci_indicators/data/indicator_data.xml @@ -18,7 +18,7 @@ boolean external - dr.dci.has_disability + r.dci.dr.has_disability individual ttl 86400 @@ -34,7 +34,7 @@ boolean external - dr.dci.vision_severe + r.dci.dr.vision_severe individual ttl 86400 @@ -50,7 +50,7 @@ boolean external - dr.dci.hearing_severe + r.dci.dr.hearing_severe individual ttl 86400 @@ -66,7 +66,7 @@ boolean external - dr.dci.mobility_severe + r.dci.dr.mobility_severe individual ttl 86400 @@ -82,24 +82,24 @@ boolean external - dr.dci.assessed + r.dci.dr.assessed individual ttl 86400 True - + dci.dr.severity DCI: Disability Severity (by type) Functional severity score (0-4) for a disability type, e.g. dr.dci.severity('Vision') >= 3 + >Functional severity score (0-4) for a disability type, e.g. r.dci.dr.severity('Vision') >= 3 number external - dr.dci.severity + r.dci.dr.severity individual ttl 86400 @@ -117,7 +117,7 @@ boolean external - crvs.dci.is_alive + r.dci.crvs.is_alive individual ttl 86400 @@ -133,7 +133,7 @@ boolean external - crvs.dci.birth_verified + r.dci.crvs.birth_verified individual ttl 86400 @@ -149,24 +149,24 @@ boolean external - crvs.dci.is_married + r.dci.crvs.is_married individual ttl 86400 True - + dci.crvs.has_event DCI: Has CRVS Event (by type) Whether a CRVS event of a given type exists, e.g. crvs.dci.has_event('death') + >Whether a CRVS event of a given type exists, e.g. r.dci.crvs.has_event('death') boolean external - crvs.dci.has_event + r.dci.crvs.has_event individual ttl 86400 @@ -184,7 +184,7 @@ boolean external - ibr.dci.has_duplicate + r.dci.ibr.has_duplicate individual ttl 3600 @@ -200,7 +200,7 @@ boolean external - ibr.dci.no_duplicate + r.dci.ibr.no_duplicate individual ttl 3600 @@ -216,7 +216,7 @@ boolean external - ibr.dci.checked + r.dci.ibr.checked individual ttl 3600 @@ -234,7 +234,7 @@ boolean external - sr.dci.is_registered + r.dci.sr.is_registered individual ttl 86400 @@ -250,7 +250,7 @@ number external - sr.dci.program_count + r.dci.sr.program_count individual ttl 86400 @@ -266,7 +266,7 @@ boolean external - sr.dci.has_programs + r.dci.sr.has_programs individual ttl 86400 @@ -280,7 +280,7 @@ number external - sr.dci.household_size + r.dci.sr.household_size individual ttl 86400 @@ -296,7 +296,7 @@ boolean external - sr.dci.is_head_of_household + r.dci.sr.is_head_of_household individual ttl 86400 @@ -312,7 +312,7 @@ boolean external - sr.dci.large_household + r.dci.sr.large_household individual ttl 86400 diff --git a/spp_dci_indicators/models/cel_variable_resolver_dci.py b/spp_dci_indicators/models/cel_variable_resolver_dci.py index 9f9368b3..3f7a443f 100644 --- a/spp_dci_indicators/models/cel_variable_resolver_dci.py +++ b/spp_dci_indicators/models/cel_variable_resolver_dci.py @@ -1,11 +1,11 @@ """Let CEL authors reference DCI variables with their dotted accessor. The base resolver tokenizes with the CEL lexer and only matches single-identifier -variable references; a dotted accessor like ``crvs.dci.is_alive`` would be +variable references; a dotted accessor like ``r.dci.crvs.is_alive`` would be mis-read as field navigation on the query root. This override runs a pre-pass that rewrites any *registered cached-variable dotted accessor* into its ``metric('', me)`` call before normal resolution, so users can write -``crvs.dci.is_alive == true`` and it resolves against the value cache. +``r.dci.crvs.is_alive == true`` and it resolves against the value cache. Only accessors that exactly match a registered ttl/manual variable are touched; ordinary navigation (``me.gender``, ``r.age``) matches no accessor and is left @@ -39,7 +39,7 @@ def expand_expression(self, expression, program_id=None, context_type="group", _ @api.model def _expand_dci_methods(self, expression): """Rewrite parameterized DCI method calls into params-carrying metric() - calls: e.g. dr.dci.severity('Vision') -> metric('dr.dci.severity', me, + calls: e.g. r.dci.dr.severity('Vision') -> metric('r.dci.dr.severity', me, arg='Vision'). The named arg becomes the metric params (params_hash).""" from .dci_cel_fetcher import DCI_METHOD_ACCESSORS diff --git a/spp_dci_indicators/models/data_cache_manager_dci.py b/spp_dci_indicators/models/data_cache_manager_dci.py index f57a4cf6..d618cfde 100644 --- a/spp_dci_indicators/models/data_cache_manager_dci.py +++ b/spp_dci_indicators/models/data_cache_manager_dci.py @@ -22,7 +22,7 @@ def _cache_computed_values(self, variable, computed, period_key): The base method keys the cache on variable.name, but the CEL resolver emits metric('', me) - i.e. the cel_accessor - (e.g. crvs.dci.is_alive), which differs from the name (dci.crvs.is_alive) + (e.g. r.dci.crvs.is_alive), which differs from the name (dci.crvs.is_alive) for DCI variables. Keying on cel_accessor keeps the written value readable by the compiled metric() subquery. """ diff --git a/spp_dci_indicators/models/dci_cel_fetcher.py b/spp_dci_indicators/models/dci_cel_fetcher.py index 40fe14e3..d2d64671 100644 --- a/spp_dci_indicators/models/dci_cel_fetcher.py +++ b/spp_dci_indicators/models/dci_cel_fetcher.py @@ -8,7 +8,7 @@ The result is consumed by the cache manager override (see data_cache_manager_dci.py), which stores the values in spp.data.value, so all CEL consumers read them uniformly. The metric a variable represents is keyed -by its cel_accessor, following the .dci. convention. +by its cel_accessor, following the r.dci.. convention. """ import logging @@ -23,10 +23,10 @@ # Parameterized DCI methods: cel_accessor -> the enumerated argument set + value # type. Each (subject, arg) is cached as a separate spp.data.value row keyed by -# params (params_hash), e.g. dr.dci.severity('Vision') or crvs.dci.has_event('death'). +# params (params_hash), e.g. r.dci.dr.severity('Vision') or r.dci.crvs.has_event('death'). DCI_METHOD_ACCESSORS = { - "dr.dci.severity": {"args": ["Vision", "Hearing", "Mobility"], "value_type": "number"}, - "crvs.dci.has_event": {"args": ["birth", "death"], "value_type": "boolean"}, + "r.dci.dr.severity": {"args": ["Vision", "Hearing", "Mobility"], "value_type": "number"}, + "r.dci.crvs.has_event": {"args": ["birth", "death"], "value_type": "boolean"}, } @@ -159,10 +159,10 @@ def _materialize_method_variable(self, variable, partner_ids): def _compute_method_values(self, accessor, data_source, partner, id_type, id_value): """Return [(params, value), ...] for each enumerated argument of a method.""" args = DCI_METHOD_ACCESSORS[accessor]["args"] - if accessor == "dr.dci.severity": + if accessor == "r.dci.dr.severity": scores = self._dr_status(data_source, partner).get("functional_scores") or {} return [({"arg": t}, scores.get(t) or 0) for t in args] - if accessor == "crvs.dci.has_event": + if accessor == "r.dci.crvs.has_event": svc = self._crvs_service(data_source) out = [] for event in args: @@ -208,23 +208,23 @@ def _dci_metric_handlers(self): """Map a variable's cel_accessor to a handler computing its value. Handlers receive (self, data_source, id_type, id_value) and return the - metric value (or None to skip). Keyed by the .dci. + metric value (or None to skip). Keyed by the r.dci.. accessor convention. """ return { - "crvs.dci.is_alive": self._crvs_is_alive, - "crvs.dci.birth_verified": self._crvs_birth_verified, - "dr.dci.has_disability": self._dr_has_disability, - "dr.dci.assessed": self._dr_assessed, - "dr.dci.vision_severe": self._dr_vision_severe, - "dr.dci.hearing_severe": self._dr_hearing_severe, - "dr.dci.mobility_severe": self._dr_mobility_severe, - "sr.dci.is_registered": self._sr_is_registered, - "sr.dci.program_count": self._sr_program_count, - "sr.dci.has_programs": self._sr_has_programs, - "sr.dci.household_size": self._sr_household_size, - "sr.dci.is_head_of_household": self._sr_is_head_of_household, - "sr.dci.large_household": self._sr_large_household, + "r.dci.crvs.is_alive": self._crvs_is_alive, + "r.dci.crvs.birth_verified": self._crvs_birth_verified, + "r.dci.dr.has_disability": self._dr_has_disability, + "r.dci.dr.assessed": self._dr_assessed, + "r.dci.dr.vision_severe": self._dr_vision_severe, + "r.dci.dr.hearing_severe": self._dr_hearing_severe, + "r.dci.dr.mobility_severe": self._dr_mobility_severe, + "r.dci.sr.is_registered": self._sr_is_registered, + "r.dci.sr.program_count": self._sr_program_count, + "r.dci.sr.has_programs": self._sr_has_programs, + "r.dci.sr.household_size": self._sr_household_size, + "r.dci.sr.is_head_of_household": self._sr_is_head_of_household, + "r.dci.sr.large_household": self._sr_large_household, } # ── CRVS handlers (identifier-based) ────────────────────────────────────── diff --git a/spp_dci_indicators/readme/DESCRIPTION.md b/spp_dci_indicators/readme/DESCRIPTION.md index 57ab6601..bedec570 100644 --- a/spp_dci_indicators/readme/DESCRIPTION.md +++ b/spp_dci_indicators/readme/DESCRIPTION.md @@ -10,11 +10,11 @@ registry per record, so eligibility scales to full populations. making the provider "DCI-backed" - Fetch and cache registry values per registrant via the **Sync DCI Values** action (or the disabled-by-default daily cron) -- Query Civil Registration via `crvs.dci.is_alive`, `crvs.dci.birth_verified`, - and the parameterized `crvs.dci.has_event('birth'|'death')` -- Query the Disability Registry via `dr.dci.has_disability`, `dr.dci.assessed`, - `dr.dci.vision_severe`/`hearing_severe`/`mobility_severe`, and the - parameterized `dr.dci.severity('Vision'|'Hearing'|'Mobility')` +- Query Civil Registration via `r.dci.crvs.is_alive`, `r.dci.crvs.birth_verified`, + and the parameterized `r.dci.crvs.has_event('birth'|'death')` +- Query the Disability Registry via `r.dci.dr.has_disability`, `r.dci.dr.assessed`, + `r.dci.dr.vision_severe`/`hearing_severe`/`mobility_severe`, and the + parameterized `r.dci.dr.severity('Vision'|'Hearing'|'Mobility')` - Parameterized methods cache one value per (registrant, argument), keyed via `params_hash`; arguments come from a fixed, pre-synced set @@ -55,22 +55,22 @@ cache (`spp.data.value`) and the DCI client modules. simple metrics, `DCI_METHOD_ACCESSORS` + `_compute_method_values` for parameterized ones) - Create the matching `spp.cel.variable` record (external source type, `ttl` - cache strategy, `.dci.` accessor) + cache strategy, `r.dci..` accessor) ### CEL Expression Examples ```python # Vital statistics verification -crvs.dci.is_alive == true and crvs.dci.birth_verified == true +r.dci.crvs.is_alive == true and r.dci.crvs.birth_verified == true # Parameterized event check -crvs.dci.has_event('death') == true +r.dci.crvs.has_event('death') == true # Disability-based eligibility -dr.dci.has_disability == true and dr.dci.severity('Mobility') >= 3 +r.dci.dr.has_disability == true and r.dci.dr.severity('Mobility') >= 3 # Multi-registry combined criteria -crvs.dci.is_alive == true and dr.dci.has_disability == true and age_years(me.birthdate) >= 18 +r.dci.crvs.is_alive == true and r.dci.dr.has_disability == true and age_years(me.birthdate) >= 18 ``` ### Dependencies diff --git a/spp_dci_indicators/static/description/index.html b/spp_dci_indicators/static/description/index.html index 55cdc2b9..12912881 100644 --- a/spp_dci_indicators/static/description/index.html +++ b/spp_dci_indicators/static/description/index.html @@ -383,13 +383,13 @@

Key Capabilities

Integration tab, making the provider “DCI-backed”
  • Fetch and cache registry values per registrant via the Sync DCI Values action (or the disabled-by-default daily cron)
    • -
  • Query Civil Registration via crvs.dci.is_alive, -crvs.dci.birth_verified, and the parameterized -crvs.dci.has_event('birth'|'death')
    • -
  • Query the Disability Registry via dr.dci.has_disability, -dr.dci.assessed, -dr.dci.vision_severe/hearing_severe/mobility_severe, and -the parameterized dr.dci.severity('Vision'|'Hearing'|'Mobility')
    • +
  • Query Civil Registration via r.dci.crvs.is_alive, +r.dci.crvs.birth_verified, and the parameterized +r.dci.crvs.has_event('birth'|'death')
    • +
  • Query the Disability Registry via r.dci.dr.has_disability, +r.dci.dr.assessed, +r.dci.dr.vision_severe/hearing_severe/mobility_severe, and +the parameterized r.dci.dr.severity('Vision'|'Hearing'|'Mobility')
  • Parameterized methods cache one value per (registrant, argument), keyed via params_hash; arguments come from a fixed, pre-synced set
    • @@ -458,23 +458,23 @@

    Extension Points

    (_dci_metric_handlers for simple metrics, DCI_METHOD_ACCESSORS + _compute_method_values for parameterized ones)
  • Create the matching spp.cel.variable record (external source type, -ttl cache strategy, <registry>.dci.<metric> accessor)
    • +ttl cache strategy, r.dci.<registry>.<metric> accessor)

    CEL Expression Examples

     # Vital statistics verification
-crvs.dci.is_alive == true and crvs.dci.birth_verified == true
+r.dci.crvs.is_alive == true and r.dci.crvs.birth_verified == true
 
 # Parameterized event check
-crvs.dci.has_event('death') == true
+r.dci.crvs.has_event('death') == true
 
 # Disability-based eligibility
-dr.dci.has_disability == true and dr.dci.severity('Mobility') >= 3
+r.dci.dr.has_disability == true and r.dci.dr.severity('Mobility') >= 3
 
 # Multi-registry combined criteria
-crvs.dci.is_alive == true and dr.dci.has_disability == true and age_years(me.birthdate) >= 18
+r.dci.crvs.is_alive == true and r.dci.dr.has_disability == true and age_years(me.birthdate) >= 18
    diff --git a/spp_dci_indicators/tests/test_dci_cel_dr.py b/spp_dci_indicators/tests/test_dci_cel_dr.py index 8c6b8e55..3fae546e 100644 --- a/spp_dci_indicators/tests/test_dci_cel_dr.py +++ b/spp_dci_indicators/tests/test_dci_cel_dr.py @@ -55,33 +55,33 @@ def _var(self, accessor): ) def test_dr_has_disability_true(self): - var = self._var("dr.dci.has_disability") + var = self._var("r.dci.dr.has_disability") with patch(GET_STATUS, return_value={"has_disability": True, "functional_scores": {}}): result = self.Fetcher.fetch_values(var, [self.partner.id]) self.assertEqual(result, {self.partner.id: True}) def test_dr_has_disability_false_when_no_record(self): - var = self._var("dr.dci.has_disability") + var = self._var("r.dci.dr.has_disability") with patch(GET_STATUS, return_value=None): result = self.Fetcher.fetch_values(var, [self.partner.id]) self.assertEqual(result, {self.partner.id: False}) def test_dr_assessed(self): - var = self._var("dr.dci.assessed") + var = self._var("r.dci.dr.assessed") with patch(GET_STATUS, return_value={"assessment_date": "2024-11-15"}): self.assertEqual(self.Fetcher.fetch_values(var, [self.partner.id]), {self.partner.id: True}) def test_dr_vision_severe_true_at_threshold(self): - var = self._var("dr.dci.vision_severe") + var = self._var("r.dci.dr.vision_severe") with patch(GET_STATUS, return_value={"functional_scores": {"Vision": 3}}): self.assertEqual(self.Fetcher.fetch_values(var, [self.partner.id]), {self.partner.id: True}) def test_dr_vision_severe_false_below_threshold(self): - var = self._var("dr.dci.vision_severe") + var = self._var("r.dci.dr.vision_severe") with patch(GET_STATUS, return_value={"functional_scores": {"Vision": 2}}): self.assertEqual(self.Fetcher.fetch_values(var, [self.partner.id]), {self.partner.id: False}) def test_dr_mobility_severe(self): - var = self._var("dr.dci.mobility_severe") + var = self._var("r.dci.dr.mobility_severe") with patch(GET_STATUS, return_value={"functional_scores": {"Mobility": 4}}): self.assertEqual(self.Fetcher.fetch_values(var, [self.partner.id]), {self.partner.id: True}) diff --git a/spp_dci_indicators/tests/test_dci_cel_end_to_end.py b/spp_dci_indicators/tests/test_dci_cel_end_to_end.py index 7c6d5fc4..0d24d73e 100644 --- a/spp_dci_indicators/tests/test_dci_cel_end_to_end.py +++ b/spp_dci_indicators/tests/test_dci_cel_end_to_end.py @@ -3,10 +3,10 @@ usable in a real CEL expression. Chain exercised: - seeded variable (crvs.dci.is_alive, external, DCI-backed provider) + seeded variable (r.dci.crvs.is_alive, external, DCI-backed provider) -> precompute_variable -> fetcher -> CRVS check_death (mocked) -> cached in spp.data.value (keyed by cel_accessor) - -> compile_expression("crvs.dci.is_alive == true") filters partners. + -> compile_expression("r.dci.crvs.is_alive == true") filters partners. This nails both the name-vs-accessor cache keying and the value encoding the comparison SQL expects. @@ -59,8 +59,8 @@ def _make_registrant(cls, name, id_value): return partner def test_variable_accessor_is_new_format(self): - """Guard: the seeded variable uses the .dci. accessor.""" - self.assertEqual(self.var.cel_accessor, "crvs.dci.is_alive") + """Guard: the seeded variable uses the r.dci.. accessor.""" + self.assertEqual(self.var.cel_accessor, "r.dci.crvs.is_alive") def test_fetch_caches_under_accessor_key(self): """precompute must write spp.data.value keyed by cel_accessor (not name).""" @@ -68,7 +68,7 @@ def test_fetch_caches_under_accessor_key(self): self.CacheMgr.precompute_variable("dci.crvs.is_alive", [self.alive.id]) cached = self.env["spp.data.value"].search( - [("variable_name", "=", "crvs.dci.is_alive"), ("subject_id", "=", self.alive.id)] + [("variable_name", "=", "r.dci.crvs.is_alive"), ("subject_id", "=", self.alive.id)] ) self.assertTrue(cached, "value should be cached under the cel_accessor key") # Booleans are stored as 1/0 so the metric comparison SQL can cast them. @@ -85,7 +85,7 @@ def test_cel_expression_filters_on_cached_dci_value(self): self.CacheMgr.precompute_variable("dci.crvs.is_alive", [self.dead.id]) result = self.CelService.compile_expression( - "crvs.dci.is_alive == true", + "r.dci.crvs.is_alive == true", profile="registry_individuals", base_domain=[("id", "in", [self.alive.id, self.dead.id])], limit=0, @@ -94,5 +94,5 @@ def test_cel_expression_filters_on_cached_dci_value(self): self.assertTrue(result.get("valid"), result.get("error")) matched = self.env["res.partner"].search(result["domain"]) - self.assertIn(self.alive, matched, "alive person should match crvs.dci.is_alive == true") + self.assertIn(self.alive, matched, "alive person should match r.dci.crvs.is_alive == true") self.assertNotIn(self.dead, matched, "dead person should not match") diff --git a/spp_dci_indicators/tests/test_dci_cel_fetcher.py b/spp_dci_indicators/tests/test_dci_cel_fetcher.py index 86c81226..422bdfa7 100644 --- a/spp_dci_indicators/tests/test_dci_cel_fetcher.py +++ b/spp_dci_indicators/tests/test_dci_cel_fetcher.py @@ -47,7 +47,7 @@ def setUpClass(cls): { "name": "zz_test.crvs.is_alive", "label": "DCI: Is Alive", - "cel_accessor": "crvs.dci.is_alive", + "cel_accessor": "r.dci.crvs.is_alive", "source_type": "external", "value_type": "boolean", "external_provider_id": cls.provider.id, @@ -79,7 +79,7 @@ def test_fetch_birth_verified(self): { "name": "zz_test.crvs.birth_verified", "label": "DCI: Birth Verified", - "cel_accessor": "crvs.dci.birth_verified", + "cel_accessor": "r.dci.crvs.birth_verified", "source_type": "external", "value_type": "boolean", "external_provider_id": self.provider.id, @@ -101,7 +101,7 @@ def test_fetch_unknown_accessor_returns_empty(self): { "name": "zz_test.crvs.unknown", "label": "Unknown", - "cel_accessor": "crvs.dci.not_a_metric", + "cel_accessor": "r.dci.crvs.not_a_metric", "source_type": "external", "value_type": "boolean", "external_provider_id": self.provider.id, @@ -143,7 +143,7 @@ def test_sync_for_partners_caches_values(self): count = self.Fetcher.sync_for_partners([self.partner.id], variables=self.var_is_alive) self.assertGreaterEqual(count, 1) cached = self.env["spp.data.value"].search( - [("variable_name", "=", "crvs.dci.is_alive"), ("subject_id", "=", self.partner.id)] + [("variable_name", "=", "r.dci.crvs.is_alive"), ("subject_id", "=", self.partner.id)] ) self.assertTrue(cached) self.assertEqual(cached.value_json, {"value": 1}) diff --git a/spp_dci_indicators/tests/test_dci_cel_fetcher_errors.py b/spp_dci_indicators/tests/test_dci_cel_fetcher_errors.py index 58e21b0e..b8e89fa1 100644 --- a/spp_dci_indicators/tests/test_dci_cel_fetcher_errors.py +++ b/spp_dci_indicators/tests/test_dci_cel_fetcher_errors.py @@ -74,7 +74,7 @@ def test_fetch_values_handler_returns_none_skips_subject(self): { "name": "zz_test_edge.handler_none", "label": "Handler None Test", - "cel_accessor": "dr.dci.has_disability", + "cel_accessor": "r.dci.dr.has_disability", "source_type": "external", "value_type": "boolean", "external_provider_id": self.provider.id, @@ -100,7 +100,7 @@ def test_fetch_values_no_data_source_returns_empty(self): { "name": "zz_test_edge.no_ds_var", "label": "No DS var", - "cel_accessor": "dr.dci.assessed", + "cel_accessor": "r.dci.dr.assessed", "source_type": "external", "value_type": "boolean", "external_provider_id": provider_no_ds.id, @@ -248,20 +248,20 @@ def test_unknown_event_arg_is_skipped(self): """ from odoo.addons.spp_dci_indicators.models.dci_cel_fetcher import DCI_METHOD_ACCESSORS - original_args = list(DCI_METHOD_ACCESSORS["crvs.dci.has_event"]["args"]) - DCI_METHOD_ACCESSORS["crvs.dci.has_event"]["args"] = ["birth", "unknown_event", "death"] + original_args = list(DCI_METHOD_ACCESSORS["r.dci.crvs.has_event"]["args"]) + DCI_METHOD_ACCESSORS["r.dci.crvs.has_event"]["args"] = ["birth", "unknown_event", "death"] try: with patch(VERIFY_BIRTH, return_value={"x": 1}), patch(CHECK_DEATH, return_value=False): partner = self.env["res.partner"].browse([]) pairs = self.Fetcher._compute_method_values( - "crvs.dci.has_event", + "r.dci.crvs.has_event", self.dci_source, partner, "NID", "VAL-EDGE", ) finally: - DCI_METHOD_ACCESSORS["crvs.dci.has_event"]["args"] = original_args + DCI_METHOD_ACCESSORS["r.dci.crvs.has_event"]["args"] = original_args arg_keys = [p[0]["arg"] for p in pairs] self.assertIn("birth", arg_keys) @@ -300,7 +300,7 @@ def setUpClass(cls): { "name": "zz_cron_edge.crvs.is_alive", "label": "DCI: Is Alive (cron edge)", - "cel_accessor": "crvs.dci.is_alive", + "cel_accessor": "r.dci.crvs.is_alive", "source_type": "external", "value_type": "boolean", "external_provider_id": cls.provider.id, @@ -330,7 +330,7 @@ def test_cron_syncs_registrants_in_loop(self): # Use a very large batch_size so the loop runs exactly once. self.Fetcher.cron_sync_all_registrants(batch_size=10000) cached = self.env["spp.data.value"].search( - [("variable_name", "=", "crvs.dci.is_alive"), ("subject_id", "=", self.partner.id)] + [("variable_name", "=", "r.dci.crvs.is_alive"), ("subject_id", "=", self.partner.id)] ) self.assertTrue(cached) @@ -362,13 +362,13 @@ def setUpClass(cls): "dci_data_source_id": cls.dci_source.id, } ) - # crvs.dci.is_alive is NOT in DCI_METHOD_ACCESSORS, so it goes through + # r.dci.crvs.is_alive is NOT in DCI_METHOD_ACCESSORS, so it goes through # the precompute_variable path in sync_for_partners. cls.var_is_alive = cls.env["spp.cel.variable"].create( { "name": "zz_precomp_edge.crvs.is_alive", "label": "DCI: Is Alive (precompute edge)", - "cel_accessor": "crvs.dci.is_alive", + "cel_accessor": "r.dci.crvs.is_alive", "source_type": "external", "value_type": "boolean", "external_provider_id": cls.provider.id, diff --git a/spp_dci_indicators/tests/test_dci_cel_methods.py b/spp_dci_indicators/tests/test_dci_cel_methods.py index 3a2fa9f6..5f02e55b 100644 --- a/spp_dci_indicators/tests/test_dci_cel_methods.py +++ b/spp_dci_indicators/tests/test_dci_cel_methods.py @@ -60,12 +60,12 @@ def _provider(name, code, registry_type): # ── resolver rewrite ───────────────────────────────────────────────────── def test_resolver_rewrites_severity_call(self): - out = self.Resolver.expand_expression("dr.dci.severity('Vision') >= 3", context_type="individual") - self.assertIn("metric('dr.dci.severity', me, arg='Vision')", out["expression"]) + out = self.Resolver.expand_expression("r.dci.dr.severity('Vision') >= 3", context_type="individual") + self.assertIn("metric('r.dci.dr.severity', me, arg='Vision')", out["expression"]) def test_resolver_rewrites_has_event_call(self): - out = self.Resolver.expand_expression("crvs.dci.has_event('death') == true", context_type="individual") - self.assertIn("metric('crvs.dci.has_event', me, arg='death')", out["expression"]) + out = self.Resolver.expand_expression("r.dci.crvs.has_event('death') == true", context_type="individual") + self.assertIn("metric('r.dci.crvs.has_event', me, arg='death')", out["expression"]) # ── materialization ────────────────────────────────────────────────────── @@ -73,7 +73,7 @@ def test_materialize_severity_one_row_per_type(self): with patch(GET_STATUS, return_value={"functional_scores": {"Vision": 4, "Hearing": 1}}): n = self.Fetcher.sync_for_partners([self.partner.id], variables=self.sev_var) self.assertEqual(n, 3) # Vision, Hearing, Mobility - rows = self.DV.search([("variable_name", "=", "dr.dci.severity"), ("subject_id", "=", self.partner.id)]) + rows = self.DV.search([("variable_name", "=", "r.dci.dr.severity"), ("subject_id", "=", self.partner.id)]) self.assertEqual(len(rows), 3) def test_materialize_has_event_one_row_per_event(self): @@ -97,11 +97,11 @@ def _match(self, expr): def test_e2e_severity_discriminates_by_arg(self): with patch(GET_STATUS, return_value={"functional_scores": {"Vision": 4, "Hearing": 1}}): self.Fetcher.sync_for_partners([self.partner.id], variables=self.sev_var) - self.assertIn(self.partner, self._match("dr.dci.severity('Vision') >= 3")) - self.assertNotIn(self.partner, self._match("dr.dci.severity('Hearing') >= 3")) + self.assertIn(self.partner, self._match("r.dci.dr.severity('Vision') >= 3")) + self.assertNotIn(self.partner, self._match("r.dci.dr.severity('Hearing') >= 3")) def test_e2e_has_event_death(self): with patch(VERIFY_BIRTH, return_value=None), patch(CHECK_DEATH, return_value=True): self.Fetcher.sync_for_partners([self.partner.id], variables=self.event_var) - self.assertIn(self.partner, self._match("crvs.dci.has_event('death') == true")) - self.assertNotIn(self.partner, self._match("crvs.dci.has_event('birth') == true")) + self.assertIn(self.partner, self._match("r.dci.crvs.has_event('death') == true")) + self.assertNotIn(self.partner, self._match("r.dci.crvs.has_event('birth') == true")) diff --git a/spp_dci_indicators/tests/test_dci_cel_sr.py b/spp_dci_indicators/tests/test_dci_cel_sr.py index d2a8b4bc..9eb58f50 100644 --- a/spp_dci_indicators/tests/test_dci_cel_sr.py +++ b/spp_dci_indicators/tests/test_dci_cel_sr.py @@ -1,7 +1,7 @@ # Part of OpenSPP. See LICENSE file for full copyright and licensing details. """Tests for the SR (Social Registry) DCI fetch handlers. -All six sr.dci.* metrics derive from a single person record returned by +All six r.dci.sr.* metrics derive from a single person record returned by SRService.search_person (identifiers -> registration, enrolled_programs, household_info). The service is mocked here. @@ -73,63 +73,63 @@ def _fetch(self, accessor, person, value_type="boolean"): # -- is_registered --------------------------------------------------------- def test_sr_is_registered_true(self): - result = self._fetch("sr.dci.is_registered", {"id": "EXT-1", "name": "SR Person"}) + result = self._fetch("r.dci.sr.is_registered", {"id": "EXT-1", "name": "SR Person"}) self.assertEqual(result, {self.partner.id: True}) def test_sr_is_registered_false_when_not_found(self): - result = self._fetch("sr.dci.is_registered", None) + result = self._fetch("r.dci.sr.is_registered", None) self.assertEqual(result, {self.partner.id: False}) # -- programmes ------------------------------------------------------------ def test_sr_program_count(self): person = {"enrolled_programs": [{"programme_name": "A"}, {"programme_name": "B"}]} - result = self._fetch("sr.dci.program_count", person, value_type="number") + result = self._fetch("r.dci.sr.program_count", person, value_type="number") self.assertEqual(result, {self.partner.id: 2}) def test_sr_program_count_zero_without_enrollments(self): - result = self._fetch("sr.dci.program_count", {"id": "EXT-1"}, value_type="number") + result = self._fetch("r.dci.sr.program_count", {"id": "EXT-1"}, value_type="number") self.assertEqual(result, {self.partner.id: 0}) def test_sr_program_count_zero_when_not_found(self): """Not found in SR -> 0 programs (a value, so the cache stays complete).""" - result = self._fetch("sr.dci.program_count", None, value_type="number") + result = self._fetch("r.dci.sr.program_count", None, value_type="number") self.assertEqual(result, {self.partner.id: 0}) def test_sr_has_programs(self): person = {"enrolled_programs": [{"programme_name": "A"}]} - self.assertEqual(self._fetch("sr.dci.has_programs", person), {self.partner.id: True}) - self.assertEqual(self._fetch("sr.dci.has_programs", {"id": "X"}), {self.partner.id: False}) + self.assertEqual(self._fetch("r.dci.sr.has_programs", person), {self.partner.id: True}) + self.assertEqual(self._fetch("r.dci.sr.has_programs", {"id": "X"}), {self.partner.id: False}) # -- household ------------------------------------------------------------- def test_sr_household_size(self): person = {"household_info": {"household_size": 4, "is_household_head": False}} - result = self._fetch("sr.dci.household_size", person, value_type="number") + result = self._fetch("r.dci.sr.household_size", person, value_type="number") self.assertEqual(result, {self.partner.id: 4}) def test_sr_household_size_zero_without_household(self): """Registered but household-less -> size 0 (complete cache).""" - result = self._fetch("sr.dci.household_size", {"id": "EXT-1"}, value_type="number") + result = self._fetch("r.dci.sr.household_size", {"id": "EXT-1"}, value_type="number") self.assertEqual(result, {self.partner.id: 0}) def test_sr_is_head_of_household(self): person = {"household_info": {"household_size": 3, "is_household_head": True}} - self.assertEqual(self._fetch("sr.dci.is_head_of_household", person), {self.partner.id: True}) + self.assertEqual(self._fetch("r.dci.sr.is_head_of_household", person), {self.partner.id: True}) def test_sr_is_head_false_without_household(self): - result = self._fetch("sr.dci.is_head_of_household", {"id": "EXT-1"}) + result = self._fetch("r.dci.sr.is_head_of_household", {"id": "EXT-1"}) self.assertEqual(result, {self.partner.id: False}) def test_sr_large_household_above_threshold(self): person = {"household_info": {"household_size": 6}} - self.assertEqual(self._fetch("sr.dci.large_household", person), {self.partner.id: True}) + self.assertEqual(self._fetch("r.dci.sr.large_household", person), {self.partner.id: True}) def test_sr_large_household_at_threshold_is_false(self): # The seeded variable documents "more than 5 members" person = {"household_info": {"household_size": 5}} - self.assertEqual(self._fetch("sr.dci.large_household", person), {self.partner.id: False}) + self.assertEqual(self._fetch("r.dci.sr.large_household", person), {self.partner.id: False}) def test_sr_large_household_false_without_household(self): - result = self._fetch("sr.dci.large_household", {"id": "EXT-1"}) + result = self._fetch("r.dci.sr.large_household", {"id": "EXT-1"}) self.assertEqual(result, {self.partner.id: False}) diff --git a/spp_dci_indicators/tests/test_indicator_data.py b/spp_dci_indicators/tests/test_indicator_data.py index 00784fe7..e9d968b5 100644 --- a/spp_dci_indicators/tests/test_indicator_data.py +++ b/spp_dci_indicators/tests/test_indicator_data.py @@ -31,7 +31,7 @@ def test_dr_variables_exist(self): self.assertTrue(var, "dci.dr.has_disability should exist") self.assertEqual(var.value_type, "boolean") self.assertEqual(var.source_type, "external") - self.assertEqual(var.cel_accessor, "dr.dci.has_disability") + self.assertEqual(var.cel_accessor, "r.dci.dr.has_disability") self.assertEqual(var.applies_to, "individual") self.assertTrue(var.is_system) @@ -77,7 +77,7 @@ def test_all_dci_variables_count(self): variables = Variable.search([("category_id", "=", category.id)]) # 5 DR + 3 CRVS + 3 IBR + 6 SR + 2 parameterized methods - # (dr.dci.severity, crvs.dci.has_event) = 19 variables + # (r.dci.dr.severity, r.dci.crvs.has_event) = 19 variables self.assertEqual(len(variables), 19, "Should have 19 DCI variables") def test_dci_variables_have_labels(self): From 8c563bfbe63fa4e047f8f20323e60cf819a45203 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Mon, 8 Jun 2026 20:45:29 +0800 Subject: [PATCH 19/22] feat(spp_dci_client): drop unsupported Basic Authentication option Basic auth was a non-functional stub: the auth_type selection offered it, but get_headers() raised 'not yet implemented' and no username/password fields existed, so picking it was a dead end. Remove the 'basic' selection option and the dead handler branch. Tests covering the stub are removed; the sender-id constraint test now exercises bearer auth instead. --- spp_dci_client/__manifest__.py | 2 +- spp_dci_client/models/data_source.py | 4 ---- spp_dci_client/tests/test_data_source.py | 5 +++-- spp_dci_client/tests/test_data_source_http.py | 13 ------------- 4 files changed, 4 insertions(+), 20 deletions(-) diff --git a/spp_dci_client/__manifest__.py b/spp_dci_client/__manifest__.py index 9d5111ca..457a4066 100644 --- a/spp_dci_client/__manifest__.py +++ b/spp_dci_client/__manifest__.py @@ -2,7 +2,7 @@ { "name": "OpenSPP DCI Client", "summary": "Base DCI client infrastructure with OAuth2 and data source management", - "version": "19.0.2.0.0", + "version": "19.0.2.0.1", "category": "OpenSPP/Integration", "author": "OpenSPP.org", "website": "https://github.com/OpenSPP/OpenSPP2", diff --git a/spp_dci_client/models/data_source.py b/spp_dci_client/models/data_source.py index 8a21656b..71ff54fb 100644 --- a/spp_dci_client/models/data_source.py +++ b/spp_dci_client/models/data_source.py @@ -48,7 +48,6 @@ class DCIDataSource(models.Model): [ ("none", "None"), ("bearer", "Bearer Token"), - ("basic", "Basic Authentication"), ("oauth2", "OAuth2"), ], default="oauth2", @@ -486,9 +485,6 @@ def get_headers(self, force_refresh_token=False): if not self.bearer_token: raise UserError(_("Bearer token is not configured for this data source.")) headers["Authorization"] = f"Bearer {self.bearer_token}" - elif self.auth_type == "basic": - # Basic auth would require username/password fields (not in current spec) - raise UserError(_("Basic authentication is not yet implemented.")) return headers diff --git a/spp_dci_client/tests/test_data_source.py b/spp_dci_client/tests/test_data_source.py index b8b069e5..118ea632 100644 --- a/spp_dci_client/tests/test_data_source.py +++ b/spp_dci_client/tests/test_data_source.py @@ -238,14 +238,15 @@ def test_oauth2_secret_set_via_display_field_on_write(self): def test_sender_id_required_for_auth(self): """Test sender ID is required when auth_type is not 'none'""" - # Should fail for basic auth without sender_id + # Should fail for bearer auth without sender_id with self.assertRaises(ValidationError) as cm: self.DataSource.create( { "name": "Test CRVS", "code": "test_crvs", "base_url": "https://crvs.example.org/api", - "auth_type": "basic", + "auth_type": "bearer", + "bearer_token": "tok", } ) self.assertIn("sender id", str(cm.exception).lower()) diff --git a/spp_dci_client/tests/test_data_source_http.py b/spp_dci_client/tests/test_data_source_http.py index c8fcb591..e93203fb 100644 --- a/spp_dci_client/tests/test_data_source_http.py +++ b/spp_dci_client/tests/test_data_source_http.py @@ -142,19 +142,6 @@ def test_get_headers_bearer(self): ) self.assertEqual(ds.get_headers()["Authorization"], "Bearer btok") - def test_get_headers_basic_not_implemented(self): - ds = self.DataSource.create( - { - "name": "Basic DS", - "code": "basic_ds", - "base_url": "https://dci.example.org/api", - "auth_type": "basic", - "our_sender_id": "openspp.test", - } - ) - with self.assertRaises(UserError): - ds.get_headers() - # --- test_connection ----------------------------------------------------- def test_connection_success_activates(self): From 57a8505ab2de4e53f7ea2bfc7099b8c7861d4e8f Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Mon, 8 Jun 2026 21:01:25 +0800 Subject: [PATCH 20/22] feat(spp_dci_server): accept OAuth2 access tokens for bearer auth The DCI server only string-matched a static bearer-token list (dci.api_tokens), so the client's OAuth2 client-credentials support had nothing to authenticate against. Extend verify_bearer_token to also accept an OAuth2 access token: a JWT issued by spp_api_v2 (POST /api_v2/oauth/token) is validated with spp_api_v2's own verifier (HS256 signature, iss/aud, expiry), then the client_id claim must resolve to an active spp.api.client. Static bearer tokens keep working unchanged; the JWT path is an additive fallback. spp_dci_server already depends on spp_api_v2, so the verifier is reused, not duplicated. End-to-end: point the client data source's oauth2_token_url at the server's /api_v2/oauth/token with an spp.api.client's credentials. --- spp_dci_server/README.rst | 4 + spp_dci_server/__manifest__.py | 2 +- spp_dci_server/middleware/signature.py | 59 +++++++++-- spp_dci_server/readme/DESCRIPTION.md | 1 + spp_dci_server/static/description/index.html | 4 + .../tests/test_bearer_middleware.py | 100 ++++++++++++++++++ 6 files changed, 160 insertions(+), 10 deletions(-) diff --git a/spp_dci_server/README.rst b/spp_dci_server/README.rst index 73c4acb6..2832ca2b 100644 --- a/spp_dci_server/README.rst +++ b/spp_dci_server/README.rst @@ -35,6 +35,10 @@ Key Capabilities ``/dci_api/v1`` with automatic OpenAPI documentation - **HTTP Signature Verification**: Validates inbound requests using Ed25519/RSA signatures against sender public keys +- **Bearer Authentication**: Accepts either a static token (system + parameter ``dci.api_tokens``) or an OAuth2 client-credentials access + token (a JWT issued by ``spp_api_v2`` at ``POST /api_v2/oauth/token``, + validated against an active ``spp.api.client``) - **Async Transaction Processing**: Queues search, subscribe, and unsubscribe operations for background processing with automatic callbacks diff --git a/spp_dci_server/__manifest__.py b/spp_dci_server/__manifest__.py index 9f64bde3..4ee8dfa7 100644 --- a/spp_dci_server/__manifest__.py +++ b/spp_dci_server/__manifest__.py @@ -1,7 +1,7 @@ { # pylint: disable=pointless-statement "name": "OpenSPP DCI Server", "summary": "DCI API server infrastructure with FastAPI routers", - "version": "19.0.2.0.1", + "version": "19.0.2.0.2", "category": "OpenSPP/Integration", "author": "OpenSPP.org", "website": "https://github.com/OpenSPP/OpenSPP2", diff --git a/spp_dci_server/middleware/signature.py b/spp_dci_server/middleware/signature.py index ee780e97..54a6bb04 100644 --- a/spp_dci_server/middleware/signature.py +++ b/spp_dci_server/middleware/signature.py @@ -246,6 +246,40 @@ async def verify_dci_signature( _empty_tokens_warning_logged = False +def _validate_oauth2_jwt(env: Environment, token: str) -> str | None: + """Return the client_id when ``token`` is a valid OAuth2 access token. + + Lets a DCI caller authenticate with an OAuth2 client-credentials JWT issued + by spp_api_v2 (``POST /api_v2/oauth/token``) instead of a static bearer + token. The JWT is validated with spp_api_v2's own verifier (HS256 signature, + ``iss``/``aud``, expiry), then the ``client_id`` claim must resolve to an + active ``spp.api.client``. + + Returns the client_id on success, or ``None`` on any failure (bad signature, + expired, wrong issuer/audience, secret unset, unknown/inactive client, or a + non-JWT opaque token) so the caller can fall back to other auth paths. + """ + try: + from odoo.addons.spp_api_v2.middleware.auth import _validate_jwt_token + except ImportError: + return None + try: + payload = _validate_jwt_token(env, token) + except Exception: + # Not a valid spp_api_v2 JWT — fall back to static-token handling. + return None + client_id = payload.get("client_id") + if not client_id: + return None + # nosemgrep: odoo-sudo-without-context + client = env["spp.api.client"].sudo().search([("client_id", "=", client_id), ("active", "=", True)], limit=1) + if not client: + _logger.warning("OAuth2 JWT names unknown/inactive client_id: %s", client_id) + return None + _logger.debug("DCI request authenticated via OAuth2 JWT for client %s", client_id) + return client_id + + async def verify_bearer_token( env: Annotated[Environment, Depends(odoo_env)], authorization: Annotated[str | None, Header()] = None, @@ -332,15 +366,22 @@ async def verify_bearer_token( for candidate in accepted_tokens: if hmac.compare_digest(token, candidate): matched = True - if not matched: - _logger.warning("DCI request has invalid Bearer token") - raise DCIHTTPException( - status_code=status.HTTP_401_UNAUTHORIZED, - error_message="Invalid Bearer token", - error_code="err.auth.invalid_token", - headers={"WWW-Authenticate": "Bearer"}, - ) - _logger.debug("Bearer token validated against configured tokens") + if matched: + _logger.debug("Bearer token validated against configured tokens") + return token + # Not a configured static token — it may be an OAuth2 access token. + if _validate_oauth2_jwt(env, token): + return token + _logger.warning("DCI request has invalid Bearer token") + raise DCIHTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + error_message="Invalid Bearer token", + error_code="err.auth.invalid_token", + headers={"WWW-Authenticate": "Bearer"}, + ) + + # No static tokens configured: still accept a valid OAuth2 access token. + if _validate_oauth2_jwt(env, token): return token # nosemgrep: odoo-timing-attack-password # not a token compare; matches a config flag value diff --git a/spp_dci_server/readme/DESCRIPTION.md b/spp_dci_server/readme/DESCRIPTION.md index ca469ea9..e798daa7 100644 --- a/spp_dci_server/readme/DESCRIPTION.md +++ b/spp_dci_server/readme/DESCRIPTION.md @@ -4,6 +4,7 @@ DCI API server infrastructure for receiving and processing Digital Convergence I - **FastAPI Endpoints**: Exposes DCI-compliant REST API at `/dci_api/v1` with automatic OpenAPI documentation - **HTTP Signature Verification**: Validates inbound requests using Ed25519/RSA signatures against sender public keys +- **Bearer Authentication**: Accepts either a static token (system parameter `dci.api_tokens`) or an OAuth2 client-credentials access token (a JWT issued by `spp_api_v2` at `POST /api_v2/oauth/token`, validated against an active `spp.api.client`) - **Async Transaction Processing**: Queues search, subscribe, and unsubscribe operations for background processing with automatic callbacks - **Event Subscriptions**: Manages external system subscriptions to registry events (registration, update, delete) with notification delivery - **JWKS Distribution**: Publishes server public keys at `/.well-known/jwks.json` for signature verification by clients diff --git a/spp_dci_server/static/description/index.html b/spp_dci_server/static/description/index.html index 960c9c7e..a7c6cfbc 100644 --- a/spp_dci_server/static/description/index.html +++ b/spp_dci_server/static/description/index.html @@ -382,6 +382,10 @@

    Key Capabilities

    /dci_api/v1 with automatic OpenAPI documentation
  • HTTP Signature Verification: Validates inbound requests using Ed25519/RSA signatures against sender public keys
    • +
  • Bearer Authentication: Accepts either a static token (system +parameter dci.api_tokens) or an OAuth2 client-credentials access +token (a JWT issued by spp_api_v2 at POST /api_v2/oauth/token, +validated against an active spp.api.client)
  • Async Transaction Processing: Queues search, subscribe, and unsubscribe operations for background processing with automatic callbacks
    • diff --git a/spp_dci_server/tests/test_bearer_middleware.py b/spp_dci_server/tests/test_bearer_middleware.py index 0596e75c..ecccfc9c 100644 --- a/spp_dci_server/tests/test_bearer_middleware.py +++ b/spp_dci_server/tests/test_bearer_middleware.py @@ -9,6 +9,8 @@ """ import asyncio +import os +from datetime import datetime, timedelta from odoo.tests import tagged @@ -16,6 +18,10 @@ from .common import DCIServerCommon +# 48-char high-entropy secret so spp_api_v2's _validate_jwt_secret_strength +# (>=32 chars, entropy >= 3.0) accepts it. +_TEST_JWT_SECRET = "Zx9Kq2Lm7Pw4Rt6Yv1Nb8Hc3Jd5Fg0SaUeWiOqTzXyMnBvCr" + def _run(coro): loop = asyncio.new_event_loop() @@ -191,3 +197,97 @@ def test_security_flags_default_to_fail_closed(self): safe_value, f"{key} must default to {safe_value!r} (fail-closed)", ) + + +@tagged("post_install", "-at_install") +class TestOAuth2BearerToken(DCIServerCommon): + """The bearer dependency also accepts OAuth2 access tokens (spp_api_v2 + JWTs) so DCI callers can authenticate with client-credentials, not only + static tokens.""" + + def setUp(self): + super().setUp() + from odoo.addons.spp_dci_server.middleware import signature as sig_module + + self.verify_bearer_token = sig_module.verify_bearer_token + sig_module._bearer_bypass_warning_logged = False + sig_module._empty_tokens_warning_logged = False + self.ICP = self.env["ir.config_parameter"].sudo() + # Sign with the secret the verifier will use (env var wins over param). + self.secret = os.environ.get("OPENSPP_JWT_SECRET") or _TEST_JWT_SECRET + if not os.environ.get("OPENSPP_JWT_SECRET"): + self.ICP.set_param("spp_api_v2.jwt_secret", self.secret) + self.client = self.env["spp.api.client"].create( + { + "name": "DCI OAuth Test Client", + "partner_id": self.test_partner.id, + "organization_type_id": self.org_type_government.id, + } + ) + + def _call(self, authorization=None): + return _run(self.verify_bearer_token(self.env, authorization)) + + def _mint_jwt(self, client_id=None, expires_in_hours=1, secret=None): + import jwt + + now = datetime.utcnow() + payload = { + "iss": "openspp-api-v2", + "aud": "openspp", + "sub": client_id or self.client.client_id, + "client_id": client_id or self.client.client_id, + "iat": now, + "exp": now + timedelta(hours=expires_in_hours), + "scopes": [], + } + return jwt.encode(payload, secret or self.secret, algorithm="HS256") + + def test_valid_oauth2_jwt_accepted(self): + """A valid OAuth2 JWT is accepted even with no static tokens and + dci.api_tokens_required=true.""" + self.ICP.set_param("dci.api_tokens", "") + self.ICP.set_param("dci.api_tokens_required", "true") + token = self._mint_jwt() + self.assertEqual(self._call(f"Bearer {token}"), token) + + def test_oauth2_jwt_accepted_alongside_nonmatching_static(self): + """A valid OAuth2 JWT is accepted even when a (non-matching) static + token list is configured.""" + self.ICP.set_param("dci.api_tokens", "some-other-static-token") + token = self._mint_jwt() + self.assertEqual(self._call(f"Bearer {token}"), token) + + def test_static_token_still_accepted(self): + """Regression: configured static tokens keep working.""" + self.ICP.set_param("dci.api_tokens", "static-abc") + self.assertEqual(self._call("Bearer static-abc"), "static-abc") + + def test_expired_oauth2_jwt_rejected(self): + self.ICP.set_param("dci.api_tokens", "") + token = self._mint_jwt(expires_in_hours=-1) + with self.assertRaises(HTTPException) as ctx: + self._call(f"Bearer {token}") + self.assertEqual(ctx.exception.status_code, 401) + + def test_invalid_signature_jwt_rejected(self): + self.ICP.set_param("dci.api_tokens", "") + token = self._mint_jwt(secret="wrong-but-long-enough-secret-aB3dE6fH9jK2mN5pQ8rT1v") + with self.assertRaises(HTTPException) as ctx: + self._call(f"Bearer {token}") + self.assertEqual(ctx.exception.status_code, 401) + + def test_oauth2_jwt_for_inactive_client_rejected(self): + self.ICP.set_param("dci.api_tokens", "") + self.client.active = False + token = self._mint_jwt() + with self.assertRaises(HTTPException) as ctx: + self._call(f"Bearer {token}") + self.assertEqual(ctx.exception.status_code, 401) + + def test_oauth2_jwt_unknown_client_rejected(self): + self.ICP.set_param("dci.api_tokens", "") + token = self._mint_jwt(client_id="no-such-client-id") + with self.assertRaises(HTTPException) as ctx: + self._call(f"Bearer {token}") + self.assertEqual(ctx.exception.status_code, 401) From 2e4063fe12aff04fccc8c2cac666cac4caec18f8 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Mon, 8 Jun 2026 21:33:36 +0800 Subject: [PATCH 21/22] docs(spp_dci_server): document the endpoint-user requirement for SR search Social Registry search is access-gated on spp_registry.group_registry_viewer, evaluated against the DCI FastAPI endpoint's user. The endpoint ships as the public user, which lacks that group, so a fresh deployment rejects SR searches until a registry-viewer service user is assigned to the endpoint. Document the setup in spp_dci_server (Configuration) and cross-reference it from spp_dci_server_social (Security). --- spp_dci_server/README.rst | 21 +++++++++++++++++++ spp_dci_server/__manifest__.py | 2 +- spp_dci_server/readme/DESCRIPTION.md | 18 ++++++++++++++++ spp_dci_server_social/README.rst | 9 +++++++- spp_dci_server_social/__manifest__.py | 2 +- spp_dci_server_social/readme/DESCRIPTION.md | 4 +++- .../static/description/index.html | 8 ++++++- 7 files changed, 59 insertions(+), 5 deletions(-) diff --git a/spp_dci_server/README.rst b/spp_dci_server/README.rst index 2832ca2b..162aa9b6 100644 --- a/spp_dci_server/README.rst +++ b/spp_dci_server/README.rst @@ -90,6 +90,27 @@ Server signing keys are automatically generated and activated on installation. To manage keys manually, use the technical interface for ``spp.dci.server.key``. +Endpoint user (required for Social Registry search) +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +The DCI FastAPI endpoint ships configured to run as the **public user**. +Serving Social Registry searches additionally requires the endpoint's +user to hold the ``spp_registry.group_registry_viewer`` group (the +search is access-gated). With the default public user, SR searches are +rejected with an access error. + +Assign a dedicated service user before going live: + +1. Create a user (e.g. *DCI Endpoint Service*) and grant it **Registry: + Viewer** (``spp_registry.group_registry_viewer``). +2. Open the DCI ``fastapi.endpoint`` record (the ``dci_api`` app, root + path ``/dci_api/v1``) and set its **User** to that service account. + +Authentication of the *caller* is independent of this: requests are +authenticated with a static bearer token (``dci.api_tokens``) or an +OAuth2 access token. The endpoint user only sets the Odoo permission +context the search executes under. + UI Location ~~~~~~~~~~~ diff --git a/spp_dci_server/__manifest__.py b/spp_dci_server/__manifest__.py index 4ee8dfa7..ebfd938c 100644 --- a/spp_dci_server/__manifest__.py +++ b/spp_dci_server/__manifest__.py @@ -1,7 +1,7 @@ { # pylint: disable=pointless-statement "name": "OpenSPP DCI Server", "summary": "DCI API server infrastructure with FastAPI routers", - "version": "19.0.2.0.2", + "version": "19.0.2.0.3", "category": "OpenSPP/Integration", "author": "OpenSPP.org", "website": "https://github.com/OpenSPP/OpenSPP2", diff --git a/spp_dci_server/readme/DESCRIPTION.md b/spp_dci_server/readme/DESCRIPTION.md index e798daa7..b893d469 100644 --- a/spp_dci_server/readme/DESCRIPTION.md +++ b/spp_dci_server/readme/DESCRIPTION.md @@ -32,6 +32,24 @@ After installing: Server signing keys are automatically generated and activated on installation. To manage keys manually, use the technical interface for `spp.dci.server.key`. +#### Endpoint user (required for Social Registry search) + +The DCI FastAPI endpoint ships configured to run as the **public user**. Serving +Social Registry searches additionally requires the endpoint's user to hold the +`spp_registry.group_registry_viewer` group (the search is access-gated). With the +default public user, SR searches are rejected with an access error. + +Assign a dedicated service user before going live: + +1. Create a user (e.g. *DCI Endpoint Service*) and grant it **Registry: Viewer** + (`spp_registry.group_registry_viewer`). +2. Open the DCI `fastapi.endpoint` record (the `dci_api` app, root path + `/dci_api/v1`) and set its **User** to that service account. + +Authentication of the *caller* is independent of this: requests are authenticated +with a static bearer token (`dci.api_tokens`) or an OAuth2 access token. The +endpoint user only sets the Odoo permission context the search executes under. + ### UI Location - **Menu**: Settings > DCI > Configuration > Sender Registry diff --git a/spp_dci_server_social/README.rst b/spp_dci_server_social/README.rst index bbe9fae4..158b31f4 100644 --- a/spp_dci_server_social/README.rst +++ b/spp_dci_server_social/README.rst @@ -127,11 +127,18 @@ Notifications triggered automatically on registrant changes. Security ~~~~~~~~ -No new access rules. Search requires +No new access rules. Search requires the ``spp_registry.group_registry_viewer`` group (enforced in ``DCISocialSearchService._process_search_item()``). Inherits access control from ``spp_registry`` and ``spp_dci_server``. +Because the search runs under the DCI FastAPI endpoint's user, that user +must hold ``spp_registry.group_registry_viewer``. The endpoint ships as +the public user, which lacks it, so Social Registry searches are +rejected until a registry-viewer service user is assigned to the +endpoint — see **Endpoint user (required for Social Registry search)** +in the ``spp_dci_server`` documentation. + Extension Points ~~~~~~~~~~~~~~~~ diff --git a/spp_dci_server_social/__manifest__.py b/spp_dci_server_social/__manifest__.py index 8c80dc51..387414e5 100644 --- a/spp_dci_server_social/__manifest__.py +++ b/spp_dci_server_social/__manifest__.py @@ -2,7 +2,7 @@ { "name": "OpenSPP DCI Server - Social Registry", "summary": "Expose Social Registry beneficiaries via DCI API", - "version": "19.0.1.0.0", + "version": "19.0.1.0.1", "category": "OpenSPP/Integration", "author": "OpenSPP.org", "website": "https://github.com/OpenSPP/OpenSPP2", diff --git a/spp_dci_server_social/readme/DESCRIPTION.md b/spp_dci_server_social/readme/DESCRIPTION.md index fb515d1e..ded0baae 100644 --- a/spp_dci_server_social/readme/DESCRIPTION.md +++ b/spp_dci_server_social/readme/DESCRIPTION.md @@ -64,7 +64,9 @@ No standalone UI. Search functionality accessed via DCI API endpoints. Notificat ### Security -No new access rules. Search requires `spp_registry.group_registry_viewer` group (enforced in `DCISocialSearchService._process_search_item()`). Inherits access control from `spp_registry` and `spp_dci_server`. +No new access rules. Search requires the `spp_registry.group_registry_viewer` group (enforced in `DCISocialSearchService._process_search_item()`). Inherits access control from `spp_registry` and `spp_dci_server`. + +Because the search runs under the DCI FastAPI endpoint's user, that user must hold `spp_registry.group_registry_viewer`. The endpoint ships as the public user, which lacks it, so Social Registry searches are rejected until a registry-viewer service user is assigned to the endpoint — see **Endpoint user (required for Social Registry search)** in the `spp_dci_server` documentation. ### Extension Points diff --git a/spp_dci_server_social/static/description/index.html b/spp_dci_server_social/static/description/index.html index ec18c75b..b95d0a51 100644 --- a/spp_dci_server_social/static/description/index.html +++ b/spp_dci_server_social/static/description/index.html @@ -484,10 +484,16 @@

    UI Location

    Security

    -

    No new access rules. Search requires +

    No new access rules. Search requires the spp_registry.group_registry_viewer group (enforced in DCISocialSearchService._process_search_item()). Inherits access control from spp_registry and spp_dci_server.

    +

    Because the search runs under the DCI FastAPI endpoint’s user, that user +must hold spp_registry.group_registry_viewer. The endpoint ships as +the public user, which lacks it, so Social Registry searches are +rejected until a registry-viewer service user is assigned to the +endpoint — see Endpoint user (required for Social Registry search) +in the spp_dci_server documentation.

    Extension Points

    From aba70d8af555cb69a1d30ca38921ddb961923258 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Mon, 8 Jun 2026 22:06:01 +0800 Subject: [PATCH 22/22] docs(spp_dci_server): fix readme heading level skip The endpoint-user note used a level-4 (####) heading directly under the level-2 Configuration section. CI's docutils rejects the skipped heading level (Inconsistent title style: skip from level 2 to 4), failing the oca-gen-addon-readme pre-commit hook. Render the note as inline bold + continued steps instead of a deeper heading. --- spp_dci_server/README.rst | 22 ++++++++------------ spp_dci_server/readme/DESCRIPTION.md | 19 ++++------------- spp_dci_server/static/description/index.html | 16 ++++++++++++++ 3 files changed, 29 insertions(+), 28 deletions(-) diff --git a/spp_dci_server/README.rst b/spp_dci_server/README.rst index 162aa9b6..bc1b589d 100644 --- a/spp_dci_server/README.rst +++ b/spp_dci_server/README.rst @@ -90,20 +90,16 @@ Server signing keys are automatically generated and activated on installation. To manage keys manually, use the technical interface for ``spp.dci.server.key``. -Endpoint user (required for Social Registry search) -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - -The DCI FastAPI endpoint ships configured to run as the **public user**. -Serving Social Registry searches additionally requires the endpoint's -user to hold the ``spp_registry.group_registry_viewer`` group (the -search is access-gated). With the default public user, SR searches are -rejected with an access error. - -Assign a dedicated service user before going live: - -1. Create a user (e.g. *DCI Endpoint Service*) and grant it **Registry: +**Endpoint user (required for Social Registry search):** The DCI FastAPI +endpoint ships configured to run as the **public user**. Serving Social +Registry searches additionally requires the endpoint's user to hold the +``spp_registry.group_registry_viewer`` group (the search is +access-gated), so with the default public user SR searches are rejected +with an access error. Assign a dedicated service user before going live: + +5. Create a user (e.g. *DCI Endpoint Service*) and grant it **Registry: Viewer** (``spp_registry.group_registry_viewer``). -2. Open the DCI ``fastapi.endpoint`` record (the ``dci_api`` app, root +6. Open the DCI ``fastapi.endpoint`` record (the ``dci_api`` app, root path ``/dci_api/v1``) and set its **User** to that service account. Authentication of the *caller* is independent of this: requests are diff --git a/spp_dci_server/readme/DESCRIPTION.md b/spp_dci_server/readme/DESCRIPTION.md index b893d469..9a8932c8 100644 --- a/spp_dci_server/readme/DESCRIPTION.md +++ b/spp_dci_server/readme/DESCRIPTION.md @@ -32,23 +32,12 @@ After installing: Server signing keys are automatically generated and activated on installation. To manage keys manually, use the technical interface for `spp.dci.server.key`. -#### Endpoint user (required for Social Registry search) +**Endpoint user (required for Social Registry search):** The DCI FastAPI endpoint ships configured to run as the **public user**. Serving Social Registry searches additionally requires the endpoint's user to hold the `spp_registry.group_registry_viewer` group (the search is access-gated), so with the default public user SR searches are rejected with an access error. Assign a dedicated service user before going live: -The DCI FastAPI endpoint ships configured to run as the **public user**. Serving -Social Registry searches additionally requires the endpoint's user to hold the -`spp_registry.group_registry_viewer` group (the search is access-gated). With the -default public user, SR searches are rejected with an access error. +5. Create a user (e.g. *DCI Endpoint Service*) and grant it **Registry: Viewer** (`spp_registry.group_registry_viewer`). +6. Open the DCI `fastapi.endpoint` record (the `dci_api` app, root path `/dci_api/v1`) and set its **User** to that service account. -Assign a dedicated service user before going live: - -1. Create a user (e.g. *DCI Endpoint Service*) and grant it **Registry: Viewer** - (`spp_registry.group_registry_viewer`). -2. Open the DCI `fastapi.endpoint` record (the `dci_api` app, root path - `/dci_api/v1`) and set its **User** to that service account. - -Authentication of the *caller* is independent of this: requests are authenticated -with a static bearer token (`dci.api_tokens`) or an OAuth2 access token. The -endpoint user only sets the Odoo permission context the search executes under. +Authentication of the *caller* is independent of this: requests are authenticated with a static bearer token (`dci.api_tokens`) or an OAuth2 access token. The endpoint user only sets the Odoo permission context the search executes under. ### UI Location diff --git a/spp_dci_server/static/description/index.html b/spp_dci_server/static/description/index.html index a7c6cfbc..980e5429 100644 --- a/spp_dci_server/static/description/index.html +++ b/spp_dci_server/static/description/index.html @@ -450,6 +450,22 @@

    Configuration

    Server signing keys are automatically generated and activated on installation. To manage keys manually, use the technical interface for spp.dci.server.key.

    +

    Endpoint user (required for Social Registry search): The DCI FastAPI +endpoint ships configured to run as the public user. Serving Social +Registry searches additionally requires the endpoint’s user to hold the +spp_registry.group_registry_viewer group (the search is +access-gated), so with the default public user SR searches are rejected +with an access error. Assign a dedicated service user before going live:

    +
      +
    1. Create a user (e.g. DCI Endpoint Service) and grant it Registry: +Viewer (spp_registry.group_registry_viewer).
      2. +
    2. Open the DCI fastapi.endpoint record (the dci_api app, root +path /dci_api/v1) and set its User to that service account.
      3. +
    +

    Authentication of the caller is independent of this: requests are +authenticated with a static bearer token (dci.api_tokens) or an +OAuth2 access token. The endpoint user only sets the Odoo permission +context the search executes under.

    UI Location