From 8c30699f2ac569e63ad35c3b1a045f95fe9ac97a Mon Sep 17 00:00:00 2001 From: Florian Vranckx Date: Wed, 8 Feb 2023 09:04:15 +0000 Subject: [PATCH] [FIX] http: no rotate sid for unidentified user This commit fixes a change in behavior between 15.2 and 15.3. Previously, if an unidentified user tried to reach a route that had auth='user', it would simply redirect to the login page. Currently, it redirects and invalidates the session_id. This is an issue in the latest version of master after this PR https://github.com/odoo/enterprise/pull/36521 This commit changes the route of service-worker.js to auth='user'. This route is called on the login page, which rotates the sid and therefore invalidates the csrf token. Making it impossible for a user to log in. This is a race condition, meaning it would only appear if the user stayed on the login page for a few seconds, hence why the automated testing did not block the commit. closes odoo/odoo#112239 X-original-commit: d5d80d172616afe02bd41934930ea18dc273c739 Signed-off-by: Julien Castiaux (juc) Co-authored-by: Julien Castiaux --- addons/web/tests/test_login.py | 30 ++++++++++++++++++++++-------- odoo/http.py | 3 ++- 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/addons/web/tests/test_login.py b/addons/web/tests/test_login.py index e811c48d007..0fd6236c7ac 100644 --- a/addons/web/tests/test_login.py +++ b/addons/web/tests/test_login.py @@ -5,21 +5,25 @@ from odoo.tests.common import get_db_name, HOST, HttpCase, new_test_user, Opener class TestWebLoginCommon(HttpCase): + @classmethod + def setUpClass(cls): + super().setUpClass() + new_test_user(cls.env, 'internal_user', context={'lang': 'en_US'}) + new_test_user(cls.env, 'portal_user', groups='base.group_portal') + def setUp(self): super().setUp() - new_test_user(self.env, 'portal_user', groups='base.group_portal') - - def login(self, username, password): - """Log in with provided credentials and return response to POST request or raises for status.""" self.session = http.root.session_store.new() self.session.update(http.get_default_session(), db=get_db_name()) self.opener = Opener(self.env.cr) self.opener.cookies.set('session_id', self.session.sid, domain=HOST, path='/') + def login(self, username, password, csrf_token=None): + """Log in with provided credentials and return response to POST request or raises for status.""" res_post = self.url_open('/web/login', data={ 'login': username, 'password': password, - 'csrf_token': http.Request.csrf_token(self), + 'csrf_token':csrf_token or http.Request.csrf_token(self), }) res_post.raise_for_status() @@ -28,9 +32,7 @@ class TestWebLoginCommon(HttpCase): class TestWebLogin(TestWebLoginCommon): def test_web_login(self): - new_test_user(self.env, 'jackoneill', context={'lang': 'en_US'}) - - res_post = self.login('jackoneill', 'jackoneill') + res_post = self.login('internal_user', 'internal_user') # ensure we are logged-in self.url_open( '/web/session/check', @@ -44,3 +46,15 @@ class TestWebLogin(TestWebLoginCommon): res_post = self.login('portal_user', 'portal_user') # ensure we end up on the right page for external users. Valid without portal installed. self.assertEqual(res_post.request.path_url, '/web/login_successful') + + def test_web_login_bad_xhr(self): + # simulate the user downloaded the login form + csrf_token = http.Request.csrf_token(self) + + # simulate that the JS sended a bad XHR to a route that is + # auth='none' using the same session (e.g. via a service worker) + bad_xhr = self.url_open('/web/login_successful', allow_redirects=False) + self.assertNotEqual(bad_xhr.status_code, 200) + + # log in using the above form, it should still be valid + self.login('internal_user', 'internal_user', csrf_token) diff --git a/odoo/http.py b/odoo/http.py index b47629cdc07..7226ef0afc9 100644 --- a/odoo/http.py +++ b/odoo/http.py @@ -1841,9 +1841,10 @@ class HttpDispatcher(Dispatcher): """ if isinstance(exc, SessionExpiredException): session = self.request.session + was_connected = session.uid is not None session.logout(keep_db=True) response = self.request.redirect_query('/web/login', {'redirect': self.request.httprequest.full_path}) - if not session.is_explicit: + if not session.is_explicit and was_connected: root.session_store.rotate(session, self.request.env) response.set_cookie('session_id', session.sid, max_age=SESSION_LIFETIME, httponly=True) return response