diff --git a/web/pgadmin/utils/driver/psycopg3/__init__.py b/web/pgadmin/utils/driver/psycopg3/__init__.py index 60d7f4c2d54..0264c5ae08f 100644 --- a/web/pgadmin/utils/driver/psycopg3/__init__.py +++ b/web/pgadmin/utils/driver/psycopg3/__init__.py @@ -91,13 +91,68 @@ 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): + # 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. Drop it and + # let the manager start clean. + manager.update_session() + else: + manager._restore(saved) + manager.update_session() return managers return {} + @staticmethod + def _saved_state_is_stale(saved, server_data): + """ + 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. + """ + return ( + 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): + """ + 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. + """ + return ( + 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(...) @@ -144,8 +199,32 @@ def connection_manager(self, sid=None): if str(sid) in managers: manager = managers[str(sid)] with connection_restore_lock: - manager._restore_connections() - manager.update_session() + if self._manager_is_stale(manager, server_data): + # 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. + manager.release() + manager.update(server_data) + 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: diff --git a/web/pgadmin/utils/driver/psycopg3/server_manager.py b/web/pgadmin/utils/driver/psycopg3/server_manager.py index 00738355ee4..9a9320cdbd1 100644 --- a/web/pgadmin/utils/driver/psycopg3/server_manager.py +++ b/web/pgadmin/utils/driver/psycopg3/server_manager.py @@ -154,6 +154,18 @@ 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 + 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..a78f00908bd --- /dev/null +++ b/web/pgadmin/utils/driver/psycopg3/tests/test_manager_is_stale.py @@ -0,0 +1,184 @@ +########################################################################## +# +# 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)