Skip to content

[FIX] mail_notification_custom_subject: read templates with sudo#186

Open
sanderlienaerts wants to merge 1 commit intoOCA:18.0from
codeforward-bv:18.0-mail_notification_custom_subject-portal-user-access
Open

[FIX] mail_notification_custom_subject: read templates with sudo#186
sanderlienaerts wants to merge 1 commit intoOCA:18.0from
codeforward-bv:18.0-mail_notification_custom_subject-portal-user-access

Conversation

@sanderlienaerts
Copy link
Copy Markdown

Problem

mail.thread.message_post override in this module does:

custom_subjects = (
    self.env["mail.message.custom.subject"]
    .sudo()
    .search(domain)
    .sudo(False)
)
for template in custom_subjects:
    try:
        ... template.subject_template ...
        ... template.position ...
    except Exception:
        _logger.warning(...)

The .sudo(False) explicitly drops back to the caller's environment. When a non-internal user (portal or public) triggers message_post — typically via a portal controller that does record.sudo().message_post(...) — the subsequent field reads check ACL against the caller. The module's ACL only grants base.group_user read and base.group_system full access on mail.message.custom.subject, so portal/public users get an AccessError.

The per-template try/except Exception catches the error, so the message gets posted without the custom subject being applied (silent degradation). However the AccessError can also leak from places outside the try block — e.g. prefetch on related fields triggered by iteration — and surfaces to the caller as a hard failure.

Fix (this PR)

Read the templates with sudo and drop the .sudo(False) switch-back. These templates are admin-managed configuration records, analogous to mail.template / ir.sequence, and should always be readable by whoever invokes message_post. Template rendering itself still honors the caller's permissions on the business record via res_ids, so evaluating {{object.xxx}} expressions remains bounded by the user's access on the business model.

Alternative approach considered

Instead of the code change, the ACL could be extended with explicit portal and public read access:

access_mail_message_custom_subject_portal,...,base.group_portal,1,0,0,0
access_mail_message_custom_subject_public,...,base.group_public,1,0,0,0

This preserves the .sudo(False) pattern and makes permissions explicit. Trade-offs vs. the current PR:

  • For ACL approach: less invasive, keeps the original security-aware iteration pattern, makes permission grants auditable in the CSV.
  • Against ACL approach: exposes configuration records to portal/public users (harmless subject templates, but still visible); requires remembering to cover both base.group_portal and base.group_public; the pattern remains inconsistent — the .sudo() before .search(domain) already acknowledges that all matching templates must be found regardless of caller permissions, so it is odd to then flip back to .sudo(False) only for the reads that follow.
  • For the code change (current PR): single change covers all non-internal users, consistent with how mail.template is handled in Odoo core (admin-managed rendering config, always read with sudo during message flows).

Happy to switch to the ACL approach if maintainers prefer that.

Test

Added test_email_subject_template_portal_user that:

  1. Creates a base.group_portal user.
  2. Creates a template as admin.
  3. Posts a message as the portal user via partner.sudo().message_post(...) — the exact pattern used by portal controllers.
  4. Asserts the subject is rendered from the template.

Without this fix the test fails (subject falls back to Re: Test partner 1); with the fix it passes.

The mail.thread override searches mail.message.custom.subject with sudo
and then explicitly drops back to the caller's env with .sudo(False).
When iterating the resulting recordset, field reads trigger ACL checks
against the caller.

Portal and public users are allowed to trigger message_post through
portal controllers (which typically call record.sudo().message_post),
but they lack read access to mail.message.custom.subject — an
internal-only ACL. The resulting AccessError is silently caught by the
per-template try/except, but the custom subject is never applied for
those users, and the raw AccessError can leak when field access is
triggered outside the try block (e.g. via prefetch on related fields).

Templates are admin-managed configuration records, akin to
mail.template. They should be read with sudo, while rendering still
honors the caller's permissions on the business record via res_ids.

Add a test that reproduces the portal-user path: runs as a portal user
(asserts base.group_portal / not base.group_user), posts via
partner.sudo().message_post(..., author_id=portal_user.partner_id) —
the exact pattern used by portal controllers. Without the fix the
subject falls back to the default "Re: <name>"; with the fix it is
rendered from the template.
@OCA-git-bot
Copy link
Copy Markdown
Contributor

Hi @yajo,
some modules you are maintaining are being modified, check this out!

@OCA-git-bot OCA-git-bot added series:18.0 mod:mail_notification_custom_subject Module mail_notification_custom_subject labels Apr 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mod:mail_notification_custom_subject Module mail_notification_custom_subject series:18.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants