From bb2a140a59f5b0e98d9f3ed998914d096579a261 Mon Sep 17 00:00:00 2001 From: tsm-odoo Date: Thu, 28 Dec 2023 14:43:56 +0100 Subject: [PATCH] [FIX] bus, im_livechat: add guest channel instead of using context In [1], a context object was added to websocket requests to subscribe to guests channels on cors live chats. However, the common mechanism to subscribe to record channels is to pass a string channel that will then be validated by the `_subscribe` method of `ir_websocket`. This approach works with OdooSH out of the box while the other one does not. This PR removes the mechanism introduce in [1] in favor of the string approach. [1]: https://github.com/odoo/odoo/pull/130052 closes odoo/odoo#147114 Signed-off-by: Matthieu Stockbauer (tsm) --- addons/bus/static/src/services/bus_service.js | 18 ---------- .../static/src/workers/websocket_worker.js | 10 +----- addons/bus/tests/common.py | 3 +- addons/bus/websocket.py | 7 ++-- addons/im_livechat/controllers/cors/main.py | 4 +-- .../src/embed/common/livechat_service.js | 13 +------ addons/im_livechat/tools/misc.py | 16 +++++---- addons/mail/models/discuss/ir_websocket.py | 6 ++++ addons/mail/models/discuss/mail_guest.py | 3 +- .../mail/tests/discuss/test_guest_feature.py | 35 +++++++++++++++++-- 10 files changed, 57 insertions(+), 58 deletions(-) diff --git a/addons/bus/static/src/services/bus_service.js b/addons/bus/static/src/services/bus_service.js index c11e01c5a50..2986f424c70 100644 --- a/addons/bus/static/src/services/bus_service.js +++ b/addons/bus/static/src/services/bus_service.js @@ -31,7 +31,6 @@ export const busService = { let isUsingSharedWorker = browser.SharedWorker && !isIosApp(); const startTs = new Date().getTime(); const connectionInitializedDeferred = new Deferred(); - let context = {}; /** * Send a message to the worker. @@ -171,23 +170,6 @@ export const busService = { send("start"); isActive = true; }, - get context() { - return context; - }, - /** - * Update the context to be sent with every websocket - * message. - * - * @param {object} newContext - */ - async updateContext(newContext) { - context = newContext; - if (!worker) { - startWorker(); - await connectionInitializedDeferred; - } - send("update_context", context); - }, deleteChannel: (channel) => send("delete_channel", channel), forceUpdateChannels: () => send("force_update_channels"), trigger: bus.trigger.bind(bus), diff --git a/addons/bus/static/src/workers/websocket_worker.js b/addons/bus/static/src/workers/websocket_worker.js index 86ead52e1b8..a58d00a9ac1 100644 --- a/addons/bus/static/src/workers/websocket_worker.js +++ b/addons/bus/static/src/workers/websocket_worker.js @@ -11,7 +11,7 @@ import { debounce } from "@bus/workers/websocket_worker_utils"; /** * Type of action that can be sent from the client to the worker. * - * @typedef {'add_channel' | 'delete_channel' | 'force_update_channels' | 'initialize_connection' | 'send' | 'leave' | 'stop' | 'start' | 'update_context'} WorkerAction + * @typedef {'add_channel' | 'delete_channel' | 'force_update_channels' | 'initialize_connection' | 'send' | 'leave' | 'stop' | 'start'} WorkerAction */ export const WEBSOCKET_CLOSE_CODES = Object.freeze({ @@ -64,8 +64,6 @@ export class WebsocketWorker { this.lastChannelSubscription = null; this.lastNotificationId = 0; this.messageWaitQueue = []; - // Context to be attached to each websocket request - this._context = {}; this._forceUpdateChannels = debounce(this._forceUpdateChannels, 300); this._onWebsocketClose = this._onWebsocketClose.bind(this); @@ -148,9 +146,6 @@ export class WebsocketWorker { return this._forceUpdateChannels(); case "initialize_connection": return this._initializeConnection(client, data); - case "update_context": - this._context = data; - break; } } @@ -406,9 +401,6 @@ export class WebsocketWorker { * @param {{event_name: string, data: any }} message Message to send to the server. */ _sendToServer(message) { - if (Object.keys(this._context).length > 0) { - message.context = this._context; - } const payload = JSON.stringify(message); if (!this._isWebsocketConnected()) { this.messageWaitQueue.push(payload); diff --git a/addons/bus/tests/common.py b/addons/bus/tests/common.py index 7d260983e2d..10e7fcf40ea 100644 --- a/addons/bus/tests/common.py +++ b/addons/bus/tests/common.py @@ -112,8 +112,7 @@ class WebsocketCase(HttpCase): it during tests. """ channels = [ - hashable(channel_with_db(self.registry.db_name, c)) - if isinstance(c, str) else c for c in channels + hashable(channel_with_db(self.registry.db_name, c)) for c in channels ] websockets = set() for channel in channels: diff --git a/addons/bus/websocket.py b/addons/bus/websocket.py index d44938c1d93..d7654f7c36e 100644 --- a/addons/bus/websocket.py +++ b/addons/bus/websocket.py @@ -756,7 +756,6 @@ class WebsocketRequest: f'Invalid JSON data, {exc.args[0]}' ) from exc data = jsonrequest.get('data') - context = jsonrequest.get('context') self.session = self._get_session() try: @@ -771,11 +770,11 @@ class WebsocketRequest: self.env = api.Environment(cr, self.session.uid, self.session.context) threading.current_thread().uid = self.env.uid service_model.retrying( - functools.partial(self._serve_ir_websocket, event_name, data, context), + functools.partial(self._serve_ir_websocket, event_name, data), self.env, ) - def _serve_ir_websocket(self, event_name, data, context=None): + def _serve_ir_websocket(self, event_name, data): """ Delegate most of the processing to the ir.websocket model which is extensible by applications. Directly call the @@ -783,8 +782,6 @@ class WebsocketRequest: tolerated: `subscribe` and `update_presence`. """ self.env['ir.websocket']._authenticate() - if context: - self.update_context(**context) if event_name == 'subscribe': self.env['ir.websocket']._subscribe(data) if event_name == 'update_presence': diff --git a/addons/im_livechat/controllers/cors/main.py b/addons/im_livechat/controllers/cors/main.py index 0d25b7c63db..bbbede54693 100644 --- a/addons/im_livechat/controllers/cors/main.py +++ b/addons/im_livechat/controllers/cors/main.py @@ -2,7 +2,7 @@ from odoo.http import route from odoo.addons.im_livechat.controllers.main import LivechatController -from odoo.addons.im_livechat.tools.misc import downgrade_to_public_user, force_guest_env +from odoo.addons.im_livechat.tools.misc import force_guest_env class CorsLivechatController(LivechatController): @@ -15,7 +15,7 @@ class CorsLivechatController(LivechatController): def cors_get_session( self, channel_id, anonymous_name, previous_operator_id=None, chatbot_script_id=None, persisted=True, **kwargs ): - downgrade_to_public_user() + force_guest_env(kwargs.get("guest_token", ""), raise_if_not_found=False) return self.get_session( channel_id, anonymous_name, previous_operator_id, chatbot_script_id, persisted, **kwargs ) diff --git a/addons/im_livechat/static/src/embed/common/livechat_service.js b/addons/im_livechat/static/src/embed/common/livechat_service.js index 73e6a212a8c..6488a3bb355 100644 --- a/addons/im_livechat/static/src/embed/common/livechat_service.js +++ b/addons/im_livechat/static/src/embed/common/livechat_service.js @@ -136,7 +136,6 @@ export class LivechatService { await this.rpc("/im_livechat/visitor_leave_session", { uuid: session.uuid }); } } finally { - localStorage.removeItem(this.GUEST_TOKEN_STORAGE_KEY); cookie.delete(this.SESSION_COOKIE); this.state = SESSION_STATE.NONE; this.sessionInitialized = false; @@ -229,17 +228,7 @@ export class LivechatService { } async initializePersistedSession() { - if (this.guestToken) { - await this.busService.updateContext({ - ...this.busService.context, - guest_token: this.guestToken, - }); - } - if (this.busService.isActive) { - this.busService.forceUpdateChannels(); - } else { - await this.busService.start(); - } + await this.busService.addChannel(`mail.guest_${this.guestToken}`); await this.env.services["mail.messaging"].initialize(); } diff --git a/addons/im_livechat/tools/misc.py b/addons/im_livechat/tools/misc.py index a85ced40f3d..c5cb32b3223 100644 --- a/addons/im_livechat/tools/misc.py +++ b/addons/im_livechat/tools/misc.py @@ -11,15 +11,19 @@ def downgrade_to_public_user(): request.httprequest.cookies = {} -def force_guest_env(guest_token): +def force_guest_env(guest_token, raise_if_not_found=True): """Retrieve the guest from the given token and add it to the context. The request user is then replaced by the public one. :param str guest_token: - :raise NotFound: if the guest cannot be found from the token + :param bool raise_if_not_found: whether to raise if the guest cannot be + found from the token + :raise NotFound: if the guest cannot be found from the token and the + ``raise_if_not_found`` parameter is set to ``True`` """ - guest = request.env["mail.guest"]._get_guest_from_token(guest_token) - if not guest: - raise NotFound() - request.update_context(guest=guest) downgrade_to_public_user() + guest = request.env["mail.guest"]._get_guest_from_token(guest_token) + if guest: + request.update_context(guest=guest) + elif raise_if_not_found: + raise NotFound() diff --git a/addons/mail/models/discuss/ir_websocket.py b/addons/mail/models/discuss/ir_websocket.py index aed9b5c39c4..fb82456657b 100644 --- a/addons/mail/models/discuss/ir_websocket.py +++ b/addons/mail/models/discuss/ir_websocket.py @@ -22,6 +22,12 @@ class IrWebsocket(models.AbstractModel): @add_guest_to_context def _build_bus_channel_list(self, channels): channels = list(channels) # do not alter original list + for channel in list(channels): + if isinstance(channel, str) and channel.startswith("mail.guest_"): + channels.remove(channel) + guest = self.env["mail.guest"]._get_guest_from_token(channel.split("_")[1]) + if guest: + self = self.with_context(guest=guest) guest = self.env["mail.guest"]._get_guest_from_context() if guest: channels.append(guest) diff --git a/addons/mail/models/discuss/mail_guest.py b/addons/mail/models/discuss/mail_guest.py index 10771a5a883..de51d0c1673 100644 --- a/addons/mail/models/discuss/mail_guest.py +++ b/addons/mail/models/discuss/mail_guest.py @@ -25,8 +25,7 @@ def add_guest_to_context(func): def wrapper(self, *args, **kwargs): req = request or wsrequest token = ( - req.httprequest.cookies.get(req.env["mail.guest"]._cookie_name) - or req.env.context.get("guest_token", "") + req.httprequest.cookies.get(req.env["mail.guest"]._cookie_name, "") ) guest = req.env["mail.guest"]._get_guest_from_token(token) if guest and not guest.timezone: diff --git a/addons/mail/tests/discuss/test_guest_feature.py b/addons/mail/tests/discuss/test_guest_feature.py index 76a55c93c44..665fa5eb317 100644 --- a/addons/mail/tests/discuss/test_guest_feature.py +++ b/addons/mail/tests/discuss/test_guest_feature.py @@ -1,9 +1,12 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. -from odoo.tests import HttpCase, tagged +import json +from odoo.tests import tagged +from odoo.addons.bus.tests.common import WebsocketCase + @tagged("post_install", "-at_install") -class TestGuestFeature(HttpCase): +class TestGuestFeature(WebsocketCase): def test_channel_seen_as_guest(self): guest = self.env["mail.guest"].create({"name": "Guest"}) partner = self.env["res.partner"].create({"name": "John"}) @@ -29,3 +32,31 @@ class TestGuestFeature(HttpCase): }, ) self.assertEqual(guest_member.seen_message_id, channel.message_ids[0]) + + def test_subscribe_to_guest_channel(self): + self.env["bus.bus"].search([]).unlink() + guest = self.env["mail.guest"].create({"name": "Guest"}) + guest_websocket = self.websocket_connect() + self.subscribe(guest_websocket, [f"mail.guest_{guest._format_auth_cookie()}"], guest.id) + self.env["bus.bus"]._sendone(guest, "lambda", {"foo": "bar"}) + self.trigger_notification_dispatching([guest]) + notifications = json.loads(guest_websocket.recv()) + self.assertEqual(1, len(notifications)) + self.assertEqual(notifications[0]["message"]["type"], "lambda") + self.assertEqual(notifications[0]["message"]["payload"], {"foo": "bar"}) + + def test_subscribe_to_discuss_channel(self): + guest = self.env["mail.guest"].create({"name": "Guest"}) + channel = self.env["discuss.channel"].channel_create( + group_id=None, name="General" + ) + channel.add_members(guest_ids=[guest.id]) + self.env["bus.bus"].search([]).unlink() + guest_websocket = self.websocket_connect() + self.subscribe(guest_websocket, [f"mail.guest_{guest._format_auth_cookie()}"], guest.id) + self.env["bus.bus"]._sendone(channel, "lambda", {"foo": "bar"}) + self.trigger_notification_dispatching([channel]) + notifications = json.loads(guest_websocket.recv()) + self.assertEqual(1, len(notifications)) + self.assertEqual(notifications[0]["message"]["type"], "lambda") + self.assertEqual(notifications[0]["message"]["payload"], {"foo": "bar"})