From 0d0363402c4add74d296b5db404a1fd5f5447eff Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 17 Sep 2026 10:35:54 +0800 Subject: [PATCH 1/4] fix(registry): drop broken /mail/message/update_content override The override was ported from Odoo 17, where it was already redundant: stock 17 granted author-or-administrator editing too, and the override only narrowed "administrator" to base.group_system and answered with an AccessError instead of a 404. On Odoo 19 it never worked: it called the removed ir.attachment._check_attachments_access, expected the Odoo 17 request shape while the web client now sends update_data, and returned through the removed message_format(). Every chatter Edit/Delete errored. Stock Odoo 19 grants author-or-administrator editing through ThreadController._can_edit_message, so the route is stock again. Tests speak the Odoo 19 payload, are un-skipped, and assert a real NotFound denial so a controller crash can no longer pass as a denial. Stale translation entries for the deleted error string are dropped. Closes #419 --- spp_registry/__manifest__.py | 2 +- spp_registry/controllers/mail.py | 26 ----- spp_registry/i18n/es.po | 6 -- spp_registry/i18n/fr.po | 6 -- spp_registry/i18n/spp_registry.pot | 6 -- spp_registry/readme/HISTORY.md | 4 + spp_registry/tests/test_mail_controllers.py | 105 +++++++++++--------- 7 files changed, 65 insertions(+), 90 deletions(-) diff --git a/spp_registry/__manifest__.py b/spp_registry/__manifest__.py index 2ed500e32..dd231ba18 100644 --- a/spp_registry/__manifest__.py +++ b/spp_registry/__manifest__.py @@ -3,7 +3,7 @@ { "name": "OpenSPP Registry", "category": "OpenSPP/Core", - "version": "19.0.2.2.4", + "version": "19.0.2.2.5", "sequence": 1, "author": "OpenSPP.org", "website": "https://github.com/OpenSPP/OpenSPP2", diff --git a/spp_registry/controllers/mail.py b/spp_registry/controllers/mail.py index 804d62fdc..75c01aca4 100644 --- a/spp_registry/controllers/mail.py +++ b/spp_registry/controllers/mail.py @@ -1,6 +1,5 @@ import logging -from markupsafe import Markup from werkzeug.exceptions import NotFound from odoo import http @@ -10,7 +9,6 @@ from odoo.tools.translate import _ from odoo.addons.mail.controllers.attachment import AttachmentController -from odoo.addons.mail.controllers.thread import ThreadController from odoo.addons.mail.tools.discuss import add_guest_to_context logger = logging.getLogger(__name__) @@ -54,27 +52,3 @@ def mail_attachment_delete(self, attachment_id, access_token=None): if attachment_sudo.res_model != "mail.compose.message" or attachment_sudo.res_id != 0: raise NotFound() attachment_sudo._delete_and_notify(message) - - -class SPPThreadController(ThreadController): - @http.route("/mail/message/update_content", methods=["POST"], type="jsonrpc", auth="public") - @add_guest_to_context - def mail_message_update_content(self, message_id, body, attachment_ids, attachment_tokens=None, partner_ids=None): - guest = request.env["mail.guest"]._get_guest_from_context() - guest.env["ir.attachment"].browse(attachment_ids)._check_attachments_access(attachment_tokens) - # nosemgrep: odoo-sudo-without-context - message_sudo = guest.env["mail.message"].browse(message_id).sudo().exists() - - # Check if current user is admin or the creator (user or guest) - is_admin = request.env.user.has_group("base.group_system") - is_author = message_sudo.is_current_user_or_guest_author - if not (is_admin or is_author): - raise AccessError(_("You do not have permission to update this message.")) - - if not message_sudo.model or not message_sudo.res_id: - raise NotFound() - body = Markup(body) if body else body # may contain HTML such as @mentions - guest.env[message_sudo.model].browse([message_sudo.res_id])._message_update_content( - message_sudo, body, attachment_ids=attachment_ids, partner_ids=partner_ids - ) - return message_sudo.message_format()[0] diff --git a/spp_registry/i18n/es.po b/spp_registry/i18n/es.po index 04300400f..da5dc88d1 100644 --- a/spp_registry/i18n/es.po +++ b/spp_registry/i18n/es.po @@ -1530,12 +1530,6 @@ msgstr "No puede seleccionar una fecha de nacimiento posterior a hoy" msgid "You do not have permission to delete this attachment." msgstr "No tiene permiso para eliminar este adjunto." -#. module: spp_registry -#. odoo-python -#: code:addons/spp_registry/controllers/mail.py:0 -msgid "You do not have permission to update this message." -msgstr "No tiene permiso para actualizar este mensaje." - #. module: spp_registry #. odoo-python #: code:addons/spp_registry/models/registrant.py:0 diff --git a/spp_registry/i18n/fr.po b/spp_registry/i18n/fr.po index 098f1760a..5de848642 100644 --- a/spp_registry/i18n/fr.po +++ b/spp_registry/i18n/fr.po @@ -1534,12 +1534,6 @@ msgstr "" msgid "You do not have permission to delete this attachment." msgstr "Vous n’avez pas l’autorisation de supprimer cette pièce jointe." -#. module: spp_registry -#. odoo-python -#: code:addons/spp_registry/controllers/mail.py:0 -msgid "You do not have permission to update this message." -msgstr "Vous n’avez pas l’autorisation de mettre à jour ce message." - #. module: spp_registry #. odoo-python #: code:addons/spp_registry/models/registrant.py:0 diff --git a/spp_registry/i18n/spp_registry.pot b/spp_registry/i18n/spp_registry.pot index 2c70d8880..9401f3252 100644 --- a/spp_registry/i18n/spp_registry.pot +++ b/spp_registry/i18n/spp_registry.pot @@ -1495,12 +1495,6 @@ msgstr "" msgid "You do not have permission to delete this attachment." msgstr "" -#. module: spp_registry -#. odoo-python -#: code:addons/spp_registry/controllers/mail.py:0 -msgid "You do not have permission to update this message." -msgstr "" - #. module: spp_registry #. odoo-python #: code:addons/spp_registry/models/registrant.py:0 diff --git a/spp_registry/readme/HISTORY.md b/spp_registry/readme/HISTORY.md index d2f80ecda..58797df54 100644 --- a/spp_registry/readme/HISTORY.md +++ b/spp_registry/readme/HISTORY.md @@ -1,3 +1,7 @@ +### 19.0.2.2.5 + +- fix(registry): remove the `/mail/message/update_content` controller override. It was ported from Odoo 17, where it was already redundant: stock 17 granted author-or-administrator editing too, and the override only narrowed "administrator" to `base.group_system` and answered with an access error instead of a 404. On Odoo 19 it no longer worked at all, breaking **Edit** and **Delete** on every chatter message across the instance: it called the removed `ir.attachment._check_attachments_access`, expected the Odoo 17 request shape while the Odoo 19 web client sends `update_data`, and returned through the removed `message_format()`. Odoo 19's own route already grants author-or-administrator editing, so the override is dropped and the route is stock again (#419) + ### 19.0.2.2.4 - fix(registry): refuse a date of birth in the future on every write path. `_birthdate_onchange` only runs in the form UI, so ORM `create`/`write`, CSV/Excel import and API writes (XML-RPC, API v2, DCI) all persisted a future `birthdate` — which the non-stored `age` compute then rendered as a negative number in views, exports and API reads. A stored-field constraint now enforces it server-side, comparing against the user's own today so a registrar east of UTC is not refused a birth recorded earlier that local day, and naming the record and the offending value so a bad row in a bulk import can be found. The onchange is kept as the friendlier silent-reset UX in the form (#362) diff --git a/spp_registry/tests/test_mail_controllers.py b/spp_registry/tests/test_mail_controllers.py index ae92d3ca7..011f942f2 100644 --- a/spp_registry/tests/test_mail_controllers.py +++ b/spp_registry/tests/test_mail_controllers.py @@ -1,17 +1,20 @@ # Part of OpenSPP. See LICENSE file for full copyright and licensing details. -"""Author/admin authorisation on the overridden mail endpoints. +"""Author/admin authorisation on the mail endpoints spp_registry relies on. -Covers spp_registry/controllers/mail.py: -- ``POST /mail/attachment/delete`` (SPPAttachmentController.mail_attachment_delete) -- ``POST /mail/message/update_content`` (SPPThreadController.mail_message_update_content) +- ``POST /mail/attachment/delete`` — overridden in + spp_registry/controllers/mail.py (SPPAttachmentController.mail_attachment_delete). + Its only application-level guard is:: -Both are ``auth="public"`` JSON-RPC endpoints whose only application-level -guard is:: + is_admin = request.env.user.has_group("base.group_system") + is_author = message.is_current_user_or_guest_author + if not (is_admin or is_author): + raise AccessError(...) - is_admin = request.env.user.has_group("base.group_system") - is_author = message.is_current_user_or_guest_author - if not (is_admin or is_author): - raise AccessError(...) +- ``POST /mail/message/update_content`` — stock Odoo 19 + (``mail.controllers.thread.ThreadController``). spp_registry used to override + it to let ``base.group_system`` edit any message; Odoo 19 grants that itself + (``_can_edit_message`` is author OR ``res.users._is_admin()``), so the override + was removed. The tests pin the behaviour spp_registry depends on. These tests assert: author allowed, admin allowed, third party denied, unauthenticated denied. They run as ``HttpCase`` so the controller stack @@ -141,18 +144,30 @@ def test_missing_attachment_returns_without_error(self): @tagged("post_install", "-at_install") class TestMailMessageUpdateContentController(HttpCase): - """``/mail/message/update_content`` — author/admin gate.""" + """``/mail/message/update_content`` — author/admin gate (stock Odoo 19 route). + + The payload mirrors what ``mail/static/src/core/common/message_model.js`` + sends: ``{"message_id": ..., "update_data": {"body": ..., "attachment_ids": []}}``. + A denied edit is a ``werkzeug.exceptions.NotFound`` (JSON-RPC error code + 404); the assertions check that name so a controller crash (``TypeError``, + ``AttributeError``) can never pass as a denial. + """ @classmethod def setUpClass(cls): super().setUpClass() + # Stock Odoo 19 also requires the editor to be allowed to *post* on the + # thread (``_mail_post_access`` = write on the document). Both users get + # contact write rights so the author can edit, and so the bystander is + # refused by the author gate alone rather than by lacking thread access. + groups = [(6, 0, [cls.env.ref("base.group_user").id, cls.env.ref("base.group_partner_manager").id])] cls.author = cls.env["res.users"].create( { "name": "Msg Author", "login": "spp_registry_msg_author", "email": "msg_author@example.test", "password": "author_pw", - "group_ids": [(6, 0, [cls.env.ref("base.group_user").id])], + "group_ids": groups, } ) cls.bystander = cls.env["res.users"].create( @@ -161,7 +176,7 @@ def setUpClass(cls): "login": "spp_registry_msg_bystander", "email": "msg_bystander@example.test", "password": "bystander_pw", - "group_ids": [(6, 0, [cls.env.ref("base.group_user").id])], + "group_ids": groups, } ) @@ -185,54 +200,54 @@ def _call_update(self, message_id, body="

updated

"): { "params": { "message_id": message_id, - "body": body, - "attachment_ids": [], + "update_data": {"body": body, "attachment_ids": []}, } } ), headers={"Content-Type": "application/json"}, ) + def _assert_updated(self, resp, msg, body_fragment): + self.assertEqual(resp.status_code, 200) + payload = resp.json() + self.assertNotIn("error", payload, f"expected a successful edit, got {payload!r}") + msg.invalidate_recordset(["body"]) + self.assertIn(body_fragment, msg.body) + self.assertNotIn("original", msg.body) + + def _assert_denied_not_found(self, resp, msg, original_body): + payload = resp.json() + self.assertIn("error", payload, f"expected error envelope, got {payload!r}") + error = payload["error"] + self.assertEqual(error.get("code"), 404, f"expected a 404 denial, got {error!r}") + self.assertEqual(error["data"]["name"], "werkzeug.exceptions.NotFound") + msg.invalidate_recordset(["body"]) + self.assertEqual(msg.body, original_body) + def test_author_can_update_own_message(self): - """FINDING: controller is BROKEN on Odoo 19. - - ``spp_registry/controllers/mail.py::mail_message_update_content`` - calls ``ir.attachment._check_attachments_access(attachment_tokens)``, - which no longer exists on ``ir.attachment`` in Odoo 19. The - method was renamed/removed upstream. Every call to the endpoint - fails with ``AttributeError`` — happens to surface as - ``error`` in the JSON-RPC envelope, so the bystander/unauth - tests below pass for the WRONG reason. - - TODO (fix the impl, not the test): port the controller to use - whatever upstream attachment-access check replaced - ``_check_attachments_access`` in Odoo 19. Once the controller - runs, drop this skip and the second-finding skip below. - """ - self.skipTest("BROKEN: controller calls removed Odoo 18 API — see docstring") + msg = self._post_message(self.author) + self.authenticate("spp_registry_msg_author", "author_pw") + resp = self._call_update(msg.id, body="

updated by author

") + self._assert_updated(resp, msg, "updated by author") def test_admin_can_update_any_message(self): - """Same Odoo 19 incompatibility as above — skip until controller - is fixed.""" - self.skipTest("BROKEN: controller calls removed Odoo 18 API — see test_author_can_update_own_message") + msg = self._post_message(self.author) + self.authenticate("admin", "admin") + resp = self._call_update(msg.id, body="

updated by admin

") + self._assert_updated(resp, msg, "updated by admin") def test_bystander_cannot_update_anothers_message(self): msg = self._post_message(self.author) original_body = msg.body self.authenticate("spp_registry_msg_bystander", "bystander_pw") resp = self._call_update(msg.id, body="

hostile edit

") - self.assertIn("error", resp.json()) - msg.invalidate_recordset(["body"]) - self.assertEqual(msg.body, original_body) + self._assert_denied_not_found(resp, msg, original_body) def test_unauthenticated_request_is_denied(self): msg = self._post_message(self.author) + original_body = msg.body + # No authenticate() call — HttpCase starts as the public user. The + # public user is read-only on res.partner, so the 404 comes from the + # thread-post-access gate before the author gate is even consulted. resp = self._call_update(msg.id, body="

anon edit

") - self.assertIn("error", resp.json()) - - def test_message_without_model_returns_not_found(self): - """If the message has no ``model`` / ``res_id`` the controller raises - ``werkzeug.exceptions.NotFound`` (404 over HTTP).""" - # TODO: create a mail.message with empty model/res_id (requires - # sudo + careful create vals) and assert a 404 / NotFound surface. - self.skipTest("not yet implemented — see TODO") + self._assert_denied_not_found(resp, msg, original_body) From 6526cb1cfcf9b070e945d82ff7c923c85335b028 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 17 Sep 2026 10:56:29 +0800 Subject: [PATCH 2/4] docs(registry): apply CI-generated README for 19.0.2.2.5 --- spp_registry/README.rst | 16 +++++++++++ spp_registry/static/description/index.html | 31 +++++++++++++++++----- 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/spp_registry/README.rst b/spp_registry/README.rst index 4eb0e993a..fc7043699 100644 --- a/spp_registry/README.rst +++ b/spp_registry/README.rst @@ -139,6 +139,22 @@ Dependencies Changelog ========= +19.0.2.2.5 +~~~~~~~~~~ + +- fix(registry): remove the ``/mail/message/update_content`` controller + override. It was ported from Odoo 17, where it was already redundant: + stock 17 granted author-or-administrator editing too, and the override + only narrowed "administrator" to ``base.group_system`` and answered + with an access error instead of a 404. On Odoo 19 it no longer worked + at all, breaking **Edit** and **Delete** on every chatter message + across the instance: it called the removed + ``ir.attachment._check_attachments_access``, expected the Odoo 17 + request shape while the Odoo 19 web client sends ``update_data``, and + returned through the removed ``message_format()``. Odoo 19's own route + already grants author-or-administrator editing, so the override is + dropped and the route is stock again (#419) + 19.0.2.2.4 ~~~~~~~~~~ diff --git a/spp_registry/static/description/index.html b/spp_registry/static/description/index.html index 48cfcf7c2..c12b574f2 100644 --- a/spp_registry/static/description/index.html +++ b/spp_registry/static/description/index.html @@ -518,6 +518,23 @@

Changelog

+

19.0.2.2.5

+
    +
  • fix(registry): remove the /mail/message/update_content controller +override. It was ported from Odoo 17, where it was already redundant: +stock 17 granted author-or-administrator editing too, and the override +only narrowed “administrator” to base.group_system and answered +with an access error instead of a 404. On Odoo 19 it no longer worked +at all, breaking Edit and Delete on every chatter message +across the instance: it called the removed +ir.attachment._check_attachments_access, expected the Odoo 17 +request shape while the Odoo 19 web client sends update_data, and +returned through the removed message_format(). Odoo 19’s own route +already grants author-or-administrator editing, so the override is +dropped and the route is stock again (#419)
  • +
+
+

19.0.2.2.4

  • fix(registry): refuse a date of birth in the future on every write @@ -538,7 +555,7 @@

    19.0.2.2.4

    SELECT id, display_name, birthdate FROM res_partner WHERE birthdate > CURRENT_DATE;
-
+

19.0.2.2.3

  • fix(registry): repair the stored status/is_ended computes on @@ -562,7 +579,7 @@

    19.0.2.2.3

    CREATE INDEX CONCURRENTLY IF NOT EXISTS spp_group_membership__ended_date_index ON spp_group_membership (ended_date) WHERE ended_date IS NOT NULL;
-
+

19.0.2.2.2

  • fix(registry): let an ID type be used again after its ID was removed. @@ -574,7 +591,7 @@

    19.0.2.2.2

    surfacing a database error (#1136)
-
+

19.0.2.2.1

  • feat(registry): registry configuration is consolidated into one @@ -585,7 +602,7 @@

    19.0.2.2.1

    framework refuses a settings save from anyone else (#1009)
-
+

19.0.2.1.4

  • fix(registry): remove the dead @api.constrains("age") @@ -597,7 +614,7 @@

    19.0.2.1.4

    dropped
-
+

19.0.2.1.3

  • fix(registry): show an ID Status column on the group form @@ -608,7 +625,7 @@

    19.0.2.1.3

    (#1110)
-
+

19.0.2.1.1

  • fix(views): add reusable x2many_no_padding JS widget that @@ -618,7 +635,7 @@

    19.0.2.1.1

    don’t bloat the layout (#943).
-
+

19.0.2.0.0

  • Initial migration to OpenSPP2
  • From 15a1b10fe399e0ffa42a4c0e9beff49de84b3350 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 17 Sep 2026 11:20:53 +0800 Subject: [PATCH 3/4] test(registry): cover the Delete payload and pin which gate refuses Review round on #522. The web client's Delete goes through the same route with removeParams, so it gets its own author/admin/bystander/ anonymous tests. Fixture users are now Registry Officers, the module's own persona with write on res.partner, instead of the generic Contact Creation group, so a regression in the module's ACL would surface here. The bystander tests assert the bystander may post on the thread, so the 404 can only come from the author gate. HISTORY now leads with the signature break and states the administrator-scope change. The unused logger in the controller module is dropped. --- spp_registry/controllers/mail.py | 4 - spp_registry/readme/HISTORY.md | 2 +- spp_registry/tests/test_mail_controllers.py | 104 +++++++++++++++----- 3 files changed, 78 insertions(+), 32 deletions(-) diff --git a/spp_registry/controllers/mail.py b/spp_registry/controllers/mail.py index 75c01aca4..e009ff502 100644 --- a/spp_registry/controllers/mail.py +++ b/spp_registry/controllers/mail.py @@ -1,5 +1,3 @@ -import logging - from werkzeug.exceptions import NotFound from odoo import http @@ -11,8 +9,6 @@ from odoo.addons.mail.controllers.attachment import AttachmentController from odoo.addons.mail.tools.discuss import add_guest_to_context -logger = logging.getLogger(__name__) - class SPPAttachmentController(AttachmentController): @http.route("/mail/attachment/delete", methods=["POST"], type="jsonrpc", auth="public") diff --git a/spp_registry/readme/HISTORY.md b/spp_registry/readme/HISTORY.md index 58797df54..385ae86c7 100644 --- a/spp_registry/readme/HISTORY.md +++ b/spp_registry/readme/HISTORY.md @@ -1,6 +1,6 @@ ### 19.0.2.2.5 -- fix(registry): remove the `/mail/message/update_content` controller override. It was ported from Odoo 17, where it was already redundant: stock 17 granted author-or-administrator editing too, and the override only narrowed "administrator" to `base.group_system` and answered with an access error instead of a 404. On Odoo 19 it no longer worked at all, breaking **Edit** and **Delete** on every chatter message across the instance: it called the removed `ir.attachment._check_attachments_access`, expected the Odoo 17 request shape while the Odoo 19 web client sends `update_data`, and returned through the removed `message_format()`. Odoo 19's own route already grants author-or-administrator editing, so the override is dropped and the route is stock again (#419) +- fix(registry): remove the `/mail/message/update_content` controller override. It was ported from Odoo 17, where it was already redundant: stock 17 granted author-or-administrator editing too, and the override only narrowed "administrator" to `base.group_system` and answered with an access error instead of a 404. On Odoo 19 it no longer worked at all, breaking **Edit** and **Delete** on every chatter message across the instance: its signature still expected the Odoo 17 request shape while the Odoo 19 web client sends `update_data`, so every call failed before running, and behind that it also called the removed `ir.attachment._check_attachments_access` and returned through the removed `message_format()`. Odoo 19's own route already grants author-or-administrator editing, so the override is dropped and the route is stock again. Who may edit another user's chatter message therefore follows Odoo's own administrator test (superuser or **Administration: Access Rights**, which **Administration: Settings** implies) instead of Settings only, and the editor must also be allowed to post on the record; no OpenSPP role sits in that gap (#419) ### 19.0.2.2.4 diff --git a/spp_registry/tests/test_mail_controllers.py b/spp_registry/tests/test_mail_controllers.py index 011f942f2..36b98acf5 100644 --- a/spp_registry/tests/test_mail_controllers.py +++ b/spp_registry/tests/test_mail_controllers.py @@ -11,15 +11,20 @@ raise AccessError(...) - ``POST /mail/message/update_content`` — stock Odoo 19 - (``mail.controllers.thread.ThreadController``). spp_registry used to override - it to let ``base.group_system`` edit any message; Odoo 19 grants that itself - (``_can_edit_message`` is author OR ``res.users._is_admin()``), so the override - was removed. The tests pin the behaviour spp_registry depends on. - -These tests assert: author allowed, admin allowed, third party denied, -unauthenticated denied. They run as ``HttpCase`` so the controller stack -(routing, ``@add_guest_to_context``, JSON-RPC envelope) is exercised end -to end — not just the controller method directly. + (``mail.controllers.thread.ThreadController``). spp_registry used to carry a + redundant Odoo 17 port of this route that stopped working on 19; it was + removed in 19.0.2.2.5 (#419). Stock ``_can_edit_message`` is author OR + ``res.users._is_admin()``, after ``_get_message_with_access(mode="create")`` + has checked that the caller may post on the thread. The tests pin the + behaviour spp_registry depends on. + +For ``/mail/message/update_content`` these tests assert: author allowed, +admin allowed, third party denied, unauthenticated denied — for both the +Edit and the Delete payload. For ``/mail/attachment/delete`` the author case +is still an unimplemented placeholder (see its skip). They run as +``HttpCase`` so the controller stack (routing, ``@add_guest_to_context``, +JSON-RPC envelope) is exercised end to end — not just the controller method +directly. """ import json @@ -146,21 +151,27 @@ def test_missing_attachment_returns_without_error(self): class TestMailMessageUpdateContentController(HttpCase): """``/mail/message/update_content`` — author/admin gate (stock Odoo 19 route). - The payload mirrors what ``mail/static/src/core/common/message_model.js`` - sends: ``{"message_id": ..., "update_data": {"body": ..., "attachment_ids": []}}``. - A denied edit is a ``werkzeug.exceptions.NotFound`` (JSON-RPC error code - 404); the assertions check that name so a controller crash (``TypeError``, - ``AttributeError``) can never pass as a denial. + The payloads mirror what ``mail/static/src/core/common/message_model.js`` + sends: Edit is ``{"message_id": ..., "update_data": {"body": ..., "attachment_ids": []}}`` + and Delete is the same route with ``removeParams`` (empty body, no + attachments). A denied call is a ``werkzeug.exceptions.NotFound`` (JSON-RPC + error code 404); the assertions check that name so a controller crash + (``TypeError``, ``AttributeError``) can never pass as a denial. """ + # What the Odoo 19 web client sends for "Delete" (``Message.removeParams``). + REMOVE_PARAMS = {"attachment_ids": [], "attachment_tokens": [], "body": "", "subject": "", "partner_ids": []} + @classmethod def setUpClass(cls): super().setUpClass() # Stock Odoo 19 also requires the editor to be allowed to *post* on the - # thread (``_mail_post_access`` = write on the document). Both users get - # contact write rights so the author can edit, and so the bystander is - # refused by the author gate alone rather than by lacking thread access. - groups = [(6, 0, [cls.env.ref("base.group_user").id, cls.env.ref("base.group_partner_manager").id])] + # thread (``_mail_post_access`` = write on the document). Both users are + # Registry Officers — the module's own persona with write on + # ``res.partner`` (``security/ir.model.access.csv``) — so the author can + # edit, and so the bystander is refused by the author gate alone rather + # than by lacking thread access (pinned in the bystander tests). + groups = [(6, 0, [cls.env.ref("base.group_user").id, cls.env.ref("spp_registry.group_registry_officer").id])] cls.author = cls.env["res.users"].create( { "name": "Msg Author", @@ -193,20 +204,25 @@ def _post_message(self, owner): } ) - def _call_update(self, message_id, body="

    updated

    "): + def _call_update(self, message_id, body="

    updated

    ", update_data=None): + if update_data is None: + update_data = {"body": body, "attachment_ids": []} return self.url_open( "/mail/message/update_content", - data=json.dumps( - { - "params": { - "message_id": message_id, - "update_data": {"body": body, "attachment_ids": []}, - } - } - ), + data=json.dumps({"params": {"message_id": message_id, "update_data": update_data}}), headers={"Content-Type": "application/json"}, ) + def _assert_bystander_may_post(self, msg): + """Both stock gates raise the same bare ``NotFound``. Pinning that the + bystander clears the thread-post gate means a 404 in the bystander + tests can only have come from the author gate.""" + thread = self.env["res.partner"].browse(msg.res_id) + self.assertTrue( + thread.with_user(self.bystander).has_access("write"), + "bystander must be able to post on the thread, so the 404 proves the author gate fired", + ) + def _assert_updated(self, resp, msg, body_fragment): self.assertEqual(resp.status_code, 200) payload = resp.json() @@ -239,6 +255,7 @@ def test_admin_can_update_any_message(self): def test_bystander_cannot_update_anothers_message(self): msg = self._post_message(self.author) original_body = msg.body + self._assert_bystander_may_post(msg) self.authenticate("spp_registry_msg_bystander", "bystander_pw") resp = self._call_update(msg.id, body="

    hostile edit

    ") self._assert_denied_not_found(resp, msg, original_body) @@ -251,3 +268,36 @@ def test_unauthenticated_request_is_denied(self): # thread-post-access gate before the author gate is even consulted. resp = self._call_update(msg.id, body="

    anon edit

    ") self._assert_denied_not_found(resp, msg, original_body) + + def test_author_can_delete_own_message(self): + msg = self._post_message(self.author) + self.authenticate("spp_registry_msg_author", "author_pw") + resp = self._call_update(msg.id, update_data=self.REMOVE_PARAMS) + self.assertEqual(resp.status_code, 200) + payload = resp.json() + self.assertNotIn("error", payload, f"expected a successful delete, got {payload!r}") + msg.invalidate_recordset(["body"]) + self.assertNotIn("original", msg.body) + + def test_admin_can_delete_any_message(self): + msg = self._post_message(self.author) + self.authenticate("admin", "admin") + resp = self._call_update(msg.id, update_data=self.REMOVE_PARAMS) + payload = resp.json() + self.assertNotIn("error", payload, f"expected a successful delete, got {payload!r}") + msg.invalidate_recordset(["body"]) + self.assertNotIn("original", msg.body) + + def test_bystander_cannot_delete_anothers_message(self): + msg = self._post_message(self.author) + original_body = msg.body + self._assert_bystander_may_post(msg) + self.authenticate("spp_registry_msg_bystander", "bystander_pw") + resp = self._call_update(msg.id, update_data=self.REMOVE_PARAMS) + self._assert_denied_not_found(resp, msg, original_body) + + def test_unauthenticated_delete_is_denied(self): + msg = self._post_message(self.author) + original_body = msg.body + resp = self._call_update(msg.id, update_data=self.REMOVE_PARAMS) + self._assert_denied_not_found(resp, msg, original_body) From 283b824483ed6001cd0ed2aa413a5270776d1ba3 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Thu, 17 Sep 2026 11:26:21 +0800 Subject: [PATCH 4/4] docs(registry): apply CI-generated README for the 19.0.2.2.5 wording --- spp_registry/README.rst | 17 +++++++++++------ spp_registry/static/description/index.html | 17 +++++++++++------ 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/spp_registry/README.rst b/spp_registry/README.rst index fc7043699..ffa8a1068 100644 --- a/spp_registry/README.rst +++ b/spp_registry/README.rst @@ -148,12 +148,17 @@ Changelog only narrowed "administrator" to ``base.group_system`` and answered with an access error instead of a 404. On Odoo 19 it no longer worked at all, breaking **Edit** and **Delete** on every chatter message - across the instance: it called the removed - ``ir.attachment._check_attachments_access``, expected the Odoo 17 - request shape while the Odoo 19 web client sends ``update_data``, and - returned through the removed ``message_format()``. Odoo 19's own route - already grants author-or-administrator editing, so the override is - dropped and the route is stock again (#419) + across the instance: its signature still expected the Odoo 17 request + shape while the Odoo 19 web client sends ``update_data``, so every + call failed before running, and behind that it also called the removed + ``ir.attachment._check_attachments_access`` and returned through the + removed ``message_format()``. Odoo 19's own route already grants + author-or-administrator editing, so the override is dropped and the + route is stock again. Who may edit another user's chatter message + therefore follows Odoo's own administrator test (superuser or + **Administration: Access Rights**, which **Administration: Settings** + implies) instead of Settings only, and the editor must also be allowed + to post on the record; no OpenSPP role sits in that gap (#419) 19.0.2.2.4 ~~~~~~~~~~ diff --git a/spp_registry/static/description/index.html b/spp_registry/static/description/index.html index c12b574f2..3f2fdc563 100644 --- a/spp_registry/static/description/index.html +++ b/spp_registry/static/description/index.html @@ -526,12 +526,17 @@

    19.0.2.2.5

    only narrowed “administrator” to base.group_system and answered with an access error instead of a 404. On Odoo 19 it no longer worked at all, breaking Edit and Delete on every chatter message -across the instance: it called the removed -ir.attachment._check_attachments_access, expected the Odoo 17 -request shape while the Odoo 19 web client sends update_data, and -returned through the removed message_format(). Odoo 19’s own route -already grants author-or-administrator editing, so the override is -dropped and the route is stock again (#419) +across the instance: its signature still expected the Odoo 17 request +shape while the Odoo 19 web client sends update_data, so every +call failed before running, and behind that it also called the removed +ir.attachment._check_attachments_access and returned through the +removed message_format(). Odoo 19’s own route already grants +author-or-administrator editing, so the override is dropped and the +route is stock again. Who may edit another user’s chatter message +therefore follows Odoo’s own administrator test (superuser or +Administration: Access Rights, which Administration: Settings +implies) instead of Settings only, and the editor must also be allowed +to post on the record; no OpenSPP role sits in that gap (#419)