[IMP] mail, im_livechat, *: simplify channel ACL

* = bus, crm_livechat, hr, mail_bot, test_discuss_full, test_mail,
    website_livechat

Now that livechat uses guest, we can write proper ACL for channel and
channel member to check if the current user/guest is a member.

This allows removing most sudo in code and to simplify search domains.
Remaining sudo in discuss folder have been reviewed and commented.

task-3394829

closes odoo/odoo#138330

Related: odoo/upgrade#5295
Signed-off-by: Sébastien Theys (seb) <seb@odoo.com>

addons/crm_livechat/tests/test_chatbot_lead.py

@@ -1,10 +1,10 @@
-# -*- coding: utf-8 -*-
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
 from odoo.addons.crm_livechat.tests import chatbot_common
-from odoo.tests.common import users
+from odoo.tests.common import tagged, users
 
 
+@tagged("post_install", "-at_install")
 class CrmChatbotCase(chatbot_common.CrmChatbotCase):
 
     @users('user_public')

addons/crm_livechat/tests/test_crm_lead.py

@@ -1,12 +1,12 @@
-# -*- coding: utf-8 -*-
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
 from odoo import Command
 from odoo.addons.crm.tests.common import TestCrmCommon
 from odoo.addons.mail.tests.common import mail_new_test_user
-from odoo.tests.common import users, HttpCase
+from odoo.tests.common import HttpCase, tagged, users
 
 
+@tagged("post_install", "-at_install")
 class TestLivechatLead(HttpCase, TestCrmCommon):
 
     @classmethod
@@ -59,17 +59,23 @@ class TestLivechatLead(HttpCase, TestCrmCommon):
         self.assertTrue(any(bool(m.guest_id) for m in channel.channel_member_ids))
 
         # public + someone else: no customer (as they were anonymous)
