Skip to content

Commit

Permalink
[IMP] core: add session deletion mechanism
Browse files Browse the repository at this point in the history
Objective:
----------
A user must be able to see which of his sessions/devices are active.
Make it easy to block devices (individually or by group) and analyse current sessions.
If a user notices unusual operations concerning him on another device, he must be able
to stop these operations by blocking the session used by this device.

General:
--------
- A device is identified by a static part, the device's fingerprint (the user agent).
- A device is tracked dynamically via its IP address and the time representing its last activity.
- A device is always linked to one and only one session.
- A session is linked to at least one device.

If the fingerprint or IP address of a session is modified, we assume that a new device
is detected, so it will be created.

The same device is updated every X seconds in order to track its use in terms
of duration of activity.

The `_update_device` method is placed at the point where we want to collect information and check whether
we need to create a new device.

The location is important because it has to be able to intercept requests from scripts, for example,
so as not to establish 'false' security.

Blocking sessions:
------------------
Blocking a session by selecting a device must block the session directly.
From an administrator's point of view, if we want to block a session, we expect
the session file to be deleted directly on the filesystem.
If the session file is no longer present on the filesystem,
we can be sure that there is no longer any risk of session usurpation.

Note:
There are several ways of blocking the session, but this is the safest.
Blocking a session based on DB values does not cover scenarios
in which we perform a backup, for example.

Find the session file:
----------------------
To find a session file on the filesystem, we need to know the sid of the session (as this is its filename).
We don't want to store the sid in the database.
However, we can store a part of it with:
- a large enough part to be certain of the uniqueness of the session;
- a small enough part that we cannot brute force the end of the sid.

Browsing the filesystem has a certain performance cost (and can therefore have defects if abused).

The proposed solution is to change the granularity of the way we store the session on the filesystem.
This means finding a compromise between sub-folders and files per sub-folder to browse for a session file.

The sid will be Base64 encoded in order to increase the number of sub-folders (64^2 instead of 16^2).

Delete devices:
---------------
The record representing a device is deleted via a garbage collector.

Records for which the last activity is over a week old will be deleted.
This allows us to keep recent traces of the device as well as its duration of activity
(because we keep the first activity time).

Task:3627898
  • Loading branch information
thle-odoo committed May 16, 2024
1 parent 87b3f6f commit adc6ff0
Show file tree
Hide file tree
Showing 15 changed files with 660 additions and 10 deletions.
2 changes: 1 addition & 1 deletion addons/bus/models/ir_websocket.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def _update_bus_presence(self, inactivity_period, im_status_ids_by_model):
@classmethod
def _authenticate(cls):
if wsrequest.session.uid is not None:
if not security.check_session(wsrequest.session, wsrequest.env):
if not (security.check_session(wsrequest.session, wsrequest.env) and security.update_device(wsrequest)):
wsrequest.session.logout(keep_db=True)
raise SessionExpiredException()
else:
Expand Down
2 changes: 1 addition & 1 deletion addons/web/controllers/home.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def web_client(self, s_action=None, **kw):
return request.redirect_query('/web/login', query={'redirect': request.httprequest.full_path}, code=303)
if kw.get('redirect'):
return request.redirect(kw.get('redirect'), 303)
if not security.check_session(request.session, request.env):
if not (security.check_session(request.session, request.env) and security.update_device(request)):
raise http.SessionExpiredException("Session expired")
if not is_user_internal(request.session.uid):
return request.redirect('/web/login_successful', 303)
Expand Down
13 changes: 13 additions & 0 deletions odoo/addons/base/i18n/base.pot
Original file line number Diff line number Diff line change
Expand Up @@ -12457,6 +12457,12 @@ msgid ""
"This is the most basic SMTP authentication process and may not be accepted by all providers. \n"
msgstr ""

#. module: base
#. odoo-python
#: code:addons/base/models/res_users.py:0
msgid "Connected devices"
msgstr ""

#. module: base
#: model_terms:ir.ui.view,arch_db:base.ir_mail_server_form
msgid "Connection"
Expand Down Expand Up @@ -13921,6 +13927,13 @@ msgid ""
"before the address."
msgstr ""

#. module: base
#: model:ir.actions.act_window,name:base.action_users_device
#: model:ir.model,name:base.model_res_users_device
#: model_terms:ir.ui.view,arch_db:base.view_users_form_simple_modif
msgid "Devices"
msgstr ""

#. module: base
#: model:ir.module.module,shortdesc:base.module_digest_enterprise
msgid "Digest Enterprise"
Expand Down
2 changes: 1 addition & 1 deletion odoo/addons/base/models/ir_http.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ def _authenticate(cls, endpoint):

