diff --git a/web/pgadmin/utils/driver/psycopg3/__init__.py b/web/pgadmin/utils/driver/psycopg3/__init__.py index 60d7f4c2d54..06edde02da4 100644 --- a/web/pgadmin/utils/driver/psycopg3/__init__.py +++ b/web/pgadmin/utils/driver/psycopg3/__init__.py @@ -81,9 +81,11 @@ def _restore_connections_from_session(self): session['__pgsql_server_managers'].copy() servers = get_user_server_query().filter( Server.is_adhoc == 0) + pga_user = self._current_pga_user() for server in servers: manager = managers[str(server.id)] = \ ServerManager(server) + manager.pga_user = pga_user # Suppress passexec for non-owners of shared # servers — it runs commands on the client # machine and must not inherit the owner's. @@ -91,13 +93,91 @@ def _restore_connections_from_session(self): server.user_id != current_user.id: manager.passexec = None if server.id in session_managers: - manager._restore( - session_managers[server.id]) - manager.update_session() + saved = session_managers[server.id] + if self._saved_state_is_stale( + saved, server, pga_user): + # The persisted blob was serialized under + # this numeric id by whatever Server row + # held it before (e.g. the configuration + # database was reset or restored without + # restarting pgAdmin), so it no longer + # describes this row. Restoring it would + # hand the new row the previous row's + # password/connection state. The same goes + # for state saved by a different pgAdmin + # user on this browser session. Drop it and + # let the manager start clean. + manager.update_session() + else: + manager._restore(saved) + manager.update_session() return managers return {} + @staticmethod + def _current_pga_user(): + """ + The fs_uniquifier of the logged-in pgAdmin user, which cached and + serialized ServerManager state is bound to. Unlike User.id it is + random, so it is not reused when the configuration database is + reset and a new user is created. + """ + return getattr(current_user, 'fs_uniquifier', None) + + @staticmethod + def _saved_state_is_stale(saved, server_data, pga_user=None): + """ + Same identity check as _manager_is_stale, applied to the + serialized ServerManager state carried across worker + restarts/new sessions in the Flask session + ('__pgsql_server_managers'), before it is restored onto a + manager that was just built fresh from the current Server row. + Without this, a reused server id would have its old serialized + password/connections restored onto the new row on the very + first request, before any manager exists to run + _manager_is_stale against. + + The state is also stale if it was saved by a different pgAdmin + user: nothing rotates the session id at login, so a new user + logging in on the same browser session must not inherit the + previous user's password or connections, even for a server with + identical connection details. + """ + return ( + saved.get('pga_user') != pga_user or + saved.get('host') != server_data.host or + saved.get('port') != server_data.port or + saved.get('db') != server_data.maintenance_db or + saved.get('user') != server_data.username or + saved.get('service') != server_data.service or + saved.get('tunnel_host') != server_data.tunnel_host + ) + + @staticmethod + def _manager_is_stale(manager, server_data, pga_user=None): + """ + A cached manager is normally kept in sync with edits to its + Server row via explicit manager.update() calls from the + server-edit endpoints. It can still go stale in place if the + row itself was swapped out from under it, e.g. a numeric + server id reused by an unrelated row after the configuration + database was reset or restored without restarting pgAdmin, so + compare against what actually identifies the target rather + than trusting the id match alone. It is also stale if it was + built for a different pgAdmin user on the same browser session + (see _saved_state_is_stale). + """ + return ( + getattr(manager, 'pga_user', None) != pga_user or + manager.host != server_data.host or + manager.port != server_data.port or + manager.db != server_data.maintenance_db or + manager.user != server_data.username or + manager.service != server_data.service or + manager.tunnel_host != server_data.tunnel_host + ) + def connection_manager(self, sid=None): """ connection_manager(...) @@ -143,15 +223,45 @@ def connection_manager(self, sid=None): managers = self.managers[session.sid] if str(sid) in managers: manager = managers[str(sid)] + pga_user = self._current_pga_user() with connection_restore_lock: - manager._restore_connections() - manager.update_session() + if self._manager_is_stale( + manager, server_data, pga_user): + # The id has been reused by an unrelated Server + # row (e.g. the configuration database was reset + # or restored without restarting pgAdmin), so the + # cached manager still points at whatever server + # it was originally built from. Drop it rather + # than report a live connection to a server that, + # from this row's perspective, was never opened. + # The same applies to a manager built for another + # pgAdmin user on this browser session. + manager.release() + manager.update(server_data) + manager.pga_user = pga_user + else: + manager._restore_connections() + manager.update_session() + # Identity (host/port/db/user/service/tunnel) + # still matches, so the live connection is kept, + # but access-control-relevant metadata such as + # shared/ownership is not part of that identity + # check and manager.update() was skipped above - + # refresh it here too, otherwise a row whose + # sharing/ownership changed via the same reused-id + # path could keep serving the previous owner's + # passexec to a new, non-owning user. + manager.shared = server_data.shared + if config.SERVER_MODE and server_data.shared and \ + server_data.user_id != current_user.id: + manager.passexec = None managers['pinged'] = datetime.datetime.now() if str(sid) not in managers: # server_data was already access-checked above; # it cannot be None at this point. manager = ServerManager(server_data) + manager.pga_user = self._current_pga_user() # Suppress passexec for non-owners of shared # servers — it runs commands on the client machine # and must not inherit the owner's. diff --git a/web/pgadmin/utils/driver/psycopg3/server_manager.py b/web/pgadmin/utils/driver/psycopg3/server_manager.py index 00738355ee4..febcfca4feb 100644 --- a/web/pgadmin/utils/driver/psycopg3/server_manager.py +++ b/web/pgadmin/utils/driver/psycopg3/server_manager.py @@ -54,6 +54,9 @@ def __init__(self, server): self.tunnel_object = None self.tunnel_created = False self.display_connection_string = '' + # fs_uniquifier of the pgAdmin user this manager was built for; + # set by the driver, see Driver._current_pga_user. + self.pga_user = None self.update(server) @@ -154,6 +157,19 @@ def as_dict(self): res['ver'] = self.ver res['sversion'] = self.sversion + # Persisted alongside the connection state so a later restore + # (e.g. after a worker restart) can tell whether this blob still + # belongs to the Server row for this id, or whether the id was + # reused by an unrelated row after the configuration database + # was reset/restored - see Driver._manager_is_stale. + res['host'] = self.host + res['port'] = self.port + res['db'] = self.db + res['user'] = self.user + res['service'] = self.service + res['tunnel_host'] = self.tunnel_host + res['pga_user'] = self.pga_user + self._set_password(res) if self.use_ssh_tunnel: diff --git a/web/pgadmin/utils/driver/psycopg3/tests/test_manager_is_stale.py b/web/pgadmin/utils/driver/psycopg3/tests/test_manager_is_stale.py new file mode 100644 index 00000000000..77e74e07d0c --- /dev/null +++ b/web/pgadmin/utils/driver/psycopg3/tests/test_manager_is_stale.py @@ -0,0 +1,222 @@ +########################################################################## +# +# pgAdmin 4 - PostgreSQL Tools +# +# Copyright (C) 2013 - 2026, The pgAdmin Development Team +# This software is released under the PostgreSQL Licence +# +########################################################################## + +""" +Unit tests for Driver._manager_is_stale and Driver._saved_state_is_stale. + +These are pure attribute-comparison tests, run as plain unittest +TestCases without needing a Postgres server connection. +""" + +import unittest +from types import SimpleNamespace + +from pgadmin.utils.route import BaseTestGenerator +from pgadmin.utils.driver.psycopg3 import Driver + + +def make_manager(**overrides): + fields = dict( + host='old-host', port=5432, db='postgres', user='old-user', + service=None, tunnel_host=None, + ) + fields.update(overrides) + return SimpleNamespace(**fields) + + +def make_server_data(**overrides): + fields = dict( + host='old-host', port=5432, maintenance_db='postgres', + username='old-user', service=None, tunnel_host=None, + ) + fields.update(overrides) + return SimpleNamespace(**fields) + + +def make_saved_state(**overrides): + """Mimics the identity fields ServerManager.as_dict() persists into + the Flask session ('__pgsql_server_managers') alongside the + serialized password/connections.""" + fields = dict( + host='old-host', port=5432, db='postgres', user='old-user', + service=None, tunnel_host=None, + ) + fields.update(overrides) + return fields + + +class _PureUnitTestSetupMixin: + """setUp here calls unittest.TestCase.setUp directly, skipping + BaseTestGenerator.setUp's Postgres connection.""" + + def setUp(self): + unittest.TestCase.setUp(self) + + +class TestManagerIsStaleMatchesUnchanged( + _PureUnitTestSetupMixin, BaseTestGenerator): + """A manager whose identity fields still match the current Server + row is not stale, even if the row was legitimately edited via the + normal manager.update() flow elsewhere.""" + + scenarios = [('default', dict())] + + def runTest(self): + manager = make_manager() + server_data = make_server_data() + + self.assertFalse(Driver._manager_is_stale(manager, server_data)) + + +class TestManagerIsStaleDetectsReusedId( + _PureUnitTestSetupMixin, BaseTestGenerator): + """A manager built from a Server row that no longer matches the + current row for this id (e.g. the id was reused after the + configuration database was reset) must be treated as stale.""" + + scenarios = [('default', dict())] + + def runTest(self): + manager = make_manager(host='deleted-server.example.com') + server_data = make_server_data(host='new-server.example.com') + + self.assertTrue(Driver._manager_is_stale(manager, server_data)) + + +class TestManagerIsStaleChecksEachIdentityField( + _PureUnitTestSetupMixin, BaseTestGenerator): + """Any one of host/port/db/user/service/tunnel_host differing is + enough to mark the manager stale.""" + + scenarios = [('default', dict())] + + def runTest(self): + server_data = make_server_data() + + for field, value in ( + ('port', 5433), + ('maintenance_db', 'template1'), + ('username', 'new-user'), + ('service', 'myservice'), + ('tunnel_host', 'bastion.example.com'), + ): + manager = make_manager() + changed_server_data = make_server_data(**{field: value}) + self.assertTrue( + Driver._manager_is_stale(manager, changed_server_data), + "expected stale manager when %s changes" % field) + + +class TestSavedStateIsStaleMatchesUnchanged( + _PureUnitTestSetupMixin, BaseTestGenerator): + """Serialized session state whose identity fields still match the + current Server row is safe to restore onto a freshly built + manager.""" + + scenarios = [('default', dict())] + + def runTest(self): + saved = make_saved_state() + server_data = make_server_data() + + self.assertFalse(Driver._saved_state_is_stale(saved, server_data)) + + +class TestSavedStateIsStaleDetectsReusedId( + _PureUnitTestSetupMixin, BaseTestGenerator): + """Serialized state left over from a deleted Server row (e.g. after + the configuration database was reset/restored without restarting + pgAdmin) must not be restored onto the row that reused its id.""" + + scenarios = [('default', dict())] + + def runTest(self): + saved = make_saved_state(host='deleted-server.example.com') + server_data = make_server_data(host='new-server.example.com') + + self.assertTrue(Driver._saved_state_is_stale(saved, server_data)) + + +class TestSavedStateIsStaleMissingFieldsAreStale( + _PureUnitTestSetupMixin, BaseTestGenerator): + """State serialized before identity fields were added to + ServerManager.as_dict() (i.e. a dict without host/port/etc. keys) + cannot be verified, so it must be treated as stale rather than + trusted blindly.""" + + scenarios = [('default', dict())] + + def runTest(self): + saved = {'sid': 1, 'ver': '18.0', 'sversion': 180000, + 'connections': {}} + server_data = make_server_data() + + self.assertTrue(Driver._saved_state_is_stale(saved, server_data)) + + +class TestSavedStateIsStaleChecksEachIdentityField( + _PureUnitTestSetupMixin, BaseTestGenerator): + """Any one of host/port/db/user/service/tunnel_host differing is + enough to discard the serialized state.""" + + scenarios = [('default', dict())] + + def runTest(self): + server_data = make_server_data() + + for field, value in ( + ('port', 5433), + ('maintenance_db', 'template1'), + ('username', 'new-user'), + ('service', 'myservice'), + ('tunnel_host', 'bastion.example.com'), + ): + saved = make_saved_state() + changed_server_data = make_server_data(**{field: value}) + self.assertTrue( + Driver._saved_state_is_stale(saved, changed_server_data), + "expected stale saved state when %s changes" % field) + + +class TestManagerIsStaleDetectsDifferentPgAdminUser( + _PureUnitTestSetupMixin, BaseTestGenerator): + """A manager built for one pgAdmin user must not be reused for + another user who logs in on the same browser session, even when + every connection field matches (issue #6090: the same servers + re-imported by a new user after the configuration database was + reset).""" + + scenarios = [('default', dict())] + + def runTest(self): + manager = make_manager(pga_user='old-user-uniquifier') + server_data = make_server_data() + + self.assertTrue(Driver._manager_is_stale( + manager, server_data, 'new-user-uniquifier')) + self.assertFalse(Driver._manager_is_stale( + manager, server_data, 'old-user-uniquifier')) + + +class TestSavedStateIsStaleDetectsDifferentPgAdminUser( + _PureUnitTestSetupMixin, BaseTestGenerator): + """Serialized state saved by one pgAdmin user must not be restored + for another user on the same browser session, even when every + connection field matches.""" + + scenarios = [('default', dict())] + + def runTest(self): + saved = make_saved_state(pga_user='old-user-uniquifier') + server_data = make_server_data() + + self.assertTrue(Driver._saved_state_is_stale( + saved, server_data, 'new-user-uniquifier')) + self.assertFalse(Driver._saved_state_is_stale( + saved, server_data, 'old-user-uniquifier'))