pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[IMP] core: don't save visitor default session

Browse Source 
Every request comes with a session, a dictionary that is persisted on
the filesystem and that saves various information such as the user
cart on the ecommerce.

When a user simply visits the website, a default session is created and
saved on disk, this bloats the filestore with many sessions. Creating
the session on-the-fly is cheaper than loading it from the filesystem.
With this work the default session is not saved on disk anymore unless
explicitly asked via `session.touch()`.

An exception to the statement "creating the session on-the-fly is
cheaper" is geoip, the ip geolocalization is not cheap. In this work,
geoip have been moved from http_routing/request.session.geoip to a
lazy property core/request.geoip. When requested the info is persisted
on the session. Like other keys from the default session, geoip will not
be persisted unless there is non-default stuff in the session.

Because the CSRF-TOKEN is based on the session-id, it is important the
session-id stays the same across multiples requests even when the
session is not persisted on disk. Even when a session is not persisted
on disk, the session-id cookie is still set so that the next session
created on-the-fly uses the same session-id.

Technical note regarding the session, it has been decided to drop the
session-snapshot protocol and to reintroduce a "modified" flag. It has
been decided not to use werkzeug's session (which natively comes with a
"modified" flag) and to keep our own session object. We decided to
extend MutableMapping instead of dict; using MutableMapping we only
have to override __setitem__ and __detitem__; using dict we would had to
override update()/pop()/... too.