try:
if request.session.uid is not None:
if not security.check_session(request.session, request.env):
if not (security.check_session(request.session, request.env) and security.update_device(request)):
request.session.logout(keep_db=True)
request.env = api.Environment(request.env.cr, None, request.session.context)
getattr(cls, f'_auth_method_{auth}')()
Expand Down
146 changes: 144 additions & 2 deletions odoo/addons/base/models/res_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import os
import time
from collections import defaultdict
from dateutil.relativedelta import relativedelta
from functools import wraps
from hashlib import sha256
from itertools import chain, repeat
Expand All @@ -24,12 +25,13 @@
from passlib.context import CryptContext
from psycopg2 import sql

import odoo
from odoo import api, fields, models, tools, SUPERUSER_ID, _, Command
from odoo.addons.base.models.ir_model import MODULE_UNINSTALL_FLAG
from odoo.exceptions import AccessDenied, AccessError, UserError, ValidationError
from odoo.http import request, DEFAULT_LANG
from odoo.http import GeoIP, request, DEFAULT_LANG
from odoo.osv import expression
from odoo.tools import is_html_empty, partition, collections, frozendict, lazy_property, SetDefinitions
from odoo.tools import is_html_empty, partition, collections, frozendict, lazy_property, SetDefinitions, SQL

_logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -330,6 +332,7 @@ def _default_groups(self):
help="If specified, this action will be opened at log on for this user, in addition to the standard menu.")
groups_id = fields.Many2many('res.groups', 'res_groups_users_rel', 'uid', 'gid', string='Groups', default=lambda s: s._default_groups())
log_ids = fields.One2many('res.users.log', 'create_uid', string='User log entries')
active_device_ids = fields.One2many('res.users.device', 'user_id', string='User devices', domain=[('is_active', '=', True)])
login_date = fields.Datetime(related='log_ids.create_date', string='Latest authentication', readonly=False)
share = fields.Boolean(compute='_compute_share', compute_sudo=True, string='Share User', store=True,
help="External user with limited access, created only for the purpose of sharing data.")
Expand Down Expand Up @@ -1040,6 +1043,16 @@ def action_revoke_all_devices(self):
'context': ctx,
}

def action_view_devices(self):
context = dict(self.env.context, search_default_filter_active_device=True)
return {
'name': _('Connected devices'),
'type': 'ir.actions.act_window',
'res_model': 'res.users.device',
'view_mode': 'tree,gantt,form',
'context': context,
}