-        channel.write({
+        # sudo: discuss.channel.member - removing non-self member for test setup purposes
+        channel.sudo().write({
             'channel_partner_ids': [(4, self.user_sales_manager.partner_id.id)]
         })
         lead = channel._convert_visitor_to_lead(self.env.user.partner_id, '/lead TestLead command')
         self.assertEqual(lead.partner_id, self.env['res.partner'])
 
+    @users('user_sales_leads')
+    def test_crm_lead_creation_portal(self):
         # portal: should be set as customer
-        channel = self.env['discuss.channel'].create({
-            'name': 'Chat with Visitor',
-            'channel_partner_ids': [(4, self.user_portal.partner_id.id)]
+        self.authenticate("user_portal", "user_portal")
+        channel_info = self.make_jsonrpc_request("/im_livechat/get_session", {
+            'anonymous_name': 'Visitor',
+            'channel_id': self.livechat_channel.id,
+            'persisted': True,
         })
+        channel = self.env['discuss.channel'].browse(channel_info['id'])
         lead = channel._convert_visitor_to_lead(self.env.user.partner_id, '/lead TestLead command')
 
         self.assertEqual(

addons/hr/tests/test_channel.py

@@ -1,9 +1,10 @@
-# -*- coding: utf-8 -*-
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
 from odoo.addons.hr.tests.common import TestHrCommon
+from odoo.tests.common import tagged
 
 
+@tagged("post_install", "-at_install")
 class TestChannel(TestHrCommon):
 
     @classmethod

addons/im_livechat/controllers/attachment.py

@@ -1,5 +1,7 @@
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
+from werkzeug.exceptions import NotFound
+
 from odoo import _
 from odoo.http import route, request
 from odoo.addons.mail.controllers.attachment import AttachmentController
@@ -11,7 +13,9 @@ class LivechatAttachmentController(AttachmentController):
     @route()
     @add_guest_to_context
     def mail_attachment_upload(self, ufile, thread_id, thread_model, is_pending=False, **kwargs):
-        thread = request.env[thread_model]._get_from_context_or_raise(int(thread_id))
+        thread = request.env[thread_model].search([("id", "=", thread_id)])
+        if not thread:
+            raise NotFound()
         if (
             thread_model == "discuss.channel"
             and thread.channel_type == "livechat"

addons/im_livechat/controllers/main.py

@@ -6,7 +6,9 @@ from werkzeug.exceptions import NotFound
 from urllib.parse import urlsplit
 
 from odoo import http, tools, _, release
+from odoo.exceptions import UserError
 from odoo.http import request
+from odoo.tools import replace_exceptions
 from odoo.addons.base.models.assetsbundle import AssetsBundle
 from odoo.addons.mail.models.discuss.mail_guest import add_guest_to_context
 
@@ -192,12 +194,15 @@ class LivechatController(http.Controller):
                 'chatbot_script_id': chatbot_script.id if chatbot_script else None
             }
         channel = request.env['discuss.channel'].with_context(mail_create_nosubscribe=False).sudo().create(channel_vals)
-        __, guest = channel._find_or_create_persona_for_channel(
-            guest_name=self._get_guest_name(),
-            country_code=request.geoip.country_code,
-            timezone=request.env['mail.guest']._get_timezone_from_request(request),
-            post_joined_message=False
-        )
+        with replace_exceptions(UserError, by=NotFound()):
+            # sudo: mail.guest - creating a guest and their member in a dedicated channel created from livechat
+            __, guest = channel.sudo()._find_or_create_persona_for_channel(
+                guest_name=self._get_guest_name(),
+                country_code=request.geoip.country_code,
+                timezone=request.env['mail.guest']._get_timezone_from_request(request),
+                post_joined_message=False
+            )
+        channel = channel.with_context(guest=guest)  # a new guest was possibly created
         if not chatbot_script or chatbot_script.operator_partner_id != channel.livechat_operator_id:
             channel._broadcast([channel.livechat_operator_id.id])
         channel_info = channel._channel_info()[0]
@@ -264,16 +269,19 @@ class LivechatController(http.Controller):
         if channel:
             channel._email_livechat_transcript(email)
 
-    @http.route('/im_livechat/visitor_leave_session', type='json', auth="public")
+    @http.route("/im_livechat/visitor_leave_session", type="json", auth="public")
     @add_guest_to_context
     def visitor_leave_session(self, uuid):
-        """ Called when the livechat visitor leaves the conversation.
-         This will clean the chat request and warn the operator that the conversation is over.
-         This allows also to re-send a new chat request to the visitor, as while the visitor is
-         in conversation with an operator, it's not possible to send the visitor a chat request."""
-        discuss_channel = request.env['discuss.channel'].sudo().search([('uuid', '=', uuid)])
-        if not discuss_channel:
+        """Called when the livechat visitor leaves the conversation.
+        This will clean the chat request and warn the operator that the conversation is over.
+        This allows also to re-send a new chat request to the visitor, as while the visitor is
+        in conversation with an operator, it's not possible to send the visitor a chat request."""
+        # sudo: channel access is validated with uuid
+        channel_sudo = request.env["discuss.channel"].sudo().search([("uuid", "=", uuid)])
+        if not channel_sudo:
             return
-        channel_member_sudo = request.env["discuss.channel.member"]._get_as_sudo_from_context_or_raise(channel_id=discuss_channel.id)
-        channel_member_sudo._rtc_leave_call()
-        discuss_channel._close_livechat_session()
+        domain = [("channel_id", "=", channel_sudo.id), ("is_self", "=", True)]
+        member = request.env["discuss.channel.member"].search(domain)
+        # sudo: discuss.channel.rtc.session - member of current user can leave call
+        member.sudo()._rtc_leave_call()
+        channel_sudo._close_livechat_session()

addons/im_livechat/models/discuss_channel.py

@@ -2,6 +2,7 @@
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
 from odoo import api, fields, models, _
+from odoo.osv import expression
 from odoo.tools import email_normalize, html_escape, html2plaintext, plaintext2html
 
 from markupsafe import Markup
@@ -29,6 +30,17 @@ class DiscussChannel(models.Model):
     _sql_constraints = [('livechat_operator_id', "CHECK((channel_type = 'livechat' and livechat_operator_id is not null) or (channel_type != 'livechat'))",
                          'Livechat Operator ID is required for a channel of type livechat.')]
 
+    def _compute_is_accessible(self):
+        super()._compute_is_accessible()
+        if self.env.user.has_group("im_livechat.im_livechat_group_manager"):
+            self.filtered(lambda channel: channel.channel_type == "livechat").is_accessible = True
+
+    def _search_is_accessible(self, operator, operand):
+        res = super()._search_is_accessible(operator, operand)
+        if self.env.user.has_group("im_livechat.im_livechat_group_manager"):
+            return expression.OR([res, [("channel_type", "=", "livechat")]])
+        return res
+
     @api.depends('message_ids')
     def _compute_duration(self):
         for record in self:
@@ -50,7 +62,8 @@ class DiscussChannel(models.Model):
         channel_infos_dict = dict((c['id'], c) for c in channel_infos)
         for channel in self:
             if channel.chatbot_current_step_id:
-                channel_infos_dict[channel.id]["chatbot_script_id"] = channel.chatbot_current_step_id.chatbot_script_id.id
+                # sudo: chatbot.script.step - returning the current script of the channel
+                channel_infos_dict[channel.id]["chatbot_script_id"] = channel.chatbot_current_step_id.sudo().chatbot_script_id.id
             channel_infos_dict[channel.id]['anonymous_name'] = channel.anonymous_name
             channel_infos_dict[channel.id]['anonymous_country'] = {
                 'code': channel.country_id.code,

addons/im_livechat/models/mail_message.py

@@ -41,8 +41,9 @@ class MailMessage(models.Model):
                 if message_sudo.author_id.user_livechat_username:
                     del vals['author']['name']
                     vals['author']['user_livechat_username'] = message_sudo.author_id.user_livechat_username
+                # sudo: chatbot.script.step - checking whether the current message is from chatbot
                 if discuss_channel.chatbot_current_step_id \
-                        and message_sudo.author_id == discuss_channel.chatbot_current_step_id.chatbot_script_id.operator_partner_id:
+                        and message_sudo.author_id == discuss_channel.chatbot_current_step_id.sudo().chatbot_script_id.operator_partner_id:
                     chatbot_message_id = self.env['chatbot.message'].sudo().search([
                         ('mail_message_id', '=', message_sudo.id)], limit=1)
                     if chatbot_message_id.script_step_id:

addons/im_livechat/models/res_partner.py

@@ -34,17 +34,6 @@ class Partners(models.Model):
             })
         return result
 
-    def _get_channels_as_member(self):
-        channels = super()._get_channels_as_member()
-        channels |= self.env['discuss.channel'].search([
-            ('channel_type', '=', 'livechat'),
-            ('channel_member_ids', 'in', self.env['discuss.channel.member'].sudo()._search([
-                ('partner_id', '=', self.id),
-                ('is_pinned', '=', True),
-            ])),
-        ])
-        return channels
-
     @api.depends('user_ids.livechat_username')
     def _compute_user_livechat_username(self):
         for partner in self:

addons/im_livechat/security/im_livechat_channel_security.xml

@@ -1,6 +1,5 @@
 <?xml version="1.0"?>
 <odoo>
-
         <record id="module_category_im_livechat" model="ir.module.category">
             <field name="name">Live Chat</field>
             <field name="sequence" eval="20" />
@@ -21,17 +20,25 @@
         </record>
 
     <data noupdate="1">
-        <record id="im_livechat_rule_manager_read_all_discuss_channel" model="ir.rule">
-            <field name="name">Livechat: Administrator: read all livechat channel</field>
+        <record id="ir_rule_discuss_channel_group_im_livechat_group_manager" model="ir.rule">
+            <field name="name">discuss.channel: livechat manager can read all livechat channels</field>
             <field name="model_id" ref="mail.model_discuss_channel"/>
             <field name="groups" eval="[(4, ref('im_livechat_group_manager'))]"/>
             <field name="domain_force">[('channel_type', '=', 'livechat')]</field>
-            <field name="perm_read" eval="True"/>
             <field name="perm_create" eval="False"/>
             <field name="perm_write" eval="False"/>
             <field name="perm_unlink" eval="False"/>
         </record>
 
+        <record id="ir_rule_discuss_channel_member_group_im_livechat_group_manager" model="ir.rule">
+            <field name="name">discuss.channel.member: livechat manager can read all livechat channel members</field>
+            <field name="model_id" ref="mail.model_discuss_channel_member"/>
+            <field name="groups" eval="[(4, ref('im_livechat_group_manager'))]"/>
+            <field name="domain_force">[('channel_id.channel_type', '=', 'livechat')]</field>
+            <field name="perm_create" eval="False"/>
+            <field name="perm_write" eval="False"/>
+            <field name="perm_unlink" eval="False"/>
+        </record>
 
         <record id="base.default_user" model="res.users">
             <field name="groups_id" eval="[(4,ref('im_livechat.im_livechat_group_manager'))]"/>

addons/im_livechat/static/tests/helpers/mock_server/models/discuss_channel.js

@@ -77,15 +77,11 @@ patch(MockServer.prototype, {
     /**
      * Simulates `_find_or_create_persona_for_channel` on `discuss.channel`.
      */
-    _mockDiscussChannel__findOrCreatePersonaForChannel(channelId, guestName, addAsMember = true) {
+    _mockDiscussChannel__findOrCreatePersonaForChannel(channelId, guestName) {
         if (this._mockDiscussChannelMember__getAsSudoFromContext(channelId)) {
             return;
         }
-        const guestId = this._mockMailGuest__findOrCreateForChannel(
-            channelId,
-            guestName,
-            addAsMember
-        );
+        const guestId = this._mockMailGuest__findOrCreateForChannel(channelId, guestName);
         this._mockMailGuest__setAuthCookie(guestId);
     },
 });

addons/im_livechat/static/tests/helpers/mock_server/models/mail_guest.js

@@ -9,15 +9,13 @@ patch(MockServer.prototype, {
     /**
      * Simulates `_find_or_create_for_channel` on `mail.guest`.
      */
-    _mockMailGuest__findOrCreateForChannel(channelId, guestName, addAsMember = true) {
+    _mockMailGuest__findOrCreateForChannel(channelId, guestName) {
         const guestId =
             this._mockMailGuest__getGuestFromContext()?.id ??
             this.pyEnv["mail.guest"].create({ name: guestName });
-        if (addAsMember) {
-            this.pyEnv["discuss.channel"].write([channelId], {
-                channel_member_ids: [Command.create({ guest_id: guestId })],
-            });
-        }
+        this.pyEnv["discuss.channel"].write([channelId], {
+            channel_member_ids: [Command.create({ guest_id: guestId })],
+        });
         return guestId;
     },
     /**

addons/im_livechat/tests/test_chatbot_internals.py

@@ -1,10 +1,10 @@
-# -*- coding: utf-8 -*-
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
 from odoo.addons.im_livechat.tests import chatbot_common
 from odoo.exceptions import ValidationError
+from odoo.tests.common import tagged
 
-
+@tagged("post_install", "-at_install")
 class ChatbotCase(chatbot_common.ChatbotCase):
 
     def test_chatbot_duplicate(self):

addons/im_livechat/tests/test_get_discuss_channel.py

@@ -1,13 +1,15 @@
-# -*- coding: utf-8 -*-
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
+
 from datetime import timedelta
 from freezegun import freeze_time
 from unittest.mock import patch, PropertyMock
 
 from odoo import fields
 from odoo.addons.im_livechat.tests.common import TestImLivechatCommon
+from odoo.tests.common import tagged
 
 
+@tagged("post_install", "-at_install")
 class TestGetDiscussChannel(TestImLivechatCommon):
     def test_get_discuss_channel(self):
         """For a livechat with 5 available operators, we open 5 channels 5 times (25 channels total).
@@ -139,13 +141,20 @@ class TestGetDiscussChannel(TestImLivechatCommon):
         return discuss_channels
 
     def test_channel_not_pinned_for_operator_before_first_message(self):
-        channel_info = self.make_jsonrpc_request('/im_livechat/get_session', {'anonymous_name': 'whatever', 'channel_id': self.livechat_channel.id})
-        operator_channel_member = self.env['discuss.channel.member'].search([('channel_id', '=', channel_info['id']), ('partner_id', 'in', self.operators.partner_id.ids)])
-        self.assertEqual(len(operator_channel_member), 1, "operator should be member of channel")
-        self.assertFalse(operator_channel_member.is_pinned, "channel should not be pinned for operator initially")
-        self.env['discuss.channel'].browse(channel_info['id']).message_post(body='cc')
-        self.assertTrue(operator_channel_member.is_pinned, "channel should be pinned for operator after visitor sent a message")
-        self.assertIn(channel_info['id'], operator_channel_member.partner_id._get_channels_as_member().ids, "channel should be fetched by operator on new page")
+        operator = self.operators[0]
+        params = {
+            "anonymous_name": "whatever",
+            "channel_id": self.livechat_channel.id,
+            "previous_operator_id": operator.partner_id.id
+        }
+        channel_id = self.make_jsonrpc_request("/im_livechat/get_session", params)["id"]
+        member_domain = [("channel_id", "=", channel_id), ("is_self", "=", True)]
+        member = self.env["discuss.channel.member"].with_user(operator).search(member_domain)
+        self.assertEqual(len(member), 1, "operator should be member of channel")
+        self.assertFalse(member.is_pinned, "channel should not be pinned for operator initially")
+        self.env["discuss.channel"].browse(channel_id).message_post(body="cc")
+        self.assertTrue(member.is_pinned, "channel should be pinned for operator after visitor sent a message")
+        self.assertIn(channel_id, [c["id"] for c in operator._init_messaging()["channels"]], "channel should be fetched by operator on new page")
 
     def test_operator_livechat_username(self):
         """Ensures the operator livechat_username is returned by `_channel_fetch_message`, which is

addons/im_livechat/tests/test_im_livechat_report.py

@@ -1,11 +1,12 @@
-# -*- coding: utf-8 -*-
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
 from unittest.mock import patch
 
 from odoo.addons.im_livechat.tests.common import TestImLivechatCommon
+from odoo.tests.common import tagged
 
 
+@tagged("post_install", "-at_install")
 class TestImLivechatReport(TestImLivechatCommon):
     def setUp(self):
         super().setUp()

addons/im_livechat/views/discuss_channel_views.xml

@@ -8,7 +8,7 @@
             <field name="arch" type="xml">
                 <search string="Search history">
                     <field name="name" string="Participant"/>
-                    <filter name="filter_my_sessions" domain="[('channel_member_ids.partner_id.user_ids.id', 'in', [uid])]" string="My Sessions"/>
+                    <filter name="filter_my_sessions" domain="[('is_member', '=', True)]" string="My Sessions"/>
                     <separator/>
                     <filter name="filter_session_date" date="create_date" string="Session Date"/>
                     <separator/>

addons/mail/controllers/attachment.py

@@ -15,8 +15,8 @@ from ..models.discuss.mail_guest import add_guest_to_context
 
 logger = logging.getLogger(__name__)
 
-class AttachmentController(http.Controller):
 
+class AttachmentController(http.Controller):
     def _make_zip(self, name, attachments):
         streams = (request.env['ir.binary']._get_stream_from(record, 'raw') for record in attachments)
         # TODO: zip on-the-fly while streaming instead of loading the
@@ -47,7 +47,9 @@ class AttachmentController(http.Controller):
     @http.route("/mail/attachment/upload", methods=["POST"], type="http", auth="public")
     @add_guest_to_context
     def mail_attachment_upload(self, ufile, thread_id, thread_model, is_pending=False, **kwargs):
-        thread = request.env[thread_model]._get_from_context_or_raise(int(thread_id))
+        thread = request.env[thread_model].search([("id", "=", thread_id)])
+        if not thread:
+            raise NotFound()
         if thread_model == "discuss.channel" and not thread.allow_public_upload and not request.env.user._is_internal():
             raise AccessError(_("You are not allowed to upload attachments on this channel."))
         vals = {
@@ -65,11 +67,12 @@ class AttachmentController(http.Controller):
                     "res_model": "mail.compose.message",
                 }
             )
-        if thread.env.user.share:
+        if request.env.user.share:
             # Only generate the access token if absolutely necessary (= not for internal user).
-            vals["access_token"] = thread.env["ir.attachment"]._generate_access_token()
+            vals["access_token"] = request.env["ir.attachment"]._generate_access_token()
         try:
-            attachment = thread.env["ir.attachment"].create(vals)
+            # sudo: ir.attachment - posting a new attachment on an accessible thread
+            attachment = request.env["ir.attachment"].sudo().create(vals)
             attachment._post_add_create(**kwargs)
             attachmentData = attachment._attachment_format()[0]
             if attachment.access_token:
@@ -81,22 +84,23 @@ class AttachmentController(http.Controller):
     @http.route("/mail/attachment/delete", methods=["POST"], type="json", auth="public")
     @add_guest_to_context
     def mail_attachment_delete(self, attachment_id, access_token=None):
-        attachment_sudo = request.env["ir.attachment"].browse(int(attachment_id)).sudo().exists()
-        guest = request.env["mail.guest"]._get_guest_from_context()
-        message_sudo = guest.env["mail.message"].sudo().search([("attachment_ids", "in", attachment_sudo.ids)], limit=1)
-        if not attachment_sudo:
+        attachment = request.env["ir.attachment"].browse(int(attachment_id)).exists()
+        if not attachment:
             target = request.env.user.partner_id
             request.env["bus.bus"]._sendone(target, "ir.attachment/delete", {"id": attachment_id})
             return
+        message = request.env["mail.message"].search([("attachment_ids", "in", attachment.ids)], limit=1)
         if not request.env.user.share:
             # Check through standard access rights/rules for internal users.
-            attachment_sudo.sudo(False)._delete_and_notify(message_sudo)
+            attachment._delete_and_notify(message)
             return
         # For non-internal users 2 cases are supported:
         #   - Either the attachment is linked to a message: verify the request is made by the author of the message (portal user or guest).
         #   - Either a valid access token is given: also verify the message is pending (because unfortunately in portal a token is also provided to guest for viewing others' attachments).
-        if message_sudo:
-            if not message_sudo.is_current_user_or_guest_author:
+        # sudo: ir.attachment: access is validated below with membership of message or access token
+        attachment_sudo = attachment.sudo()
+        if message:
+            if not message.is_current_user_or_guest_author:
                 raise NotFound()
         else:
             if (
@@ -107,7 +111,7 @@ class AttachmentController(http.Controller):
                 raise NotFound()
             if attachment_sudo.res_model != "mail.compose.message" or attachment_sudo.res_id != 0:
                 raise NotFound()
-        attachment_sudo._delete_and_notify(message_sudo)
+        attachment_sudo._delete_and_notify(message)
 
     @http.route(['/mail/attachment/zip'], methods=["POST"], type="http", auth="public")
     def mail_attachment_get_zip(self, file_ids, zip_name, **kw):

addons/mail/controllers/discuss/binary.py

@@ -5,9 +5,10 @@ from werkzeug.exceptions import NotFound
 from odoo import http
 from odoo.http import request
 from odoo.addons.mail.models.discuss.mail_guest import add_guest_to_context
+from odoo.addons.web.controllers.binary import Binary
 
 
-class BinaryController(http.Controller):
+class BinaryController(Binary):
     @http.route(
         "/discuss/channel/<int:channel_id>/partner/<int:partner_id>/avatar_128",
         methods=["GET"],
@@ -16,81 +17,52 @@ class BinaryController(http.Controller):
     )
     @add_guest_to_context
     def discuss_channel_partner_avatar_128(self, channel_id, partner_id, **kwargs):
-        channel_member_sudo = request.env["discuss.channel.member"]._get_as_sudo_from_context(channel_id=channel_id)
-        partner_sudo = channel_member_sudo.env["res.partner"].browse(partner_id).exists()
-        placeholder = partner_sudo._avatar_get_placeholder_path()
+        channel = request.env["discuss.channel"].search([("id", "=", channel_id)])
+        partner = request.env["res.partner"].browse(partner_id).exists()
         domain = [("channel_id", "=", channel_id), ("partner_id", "=", partner_id)]
-        if channel_member_sudo and channel_member_sudo.env["discuss.channel.member"].search(domain, limit=1):
+        if channel and partner and request.env["discuss.channel.member"].search(domain):
+            # sudo: res.partner - the partner is in the same channel as the current user, so they can see their avatar
             return (
-                request.env["ir.binary"]
-                ._get_image_stream_from(partner_sudo, field_name="avatar_128", placeholder=placeholder)
-                .get_response()
+                request.env["ir.binary"]._get_image_stream_from(partner.sudo(), field_name="avatar_128").get_response()
             )
-        if request.env.user.share:
-            return request.env["ir.binary"]._get_placeholder_stream(placeholder).get_response()
-        return (
-            request.env["ir.binary"]
-            ._get_image_stream_from(partner_sudo.sudo(False), field_name="avatar_128", placeholder=placeholder)
-            .get_response()
-        )
+        return self.content_image(model="res.partner", id=partner_id, field="avatar_128")
 
     @http.route(
         "/discuss/channel/<int:channel_id>/guest/<int:guest_id>/avatar_128", methods=["GET"], type="http", auth="public"
     )
     @add_guest_to_context
     def discuss_channel_guest_avatar_128(self, channel_id, guest_id, **kwargs):
-        channel_member_sudo = request.env["discuss.channel.member"]._get_as_sudo_from_context(channel_id=channel_id)
-        guest_sudo = channel_member_sudo.env["mail.guest"].browse(guest_id).exists()
-        placeholder = guest_sudo._avatar_get_placeholder_path()
+        channel = request.env["discuss.channel"].search([("id", "=", channel_id)])
+        guest = request.env["mail.guest"].browse(guest_id).exists()
         domain = [("channel_id", "=", channel_id), ("guest_id", "=", guest_id)]
-        if channel_member_sudo and channel_member_sudo.env["discuss.channel.member"].search(domain, limit=1):
-            return (
-                request.env["ir.binary"]
-                ._get_image_stream_from(guest_sudo, field_name="avatar_128", placeholder=placeholder)
-                .get_response()
-            )
-        if request.env.user.share:
-            return request.env["ir.binary"]._get_placeholder_stream(placeholder).get_response()
-        return (
-            request.env["ir.binary"]
-            ._get_image_stream_from(guest_sudo.sudo(False), field_name="avatar_128", placeholder=placeholder)
-            .get_response()
-        )
+        if channel and guest and request.env["discuss.channel.member"].search(domain):
+            # sudo: mail.guest - the guest is in the same channel as the current user, so they can see their avatar
+            return request.env["ir.binary"]._get_image_stream_from(guest.sudo(), field_name="avatar_128").get_response()
+        return self.content_image(model="mail.guest", id=guest_id, field="avatar_128")
 
     @http.route(
         "/discuss/channel/<int:channel_id>/attachment/<int:attachment_id>", methods=["GET"], type="http", auth="public"
     )
     @add_guest_to_context
     def discuss_channel_attachment(self, channel_id, attachment_id, download=None, **kwargs):
-        channel_member_sudo = request.env["discuss.channel.member"]._get_as_sudo_from_context_or_raise(channel_id=int(channel_id))
+        channel = request.env["discuss.channel"].search([("id", "=", channel_id)])
+        if not channel:
+            raise NotFound()
         domain = [
             ("id", "=", int(attachment_id)),
             ("res_id", "=", int(channel_id)),
             ("res_model", "=", "discuss.channel"),
         ]
-        attachment_sudo = channel_member_sudo.env["ir.attachment"].search(domain, limit=1)
+        # sudo: ir.attachment - searching for an attachment on a specific channel that the current user can access
+        attachment_sudo = request.env["ir.attachment"].sudo().search(domain)
         if not attachment_sudo:
             raise NotFound()
         return request.env["ir.binary"]._get_stream_from(attachment_sudo).get_response(as_attachment=download)
 
-    @http.route(
-        "/discuss/channel/<int:channel_id>/avatar_128",
-        methods=["GET"],
-        type="http",
-        auth="public",
-    )
+    @http.route("/discuss/channel/<int:channel_id>/avatar_128", methods=["GET"], type="http", auth="public")
     @add_guest_to_context
     def discuss_channel_avatar_128(self, channel_id, **kwargs):
-        channel_member_sudo = request.env["discuss.channel.member"]._get_as_sudo_from_context_or_raise(channel_id=channel_id)
-        domain = [("id", "=", channel_id)]
-        channel_sudo = channel_member_sudo.env["discuss.channel"].search(domain, limit=1)
-        if not channel_sudo:
-            raise NotFound()
-        return (
-            request.env["ir.binary"]
-            ._get_image_stream_from(channel_sudo, field_name="avatar_128")
-            .get_response()
-        )
+        return self.content_image(model="discuss.channel", id=channel_id, field="avatar_128")
 
     @http.route(
         [
@@ -103,13 +75,16 @@ class BinaryController(http.Controller):
     )
     @add_guest_to_context
     def fetch_image(self, channel_id, attachment_id, width=0, height=0, **kwargs):
-        channel_member_sudo = request.env["discuss.channel.member"]._get_as_sudo_from_context_or_raise(channel_id=channel_id)
+        channel = request.env["discuss.channel"].search([("id", "=", channel_id)])
+        if not channel:
+            raise NotFound()
         domain = [
             ("id", "=", attachment_id),
             ("res_id", "=", channel_id),
             ("res_model", "=", "discuss.channel"),
         ]
-        attachment_sudo = channel_member_sudo.env["ir.attachment"].search(domain, limit=1)
+        # sudo: ir.attachment - searching for an attachment on a specific channel that the current user can access
+        attachment_sudo = request.env["ir.attachment"].sudo().search(domain, limit=1)
         if not attachment_sudo:
             raise NotFound()
         return (

addons/mail/controllers/discuss/channel.py

@@ -3,9 +3,7 @@
 from werkzeug.exceptions import NotFound
 
 from odoo import http
-from odoo.exceptions import UserError
 from odoo.http import request
-from odoo.tools import consteq
 from odoo.addons.mail.models.discuss.mail_guest import add_guest_to_context
 
 
@@ -13,29 +11,10 @@ class ChannelController(http.Controller):
     @http.route("/discuss/channel/members", methods=["POST"], type="json", auth="public")
     @add_guest_to_context
     def discuss_channel_members(self, channel_id, known_member_ids):
-        channel_member = request.env["discuss.channel.member"]._get_as_sudo_from_context_or_raise(channel_id=channel_id)
-        return channel_member.channel_id.sudo().load_more_members(known_member_ids)
-
-    @http.route("/discuss/channel/add_guest_as_member", methods=["POST"], type="json", auth="public")
-    @add_guest_to_context
-    def discuss_channel_add_guest_as_member(self, channel_id, channel_uuid):
-        channel_sudo = request.env["discuss.channel"].browse(int(channel_id)).sudo().exists()
-        if not channel_sudo or not channel_sudo.uuid or not consteq(channel_sudo.uuid, channel_uuid):
+        channel = request.env["discuss.channel"].search([("id", "=", channel_id)])
+        if not channel:
             raise NotFound()
-        if channel_sudo.channel_type == "chat":
-            raise NotFound()
-        guest = channel_sudo.env["mail.guest"]._get_guest_from_context()
-        # Only guests should take this route.
-        if not guest:
-            raise NotFound()
-        channel_member = channel_sudo.env["discuss.channel.member"]._get_as_sudo_from_context(channel_id=channel_id)
-        # Do not add the guest to channel members if they are already member.
-        if not channel_member:
-            channel_sudo = channel_sudo.with_context(guest=guest)
-            try:
-                channel_sudo.add_members(guest_ids=[guest.id])
-            except UserError:
-                raise NotFound()
+        return channel.load_more_members(known_member_ids)
 
     @http.route("/discuss/channel/update_avatar", methods=["POST"], type="json")
     def discuss_channel_avatar_update(self, channel_id, data):
@@ -47,19 +26,22 @@ class ChannelController(http.Controller):
     @http.route("/discuss/channel/info", methods=["POST"], type="json", auth="public")
     @add_guest_to_context
     def discuss_channel_info(self, channel_id):
-        member_sudo = request.env["discuss.channel.member"]._get_as_sudo_from_context(channel_id=int(channel_id))
-        return member_sudo.channel_id._channel_info()
+        return request.env["discuss.channel"].search([("id", "=", channel_id)])._channel_info()
 
     @http.route("/discuss/channel/messages", methods=["POST"], type="json", auth="public")
     @add_guest_to_context
     def discuss_channel_messages(self, channel_id, search_term=None, before=None, after=None, limit=30, around=None):
-        channel_member_sudo = request.env["discuss.channel.member"]._get_as_sudo_from_context_or_raise(channel_id=int(channel_id))
+        channel = request.env["discuss.channel"].search([("id", "=", channel_id)])
+        if not channel:
+            raise NotFound()
         domain = [
             ("res_id", "=", channel_id),
             ("model", "=", "discuss.channel"),
             ("message_type", "!=", "user_notification"),
         ]
-        res = channel_member_sudo.env["mail.message"]._message_fetch(domain, search_term=search_term, before=before, after=after, around=around, limit=limit)
+        res = request.env["mail.message"]._message_fetch(
+            domain, search_term=search_term, before=before, after=after, around=around, limit=limit
+        )
         if not request.env.user._is_public() and not around:
             res["messages"].set_message_done()
         return {**res, "messages": res["messages"].message_format()}
@@ -67,20 +49,26 @@ class ChannelController(http.Controller):
     @http.route("/discuss/channel/pinned_messages", methods=["POST"], type="json", auth="public")
     @add_guest_to_context
     def discuss_channel_pins(self, channel_id):
-        channel_member_sudo = request.env["discuss.channel.member"]._get_as_sudo_from_context_or_raise(channel_id=int(channel_id))
-        return channel_member_sudo.channel_id.pinned_message_ids.sorted(key="pinned_at", reverse=True).message_format()
+        channel = request.env["discuss.channel"].search([("id", "=", channel_id)])
+        if not channel:
+            raise NotFound()
+        return channel.pinned_message_ids.sorted(key="pinned_at", reverse=True).message_format()
 
     @http.route("/discuss/channel/set_last_seen_message", methods=["POST"], type="json", auth="public")
     @add_guest_to_context
     def discuss_channel_mark_as_seen(self, channel_id, last_message_id, allow_older=False):
-        channel_member_sudo = request.env["discuss.channel.member"]._get_as_sudo_from_context_or_raise(channel_id=int(channel_id))
-        return channel_member_sudo.channel_id._channel_seen(last_message_id, allow_older=allow_older)
+        channel = request.env["discuss.channel"].search([("id", "=", channel_id)])
+        if not channel:
+            raise NotFound()
+        return channel._channel_seen(last_message_id, allow_older=allow_older)
 
     @http.route("/discuss/channel/notify_typing", methods=["POST"], type="json", auth="public")
     @add_guest_to_context
     def discuss_channel_notify_typing(self, channel_id, is_typing):
-        channel_member_sudo = request.env["discuss.channel.member"]._get_as_sudo_from_context_or_raise(channel_id=int(channel_id))
-        channel_member_sudo._notify_typing(is_typing)
+        member = request.env["discuss.channel.member"].search([("channel_id", "=", channel_id), ("is_self", "=", True)])
+        if not member:
+            raise NotFound()
+        member._notify_typing(is_typing)
 
     @http.route("/discuss/channel/attachments", methods=["POST"], type="json", auth="public")
     @add_guest_to_context
@@ -91,11 +79,14 @@ class ChannelController(http.Controller):
         :param limit: maximum number of attachments to return
         :param before: id of the attachment from which to load older attachments
         """
-        channel_member_sudo = request.env["discuss.channel.member"]._get_as_sudo_from_context_or_raise(channel_id=channel_id)
+        channel = request.env["discuss.channel"].search([("id", "=", channel_id)])
+        if not channel:
+            raise NotFound()
         domain = [
             ["res_id", "=", channel_id],
             ["res_model", "=", "discuss.channel"],
         ]
         if before:
             domain.append(["id", "<", before])
-        return channel_member_sudo.env["ir.attachment"].search(domain, limit=limit, order="id DESC")._attachment_format()
+        # sudo: ir.attachment - reading attachments of a channel that the current user can access
+        return request.env["ir.attachment"].sudo().search(domain, limit=limit, order="id DESC")._attachment_format()

addons/mail/controllers/discuss/gif.py

@@ -2,6 +2,7 @@
 
 import requests
 import werkzeug.urls
+
 from odoo.http import request, route, Controller
 
 
@@ -15,6 +16,7 @@ class DiscussGifController(Controller):
 
     @route("/discuss/gif/search", type="json", auth="user")
     def search(self, search_term, locale="en", country="US", position=None):
+        # sudo: ir.config_parameter - read keys are hard-coded and values are only used for server requests
         ir_config = request.env["ir.config_parameter"].sudo()
         query_string = werkzeug.urls.url_encode(
             {
@@ -35,6 +37,7 @@ class DiscussGifController(Controller):
 
     @route("/discuss/gif/categories", type="json", auth="user")
     def categories(self, locale="en", country="US"):
+        # sudo: ir.config_parameter - read keys are hard-coded and values are only used for server requests
         ir_config = request.env["ir.config_parameter"].sudo()
         query_string = werkzeug.urls.url_encode(
             {
@@ -55,6 +58,7 @@ class DiscussGifController(Controller):
         request.env["discuss.gif.favorite"].create({"tenor_gif_id": tenor_gif_id})
 
     def _gif_posts(self, ids):
+        # sudo: ir.config_parameter - read keys are hard-coded and values are only used for server requests
         ir_config = request.env["ir.config_parameter"].sudo()
         query_string = werkzeug.urls.url_encode(
             {

addons/mail/controllers/discuss/public_page.py

@@ -5,8 +5,9 @@ from psycopg2.errorcodes import UNIQUE_VIOLATION
 from werkzeug.exceptions import NotFound
 
 from odoo import _, http
+from odoo.exceptions import UserError
 from odoo.http import request
-from odoo.tools import consteq
+from odoo.tools import consteq, replace_exceptions
 from odoo.addons.mail.models.discuss.mail_guest import add_guest_to_context
 
 
@@ -42,20 +43,25 @@ class PublicPageController(http.Controller):
     @http.route("/chat/<int:channel_id>/<string:invitation_token>", methods=["GET"], type="http", auth="public")
     @add_guest_to_context
     def discuss_channel_invitation(self, channel_id, invitation_token):
-        channel_sudo = request.env["discuss.channel"].browse(channel_id).sudo().exists()
-        if not channel_sudo or not channel_sudo.uuid or not consteq(channel_sudo.uuid, invitation_token):
+        channel = request.env["discuss.channel"].browse(channel_id).exists()
+        # sudo: discuss.channel - channel access is validated with invitation_token
+        if not channel or not channel.sudo().uuid or not consteq(channel.sudo().uuid, invitation_token):
             raise NotFound()
-        return self._response_discuss_channel_invitation(channel_sudo=channel_sudo)
+        return self._response_discuss_channel_invitation(channel)
 
     @http.route("/discuss/channel/<int:channel_id>", methods=["GET"], type="http", auth="public")
     @add_guest_to_context
     def discuss_channel(self, channel_id):
-        channel_member_sudo = request.env["discuss.channel.member"]._get_as_sudo_from_context_or_raise(channel_id=int(channel_id))
-        return self._response_discuss_public_template(channel_sudo=channel_member_sudo.channel_id)
+        channel = request.env["discuss.channel"].search([("id", "=", channel_id)])
+        if not channel:
+            raise NotFound()
+        return self._response_discuss_public_template(channel)
 
     def _response_discuss_channel_from_token(self, create_token, channel_name=None, default_display_mode=False):
+        # sudo: ir.config_parameter - reading hard-coded key and using it in a simple condition
         if not request.env["ir.config_parameter"].sudo().get_param("mail.chat_from_token"):
             raise NotFound()
+        # sudo: discuss.channel - channel access is validated with invitation_token
         channel_sudo = request.env["discuss.channel"].sudo().search([("uuid", "=", create_token)])
         if not channel_sudo:
             try:
@@ -75,49 +81,43 @@ class PublicPageController(http.Controller):
                 # commit the current transaction and get the channel.
                 request.env.cr.commit()
                 channel_sudo = channel_sudo.search([("uuid", "=", create_token)])
-        return self._response_discuss_channel_invitation(channel_sudo=channel_sudo, is_channel_token_secret=False)
+        return self._response_discuss_channel_invitation(channel_sudo.sudo(False), is_channel_token_secret=False)
 
-    def _response_discuss_channel_invitation(self, channel_sudo, is_channel_token_secret=True):
-        if channel_sudo.channel_type == "chat":
-            raise NotFound()
+    def _response_discuss_channel_invitation(self, channel, is_channel_token_secret=True):
         discuss_public_view_data = {
             "isChannelTokenSecret": is_channel_token_secret,
         }
-        guest_already_known = channel_sudo.env["mail.guest"]._get_guest_from_context().exists()
-        __, guest = channel_sudo._find_or_create_persona_for_channel(
-            guest_name=_("Guest"),
-            country_code=request.geoip.country_code,
-            timezone=request.env['mail.guest']._get_timezone_from_request(request),
-            add_as_member=guest_already_known or not channel_sudo.env.user._is_public(),
-        )
+        guest_already_known = channel.env["mail.guest"]._get_guest_from_context()
+        with replace_exceptions(UserError, by=NotFound()):
+            # sudo: mail.guest - creating a guest and its member inside a channel of which they have the token
+            __, guest = channel.sudo()._find_or_create_persona_for_channel(
+                guest_name=_("Guest"),
+                country_code=request.geoip.country_code,
+                timezone=request.env["mail.guest"]._get_timezone_from_request(request),
+            )
         if guest and not guest_already_known:
             discuss_public_view_data.update(
                 {
-                    "addGuestAsMemberOnJoin": True,
                     "shouldDisplayWelcomeViewInitially": True,
                 }
             )
-        channel_sudo = channel_sudo.with_context(guest=guest)
-        response = self._response_discuss_public_template(
-            channel_sudo=channel_sudo, discuss_public_view_data=discuss_public_view_data
-        )
-        return response
+            channel = channel.with_context(guest=guest)
+        return self._response_discuss_public_template(channel, discuss_public_view_data=discuss_public_view_data)
 
-    def _response_discuss_public_template(self, channel_sudo, discuss_public_view_data=None):
+    def _response_discuss_public_template(self, channel, discuss_public_view_data=None):
         discuss_public_view_data = discuss_public_view_data or {}
         return request.render(
             "mail.discuss_public_channel_template",
             {
                 "data": {
-                    "channelData": channel_sudo._channel_info()[0],
+                    "channelData": channel._channel_info()[0],
                     "discussPublicViewData": dict(
                         {
-                            "shouldDisplayWelcomeViewInitially": channel_sudo.default_display_mode
-                            == "video_full_screen",
+                            "shouldDisplayWelcomeViewInitially": channel.default_display_mode == "video_full_screen",
                         },
                         **discuss_public_view_data,
                     ),
                 },
-                "session_info": channel_sudo.env["ir.http"].session_info(),
+                "session_info": channel.env["ir.http"].session_info(),
             },
         )

addons/mail/controllers/discuss/rtc.py

@@ -1,6 +1,7 @@
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
 from collections import defaultdict
+from werkzeug.exceptions import NotFound
 
 from odoo import http
 from odoo.http import request
@@ -23,7 +24,8 @@ class RtcController(http.Controller):
         guest = request.env["mail.guest"]._get_guest_from_context()
         notifications_by_session = defaultdict(list)
         for sender_session_id, target_session_ids, content in peer_notifications:
-            session_sudo = guest.env["discuss.channel.rtc.session"].sudo().browse(int(sender_session_id)).exists()
+            # sudo: discuss.channel.rtc.session - only keeping sessions matching the current user
+            session_sudo = request.env["discuss.channel.rtc.session"].sudo().browse(int(sender_session_id)).exists()
             if (
                 not session_sudo
                 or (session_sudo.guest_id and session_sudo.guest_id != guest)
@@ -45,11 +47,13 @@ class RtcController(http.Controller):
         if request.env.user._is_public():
             guest = request.env["mail.guest"]._get_guest_from_context()
             if guest:
+                # sudo: discuss.channel.rtc.session - only keeping sessions matching the current user
                 session = guest.env["discuss.channel.rtc.session"].sudo().browse(int(session_id)).exists()
                 if session and session.guest_id == guest:
                     session._update_and_broadcast(values)
                     return
             return
+        # sudo: discuss.channel.rtc.session - only keeping sessions matching the current user
         session = request.env["discuss.channel.rtc.session"].sudo().browse(int(session_id)).exists()
         if session and session.partner_id == request.env.user.partner_id:
             session._update_and_broadcast(values)
@@ -60,8 +64,11 @@ class RtcController(http.Controller):
         """Joins the RTC call of a channel if the user is a member of that channel
         :param int channel_id: id of the channel to join
         """
-        channel_member_sudo = request.env["discuss.channel.member"]._get_as_sudo_from_context_or_raise(channel_id=int(channel_id))
-        return channel_member_sudo._rtc_join_call(check_rtc_session_ids=check_rtc_session_ids)
+        member = request.env["discuss.channel.member"].search([("channel_id", "=", channel_id), ("is_self", "=", True)])
+        if not member:
+            raise NotFound()
+        # sudo: discuss.channel.rtc.session - member of current user can join call
+        return member.sudo()._rtc_join_call(check_rtc_session_ids=check_rtc_session_ids)
 
     @http.route("/mail/rtc/channel/leave_call", methods=["POST"], type="json", auth="public")
     @add_guest_to_context
@@ -69,8 +76,11 @@ class RtcController(http.Controller):
         """Disconnects the current user from a rtc call and clears any invitation sent to that user on this channel
         :param int channel_id: id of the channel from which to disconnect
         """
-        channel_member_sudo = request.env["discuss.channel.member"]._get_as_sudo_from_context_or_raise(channel_id=int(channel_id))
-        return channel_member_sudo._rtc_leave_call()
+        member = request.env["discuss.channel.member"].search([("channel_id", "=", channel_id), ("is_self", "=", True)])
+        if not member:
+            raise NotFound()
+        # sudo: discuss.channel.rtc.session - member of current user can leave call
+        return member.sudo()._rtc_leave_call()
 
     @http.route("/mail/rtc/channel/cancel_call_invitation", methods=["POST"], type="json", auth="public")
     @add_guest_to_context
@@ -79,8 +89,11 @@ class RtcController(http.Controller):
         :param member_ids: members whose invitation is to cancel
         :type member_ids: list(int) or None
         """
-        channel_member_sudo = request.env["discuss.channel.member"]._get_as_sudo_from_context_or_raise(channel_id=int(channel_id))
-        return channel_member_sudo.channel_id._rtc_cancel_invitations(member_ids=member_ids)
+        member = request.env["discuss.channel.member"].search([("channel_id", "=", channel_id), ("is_self", "=", True)])
+        if not member:
+            raise NotFound()
+        # sudo: discuss.channel.rtc.session - member of current user can cancel invitations
+        return member.sudo().channel_id._rtc_cancel_invitations(member_ids=member_ids)
 
     @http.route("/mail/rtc/audio_worklet_processor", methods=["GET"], type="http", auth="public")
     def audio_worklet_processor(self):
@@ -92,18 +105,22 @@ class RtcController(http.Controller):
             file_open("mail/static/src/worklets/audio_processor.js", "rb").read(),
             headers=[
                 ("Content-Type", "application/javascript"),
-                ("Cache-Control", "max-age=%s" % http.STATIC_CACHE),
+                ("Cache-Control", f"max-age={http.STATIC_CACHE}"),
             ],
         )
 
     @http.route("/discuss/channel/ping", methods=["POST"], type="json", auth="public")
     @add_guest_to_context
     def channel_ping(self, channel_id, rtc_session_id=None, check_rtc_session_ids=None):
-        channel_member_sudo = request.env["discuss.channel.member"]._get_as_sudo_from_context(channel_id=int(channel_id))
+        member = request.env["discuss.channel.member"].search([("channel_id", "=", channel_id), ("is_self", "=", True)])
+        if not member:
+            raise NotFound()
+        # sudo: discuss.channel.rtc.session - member of current user can access related sessions
+        channel_member_sudo = member.sudo()
         if rtc_session_id:
             domain = [
                 ("id", "=", int(rtc_session_id)),
-                ("channel_member_id", "=", channel_member_sudo.id),
+                ("channel_member_id", "=", member.id),
             ]
             channel_member_sudo.channel_id.rtc_session_ids.filtered_domain(domain).write({})  # update write_date
         current_rtc_sessions, outdated_rtc_sessions = channel_member_sudo._rtc_sync_sessions(check_rtc_session_ids)

addons/mail/controllers/message_reaction.py

@@ -11,8 +11,7 @@ class MessageReactionController(http.Controller):
     @http.route("/mail/message/reaction", methods=["POST"], type="json", auth="public")
     @add_guest_to_context
     def mail_message_add_reaction(self, message_id, content, action):
-        guest_sudo = request.env["mail.guest"]._get_guest_from_context().sudo()
-        message_sudo = guest_sudo.env["mail.message"].browse(int(message_id)).exists()
-        if not message_sudo._validate_access_for_current_persona("write"):
+        message = request.env["mail.message"].browse(int(message_id)).exists()
+        if not message._validate_access_for_current_persona("write"):
             raise NotFound()
-        message_sudo._message_reaction(content, action)
+        message.sudo()._message_reaction(content, action)

addons/mail/controllers/thread.py

@@ -93,7 +93,9 @@ class ThreadController(http.Controller):
                 'last_used': datetime.now(),
                 'ids': canned_response_ids,
             })
-        thread = request.env[thread_model]._get_from_context_or_raise(int(thread_id))
+        thread = request.env[thread_model].search([("id", "=", thread_id)])
+        if not thread:
+            raise NotFound()
         if "body" in post_data:
             post_data["body"] = Markup(post_data["body"])  # contains HTML such as @mentions
         new_partners = []

addons/mail/controllers/webclient.py

@@ -11,13 +11,13 @@ class WebclientController(http.Controller):
     @http.route("/mail/init_messaging", methods=["POST"], type="json", auth="public")
     @add_guest_to_context
     def mail_init_messaging(self):
-        if not request.env.user.sudo()._is_public():
-            return request.env.user.sudo(request.env.user.has_group("base.group_portal"))._init_messaging()
+        if not request.env.user._is_public():
+            return request.env.user.sudo(False)._init_messaging()
         guest = request.env["mail.guest"]._get_guest_from_context()
         if guest:
-            return guest.sudo()._init_messaging()
+            return guest._init_messaging()
         raise NotFound()
 
     @http.route("/mail/load_message_failures", methods=["POST"], type="json", auth="user")
     def mail_load_message_failures(self):
-        return request.env.user.partner_id._message_fetch_failed()
+        return request.env.user.sudo(False).partner_id._message_fetch_failed()

addons/mail/models/discuss/discuss_channel.py

@@ -7,7 +7,6 @@ from hashlib import sha512
 from markupsafe import Markup
 from secrets import choice
 from markupsafe import Markup
-from werkzeug.exceptions import NotFound
 
 from odoo import _, api, fields, models, tools, Command
 from odoo.addons.base.models.avatar_mixin import get_hsl_from_seed
@@ -29,8 +28,6 @@ group_avatar = '''<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 530.06 53
 
 
 class Channel(models.Model):
-    """ A discuss.channel is a discussion group that may behave like a listener
-    on documents. """
     _description = 'Discussion Channel'
     _name = 'discuss.channel'
     _mail_flat_thread = False
@@ -63,11 +60,8 @@ class Channel(models.Model):
     channel_partner_ids = fields.Many2many(
         'res.partner', string='Partners',
         compute='_compute_channel_partner_ids', inverse='_inverse_channel_partner_ids',
-        compute_sudo=True, search='_search_channel_partner_ids',
-        groups='base.group_user')
-    channel_member_ids = fields.One2many(
-        'discuss.channel.member', 'channel_id', string='Members',
-        groups='base.group_user')
+        search='_search_channel_partner_ids')
+    channel_member_ids = fields.One2many('discuss.channel.member', 'channel_id', string='Members')
     pinned_message_ids = fields.One2many('mail.message', 'res_id', domain=[('model', '=', 'discuss.channel'), ('pinned_at', '!=', False)], string='Pinned Messages')
     sfu_channel_uuid = fields.Char(groups="base.group_system")
     sfu_server_url = fields.Char(groups="base.group_system")
@@ -95,14 +89,16 @@ class Channel(models.Model):
 
     # CONSTRAINTS
 
-    @api.constrains('channel_member_ids', 'channel_partner_ids')
+    @api.constrains('channel_member_ids')
     def _constraint_partners_chat(self):
+        # sudo: discuss.channel - skipping ACL for constraint, more performant and no sensitive information is leaked
         for ch in self.sudo().filtered(lambda ch: ch.channel_type == 'chat'):
-            if len(ch.channel_member_ids) > 2 or len(ch.channel_partner_ids) > 2:
+            if len(ch.channel_member_ids) > 2:
                 raise ValidationError(_("A channel of type 'chat' cannot have more than two users."))
 
     @api.constrains('group_public_id', 'group_ids')
     def _constraint_group_id_channel(self):
+        # sudo: discuss.channel - skipping ACL for constraint, more performant and no sensitive information is leaked
         failing_channels = self.sudo().filtered(lambda channel: channel.channel_type != 'channel' and (channel.group_public_id or channel.group_ids))
         if failing_channels:
             raise ValidationError(_("For %(channels)s, channel_type should be 'channel' to have the group-based authorization or group auto-subscription.", channels=', '.join([ch.name for ch in failing_channels])))
@@ -118,9 +114,9 @@ class Channel(models.Model):
     def _compute_is_editable(self):
         for channel in self:
             if channel.channel_type == 'channel':
-                channel.is_editable = self.sudo(False).env.is_admin() or channel.create_uid.id == self.env.user.id
+                channel.is_editable = self.env.user._is_admin() or channel.create_uid.id == self.env.user.id
             elif channel.channel_type == 'group':
-                channel.is_editable = channel.is_member and not self.sudo(False).env.user._is_public()
+                channel.is_editable = channel.is_member and not self.env.user.share
             else:
                 channel.is_editable = False
 
@@ -155,53 +151,27 @@ class Channel(models.Model):
                 'partner_id': partner.id,
             } for partner in partners_new]
             outdated += current_members.filtered(lambda m: m.partner_id not in partners)
-
         if new_members:
             self.env['discuss.channel.member'].create(new_members)
         if outdated:
-            outdated.sudo().unlink()
+            outdated.unlink()
 
     def _search_channel_partner_ids(self, operator, operand):
-        return [(
-            'channel_member_ids',
-            'in',
-            self.env['discuss.channel.member'].sudo()._search([
-                ('partner_id', operator, operand)
-            ])
-        )]
+        return [('channel_member_ids', 'any', [('partner_id', operator, operand)])]
 
     @api.depends_context('uid', 'guest')
     @api.depends('channel_member_ids')
     def _compute_is_member(self):
         if not self:
             return
-        if self.env.user._is_public():
-            guest = self.env['mail.guest']._get_guest_from_context()
-            if not guest:
-                self.is_member = False
-                return
-            user_domain = [('guest_id', '=', guest.id)]
-        else:
-            user_domain = [('partner_id', '=', self.env.user.partner_id.id)]
-        members = self.env['discuss.channel.member'].sudo().search(expression.AND([[('channel_id', 'in', self.ids)], user_domain]))
-        is_member_channels = {member.channel_id for member in members}
+        members = self.env['discuss.channel.member'].search([('channel_id', 'in', self.ids), ('is_self', '=', True)])
+        is_member_channels = members.channel_id
         for channel in self:
             channel.is_member = channel in is_member_channels
 
     def _search_is_member(self, operator, operand):
         is_in = (operator == '=' and operand) or (operator == '!=' and not operand)
-        if self.env.user._is_public():
-            guest = self.env['mail.guest']._get_guest_from_context()
-            if not guest:
-                return expression.FALSE_DOMAIN if is_in else expression.TRUE_DOMAIN
-            user_domain = [('guest_id', '=', guest.id)]
-        else:
-            user_domain = [('partner_id', '=', self.env.user.partner_id.id)]
-        return [(
-            'channel_member_ids',
-            'in' if is_in else 'not in',
-            self.env['discuss.channel.member'].sudo()._search(user_domain)
-        )]
+        return [('channel_member_ids', "any" if is_in else "not any", [('is_self', '=', True)])]
 
     @api.depends('channel_member_ids')
     def _compute_member_count(self):
@@ -256,7 +226,8 @@ class Channel(models.Model):
 
         # Create channel and alias
         channels = super(Channel, self.with_context(mail_create_bypass_create_check=self.env['discuss.channel.member']._bypass_create_check, mail_create_nolog=True, mail_create_nosubscribe=True)).create(vals_list)
-
+        # pop the mail_create_bypass_create_check key to avoid leaking it outside of create)
+        channels = channels.with_context(mail_create_bypass_create_check=None)
         channels._subscribe_users_automatically()
 
         return channels
@@ -273,7 +244,7 @@ class Channel(models.Model):
 
     def write(self, vals):
         if 'channel_type' in vals:
-            failing_channels = self.sudo().filtered(lambda channel: channel.channel_type != vals.get('channel_type'))
+            failing_channels = self.filtered(lambda channel: channel.channel_type != vals.get('channel_type'))
             if failing_channels:
                 raise UserError(_('Cannot change the channel type of: %(channel_names)s', channel_names=', '.join(failing_channels.mapped('name'))))
         notifications = []
@@ -311,13 +282,6 @@ class Channel(models.Model):
         if not self._cr.fetchone():
             self._cr.execute('CREATE INDEX discuss_channel_member_seen_message_id_idx ON discuss_channel_member (channel_id,partner_id,seen_message_id)')
 
-    @api.model
-    def _get_from_context_or_raise(self, thread_id):
-        """Overridden because guests and (portal) users need sudo to post on channels. This ensures they are actually
-        members before granting them access, as well as properly setting up the guest context on the resulting thread
-        if applicable."""
-        return self.env["discuss.channel.member"]._get_as_sudo_from_context_or_raise(thread_id).channel_id
-
     # ------------------------------------------------------------
     # MEMBERS MANAGEMENT
     # ------------------------------------------------------------
@@ -330,6 +294,7 @@ class Channel(models.Model):
                 for channel_id in new_members
                 for partner_id in new_members[channel_id]
             ]
+            # sudo: discuss.channel.member - adding member of other users based on channel auto-subscribe
             self.env['discuss.channel.member'].sudo().create(to_create)
 
     def _subscribe_users_automatically_get_members(self):
@@ -344,22 +309,21 @@ class Channel(models.Model):
 
     def _action_unfollow(self, partner):
         self.message_unsubscribe(partner.ids)
-        if partner not in self.with_context(active_test=False).channel_partner_ids:
+        member = self.env['discuss.channel.member'].search([('channel_id', '=', self.id), ('partner_id', '=', partner.id)])
+        if not member:
             return True
         channel_info = self._channel_info()[0]  # must be computed before leaving the channel (access rights)
-        member = self.env['discuss.channel.member'].search([('channel_id', '=', self.id), ('partner_id', '=', partner.id)])
-        member_id = member.id
         member.unlink()
         # side effect of unsubscribe that wasn't taken into account because
         # channel_info is called before actually unpinning the channel
         channel_info['is_pinned'] = False
         self.env['bus.bus']._sendone(partner, 'discuss.channel/leave', channel_info)
         notification = Markup('<div class="o_mail_notification">%s</div>') % _('left the channel')
-        # post 'channel left' message as root since the partner just unsubscribed from the channel
+        # sudo: mail.message - post as sudo since the user just unsubscribed from the channel
         self.sudo().message_post(body=notification, subtype_xmlid="mail.mt_comment", author_id=partner.id)
         self.env['bus.bus']._sendone(self, 'mail.record/insert', {
             'Thread': {
-                'channelMembers': [('DELETE', {'id': member_id})],
+                'channelMembers': [('DELETE', {'id': member.id})],
                 'id': self.id,
                 'memberCount': self.member_count,
                 'model': "discuss.channel",
@@ -368,30 +332,12 @@ class Channel(models.Model):
 
     def add_members(self, partner_ids=None, guest_ids=None, invite_to_rtc_call=False, open_chat_window=False, post_joined_message=True):
         """ Adds the given partner_ids and guest_ids as member of self channels. """
-        self.check_access_rights('write')
-        self.check_access_rule('write')
         current_partner, current_guest = self.env["res.partner"]._get_current_persona()
         partners = self.env['res.partner'].browse(partner_ids or []).exists()
         guests = self.env['mail.guest'].browse(guest_ids or []).exists()
         notifications = []
         for channel in self:
             members_to_create = []
-            if channel.group_public_id:
-                invalid_partners = partners.filtered(lambda partner: channel.group_public_id not in partner.user_ids.groups_id)
-                if invalid_partners:
-                    raise UserError(_(
-                        'Channel "%(channel_name)s" only accepts members of group "%(group_name)s". Forbidden for: %(partner_names)s',
-                        channel_name=channel.name,
-                        group_name=channel.group_public_id.name,
-                        partner_names=', '.join(partner.name for partner in invalid_partners)
-                    ))
-                if guests:
-                    raise UserError(_(
-                        'Channel "%(channel_name)s" only accepts members of group "%(group_name)s". Forbidden for: %(guest_names)s',
-                        channel_name=channel.name,
-                        group_name=channel.group_public_id.name,
-                        guest_names=', '.join(guest.name for guest in guests)
-                    ))
             existing_members = self.env['discuss.channel.member'].search(expression.AND([
                 [('channel_id', '=', channel.id)],
                 expression.OR([
@@ -407,17 +353,16 @@ class Channel(models.Model):
                 'guest_id': guest.id,
                 'channel_id': channel.id,
             } for guest in guests - existing_members.guest_id]
-            new_members = self.env['discuss.channel.member'].sudo().create(members_to_create)
+            new_members = self.env['discuss.channel.member'].create(members_to_create)
             for member in new_members.filtered(lambda member: member.partner_id):
                 # notify invited members through the bus
                 user = member.partner_id.user_ids[0] if member.partner_id.user_ids else self.env['res.users']
                 if user:
                     notifications.append((member.partner_id, 'discuss.channel/joined', {
-                        'channel': member.channel_id.with_user(user).with_context(allowed_company_ids=user.company_ids.ids).sudo()._channel_info()[0],
+                        'channel': member.channel_id.with_user(user).with_context(allowed_company_ids=user.company_ids.ids)._channel_info()[0],
                         'invited_by_user_id': self.env.user.id,
                         'open_chat_window': open_chat_window,
                     }))
-
                 if post_joined_message:
                     # notify existing members with a new message in the channel
                     if member.partner_id == self.env.user.partner_id:
@@ -432,7 +377,7 @@ class Channel(models.Model):
                 guest = member.guest_id
                 if guest:
                     notifications.append((guest, 'discuss.channel/joined', {
-                        'channel': member.channel_id.sudo()._channel_info()[0],
+                        'channel': member.channel_id.with_context(guest=guest)._channel_info()[0],
                     }))
             notifications.append((channel, 'mail.record/insert', {
                 'Thread': {
@@ -442,7 +387,7 @@ class Channel(models.Model):
                     'model': "discuss.channel",
                 }
             }))
-            if existing_members:
+            if existing_members and (current_partner or current_guest):
                 # If the current user invited these members but they are already present, notify the current user about their existence as well.
                 # In particular this fixes issues where the current user is not aware of its own member in the following case:
                 # create channel from form view, and then join from discuss without refreshing the page.
@@ -456,31 +401,13 @@ class Channel(models.Model):
                 }))
         if invite_to_rtc_call:
             for channel in self:
-                current_channel_member = self.env['discuss.channel.member'].sudo().search([('channel_id', '=', channel.id), ('partner_id', '=', current_partner.id), ('guest_id', '=', current_guest.id)])
-                if current_channel_member and current_channel_member.rtc_session_ids:
-                    current_channel_member._rtc_invite_members(member_ids=new_members.ids)
+                current_channel_member = self.env['discuss.channel.member'].search([('channel_id', '=', channel.id), ('is_self', '=', 'True')])
+                # sudo: discuss.channel.rtc.session - reading rtc sessions of current user
+                if current_channel_member and current_channel_member.sudo().rtc_session_ids:
+                    # sudo: discuss.channel.rtc.session - current user can invite new members in call
+                    current_channel_member.sudo()._rtc_invite_members(member_ids=new_members.ids)
         self.env['bus.bus']._sendmany(notifications)
 
-    def _can_invite(self, partner_id):
-        """Return True if the current user can invite the partner to the channel.
-
-          * channel -- public channel: ok;
-          *         -- group restricted channel: both current user and target must in the group;
-          * chat/group: current user must be member;
-
-        :return boolean: whether inviting is ok"""
-        partner = self.env['res.partner'].browse(partner_id)
-
-        for channel in self.sudo():
-            if channel.channel_type != 'channel' and not channel.is_member:
-                return False
-            if channel.group_public_id:
-                if not partner.user_ids or channel.group_public_id not in partner.user_ids.groups_id:
-                    return False
-                if channel.group_public_id not in self.env.user.groups_id:
-                    return False
-        return True
-
     # ------------------------------------------------------------
     # RTC
     # ------------------------------------------------------------
@@ -538,9 +465,8 @@ class Channel(models.Model):
           and use;
         """
         # get values from msg_vals or from message if msg_vals doen't exists
-        msg_sudo = message.sudo()
-        message_type = msg_vals.get('message_type', 'email') if msg_vals else msg_sudo.message_type
-        pids = msg_vals.get('partner_ids', []) if msg_vals else msg_sudo.partner_ids.ids
+        message_type = msg_vals.get('message_type', 'email') if msg_vals else message.message_type
+        pids = msg_vals.get('partner_ids', []) if msg_vals else message.partner_ids.ids
 
         # notify only user input (comment or incoming emails)
         if message_type not in ('comment', 'email'):
@@ -549,8 +475,8 @@ class Channel(models.Model):
         if not pids:
             return []
 
-        email_from = tools.email_normalize(msg_vals.get('email_from') or msg_sudo.email_from)
-        author_id = msg_vals.get('author_id') or msg_sudo.author_id.id
+        email_from = tools.email_normalize(msg_vals.get('email_from') or message.email_from)
+        author_id = msg_vals.get('author_id') or message.author_id.id
 
         recipients_data = []
         if pids:
@@ -601,16 +527,15 @@ class Channel(models.Model):
 
         message_format_values = message.message_format()[0]
         bus_notifications = self._channel_message_notifications(message, message_format_values)
-        # Last interest and is_pinned are updated for a chat when posting a message.
+        # Last interest and is_pinned are updated for a channel when posting a message.
         # So a notification is needed to update UI, and it should come before the
         # notification of the message itself to ensure the channel automatically opens.
-        if self.is_chat or self.channel_type in ['channel', 'group']:
-            for member in self.channel_member_ids.filtered('partner_id'):
-                bus_notifications.insert(0, [member.partner_id, 'discuss.channel/last_interest_dt_changed', {
-                    'id': self.id,
-                    'isServerPinned': member.is_pinned,
-                    'last_interest_dt': member.last_interest_dt,
-                }])
+        bus_notifications.insert(0, [self, 'discuss.channel/last_interest_dt_changed', {
+            'id': self.id,
+            'isServerPinned': True,
+            'last_interest_dt': fields.Datetime.now(),
+        }])
+        # sudo: bus.bus - sending on safe channel (target channel or partner)
         self.env['bus.bus'].sudo()._sendmany(bus_notifications)
         if self.is_chat or self.channel_type == 'group':
             self._notify_thread_by_web_push(message, rdata, msg_vals, **kwargs)
@@ -639,11 +564,14 @@ class Channel(models.Model):
 
     @api.returns('mail.message', lambda value: value.id)
     def message_post(self, *, message_type='notification', **kwargs):
-        self.filtered(lambda channel: channel.is_chat or channel.channel_type in ['channel', 'group']).mapped('channel_member_ids').sudo().write({
+        if (not self.env.user or self.env.user._is_public()) and self.is_member:
+            # sudo: discuss.channel - guests don't have access for creating mail.message
+            self = self.sudo()
+        # sudo: discuss.channel.member - updating hard-coded fields/values for non-self members
+        self.sudo().channel_member_ids.write({
             'is_pinned': True,
             'last_interest_dt': fields.Datetime.now(),
         })
-
         # mail_post_autofollow=False is necessary to prevent adding followers
         # when using mentions in channels. Followers should not be added to
         # channels, and especially not automatically (because channel membership
@@ -772,30 +700,25 @@ class Channel(models.Model):
             }
             self.message_post(body=notification, message_type="notification", subtype_xmlid="mail.mt_comment")
 
-    def _find_or_create_persona_for_channel(self, guest_name, timezone, country_code, add_as_member=True, post_joined_message=True):
+    def _find_or_create_persona_for_channel(self, guest_name, timezone, country_code, post_joined_message=True):
         """
         :param channel: channel to add the persona to
         :param guest_name: name of the persona
-        :param add_as_member: whether to add the persona as a member of the channel
         :param post_joined_message: whether to post a message to the channel
             to notify that the persona joined
         :return tuple(partner, guest):
         """
         self.ensure_one()
         guest = None
-        member = self.env["discuss.channel.member"]._get_as_sudo_from_context(channel_id=self.id)
+        member = self.env["discuss.channel.member"].search([("channel_id", "=", self.id), ("is_self", "=", True)])
         if member:
             return member.partner_id, member.guest_id
-        if not self.env.user._is_public() and add_as_member:
-            try:
-                self.add_members([self.env.user.partner_id.id], post_joined_message=post_joined_message)
-            except UserError:
-                raise NotFound()
+        if not self.env.user._is_public():
+            self.add_members([self.env.user.partner_id.id], post_joined_message=post_joined_message)
         elif self.env.user._is_public():
-            is_guest_known = self.env["mail.guest"]._get_guest_from_context().exists()
-            country_id = self.env["res.country"].search([("code", "=", country_code)], limit=1).id
+            is_guest_known = self.env["mail.guest"]._get_guest_from_context()
+            country_id = self.env["res.country"].search([("code", "=", country_code)]).id
             guest = self.env["mail.guest"]._find_or_create_for_channel(
-                add_as_member=add_as_member,
                 channel=self,
                 country_id=country_id,
                 name=guest_name,
@@ -814,6 +737,7 @@ class Channel(models.Model):
         if not self:
             return []
         channel_infos = []
+        # sudo: discuss.channel.rtc.session - reading sessions of accessible channel is acceptable
         rtc_sessions_by_channel = self.sudo().rtc_session_ids._mail_rtc_session_format_by_channel()
         current_partner, current_guest = self.env["res.partner"]._get_current_persona()
         self.env['discuss.channel'].flush_model()
@@ -837,14 +761,14 @@ class Channel(models.Model):
                ORDER BY discuss_channel_member.id ASC
         """, {'channel_ids': tuple(self.ids), 'current_partner_id': current_partner.id or None, 'current_guest_id': current_guest.id or None})
         all_needed_members = self.env['discuss.channel.member'].browse([m['id'] for m in self.env.cr.dictfetchall()])
-        all_needed_members.partner_id.sudo().mail_partner_format()  # prefetch in batch
+        all_needed_members._discuss_channel_member_format()  # prefetch in batch
         members_by_channel = defaultdict(lambda: self.env['discuss.channel.member'])
         invited_members_by_channel = defaultdict(lambda: self.env['discuss.channel.member'])
         member_of_current_user_by_channel = defaultdict(lambda: self.env['discuss.channel.member'])
         for member in all_needed_members:
-            members_by_channel[member.channel_id] |= member
+            members_by_channel[member.channel_id] += member
             if member.rtc_inviting_session_id:
-                invited_members_by_channel[member.channel_id] |= member
+                invited_members_by_channel[member.channel_id] += member
             if (current_partner and member.partner_id == current_partner) or (current_guest and member.guest_id == current_guest):
                 member_of_current_user_by_channel[member.channel_id] = member
         for channel in self:
@@ -976,7 +900,6 @@ class Channel(models.Model):
         else:
             # create a new one
             channel = self.create({
-                'channel_partner_ids': [Command.link(partner_id) for partner_id in partners_to],
                 'channel_member_ids': [
                     Command.create({
                         'partner_id': partner_id,
@@ -985,7 +908,7 @@ class Channel(models.Model):
                     }) for partner_id in partners_to
                 ],
                 'channel_type': 'chat',
-                'name': ', '.join(self.env['res.partner'].sudo().browse(partners_to).mapped('name')),
+                'name': ', '.join(self.env['res.partner'].browse(partners_to).mapped('name')),
             })
             channel._broadcast(partners_to)
         return channel
@@ -1133,7 +1056,7 @@ class Channel(models.Model):
         self.write({'description': description})
 
     def channel_join(self):
-        """ Shortcut to add the current user as member of self channels.
+        """Shortcut to add the current user as member of self channels.
         Prefer calling add_members() directly when possible.
         """
         self.add_members(self.env.user.partner_id.ids)
@@ -1179,7 +1102,7 @@ class Channel(models.Model):
             'default_display_mode': default_display_mode,
             'name': name,
         })
-        channel._broadcast(partners_to)
+        channel._broadcast(channel.channel_member_ids.partner_id.ids)
         return channel
 
     @api.model

addons/mail/models/discuss/discuss_channel_member.py

@@ -1,12 +1,9 @@
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
-from werkzeug.exceptions import NotFound
-
 from odoo import api, fields, models, _
-from odoo.exceptions import AccessError, ValidationError
+from odoo.exceptions import AccessError, UserError, ValidationError
 from odoo.osv import expression
 
-
 class ChannelMember(models.Model):
     _name = "discuss.channel.member"
     _description = "Channel Member"
@@ -16,6 +13,7 @@ class ChannelMember(models.Model):
     # identity
     partner_id = fields.Many2one("res.partner", "Partner", ondelete="cascade", index=True)
     guest_id = fields.Many2one("mail.guest", "Guest", ondelete="cascade", readonly=True, index=True)
+    is_self = fields.Boolean(compute="_compute_is_self", search="_search_is_self")
     # channel
     channel_id = fields.Many2one("discuss.channel", "Channel", ondelete="cascade", readonly=True, required=True)
     # state
@@ -38,7 +36,34 @@ class ChannelMember(models.Model):
             if any(user._is_public() for user in member.partner_id.user_ids):
                 raise ValidationError(_("Channel members cannot include public users."))
 
-    @api.depends('channel_id.message_ids', 'seen_message_id')
+    @api.depends_context("uid", "guest")
+    def _compute_is_self(self):
+        if not self:
+            return
+        current_partner, current_guest = self.env["res.partner"]._get_current_persona()
+        self.is_self = False
+        for member in self:
+            if current_partner and member.partner_id == current_partner:
+                member.is_self = True
+            if current_guest and member.guest_id == current_guest:
+                member.is_self = True
+
+    def _search_is_self(self, operator, operand):
+        is_in = (operator == "=" and operand) or (operator == "!=" and not operand)
+        current_partner, current_guest = self.env["res.partner"]._get_current_persona()
+        if is_in:
+            return [
+                '|',
+                ("partner_id", "=", current_partner.id) if current_partner else expression.FALSE_LEAF,
+                ("guest_id", "=", current_guest.id) if current_guest else expression.FALSE_LEAF,
+            ]
+        else:
+            return [
+                ("partner_id", "!=", current_partner.id) if current_partner else expression.TRUE_LEAF,
+                ("guest_id", "!=", current_guest.id) if current_guest else expression.TRUE_LEAF,
+            ]
+
+    @api.depends("channel_id.message_ids", "seen_message_id")
     def _compute_message_unread(self):
         if self.ids:
             self.env['mail.message'].flush_model()
@@ -83,58 +108,35 @@ class ChannelMember(models.Model):
 
     @api.model_create_multi
     def create(self, vals_list):
-        """Similar access rule as the access rule of the mail channel.
-
-        It can not be implemented in XML, because when the record will be created, the
-        partner will be added in the channel and the security rule will always authorize
-        the creation.
-        """
-        if not self.env.is_admin() and not self.env.context.get('mail_create_bypass_create_check') is self._bypass_create_check:
-            for vals in vals_list:
-                if 'channel_id' in vals:
-                    channel_id = self.env['discuss.channel'].browse(vals['channel_id'])
-                    if not channel_id._can_invite(vals.get('partner_id')):
-                        raise AccessError(_('This user can not be added in this channel'))
+        if self.env.context.get("mail_create_bypass_create_check") is self._bypass_create_check:
+            self = self.sudo()
+        for vals in vals_list:
+            if "channel_id" not in vals:
+                raise UserError(
+                    _(
+                        "It appears you're trying to create a channel member, but it seems like you forgot to specify the related channel. "
+                        "To move forward, please make sure to provide the necessary channel information."
+                    )
+                )
+            channel = self.env["discuss.channel"].browse(vals["channel_id"])
+            if channel.channel_type == "chat" and len(channel.channel_member_ids) > 0:
+                raise UserError(
+                    _("Adding more members to this chat isn't possible; it's designed for just two people.")
+                )
         return super().create(vals_list)
 
     def write(self, vals):
         for channel_member in self:
-            for field_name in {'channel_id', 'partner_id', 'guest_id'}:
+            for field_name in ['channel_id', 'partner_id', 'guest_id']:
                 if field_name in vals and vals[field_name] != channel_member[field_name].id:
                     raise AccessError(_('You can not write on %(field_name)s.', field_name=field_name))
         return super().write(vals)
 
     def unlink(self):
-        self.sudo().rtc_session_ids.unlink()
+        # sudo: discuss.channel.rtc.session - cascade unlink of sessions for self member
+        self.sudo().rtc_session_ids.unlink()  # ensure unlink overrides are applied
         return super().unlink()
 
-    @api.model
-    def _get_as_sudo_from_context_or_raise(self, channel_id):
-        channel_member = self._get_as_sudo_from_context(channel_id=channel_id)
-        if not channel_member:
-            raise NotFound()
-        return channel_member
-
-    @api.model
-    def _get_as_sudo_from_context(self, channel_id):
-        """ Seeks a channel member matching the provided `channel_id` and the
-        current user or guest.
-
-        :param channel_id: The id of the channel of which the user/guest is
-            expected to be member.
-        :type channel_id: int
-        :return: A record set containing the channel member if found, or an
-            empty record set otherwise. In case of guest, the record is returned
-            with the 'guest' record in the context.
-        :rtype: discuss.channel.member
-        """
-        if self.env.uid and not self.env.user._is_public():
-            return self.env['discuss.channel.member'].sudo().search([('channel_id', '=', channel_id), ('partner_id', '=', self.env.user.partner_id.id)], limit=1)
-        guest = self.env['mail.guest']._get_guest_from_context()
-        if guest:
-            return guest.env['discuss.channel.member'].sudo().search([('channel_id', '=', channel_id), ('guest_id', '=', guest.id)], limit=1)
-        return self.env['discuss.channel.member'].sudo()
-
     def _notify_typing(self, is_typing):
         """ Broadcast the typing notification to channel members
             :param is_typing: (boolean) tells whether the members are typing or not
@@ -159,9 +161,11 @@ class ChannelMember(models.Model):
                 data['thread'] = member.channel_id._channel_format(fields=fields.get('channel')).get(member.channel_id)
             if 'persona' in fields:
                 if member.partner_id:
-                    persona = member._get_partner_data(fields=fields.get('persona', {}).get('partner'))
+                    # sudo: res.partner - reading _get_partner_data related to a member is considered acceptable
+                    persona = member.sudo()._get_partner_data(fields=fields.get('persona', {}).get('partner'))
                     persona['type'] = "partner"
                 if member.guest_id:
+                    # sudo: mail.guest - reading _guest_format related to a member is considered acceptable
                     persona = member.guest_id.sudo()._guest_format(fields=fields.get('persona', {}).get('guest')).get(member.guest_id)
                 data['persona'] = persona
             members_formatted_data[member] = data

addons/mail/models/discuss/ir_attachment.py

@@ -20,6 +20,7 @@ class IrAttachment(models.Model):
     def _attachment_format(self):
         attachment_format = super()._attachment_format()
         for a in attachment_format:
+        # sudo: discuss.voice.metadata - checking the existence of voice metadata for accessible attachments is fine
             a["voice"] = bool(self.browse(a["id"]).with_prefetch(self._prefetch_ids).sudo().voice_ids)
         return attachment_format
 

addons/mail/models/discuss/ir_websocket.py

@@ -1,3 +1,5 @@
+# Part of Odoo. See LICENSE file for full copyright and licensing details.
+
 from odoo import models
 from odoo.addons.mail.models.discuss.mail_guest import add_guest_to_context
 
@@ -8,6 +10,7 @@ class IrWebsocket(models.AbstractModel):
     def _get_im_status(self, data):
         im_status = super()._get_im_status(data)
         if "mail.guest" in data:
+            # sudo: mail.guest - necessary to read im_status from other guests, information is not considered sensitive
             im_status["Persona"] += [{**g, 'type': "guest"} for g in (
                 self.env["mail.guest"]
                 .sudo()
@@ -19,26 +22,22 @@ class IrWebsocket(models.AbstractModel):
     @add_guest_to_context
     def _build_bus_channel_list(self, channels):
         channels = list(channels)  # do not alter original list
-        guest_sudo = self.env["mail.guest"]._get_guest_from_context().sudo()
-        discuss_channels = self.env["discuss.channel"]
-        if self.env.uid and not self.env.user._is_public():
-            discuss_channels = self.env.user.partner_id.channel_ids
-        elif guest_sudo:
-            discuss_channels = guest_sudo.channel_ids
-            channels.append(guest_sudo)
-        for discuss_channel in discuss_channels:
-            channels.append(discuss_channel)
+        guest = self.env["mail.guest"]._get_guest_from_context()
+        if guest:
+            channels.append(guest)
+        channels.extend(self.env["discuss.channel"].search([("is_member", "=", True)]))
         return super()._build_bus_channel_list(channels)
 
     @add_guest_to_context
     def _update_bus_presence(self, inactivity_period, im_status_ids_by_model):
         super()._update_bus_presence(inactivity_period, im_status_ids_by_model)
         if not self.env.user or self.env.user._is_public():
-            guest_sudo = self.env["mail.guest"]._get_guest_from_context().sudo()
-            if not guest_sudo:
+            guest = self.env["mail.guest"]._get_guest_from_context()
+            if not guest:
                 return
-            guest_sudo.env["bus.presence"].update_presence(
+            # sudo: bus.presence - guests currently need sudo to write their own presence
+            self.env["bus.presence"].sudo().update_presence(
                 inactivity_period,
                 identity_field="guest_id",
-                identity_value=guest_sudo.id,
+                identity_value=guest.id,
             )

addons/mail/models/discuss/mail_guest.py

@@ -5,7 +5,6 @@ import uuid
 from datetime import datetime, timedelta
 from functools import wraps
 from inspect import Parameter, signature
-from werkzeug.exceptions import NotFound
 
 from odoo.tools import consteq, get_lang
 from odoo import _, api, fields, models
@@ -83,6 +82,7 @@ class MailGuest(models.Model):
         parts = token.split(self._cookie_separator)
         if len(parts) == 2:
             guest_id, guest_access_token = parts
+            # sudo: mail.guest: guests need sudo to read their access_token
             guest = self.browse(int(guest_id)).sudo().exists()
             if not guest or not guest.access_token or not consteq(guest.access_token, guest_access_token):
                 guest = self.env["mail.guest"]
@@ -92,7 +92,7 @@ class MailGuest(models.Model):
         """Returns the current guest record from the context, if applicable."""
         guest = self.env.context.get('guest')
         if isinstance(guest, self.pool['mail.guest']):
-            return guest.with_context(guest=guest)
+            return guest.sudo(False).with_context(guest=guest)
         return self.env['mail.guest']
 
     def _get_timezone_from_request(self, request):
@@ -129,20 +129,25 @@ class MailGuest(models.Model):
 
     def _init_messaging(self):
         self.ensure_one()
-        odoobot = self.env.ref('base.partner_root')
+        # sudo: res.partner - exposing OdooBot name and id
+        odoobot = self.env.ref('base.partner_root').sudo()
+        # sudo: mail.guest - guest reading their own id/name/channels
+        guest_sudo = self.sudo()
         return {
-            'channels': self.channel_ids._channel_info(),
+            'channels': guest_sudo.channel_ids.sudo(False)._channel_info(),
             'companyName': self.env.company.name,
             'currentGuest': {
-                'id': self.id,
-                'name': self.name,
+                'id': guest_sudo.id,
+                'name': guest_sudo.name,
                 'type': "guest",
             },
             'current_partner': False,
             'current_user_id': False,
             'current_user_settings': False,
+             # sudo: ir.config_parameter: safe to check for existence of tenor api key
             'hasGifPickerFeature': bool(self.env["ir.config_parameter"].sudo().get_param("discuss.tenor_api_key")),
             'hasLinkPreviewFeature': self.env['mail.link.preview']._is_link_preview_enabled(),
+             # sudo: bus.bus: reading non-sensitive last id
             'initBusId': self.env['bus.bus'].sudo()._bus_last_id(),
             'menu_id': False,
             'needaction_inbox_counter': False,
@@ -171,7 +176,7 @@ class MailGuest(models.Model):
             guests_formatted_data[guest] = data
         return guests_formatted_data
 
-    def _find_or_create_for_channel(self, channel, name, country_id, timezone, add_as_member=True, post_joined_message=False):
+    def _find_or_create_for_channel(self, channel, name, country_id, timezone, post_joined_message=False):
         """Get a guest for the given channel. If there is no guest yet,
         create one.
 
@@ -183,8 +188,6 @@ class MailGuest(models.Model):
         :param post_joined_message: whether to post a message to the channel
             to notify that the guest joined
         """
-        if channel.group_public_id:
-            raise NotFound()
         guest = self._get_guest_from_context()
         if not guest:
             guest = self.create(
@@ -194,19 +197,14 @@ class MailGuest(models.Model):
                     "name": name,
                     "timezone": timezone,
                 }
-            )
-        if add_as_member:
-            channel = channel.with_context(guest=guest)
-            try:
-                channel.add_members(guest_ids=[guest.id], post_joined_message=post_joined_message)
-            except UserError:
-                raise NotFound()
+            ).sudo(False)
+        channel.add_members(guest_ids=guest.ids, post_joined_message=post_joined_message)
         return guest
 
     def _set_auth_cookie(self):
         """Add a cookie to the response to identify the guest. Every route
         that expects a guest will make use of it to authenticate the guest
-        through `_get_as_sudo_from_context` or `_get_as_sudo_from_context_or_raise`.
+        through `add_guest_to_context`.
         """
         self.ensure_one()
         expiration_date = datetime.now() + timedelta(days=365)
@@ -216,6 +214,7 @@ class MailGuest(models.Model):
             httponly=True,
             expires=expiration_date,
         )
+        request.update_context(guest=self.sudo(False))
 
     def _format_auth_cookie(self):
         """Format the cookie value for the given guest.

addons/mail/models/discuss/mail_message.py

@@ -12,6 +12,7 @@ class MailMessage(models.Model):
         self.ensure_one()
         if self.env.user._is_public():
             guest = self.env["mail.guest"]._get_guest_from_context()
+            # sudo: mail.guest - current guest can read channels they are member of
             return guest and self.model == "discuss.channel" and self.res_id in guest.sudo().channel_ids.ids
         return super()._validate_access_for_current_persona(operation)
 

addons/mail/models/discuss/res_partner.py

@@ -16,37 +16,6 @@ class ResPartner(models.Model):
         copy=False,
     )
 
-    def _get_channels_as_member(self):
-        """Returns the channels of the partner."""
-        self.ensure_one()
-        channels = self.env["discuss.channel"]
-        # get the channels and groups
-        channels |= self.env["discuss.channel"].search(
-            [
-                ("channel_type", "in", ("channel", "group")),
-                ("channel_partner_ids", "in", [self.id]),
-            ]
-        )
-        # get the pinned direct messages (directly include whatsapp to avoid overrides)
-        channels |= self.env["discuss.channel"].search(
-            [
-                ("channel_type", "in", ["chat", "whatsapp"]),
-                (
-                    "channel_member_ids",
-                    "in",
-                    self.env["discuss.channel.member"]
-                    .sudo()
-                    ._search(
-                        [
-                            ("partner_id", "=", self.id),
-                            ("is_pinned", "=", True),
-                        ]
-                    ),
-                ),
-            ]
-        )
-        return channels
-
     @api.model
     def search_for_channel_invite(self, search_term, channel_id=None, limit=30):
         """Returns partners matching search_term that can be invited to a channel.

addons/mail/models/discuss/res_users.py

@@ -38,11 +38,9 @@ class ResUsers(models.Model):
     def _unsubscribe_from_non_public_channels(self):
         """This method un-subscribes users from group restricted channels. Main purpose
         of this method is to prevent sending internal communication to archived / deleted users.
-        We do not un-subscribes users from public channels because in most common cases,
-        public channels are mailing list (e-mail based) and so users should always receive
-        updates from public channels until they manually un-subscribe themselves.
         """
         domain = [("partner_id", "in", self.partner_id.ids)]
+        # sudo: discuss.channel.member - removing member of other users based on channel restrictions
         current_cm = self.env["discuss.channel.member"].sudo().search(domain)
         current_cm.filtered(
             lambda cm: (cm.channel_id.channel_type == "channel" and cm.channel_id.group_public_id)
@@ -50,8 +48,18 @@ class ResUsers(models.Model):
 
     def _init_messaging(self):
         self.ensure_one()
+        # 2 different queries because the 2 sub-queries together with OR are less efficient
+        channels = self.env["discuss.channel"].with_user(self)
+        channels += channels.search([("channel_type", "in", ("channel", "group")), ("is_member", "=", True)])
+        channels += channels.search(
+            [
+                ("channel_type", "not in", ("channel", "group")),
+                ("channel_member_ids", "any", [("is_self", "=", True), ("is_pinned", "=", True)]),
+            ]
+        )
         return {
-            'channels': self.partner_id._get_channels_as_member()._channel_info(),
-            'hasGifPickerFeature': bool(self.env["ir.config_parameter"].sudo().get_param("discuss.tenor_api_key")),
+            "channels": channels._channel_info(),
+            # sudo: ir.config_parameter - reading hard-coded key to check its existence, safe to return if the feature is enabled
+            "hasGifPickerFeature": bool(self.env["ir.config_parameter"].sudo().get_param("discuss.tenor_api_key")),
             **super()._init_messaging(),
         }

addons/mail/models/ir_attachment.py

@@ -62,7 +62,8 @@ class IrAttachment(models.Model):
 
     def _delete_and_notify(self, message=None):
         if message:
-            message.write({})  # to make sure write_date on the message is updated
+            # sudo: mail.message - safe write just updating the date, because guests don't have the rights
+            message.sudo().write({})  # to make sure write_date on the message is updated
         self.env['bus.bus']._sendmany((attachment._bus_notification_target(), 'ir.attachment/delete', {
             'id': attachment.id, 'message': {'id': message.id, 'write_date': message.write_date} if message else None
         }) for attachment in self)

addons/mail/models/mail_thread.py

@@ -22,7 +22,6 @@ from email.message import EmailMessage
 from email import message_from_string
 from lxml import etree
 from werkzeug import urls
-from werkzeug.exceptions import NotFound
 from xmlrpc import client as xmlrpclib
 from markupsafe import Markup, escape
 
@@ -479,13 +478,6 @@ class MailThread(models.AbstractModel):
         if any(message.message_type != 'comment' for message in messages):
             raise exceptions.UserError(_("Only messages type comment can have their content updated"))
 
-    @api.model
-    def _get_from_context_or_raise(self, thread_id):
-        thread = self.search([["id", "=", thread_id]])
-        if not thread:
-            raise NotFound()
-        return thread
-
     # ------------------------------------------------------
     # TRACKING / LOG
     # ------------------------------------------------------

addons/mail/models/res_partner.py

@@ -6,7 +6,6 @@ import re
 from odoo import _, api, fields, models, tools
 from odoo.osv import expression
 
-
 class Partner(models.Model):
     """ Update partner to add a field about notification preferences. Add a generic opt-out field that can be used
        to restrict usage of automatic email templates. """
@@ -319,7 +318,6 @@ class Partner(models.Model):
 
     @api.model
     def _get_current_persona(self):
-        guest = self.env["mail.guest"]._get_guest_from_context()
-        if self.env.user._is_public() and guest:
-            return (self.env["res.partner"], guest)
+        if not self.env.user or self.env.user._is_public():
+            return (self.env["res.partner"], self.env["mail.guest"]._get_guest_from_context())
         return (self.env.user.partner_id, self.env["mail.guest"])

addons/mail/security/ir.model.access.csv

@@ -3,40 +3,29 @@ access_fetchmail_server,fetchmail.server,model_fetchmail_server,base.group_syste
 access_mail_message_all,mail.message.all,model_mail_message,base.group_public,1,0,0,0
 access_mail_message_portal,mail.message.portal,model_mail_message,base.group_portal,1,1,1,1
 access_mail_message_user,mail.message.user,model_mail_message,base.group_user,1,1,1,1
-access_mail_message_scheduled_all,mail.message.scheduled.all,model_mail_message_schedule,,0,0,0,0
 access_mail_message_scheduled_system,mail.message.scheduled.system,model_mail_message_schedule,base.group_system,1,1,1,1
-access_mail_mail_all,mail.mail.all,model_mail_mail,,0,0,0,0
-access_mail_mail_portal,mail.mail.portal,model_mail_mail,base.group_portal,0,0,0,0
-access_mail_mail_user,mail.mail.user,model_mail_mail,base.group_user,0,0,0,0
 access_mail_mail_system,mail.mail.system,model_mail_mail,base.group_system,1,1,1,1
-access_mail_followers_all,mail.followers.all,model_mail_followers,,0,0,0,0
 access_mail_followers_user,mail.followers.user,model_mail_followers,base.group_user,1,0,0,0
 access_mail_followers_system,mail.followers.system,model_mail_followers,base.group_system,1,1,1,1
 access_mail_notification_portal,mail.notification.portal,model_mail_notification,base.group_portal,1,0,0,0
 access_mail_notification_user,mail.notification.user,model_mail_notification,base.group_user,1,1,1,0
 access_mail_notification_system, mail.notification.system,model_mail_notification,base.group_system,1,1,1,1
-access_discuss_channel_public,discuss.channel.all,model_discuss_channel,base.group_public,1,0,0,0
-access_discuss_channel_portal,discuss.channel.all,model_discuss_channel,base.group_portal,1,0,0,0
+access_discuss_channel_public,discuss.channel.public,model_discuss_channel,base.group_public,1,0,0,0
+access_discuss_channel_portal,discuss.channel.portal,model_discuss_channel,base.group_portal,1,0,0,0
 access_discuss_channel_user,discuss.channel.user,model_discuss_channel,base.group_user,1,1,1,0
-access_discuss_channel_admin,discuss.channel.system,model_discuss_channel,base.group_system,1,1,1,1
-access_discuss_channel_member_public,discuss.channel.member.public,model_discuss_channel_member,base.group_public,1,0,0,0
+access_discuss_channel_system,discuss.channel.system,model_discuss_channel,base.group_system,1,1,1,1
+access_discuss_channel_member_public,discuss.channel.member.public,model_discuss_channel_member,base.group_public,1,1,1,1
 access_discuss_channel_member_portal,discuss.channel.member.portal,model_discuss_channel_member,base.group_portal,1,1,1,1
 access_discuss_channel_member_user,discuss.channel.member.user,model_discuss_channel_member,base.group_user,1,1,1,1
-access_discuss_channel_rtc_session_all,discuss.channel.rtc.session.all,model_discuss_channel_rtc_session,,0,0,0,0
 access_discuss_channel_rtc_session_system,discuss.channel.rtc.session.system,model_discuss_channel_rtc_session,base.group_system,1,1,1,1
-access_mail_alias_all,mail.alias.all,model_mail_alias,,0,0,0,0
 access_mail_alias_user,mail.alias.user,model_mail_alias,base.group_user,1,0,0,0
 access_mail_alias_system,mail.alias.system,model_mail_alias,base.group_system,1,1,1,1
 access_mail_gateway_allowed_system,mail.gateway.allowed.system,model_mail_gateway_allowed,base.group_system,1,1,1,1
-access_mail_message_reaction_all,mail.message.reaction.all,model_mail_message_reaction,,0,0,0,0
 access_mail_message_reaction_system,mail.message.reaction.system,model_mail_message_reaction,base.group_system,1,1,1,1
 access_mail_message_subtype_public,mail.message.subtype.all,model_mail_message_subtype,base.group_public,1,0,0,0
 access_mail_message_subtype_portal,mail.message.subtype.all,model_mail_message_subtype,base.group_portal,1,0,0,0
 access_mail_message_subtype_user,mail.message.subtype.user,model_mail_message_subtype,base.group_user,1,0,0,0
 access_mail_message_subtype_system,mail.message.subtype.system,model_mail_message_subtype,base.group_system,1,1,1,1
-access_mail_tracking_value_all,mail.tracking.value.all,model_mail_tracking_value,,0,0,0,0
-access_mail_tracking_value_portal,mail.tracking.value.portal,model_mail_tracking_value,base.group_portal,0,0,0,0
-access_mail_tracking_value_user,mail.tracking.value.user,model_mail_tracking_value,base.group_user,0,0,0,0
 access_mail_tracking_value_system,mail.tracking.value.system,model_mail_tracking_value,base.group_system,1,1,1,1
 access_publisher_warranty_contract_all,publisher.warranty.contract.all,model_publisher_warranty_contract,base.group_system,1,1,1,1
 access_mail_template,mail.template,model_mail_template,base.group_user,1,1,1,1
@@ -44,17 +33,12 @@ access_mail_template_editor,mail.template_editor,model_mail_template,mail.group_
 access_mail_template_system,mail.template_system,model_mail_template,base.group_system,1,1,1,1
 access_mail_shortcode,mail.shortcode,model_mail_shortcode,base.group_user,1,1,1,1
 access_mail_shortcode_portal,mail.shortcode.portal,model_mail_shortcode,base.group_portal,1,0,0,0
-access_mail_activity_all,mail.activity.all,model_mail_activity,,0,0,0,0
 access_mail_activity_user,mail.activity.user,model_mail_activity,base.group_user,1,1,1,1
-access_mail_activity_plan_all,mail.activity.plan.all,model_mail_activity_plan,,0,0,0,0
 access_mail_activity_plan_user,mail.activity.plan.user,model_mail_activity_plan,base.group_user,1,0,0,0
 access_mail_activity_plan_system,mail.activity.plan.system,model_mail_activity_plan,base.group_system,1,1,1,1
-access_mail_activity_plan_template_all,mail.activity.plan.template.all,model_mail_activity_plan_template,,0,0,0,0
 access_mail_activity_plan_template_user,mail.activity.plan.template.user,model_mail_activity_plan_template,base.group_user,1,0,0,0
 access_mail_activity_plan_template_system,mail.activity.plan.template.system,model_mail_activity_plan_template,base.group_system,1,1,1,1
-access_mail_activity_schedule_all,mail.activity.schedule.all,model_mail_activity_schedule,,0,0,0,0
 access_mail_activity_schedule_user,mail.activity.schedule.user,model_mail_activity_schedule,base.group_user,1,1,1,0
-access_mail_activity_type_all,mail.activity.type.all,model_mail_activity_type,,0,0,0,0
 access_mail_activity_type_user,mail.activity.type.user,model_mail_activity_type,base.group_user,1,0,0,0
 access_mail_activity_type_system,mail.activity.type.system,model_mail_activity_type,base.group_system,1,1,1,1
 access_mail_blacklist_system,access_mail_blacklist_system,model_mail_blacklist,base.group_system,1,1,1,1
@@ -65,17 +49,14 @@ access_mail_resend_message,access.mail.resend.message,model_mail_resend_message,
 access_mail_resend_partner,access.mail.resend.partner,model_mail_resend_partner,base.group_user,1,1,1,0
 access_mail_template_preview,access.mail.template.preview,model_mail_template_preview,base.group_user,1,1,1,0
 access_mail_blacklist_remove_system,acesss.mail.blacklist.remove.system,model_mail_blacklist_remove,base.group_system,1,1,1,1
-access_mail_guest_all,mail.guest,model_mail_guest,,0,0,0,0
 access_mail_guest_user,mail.guest,model_mail_guest,base.group_user,1,0,0,0
 access_mail_guest_system,mail.guest,model_mail_guest,base.group_system,1,1,1,1
-access_mail_ice_server_all,mail.ice.server.all,model_mail_ice_server,,0,0,0,0
 access_mail_ice_server_system,mail.ice.server.system,model_mail_ice_server,base.group_system,1,1,1,1
-access_res_users_settings_volumes_all,res.users.settings.volumes,model_res_users_settings_volumes,,0,0,0,0
 access_res_users_settings_volumes_user,res.users.settings.volumes,model_res_users_settings_volumes,base.group_user,1,1,1,1
 access_mail_template_reset,access.mail.template.reset,model_mail_template_reset,mail.group_mail_template_editor,1,1,1,1
 ir_actions_report_access_user,ir.actions.report.access.user,base.model_ir_actions_report,base.group_user,1,0,0,0
 access_mail_link_preview_admin,mail.link.preview.admin,model_mail_link_preview,base.group_erp_manager,1,1,1,1
 access_discuss_gif_favorite,discuss.gif.favorite,model_discuss_gif_favorite,base.group_user,1,1,1,1
-access_discuss_voice_metadata_user,discuss.voice.metadata.user,model_discuss_voice_metadata,,0,0,0,0
-access_mail_partner_device,access_mail_partner_device,mail.model_mail_partner_device,base.group_user,0,0,0,0
-access_mail_notification_web_push,access_mail_notification_web_push,mail.model_mail_notification_web_push,base.group_user,0,0,0,0
+access_discuss_voice_metadata_user,discuss.voice.metadata.user,model_discuss_voice_metadata,base.group_system,1,1,1,1
+access_mail_partner_device,access_mail_partner_device,mail.model_mail_partner_device,base.group_system,1,1,1,1
+access_mail_notification_web_push,access_mail_notification_web_push,mail.model_mail_notification_web_push,base.group_system,1,1,1,1

addons/mail/security/mail_security.xml

@@ -1,28 +1,167 @@
 <?xml version="1.0" encoding="utf-8"?>
 <odoo noupdate="1">
-
-        <!-- RULES -->
-        <record id="discuss_channel_rule" model="ir.rule">
-            <field name="name">discuss.channel: access only public and joined groups</field>
+        <record id="ir_rule_discuss_channel_all" model="ir.rule">
+            <field name="name">discuss.channel: can access channels (as member or as group allowed)</field>
             <field name="model_id" ref="mail.model_discuss_channel"/>
-            <field name="groups" eval="[Command.link(ref('base.group_user')), Command.link(ref('base.group_portal')), Command.link(ref('base.group_public'))]"/>
-            <field name="domain_force">[
-                '|',
-                    '&amp;',
-                        ('channel_type', '!=', 'channel'),
-                        ('is_member', '=', True),
-                    '&amp;',
-                        ('channel_type', '=', 'channel'),
-                        '|',
-                            ('group_public_id', '=', False),
-                            ('group_public_id', 'in', [g.id for g in user.groups_id])]
+            <field name="groups"
+                eval="[
+                    Command.link(ref('base.group_user')),
+                    Command.link(ref('base.group_portal')),
+                    Command.link(ref('base.group_public')),
+                ]"
+            />
+            <field name="domain_force">
+                [
+                    "|",
+                        "&amp;",
+                            ("channel_type", "!=", "channel"),
+                            ("is_member", "=", True),
+                        "&amp;",
+                            ("channel_type", "=", "channel"),
+                            '|',
+                                ('group_public_id', '=', False),
+                                ('group_public_id', 'in', user.groups_id.ids)
+                ]
             </field>
+            <field name="perm_create" eval="False"/><!-- create follows access rules (allowed for internal and system) -->
+            <field name="perm_unlink" eval="False"/><!-- unlink follows access rules (allowed for system) -->
+        </record>
+
+        <record id="ir_rule_discuss_channel_group_system" model="ir.rule">
+            <field name="name">discuss.channel: admin full access</field>
+            <field name="model_id" ref="mail.model_discuss_channel"/>
+            <field name="groups" eval="[Command.link(ref('base.group_system'))]"/>
+            <field name="domain_force">[(1, '=', 1)]</field>
+        </record>
+
+        <record id="ir_rule_discuss_channel_member_is_self_all" model="ir.rule">
+            <field name="name">discuss.channel.member: access their own entries</field>
+            <field name="model_id" ref="mail.model_discuss_channel_member"/>
+            <field name="groups"
+                eval="[
+                    Command.link(ref('base.group_user')),
+                    Command.link(ref('base.group_portal')),
+                    Command.link(ref('base.group_public')),
+                ]"
+            />
+            <field name="domain_force">
+                [
+                    ('is_self', '=', True),
+                    "|",
+                        "&amp;",
+                            ("channel_id.channel_type", "!=", "channel"),
+                            ("channel_id.is_member", "=", True),
+                        "&amp;",
+                            ("channel_id.channel_type", "=", "channel"),
+                            '|',
+                                ('channel_id.group_public_id', '=', False),
+                                ('channel_id.group_public_id', 'in', user.groups_id.ids)
+                ]
+            </field>
+            <!--
+                create() is controlled by other rules because create() rules are applied after the record contains
+                its data, which means just using 'is_self' would allow any user to add themselves in any channel.
+            -->
             <field name="perm_create" eval="False"/>
         </record>
 
-        <record id="discuss_channel_admin" model="ir.rule">
-            <field name="name">discuss.channel: admin full access</field>
-            <field name="model_id" ref="mail.model_discuss_channel"/>
+        <record id="ir_rule_discuss_channel_member_read_all" model="ir.rule">
+            <field name="name">discuss.channel.member: read members of accessible channels</field>
+            <field name="model_id" ref="mail.model_discuss_channel_member"/>
+            <field name="groups"
+                eval="[
+                    Command.link(ref('base.group_user')),
+                    Command.link(ref('base.group_portal')),
+                    Command.link(ref('base.group_public')),
+                ]"
+            />
+            <field name="domain_force">
+                [
+                    "|",
+                        "&amp;",
+                            ("channel_id.channel_type", "!=", "channel"),
+                            ("channel_id.is_member", "=", True),
+                        "&amp;",
+                            ("channel_id.channel_type", "=", "channel"),
+                            '|',
+                                ('channel_id.group_public_id', '=', False),
+                                ('channel_id.group_public_id', 'in', user.groups_id.ids)
+                ]
+            </field>
+            <field name="perm_create" eval="False"/>
+            <field name="perm_write" eval="False"/>
+            <field name="perm_unlink" eval="False"/>
+        </record>
+
+        <record id="ir_rule_discuss_channel_member_create_is_group_matching_all" model="ir.rule">
+            <field name="name">discuss.channel.member: can join group restricted channels when group is matching</field>
+            <field name="model_id" ref="mail.model_discuss_channel_member"/>
+            <field name="groups"
+                eval="[
+                    Command.link(ref('base.group_user')),
+                    Command.link(ref('base.group_portal')),
+                    Command.link(ref('base.group_public')),
+                ]"
+            />
+            <field name="domain_force">
+                [
+                    ('is_self', '=', True),
+                    ('channel_id.channel_type', '=', 'channel'),
+                    '|',
+                        ('channel_id.group_public_id', '=', False),
+                        ('channel_id.group_public_id', 'in', user.groups_id.ids)
+                ]
+            </field>
+            <!--
+                This is the only case where the current user can join themselves (is_self = True) when the channel
+                is already created, in all other cases they must be invited by someone else.
+            -->
+            <field name="perm_read" eval="False"/>
+            <field name="perm_write" eval="False"/>
+            <field name="perm_unlink" eval="False"/>
+        </record>
+
+        <record id="ir_rule_discuss_channel_member_create_is_group_matching_group_user" model="ir.rule">
+            <field name="name">discuss.channel.member: internal users can invite others in group restricted channels when group is matching</field>
+            <field name="model_id" ref="mail.model_discuss_channel_member"/>
+            <field name="groups" eval="[Command.link(ref('base.group_user'))]"/>
+            <field name="domain_force">
+                [
+                    ('is_self', '=', False),
+                    ('channel_id.channel_type', '=', 'channel'),
+                    '|',
+                        ('channel_id.group_public_id', '=', False),
+                        ('channel_id.group_public_id', 'in', user.groups_id.ids)
+                ]
+            </field>
+            <field name="perm_read" eval="False"/>
+            <field name="perm_write" eval="False"/>
+            <field name="perm_unlink" eval="False"/>
+        </record>
+
+        <record id="ir_rule_discuss_channel_member_create_is_member_group_user" model="ir.rule">
+            <field name="name">discuss.channel.member: internal users can invite others in channels they are member of</field>
+            <field name="model_id" ref="mail.model_discuss_channel_member"/>
+            <field name="groups" eval="[Command.link(ref('base.group_user'))]"/>
+            <field name="domain_force">
+                [
+                    ('is_self', '=', False),
+                    ('channel_id.channel_type', 'not in', ('channel', 'chat')),
+                    ('channel_id.is_member', '=', True)
+                ]
+            </field>
+            <!--
+                create() for the current user is controlled by other rules because create() rules are applied after the record
+                contains its data, which means allowing 'is_self' would allow any user to add themselves in any channel.
+            -->
+            <field name="perm_read" eval="False"/>
+            <field name="perm_write" eval="False"/>
+            <field name="perm_unlink" eval="False"/>
+        </record>
+
+        <record id="ir_rule_discuss_channel_member_group_system" model="ir.rule">
+            <field name="name">discuss.channel.member: admin can manipulate all entries</field>
+            <field name="model_id" ref="mail.model_discuss_channel_member"/>
             <field name="groups" eval="[Command.link(ref('base.group_system'))]"/>
             <field name="domain_force">[(1, '=', 1)]</field>
         </record>
@@ -41,34 +180,6 @@
             <field name="domain_force">[(1, '=', 1)]</field>
         </record>
 
-        <record id="ir_rule_discuss_channel_member_group_user" model="ir.rule">
-            <field name="name">discuss.channel.member: write its own entries</field>
-            <field name="model_id" ref="mail.model_discuss_channel_member"/>
-            <field name="groups" eval="[(4, ref('base.group_user')), (4, ref('base.group_portal'))]"/>
-            <field name="domain_force">[
-                '|',
-                    '&amp;',
-                        ('channel_id.channel_type', '!=', 'channel'),
-                        ('channel_id.is_member', '=', True),
-                    '&amp;',
-                        ('channel_id.channel_type', '=', 'channel'),
-                        '|',
-                            ('channel_id.group_public_id', '=', False),
-                            ('channel_id.group_public_id', 'in', [g.id for g in user.groups_id])]
-            </field>
-            <field name="perm_read" eval="False"/>
-            <field name="perm_write" eval="True"/>
-            <field name="perm_create" eval="False"/>
-            <field name="perm_unlink" eval="True"/>
-        </record>
-
-        <record id="ir_rule_discuss_channel_member_group_system" model="ir.rule">
-            <field name="name">discuss.channel.member: admin can manipulate all entries</field>
-            <field name="model_id" ref="mail.model_discuss_channel_member"/>
-            <field name="groups" eval="[Command.link(ref('base.group_system'))]"/>
-            <field name="domain_force">[(1, '=', 1)]</field>
-        </record>
-
         <record id="ir_rule_mail_notifications_group_user" model="ir.rule">
             <field name="name">mail.notifications: group_user: write its own entries</field>
             <field name="model_id" ref="model_mail_notification"/>
@@ -164,5 +275,4 @@
             <field name="perm_create" eval="True"/>
             <field name="perm_unlink" eval="True"/>
         </record>
-
 </odoo>

addons/mail/static/src/discuss/core/public/welcome_page.js

@@ -32,12 +32,6 @@ export class WelcomePage extends Component {
         if (this.store.guest) {
             await this.personaService.updateGuestName(this.store.self, this.state.userName.trim());
         }
-        if (this.props.data?.discussPublicViewData.addGuestAsMemberOnJoin) {
-            await this.rpc("/discuss/channel/add_guest_as_member", {
-                channel_id: this.props.data.channelData.id,
-                channel_uuid: this.props.data.channelData.uuid,
-            });
-        }
         this.props.proceed?.();
     }
 

addons/mail/tests/common.py

@@ -1097,12 +1097,8 @@ class MailCase(MockEmail):
             }, {...}]
         """
         bus_notifs = self.env['bus.bus'].sudo().search([('channel', 'in', [json_dump(channel) for channel in channels])])
-        if check_unique:
-            self.assertEqual(len(bus_notifs), len(channels))
         self.assertEqual(set(bus_notifs.mapped('channel')), set([json_dump(channel) for channel in channels]))
-
         notif_messages = [n.message for n in bus_notifs]
-
         for expected in message_items or []:
             for notification in notif_messages:
                 if json_dump(expected) == notification:
@@ -1110,7 +1106,8 @@ class MailCase(MockEmail):
             else:
                 raise AssertionError('No notification was found with the expected value.\nExpected:\n%s\nReturned:\n%s' %
                     (json_dump(expected), '\n'.join([n for n in notif_messages])))
-
+        if check_unique:
+            self.assertEqual(len(bus_notifs), len(channels))
         return bus_notifs
 
     def assertNotified(self, message, recipients_info, is_complete=False):

addons/mail/tests/discuss/__init__.py

@@ -1,6 +1,7 @@
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
 from . import test_discuss_channel
+from . import test_discuss_channel_access
 from . import test_discuss_channel_as_guest
 from . import test_discuss_channel_member
 from . import test_message_controller

addons/mail/tests/discuss/test_discuss_channel.py

@@ -1,4 +1,3 @@
-# -*- coding: utf-8 -*-
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
 import base64
@@ -9,200 +8,13 @@ from odoo import Command, fields
 from odoo.addons.mail.models.discuss.discuss_channel import channel_avatar, group_avatar
 from odoo.addons.mail.tests.common import mail_new_test_user
 from odoo.addons.mail.tests.common import MailCommon
-from odoo.exceptions import AccessError, UserError, ValidationError
-from odoo.tests import tagged, Form
-from odoo.tests.common import users
+from odoo.exceptions import ValidationError
+from odoo.tests.common import tagged, users
 from odoo.tools import html_escape, mute_logger
 from odoo.tools.misc import DEFAULT_SERVER_DATETIME_FORMAT
 
 
-class TestChannelAccessRights(MailCommon):
-
-    @classmethod
-    def setUpClass(cls):
-        super().setUpClass()
-
-        cls.user_employee_1 = mail_new_test_user(cls.env, login='user_employee_1', groups='base.group_user', name='Tao Lee')
-        cls.user_public = mail_new_test_user(cls.env, login='user_public', groups='base.group_public', name='Bert Tartignole')
-        cls.user_portal = mail_new_test_user(cls.env, login='user_portal', groups='base.group_portal', name='Chell Gladys')
-
-        # Channel for certain group
-        cls.group_restricted_channel = cls.env['discuss.channel'].channel_create(name='Channel for Groups', group_id=cls.env.ref('base.group_user').id)
-        # Public Channel
-        cls.public_channel = cls.env['discuss.channel'].channel_create(name='Public Channel', group_id=None)
-        # Group
-        cls.private_group = cls.env['discuss.channel'].create_group(partners_to=cls.user_employee.partner_id.ids, name="Group")
-        # Chat
-        cls.chat_user_employee = cls.env['discuss.channel'].channel_get(cls.user_employee.partner_id.ids)
-        cls.chat_user_employee_1 = cls.env['discuss.channel'].channel_get(cls.user_employee_1.partner_id.ids)
-        cls.chat_user_portal = cls.env['discuss.channel'].channel_get(cls.user_portal.partner_id.ids)
-
-    @mute_logger('odoo.addons.base.models.ir_rule', 'odoo.addons.base.models.ir_model', 'odoo.models')
-    @users('user_public')
-    def test_access_public(self):
-        # Read public channel -> ok
-        self.env['discuss.channel'].browse(self.public_channel.id).read()
-
-        # Read group restricted channel -> ko, no access rights
-        with self.assertRaises(AccessError):
-            self.env['discuss.channel'].browse(self.group_restricted_channel.id).read()
-        # Read group -> ko, no access rights
-        with self.assertRaises(AccessError):
-            self.env['discuss.channel'].browse(self.private_group.id).read()
-
-        # Being a member of public channel: -> ko
-        with self.assertRaises(ValidationError):
-            self.public_channel.add_members(self.user_public.partner_id.id)
-        # Being a member of group restricted channel: -> ko, no access rights
-        with self.assertRaises(UserError):
-            self.group_restricted_channel.add_members(self.user_public.partner_id.id)
-        # Being a group member: -> ko
-        with self.assertRaises(ValidationError):
-            self.private_group.add_members(self.user_public.partner_id.id)
-
-        # Create channel/group/chat: ko, no access rights
-        with self.assertRaises(AccessError):
-            self.env['discuss.channel'].create({'name': 'Test', 'channel_type': 'channel'})
-        with self.assertRaises(AccessError):
-            self.env['discuss.channel'].create({'name': 'Test', 'channel_type': 'group'})
-        with self.assertRaises(AccessError):
-            self.env['discuss.channel'].create({'name': 'Test', 'channel_type': 'chat'})
-
-        # Update channel/group/chat: ko, no access rights
-        with self.assertRaises(AccessError):
-            self.env['discuss.channel'].browse(self.public_channel.id).write({'name': 'modified'})
-        with self.assertRaises(AccessError):
-            self.env['discuss.channel'].browse(self.group_restricted_channel.id).write({'name': 'modified'})
-        with self.assertRaises(AccessError):
-            self.env['discuss.channel'].browse(self.private_group.id).write({'name': 'modified'})
-
-        # Unlink channel/group/chat: ko, no access rights
-        with self.assertRaises(AccessError):
-            self.env['discuss.channel'].browse(self.public_channel.id).unlink()
-        with self.assertRaises(AccessError):
-            self.env['discuss.channel'].browse(self.group_restricted_channel.id).unlink()
-        with self.assertRaises(AccessError):
-            self.env['discuss.channel'].browse(self.private_group.id).unlink()
-
-    @mute_logger('odoo.addons.base.models.ir_rule', 'odoo.addons.base.models.ir_model', 'odoo.models')
-    @users('employee')
-    def test_access_employee(self):
-        # Read public channel -> ok
-        self.env['discuss.channel'].browse(self.public_channel.id).read()
-        # Read group restricted channel -> ok
-        self.env['discuss.channel'].browse(self.group_restricted_channel.id).read()
-        # Read chat when not being a member: ko, no access rights
-        with self.assertRaises(AccessError):
-            self.env['discuss.channel'].browse(self.chat_user_employee_1.id).read()
-
-        # Update chat when not being a member: ko, no access rights
-        with self.assertRaises(AccessError):
-            self.env['discuss.channel'].browse(self.chat_user_employee_1.id).write({'name': 'modified'})
-
-        # Being a channel/group member: -> ok
-        self.public_channel.add_members(self.env.user.partner_id.id)
-        self.group_restricted_channel.add_members(self.env.user.partner_id.id)
-        self.private_group.add_members(self.env.user.partner_id.id)
-
-        # Read a group when being a member: ok
-        self.env['discuss.channel'].browse(self.private_group.id).read()
-        # Read a chat when being a member: ok
-        self.env['discuss.channel'].browse(self.chat_user_employee.id).read()
-
-        # Update channel/group/chat when being a member: ok
-        self.env['discuss.channel'].browse(self.public_channel.id).write({'name': 'modified again'})
-        self.env['discuss.channel'].browse(self.group_restricted_channel.id).write({'name': 'modified again'})
-        self.env['discuss.channel'].browse(self.private_group.id).write({'name': 'modified again'})
-        self.env['discuss.channel'].browse(self.chat_user_employee.id).write({'name': 'modified again'})
-
-        # Create channel/group/chat: ok
-        new_channel = self.env['discuss.channel'].create(
-            {'name': 'Test', 'channel_type': 'channel'})
-        new_group = self.env['discuss.channel'].create(
-            {'name': 'Test', 'channel_type': 'group'})
-        new_chat = self.env['discuss.channel'].create(
-            {'name': 'Test', 'channel_type': 'chat'})
-
-        # Employee should be inside the created chat/group/chat
-        self.assertIn(new_channel.channel_partner_ids, self.partner_employee)
-        self.assertIn(new_group.channel_partner_ids, self.partner_employee)
-        self.assertIn(new_chat.channel_partner_ids, self.partner_employee)
-
-        # Unlink channel/group/chat: ko, no access rights
-        with self.assertRaises(AccessError):
-            self.env['discuss.channel'].browse(self.public_channel.id).unlink()
-        with self.assertRaises(AccessError):
-            self.env['discuss.channel'].browse(self.group_restricted_channel.id).unlink()
-        with self.assertRaises(AccessError):
-            self.env['discuss.channel'].browse(self.private_group.id).unlink()
-        with self.assertRaises(AccessError):
-            self.env['discuss.channel'].browse(self.chat_user_employee.id).unlink()
-
-    @mute_logger('odoo.addons.base.models.ir_rule', 'odoo.addons.base.models.ir_model', 'odoo.models')
-    @users('user_portal')
-    def test_access_portal(self):
-        # Read public channel -> ok
-        self.env['discuss.channel'].browse(self.public_channel.id).read()
-        # Read group restricted channel/group -> ko, no access rights
-        with self.assertRaises(AccessError):
-            self.env['discuss.channel'].browse(self.group_restricted_channel.id).read()
-        with self.assertRaises(AccessError):
-            self.env['discuss.channel'].browse(self.private_group.id).read()
-
-        # Being a group member: -> ok
-        self.private_group.add_members(self.user_portal.partner_id.id)
-
-        # Read a group/chat when being a member: ok
-        self.env['discuss.channel'].browse(self.private_group.id).read()
-        self.env['discuss.channel'].browse(self.chat_user_portal.id).read()
-
-        # Update group/chat when being a member: ko, no access rights
-        with self.assertRaises(AccessError):
-            self.env['discuss.channel'].browse(self.private_group.id).write({'name': 'modified'})
-        with self.assertRaises(AccessError):
-            self.env['discuss.channel'].browse(self.chat_user_portal.id).write({'name': 'modified'})
-
-        # Create group/chat: ko, no access rights
-        with self.assertRaises(AccessError):
-            self.env['discuss.channel'].create({'name': 'Test', 'channel_type': 'group'})
-        with self.assertRaises(AccessError):
-            self.env['discuss.channel'].create({'name': 'Test', 'channel_type': 'chat'})
-
-        # Unlink group/chat: ko, no access rights
-        with self.assertRaises(AccessError):
-            self.env['discuss.channel'].browse(self.private_group.id).unlink()
-        with self.assertRaises(AccessError):
-            self.env['discuss.channel'].browse(self.chat_user_portal.id).unlink()
-
-        # Read message from group/chat: ok
-        group_portal = self.env['discuss.channel'].browse(self.private_group.id)
-        for message in group_portal.message_ids:
-            message.read(['subject'])
-        chat_portal = self.env['discuss.channel'].browse(self.chat_user_portal.id)
-        for message in chat_portal.message_ids:
-            message.read(['subject'])
-
-        # Read partner list from group: ko, no access rights
-        with self.assertRaises(AccessError):
-            group_portal.message_partner_ids
-        for partner in self.private_group.message_partner_ids:
-            if partner.id == self.user_portal.partner_id.id:
-                # Portal user can read their own partner record
-                continue
-            with self.assertRaises(AccessError):
-                partner.with_user(self.user_portal).name
-
-        # Read partner list from chat: ko, no access rights
-        with self.assertRaises(AccessError):
-            chat_portal.message_partner_ids
-        for partner in self.chat_user_portal.message_partner_ids:
-            if partner.id == self.user_portal.partner_id.id:
-                # Portal user can read their own partner record
-                continue
-            with self.assertRaises(AccessError):
-                partner.with_user(self.user_portal).name
-
-
+@tagged("post_install", "-at_install")
 class TestChannelInternals(MailCommon):
 
     @classmethod
@@ -224,6 +36,13 @@ class TestChannelInternals(MailCommon):
         )
         cls.partner_employee_nomail = cls.user_employee_nomail.partner_id
 
+    def test_channel_member_cannot_be_public_user(self):
+        """Public users can only join channels as guest."""
+        user_public = mail_new_test_user(self.env, login='user_public', groups='base.group_public', name='Bert Tartignole')
+        public_channel = self.env['discuss.channel'].channel_create(name='Public Channel', group_id=None)
+        with self.assertRaises(ValidationError):
+            public_channel.add_members(user_public.partner_id.id)
+
     @users('employee')
     def test_channel_members(self):
         channel = self.env['discuss.channel'].browse(self.test_channel.ids)
@@ -270,7 +89,8 @@ class TestChannelInternals(MailCommon):
     def test_channel_recipients_channel(self):
         """ Posting a message on a channel should not send emails """
         channel = self.env['discuss.channel'].browse(self.test_channel.ids)
-        channel.add_members((self.partner_employee | self.partner_admin | self.test_partner).ids)
+        # sudo: discuss.channel.member - adding members in non-accessible channel in a test file
+        channel.sudo().add_members((self.partner_employee | self.partner_admin | self.test_partner).ids)
         with self.mock_mail_gateway():
             new_msg = channel.message_post(body="Test", message_type='comment', subtype_xmlid='mail.mt_comment')
         self.assertNotSentEmail()
@@ -431,19 +251,20 @@ class TestChannelInternals(MailCommon):
         public_channel = self.env['discuss.channel'].channel_create(name='Channel for Everyone', group_id=None)
         private_group = self.env['discuss.channel'].create_group(partners_to=self.user_employee.partner_id.ids, name="Group")
         chat_user_current = self.env['discuss.channel'].channel_get(self.env.user.partner_id.ids)
-
-        group_restricted_channel.add_members(self.env.user.partner_id.id)
-        public_channel.add_members(self.env.user.partner_id.id)
-
+        self.assertEqual(len(group_restricted_channel.channel_member_ids), 1)
+        self.assertEqual(len(public_channel.channel_member_ids), 1)
+        self.assertEqual(len(private_group.sudo().channel_member_ids), 1)
+        self.assertEqual(len(chat_user_current.sudo().channel_member_ids), 1)
         group_restricted_channel.action_unfollow()
         public_channel.action_unfollow()
         private_group.action_unfollow()
         chat_user_current.action_unfollow()
-
-        self.assertEqual(group_restricted_channel.channel_partner_ids, self.env['res.partner'])
-        self.assertEqual(public_channel.channel_partner_ids, self.env['res.partner'])
-        self.assertEqual(private_group.channel_partner_ids, self.env['res.partner'])
-        self.assertEqual(chat_user_current.channel_partner_ids, self.env['res.partner'])
+        self.assertEqual(len(group_restricted_channel.channel_member_ids), 0)
+        self.assertEqual(len(public_channel.channel_member_ids), 0)
+        # sudo: mail.channel - reading members of non-accessible channel for testing purposes
+        self.assertEqual(len(private_group.sudo().channel_member_ids), 0)
+        # sudo: mail.channel - reading members of non-accessible channel for testing purposes
+        self.assertEqual(len(chat_user_current.sudo().channel_member_ids), 0)
 
     def test_channel_unfollow_should_not_post_message_if_the_partner_has_been_removed(self):
         '''
@@ -569,10 +390,10 @@ class TestChannelInternals(MailCommon):
         self.assertEqual(self.env.user.company_id, self.company_admin)
 
         with self.with_user('employee'):
-            initial_channel_info = self.env['discuss.channel'].with_context(
+            chat = self.env['discuss.channel'].with_context(
                 allowed_company_ids=self.company_admin.ids
-            ).channel_get(self.partner_employee_c2.ids)._channel_info()[0]
-            self.assertTrue(initial_channel_info, 'should be able to chat with multi company user')
+            ).channel_get(self.partner_employee_c2.ids)
+            self.assertTrue(chat, 'should be able to chat with multi company user')
 
     @users('employee')
     def test_create_chat_channel_should_only_pin_the_channel_for_the_current_user(self):

addons/mail/tests/discuss/test_discuss_channel_access.py

@@ -0,0 +1,484 @@
+# Part of Odoo. See LICENSE file for full copyright and licensing details.
+
+from odoo.addons.mail.tests.common import mail_new_test_user
+from odoo.addons.mail.tests.common import MailCommon
+from odoo.tests.common import tagged
+from odoo.tools import mute_logger
+
+
+@tagged("post_install", "-at_install")
+class TestDiscussChannelAccess(MailCommon):
+    def test_01_discuss_channel_access(self):
+        """Exhaustive list of operations on channel that would make sense in various setup and check
+        whether or not they can be executed.
+        Combinations that make no sense have not been listed, such as create a channel while already
+        being member of the channel.
+        """
+        self.secret_group = self.env["res.groups"].create({"name": "Secret User Group"})
+        self.env["ir.model.data"].create(
+            {
+                "name": "secret_group",
+                "module": "mail",
+                "model": self.secret_group._name,
+                "res_id": self.secret_group.id,
+            }
+        )
+        self.guest = self.env["mail.guest"].create({"name": "A Guest"}).sudo(False)
+        self.users = {
+            "public": mail_new_test_user(
+                self.env, login="public1", name="A Public User", groups="base.group_public,mail.secret_group"
+            ),
+            "portal": mail_new_test_user(
+                self.env, login="portal1", name="A Portal User", groups="base.group_portal,mail.secret_group"
+            ),
+            "user": mail_new_test_user(
+                self.env, login="user1", name="An Internal User", groups="base.group_user,mail.secret_group"
+            ),
+        }
+        self.other_user = mail_new_test_user(
+            self.env, login="other1", name="Another User 1", groups="base.group_user,mail.secret_group"
+        )
+        tests = [
+            ("public", "no_group", "member", "read", True),
+            ("public", "no_group", "member", "write", False),
+            ("public", "no_group", "member", "unlink", False),
+            ("public", "no_group", "outside", "create", False),
+            ("public", "no_group", "outside", "read", True),
+            ("public", "no_group", "outside", "write", False),
+            ("public", "no_group", "outside", "unlink", False),
+            ("public", "group_matching", "member", "read", True),
+            ("public", "group_matching", "member", "write", False),
+            ("public", "group_matching", "member", "unlink", False),
+            ("public", "group_matching", "outside", "create", False),
+            ("public", "group_matching", "outside", "read", True),
+            ("public", "group_matching", "outside", "write", False),
+            ("public", "group_matching", "outside", "unlink", False),
+            ("public", "group_failing", "member", "read", False),
+            ("public", "group_failing", "member", "write", False),
+            ("public", "group_failing", "member", "unlink", False),
+            ("public", "group_failing", "outside", "create", False),
+            ("public", "group_failing", "outside", "read", False),
+            ("public", "group_failing", "outside", "write", False),
+            ("public", "group_failing", "outside", "unlink", False),
+            ("public", "group", "member", "read", True),
+            ("public", "group", "member", "write", False),
+            ("public", "group", "member", "unlink", False),
+            ("public", "group", "outside", "create", False),
+            ("public", "group", "outside", "read", False),
+            ("public", "group", "outside", "write", False),
+            ("public", "group", "outside", "unlink", False),
+            ("public", "chat", "outside", "create", False),
+            ("public", "chat", "outside", "read", False),
+            ("public", "chat", "outside", "write", False),
+            ("public", "chat", "outside", "unlink", False),
+            ("portal", "no_group", "member", "read", True),
+            ("portal", "no_group", "member", "write", False),
+            ("portal", "no_group", "member", "unlink", False),
+            ("portal", "no_group", "outside", "create", False),
+            ("portal", "no_group", "outside", "read", True),
+            ("portal", "no_group", "outside", "write", False),
+            ("portal", "no_group", "outside", "unlink", False),
+            ("portal", "group_matching", "member", "read", True),
+            ("portal", "group_matching", "member", "write", False),
+            ("portal", "group_matching", "member", "unlink", False),
+            ("portal", "group_matching", "outside", "create", False),
+            ("portal", "group_matching", "outside", "read", True),
+            ("portal", "group_matching", "outside", "write", False),
+            ("portal", "group_matching", "outside", "unlink", False),
+            ("portal", "group_failing", "member", "read", False),
+            ("portal", "group_failing", "member", "write", False),
+            ("portal", "group_failing", "member", "unlink", False),
+            ("portal", "group_failing", "outside", "create", False),
+            ("portal", "group_failing", "outside", "read", False),
+            ("portal", "group_failing", "outside", "write", False),
+            ("portal", "group_failing", "outside", "unlink", False),
+            ("portal", "group", "member", "read", True),
+            ("portal", "group", "member", "write", False),
+            ("portal", "group", "member", "unlink", False),
+            ("portal", "group", "outside", "create", False),
+            ("portal", "group", "outside", "read", False),
+            ("portal", "group", "outside", "write", False),
+            ("portal", "group", "outside", "unlink", False),
+            ("portal", "chat", "member", "read", True),
+            ("portal", "chat", "member", "write", False),
+            ("portal", "chat", "member", "unlink", False),
+            ("portal", "chat", "outside", "create", False),
+            ("portal", "chat", "outside", "read", False),
+            ("portal", "chat", "outside", "write", False),
+            ("portal", "chat", "outside", "unlink", False),
+            ("user", "no_group", "member", "read", True),
+            ("user", "no_group", "member", "write", True),
+            ("user", "no_group", "member", "unlink", False),
+            ("user", "no_group", "outside", "create", True),
+            ("user", "no_group", "outside", "read", True),
+            ("user", "no_group", "outside", "write", True),
+            ("user", "no_group", "outside", "unlink", False),
+            ("user", "group_matching", "member", "read", True),
+            ("user", "group_matching", "member", "write", True),
+            ("user", "group_matching", "member", "unlink", False),
+            ("user", "group_matching", "outside", "create", True),
+            ("user", "group_matching", "outside", "read", True),
+            ("user", "group_matching", "outside", "write", True),
+            ("user", "group_matching", "outside", "unlink", False),
+            ("user", "group_failing", "member", "read", False),
+            ("user", "group_failing", "member", "write", False),
+            ("user", "group_failing", "member", "unlink", False),
+            ("user", "group_failing", "outside", "create", True),
+            ("user", "group_failing", "outside", "read", False),
+            ("user", "group_failing", "outside", "write", False),
+            ("user", "group_failing", "outside", "unlink", False),
+            ("user", "group", "member", "read", True),
+            ("user", "group", "member", "write", True),
+            ("user", "group", "member", "unlink", False),
+            ("user", "group", "outside", "create", True),
+            ("user", "group", "outside", "read", False),
+            ("user", "group", "outside", "write", False),
+            ("user", "group", "outside", "unlink", False),
+            ("user", "chat", "member", "read", True),
+            ("user", "chat", "member", "write", True),
+            ("user", "chat", "member", "unlink", False),
+            ("user", "chat", "outside", "create", True),
+            ("user", "chat", "outside", "read", False),
+            ("user", "chat", "outside", "write", False),
+            ("user", "chat", "outside", "unlink", False),
+        ]
+        for user_key, channel_key, membership, operation, result in tests:
+            if result:
+                try:
+                    self._execute_action_channel(user_key, channel_key, membership, operation, result)
+                except Exception as e:  # noqa: BLE001 - re-raising, just with a more contextual message
+                    raise AssertionError(f"{user_key, channel_key, membership, operation} should not raise") from e
+            else:
+                try:
+                    with self.assertRaises(Exception), mute_logger("odoo.sql_db"), mute_logger(
+                        "odoo.addons.base.models.ir_rule"
+                    ):
+                        self._execute_action_channel(user_key, channel_key, membership, operation, result)
+                except AssertionError as e:
+                    raise AssertionError(f"{user_key, channel_key, membership, operation} should raise") from e
+
+    def test_10_discuss_channel_member_access(self):
+        """Exhaustive list of operations on channel.member that would make sense in various setup
+        and check whether or not they can be executed.
+        Combinations that make no sense have not been listed, such as reading self-member when not
+        being member, or having a chat (type) as a guest (which is a current restriction).
+        """
+        self.secret_group = self.env["res.groups"].create({"name": "Secret User Group"})
+        self.env["ir.model.data"].create(
+            {
+                "name": "secret_group",
+                "module": "mail",
+                "model": self.secret_group._name,
+                "res_id": self.secret_group.id,
+            }
+        )
+        self.guest = self.env["mail.guest"].create({"name": "A Guest"}).sudo(False)
+        self.users = {
+            "public": mail_new_test_user(
+                self.env, login="public1", name="A Public User", groups="base.group_public,mail.secret_group"
+            ),
+            "portal": mail_new_test_user(
+                self.env, login="portal1", name="A Portal User", groups="base.group_portal,mail.secret_group"
+            ),
+            "user": mail_new_test_user(
+                self.env, login="user1", name="An Internal User", groups="base.group_user,mail.secret_group"
+            ),
+        }
+        self.other_user = mail_new_test_user(
+            self.env, login="other1", name="Another User 1", groups="base.group_user,mail.secret_group"
+        )
+        self.other_user_2 = mail_new_test_user(
+            self.env, login="other2", name="Another User 2", groups="base.group_user,mail.secret_group"
+        )
+        tests = [
+            ("public", "no_group", "member", "self", "create", False),
+            ("public", "no_group", "member", "self", "read", True),
+            ("public", "no_group", "member", "self", "write", True),
+            ("public", "no_group", "member", "self", "unlink", True),
+            ("public", "no_group", "member", "other", "create", False),
+            ("public", "no_group", "member", "other", "read", True),
+            ("public", "no_group", "member", "other", "write", False),
+            ("public", "no_group", "member", "other", "unlink", False),
+            ("public", "no_group", "outside", "self", "create", True),
+            ("public", "no_group", "outside", "other", "create", False),
+            ("public", "no_group", "outside", "other", "read", True),
+            ("public", "no_group", "outside", "other", "write", False),
+            ("public", "no_group", "outside", "other", "unlink", False),
+            ("public", "group_matching", "member", "self", "create", False),
+            ("public", "group_matching", "member", "self", "read", True),
+            ("public", "group_matching", "member", "self", "write", True),
+            ("public", "group_matching", "member", "self", "unlink", True),
+            ("public", "group_matching", "member", "other", "create", False),
+            ("public", "group_matching", "member", "other", "read", True),
+            ("public", "group_matching", "member", "other", "write", False),
+            ("public", "group_matching", "member", "other", "unlink", False),
+            ("public", "group_matching", "outside", "self", "create", True),
+            ("public", "group_matching", "outside", "other", "create", False),
+            ("public", "group_matching", "outside", "other", "read", True),
+            ("public", "group_matching", "outside", "other", "write", False),
+            ("public", "group_matching", "outside", "other", "unlink", False),
+            ("public", "group_failing", "member", "self", "create", False),
+            ("public", "group_failing", "member", "self", "read", False),
+            ("public", "group_failing", "member", "self", "write", False),
+            ("public", "group_failing", "member", "self", "unlink", False),
+            ("public", "group_failing", "member", "other", "create", False),
+            ("public", "group_failing", "member", "other", "read", False),
+            ("public", "group_failing", "member", "other", "write", False),
+            ("public", "group_failing", "member", "other", "unlink", False),
+            ("public", "group_failing", "outside", "self", "create", False),
+            ("public", "group_failing", "outside", "other", "create", False),
+            ("public", "group_failing", "outside", "other", "read", False),
+            ("public", "group_failing", "outside", "other", "write", False),
+            ("public", "group_failing", "outside", "other", "unlink", False),
+            ("public", "group", "member", "self", "create", False),
+            ("public", "group", "member", "self", "read", True),
+            ("public", "group", "member", "self", "write", True),
+            ("public", "group", "member", "self", "unlink", True),
+            ("public", "group", "member", "other", "create", False),
+            ("public", "group", "member", "other", "read", True),
+            ("public", "group", "member", "other", "write", False),
+            ("public", "group", "member", "other", "unlink", False),
+            ("public", "group", "outside", "self", "create", False),
+            ("public", "group", "outside", "other", "create", False),
+            ("public", "group", "outside", "other", "read", False),
+            ("public", "group", "outside", "other", "write", False),
+            ("public", "group", "outside", "other", "unlink", False),
+            ("public", "chat", "outside", "self", "create", False),
+            ("public", "chat", "outside", "other", "create", False),
+            ("public", "chat", "outside", "other", "read", False),
+            ("public", "chat", "outside", "other", "write", False),
+            ("public", "chat", "outside", "other", "unlink", False),
+            ("portal", "no_group", "member", "self", "create", False),
+            ("portal", "no_group", "member", "self", "read", True),
+            ("portal", "no_group", "member", "self", "write", True),
+            ("portal", "no_group", "member", "self", "unlink", True),
+            ("portal", "no_group", "member", "other", "create", False),
+            ("portal", "no_group", "member", "other", "read", True),
+            ("portal", "no_group", "member", "other", "write", False),
+            ("portal", "no_group", "member", "other", "unlink", False),
+            ("portal", "no_group", "outside", "self", "create", True),
+            ("portal", "no_group", "outside", "other", "create", False),
+            ("portal", "no_group", "outside", "other", "read", True),
+            ("portal", "no_group", "outside", "other", "write", False),
+            ("portal", "no_group", "outside", "other", "unlink", False),
+            ("portal", "group_matching", "member", "self", "create", False),
+            ("portal", "group_matching", "member", "self", "read", True),
+            ("portal", "group_matching", "member", "self", "write", True),
+            ("portal", "group_matching", "member", "self", "unlink", True),
+            ("portal", "group_matching", "member", "other", "create", False),
+            ("portal", "group_matching", "member", "other", "read", True),
+            ("portal", "group_matching", "member", "other", "write", False),
+            ("portal", "group_matching", "member", "other", "unlink", False),
+            ("portal", "group_matching", "outside", "self", "create", True),
+            ("portal", "group_matching", "outside", "other", "create", False),
+            ("portal", "group_matching", "outside", "other", "read", True),
+            ("portal", "group_matching", "outside", "other", "write", False),
+            ("portal", "group_matching", "outside", "other", "unlink", False),
+            ("portal", "group_failing", "member", "self", "create", False),
+            ("portal", "group_failing", "member", "self", "read", False),
+            ("portal", "group_failing", "member", "self", "write", False),
+            ("portal", "group_failing", "member", "self", "unlink", False),
+            ("portal", "group_failing", "member", "other", "create", False),
+            ("portal", "group_failing", "member", "other", "read", False),
+            ("portal", "group_failing", "member", "other", "write", False),
+            ("portal", "group_failing", "member", "other", "unlink", False),
+            ("portal", "group_failing", "outside", "self", "create", False),
+            ("portal", "group_failing", "outside", "other", "create", False),
+            ("portal", "group_failing", "outside", "other", "read", False),
+            ("portal", "group_failing", "outside", "other", "write", False),
+            ("portal", "group_failing", "outside", "other", "unlink", False),
+            ("portal", "group", "member", "self", "create", False),
+            ("portal", "group", "member", "self", "read", True),
+            ("portal", "group", "member", "self", "write", True),
+            ("portal", "group", "member", "self", "unlink", True),
+            ("portal", "group", "member", "other", "create", False),
+            ("portal", "group", "member", "other", "read", True),
+            ("portal", "group", "member", "other", "write", False),
+            ("portal", "group", "member", "other", "unlink", False),
+            ("portal", "group", "outside", "self", "create", False),
+            ("portal", "group", "outside", "other", "create", False),
+            ("portal", "group", "outside", "other", "read", False),
+            ("portal", "group", "outside", "other", "write", False),
+            ("portal", "group", "outside", "other", "unlink", False),
+            ("portal", "chat", "member", "self", "create", False),
+            ("portal", "chat", "member", "self", "read", True),
+            ("portal", "chat", "member", "self", "write", True),
+            ("portal", "chat", "member", "self", "unlink", True),
+            ("portal", "chat", "member", "other", "create", False),
+            ("portal", "chat", "member", "other", "read", True),
+            ("portal", "chat", "member", "other", "write", False),
+            ("portal", "chat", "member", "other", "unlink", False),
+            ("portal", "chat", "outside", "self", "create", False),
+            ("portal", "chat", "outside", "other", "create", False),
+            ("portal", "chat", "outside", "other", "read", False),
+            ("portal", "chat", "outside", "other", "write", False),
+            ("portal", "chat", "outside", "other", "unlink", False),
+            ("user", "no_group", "member", "self", "create", False),
+            ("user", "no_group", "member", "self", "read", True),
+            ("user", "no_group", "member", "self", "write", True),
+            ("user", "no_group", "member", "self", "unlink", True),
+            ("user", "no_group", "member", "other", "create", True),
+            ("user", "no_group", "member", "other", "read", True),
+            ("user", "no_group", "member", "other", "write", False),
+            ("user", "no_group", "member", "other", "unlink", False),
+            ("user", "no_group", "outside", "self", "create", True),
+            ("user", "no_group", "outside", "other", "create", True),
+            ("user", "no_group", "outside", "other", "read", True),
+            ("user", "no_group", "outside", "other", "write", False),
+            ("user", "no_group", "outside", "other", "unlink", False),
+            ("user", "group_matching", "member", "self", "create", False),
+            ("user", "group_matching", "member", "self", "read", True),
+            ("user", "group_matching", "member", "self", "write", True),
+            ("user", "group_matching", "member", "self", "unlink", True),
+            ("user", "group_matching", "member", "other", "create", True),
+            ("user", "group_matching", "member", "other", "read", True),
+            ("user", "group_matching", "member", "other", "write", False),
+            ("user", "group_matching", "member", "other", "unlink", False),
+            ("user", "group_matching", "outside", "self", "create", True),
+            ("user", "group_matching", "outside", "other", "create", True),
+            ("user", "group_matching", "outside", "other", "read", True),
+            ("user", "group_matching", "outside", "other", "write", False),
+            ("user", "group_matching", "outside", "other", "unlink", False),
+            ("user", "group_failing", "member", "self", "create", False),
+            ("user", "group_failing", "member", "self", "read", False),
+            ("user", "group_failing", "member", "self", "write", False),
+            ("user", "group_failing", "member", "self", "unlink", False),
+            ("user", "group_failing", "member", "other", "create", False),
+            ("user", "group_failing", "member", "other", "read", False),
+            ("user", "group_failing", "member", "other", "write", False),
+            ("user", "group_failing", "member", "other", "unlink", False),
+            ("user", "group_failing", "outside", "self", "create", False),
+            ("user", "group_failing", "outside", "other", "create", False),
+            ("user", "group_failing", "outside", "other", "read", False),
+            ("user", "group_failing", "outside", "other", "write", False),
+            ("user", "group_failing", "outside", "other", "unlink", False),
+            ("user", "group", "member", "self", "create", False),
+            ("user", "group", "member", "self", "read", True),
+            ("user", "group", "member", "self", "write", True),
+            ("user", "group", "member", "self", "unlink", True),
+            ("user", "group", "member", "other", "create", True),
+            ("user", "group", "member", "other", "read", True),
+            ("user", "group", "member", "other", "write", False),
+            ("user", "group", "member", "other", "unlink", False),
+            ("user", "group", "outside", "self", "create", False),
+            ("user", "group", "outside", "other", "create", False),
+            ("user", "group", "outside", "other", "read", False),
+            ("user", "group", "outside", "other", "write", False),
+            ("user", "group", "outside", "other", "unlink", False),
+            ("user", "chat", "member", "self", "create", False),
+            ("user", "chat", "member", "self", "read", True),
+            ("user", "chat", "member", "self", "write", True),
+            ("user", "chat", "member", "self", "unlink", True),
+            ("user", "chat", "member", "other", "create", False),
+            ("user", "chat", "member", "other", "read", True),
+            ("user", "chat", "member", "other", "write", False),
+            ("user", "chat", "member", "other", "unlink", False),
+            ("user", "chat", "outside", "self", "create", False),
+            ("user", "chat", "outside", "other", "create", False),
+            ("user", "chat", "outside", "other", "read", False),
+            ("user", "chat", "outside", "other", "write", False),
+            ("user", "chat", "outside", "other", "unlink", False),
+        ]
+        for user_key, channel_key, membership, target, operation, result in tests:
+            channel_id = self._get_channel_id(user_key, channel_key, membership)
+            if result:
+                try:
+                    self._execute_action_member(channel_id, user_key, target, operation, result)
+                except Exception as e:  # noqa: BLE001 - re-raising, just with a more contextual message
+                    raise AssertionError(
+                        f"{user_key, channel_key, membership, target, operation} should not raise"
+                    ) from e
+            else:
+                try:
+                    with self.assertRaises(Exception), mute_logger("odoo.sql_db"), mute_logger(
+                        "odoo.addons.base.models.ir_rule"
+                    ):
+                        self._execute_action_member(channel_id, user_key, target, operation, result)
+                except AssertionError as e:
+                    raise AssertionError(f"{user_key, channel_key, membership, target, operation} should raise") from e
+
+    def _get_channel_id(self, user_key, channel_key, membership):
+        partner = self.env["res.partner"] if user_key == "public" else self.users[user_key].partner_id
+        guest = self.guest if user_key == "public" else self.env["mail.guest"]
+        partners = self.other_user.partner_id
+        if membership == "member":
+            partners += partner
+        DiscussChannel = self.env["discuss.channel"].with_user(self.other_user)
+        if channel_key == "group":
+            channel = DiscussChannel.create_group(partners.ids)
+            if membership == "member":
+                channel.add_members(partner_ids=partner.ids, guest_ids=guest.ids)
+        elif channel_key == "chat":
+            channel = DiscussChannel.channel_get(partners.ids)
+        else:
+            channel = DiscussChannel.channel_create("Channel", group_id=None)
+            if membership == "member":
+                channel.add_members(partner_ids=partner.ids, guest_ids=guest.ids)
+        if channel_key == "no_group":
+            channel.group_public_id = None
+        elif channel_key == "group_matching":
+            channel.group_public_id = self.secret_group
+        elif channel_key == "group_failing":
+            channel.group_public_id = self.env.ref("base.group_system")
+        return channel.id
+
+    def _execute_action_channel(self, user_key, channel_key, membership, operation, result):
+        current_user = self.users[user_key]
+        guest = self.guest if user_key == "public" else self.env["mail.guest"]
+        ChannelAsUser = self.env["discuss.channel"].with_user(current_user).with_context(guest=guest)
+        if operation == "create":
+            group_public_id = None
+            if channel_key == "group_matching":
+                group_public_id = self.secret_group.id
+            elif channel_key == "group_failing":
+                group_public_id = self.env.ref("base.group_system").id
+            data = {
+                "name": "Test Channel",
+                "channel_type": channel_key if channel_key in ("group", "chat") else "channel",
+                "group_public_id": group_public_id,
+            }
+            ChannelAsUser.create(data)
+        else:
+            channel = ChannelAsUser.browse(self._get_channel_id(user_key, channel_key, membership))
+            self.assertEqual(len(channel), 1, "should find the channel")
+            if operation == "read":
+                self.assertEqual(len(ChannelAsUser.search([("id", "=", channel.id)])), 1 if result else 0)
+                channel.read(["name"])
+            elif operation == "write":
+                channel.write({"name": "new name"})
+            elif operation == "unlink":
+                channel.unlink()
+
+    def _execute_action_member(self, channel_id, user_key, target, operation, result):
+        current_user = self.users[user_key]
+        partner = self.env["res.partner"] if user_key == "public" else current_user.partner_id
+        guest = self.guest if user_key == "public" else self.env["mail.guest"]
+        ChannelMemberAsUser = self.env["discuss.channel.member"].with_user(current_user).with_context(guest=guest)
+        if operation == "create":
+            create_data = {"channel_id": channel_id}
+            if target == "self":
+                if guest:
+                    create_data["guest_id"] = guest.id
+                else:
+                    create_data["partner_id"] = partner.id
+            else:
+                create_data["partner_id"] = self.other_user_2.partner_id.id
+            ChannelMemberAsUser.create(create_data)
+        else:
+            domain = [("channel_id", "=", channel_id)]
+            if target == "self":
+                if guest:
+                    domain.append(("guest_id", "=", guest.id))
+                else:
+                    domain.append(("partner_id", "=", partner.id))
+            else:
+                domain.append(("partner_id", "=", self.other_user.partner_id.id))
+            member = ChannelMemberAsUser.sudo().search(domain).sudo(False)
+            self.assertEqual(len(member), 1, "should find the target member")
+            if operation == "read":
+                self.assertEqual(len(ChannelMemberAsUser.search(domain)), 1 if result else 0)
+                member.read(["custom_channel_name"])
+            elif operation == "write":
+                member.write({"custom_channel_name": "new name"})
+            elif operation == "unlink":
+                member.unlink()

addons/mail/tests/discuss/test_discuss_channel_as_guest.py

@@ -1,11 +1,10 @@
-# -*- coding: utf-8 -*-
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
-import odoo
-from odoo.tests import HttpCase
 from odoo.addons.mail.tests.common import mail_new_test_user
+from odoo.tests.common import HttpCase, tagged
 
-@odoo.tests.tagged('-at_install', 'post_install')
+
+@tagged("post_install", "-at_install")
 class TestMailPublicPage(HttpCase):
     """Checks that the invite page redirects to the channel and that all
     modules load correctly on the welcome and channel page when authenticated as various users"""

addons/mail/tests/discuss/test_discuss_channel_member.py

@@ -1,17 +1,12 @@
-# -*- coding: utf-8 -*-
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
-from functools import partial
-
-from odoo.addons.mail.tests.common import mail_new_test_user
-from odoo.addons.mail.tests.common import MailCommon
-from odoo.exceptions import AccessError, UserError
-
-discuss_channel_new_test_user = partial(mail_new_test_user, context={'discuss_channel_nosubscribe': False})
+from odoo.addons.mail.tests.common import MailCommon, mail_new_test_user
+from odoo.exceptions import AccessError, UserError, ValidationError
+from odoo.tests.common import tagged
 
 
-class TestDiscussChannelMembers(MailCommon):
-
+@tagged("post_install", "-at_install")
+class TestDiscussChannelMember(MailCommon):
     @classmethod
     def setUpClass(cls):
         super().setUpClass()
@@ -25,27 +20,22 @@ class TestDiscussChannelMembers(MailCommon):
             'model': cls.secret_group._name,
             'res_id': cls.secret_group.id,
         })
+        cls.user_1 = mail_new_test_user(
+            cls.env, login="user_1", name="User 1", groups="base.group_user,mail.secret_group"
+        )
+        cls.user_2 = mail_new_test_user(
+            cls.env, login="user_2", name="User 2", groups="base.group_user,mail.secret_group"
+        )
+        cls.user_3 = mail_new_test_user(
+            cls.env, login="user_3", name="User 3", groups="base.group_user,mail.secret_group"
+        )
+        cls.user_portal = mail_new_test_user(
+            cls.env, login="user_portal", name="User Portal", groups="base.group_portal"
+        )
+        cls.user_public = mail_new_test_user(
+            cls.env, login="user_public", name="User Public", groups="base.group_public"
+        )
 
-        cls.user_1 = discuss_channel_new_test_user(
-            cls.env, login='user_1',
-            name='User 1',
-            groups='base.group_user,mail.secret_group')
-        cls.user_2 = discuss_channel_new_test_user(
-            cls.env, login='user_2',
-            name='User 2',
-            groups='base.group_user,mail.secret_group')
-        cls.user_3 = discuss_channel_new_test_user(
-            cls.env, login='user_3',
-            name='User 3',
-            groups='base.group_user,mail.secret_group')
-        cls.user_portal = discuss_channel_new_test_user(
-            cls.env, login='user_portal',
-            name='User Portal',
-            groups='base.group_portal')
-        cls.user_public = discuss_channel_new_test_user(
-            cls.env, login='user_ublic',
-            name='User Public',
-            groups='base.group_public')
 
         cls.group = cls.env['discuss.channel'].create({
             'name': 'Group',
@@ -163,8 +153,9 @@ class TestDiscussChannelMembers(MailCommon):
         with self.assertRaises(AccessError):
             channel_members.with_user(self.user_2).unlink()
 
-        # User 3 is in the group, they can kick user 1
-        channel_members.with_user(self.user_portal).unlink()
+        # User 3 is in the group, but not admin/owner, they can not kick user 1
+        with self.assertRaises(AccessError):
+            channel_members.with_user(self.user_portal).unlink()
 
     # ------------------------------------------------------------
     # GROUP BASED CHANNELS
@@ -219,10 +210,8 @@ class TestDiscussChannelMembers(MailCommon):
         channel_members = self.env['discuss.channel.member'].search([('channel_id', '=', self.public_channel.id)])
         self.assertEqual(channel_members.mapped('partner_id'), self.user_1.partner_id | self.user_2.partner_id)
 
-        # portal/public users still cannot join a public channel, should go through dedicated controllers
-        with self.assertRaises(AccessError):
-            self.public_channel.with_user(self.user_portal).add_members(self.user_portal.partner_id.ids)
-        with self.assertRaises(AccessError):
+        self.public_channel.with_user(self.user_portal).add_members(self.user_portal.partner_id.ids)
+        with self.assertRaises(ValidationError):  # public cannot join without having a guest
             self.public_channel.with_user(self.user_public).add_members(self.user_public.partner_id.ids)
 
     def test_channel_member_invite_with_guest(self):

addons/mail/tests/discuss/test_rtc.py

@@ -1,17 +1,15 @@
-# -*- coding: utf-8 -*-
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
 from dateutil.relativedelta import relativedelta
 
 from odoo import fields
 from odoo.addons.mail.tests.common import MailCommon
-from odoo.tests import tagged
-from odoo.tests.common import users
+from odoo.tests.common import tagged, users
 from odoo.tools import mute_logger
 
 
-@tagged('RTC')
-class TestChannelInternals(MailCommon):
+@tagged("RTC", "post_install", "-at_install")
+class TestChannelRTC(MailCommon):
 
     @users('employee')
     @mute_logger('odoo.models.unlink')
@@ -108,8 +106,7 @@ class TestChannelInternals(MailCommon):
             [
                 (self.cr.dbname, 'discuss.channel', channel.id),  # update new session
                 (self.cr.dbname, 'discuss.channel', channel.id),  # message_post "started a live conference" (not asserted below)
-                (self.cr.dbname, 'res.partner', self.user_employee.partner_id.id),  # update of last interest (not asserted below)
-                (self.cr.dbname, 'res.partner', test_user.partner_id.id),  # update of last interest (not asserted below)
+                (self.cr.dbname, 'discuss.channel', channel.id),  # update of last interest (not asserted below)
                 (self.cr.dbname, 'res.partner', test_user.partner_id.id),  # incoming invitation
                 (self.cr.dbname, 'discuss.channel', channel.id),  # update list of invitations
             ],
@@ -190,8 +187,7 @@ class TestChannelInternals(MailCommon):
             [
                 (self.cr.dbname, 'discuss.channel', channel.id),  # update new session
                 (self.cr.dbname, 'discuss.channel', channel.id),  # message_post "started a live conference" (not asserted below)
-                (self.cr.dbname, 'res.partner', self.user_employee.partner_id.id),  # update of last interest (not asserted below)
-                (self.cr.dbname, 'res.partner', test_user.partner_id.id),  # update of last interest (not asserted below)
+                (self.cr.dbname, 'discuss.channel', channel.id),  # update of last interest (not asserted below)
                 (self.cr.dbname, 'res.partner', test_user.partner_id.id),  # incoming invitation
                 (self.cr.dbname, 'mail.guest', test_guest.id),  # incoming invitation
                 (self.cr.dbname, 'discuss.channel', channel.id),  # update list of invitations
@@ -646,14 +642,12 @@ class TestChannelInternals(MailCommon):
             [
                 (self.cr.dbname, 'res.partner', test_user.partner_id.id),  # channel joined (not asserted below)
                 (self.cr.dbname, 'discuss.channel', channel.id),  # message_post "invited" (not asserted below)
-                (self.cr.dbname, 'res.partner', self.user_employee.partner_id.id),  # update of last interest (not asserted below)
-                (self.cr.dbname, 'res.partner', test_user.partner_id.id),  # update of last interest (not asserted below)
+                (self.cr.dbname, 'discuss.channel', channel.id),  # update of last interest (not asserted below)
                 (self.cr.dbname, 'discuss.channel', channel.id),  # new members (not asserted below)
                 (self.cr.dbname, 'res.partner', test_user.partner_id.id),  # incoming invitation
                 (self.cr.dbname, 'mail.guest', test_guest.id),  # incoming invitation
                 (self.cr.dbname, 'discuss.channel', channel.id),  # update list of invitations
-                (self.cr.dbname, 'res.partner', self.user_employee.partner_id.id),  # update of last interest (not asserted below)
-                (self.cr.dbname, 'res.partner', test_user.partner_id.id),  # update of last interest (not asserted below)
+                (self.cr.dbname, 'discuss.channel', channel.id),  # update of last interest (not asserted below)
                 (self.cr.dbname, 'discuss.channel', channel.id),  # new member (guest) (not asserted below)
                 (self.cr.dbname, 'mail.guest', test_guest.id),  # channel joined for guest (not asserted below)
             ],

addons/mail/tests/discuss/test_toggle_upload.py

@@ -3,7 +3,7 @@
 from requests.exceptions import HTTPError
 
 from odoo import Command, http
-from odoo.tests import tagged, HttpCase
+from odoo.tests.common import tagged, HttpCase
 from odoo.tools import file_open, mute_logger
 
 
@@ -11,7 +11,7 @@ from odoo.tools import file_open, mute_logger
 class TestToggleUpload(HttpCase):
     def test_upload_allowed(self):
         self.authenticate(None, None)
-        channel = self.env["discuss.channel"].create({"name": "General"})
+        channel = self.env["discuss.channel"].create({"name": "General", "group_public_id": None})
         guest = self.env["mail.guest"].create({"name": "Guest"})
         channel.write({"channel_member_ids": [Command.create({"guest_id": guest.id})]})
         self.assertFalse(channel.allow_public_upload)
@@ -31,7 +31,7 @@ class TestToggleUpload(HttpCase):
 
     def test_upload_denied(self):
         self.authenticate(None, None)
-        channel = self.env["discuss.channel"].create({"name": "General"})
+        channel = self.env["discuss.channel"].create({"name": "General", "group_public_id": None})
         guest = self.env["mail.guest"].create({"name": "Guest"})
         channel.write({"channel_member_ids": [Command.create({"guest_id": guest.id})]})
         self.assertFalse(channel.allow_public_upload)

addons/mail/tests/test_link_preview.py

@@ -1,4 +1,3 @@
-# -*- coding: utf-8 -*-
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
 from markupsafe import Markup
@@ -9,10 +8,10 @@ import requests
 
 from odoo.addons.mail.tests.common import MailCommon
 from odoo.addons.mail.tools import link_preview
-from odoo.tests import tagged
+from odoo.tests.common import tagged
 
 
-@tagged('mail_link_preview', 'mail_message')
+@tagged("mail_link_preview", "mail_message", "post_install", "-at_install")
 class TestLinkPreview(MailCommon):
 
     @classmethod

addons/mail/tests/test_res_users_settings.py

@@ -1,10 +1,10 @@
-# -*- coding: utf-8 -*-
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
 from odoo.addons.mail.tests.common import MailCommon
-from odoo.tests.common import users
+from odoo.tests.common import tagged, users
 
 
+@tagged("post_install", "-at_install")
 class TestResUsersSettings(MailCommon):
 
     @users('employee')

addons/mail_bot/models/res_users.py

@@ -36,8 +36,7 @@ class Users(models.Model):
     def _init_odoobot(self):
         self.ensure_one()
         odoobot_id = self.env['ir.model.data']._xmlid_to_res_id("base.partner_root")
-        channel_info = self.env['discuss.channel'].channel_get([odoobot_id, self.partner_id.id])
-        channel = self.env['discuss.channel'].browse(channel_info['id'])
+        channel = self.env['discuss.channel'].channel_get([odoobot_id, self.partner_id.id])
         message = Markup("%s<br/>%s<br/><b>%s</b> <span class=\"o_odoobot_command\">:)</span>") % (
             _("Hello,"),
             _("Odoo's chat helps employees collaborate efficiently. I'm here to help you discover its features."),

addons/test_discuss_full/__manifest__.py

@@ -12,7 +12,10 @@
         'calendar',
         'crm',
         'crm_livechat',
+        'hr_attendance',
+        'hr_fleet',
         'hr_holidays',
+        'hr_homeworking',
         'im_livechat',
         'mail',
         'mail_bot',

addons/test_discuss_full/tests/test_performance.py

@@ -12,7 +12,7 @@ from odoo.tools.misc import DEFAULT_SERVER_DATETIME_FORMAT
 
 @tagged('post_install', '-at_install')
 class TestDiscussFullPerformance(HttpCase):
-    _query_count = 73
+    _query_count = 62
 
     def setUp(self):
         super().setUp()

addons/test_mail/tests/test_mail_gateway.py

@@ -1157,6 +1157,7 @@ class TestMailgateway(MailCommon):
         test_channel = self.env['discuss.channel'].create({
             'name': 'Test',
             'channel_partner_ids': [(4, self.partner_1.id)],
+            'group_public_id': None,
         })
         self.fake_email.write({
             'model': 'discuss.channel',

addons/test_mail/tests/test_mail_message.py

@@ -1,4 +1,3 @@
-# -*- coding: utf-8 -*-
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
 import base64
@@ -8,11 +7,11 @@ from unittest.mock import patch
 from odoo.addons.mail.tests.common import mail_new_test_user, MailCommon
 from odoo.addons.test_mail.models.test_mail_models import MailTestSimple
 from odoo.exceptions import AccessError, UserError
+from odoo.tests.common import tagged, users
 from odoo.tools import is_html_empty, mute_logger, formataddr
-from odoo.tests import tagged, users
 
 
-@tagged('mail_message')
+@tagged("mail_message", "post_install", "-at_install")
 class TestMessageValues(MailCommon):
 
     @classmethod
@@ -321,7 +320,7 @@ class TestMessageValues(MailCommon):
         self.assertNotIn('-%d-' % self.alias_record.id, msg.message_id.split('@')[0])
 
 
-@tagged('mail_message')
+@tagged("mail_message", "post_install", "-at_install")
 class TestMessageAccess(MailCommon):
 
     @classmethod

addons/website_livechat/models/discuss_channel.py

@@ -79,5 +79,6 @@ class DiscussChannel(models.Model):
         message_author_id = message.author_id
         visitor = self.livechat_visitor_id
         if len(self) == 1 and visitor and message_author_id != self.livechat_operator_id:
-            visitor._update_visitor_last_visit()
+            # sudo: website.visitor: updating data of a specific visitor
+            visitor.sudo()._update_visitor_last_visit()
         return message

addons/website_livechat/models/website.py

@@ -56,6 +56,7 @@ class Website(models.Model):
                         chat_request_channel.write({'channel_member_ids': [Command.unlink(channel_guest_member.id), Command.create({'guest_id': current_guest.id})]})
                     if not current_guest:
                         channel_guest_member.guest_id._set_auth_cookie()
+                        chat_request_channel = chat_request_channel.with_context(guest=channel_guest_member.guest_id.sudo(False))
                 return {
                     "folded": False,
                     "id": chat_request_channel.id,

addons/website_livechat/models/website_visitor.py

@@ -82,7 +82,8 @@ class WebsiteVisitor(models.Model):
             discuss_channels = self.env['discuss.channel'].create(discuss_channel_vals_list)
             for channel in discuss_channels:
                 if not channel.livechat_visitor_id.partner_id:
-                    self.env['mail.guest']._find_or_create_for_channel(
+                    # sudo: mail.guest - creating a guest and their member in a dedicated channel created from livechat
+                    self.env['mail.guest'].sudo()._find_or_create_for_channel(
                         channel,
                         country_id=country.id,
                         name=_("Visitor #%d", channel.livechat_visitor_id.id),