Task: 2789035
Part-of: odoo/odoo#86015
This commit is contained in:
Julien Castiaux
2022-04-05 14:13:54 +02:00
parent 815b754ed2
commit 04e972660b
14 changed files with 215 additions and 84 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/auth_totp/controllers/home.py
+3 -3
View File
@@ -53,7 +53,7 @@ class Home(web_home.Home):
browser=request.httprequest.user_agent.browser.capitalize(),
platform=request.httprequest.user_agent.platform.capitalize(),
)
geoip = request.session.geoip
geoip = request.geoip
if geoip:
name += " (%s, %s)" % (geoip['city'], geoip['country_name'])
@@ -66,11 +66,11 @@ class Home(web_home.Home):
samesite='Lax'
)
# Crapy workaround for unupdatable Odoo Mobile App iOS (Thanks Apple :@)
request.session.should_touch = True
request.session.touch()
return response
# Crapy workaround for unupdatable Odoo Mobile App iOS (Thanks Apple :@)
request.session.should_touch = True
request.session.touch()
return request.render('auth_totp.auth_totp_form', {
'user': user,
'error': error,
addons/http_routing/models/ir_http.py
+1 -26
View File
@@ -1,4 +1,4 @@
# -*- coding: utf-8 -*-
# Part of Odoo. See LICENSE file for full copyright and licensing details.
import contextlib
import logging
@@ -27,8 +27,6 @@ from odoo.http import request
from odoo.osv import expression
from odoo.tools import config, ustr, pycompat
from ..geoipresolver import GeoIPResolver
_logger = logging.getLogger(__name__)
# global resolver (GeoIP API is thread-safe, for multithreaded workers)
@@ -520,33 +518,10 @@ class IrHttp(models.AbstractModel):
threading.current_thread().url = httprequest.url
request.httprequest = httprequest
@classmethod
def _geoip_setup_resolver(cls):
# Lazy init of GeoIP resolver
if odoo._geoip_resolver is not None:
return
geofile = config.get('geoip_database')
try:
odoo._geoip_resolver = GeoIPResolver.open(geofile) or False
except Exception as e:
_logger.warning('Cannot load GeoIP: %s', ustr(e))
@classmethod
def _geoip_resolve(cls):
if 'geoip' not in request.session:
record = {}
if odoo._geoip_resolver and request.httprequest.remote_addr:
record = odoo._geoip_resolver.resolve(request.httprequest.remote_addr) or {}
request.session['geoip'] = record
@classmethod
def _pre_dispatch(cls, rule, args):
super()._pre_dispatch(rule, args)
cls._geoip_setup_resolver()
cls._geoip_resolve()
if request.is_frontend:
cls._frontend_pre_dispatch()
addons/web/controllers/home.py
+1 -1
View File
@@ -44,7 +44,7 @@ class Home(http.Controller):
raise http.SessionExpiredException("Session expired")
# Side-effect, refresh the session lifetime
request.session.should_touch = True
request.session.touch()
# Restore the user on the environment, it was lost due to auth="none"
request.update_env(user=request.session.uid)
addons/web/controllers/session.py
+1 -1
View File
@@ -24,7 +24,7 @@ class Session(http.Controller):
@http.route('/web/session/get_session_info', type='json', auth="user")
def get_session_info(self):
# Crapy workaround for unupdatable Odoo Mobile App iOS (Thanks Apple :@)
request.session.should_touch = True
request.session.touch()
return request.env['ir.http'].session_info()
@http.route('/web/session/authenticate', type='json', auth="none")
addons/website/tests/test_page.py
+12 -2
View File
@@ -1,7 +1,10 @@
# coding: utf-8
from odoo.addons.website.tools import MockRequest
# Part of Odoo. See LICENSE file for full copyright and licensing details.
from unittest.mock import patch
from odoo.http import root
from odoo.tests import common, HttpCase, tagged
from odoo.tools import mute_logger
from odoo.addons.website.tools import MockRequest
@tagged('-at_install', 'post_install')
@@ -273,3 +276,10 @@ class WithContext(HttpCase):
r = self.url_open(self.page.url)
self.assertEqual(r.status_code, 500, "15/0 raise a 500 error page (2)")
self.assertIn('ZeroDivisionError: division by zero', r.text, "Error should be shown in debug.")
def test_04_visitor_no_session(self):
with patch.object(root.session_store, 'save') as session_save,\
MockRequest(self.env, website=self.env['website'].browse(1)):
# no session should be saved for website visitor
self.url_open(self.page.url).raise_for_status()
session_save.assert_not_called()
addons/website/tools.py
+1
View File
@@ -47,6 +47,7 @@ def MockRequest(
sale_order_id=sale_order_id,
website_sale_current_pl=website_sale_current_pl,
),
geoip={},
db=None,
env=env,
registry=env.registry,
odoo/addons/base/models/ir_http.py
+4
View File
@@ -119,6 +119,10 @@ class IrHttp(models.AbstractModel):
_logger.info("Exception during request Authentication.", exc_info=True)
raise AccessDenied()
@classmethod
def _geoip_resolve(cls):
return request._geoip_resolve()
@classmethod
def _pre_dispatch(cls, rule, args):
request.dispatcher.pre_dispatch(rule, args)
odoo/addons/base/tests/test_xmlrpc.py
+2 -1
View File
@@ -128,7 +128,8 @@ class TestAPIKeys(common.HttpCase):
'cookies': {},
}),
# bypass check_identity flow
'session': {'identity-check-last': time.time()}
'session': {'identity-check-last': time.time()},
'geoip': {},
})
_request_stack.push(fake_req)
self.addCleanup(_request_stack.pop)
odoo/addons/test_http/controllers.py
+12
View File
@@ -120,3 +120,15 @@ class TestHttp(http.Controller):
ensure_db()
assert request.db, "There should be a database"
return request.db
# =====================================================
# Session
# =====================================================
@http.route('/test_http/geoip', type='http', auth='none')
def geoip(self):
return str(request.geoip)
@http.route('/test_http/save_session', type='http', auth='none')
def touch(self):
request.session.touch()
return ''
odoo/addons/test_http/tests/test_http.py
+72 -14
View File
@@ -12,7 +12,19 @@ from odoo.tests.common import HOST, HttpCase, new_test_user
from odoo.tools import config, file_open, mute_logger
from odoo.tools.func import lazy_property
from odoo.addons.test_http.controllers import CT_JSON
from odoo.addons.test_http.utils import MemorySessionStore, HtmlTokenizer
from odoo.addons.test_http.utils import (
MemoryGeoipResolver, MemorySessionStore, HtmlTokenizer
)
GEOIP_ODOO_FARM_2 = {
'city': 'Ramillies',
'country_code': 'BE',
'country_name': 'Belgium',
'latitude': 50.6314,
'longitude': 4.8573,
'region': 'WAL',
'time_zone': 'Europe/Brussels'
}
class TestHttpBase(HttpCase):
@@ -23,6 +35,7 @@ class TestHttpBase(HttpCase):
cls.classPatch(odoo.conf, 'server_wide_modules', ['base', 'web', 'test_http'])
lazy_property.reset_all(odoo.http.root)
cls.classPatch(odoo.http.root, 'session_store', MemorySessionStore(session_class=Session))
cls.classPatch(odoo.http.root, 'geoip_resolver', MemoryGeoipResolver())
def setUp(self):
super().setUp()
@@ -392,19 +405,6 @@ class TestHttpMisc(TestHttpBase):
self.assertEqual(res.json()['REMOTE_ADDR'], client_ip)
self.assertEqual(res.json()['HTTP_HOST'], host)
@mute_logger('odoo.http') # greeting_none called ignoring args {'debug'}
def test_misc3_debug_mode(self):
session = self.authenticate(None, None)
self.assertEqual(session.debug, '')
self.db_url_open('/test_http/greeting').raise_for_status()
self.assertEqual(session.debug, '')
self.db_url_open('/test_http/greeting?debug=1').raise_for_status()
self.assertEqual(session.debug, '1')
self.db_url_open('/test_http/greeting').raise_for_status()
self.assertEqual(session.debug, '1')
self.db_url_open('/test_http/greeting?debug=').raise_for_status()
self.assertEqual(session.debug, '')
@tagged('post_install', '-at_install')
class TestHttpCors(TestHttpBase):
@@ -502,3 +502,61 @@ class TestHttpEnsureDb(TestHttpBase):
res.raise_for_status()
self.assertEqual(res.status_code, 200)
self.assertEqual(res.text, 'db1')
class TestHttpSession(TestHttpBase):
@mute_logger('odoo.http') # greeting_none called ignoring args {'debug'}
def test_session0_debug_mode(self):
session = self.authenticate(None, None)
self.assertEqual(session.debug, '')
self.db_url_open('/test_http/greeting').raise_for_status()
self.assertEqual(session.debug, '')
self.db_url_open('/test_http/greeting?debug=1').raise_for_status()
self.assertEqual(session.debug, '1')
self.db_url_open('/test_http/greeting').raise_for_status()
self.assertEqual(session.debug, '1')
self.db_url_open('/test_http/greeting?debug=').raise_for_status()
self.assertEqual(session.debug, '')
def test_session1_default_session(self):
# The default session should not be saved on the filestore.
with patch.object(odoo.http.root.session_store, 'save') as mock_save:
res = self.db_url_open('/test_http/greeting')
res.raise_for_status()
try:
mock_save.assert_not_called()
except AssertionError as exc:
msg = f'save() was called with args: {mock_save.call_args}'
raise AssertionError(msg) from exc
def test_session2_geoip(self):
real_save = odoo.http.root.session_store.save
with patch.object(odoo.http.root.geoip_resolver, 'resolve') as mock_resolve,\
patch.object(odoo.http.root.session_store, 'save') as mock_save:
mock_resolve.return_value = GEOIP_ODOO_FARM_2
mock_save.side_effect = real_save
# Geoip is lazy: it should be computed only when necessary.
self.nodb_url_open('/test_http/greeting').raise_for_status()
mock_resolve.assert_not_called()
# Geoip is like the defaut session: the session should not
# be stored only due to geoip.
mock_resolve.reset_mock()
mock_save.reset_mock()
res = self.nodb_url_open('/test_http/geoip')
res.raise_for_status()
self.assertEqual(res.text, str(GEOIP_ODOO_FARM_2))
mock_save.assert_not_called()
# Geoip is cached on the session: we shouldn't geolocate the
# same ip multiple times.
mock_resolve.reset_mock()
mock_save.reset_mock()
self.nodb_url_open('/test_http/save_session').raise_for_status()
self.nodb_url_open('/test_http/geoip').raise_for_status()
res = self.nodb_url_open('/test_http/geoip')
res.raise_for_status()
self.assertEqual(res.text, str(GEOIP_ODOO_FARM_2))
mock_resolve.assert_called_once()
odoo/addons/test_http/utils.py
+5
View File
@@ -5,6 +5,11 @@ from odoo.http import FilesystemSessionStore
from odoo.tools._vendor.sessions import SessionStore
class MemoryGeoipResolver:
def resolve(self, ip):
return {}
class MemorySessionStore(SessionStore):
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
odoo/http.py
+100 -34
View File
@@ -110,6 +110,7 @@ endpoint
import cgi
import collections
import collections.abc
import contextlib
import functools
import glob
@@ -124,6 +125,7 @@ import re
import threading
import time
import traceback
import warnings
import zlib
from abc import ABC, abstractmethod
from datetime import datetime
@@ -154,6 +156,7 @@ from .modules.registry import Registry
from .service import security, model as service_model
from .tools import (config, consteq, date_utils, profiler, resolve_attr,
submap, unique, ustr,)
from .tools.geoipresolver import GeoIPResolver
from .tools.func import filter_kwargs, lazy_property
from .tools.mimetypes import guess_mimetype
from .tools._vendor import sessions
@@ -703,29 +706,56 @@ class FilesystemSessionStore(sessions.FilesystemSessionStore):
os.unlink(path)
class Session(dict):
class Session(collections.abc.MutableMapping):
""" Structure containing data persisted across requests. """
__slots__ = ('can_save', 'is_explicit', 'json_data', 'new', 'should_rotate', 'should_touch', 'sid')
__slots__ = ('can_save', 'data', 'is_dirty', 'is_explicit', 'is_new',
'should_rotate', 'sid')
def __init__(self, data, sid, new=False):
super().__init__(data)
object.__setattr__(self, 'can_save', True)
object.__setattr__(self, 'is_explicit', False)
object.__setattr__(self, 'json_data', json.dumps(data))
object.__setattr__(self, 'new', new)
object.__setattr__(self, 'should_rotate', False)
object.__setattr__(self, 'should_touch', False)
object.__setattr__(self, 'sid', sid)
self.can_save = True
self.data = data
self.is_dirty = False
self.is_explicit = False
self.is_new = new
self.should_rotate = False
self.sid = sid
#
# MutableMapping implementation with DocDict-like extension
#
def __getitem__(self, item):
if item == 'geoip':
warnings.warn('request.session.geoip have been moved to request.geoip', DeprecationWarning)
return request.geoip if request else {}
return self.data[item]
def __setitem__(self, item, value):
if item not in self.data or self.data[item] != value:
self.is_dirty = True
self.data[item] = value
def __delitem__(self, item):
del self.data[item]
self.is_dirty = True
def __len__(self):
return len(self.data)
def __iter__(self):
return iter(self.data)
def __getattr__(self, attr):
return self.get(attr, None)
def __setattr__(self, key, val):
if key in self.__slots__:
object.__setattr__(self, key, val)
super().__setattr__(key, val)
else:
self[key] = val
#
# Session methods
#
def authenticate(self, dbname, login=None, password=None):
"""
Authenticate the current user with the given db, login and
@@ -796,6 +826,9 @@ class Session(dict):
self.context['lang'] = request.default_lang() if request else DEFAULT_LANG
self.should_rotate = True
def touch(self):
self.is_dirty = True
# =========================================================
# Request and Response
@@ -919,18 +952,11 @@ class Request:
self.dispatcher = _dispatchers['http'](self) # until we match
#self.params = {} # set by the Dispatcher
self.session = self._get_session()
self.db = self._get_dbname()
self.session, self.db = self._get_session_and_dbname()
self.registry = None
self.env = None
if self.session.db != self.db:
if self.session.db:
_logger.warning("Logged into database %r, but dbfilter rejects it; logging session out.", self.session.db)
self.session.logout(keep_db=False)
self.session.db = self.db
def _get_session(self):
def _get_session_and_dbname(self):
# The session is explicit when it comes from the query-string or
# the header. It is implicit when it comes from the cookie or
# that is does not exist yet. The explicit session should be
@@ -948,26 +974,31 @@ class Request:
session = root.session_store.new()
else:
session = root.session_store.get(sid)
session.sid = sid # in case the session was not persisted
session.is_explicit = is_explicit
for key, val in DEFAULT_SESSION.items():
session.setdefault(key, val)
if not session.context.get('lang'):
session.context['lang'] = self.default_lang()
return session
dbname = None
host = self.httprequest.environ['HTTP_HOST']
if session.db and db_filter([session.db], host=host):
dbname = session.db
else:
all_dbs = db_list(force=True, host=host)
if len(all_dbs) == 1:
dbname = all_dbs[0] # monodb
def _get_dbname(self):
if self.session.db and db_filter([self.session.db], host=self.httprequest.environ['HTTP_HOST']):
return self.session.db
if session.db != dbname:
if session.db:
_logger.warning("Logged into database %r, but dbfilter rejects it; logging session out.", session.db)
session.logout(keep_db=False)
session.db = dbname
# monodb
all_dbs = db_list(force=True, host=self.httprequest.environ['HTTP_HOST'])
if len(all_dbs) == 1:
return all_dbs[0]
# nodb
return None
session.is_dirty = False
return session, dbname
# =====================================================
# Getters and setters
@@ -1014,6 +1045,27 @@ class Request:
_cr = cr
@property
def geoip(self):
"""
Get the remote address geolocalisation.
When geolocalization is successful, the return value is a
dictionary whoose format is:
{'city': str, 'country_code': str, 'country_name': str,
'latitude': float, 'longitude': float, 'region': str,
'time_zone': str}
When geolocalization fails, an empty dict is returned.
"""
if '_geoip' not in self.session:
was_dirty = self.session.is_dirty
self.session._geoip = (self.registry['ir.http']._geoip_resolve()
if self.db else self._geoip_resolve())
self.session.is_dirty = was_dirty
return self.session._geoip
# =====================================================
# Helpers
# =====================================================
@@ -1090,6 +1142,11 @@ class Request:
except (ValueError, KeyError):
return DEFAULT_LANG
def _geoip_resolve(self):
if not (root.geoip_resolver and self.httprequest.remote_addr):
return {}
return root.geoip_resolver.resolve(self.httprequest.remote_addr) or {}
def get_http_params(self):
"""
Extract key=value pairs from the query string and the forms
@@ -1212,8 +1269,10 @@ class Request:
return
if sess.should_rotate:
sess['_geoip'] = self.geoip
root.session_store.rotate(sess, self.env) # it saves
elif sess.should_touch or json.dumps(sess) != sess.json_data:
elif sess.is_dirty:
sess['_geoip'] = self.geoip
root.session_store.save(sess)
# We must not set the cookie if the session id was specified
@@ -1226,7 +1285,7 @@ class Request:
# cookie). That is a special feature of the Javascript Session.
# - It could allow session fixation attacks.
cookie_sid = self.httprequest.cookies.get('session_id')
if (sess.should_touch or cookie_sid != sess.sid and not sess.is_explicit):
if not sess.is_explicit and (sess.is_dirty or cookie_sid != sess.sid):
self.future_response.set_cookie('session_id', sess.sid, max_age=SESSION_LIFETIME, httponly=True)
def _set_request_dispatcher(self, rule):
@@ -1620,6 +1679,13 @@ class Application:
_logger.debug('HTTP sessions stored in: %s', path)
return FilesystemSessionStore(path, session_class=Session, renew_missing=True)
@lazy_property
def geoip_resolver(self):
try:
return GeoIPResolver.open(config.get('geoip_database'))
except Exception as e:
_logger.warning('Cannot load GeoIP: %s', e)
def get_db_router(self, db):
if not db:
return self.nodb_routing_map
odoo/service/model.py
+1 -2
View File
@@ -143,8 +143,7 @@ def retrying(func, env):
env.cr.rollback()
env.registry.reset_changes()
if request:
request.session.clear()
request.session.update(json.loads(request.session.json_data))
request.session = request._get_session_and_dbname()[0]
if isinstance(exc, IntegrityError):
raise _as_validation_error(env, exc) from exc
if exc.pgcode not in PG_CONCURRENCY_ERRORS_TO_RETRY:
addons/http_routing/geoipresolver.py → odoo/tools/geoipresolver.py
View File