def has_groups(self, group_spec: str) -> bool:
""" Return whether user ``self`` satisfies the given group restrictions
``group_spec``, i.e., whether it is member of at least one of the groups,
Expand Down Expand Up @@ -2300,3 +2313,132 @@ class APIKeyShow(models.AbstractModel):
# the field 'id' is necessary for the onchange that returns the value of 'key'
id = fields.Id()
key = fields.Char(readonly=True)


class Device(models.Model):
_name = 'res.users.device'
_description = 'Devices'

name = fields.Char("Device", required=True)
session_identifier = fields.Char("Session Identifier")
platform = fields.Char("Platform")
browser = fields.Char("Browser")
ip_address = fields.Char("IP Address")
country = fields.Char("Country")
city = fields.Char("City")
device_type = fields.Selection([('laptop', 'Laptop'), ('mobile', 'Mobile')], "Device Type")
first_activity = fields.Datetime("First Activity")
last_activity = fields.Datetime("Last Activity")
user_id = fields.Many2one("res.users", index=True)
on_disk = fields.Boolean("On Disk") # If `False`, we are sure that the session no longer exists on the filesystem

is_active = fields.Boolean("Active Device", compute="_compute_is_active", search="_search_is_active")
is_current = fields.Boolean("Current Device", compute="_compute_is_current")

@api.model
def _get_active_ids(self):
# A unique device can be qualified with (`session_identifier`, `name`, `ip_address`)
# Each unique device can have several records (according to activity times)
# Return max record ids for each unique device
last_devices = self._read_group(
domain=[('on_disk', '=', True)],
groupby=['session_identifier', 'name', 'ip_address'],
aggregates=['id:max']
)
return [device[-1] for device in last_devices]

def _search_is_active(self, operator, value):
if operator != '=' or not value:
return expression.FALSE_DOMAIN
return [('id', 'in', self._get_active_ids())]

def _compute_is_active(self):
active_device_ids = self._get_active_ids()
for device in self:
device.is_active = device.id in active_device_ids

def _compute_is_current(self):
for device in self:
device.is_current = request and request.session.sid.startswith(device.session_identifier)

@api.model
def _update_device(self, request):
"""
Must be called when we want to update the device for the current request.
Passage through this method must leave a "trace" in the session,
as the cursor may be readonly.
When the cursor is not readonly, it is necessary to ensure
that the devices are correctly encoded in the database.
:param request: Request or WebsocketRequest object
"""
user_agent = request.httprequest.user_agent
platform = user_agent.platform or 'Unknown'
browser = user_agent.browser or 'Unknown'
ip_address = request.httprequest.remote_addr

current_device = [platform, browser, ip_address]

now = int(datetime.datetime.now().timestamp())
for trace in request.session._trace:
device, time, sync = trace
if current_device == device:
if sync[0] or bool(now - time[1] >= 3600):
time[1] = now
sync[0] = True
request.session.is_dirty = True
break
else:
# Device doesn't exist in the session
trace = device, time, sync = [current_device, [now, now], [True]]
request.session._trace.append(trace)
request.session.is_dirty = True

if not sync[0] or self.env.cr.readonly:
return

sync[0] = False
geoip = GeoIP(ip_address)

self.env.cr.execute(SQL("""
INSERT INTO res_users_device (name, session_identifier, platform, browser, ip_address, country, city, device_type, first_activity, last_activity, user_id, on_disk)
VALUES (%(name)s, %(session_identifier)s, %(platform)s, %(browser)s, %(ip_address)s, %(country)s, %(city)s, %(device_type)s, %(first_activity)s, %(last_activity)s, %(user_id)s, %(on_disk)s)
""",
name=f"{platform.capitalize()} {browser.capitalize()}",
session_identifier=request.session.sid[:42],
platform=platform,
browser=browser,
ip_address=ip_address,
country=geoip.get('country_name'),
city=geoip.get('city'),
device_type='laptop' if platform.lower() not in ['android', 'iphone', 'ipad', 'ipod', 'blackberry', 'windows phone', 'webos'] else 'mobile',
first_activity=datetime.datetime.fromtimestamp(time[0]),
last_activity=datetime.datetime.fromtimestamp(time[1]),
user_id=request.session.uid,
on_disk=True
))

@api.autovacuum
def _gc_user_devices(self):
""" Keep only the most recent devices. """
self.env.cr.execute(SQL("""
DELETE FROM res_users_device
WHERE last_activity <= %s
""",
datetime.datetime.now(datetime.timezone.utc) - relativedelta(months=6)
))
_logger.info("GC user devices delete %d entries", self.env.cr.rowcount)

@check_identity
def revoke(self):
return self._revoke()

def _revoke(self):
self_sudo = self.sudo()
must_logout = bool(self.filtered('is_current'))
session_identifiers = list({device.session_identifier for device in self_sudo})
odoo.http.root.session_store.delete_from_identifiers(session_identifiers)
self_sudo.search([('session_identifier', 'in', session_identifiers)]).on_disk = False
_logger.info("User %d revokes user devices %s", self.env.uid, ','.join(map(str, self.ids)))
if must_logout:
request.session.logout()
14 changes: 14 additions & 0 deletions odoo/addons/base/security/base_security.xml
Original file line number Diff line number Diff line change
Expand Up @@ -203,5 +203,19 @@
<field name="domain_force">[(1, '=', 1)]</field>
<field name="groups" eval="[Command.link(ref('base.group_system'))]"/>
</record>

<record id="users_device" model="ir.rule">
<field name="name">Users can read only their own devices</field>
<field name="model_id" ref="model_res_users_device"/>
<field name="domain_force">[('user_id', '=', user.id)]</field>
<field name="groups" eval="[Command.link(ref('base.group_user'))]"/>
</record>
<record id="users_device_admin" model="ir.rule">
<field name="name">Administrators can read all devices</field>
<field name="model_id" ref="model_res_users_device"/>
<field name="domain_force">[(1, '=', 1)]</field>
<field name="groups" eval="[Command.link(ref('base.group_system'))]"/>
</record>

</data>
</odoo>
1 change: 1 addition & 0 deletions odoo/addons/base/security/ir.model.access.csv
Original file line number Diff line number Diff line change
Expand Up @@ -140,3 +140,4 @@ access_res_users_settings_user,res.users.settings,model_res_users_settings,group
"access_base_partner_merge_automatic_wizard","access.base.partner.merge.automatic.wizard","model_base_partner_merge_automatic_wizard","base.group_partner_manager",1,1,1,0
"access_ir_profile","ir_profile","model_ir_profile","group_system",1,1,1,1
"access_base_enable_profiling_wizard","access.base.enable.profiling.wizard","model_base_enable_profiling_wizard","group_system",1,1,1,0
"access_res_users_device",access_res_users_device,base.model_res_users_device,base.group_user,1,0,0,0

0 comments on commit adc6ff0

Please sign in to comment.