From b403fe8f39b8e8527db07170e11ee6b67ce16231 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicolas=20=C3=89vrard?= Date: Mon, 30 Mar 2026 16:24:44 +0200 Subject: [PATCH] Use a cookie to store the session token Closes #14412 --- modules/authentication_saml/routes.py | 17 ++- sao/CHANGELOG | 1 + sao/src/bus.js | 3 - sao/src/rpc.js | 3 - sao/src/sao.js | 3 +- sao/src/session.js | 48 ++------ trytond/CHANGELOG | 1 + trytond/doc/topics/configuration.rst | 14 +++ trytond/trytond/protocols/dispatcher.py | 40 +++++-- trytond/trytond/protocols/wrappers.py | 69 ++++++++++- trytond/trytond/tests/test_wsgi.py | 145 +++++++++++++++++++++++- trytond/trytond/wsgi.py | 8 +- 12 files changed, 284 insertions(+), 68 deletions(-) diff --git a/modules/authentication_saml/routes.py b/modules/authentication_saml/routes.py index cf935a5454..7fc1cf5a78 100644 --- a/modules/authentication_saml/routes.py +++ b/modules/authentication_saml/routes.py @@ -12,8 +12,8 @@ except ImportError: import trytond.config as config from trytond.protocols.dispatcher import register_authentication_service from trytond.protocols.wrappers import ( - Response, abort, allow_null_origin, exceptions, redirect, with_pool, - with_transaction) + Response, abort, add_auth_cookies, allow_null_origin, exceptions, redirect, + with_pool, with_transaction) from trytond.transaction import Transaction from trytond.url import http_host from trytond.wsgi import app @@ -171,8 +171,17 @@ def acs(request, pool, identity): query.append(('database', pool.database_name)) query.append(('login', login)) query.append(('user_id', user_id)) - query.append(('session', session)) + tryton_client = redirect_url.startswith('http://localhost:') + if tryton_client: + # Add the session as a parameter + # such that the Tryton client can retrieve it + query.append(('session', session)) query.append(('bus_url_host', bus_url_host if allow_subscribe else '')) parts = list(parts) parts[3] = urllib.parse.urlencode(query) - return redirect(urllib.parse.urlunsplit(parts)) + response = redirect(urllib.parse.urlunsplit(parts)) + if not tryton_client: + # Do not set cookies for Tryton client + add_auth_cookies( + response, pool.database_name, login, str(user_id), session) + return response diff --git a/sao/CHANGELOG b/sao/CHANGELOG index 7842f33714..53112828e3 100644 --- a/sao/CHANGELOG +++ b/sao/CHANGELOG @@ -1,3 +1,4 @@ +* Use cookie to store session * Move the logout entry and add a help entry to the notification menu * Add visual hint on widget of modified field * Add support for Python 3.14 diff --git a/sao/src/bus.js b/sao/src/bus.js index 6aae5ef78e..48e76c4c97 100644 --- a/sao/src/bus.js +++ b/sao/src/bus.js @@ -25,9 +25,6 @@ let url = new URL(`${session.database}/bus`, session.bus_url_host); Sao.Bus.last_message = last_message; Sao.Bus.request = jQuery.ajax({ - headers: { - Authorization: 'Session ' + session.get_auth(), - }, contentType: 'application/json', data: JSON.stringify({ last_message: last_message, diff --git a/sao/src/rpc.js b/sao/src/rpc.js index 5dcfdca623..5e3eb473c9 100644 --- a/sao/src/rpc.js +++ b/sao/src/rpc.js @@ -171,9 +171,6 @@ jQuery.ajax({ 'async': async, - 'headers': { - 'Authorization': 'Session ' + session.get_auth() - }, 'contentType': 'application/json', 'data': JSON.stringify(Sao.rpc.prepareObject({ 'id': id_, diff --git a/sao/src/sao.js b/sao/src/sao.js index cfc42e264e..04642cd808 100644 --- a/sao/src/sao.js +++ b/sao/src/sao.js @@ -1201,13 +1201,12 @@ var Sao = { jQuery(document).ready(function() { var url = new URL(window.location); - if (url.searchParams.has('session')) { + if (url.searchParams.has('login_service')) { var database = url.searchParams.get('database'); var session = { login_service: url.searchParams.get('login_service'), login: url.searchParams.get('login'), user_id: parseInt(url.searchParams.get('user_id'), 10), - session: url.searchParams.get('session'), bus_url_host: url.searchParams.get('bus_url_host'), }; if (url.searchParams.has('renew')) { diff --git a/sao/src/session.js b/sao/src/session.js index 38657aa870..e966a0c0f0 100644 --- a/sao/src/session.js +++ b/sao/src/session.js @@ -3,16 +3,10 @@ (function() { 'use strict'; - // https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/btoa#Unicode_strings - function utoa(str) { - return window.btoa(unescape(encodeURIComponent(str))); - } - Sao.Session = Sao.class_(Object, { init: function(database, login) { this.login_service = null; this.user_id = null; - this.session = null; this.bus_url_host = null; this.cache = new Cache(); this.prm = jQuery.when(); // renew promise @@ -25,9 +19,6 @@ Sao.Session.current_session = this; } }, - get_auth: function() { - return utoa(this.login + ':' + this.user_id + ':' + this.session); - }, do_login: function(parameters) { var dfd = jQuery.Deferred(); var login = this.login; @@ -47,14 +38,12 @@ new Sao.Login(func, this).run().then(result => { this.login = login; this.user_id = result[0]; - this.session = result[1]; - this.bus_url_host = result[2]; + this.bus_url_host = result[1]; this.store(); this.renew_device_cookie(); dfd.resolve(); }, () => { this.user_id = null; - this.session = null; this.bus_url_host = null; this.store(); dfd.reject(); @@ -62,29 +51,20 @@ return dfd.promise(); }, do_logout: function() { - if (!(this.user_id && this.session)) { + if (!this.user_id) { return jQuery.when(); } - var args = { - 'id': 0, - 'method': 'common.db.logout', - 'params': [] - }; var prm = jQuery.ajax({ - 'headers': { - 'Authorization': 'Session ' + this.get_auth() - }, 'contentType': 'application/json', - 'data': JSON.stringify(args), + 'data': JSON.stringify({}), 'dataType': 'json', - 'url': '/' + this.database + '/', + 'url': `/${this.database}/session/logout`, 'type': 'post', }); this.unstore(); this.database = null; this.login = null; this.user_id = null; - this.session = null; if (Sao.Session.current_session === this) { Sao.Session.current_session = null; } @@ -141,7 +121,7 @@ sessionStorage.setItem('sao_context_' + this.database, context); }, restore: function() { - if (this.database && !this.session) { + if (this.database) { var session_data = localStorage.getItem( 'sao_session_' + this.database); if (session_data !== null) { @@ -150,7 +130,6 @@ this.login_service = session_data.login_service; this.login = session_data.login; this.user_id = session_data.user_id; - this.session = session_data.session; this.bus_url_host = session_data.bus_url_host; } } @@ -160,7 +139,6 @@ var session = { 'login': this.login, 'user_id': this.user_id, - 'session': this.session, 'bus_url_host': this.bus_url_host, }; session = JSON.stringify(session); @@ -280,7 +258,7 @@ var database = database_url(); var session = new Sao.Session(database, null); - if (session.session) { + if (session.user_id) { dfd.resolve(session); return dfd; } @@ -311,7 +289,7 @@ session.database = database; session.login = login; session.restore(); - (session.session ? jQuery.when() : session.do_login()) + (session.user_id ? jQuery.when() : session.do_login()) .then(function() { dialog.modal.modal('hide'); dfd.resolve(session); @@ -348,7 +326,7 @@ session.database = database; session.login = null; session.restore(); - if (session.session) { + if (session.user_id) { dfd.resolve(session); dialog.modal.remove(); if (database_url() != database) { @@ -421,7 +399,6 @@ return session.prm; } var dfd = jQuery.Deferred(); - session.session = null; session.prm = dfd.promise(); if (!session.login_service) { session.do_login().then(dfd.resolve, function() { @@ -441,7 +418,7 @@ if (service_window.closed) { window.clearInterval(timer); session.restore(); - if (session.session) { + if (session.user_id) { dfd.resolve(); } else { Sao.logout(); @@ -472,17 +449,12 @@ 'contentType': 'application/json', 'data': JSON.stringify(data), 'dataType': 'json', - 'url': '/' + this.session.database + '/', + 'url': `/${this.session.database}/session/login`, 'type': 'post', 'complete': [function() { Sao.common.processing.hide(timeoutID); }] }; - if (this.session.user_id && this.session.session) { - args.headers = { - 'Authorization': 'Session ' + this.session.get_auth() - }; - } var ajax_prm = jQuery.ajax(args); var ajax_success = function(data) { diff --git a/trytond/CHANGELOG b/trytond/CHANGELOG index 50a5456227..1611b162d7 100644 --- a/trytond/CHANGELOG +++ b/trytond/CHANGELOG @@ -1,3 +1,4 @@ +* Add route for login / logout with cookie * Add contextual ``_log`` to force logging events * Add notify_user to ModelStorage * Check button states when testing access diff --git a/trytond/doc/topics/configuration.rst b/trytond/doc/topics/configuration.rst index bea85d9f3b..5e1eec3b12 100644 --- a/trytond/doc/topics/configuration.rst +++ b/trytond/doc/topics/configuration.rst @@ -668,6 +668,19 @@ authentication method option. Default: ``''`` +.. _config-session.cookie_domain: + +cookie_domain +~~~~~~~~~~~~~ + +The cookie `Domain attribute`_ set when the authentication completes. + +Default: ``''`` + +.. note:: + When accessing the server through ``localhost`` this option should be left + empty as cookies with the domain ``localhost`` are not strored by browsers. + .. _config-session.max_age: max_age @@ -952,3 +965,4 @@ Example:: .. _STARTTLS: http://en.wikipedia.org/wiki/STARTTLS .. _WSGI middleware: https://en.wikipedia.org/wiki/Web_Server_Gateway_Interface#Specification_overview .. _`multi-factor authentication`: https://en.wikipedia.org/wiki/Multi-factor_authentication +.. _`Domain attribute`: https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Set-Cookie#domaindomain-value diff --git a/trytond/trytond/protocols/dispatcher.py b/trytond/trytond/protocols/dispatcher.py index 3c84012ba2..6e3e4c2071 100644 --- a/trytond/trytond/protocols/dispatcher.py +++ b/trytond/trytond/protocols/dispatcher.py @@ -16,7 +16,9 @@ from trytond.transaction import Transaction, TransactionError from trytond.worker import run_task from trytond.wsgi import app -from .wrappers import HTTPStatus, Response, abort, with_pool +from .wrappers import ( + TRYTON_SESSION_COOKIE, HTTPStatus, Response, abort, add_auth_cookies, + decode_session_cookie, remove_auth_cookies, with_pool) __all__ = ['register_authentication_service'] @@ -68,6 +70,28 @@ def logout(request, database_name): context={'_request': request.context}) +@app.route('//session/login', methods=['POST']) +def login_w_cookies(request, database_name): + user_id, token, bus_url_host = login( + request, database_name, *request.json['params']) + response = app.make_response(request, (user_id, bus_url_host)) + add_auth_cookies( + response, database_name, request.json['params'][0], str(user_id), + token) + return response + + +@app.route('//session/logout', methods=['POST']) +def logout_w_cookies(request, database_name): + cookie = request.cookies.get(TRYTON_SESSION_COOKIE) + _, user_id, token = decode_session_cookie(cookie) + security.logout( + database_name, user_id, token, context={'_request': request.context}) + response = app.make_response(request, None) + remove_auth_cookies(response, database_name) + return response + + def reset_password(request, database_name, user, language=None): authentications = config.get( 'session', 'authentications', default='password').split(',') @@ -168,9 +192,14 @@ def _dispatch(request, pool, *args, **kwargs): abort(HTTPStatus.FORBIDDEN) user = request.user_id - session = None - if request.authorization.type == 'session': - session = request.authorization.get('session') + if request.session: + username = request.session.username + session = request.session.token + elif request.authorization: + username = request.authorization.username + session = None + if isinstance(username, bytes): + username = username.decode('utf-8') if rpc.fresh_session and session: context = {'_request': request.context} @@ -179,9 +208,6 @@ def _dispatch(request, pool, *args, **kwargs): abort(HTTPStatus.UNAUTHORIZED) log_message = '%s.%s%s from %s@%s%s in %i ms' - username = request.authorization.username - if isinstance(username, bytes): - username = username.decode('utf-8') log_args = ( obj.__name__, method, format_args(args, kwargs, logger.isEnabledFor(logging.DEBUG)), diff --git a/trytond/trytond/protocols/wrappers.py b/trytond/trytond/protocols/wrappers.py index 83b4bccbc5..5fd2eb5dfb 100644 --- a/trytond/trytond/protocols/wrappers.py +++ b/trytond/trytond/protocols/wrappers.py @@ -1,6 +1,7 @@ # This file is part of Tryton. The COPYRIGHT file at the top level of # this repository contains the full copyright notices and license terms. import base64 +import collections import gzip import logging import time @@ -38,11 +39,51 @@ __all__ = [ 'user_application', 'with_pool', 'with_transaction', + 'encode_session_cookie', + 'decode_session_cookie', + 'add_auth_cookies', + 'remove_auth_cookies', + 'add_cookie', + 'remove_cookie', + 'TRYTON_SESSION_COOKIE', ] +TRYTON_SESSION_COOKIE = 'tryton_session' logger = logging.getLogger(__name__) +def encode_session_cookie(login, userid, token): + return ':'.join((login, userid, token)) + + +def decode_session_cookie(cookie): + return cookie.rsplit(':', 2) + + +def add_cookie(response, database, name, value): + response.set_cookie(name, value, + max_age=config.getint('session', 'max_age'), + path=f'/{database}', + domain=config.get('session', 'cookie_domain'), + secure=True, httponly=True, samesite='Strict') + + +def add_auth_cookies(response, database, username, userid, token): + session_cookie = encode_session_cookie(username, userid, token) + add_cookie(response, database, TRYTON_SESSION_COOKIE, session_cookie) + + +def remove_cookie(response, database, name): + response.set_cookie( + name, '', expires=0, path=f'/{database}', + domain=config.get('session', 'cookie_domain'), + secure=True, httponly=True, samesite='Strict') + + +def remove_auth_cookies(response, database): + remove_cookie(response, database, TRYTON_SESSION_COOKIE) + + class Request(_Request): view_args = None @@ -91,6 +132,25 @@ class Request(_Request): def rpc_params(self): return + @cached_property + def session(self): + cookie = self.cookies.get(TRYTON_SESSION_COOKIE) + if cookie: + try: + username, userid, token = decode_session_cookie(cookie) + session = Session('cookie', username, int(userid), token) + except ValueError: + session = None + elif self.authorization and self.authorization.type == 'session': + session = Session( + 'authorization', + self.authorization.username, + int(self.authorization.get('userid')), + self.authorization.get('session')) + else: + session = None + return session + @cached_property def authorization(self): authorization = super().authorization @@ -114,12 +174,12 @@ class Request(_Request): if not database_name: return None auth = self.authorization - if not auth: + if not self.session and not auth: return None context = {'_request': self.context} - if auth.type == 'session': + if self.session: user_id = security.check( - database_name, auth.get('userid'), auth.get('session'), + database_name, self.session.userid, self.session.token, context=context) elif auth.username: parameters = getattr(auth, 'parameters', auth) @@ -143,6 +203,9 @@ class Request(_Request): } +Session = collections.namedtuple('Session', 'type username userid token') + + def parse_authorization_header(value): if not value: return diff --git a/trytond/trytond/tests/test_wsgi.py b/trytond/trytond/tests/test_wsgi.py index 55ea12aa61..eb09119087 100644 --- a/trytond/trytond/tests/test_wsgi.py +++ b/trytond/trytond/tests/test_wsgi.py @@ -10,9 +10,12 @@ from werkzeug.routing import Map, Rule from trytond import security from trytond.exceptions import TrytonException from trytond.pool import Pool -from trytond.protocols.wrappers import Response +from trytond.protocols.wrappers import ( + TRYTON_SESSION_COOKIE, Response, decode_session_cookie, + encode_session_cookie) from trytond.tests.test_tryton import Client, RouteTestCase, TestCase -from trytond.wsgi import Base64Converter, TrytondWSGI +from trytond.transaction import Transaction +from trytond.wsgi import Base64Converter, TrytondWSGI, app class WSGIAppTestCase(TestCase): @@ -152,6 +155,32 @@ class TrytonWSGITestCase(RouteTestCase): 'password': '12345678', }]) + def test_basic_good_auth(self): + "Test that auth_required works with basic auth" + @app.route('//auth_required') + @app.auth_required + def _route(request, database_name): + return Response(b'') + + basic_auth = 'Basic ' + base64.b64encode(b"user:12345678").decode() + response = self.client().get( + f'/{self.db_name}/auth_required', + headers=[('Authorization', basic_auth)]) + self.assertEqual(response.status_code, HTTPStatus.OK) + + def test_basic_bad_auth(self): + "Test that auth_required don't accept wrong password with basic auth" + @app.route('//auth_required') + @app.auth_required + def _route(request, database_name): + return Response(b'') + + basic_auth = 'Basic ' + base64.b64encode(b"1:Wrong Password").decode() + response = self.client().get( + f'/{self.db_name}/auth_required', + headers=[('Authorization', basic_auth)]) + self.assertEqual(response.status_code, HTTPStatus.UNAUTHORIZED) + def test_session_valid_good_auth(self): "Test that session_valid correctly authenticates" app = TrytondWSGI() @@ -221,3 +250,115 @@ class TrytonWSGITestCase(RouteTestCase): client = Client(app, Response) response = client.get(f'/{self.db_name}/session_required') self.assertEqual(response.status_code, HTTPStatus.UNAUTHORIZED) + + def test_cookie_authentication_good_auth(self): + "Test that session_valid authenticates with the cookie" + @app.route('//session_required') + @app.session_valid + def _route(request, database_name): + return Response(b'') + + user_id, key = security.login( + self.db_name, 'user', {'password': '12345678'}) + + client = self.client() + client.set_cookie( + TRYTON_SESSION_COOKIE, + encode_session_cookie('user', str(user_id), key), + path=f'/{self.db_name}') + response = client.get(f'/{self.db_name}/session_required') + self.assertEqual(response.status_code, HTTPStatus.OK) + + def test_cookie_authentication_bad_auth(self): + "Test that session_valid refuses wrong cookie content" + @app.route('//session_required') + @app.session_valid + def _route(request, database_name): + return Response(b'') + + client = self.client() + client.set_cookie( + TRYTON_SESSION_COOKIE, + encode_session_cookie('user', '1', 'Wrong Token'), + path=f'/{self.db_name}') + response = client.get(f'/{self.db_name}/session_required') + self.assertEqual(response.status_code, HTTPStatus.UNAUTHORIZED) + + def test_cookie_login(self): + "Test logging in through the cookie setting route" + client = self.client() + client.post(f'/{self.db_name}/session/login', json={ + 'method': 'common.db.login', + 'params': ['user', {'password': '12345678'}, 'en'], + }) + session_cookie = client.get_cookie( + TRYTON_SESSION_COOKIE, path=f'/{self.db_name}').value + _, _, token = session_cookie.rsplit(':', 2) + + with Transaction().start(self.db_name, 0): + pool = Pool() + Session = pool.get('ir.session') + sessions = Session.search([('key', '=', token)]) + self.assertEqual(len(sessions), 1) + + def test_cookie_logout(self): + "Test logging out through the cookie unsetting route" + client = self.client() + client.post(f'/{self.db_name}/session/login', json={ + 'method': 'common.db.login', + 'params': ['user', {'password': '12345678'}, 'en'], + }) + session_cookie = client.get_cookie( + TRYTON_SESSION_COOKIE, path=f'/{self.db_name}').value + _, _, token = decode_session_cookie(session_cookie) + client.post(f'/{self.db_name}/session/logout') + + self.assertIsNone( + client.get_cookie(TRYTON_SESSION_COOKIE, path=f'/{self.db_name}')) + with Transaction().start(self.db_name, 0): + pool = Pool() + Session = pool.get('ir.session') + sessions = Session.search([('key', '=', token)]) + self.assertEqual(len(sessions), 0) + + def test_cookie_precedence_good_auth(self): + "Test the cookie have precedence over Authorization header" + @app.route('//session_required') + @app.session_valid + def _route(request, database_name): + return Response(b'') + + user_id, key = security.login( + self.db_name, 'user', {'password': '12345678'}) + client = self.client() + client.set_cookie( + TRYTON_SESSION_COOKIE, + encode_session_cookie('user', str(user_id), key), + path=f'/{self.db_name}') + session_hdr = 'Session ' + base64.b64encode( + f'user:{user_id}:Wrong Key'.encode('utf8')).decode('utf8') + response = client.get( + f'/{self.db_name}/session_required', + headers=[('Authorization', session_hdr)]) + self.assertEqual(response.status_code, HTTPStatus.OK) + + def test_cookie_precedence_bad_auth(self): + "Test the cookie have precedence over Authorization header" + @app.route('//session_required') + @app.session_valid + def _route(request, database_name): + return Response(b'') + + user_id, key = security.login( + self.db_name, 'user', {'password': '12345678'}) + client = self.client() + client.set_cookie( + TRYTON_SESSION_COOKIE, + encode_session_cookie('user', str(user_id), 'Wrong Key'), + path=f'/{self.db_name}') + session_hdr = 'Session ' + base64.b64encode( + f'user:{user_id}:{key}'.encode('utf8')).decode('utf8') + response = client.get( + f'/{self.db_name}/session_required', + headers=[('Authorization', session_hdr)]) + self.assertEqual(response.status_code, HTTPStatus.UNAUTHORIZED) diff --git a/trytond/trytond/wsgi.py b/trytond/trytond/wsgi.py index 0e4ed10633..f631e03faf 100644 --- a/trytond/trytond/wsgi.py +++ b/trytond/trytond/wsgi.py @@ -95,15 +95,11 @@ class TrytondWSGI(object): def session_valid(self, func): @wraps(func) def wrapper(request, *args, **kwargs): - if (not request.authorization - or request.authorization.type != 'session'): + if request.session is None: _do_basic_auth(request) - userid = request.authorization.get('userid') - session = request.authorization.get('session') dbname = request.view_args.get('database_name') - session_check = security.check( - dbname, userid, session, { + dbname, request.session.userid, request.session.token, { '_request': { 'remote_addr': request.remote_addr, },