From 7edde2b5a55d8e270e14f19217eb888302f52aa7 Mon Sep 17 00:00:00 2001 From: Romain Derie Date: Fri, 25 Nov 2022 15:21:58 +0000 Subject: [PATCH] [FIX] website: prevent loop if auth=user route used as homepage url One can very well select a "auth=user route" as homepage for his website, like /my. There would then be an issue with such an URL being set as homepage: - As the user landed in the homepage controller which is auth=public, the system will add the public user as env user (see `_auth_method_public()`). ``` @http.route('/', type='http', auth="public", website=True, sitemap=True) def index(self, **kw): ``` - Then, that controller will reroute to the homepage url (/my). The request.httprequest.path will now be /my - Then, that controller will recall the dispatcher stack: `request._serve_ir_http()` - From there, this call won't fail as it should because the user is considered as logged in as it went already through the `_auth_method_public()`, adding the public user as env.user. `_auth_method_user()` won't raise its error. - The /my page will be rendered despite not being logged in. Once the user land on that page, the system will actually detect him as logged out on a page supposed to be accessed when logged in. It will then: 1. Show a toaster to inform the user 2. Auto reload the page As the current URL is still `/` (due to the reroute and not a redirect), the same flow will happen again, and again, looping forever. -------- Note that accessing /my directly won't be an issue, as it won't go through the homepage controller (which is auth=public), so there won't be a user_id set on the env (the public user), meaning that the dispatch layer will reject the access and raise an access error (through `_auth_method_user()`. Also note that the same behavior will occur when going through the first menu fallback mechanism: - if the user didn't setup any homepage_url - and deleted his / website.page - and his first website menu is /my In that case, it will go through the first menu redirect fallback, which will be working fine (as it's a redirect and not a reroute). With both those 2 flows (going through redirect), the user will correctly land on the login page (which will redirect to /my once logged in). ------ Finally, the other solution would be to prevent such a configuration (/my as homepage_url) but it was not easily doable (if doable at all), see https://github.com/odoo/odoo/pull/99100#discussion_r963055251 ------ A test is also added and over the more complexe case (which should cover everything): - With /my as homepage_url - With the / website.page deleted - With /my as first menu URL -> Accessing / as public user should: 1. Reroute to /my (because of the homepage_url set to it) which should fail now thanks to this commit 2. Since the reroute / re-serve failed, it should reach the "first menu fallback" mechanism, which is also /my 3. That fallback should be a redirect, not a reroute, so the user should actually land on the login page 4. Once logged in on that page, it should properly redirect to /my opw-3077339 X-original-commit: 4f5899f769266edc27a0b2d812df092b95f43c71 Part-of: odoo/odoo#107759 --- addons/website/controllers/main.py | 4 ++-- addons/website/models/ir_http.py | 8 ++++++++ addons/website/tests/test_page.py | 28 ++++++++++++++++++++++++++++ odoo/addons/base/models/ir_http.py | 6 +++++- 4 files changed, 43 insertions(+), 3 deletions(-) diff --git a/addons/website/controllers/main.py b/addons/website/controllers/main.py index af6554f483c..e2833559374 100644 --- a/addons/website/controllers/main.py +++ b/addons/website/controllers/main.py @@ -21,7 +21,7 @@ import odoo from odoo import http, models, fields, _ from odoo.exceptions import AccessError -from odoo.http import request +from odoo.http import request, SessionExpiredException from odoo.osv import expression from odoo.tools import OrderedSet, escape_psql, html_escape as escape from odoo.addons.http_routing.models.ir_http import slug, slugify, _guess_mimetype @@ -106,7 +106,7 @@ class Website(Home): if homepage_url and homepage_url != '/': try: return request._serve_ir_http() - except (AccessError, NotFound): + except (AccessError, NotFound, SessionExpiredException): pass # Fallback on first accessible menu diff --git a/addons/website/models/ir_http.py b/addons/website/models/ir_http.py index 0d6c38ce26a..5f58ebab4e8 100644 --- a/addons/website/models/ir_http.py +++ b/addons/website/models/ir_http.py @@ -123,6 +123,14 @@ class Http(models.AbstractModel): model=ModelConverter, ) + @classmethod + def _get_public_users(cls): + public_users = super()._get_public_users() + website = request.env(user=SUPERUSER_ID)['website'].get_current_website() # sudo + if website: + public_users.append(website._get_cached('user_id')) + return public_users + @classmethod def _auth_method_public(cls): """ If no user logged, set the public user of current website, or default diff --git a/addons/website/tests/test_page.py b/addons/website/tests/test_page.py index d8a859d516a..e3673cfbabc 100644 --- a/addons/website/tests/test_page.py +++ b/addons/website/tests/test_page.py @@ -407,6 +407,34 @@ class WithContext(HttpCase): self.assertEqual(r.url, home_url_full) self.assertIn(b'o_website_info', r.content) + # Case 6: Check controller redirect which has different `auth` method + website.homepage_url = '/my' + # ------------------------------------------- + # / page exists | first menu | homepage_url + # ------------------------------------------- + # no | /contactus | /my + # ------------------------------------------- + r = self.url_open(home_url) + self.assertEqual(r.status_code, 200) + self.assertNotIn(b' My Portal', r.content) + self.assertIn(b'<title> Contact Us', r.content) + self.assertEqual(r.url, contactus_url_full) + self.assertEqual(r.history[0].status_code, 303) + # Now with /contactus which is a public content + self.env['website.menu'].create({ + 'name': '/my first menu', + 'website_id': website.id, + 'parent_id': website.menu_id.id, + 'url': '/my', + 'sequence': 1, + }) + r = self.url_open(home_url) + self.assertEqual(r.status_code, 200) + self.assertNotIn(b'<title> My Portal', r.content) + self.assertIn(b'<title> Login', r.content) + self.assertIn('/web/login?redirect', r.url) + self.assertEqual(r.history[0].status_code, 303) + def test_07_alternatives(self): website = self.env.ref('website.default_website') lang_fr = self.env['res.lang']._activate_lang('fr_FR') diff --git a/odoo/addons/base/models/ir_http.py b/odoo/addons/base/models/ir_http.py index 54f422bca32..d8f21acff7b 100644 --- a/odoo/addons/base/models/ir_http.py +++ b/odoo/addons/base/models/ir_http.py @@ -91,9 +91,13 @@ class IrHttp(models.AbstractModel): rule, args = cls.routing_map().bind_to_environ(request.httprequest.environ).match(path_info=path_info, return_rule=True) return rule, args + @classmethod + def _get_public_users(cls): + return [request.env.ref('base.public_user').id] + @classmethod def _auth_method_user(cls): - if request.env.uid is None: + if request.env.uid in [None] + cls._get_public_users(): raise http.SessionExpiredException("Session expired") @classmethod