From 2c8c079c059f38b808c73bf17450a2efac2d388b Mon Sep 17 00:00:00 2001 From: Romain Derie Date: Fri, 16 Apr 2021 15:21:33 +0000 Subject: [PATCH] [FIX] http_routing, website: prevent crash when using `fw` in url MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this commit, the routing map generated and used would be the one from the website the request is performed, instead of the one from the `fw` website ID which will be the one we redirect the user to. This issue was introduced with the routing map by website, be8fc2296b38 and is restricted to a single case: a publisher using the website switcher, and it won't happen on next page naviguation/refresh as the `fw` website id will be the same as the current website's ID. Thus there won't be any routing map mismatch. Step to reproduce: - Create a page on website 2, set it as homepage - Naviguate to website 1 on '/' url - Naviguate to website 2 on '/' url This will raise a werkzeug error about `EndPoint not iterable`. ----- Technical analysis ------ This is the current flow: 1. `_dispatch()` is setting `website_routing` to `get_current_website()` -> 2 2. `_dispatch()` is calling `_match()` 3. `_match()` is calling `routing_map()` with key = `website_routing`, which was set to 2 in step 1. 4. `routing_map()` is calling `_generate_routing_rules()` which generate the rules based on `website_routing`, which was set to 2 in step 1. 5. `_dispatch()` authenticate the user by calling `_authenticate()` 6. `_dispatch()` is calling `_add_dispatch_parameter()`, where URL param `fw` is forced in session, so `get_current_website()` now return the correct `website_id` -> 1 The issue: in order to handle the `fw` URL parameter (step 6.), we need to check the rights to ensure we can allow the website switch. To check rights, user need to be authenticated (step 5.), which is done after generating the routing map (2. & 3. & 4.). The routing map is generated based on the current website (step 1.) Step 6 depends of steps 5 which depends of steps 2/3/4 which depend of step 1, but step 1 should depend of step 6, which is an impossible cycle. closes odoo/odoo#70397 X-original-commit: 4eb26497a774e934d7e1a6be9f505400fd9e2cdb Signed-off-by: Jérémy Kersten (jke) Signed-off-by: Romain Derie --- addons/website/controllers/main.py | 35 ++++++++++++++++--- addons/website/models/ir_http.py | 9 ++--- .../static/src/js/backend/dashboard.js | 2 +- .../static/src/js/content/website_root.js | 8 ++--- addons/website/tests/test_ui.py | 4 +-- addons/website/tests/test_views.py | 16 +++------ 6 files changed, 47 insertions(+), 27 deletions(-) diff --git a/addons/website/controllers/main.py b/addons/website/controllers/main.py index 04223db6b19..a486e8d64c1 100644 --- a/addons/website/controllers/main.py +++ b/addons/website/controllers/main.py @@ -85,10 +85,37 @@ class Website(Home): raise request.not_found() - @http.route('/website/force_website', type='json', auth="user") - def force_website(self, website_id): - request.env['website']._force_website(website_id) - return True + @http.route('/website/force/', type='http', auth="user", website=True, sitemap=False, multilang=False) + def website_force(self, website_id, path='/', isredir=False, **kw): + """ To switch from a website to another, we need to force the website in + session, AFTER landing on that website domain (if set) as this will be a + different session. + """ + parse = werkzeug.urls.url_parse + safe_path = parse(path).path + + if not (request.env.user.has_group('website.group_multi_website') + and request.env.user.has_group('website.group_website_publisher')): + # The user might not be logged in on the forced website, so he won't + # have rights. We just redirect to the path as the user is already + # on the domain (basically a no-op as it won't change domain or + # force website). + # Website 1 : 127.0.0.1 (admin) + # Website 2 : 127.0.0.2 (not logged in) + # Click on "Website 2" from Website 1 + return request.redirect(safe_path) + + website = request.env['website'].browse(website_id) + + if not isredir and website.domain: + domain_from = request.httprequest.environ.get('HTTP_HOST', '') + domain_to = parse(website._get_http_domain()).netloc + if domain_from != domain_to: + # redirect to correct domain for a correct routing map + url_to = werkzeug.urls.url_join(website._get_http_domain(), '/website/force/%s?isredir=1&path=%s' % (website.id, safe_path)) + return request.redirect(url_to) + website._force() + return request.redirect(safe_path) # ------------------------------------------------------ # Login - overwrite of the web login so that regular users are redirected to the backend diff --git a/addons/website/models/ir_http.py b/addons/website/models/ir_http.py index 5866bd1dfd5..fac7b590c20 100644 --- a/addons/website/models/ir_http.py +++ b/addons/website/models/ir_http.py @@ -204,11 +204,12 @@ class Http(models.AbstractModel): @classmethod def _add_dispatch_parameters(cls, func): - # Force website with query string paramater, typically set from website selector in frontend navbar + # DEPRECATED for /website/force/ - remove me in master~saas-14.4 + # Force website with query string paramater, typically set from website selector in frontend navbar and inside tests force_website_id = request.httprequest.args.get('fw') - if (force_website_id and request.session.get('force_website_id') != force_website_id and - request.env.user.has_group('website.group_multi_website') and - request.env.user.has_group('website.group_website_publisher')): + if (force_website_id and request.session.get('force_website_id') != force_website_id + and request.env.user.has_group('website.group_multi_website') + and request.env.user.has_group('website.group_website_publisher')): request.env['website']._force_website(request.httprequest.args.get('fw')) context = {} diff --git a/addons/website/static/src/js/backend/dashboard.js b/addons/website/static/src/js/backend/dashboard.js index ddec5ed7456..3752c46aa78 100644 --- a/addons/website/static/src/js/backend/dashboard.js +++ b/addons/website/static/src/js/backend/dashboard.js @@ -138,7 +138,7 @@ var Dashboard = AbstractAction.extend({ on_go_to_website: function (ev) { ev.preventDefault(); var website = _.findWhere(this.websites, {selected: true}); - window.location.href = $.param.querystring(website.domain + '/', {'fw': website.id}); + window.location.href = `/website/force/${website.id}`; }, on_save_ga_client_id: function(ga_client_id, ga_analytics_key) { diff --git a/addons/website/static/src/js/content/website_root.js b/addons/website/static/src/js/content/website_root.js index d6bdb06e8ce..048896bdbc1 100644 --- a/addons/website/static/src/js/content/website_root.js +++ b/addons/website/static/src/js/content/website_root.js @@ -327,12 +327,12 @@ var WebsiteRoot = publicRootData.PublicRoot.extend(KeyboardNavigationMixin, { _onWebsiteSwitch: function (ev) { var websiteId = ev.currentTarget.getAttribute('website-id'); var websiteDomain = ev.currentTarget.getAttribute('domain'); - var url = window.location.href; + let url = `/website/force/${websiteId}`; if (websiteDomain && window.location.hostname !== websiteDomain) { - var path = window.location.pathname + window.location.search + window.location.hash; - url = websiteDomain + path; + url = websiteDomain + url; } - window.location.href = $.param.querystring(url, {'fw': websiteId}); + const path = window.location.pathname + window.location.search + window.location.hash; + window.location.href = $.param.querystring(url, {'path': path}); }, /** * @private diff --git a/addons/website/tests/test_ui.py b/addons/website/tests/test_ui.py index 55ee86e3310..7c9d125766c 100644 --- a/addons/website/tests/test_ui.py +++ b/addons/website/tests/test_ui.py @@ -161,8 +161,8 @@ class TestUi(odoo.tests.HttpCase): 'website_id': new_website.id, }) - self.start_tour("/?fw=%s" % website_default.id, "generic_website_editor", login='admin') - self.start_tour("/?fw=%s" % new_website.id, "specific_website_editor", login='admin') + self.start_tour("/website/force/%s" % website_default.id, "generic_website_editor", login='admin') + self.start_tour("/website/force/%s" % new_website.id, "specific_website_editor", login='admin') def test_06_public_user_editor(self): website_default = self.env['website'].search([], limit=1) diff --git a/addons/website/tests/test_views.py b/addons/website/tests/test_views.py index 49c3b55ae5b..266d2cb6c78 100644 --- a/addons/website/tests/test_views.py +++ b/addons/website/tests/test_views.py @@ -1165,9 +1165,7 @@ class Crawler(HttpCase): base_url = self.env['ir.config_parameter'].sudo().get_param('web.base.url') # Simulate website 2 (that use only generic views) - url = base_url + '/website/force_website' - json = {'params': {'website_id': website_2.id}} - self.opener.post(url=url, json=json) + self.url_open(base_url + '/website/force/%s' % website_2.id) # Test controller url = base_url + '/website/get_switchable_related_views' @@ -1205,9 +1203,7 @@ class Crawler(HttpCase): # | Filter By Country # Simulate website 1 (that has specific views) - url = base_url + '/website/force_website' - json = {'params': {'website_id': website_1.id}} - self.opener.post(url=url, json=json) + self.url_open(base_url + '/website/force/%s' % website_1.id) # Test controller url = base_url + '/website/get_switchable_related_views' @@ -1380,9 +1376,7 @@ class Crawler(HttpCase): base_url = self.env['ir.config_parameter'].sudo().get_param('web.base.url') # Simulate website 2 - url = base_url + '/website/force_website' - json = {'params': {'website_id': website_2.id}} - self.opener.post(url=url, json=json) + self.url_open(base_url + '/website/force/%s' % website_2.id) # Test controller url = base_url + '/website/get_switchable_related_views' @@ -1393,9 +1387,7 @@ class Crawler(HttpCase): self.assertEqual(response.json()['result'][0]['key'], '_theme_kea_sale.products', "Only '_theme_kea_sale.products' should be returned") # Simulate website 1 - url = base_url + '/website/force_website' - json = {'params': {'website_id': website_1.id}} - self.opener.post(url=url, json=json) + self.url_open(base_url + '/website/force/%s' % website_1.id) # Test controller url = base_url + '/website/get_switchable_related_views'