Maintainers
+Maintainers
Current maintainers:
This module is part of the OpenSPP/OpenSPP2 project on GitHub.
diff --git a/spp_pii_encryption/static/src/js/masked_field.js b/spp_pii_encryption/static/src/js/masked_field.js index 01a1f09fd..7efff3ced 100644 --- a/spp_pii_encryption/static/src/js/masked_field.js +++ b/spp_pii_encryption/static/src/js/masked_field.js @@ -2,11 +2,26 @@ import {registry} from "@web/core/registry"; import {CharField} from "@web/views/fields/char/char_field"; +import {ListRenderer} from "@web/views/list/list_renderer"; +import {patch} from "@web/core/utils/patch"; import {useState} from "@odoo/owl"; import {useService} from "@web/core/utils/hooks"; import {user} from "@web/core/user"; import {_t} from "@web/core/l10n/translation"; +// The list renderer copies every char cell's FORMATTED (raw) value into the +// cell's data-tooltip so truncated columns stay readable — which would leak +// the plaintext of a masked column on hover, bypassing the widget entirely. +// Masked columns get no tooltip. +patch(ListRenderer.prototype, { + getCellTitle(column, record) { + if (column.widget === "masked_char") { + return undefined; + } + return super.getCellTitle(column, record); + }, +}); + /** * MaskedCharField - A field widget that displays masked PII values * with the ability to reveal the actual value for authorized users. diff --git a/spp_pii_encryption/static/src/xml/masked_field.xml b/spp_pii_encryption/static/src/xml/masked_field.xml index 367e76169..5c4a86bde 100644 --- a/spp_pii_encryption/static/src/xml/masked_field.xml +++ b/spp_pii_encryption/static/src/xml/masked_field.xml @@ -15,7 +15,7 @@ t-if="props.record.data[props.name]" type="button" class="btn btn-link btn-sm p-0 ms-2 o_masked_toggle" - t-on-click="toggleReveal" + t-on-click.stop="toggleReveal" t-att-disabled="state.isLoading" t-att-title="state.isRevealed ? 'Hide value' : 'Reveal value'" > diff --git a/spp_pii_encryption/tests/__init__.py b/spp_pii_encryption/tests/__init__.py index 1ccefd13b..2114d2332 100644 --- a/spp_pii_encryption/tests/__init__.py +++ b/spp_pii_encryption/tests/__init__.py @@ -1,5 +1,6 @@ # Part of OpenSPP. See LICENSE file for full copyright and licensing details. # Key provider tests are now in spp_key_management from . import test_encrypted_field_mixin +from . import test_encryption_migration from . import test_field_encryption_config from . import test_audit_log diff --git a/spp_pii_encryption/tests/fake_model_loader.py b/spp_pii_encryption/tests/fake_model_loader.py new file mode 100644 index 000000000..19c114f9a --- /dev/null +++ b/spp_pii_encryption/tests/fake_model_loader.py @@ -0,0 +1,84 @@ +# Part of OpenSPP. See LICENSE file for full copyright and licensing details. +"""Minimal fake-model loader for tests, adapted for Odoo 19. + +Registers throwaway model classes into the live registry so tests can +exercise abstract mixins (here: spp.encrypted.field.mixin) on a concrete +model without shipping one in the module. + +This is a scoped adaptation of odoo-test-helper's FakeModelLoader (LGPL, +ACSONE/Camptocamp/Akretion): the released helper (2.1.3) targets +``MetaModel.module_to_models`` and ``Registry.setup_models``, which Odoo 19 +renamed to ``_module_to_models__`` / ``_setup_models__`` (and ``Registry.load`` +now takes a module node instead of a cursor). Only the "add brand-new +models" case is supported — do NOT use this to extend existing models +(that needs the full backup/restore of their ``__bases__``). Replace with +odoo-test-helper once it supports Odoo 19. + +Usage (TransactionCase): + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.loader = FakeModelLoader(cls.env, "spp_pii_encryption") + cls.loader.backup_registry() + from .fake_models import EncryptionTestRecord + cls.loader.update_registry((EncryptionTestRecord,)) + + @classmethod + def tearDownClass(cls): + cls.loader.restore_registry() + super().tearDownClass() +""" + +from types import SimpleNamespace +from unittest import mock + +from odoo import models +from odoo.tools import OrderedSet + + +class FakeModelLoader: + def __init__(self, env, module_name): + self.env = env + self.module_name = module_name + self._module_to_models = models.MetaModel._module_to_models__ + self._known_models = None + self._orig_module_to_models = None + + def backup_registry(self): + self.env.flush_all() + self._known_models = set(self.env.registry.models) + self._orig_module_to_models = {key: list(value) for key, value in self._module_to_models.items()} + + def update_registry(self, odoo_models): + for model in odoo_models: + if any(model._name == known for known in self._known_models): + raise AssertionError(f"{model._name} already exists; this loader only adds new models") + if model not in self._module_to_models[self.module_name]: + self._module_to_models[self.module_name].append(model) + + registry = self.env.registry + # The test cursor must never commit; registry.load/init_models are + # written for the install path. + with mock.patch.object(self.env.cr, "commit"): + model_names = registry.load(SimpleNamespace(name=self.module_name)) + registry._setup_models__(self.env.cr) + new_names = [name for name in model_names if name not in self._known_models] + registry.init_models(self.env.cr, new_names, {"module": self.module_name}) + + def restore_registry(self): + registry = self.env.registry + for name in set(registry.models) - self._known_models: + del registry.models[name] + for key, value in self._orig_module_to_models.items(): + self._module_to_models[key] = list(value) + for key in set(self._module_to_models) - set(self._orig_module_to_models): + del self._module_to_models[key] + # Drop dangling references the fake models left on their parents + # (e.g. the mixin's _inherit_children) + for model_cls in registry.models.values(): + model_cls._inherit_children = OrderedSet( + name for name in model_cls._inherit_children if name in registry.models + ) + with mock.patch.object(self.env.cr, "commit"): + registry._setup_models__(self.env.cr) diff --git a/spp_pii_encryption/tests/fake_models.py b/spp_pii_encryption/tests/fake_models.py new file mode 100644 index 000000000..e6a6038f1 --- /dev/null +++ b/spp_pii_encryption/tests/fake_models.py @@ -0,0 +1,26 @@ +# Part of OpenSPP. See LICENSE file for full copyright and licensing details. +"""Throwaway models for the migration-wizard tests. + +Imported ONLY from inside a test's setUpClass (after +FakeModelLoader.backup_registry) — importing this module registers the +classes in MetaModel's module-to-models map, so a module-level import +would leak them into real registry reloads. +""" + +from odoo import fields, models + + +class EncryptionTestRecord(models.Model): + """Concrete consumer of the encrypted-field mixin (none exists in the + real stack yet — the encryption core is a provider layer).""" + + _name = "spp.encryption.test.record" + _description = "Encryption Migration Test Record" + _inherit = ["spp.encrypted.field.mixin"] + + name = fields.Char() + secret = fields.Char() + secret_index = fields.Char(index=True) + + def _get_encrypted_fields(self): + return ["secret"] diff --git a/spp_pii_encryption/tests/test_encryption_migration.py b/spp_pii_encryption/tests/test_encryption_migration.py new file mode 100644 index 000000000..82603c3ea --- /dev/null +++ b/spp_pii_encryption/tests/test_encryption_migration.py @@ -0,0 +1,234 @@ +# Part of OpenSPP. See LICENSE file for full copyright and licensing details. +"""Tests for the PII encryption migration wizard.""" + +import base64 +from unittest.mock import patch + +from odoo import Command +from odoo.exceptions import ValidationError +from odoo.tests.common import TransactionCase +from odoo.tools import config + +from .fake_model_loader import FakeModelLoader + +WIZARD_LOGGER = "odoo.addons.spp_pii_encryption.wizard.encryption_migration_wizard" + + +class TestEncryptionMigrationWizard(TransactionCase): + """Wizard behavior against a concrete mixin consumer.""" + + @classmethod + def setUpClass(cls): + super().setUpClass() + + # Master key + default provider, as in test_encrypted_field_mixin + cls._original_master_key = config.get("spp_master_key") + config["spp_master_key"] = base64.b64encode(b"M" * 32).decode() + if not cls.env["spp.key.provider.registry"].search([("is_default", "=", True)]): + cls.env["spp.key.provider.registry"].create( + { + "name": "Test Default Provider", + "provider_type": "database", + "is_default": True, + } + ) + + # Register the fake mixin consumer + cls.loader = FakeModelLoader(cls.env, "spp_pii_encryption") + cls.loader.backup_registry() + from .fake_models import EncryptionTestRecord # noqa: PLC0415 — must import after backup_registry + + cls.loader.update_registry((EncryptionTestRecord,)) + cls.TestRecord = cls.env["spp.encryption.test.record"] + + cls.Wizard = cls.env["spp.encryption.migration.wizard"] + cls.Classification = cls.env["spp.field.classification"] + + # Classify the fake model's secret field as PII (what the scan keys on) + cls.secret_classification = cls.Classification.ensure_classification( + "spp.encryption.test.record", + "secret", + "RESTRICTED", + source="manual", + pii_category="direct_id", + ) + cls.test_model = cls.secret_classification.model_id + + @classmethod + def tearDownClass(cls): + cls.loader.restore_registry() + if cls._original_master_key: + config["spp_master_key"] = cls._original_master_key + elif "spp_master_key" in config.options: + del config.options["spp_master_key"] + super().tearDownClass() + + def _make_legacy_rows(self, values): + """Create rows holding raw plaintext with no blind index — the state + of data written before the mixin/encryption was enabled. ORM creates + would auto-encrypt, so the plaintext is restored with direct SQL.""" + records = self.TestRecord.create([{"name": f"r{i}", "secret": value} for i, value in enumerate(values)]) + for record, value in zip(records, values, strict=True): + self.env.cr.execute( + "UPDATE spp_encryption_test_record SET secret = %s, secret_index = NULL WHERE id = %s", + (value, record.id), + ) + records.invalidate_recordset() + return records + + def _raw_row(self, record): + """Raw stored (secret, secret_index) straight from SQL.""" + self.env.cr.execute( + "SELECT secret, secret_index FROM spp_encryption_test_record WHERE id = %s", + (record.id,), + ) + return self.env.cr.fetchone() + + def test_batch_size_must_be_positive(self): + with self.assertRaises(ValidationError): + self.Wizard.create({"batch_size": 0}) + + def test_scan_finds_encryptable_pii_field(self): + """Scan reports the mixin-backed field as encryptable and a plain + classified field as not.""" + self._make_legacy_rows(["111", "222"]) + self.Classification.ensure_classification("res.partner", "phone", "CONFIDENTIAL", pii_category="contact") + + wizard = self.Wizard.create({}) + wizard.action_scan() + + self.assertEqual(wizard.state, "ready") + by_model = {r.model_name: r for r in wizard.scan_result_ids} + fake_row = by_model["spp.encryption.test.record"] + self.assertTrue(fake_row.is_encrypted) + self.assertEqual(fake_row.field_name, "secret") + self.assertEqual(fake_row.records_with_data, 2) + self.assertEqual(fake_row.needs_migration, 2) + partner_row = by_model["res.partner"] + self.assertFalse(partner_row.is_encrypted, "res.partner has no phone_index — not encryptable") + + def test_scan_respects_model_filter(self): + self.Classification.ensure_classification("res.partner", "phone", "CONFIDENTIAL", pii_category="contact") + wizard = self.Wizard.create({"model_ids": [Command.set(self.test_model.ids)]}) + wizard.action_scan() + self.assertEqual(wizard.scan_result_ids.mapped("model_id"), self.test_model) + + def test_scan_skips_unreadable_model(self): + """A model the operator cannot read is logged and skipped, not fatal. + The encryption admin is deliberately not a system admin.""" + self.Classification.ensure_classification( + "ir.config_parameter", "value", "RESTRICTED", pii_category="sensitive" + ) + operator = self.env["res.users"].create( + { + "name": "Encryption Operator", + "login": "encryption_operator", + "group_ids": [ + Command.link(self.env.ref("base.group_user").id), + Command.link(self.env.ref("spp_pii_encryption.group_encryption_admin").id), + ], + } + ) + wizard = self.Wizard.with_user(operator).create({}) + wizard.action_scan() + + self.assertEqual(wizard.state, "ready") + skipped = wizard.migration_log_ids.filtered(lambda log: log.status == "skipped") + self.assertIn("ir.config_parameter", skipped.mapped("model_name")) + self.assertNotIn( + "ir.config_parameter", + wizard.scan_result_ids.mapped("model_name"), + "unreadable models must not produce scan results", + ) + self.assertIn("skipped", wizard.result_summary) + + def test_dry_run_counts_without_writing(self): + records = self._make_legacy_rows(["AAA-1", "AAA-2", "AAA-3"]) + wizard = self.Wizard.create({}) + wizard.action_scan() + wizard.action_dry_run() + + self.assertEqual(wizard.state, "done") + self.assertIn("Would process 3", wizard.result_summary) + for record, plain in zip(records, ["AAA-1", "AAA-2", "AAA-3"], strict=True): + stored, index = self._raw_row(record) + self.assertEqual(stored, plain, "dry run must not modify data") + self.assertFalse(index) + dry_logs = wizard.migration_log_ids.filtered(lambda log: log.status == "dry_run") + self.assertEqual(len(dry_logs), 1) + + def test_migrate_encrypts_legacy_plaintext(self): + legacy = self._make_legacy_rows(["PLAIN-1", "PLAIN-2"]) + # An already-encrypted row (ORM create → mixin encrypts) is untouched + encrypted = self.TestRecord.create({"name": "enc", "secret": "ALREADY"}) + _stored_before, index_before = self._raw_row(encrypted) + + wizard = self.Wizard.create({}) + wizard.action_scan() + scan_row = wizard.scan_result_ids.filtered(lambda r: r.model_id == self.test_model) + self.assertEqual(scan_row.needs_migration, 2) + + wizard.action_migrate() + + self.assertEqual(wizard.state, "done") + self.assertIn("Processed 2", wizard.result_summary) + for record, plain in zip(legacy, ["PLAIN-1", "PLAIN-2"], strict=True): + stored, index = self._raw_row(record) + self.assertNotEqual(stored, plain, "stored value must be ciphertext after migration") + self.assertTrue(index, "blind index must be computed") + # Decryption happens in read() (attribute access returns the + # raw stored value by design) + self.assertEqual(record.read(["secret"])[0]["secret"], plain) + stored_after, index_after = self._raw_row(encrypted) + self.assertEqual(index_after, index_before, "already-encrypted rows must not be touched") + + def test_migrate_loops_all_batches(self): + """One click processes every batch, not just the first.""" + legacy = self._make_legacy_rows(["B-1", "B-2", "B-3"]) + wizard = self.Wizard.create({"batch_size": 1}) + wizard.action_scan() + wizard.action_migrate() + + self.assertEqual(wizard.state, "done") + self.assertIn("Processed 3", wizard.result_summary) + for record in legacy: + _stored, index = self._raw_row(record) + self.assertTrue(index) + + def test_migrate_isolates_failing_record(self): + """A record whose write blows up is excluded and reported; the rest + of the field still migrates and the batch loop terminates.""" + legacy = self._make_legacy_rows(["F-1", "F-2", "F-3"]) + bad = legacy[1] + TestRecordClass = self.env.registry["spp.encryption.test.record"] + original_write = TestRecordClass.write + + def failing_write(record_self, vals): + if bad.id in record_self.ids: + raise ValueError("simulated storage failure") + return original_write(record_self, vals) + + wizard = self.Wizard.create({"batch_size": 1}) + wizard.action_scan() + with patch.object(TestRecordClass, "write", failing_write): + with self.assertLogs(WIZARD_LOGGER, level="ERROR") as capture: + wizard.action_migrate() + self.assertIn("Error migrating record", capture.output[0]) + + self.assertEqual(wizard.state, "error") + self.assertIn("Processed 2", wizard.result_summary) + self.assertIn("1 record(s) failed", wizard.result_summary) + _stored, bad_index = self._raw_row(bad) + self.assertFalse(bad_index, "failed record stays unmigrated") + for record in (legacy[0], legacy[2]): + _stored, index = self._raw_row(record) + self.assertTrue(index) + error_logs = wizard.migration_log_ids.filtered(lambda log: log.status == "error") + self.assertTrue(error_logs) + + def test_nothing_to_migrate(self): + wizard = self.Wizard.create({}) + wizard.action_scan() + wizard.action_migrate() + self.assertEqual(wizard.state, "done") + self.assertEqual(wizard.result_summary, "No fields need migration.") diff --git a/spp_pii_encryption/wizard/__init__.py b/spp_pii_encryption/wizard/__init__.py new file mode 100644 index 000000000..2989a93dc --- /dev/null +++ b/spp_pii_encryption/wizard/__init__.py @@ -0,0 +1,2 @@ +# Part of OpenSPP. See LICENSE file for full copyright and licensing details. +from . import encryption_migration_wizard diff --git a/spp_pii_encryption/wizard/encryption_migration_views.xml b/spp_pii_encryption/wizard/encryption_migration_views.xml new file mode 100644 index 000000000..efd6eab32 --- /dev/null +++ b/spp_pii_encryption/wizard/encryption_migration_views.xml @@ -0,0 +1,149 @@ + +
+
+
+
diff --git a/spp_registry_encryption/tests/__init__.py b/spp_registry_encryption/tests/__init__.py
new file mode 100644
index 000000000..73a87731b
--- /dev/null
+++ b/spp_registry_encryption/tests/__init__.py
@@ -0,0 +1,2 @@
+# Part of OpenSPP. See LICENSE file for full copyright and licensing details.
+from . import test_view_wiring
diff --git a/spp_registry_encryption/tests/test_view_wiring.py b/spp_registry_encryption/tests/test_view_wiring.py
new file mode 100644
index 000000000..4eea18a5e
--- /dev/null
+++ b/spp_registry_encryption/tests/test_view_wiring.py
@@ -0,0 +1,39 @@
+# Part of OpenSPP. See LICENSE file for full copyright and licensing details.
+"""The individuals form renders ID numbers through the masked widget."""
+
+from lxml import etree
+
+from odoo import Command
+from odoo.tests.common import TransactionCase
+
+
+class TestMaskedIdWiring(TransactionCase):
+ def test_individuals_form_masks_id_value(self):
+ """Both Identity tabs (individual and group) render reg_ids.value
+ with the masked_char widget and the PII reveal gate. get_view also
+ validates the inherited arch, so this fails fast if view validation
+ rejects the widget attributes."""
+ user = self.env["res.users"].create(
+ {
+ "name": "Registry Viewer",
+ "login": "registry_viewer_masked",
+ "group_ids": [Command.link(self.env.ref("base.group_user").id)],
+ }
+ )
+ view = self.env.ref("spp_registry.view_individuals_form")
+ arch = self.env["res.partner"].with_user(user).get_view(view_id=view.id)["arch"]
+ tree = etree.fromstring(arch)
+
+ value_fields = tree.xpath("//field[@name='reg_ids']/list/field[@name='value']")
+ self.assertEqual(len(value_fields), 2, "both Identity tabs must be wired")
+ for node in value_fields:
+ self.assertEqual(node.get("widget"), "masked_char")
+ self.assertEqual(node.get("mask_pattern"), "****-****-####")
+ self.assertEqual(
+ node.get("reveal_group"),
+ "spp_data_classification.group_pii_full_access_admin",
+ )
+
+ def test_reveal_group_exists(self):
+ """Guards against a rename of the PR2 group this module gates on."""
+ self.assertTrue(self.env.ref("spp_data_classification.group_pii_full_access_admin"))
diff --git a/spp_registry_encryption/views/registry_id_views.xml b/spp_registry_encryption/views/registry_id_views.xml
new file mode 100644
index 000000000..17af3ffad
--- /dev/null
+++ b/spp_registry_encryption/views/registry_id_views.xml
@@ -0,0 +1,38 @@
+
+OpenSPP Registry PII Display
+ + +
Applies OpenSPP’s PII display de-emphasis to the registry: registrant ID +numbers (national IDs, passports, and other spp.registry.id values) +render masked in the Identity tabs of the individual and group forms, +with an audited reveal control.
+
+
+What it does
+-
+
- ID numbers display as ••••-••••-1234 (mask pattern +****-****-####, matching the mask the classification registry +seeds for national IDs) instead of plaintext +
- The reveal control checks membership in +spp_data_classification.group_pii_full_access_admin — the PII +access group the RESTRICTED classification level points at +
- Every reveal through the control is recorded in the PII access audit +log (spp.pii.audit.log, action reveal) +
+
+What it is — and is not
+This is display de-emphasis, not an access control (see the “widget +honesty” findings in the spp_pii_encryption hardening tracker):
+-
+
- The masking applies to readonly display. Entering the editable list +cell shows the value in the input, without a group check or an audit +entry. +
- The value is delivered to the browser by the normal record read +regardless of mask state; the widget controls presentation, not data +access. +
- The real access boundary remains the record’s ACLs and record rules: +whoever can read/write spp.registry.id can obtain the value. +
- What the mask does buy: protection against shoulder-surfing and casual +over-exposure in day-to-day screens, plus an audit trail for +deliberate reveals through the control. +
Server-side field-level enforcement is tracked in the +spp_pii_encryption hardening issue; when it lands, this module is +where the registry adopts it.
+
+
+What it does NOT do
+The stored values are not encrypted by this module — applying +spp.encrypted.field.mixin to spp.registry.id (blind-index +search, encrypted storage) is a separate change with +search/deduplication impact.
+
+
+UI Location
+-
+
- Individual form > Identity tab > Identity Documents +
- Group form > Identity tab > Identity Documents +
+
+Security
+No models and no ACLs of its own. The reveal control’s group is +spp_data_classification.group_pii_full_access_admin; grant it to +officers who legitimately need to read full ID numbers. Note the mask +shows the last 4 characters to anyone who can read the record.
+
+
+Dependencies
+spp_registry, spp_pii_encryption, spp_data_classification
+
+
+Important
+This is an alpha version, the data model and design can change at any time without warning. +Only for development or testing purpose, do not use in production.
+Table of contents
+
+
+-
+
- Bug Tracker +
- Credits
-
+
- Authors +
- Maintainers +
+
+
+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 +feedback.
+Do not contact contributors directly about support or help with technical issues.
+
+
+Credits
+
+
+Authors
+-
+
- OpenSPP.org +
+
+Maintainers
+Current maintainers:
+
This module is part of the OpenSPP/OpenSPP2 project on GitHub.
+You are welcome to contribute.
+