From 8049f5d8697a108743da3625272d65f0a97bb2c4 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 23 Jul 2026 11:45:27 +0800 Subject: [PATCH 1/7] security(dci): purge stale compliance bearer token on upgrade Earlier versions of spp_dci_client_compliance created an spp.dci.data.source record carrying the well-known token 'compliance-test-api-key-12345'. The 19.0.1.0.1 post-migration removed only the ir.config_parameter copy, so on upgraded databases the retained data source kept working with the known token once the compliance gate was enabled - and since the trigger routes are auth='none', an unauthenticated caller could drive DCI client operations through it. - Add a 19.0.1.0.2 post-migration that deletes any compliance data source still holding the default token (records re-keyed with a real token are left untouched), via a new _purge_default_compliance_bearer_token() helper. - Harden _get_test_data_source() to refuse serving a data source that still holds the default token, so a skipped migration cannot re-expose it. - Add regression tests for the purge helper and the controller guard. - Bump version to 19.0.1.0.2 and add HISTORY fragment. --- spp_dci_client_compliance/__manifest__.py | 2 +- .../controllers/trigger.py | 40 ++++- .../migrations/19.0.1.0.2/post-migration.py | 34 ++++ .../models/data_source.py | 42 ++++- spp_dci_client_compliance/readme/HISTORY.md | 6 + spp_dci_client_compliance/tests/__init__.py | 1 + .../tests/test_stale_bearer_token.py | 147 ++++++++++++++++++ 7 files changed, 264 insertions(+), 8 deletions(-) create mode 100644 spp_dci_client_compliance/migrations/19.0.1.0.2/post-migration.py create mode 100644 spp_dci_client_compliance/readme/HISTORY.md create mode 100644 spp_dci_client_compliance/tests/test_stale_bearer_token.py diff --git a/spp_dci_client_compliance/__manifest__.py b/spp_dci_client_compliance/__manifest__.py index 0c1814cc7..3a0a993b7 100644 --- a/spp_dci_client_compliance/__manifest__.py +++ b/spp_dci_client_compliance/__manifest__.py @@ -2,7 +2,7 @@ { "name": "OpenSPP DCI Client Compliance Tests", "category": "OpenSPP", - "version": "19.0.1.0.1", + "version": "19.0.1.0.2", "sequence": 1, "author": "OpenSPP.org", "website": "https://github.com/OpenSPP/OpenSPP2", diff --git a/spp_dci_client_compliance/controllers/trigger.py b/spp_dci_client_compliance/controllers/trigger.py index 0c6712b6b..261dbec6a 100644 --- a/spp_dci_client_compliance/controllers/trigger.py +++ b/spp_dci_client_compliance/controllers/trigger.py @@ -15,6 +15,8 @@ from odoo.exceptions import UserError from odoo.http import request +from ..models.data_source import DEFAULT_COMPLIANCE_BEARER_TOKEN + _logger = logging.getLogger(__name__) COMPLIANCE_ENABLED_PARAM = "dci.client_compliance.enabled" @@ -96,16 +98,20 @@ def _get_test_data_source(self): DataSource = request.env["spp.dci.data.source"].sudo() # First try to find one marked for compliance testing - test_ds = DataSource.search( - [("is_compliance_test", "=", True)], - limit=1, + test_ds = self._reject_default_token( + DataSource.search( + [("is_compliance_test", "=", True)], + limit=1, + ) ) if not test_ds: # Fall back to one named "DCI Compliance Test" - test_ds = DataSource.search( - [("name", "=", "DCI Compliance Test")], - limit=1, + test_ds = self._reject_default_token( + DataSource.search( + [("name", "=", "DCI Compliance Test")], + limit=1, + ) ) if not test_ds: @@ -114,6 +120,28 @@ def _get_test_data_source(self): return test_ds + @staticmethod + def _reject_default_token(data_source): + """Drop a data source still holding the well-known default token. + + Upgraded databases may retain a compliance data source carrying the + public default token (see the 19.0.1.0.2 migration). Serving it would + re-expose the shared secret over the ``auth='none'`` routes, so treat + such a record as absent and let the caller fall through to the + fail-closed create path. + """ + # Plain equality against a PUBLIC, well-known sentinel token - not a + # secret comparison, so timing analysis leaks nothing. This is a + # fail-closed record filter, not an authentication check. + # nosemgrep: odoo-timing-attack-password + if data_source and data_source.bearer_token == DEFAULT_COMPLIANCE_BEARER_TOKEN: + _logger.warning( + "Ignoring DCI compliance data source %r that still holds the default bearer token.", + data_source.code, + ) + return data_source.browse() + return data_source + def _create_test_data_source(self): """Create a test data source pointing to mock registry. diff --git a/spp_dci_client_compliance/migrations/19.0.1.0.2/post-migration.py b/spp_dci_client_compliance/migrations/19.0.1.0.2/post-migration.py new file mode 100644 index 000000000..613882f83 --- /dev/null +++ b/spp_dci_client_compliance/migrations/19.0.1.0.2/post-migration.py @@ -0,0 +1,34 @@ +# Part of OpenSPP. See LICENSE file for full copyright and licensing details. +"""Remove compliance data sources that still hold the old shared bearer token. + +The 19.0.1.0.1 migration cleared only the ``ir.config_parameter`` copy of the +well-known token ``compliance-test-api-key-12345``. Earlier versions also +created an ``spp.dci.data.source`` record carrying that token in its own +``bearer_token`` column, which the trigger controller would keep using once the +compliance gate was re-enabled - re-exposing the shared secret over the +``auth='none'`` routes on upgraded databases. + +Delete any such retained record so the controller falls back to its fail-closed +create path (which requires an operator-configured token). Records an operator +has re-keyed with a real token are left untouched. +""" + +import logging + +from odoo import SUPERUSER_ID, api + +_logger = logging.getLogger(__name__) + + +def migrate(cr, version): + if not version: + return + + env = api.Environment(cr, SUPERUSER_ID, {}) + removed = env["spp.dci.data.source"]._purge_default_compliance_bearer_token() + if removed: + _logger.warning( + "Removed %d DCI compliance data source(s) that still held the default bearer token. " + "Set 'dci.client_compliance.bearer_token' before re-enabling the trigger endpoints.", + removed, + ) diff --git a/spp_dci_client_compliance/models/data_source.py b/spp_dci_client_compliance/models/data_source.py index d152b5e08..38e3637a6 100644 --- a/spp_dci_client_compliance/models/data_source.py +++ b/spp_dci_client_compliance/models/data_source.py @@ -1,7 +1,16 @@ # Part of OpenSPP. See LICENSE file for full copyright and licensing details. """Extension to spp.dci.data.source for compliance testing.""" -from odoo import fields, models +import logging + +from odoo import api, fields, models + +_logger = logging.getLogger(__name__) + +# Well-known bearer token that earlier module versions (19.0.1.0.0) shipped as +# the default for the compliance test data source. It is public, so any data +# source still holding it must never be used to authenticate outbound requests. +DEFAULT_COMPLIANCE_BEARER_TOKEN = "compliance-test-api-key-12345" class DCIDataSourceCompliance(models.Model): @@ -15,3 +24,34 @@ class DCIDataSourceCompliance(models.Model): help="Mark this data source as used for DCI compliance testing. " "Only one data source should have this flag enabled.", ) + + @api.model + def _purge_default_compliance_bearer_token(self): + """Delete data sources that still hold the well-known default token. + + Earlier module versions created a compliance data source carrying + ``DEFAULT_COMPLIANCE_BEARER_TOKEN``. The 19.0.1.0.2 post-migration + calls this to remove any such retained record so the trigger + controller falls back to its fail-closed create path (which requires + an operator-configured token). Records an operator has re-keyed with a + real token are matched on the token value alone, so they are left + untouched. + + Returns: + int: number of data source records removed. + """ + # sudo(): bearer_token is field-level restricted to base.group_system; + # this maintenance sweep must see and remove records regardless of the + # calling user. Scope is limited to the exact known public secret. + # nosemgrep: odoo-sudo-without-context + stale = self.sudo().search([("bearer_token", "=", DEFAULT_COMPLIANCE_BEARER_TOKEN)]) + if not stale: + return 0 + for record in stale: + _logger.warning( + "Removing DCI compliance data source %r that still held the default bearer token.", + record.code, + ) + count = len(stale) + stale.unlink() + return count diff --git a/spp_dci_client_compliance/readme/HISTORY.md b/spp_dci_client_compliance/readme/HISTORY.md new file mode 100644 index 000000000..b941c7e58 --- /dev/null +++ b/spp_dci_client_compliance/readme/HISTORY.md @@ -0,0 +1,6 @@ +### 19.0.1.0.2 + +- fix(security): remove compliance data sources that still hold the old shared + bearer token on upgrade, and refuse to serve any such record from the trigger + controller, so upgraded databases cannot re-use the well-known credential over + the unauthenticated trigger routes. diff --git a/spp_dci_client_compliance/tests/__init__.py b/spp_dci_client_compliance/tests/__init__.py index 9f149bcbe..011bacac7 100644 --- a/spp_dci_client_compliance/tests/__init__.py +++ b/spp_dci_client_compliance/tests/__init__.py @@ -2,3 +2,4 @@ from . import test_trigger_controller from . import test_trigger_edge_cases +from . import test_stale_bearer_token diff --git a/spp_dci_client_compliance/tests/test_stale_bearer_token.py b/spp_dci_client_compliance/tests/test_stale_bearer_token.py new file mode 100644 index 000000000..4dc7fc7b1 --- /dev/null +++ b/spp_dci_client_compliance/tests/test_stale_bearer_token.py @@ -0,0 +1,147 @@ +# Part of OpenSPP. See LICENSE file for full copyright and licensing details. +"""Regression tests: an upgraded database must not keep using the old shared +compliance bearer token. + +Earlier module versions (19.0.1.0.0) created an ``spp.dci.data.source`` record +carrying the well-known token ``compliance-test-api-key-12345``. The 19.0.1.0.1 +post-migration only removed the ``ir.config_parameter`` copy, leaving the data +source usable through the ``auth='none'`` trigger routes. These tests cover the +two safeguards added in 19.0.1.0.2: + +- ``_purge_default_compliance_bearer_token`` (used by the migration) deletes + data sources that still hold the known default token, and leaves re-keyed + records untouched. +- ``_get_test_data_source`` in the trigger controller refuses to serve a record + still holding the default token, so a skipped migration cannot re-expose it. +""" + +from odoo.tests import TransactionCase, tagged + +from odoo.addons.spp_dci_client_compliance.models.data_source import ( + DEFAULT_COMPLIANCE_BEARER_TOKEN, +) + +BEARER_TOKEN_PARAM = "dci.client_compliance.bearer_token" +MOCK_URL_PARAM = "dci.client_compliance.mock_registry_url" + + +def _ds_vals(code, token, **overrides): + vals = { + "name": "DCI Compliance Test", + "code": code, + "base_url": "http://mock_registry:3335", + "registry_type": "social", + "our_sender_id": "spmis.compliance.test", + "our_callback_uri": "http://openspp.dci.local:8069/dci/callback", + "is_compliance_test": True, + "state": "active", + "auth_type": "bearer", + "bearer_token": token, + } + vals.update(overrides) + return vals + + +@tagged("post_install", "-at_install") +class TestPurgeDefaultBearerToken(TransactionCase): + """Unit-test the cleanup helper the 19.0.1.0.2 migration invokes.""" + + def test_purge_removes_record_with_default_token(self): + DataSource = self.env["spp.dci.data.source"].sudo() + stale = DataSource.create(_ds_vals("dci_compliance_stale", DEFAULT_COMPLIANCE_BEARER_TOKEN)) + + removed = DataSource._purge_default_compliance_bearer_token() + + self.assertEqual(removed, 1) + self.assertFalse(stale.exists(), "Data source holding the default token must be deleted") + + def test_purge_keeps_record_with_operator_token(self): + DataSource = self.env["spp.dci.data.source"].sudo() + rekeyed = DataSource.create(_ds_vals("dci_compliance_rekeyed", "operator-real-token")) + + removed = DataSource._purge_default_compliance_bearer_token() + + self.assertEqual(removed, 0) + self.assertTrue(rekeyed.exists(), "A record re-keyed with a real token must be left untouched") + + def test_purge_only_removes_default_token_records(self): + DataSource = self.env["spp.dci.data.source"].sudo() + stale = DataSource.create(_ds_vals("dci_compliance_stale", DEFAULT_COMPLIANCE_BEARER_TOKEN)) + rekeyed = DataSource.create(_ds_vals("dci_compliance_rekeyed", "operator-real-token")) + + removed = DataSource._purge_default_compliance_bearer_token() + + self.assertEqual(removed, 1) + self.assertFalse(stale.exists()) + self.assertTrue(rekeyed.exists()) + + +@tagged("post_install", "-at_install") +class TestControllerRejectsDefaultToken(TransactionCase): + """The controller must not serve a retained default-token data source. + + Even if the migration is skipped, ``_get_test_data_source`` must treat a + record still holding the known default token as absent and fall through to + the fail-closed create path (which requires an operator-configured token). + """ + + def setUp(self): + super().setUp() + from odoo.addons.spp_dci_client_compliance.controllers.trigger import ( + DCIClientTriggerController, + ) + + self.controller = DCIClientTriggerController() + ICP = self.env["ir.config_parameter"].sudo() + ICP.set_param(BEARER_TOKEN_PARAM, "operator-real-token") + ICP.set_param(MOCK_URL_PARAM, "http://mock_registry:3335") + + def _run_with_request(self): + import odoo.addons.spp_dci_client_compliance.controllers.trigger as mod + + class FakeRequest: + env = self.env + + original = mod.__dict__["request"] + mod.request = FakeRequest() + try: + return self.controller._get_test_data_source() + finally: + mod.request = original + + def test_default_token_record_is_not_served(self): + DataSource = self.env["spp.dci.data.source"].sudo() + DataSource.create(_ds_vals("dci_compliance_stale", DEFAULT_COMPLIANCE_BEARER_TOKEN)) + + result = self._run_with_request() + + self.assertNotEqual( + result.bearer_token, + DEFAULT_COMPLIANCE_BEARER_TOKEN, + "Controller must not return a data source still holding the default token", + ) + self.assertEqual(result.bearer_token, "operator-real-token") + + def test_default_token_record_matched_by_name_is_not_served(self): + # is_compliance_test=False so only the by-name fallback can find it. + DataSource = self.env["spp.dci.data.source"].sudo() + DataSource.create( + _ds_vals( + "dci_compliance_named", + DEFAULT_COMPLIANCE_BEARER_TOKEN, + is_compliance_test=False, + ) + ) + + result = self._run_with_request() + + self.assertNotEqual(result.bearer_token, DEFAULT_COMPLIANCE_BEARER_TOKEN) + self.assertEqual(result.bearer_token, "operator-real-token") + + def test_operator_token_record_is_served(self): + DataSource = self.env["spp.dci.data.source"].sudo() + rekeyed = DataSource.create(_ds_vals("dci_compliance_rekeyed", "operator-real-token")) + + result = self._run_with_request() + + self.assertEqual(result.id, rekeyed.id, "A properly configured record must still be used") From e4b8eb94eb9ff1d7103f027faf9850ffb42e6e79 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 23 Jul 2026 11:58:44 +0800 Subject: [PATCH 2/7] docs(spp_dci_client_compliance): regenerate README for 19.0.1.0.2 changelog Applies CI's pinned oca-gen output verbatim for the new HISTORY fragment. --- spp_dci_client_compliance/README.rst | 11 ++++++++ .../static/description/index.html | 27 ++++++++++++------- 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/spp_dci_client_compliance/README.rst b/spp_dci_client_compliance/README.rst index 32a137a5a..efaa79ae6 100644 --- a/spp_dci_client_compliance/README.rst +++ b/spp_dci_client_compliance/README.rst @@ -114,6 +114,17 @@ Dependencies .. contents:: :local: +Changelog +========= + +19.0.1.0.2 +~~~~~~~~~~ + +- fix(security): remove compliance data sources that still hold the old + shared bearer token on upgrade, and refuse to serve any such record + from the trigger controller, so upgraded databases cannot re-use the + well-known credential over the unauthenticated trigger routes. + Bug Tracker =========== diff --git a/spp_dci_client_compliance/static/description/index.html b/spp_dci_client_compliance/static/description/index.html index fd1df3adf..4b72f6f72 100644 --- a/spp_dci_client_compliance/static/description/index.html +++ b/spp_dci_client_compliance/static/description/index.html @@ -489,16 +489,23 @@

Dependencies

Table of contents

+ + +
+

19.0.1.0.2

+
    +
  • fix(security): remove compliance data sources that still hold the old +shared bearer token on upgrade, and refuse to serve any such record +from the trigger controller, so upgraded databases cannot re-use the +well-known credential over the unauthenticated trigger routes.
    • +
-

Bug Tracker

+

Bug Tracker

Bugs are tracked on GitHub Issues. In case of trouble, please check there if your issue has already been reported. If you spotted it first, help us to smash it by providing a detailed and welcomed @@ -506,15 +513,15 @@

Bug Tracker

Do not contact contributors directly about support or help with technical issues.

-

Credits

+

Credits

-

Authors

+

Authors

  • OpenSPP.org
-

Maintainers

+

Maintainers

Current maintainers:

jeremi gonzalesedwin1123

This module is part of the OpenSPP/OpenSPP2 project on GitHub.

From c6418b096c3764707c450a3cd25e0b034b85e364 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 23 Jul 2026 12:03:58 +0800 Subject: [PATCH 3/7] refactor(spp_dci_client_compliance): drop redundant empty-recordset guard An empty recordset already makes the warning loop, len(), and unlink() no-ops, so the explicit 'if not stale: return 0' early return is dead weight. --- spp_dci_client_compliance/models/data_source.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/spp_dci_client_compliance/models/data_source.py b/spp_dci_client_compliance/models/data_source.py index 38e3637a6..666c1ad59 100644 --- a/spp_dci_client_compliance/models/data_source.py +++ b/spp_dci_client_compliance/models/data_source.py @@ -45,8 +45,6 @@ def _purge_default_compliance_bearer_token(self): # calling user. Scope is limited to the exact known public secret. # nosemgrep: odoo-sudo-without-context stale = self.sudo().search([("bearer_token", "=", DEFAULT_COMPLIANCE_BEARER_TOKEN)]) - if not stale: - return 0 for record in stale: _logger.warning( "Removing DCI compliance data source %r that still held the default bearer token.", From d8a75ec775c31f64a068a65a79b7c7e3bc3a4bac Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 23 Jul 2026 15:38:33 +0800 Subject: [PATCH 4/7] security(dci): reject the well-known default compliance token on the create path Staff-engineer review found the search-path guard left the create path open: _get_compliance_bearer_token only checked the token was non-empty, so an operator following the module docs (which suggested the public default) would mint a fresh data source carrying it - recreating the exposure the migration purges. Reject the default token there, fix the misleading DESCRIPTION.md instruction, and add create-path + token-only-match regression tests. --- .../controllers/trigger.py | 12 +++ .../readme/DESCRIPTION.md | 2 +- spp_dci_client_compliance/readme/HISTORY.md | 7 +- .../tests/test_stale_bearer_token.py | 79 +++++++++++++++++++ 4 files changed, 96 insertions(+), 4 deletions(-) diff --git a/spp_dci_client_compliance/controllers/trigger.py b/spp_dci_client_compliance/controllers/trigger.py index 261dbec6a..91344b546 100644 --- a/spp_dci_client_compliance/controllers/trigger.py +++ b/spp_dci_client_compliance/controllers/trigger.py @@ -72,6 +72,18 @@ def _get_compliance_bearer_token(env): f"Set the system parameter {BEARER_TOKEN_PARAM!r} before " f"using the trigger endpoints." ) + # Refuse the well-known default token: it is public, so accepting it + # would recreate the exact exposure the 19.0.1.0.2 migration purges - + # a data source authenticating outbound requests with a shared secret. + # Plain equality against a PUBLIC sentinel - not a secret comparison, so + # timing analysis leaks nothing. + # nosemgrep: odoo-timing-attack-password + if token == DEFAULT_COMPLIANCE_BEARER_TOKEN: + raise UserError( + f"The DCI client compliance bearer token is set to the well-known " + f"default value, which is not allowed. Set {BEARER_TOKEN_PARAM!r} to a " + f"private token before using the trigger endpoints." + ) return token def _disabled_response(self): diff --git a/spp_dci_client_compliance/readme/DESCRIPTION.md b/spp_dci_client_compliance/readme/DESCRIPTION.md index 26872e9bf..cba6557c1 100644 --- a/spp_dci_client_compliance/readme/DESCRIPTION.md +++ b/spp_dci_client_compliance/readme/DESCRIPTION.md @@ -20,7 +20,7 @@ Testing infrastructure for SPDCI protocol compliance validation. Exposes HTTP en After installing: 1. Set system parameter `dci.client_compliance.mock_registry_url` to point to your mock registry (default: `http://mock_registry:3335`) -2. Set system parameter `dci.client_compliance.bearer_token` for authentication (default: `compliance-test-api-key-12345`) +2. Set system parameter `dci.client_compliance.bearer_token` to a **private** token for authentication. There is no default, and the well-known value `compliance-test-api-key-12345` is rejected; the trigger endpoints refuse to run until a private token is configured. 3. Verify test data source exists under **Settings > Technical > DCI > Configuration > Data Sources** (auto-created if missing) ### Controller Endpoints diff --git a/spp_dci_client_compliance/readme/HISTORY.md b/spp_dci_client_compliance/readme/HISTORY.md index b941c7e58..b2a72ab87 100644 --- a/spp_dci_client_compliance/readme/HISTORY.md +++ b/spp_dci_client_compliance/readme/HISTORY.md @@ -1,6 +1,7 @@ ### 19.0.1.0.2 - fix(security): remove compliance data sources that still hold the old shared - bearer token on upgrade, and refuse to serve any such record from the trigger - controller, so upgraded databases cannot re-use the well-known credential over - the unauthenticated trigger routes. + bearer token on upgrade, refuse to serve any such record from the trigger + controller, and reject the well-known default token when configured, so + upgraded or freshly configured databases cannot use the shared credential + over the unauthenticated trigger routes. diff --git a/spp_dci_client_compliance/tests/test_stale_bearer_token.py b/spp_dci_client_compliance/tests/test_stale_bearer_token.py index 4dc7fc7b1..c05280188 100644 --- a/spp_dci_client_compliance/tests/test_stale_bearer_token.py +++ b/spp_dci_client_compliance/tests/test_stale_bearer_token.py @@ -13,8 +13,11 @@ records untouched. - ``_get_test_data_source`` in the trigger controller refuses to serve a record still holding the default token, so a skipped migration cannot re-expose it. +- ``_get_compliance_bearer_token`` rejects the well-known default token when it + is configured, so the create path cannot mint a fresh default-token record. """ +from odoo.exceptions import UserError from odoo.tests import TransactionCase, tagged from odoo.addons.spp_dci_client_compliance.models.data_source import ( @@ -75,6 +78,24 @@ def test_purge_only_removes_default_token_records(self): self.assertFalse(stale.exists()) self.assertTrue(rekeyed.exists()) + def test_purge_matches_on_token_only_not_flag_or_name(self): + # A default-token record that is neither flagged nor named as a + # compliance record must still be purged - the match is token-only. + DataSource = self.env["spp.dci.data.source"].sudo() + unflagged = DataSource.create( + _ds_vals( + "dci_compliance_unflagged", + DEFAULT_COMPLIANCE_BEARER_TOKEN, + name="Some Other Data Source", + is_compliance_test=False, + ) + ) + + removed = DataSource._purge_default_compliance_bearer_token() + + self.assertEqual(removed, 1) + self.assertFalse(unflagged.exists()) + @tagged("post_install", "-at_install") class TestControllerRejectsDefaultToken(TransactionCase): @@ -145,3 +166,61 @@ def test_operator_token_record_is_served(self): result = self._run_with_request() self.assertEqual(result.id, rekeyed.id, "A properly configured record must still be used") + + +@tagged("post_install", "-at_install") +class TestCreatePathRejectsDefaultToken(TransactionCase): + """The create path must refuse the well-known default token when configured. + + Guarding only the search paths is not enough: if an operator sets + ``dci.client_compliance.bearer_token`` to the public default (as the module + docs previously suggested), the create path would mint a fresh data source + carrying it - recreating the exposure through the front door. + ``_get_compliance_bearer_token`` therefore rejects the default value. + """ + + def setUp(self): + super().setUp() + from odoo.addons.spp_dci_client_compliance.controllers.trigger import ( + DCIClientTriggerController, + ) + + self.controller = DCIClientTriggerController() + self.ICP = self.env["ir.config_parameter"].sudo() + self.ICP.set_param(MOCK_URL_PARAM, "http://mock_registry:3335") + + def _run(self, method_name): + import odoo.addons.spp_dci_client_compliance.controllers.trigger as mod + + class FakeRequest: + env = self.env + + original = mod.__dict__["request"] + mod.request = FakeRequest() + try: + return getattr(self.controller, method_name)() + finally: + mod.request = original + + def test_get_bearer_token_rejects_default(self): + self.ICP.set_param(BEARER_TOKEN_PARAM, DEFAULT_COMPLIANCE_BEARER_TOKEN) + with self.assertRaises(UserError): + self.controller._get_compliance_bearer_token(self.env) + + def test_create_path_refuses_to_mint_default_token_record(self): + self.ICP.set_param(BEARER_TOKEN_PARAM, DEFAULT_COMPLIANCE_BEARER_TOKEN) + with self.assertRaises(UserError): + self._run("_create_test_data_source") + # Nothing was created carrying the default token. + count = ( + self.env["spp.dci.data.source"] + .sudo() + .search_count([("bearer_token", "=", DEFAULT_COMPLIANCE_BEARER_TOKEN)]) + ) + self.assertEqual(count, 0) + + def test_create_path_succeeds_with_private_token(self): + self.ICP.set_param(BEARER_TOKEN_PARAM, "operator-real-token") + result = self._run("_create_test_data_source") + self.assertTrue(result) + self.assertEqual(result.bearer_token, "operator-real-token") From f9350563b610317f6886b22bd8488672dcae1ca6 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 23 Jul 2026 15:56:24 +0800 Subject: [PATCH 5/7] security(dci): purge archived default-token records and fix valid-record masking Second review pass found two more gaps: - The purge searched with default active_test=True, so an ARCHIVED data source still holding the default token survived the migration (secret at rest, still usable by non-controller consumers via get_headers). Purge now runs with active_test=False. - The controller filtered the default token AFTER a limit=1 search, so a stale record sorting ahead of a legitimately re-keyed one (same name/flag) masked the valid record and forced a needless create. Exclude the default token in the search domain instead, replacing the post-filter helper. Adds regression tests for the archived purge and the stale+rekeyed coexistence. --- .../controllers/trigger.py | 46 ++++++------------- .../models/data_source.py | 6 ++- .../tests/test_stale_bearer_token.py | 23 ++++++++++ 3 files changed, 42 insertions(+), 33 deletions(-) diff --git a/spp_dci_client_compliance/controllers/trigger.py b/spp_dci_client_compliance/controllers/trigger.py index 91344b546..fcdd931fe 100644 --- a/spp_dci_client_compliance/controllers/trigger.py +++ b/spp_dci_client_compliance/controllers/trigger.py @@ -109,21 +109,25 @@ def _get_test_data_source(self): # nosemgrep: odoo-sudo-without-context DataSource = request.env["spp.dci.data.source"].sudo() + # Exclude any record still holding the well-known default token directly + # in the domain. Upgraded databases may retain such a record (see the + # 19.0.1.0.2 migration); serving it would re-expose the shared secret + # over the ``auth='none'`` routes. Filtering in the domain rather than + # post-search means a legitimately re-keyed record is still found even + # when a stale record sorts ahead of it under limit=1. + not_default = ("bearer_token", "!=", DEFAULT_COMPLIANCE_BEARER_TOKEN) + # First try to find one marked for compliance testing - test_ds = self._reject_default_token( - DataSource.search( - [("is_compliance_test", "=", True)], - limit=1, - ) + test_ds = DataSource.search( + [("is_compliance_test", "=", True), not_default], + limit=1, ) if not test_ds: # Fall back to one named "DCI Compliance Test" - test_ds = self._reject_default_token( - DataSource.search( - [("name", "=", "DCI Compliance Test")], - limit=1, - ) + test_ds = DataSource.search( + [("name", "=", "DCI Compliance Test"), not_default], + limit=1, ) if not test_ds: @@ -132,28 +136,6 @@ def _get_test_data_source(self): return test_ds - @staticmethod - def _reject_default_token(data_source): - """Drop a data source still holding the well-known default token. - - Upgraded databases may retain a compliance data source carrying the - public default token (see the 19.0.1.0.2 migration). Serving it would - re-expose the shared secret over the ``auth='none'`` routes, so treat - such a record as absent and let the caller fall through to the - fail-closed create path. - """ - # Plain equality against a PUBLIC, well-known sentinel token - not a - # secret comparison, so timing analysis leaks nothing. This is a - # fail-closed record filter, not an authentication check. - # nosemgrep: odoo-timing-attack-password - if data_source and data_source.bearer_token == DEFAULT_COMPLIANCE_BEARER_TOKEN: - _logger.warning( - "Ignoring DCI compliance data source %r that still holds the default bearer token.", - data_source.code, - ) - return data_source.browse() - return data_source - def _create_test_data_source(self): """Create a test data source pointing to mock registry. diff --git a/spp_dci_client_compliance/models/data_source.py b/spp_dci_client_compliance/models/data_source.py index 666c1ad59..c574999b3 100644 --- a/spp_dci_client_compliance/models/data_source.py +++ b/spp_dci_client_compliance/models/data_source.py @@ -43,8 +43,12 @@ def _purge_default_compliance_bearer_token(self): # sudo(): bearer_token is field-level restricted to base.group_system; # this maintenance sweep must see and remove records regardless of the # calling user. Scope is limited to the exact known public secret. + # active_test=False: archived records still hold the token in their + # bearer_token column and remain usable by non-controller consumers + # (and readable at rest), so they must be purged too. # nosemgrep: odoo-sudo-without-context - stale = self.sudo().search([("bearer_token", "=", DEFAULT_COMPLIANCE_BEARER_TOKEN)]) + records = self.sudo().with_context(active_test=False) + stale = records.search([("bearer_token", "=", DEFAULT_COMPLIANCE_BEARER_TOKEN)]) for record in stale: _logger.warning( "Removing DCI compliance data source %r that still held the default bearer token.", diff --git a/spp_dci_client_compliance/tests/test_stale_bearer_token.py b/spp_dci_client_compliance/tests/test_stale_bearer_token.py index c05280188..825b58f20 100644 --- a/spp_dci_client_compliance/tests/test_stale_bearer_token.py +++ b/spp_dci_client_compliance/tests/test_stale_bearer_token.py @@ -78,6 +78,17 @@ def test_purge_only_removes_default_token_records(self): self.assertFalse(stale.exists()) self.assertTrue(rekeyed.exists()) + def test_purge_removes_archived_default_token_record(self): + # Archived records still hold the token at rest and stay usable by + # non-controller consumers, so the purge must reach them too. + DataSource = self.env["spp.dci.data.source"].sudo() + archived = DataSource.create(_ds_vals("dci_compliance_archived", DEFAULT_COMPLIANCE_BEARER_TOKEN, active=False)) + + removed = DataSource._purge_default_compliance_bearer_token() + + self.assertEqual(removed, 1) + self.assertFalse(archived.exists(), "Archived default-token record must be deleted") + def test_purge_matches_on_token_only_not_flag_or_name(self): # A default-token record that is neither flagged nor named as a # compliance record must still be purged - the match is token-only. @@ -167,6 +178,18 @@ def test_operator_token_record_is_served(self): self.assertEqual(result.id, rekeyed.id, "A properly configured record must still be used") + def test_rekeyed_record_served_even_when_stale_record_coexists(self): + # Both records share the name/flag; a stale record can sort ahead of the + # valid one. The good record must still be served (not masked by limit=1). + DataSource = self.env["spp.dci.data.source"].sudo() + DataSource.create(_ds_vals("dci_compliance_stale", DEFAULT_COMPLIANCE_BEARER_TOKEN)) + rekeyed = DataSource.create(_ds_vals("dci_compliance_rekeyed", "operator-real-token")) + + result = self._run_with_request() + + self.assertEqual(result.id, rekeyed.id, "Valid record must not be masked by a stale one") + self.assertEqual(result.bearer_token, "operator-real-token") + @tagged("post_install", "-at_install") class TestCreatePathRejectsDefaultToken(TransactionCase): From 674263f3a0ac9aa0ee7ed3274c2160ac10e02a04 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 23 Jul 2026 16:14:35 +0800 Subject: [PATCH 6/7] docs(spp_dci_client_compliance): regenerate README for corrected token guidance Applies CI's pinned oca-gen output verbatim for the updated DESCRIPTION/HISTORY fragments (private-token requirement + create-path rejection). --- spp_dci_client_compliance/README.rst | 13 ++++++++----- .../static/description/index.html | 13 ++++++++----- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/spp_dci_client_compliance/README.rst b/spp_dci_client_compliance/README.rst index efaa79ae6..480aa009b 100644 --- a/spp_dci_client_compliance/README.rst +++ b/spp_dci_client_compliance/README.rst @@ -59,8 +59,10 @@ After installing: 1. Set system parameter ``dci.client_compliance.mock_registry_url`` to point to your mock registry (default: ``http://mock_registry:3335``) -2. Set system parameter ``dci.client_compliance.bearer_token`` for - authentication (default: ``compliance-test-api-key-12345``) +2. Set system parameter ``dci.client_compliance.bearer_token`` to a + **private** token for authentication. There is no default, and the + well-known value ``compliance-test-api-key-12345`` is rejected; the + trigger endpoints refuse to run until a private token is configured. 3. Verify test data source exists under **Settings > Technical > DCI > Configuration > Data Sources** (auto-created if missing) @@ -121,9 +123,10 @@ Changelog ~~~~~~~~~~ - fix(security): remove compliance data sources that still hold the old - shared bearer token on upgrade, and refuse to serve any such record - from the trigger controller, so upgraded databases cannot re-use the - well-known credential over the unauthenticated trigger routes. + shared bearer token on upgrade, refuse to serve any such record from + the trigger controller, and reject the well-known default token when + configured, so upgraded or freshly configured databases cannot use the + shared credential over the unauthenticated trigger routes. Bug Tracker =========== diff --git a/spp_dci_client_compliance/static/description/index.html b/spp_dci_client_compliance/static/description/index.html index 4b72f6f72..0c887f2df 100644 --- a/spp_dci_client_compliance/static/description/index.html +++ b/spp_dci_client_compliance/static/description/index.html @@ -416,8 +416,10 @@

Configuration

  1. Set system parameter dci.client_compliance.mock_registry_url to point to your mock registry (default: http://mock_registry:3335)
    2. -
  2. Set system parameter dci.client_compliance.bearer_token for -authentication (default: compliance-test-api-key-12345)
    3. +
  3. Set system parameter dci.client_compliance.bearer_token to a +private token for authentication. There is no default, and the +well-known value compliance-test-api-key-12345 is rejected; the +trigger endpoints refuse to run until a private token is configured.
  4. Verify test data source exists under Settings > Technical > DCI > Configuration > Data Sources (auto-created if missing)
@@ -500,9 +502,10 @@

Changelog

19.0.1.0.2

  • fix(security): remove compliance data sources that still hold the old -shared bearer token on upgrade, and refuse to serve any such record -from the trigger controller, so upgraded databases cannot re-use the -well-known credential over the unauthenticated trigger routes.
    • +shared bearer token on upgrade, refuse to serve any such record from +the trigger controller, and reject the well-known default token when +configured, so upgraded or freshly configured databases cannot use the +shared credential over the unauthenticated trigger routes.

Bug Tracker

From 31b62520340ce0bb919203f301fd7121c3d954c0 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 23 Jul 2026 17:12:39 +0800 Subject: [PATCH 7/7] test(spp_dci_client_compliance): correct stale comment after domain-exclusion change The record is now dropped by the default-token exclusion in the search domain, not found-then-rejected by a post-filter. --- spp_dci_client_compliance/tests/test_stale_bearer_token.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spp_dci_client_compliance/tests/test_stale_bearer_token.py b/spp_dci_client_compliance/tests/test_stale_bearer_token.py index 825b58f20..91338cabe 100644 --- a/spp_dci_client_compliance/tests/test_stale_bearer_token.py +++ b/spp_dci_client_compliance/tests/test_stale_bearer_token.py @@ -155,7 +155,8 @@ def test_default_token_record_is_not_served(self): self.assertEqual(result.bearer_token, "operator-real-token") def test_default_token_record_matched_by_name_is_not_served(self): - # is_compliance_test=False so only the by-name fallback can find it. + # is_compliance_test=False so only the by-name lookup could match it - + # but the default-token exclusion in the search domain drops it there too. DataSource = self.env["spp.dci.data.source"].sudo() DataSource.create( _ds_vals(