From 8591374b045165f3a504f77d24696bffac3856b4 Mon Sep 17 00:00:00 2001 From: Dave Page Date: Thu, 5 Nov 2026 11:40:02 +0000 Subject: [PATCH 1/4] Tell server-mode password exec commands whether the server was shared, and by whom. #10703 A password exec command in server mode only received connection details that whoever edits the server can set, so a user could create their own server with the same host and username as a shared one, pick the same command, and be given the shared server's password. Commands now also receive PGADMIN_PASSEXEC_SHARED, PGADMIN_PASSEXEC_SERVER_ID, PGADMIN_PASSEXEC_SERVER_OWNER, PGADMIN_PASSEXEC_SERVER_OWNER_AUTH_SOURCE and PGADMIN_PASSEXEC_SERVER_GROUP. They are read from the server's own row and its owner, rather than from the overlay built for a user who does not own a shared server, so that user cannot change them. The server group is the owner's, which is the one an OAuth2 server group claim is checked against. The documentation and the config.py comment say which variables a command can rely on, and why the group name means nothing without the owner. Closes #10703 --- docs/en_US/server_dialog.rst | 39 +++++- web/config.py | 21 +++ .../servers/static/js/server.ui.js | 2 +- web/pgadmin/utils/passexec.py | 42 +++++- .../utils/tests/test_passexec_resolve.py | 20 +++ .../utils/tests/test_passexec_server.py | 4 +- .../tests/test_passexec_shared_details.py | 128 ++++++++++++++++++ 7 files changed, 250 insertions(+), 6 deletions(-) create mode 100644 web/pgadmin/utils/tests/test_passexec_shared_details.py diff --git a/docs/en_US/server_dialog.rst b/docs/en_US/server_dialog.rst index 57f146f8876..893956795fc 100644 --- a/docs/en_US/server_dialog.rst +++ b/docs/en_US/server_dialog.rst @@ -299,8 +299,25 @@ Use the fields in the *Advanced* tab to configure a connection: non-owner's own username on a shared server), ``PGADMIN_PASSEXEC_DATABASE`` (the maintenance database), ``PGADMIN_PASSEXEC_PGADMIN_USER`` (the pgAdmin user making the connection) and ``PGADMIN_PASSEXEC_AUTH_SOURCE`` (how that - user authenticated to pgAdmin). The ``stdout`` of the command is used as the - SQL password. + user authenticated to pgAdmin). The following variables describe the server + definition itself: + + * ``PGADMIN_PASSEXEC_SHARED`` is ``true`` if the server is shared and owned + by another pgAdmin user, and ``false`` otherwise, including when the user + making the connection owns a shared server. + * ``PGADMIN_PASSEXEC_SERVER_ID`` is the numeric id pgAdmin gave the server. + * ``PGADMIN_PASSEXEC_SERVER_OWNER`` and + ``PGADMIN_PASSEXEC_SERVER_OWNER_AUTH_SOURCE`` are the username of the + pgAdmin user who owns the server, and how that user authenticates. + * ``PGADMIN_PASSEXEC_SERVER_GROUP`` is the name of the owner's server group + that holds the server. A user who does not own a shared server can change + the group it appears in for them, but this variable always gives the + owner's group, which is the one checked against + ``OAUTH2_SERVER_GROUP_CLAIM`` (see :ref:`oauth2_server_group_claims`); for + an OAuth2 user restricted by that claim, it is therefore the group that + granted access to the server. + + The ``stdout`` of the command is used as the SQL password. * Use the *Password exec expiration* field to specify a maximum age, in seconds, of the password generated with a *Password exec command*. In server mode, this field is enabled only when a named command is selected. If not specified, @@ -346,6 +363,24 @@ Use the fields in the *Advanced* tab to configure a connection: decide whether to issue a password based on the pgAdmin identity, and treat the other four variables as untrusted input. + The ``PGADMIN_PASSEXEC_SHARED``, ``PGADMIN_PASSEXEC_SERVER_ID``, + ``PGADMIN_PASSEXEC_SERVER_OWNER``, + ``PGADMIN_PASSEXEC_SERVER_OWNER_AUTH_SOURCE`` and + ``PGADMIN_PASSEXEC_SERVER_GROUP`` variables cannot be changed by a user + who does not own the server. When ``PGADMIN_PASSEXEC_SHARED`` is + ``true``, the server can only have been shared by an administrator, and + only its owner can change its host, port and maintenance database, so a + command that has checked that the owner is one it trusts can rely on + ``PGADMIN_PASSEXEC_HOST``, ``PGADMIN_PASSEXEC_PORT`` and + ``PGADMIN_PASSEXEC_DATABASE``, and use them, the server id or the server + group to choose the password. ``PGADMIN_PASSEXEC_USERNAME`` is still set + by each user. Every user owns their own server groups and may give them + any name, including the name of an administrator's group, so the server + group proves nothing on its own: check ``PGADMIN_PASSEXEC_SHARED`` and + the owner as well. When ``PGADMIN_PASSEXEC_SHARED`` is ``false`` and the + owner is the user making the connection, every value was set by that + user. + Click the *Post Connection SQL* tab to continue. .. image:: images/server_post_connection_sql.png diff --git a/web/config.py b/web/config.py index 7f4345adaef..326ca9506bc 100644 --- a/web/config.py +++ b/web/config.py @@ -1110,6 +1110,27 @@ # last two identify the pgAdmin user; the host, port, username and database # are set by whoever edits the server, so commands must authorise on the # pgAdmin identity and treat the other four as untrusted input. +# +# These variables describe the server itself, and cannot be changed by a +# user who does not own it: +# +# PGADMIN_PASSEXEC_SHARED: 'true' if the server is shared and owned by +# another pgAdmin user, otherwise 'false' (including for the owner). +# PGADMIN_PASSEXEC_SERVER_ID: pgAdmin's numeric id for the server. +# PGADMIN_PASSEXEC_SERVER_OWNER and +# PGADMIN_PASSEXEC_SERVER_OWNER_AUTH_SOURCE: the pgAdmin user who owns +# the server, and how they authenticate. +# PGADMIN_PASSEXEC_SERVER_GROUP: the name of the owner's server group +# holding the server; for an OAuth2 user restricted by +# OAUTH2_SERVER_GROUP_CLAIM, this is the group that granted access. +# +# When PGADMIN_PASSEXEC_SHARED is 'true', only an administrator can have +# shared the server, and only its owner can set its host and port, so a +# command can rely on the host and port once it has checked that the owner +# is one it trusts. Every user owns their own server groups and can give +# them any name, so the group name proves nothing unless the owner is +# checked too. PGADMIN_PASSEXEC_USERNAME remains untrusted on a shared +# server, because each user sets their own. # Names may not start with "__". An empty dictionary (the default) turns the # feature off. This setting is applicable only for server mode. # diff --git a/web/pgadmin/browser/server_groups/servers/static/js/server.ui.js b/web/pgadmin/browser/server_groups/servers/static/js/server.ui.js index 13ad88058e6..f72f6134c36 100644 --- a/web/pgadmin/browser/server_groups/servers/static/js/server.ui.js +++ b/web/pgadmin/browser/server_groups/servers/static/js/server.ui.js @@ -635,7 +635,7 @@ export default class ServerSchema extends BaseUISchema { }); return {type: 'select', options: options}; }, - helpMessage: gettext('The command is chosen from those configured by the administrator. The server hostname, port, username and maintenance database, and your pgAdmin username and authentication source, are passed to it in the PGADMIN_PASSEXEC_* environment variables.') + helpMessage: gettext('The command is chosen from those configured by the administrator. The server hostname, port, username and maintenance database, your pgAdmin username and authentication source, and the server id, owner, server group and whether it is shared with you, are passed to it in the PGADMIN_PASSEXEC_* environment variables.') }, { id: 'passexec_expiration', label: gettext('Password exec expiration (seconds)'), type: 'int', diff --git a/web/pgadmin/utils/passexec.py b/web/pgadmin/utils/passexec.py index 2993b6cab43..dc1eeb02906 100644 --- a/web/pgadmin/utils/passexec.py +++ b/web/pgadmin/utils/passexec.py @@ -272,7 +272,9 @@ class ServerPasswordExec(PasswordExec): def __init__(self, name, argv, host, port, username, database, pgadmin_user, auth_source, expiration_seconds=None, - timeout=60): + timeout=60, *, shared=False, server_id=None, + server_owner=None, server_owner_auth_source=None, + server_group=None): super().__init__(None, host, port, username, expiration_seconds, timeout) self.name = name @@ -285,6 +287,17 @@ def __init__(self, name, argv, host, port, username, database, 'PGADMIN_PASSEXEC_DATABASE': database or '', 'PGADMIN_PASSEXEC_PGADMIN_USER': pgadmin_user or '', 'PGADMIN_PASSEXEC_AUTH_SOURCE': auth_source or '', + # The values below come from the server's own row and its + # owner, which a user who does not own the server cannot + # change; see the SERVER_PASSEXEC_COMMANDS comment in + # config.py for which of them a command can rely on. + 'PGADMIN_PASSEXEC_SHARED': 'true' if shared else 'false', + 'PGADMIN_PASSEXEC_SERVER_ID': + str(server_id) if server_id is not None else '', + 'PGADMIN_PASSEXEC_SERVER_OWNER': server_owner or '', + 'PGADMIN_PASSEXEC_SERVER_OWNER_AUTH_SOURCE': + server_owner_auth_source or '', + 'PGADMIN_PASSEXEC_SERVER_GROUP': server_group or '', }) def get(self): @@ -350,6 +363,27 @@ def resolve_server_passexec(server, user): return server.passexec_name, server.passexec_expiration, server.username +def server_passexec_details(server): + """Return (owner username, owner auth source, server group name) for + the server, read from its own row in the configuration database. + + The server passed in may be the overlay built for a user who does not + own a shared server, in which the server group is that user's own + (see ServerModule.get_shared_server_properties), so it is looked up + again by id: the owner's group is the one an OAuth2 server group + claim is checked against, and only the owner can rename it or move + the server out of it. Any value that cannot be found is None.""" + from pgadmin.model import db, Server, ServerGroup, User + row = db.session.query( + User.username, User.auth_source, ServerGroup.name + ).select_from(Server).outerjoin( + User, User.id == Server.user_id + ).outerjoin( + ServerGroup, ServerGroup.id == Server.servergroup_id + ).filter(Server.id == server.id).first() + return tuple(row) if row is not None else (None, None, None) + + def build_passexec(server): """Build the password exec object for a ServerManager, or None.""" if not config.SERVER_MODE: @@ -372,7 +406,11 @@ def build_passexec(server): 'Server %s uses password exec command %r, which is not in ' 'SERVER_PASSEXEC_COMMANDS; ignoring it.', server.id, name) return None + owner, owner_auth_source, group = server_passexec_details(server) return ServerPasswordExec( name, argv, server.host, server.port, db_user, server.maintenance_db, current_user.username, - getattr(current_user, 'auth_source', None), expiration) + getattr(current_user, 'auth_source', None), expiration, + shared=bool(server.shared) and server.user_id != current_user.id, + server_id=server.id, server_owner=owner, + server_owner_auth_source=owner_auth_source, server_group=group) diff --git a/web/pgadmin/utils/tests/test_passexec_resolve.py b/web/pgadmin/utils/tests/test_passexec_resolve.py index 1758a0cfd38..211810d6a04 100644 --- a/web/pgadmin/utils/tests/test_passexec_resolve.py +++ b/web/pgadmin/utils/tests/test_passexec_resolve.py @@ -85,6 +85,8 @@ class TestBuildPassexec(BaseTestGenerator): scenarios = [ ('Builds a ServerPasswordExec for a non-owner', dict(test_method='test_builds')), + ('The owner of a shared server is not flagged as shared', + dict(test_method='test_owner_not_shared')), ('Unknown name returns None and warns', dict(test_method='test_unknown_name')), ('Server mode never uses free text', @@ -102,7 +104,10 @@ def runTest(self): def _build(self, server, user=OTHER, row=None, server_mode=True, cmds=CMDS): + details = ('owner@example.com', 'internal', 'Production') with self.app.test_request_context(), \ + patch('pgadmin.utils.passexec.server_passexec_details', + return_value=details), \ patch('pgadmin.utils.passexec.config.SERVER_MODE', server_mode), \ patch('pgadmin.utils.passexec.config.' @@ -124,6 +129,21 @@ def test_builds(self): self.assertEqual(res.env['PGADMIN_PASSEXEC_AUTH_SOURCE'], 'ldap') self.assertEqual(res.argv, ['/bin/get-pass']) self.assertEqual(res.name, 'vault') + self.assertEqual(res.env['PGADMIN_PASSEXEC_SHARED'], 'true') + self.assertEqual(res.env['PGADMIN_PASSEXEC_SERVER_ID'], '7') + self.assertEqual(res.env['PGADMIN_PASSEXEC_SERVER_OWNER'], + 'owner@example.com') + self.assertEqual( + res.env['PGADMIN_PASSEXEC_SERVER_OWNER_AUTH_SOURCE'], + 'internal') + self.assertEqual(res.env['PGADMIN_PASSEXEC_SERVER_GROUP'], + 'Production') + + def test_owner_not_shared(self): + # The owner of a shared server is not told it is shared with them. + res = self._build(_server(), user=OWNER) + self.assertEqual(res.env['PGADMIN_PASSEXEC_SHARED'], 'false') + self.assertEqual(res.env['PGADMIN_PASSEXEC_USERNAME'], 'owner_db') def test_unknown_name(self): server = _server(passexec_name='gone', user_id=2) diff --git a/web/pgadmin/utils/tests/test_passexec_server.py b/web/pgadmin/utils/tests/test_passexec_server.py index 1152cb55edd..d881fe6fac6 100644 --- a/web/pgadmin/utils/tests/test_passexec_server.py +++ b/web/pgadmin/utils/tests/test_passexec_server.py @@ -150,8 +150,10 @@ def runTest(self): pexec.get() env = mock_run.call_args[1]['env'] for k in ('HOST', 'PORT', 'USERNAME', 'DATABASE', 'PGADMIN_USER', - 'AUTH_SOURCE'): + 'AUTH_SOURCE', 'SERVER_ID', 'SERVER_OWNER', + 'SERVER_OWNER_AUTH_SOURCE', 'SERVER_GROUP'): self.assertEqual(env['PGADMIN_PASSEXEC_' + k], '') + self.assertEqual(env['PGADMIN_PASSEXEC_SHARED'], 'false') class TestFreeTextRefusedInServerMode(BaseTestGenerator): diff --git a/web/pgadmin/utils/tests/test_passexec_shared_details.py b/web/pgadmin/utils/tests/test_passexec_shared_details.py new file mode 100644 index 00000000000..58ab08c780e --- /dev/null +++ b/web/pgadmin/utils/tests/test_passexec_shared_details.py @@ -0,0 +1,128 @@ +########################################################################## +# +# pgAdmin 4 - PostgreSQL Tools +# +# Copyright (C) 2013 - 2026, The pgAdmin Development Team +# This software is released under the PostgreSQL License +# +########################################################################## + +"""The server details passed to a server-mode password exec command must +come from the server's own row, so that a user cannot copy a shared +server's connection details into a server of their own, or file the +shared server in a group of their own, and be mistaken for a user the +server was shared with.""" + +import uuid +from unittest.mock import MagicMock, patch + +from pgadmin.model import db, Server, ServerGroup, SharedServer, User +from pgadmin.utils.constants import INTERNAL, OAUTH2 +from pgadmin.utils.driver.psycopg3 import Driver +from pgadmin.utils.passexec import build_passexec +from pgadmin.utils.route import BaseTestGenerator + + +class TestPassexecSharedServerDetails(BaseTestGenerator): + """An administrator shares a server in their 'Production' group. Another + user files it under their own 'Mine' group, and also creates a server + of their own with the same host and username, in a group of their own + that is also called 'Production'.""" + + def setUp(self): + self.created = [] + tag = uuid.uuid4().hex[:8] + with self.app.app_context(): + admin = self._add(User( + username='pe-admin-{0}@example.com'.format(tag), + email=None, active=True, auth_source=INTERNAL, + fs_uniquifier=uuid.uuid4().hex)) + user = self._add(User( + username='pe-user-{0}@example.com'.format(tag), + email=None, active=True, auth_source=OAUTH2, + fs_uniquifier=uuid.uuid4().hex)) + admin_group = self._add( + ServerGroup(user_id=admin.id, name='Production')) + user_group = self._add( + ServerGroup(user_id=user.id, name='Mine')) + lookalike_group = self._add( + ServerGroup(user_id=user.id, name='Production')) + shared = self._add(Server( + user_id=admin.id, servergroup_id=admin_group.id, + name='shared', host='db.example.com', port=5432, + maintenance_db='postgres', username='app', + save_password=0, shared=True, passexec_name='vault')) + self._add(SharedServer( + osid=shared.id, user_id=user.id, + server_owner=admin.username, + servergroup_id=user_group.id, name='shared', + host='db.example.com', port=5432, + maintenance_db='postgres', username='app', + save_password=0, shared=True, passexec_name=None)) + own = self._add(Server( + user_id=user.id, servergroup_id=lookalike_group.id, + name='lookalike', host='db.example.com', port=5432, + maintenance_db='postgres', username='app', + save_password=0, shared=False, passexec_name='vault')) + + self.admin = (admin.id, admin.username) + self.user = MagicMock(id=user.id, username=user.username, + auth_source=OAUTH2, + fs_uniquifier=user.fs_uniquifier, + is_authenticated=True) + self.shared_id = shared.id + self.own_id = own.id + + def _add(self, obj): + db.session.add(obj) + db.session.commit() + self.created.append(obj) + return obj + + def _env(self, sid): + with self.app.test_request_context('/'), \ + patch('pgadmin.utils.passexec.config.SERVER_MODE', True), \ + patch('pgadmin.utils.passexec.config.' + 'SERVER_PASSEXEC_COMMANDS', + {'vault': ['/bin/get-pass']}, create=True), \ + patch('pgadmin.utils.passexec.current_user', self.user), \ + patch('pgadmin.utils.driver.psycopg3.current_user', + self.user): + # Build from what the connection manager would be built from: + # for the shared server, the user's overlay. + source = Driver._manager_source(Server.query.get(sid)) + return build_passexec(source).env + + def runTest(self): + env = self._env(self.shared_id) + self.assertEqual(env['PGADMIN_PASSEXEC_SHARED'], 'true') + self.assertEqual(env['PGADMIN_PASSEXEC_SERVER_ID'], + str(self.shared_id)) + self.assertEqual(env['PGADMIN_PASSEXEC_SERVER_OWNER'], + self.admin[1]) + self.assertEqual(env['PGADMIN_PASSEXEC_SERVER_OWNER_AUTH_SOURCE'], + INTERNAL) + # The owner's group, not the one the user filed it under. + self.assertEqual(env['PGADMIN_PASSEXEC_SERVER_GROUP'], 'Production') + self.assertEqual(env['PGADMIN_PASSEXEC_PGADMIN_USER'], + self.user.username) + + # The user's own copy has the same connection details and group + # name, but is identified as theirs. + own = self._env(self.own_id) + for k in ('HOST', 'PORT', 'USERNAME', 'DATABASE', 'SERVER_GROUP'): + self.assertEqual(own['PGADMIN_PASSEXEC_' + k], + env['PGADMIN_PASSEXEC_' + k]) + self.assertEqual(own['PGADMIN_PASSEXEC_SHARED'], 'false') + self.assertEqual(own['PGADMIN_PASSEXEC_SERVER_ID'], str(self.own_id)) + self.assertEqual(own['PGADMIN_PASSEXEC_SERVER_OWNER'], + self.user.username) + self.assertEqual(own['PGADMIN_PASSEXEC_SERVER_OWNER_AUTH_SOURCE'], + OAUTH2) + + def tearDown(self): + with self.app.app_context(): + for obj in reversed(self.created): + obj = db.session.merge(obj) + db.session.delete(obj) + db.session.commit() From a30aa0baec36423e04acb5d3ec84bd079cb01e9a Mon Sep 17 00:00:00 2001 From: Dave Page Date: Thu, 5 Nov 2026 13:09:38 +0000 Subject: [PATCH 2/4] Rebuild the password exec command when the owner's server group changes, and stop implying that a shared server's owner is always an administrator. #10703 A cached connection manager kept its password exec command, and so the server group name and any password chosen for it, until sharing changed. It is now also rebuilt when the owner or server group it reports no longer matches the server's row. Servers shared before sharing was restricted to administrators may be owned by any user, so the documentation no longer says otherwise. --- docs/en_US/server_dialog.rst | 9 ++++-- web/config.py | 16 ++++++----- web/pgadmin/utils/driver/psycopg3/__init__.py | 14 ++++++---- web/pgadmin/utils/passexec.py | 14 ++++++++++ .../tests/test_passexec_shared_details.py | 28 +++++++++++++++++-- 5 files changed, 64 insertions(+), 17 deletions(-) diff --git a/docs/en_US/server_dialog.rst b/docs/en_US/server_dialog.rst index 893956795fc..a05b5673b4b 100644 --- a/docs/en_US/server_dialog.rst +++ b/docs/en_US/server_dialog.rst @@ -368,9 +368,12 @@ Use the fields in the *Advanced* tab to configure a connection: ``PGADMIN_PASSEXEC_SERVER_OWNER_AUTH_SOURCE`` and ``PGADMIN_PASSEXEC_SERVER_GROUP`` variables cannot be changed by a user who does not own the server. When ``PGADMIN_PASSEXEC_SHARED`` is - ``true``, the server can only have been shared by an administrator, and - only its owner can change its host, port and maintenance database, so a - command that has checked that the owner is one it trusts can rely on + ``true``, the server is shared and owned by another user. Only an + administrator can share a server, but a server shared before that + restriction was introduced may be owned by any user, so this flag is not + proof that the owner is an administrator. Only the owner can change the + host, port and maintenance database, so a command that has checked that + the owner is one it trusts can rely on ``PGADMIN_PASSEXEC_HOST``, ``PGADMIN_PASSEXEC_PORT`` and ``PGADMIN_PASSEXEC_DATABASE``, and use them, the server id or the server group to choose the password. ``PGADMIN_PASSEXEC_USERNAME`` is still set diff --git a/web/config.py b/web/config.py index 326ca9506bc..4047d5acb02 100644 --- a/web/config.py +++ b/web/config.py @@ -1124,13 +1124,15 @@ # holding the server; for an OAuth2 user restricted by # OAUTH2_SERVER_GROUP_CLAIM, this is the group that granted access. # -# When PGADMIN_PASSEXEC_SHARED is 'true', only an administrator can have -# shared the server, and only its owner can set its host and port, so a -# command can rely on the host and port once it has checked that the owner -# is one it trusts. Every user owns their own server groups and can give -# them any name, so the group name proves nothing unless the owner is -# checked too. PGADMIN_PASSEXEC_USERNAME remains untrusted on a shared -# server, because each user sets their own. +# PGADMIN_PASSEXEC_SHARED does not prove that the owner is an +# administrator, because a server shared before sharing was restricted to +# administrators may be owned by any user. Only the owner can set the host, +# port and maintenance database, so a command can rely on them once it has +# checked that the owner is one it trusts. Every user owns their own server +# groups and can give them any name, so the group name proves nothing unless +# the owner is checked too. PGADMIN_PASSEXEC_USERNAME remains untrusted on a +# shared server, because each user sets their own. +# # Names may not start with "__". An empty dictionary (the default) turns the # feature off. This setting is applicable only for server mode. # diff --git a/web/pgadmin/utils/driver/psycopg3/__init__.py b/web/pgadmin/utils/driver/psycopg3/__init__.py index aae84d657c2..2fb5da23c24 100644 --- a/web/pgadmin/utils/driver/psycopg3/__init__.py +++ b/web/pgadmin/utils/driver/psycopg3/__init__.py @@ -32,7 +32,8 @@ from pgadmin.utils import isolate_client_credential_files from pgadmin.utils.locker import ConnectionLocker, \ REQUIRE_AUTH_MIN_LIBPQ, apply_kerberos_conn_params, libpq_version -from pgadmin.utils.passexec import build_passexec +from pgadmin.utils.passexec import build_passexec, \ + passexec_details_changed from .keywords import scan_keyword from ..abstract import BaseDriver from .connection import Connection @@ -314,10 +315,13 @@ def connection_manager(self, sid=None): # sharing/ownership changed via the same reused-id # path could keep serving the previous owner's # passexec to a new, non-owning user. The password - # exec command is only rebuilt when sharing - # changes, so its cached password survives - # ordinary requests. - if manager.shared != server_data.shared: + # exec command is only rebuilt when sharing, or + # the owner or server group it reports, changes, + # so its cached password survives ordinary + # requests. + if manager.shared != server_data.shared or \ + passexec_details_changed( + manager.passexec, manager_source): manager.shared = server_data.shared manager.passexec = build_passexec( manager_source) diff --git a/web/pgadmin/utils/passexec.py b/web/pgadmin/utils/passexec.py index dc1eeb02906..90efb97d81b 100644 --- a/web/pgadmin/utils/passexec.py +++ b/web/pgadmin/utils/passexec.py @@ -279,6 +279,9 @@ def __init__(self, name, argv, host, port, username, database, timeout) self.name = name self.argv = list(argv) + # Compared by passexec_details_changed() to spot a renamed group. + self.server_details = (server_owner, server_owner_auth_source, + server_group) self.env = server_passexec_base_env() self.env.update({ 'PGADMIN_PASSEXEC_HOST': host or '', @@ -384,6 +387,17 @@ def server_passexec_details(server): return tuple(row) if row is not None else (None, None, None) +def passexec_details_changed(pexec, server): + """True if pexec is a server-mode command built with an owner or + server group that no longer matches the server's row, for example + because the owner renamed the group or moved the server to another. + The connection manager then rebuilds it, which also drops a password + the command may have chosen for the old group.""" + if not isinstance(pexec, ServerPasswordExec): + return False + return pexec.server_details != server_passexec_details(server) + + def build_passexec(server): """Build the password exec object for a ServerManager, or None.""" if not config.SERVER_MODE: diff --git a/web/pgadmin/utils/tests/test_passexec_shared_details.py b/web/pgadmin/utils/tests/test_passexec_shared_details.py index 58ab08c780e..f3d52339c37 100644 --- a/web/pgadmin/utils/tests/test_passexec_shared_details.py +++ b/web/pgadmin/utils/tests/test_passexec_shared_details.py @@ -19,7 +19,8 @@ from pgadmin.model import db, Server, ServerGroup, SharedServer, User from pgadmin.utils.constants import INTERNAL, OAUTH2 from pgadmin.utils.driver.psycopg3 import Driver -from pgadmin.utils.passexec import build_passexec +from pgadmin.utils.passexec import build_passexec, \ + passexec_details_changed from pgadmin.utils.route import BaseTestGenerator @@ -72,6 +73,7 @@ def setUp(self): is_authenticated=True) self.shared_id = shared.id self.own_id = own.id + self.admin_group_id = admin_group.id def _add(self, obj): db.session.add(obj) @@ -80,6 +82,9 @@ def _add(self, obj): return obj def _env(self, sid): + return self._pexec(sid).env + + def _pexec(self, sid): with self.app.test_request_context('/'), \ patch('pgadmin.utils.passexec.config.SERVER_MODE', True), \ patch('pgadmin.utils.passexec.config.' @@ -91,7 +96,14 @@ def _env(self, sid): # Build from what the connection manager would be built from: # for the shared server, the user's overlay. source = Driver._manager_source(Server.query.get(sid)) - return build_passexec(source).env + return build_passexec(source) + + def _changed(self, pexec, sid): + with self.app.test_request_context('/'), \ + patch('pgadmin.utils.driver.psycopg3.current_user', + self.user): + source = Driver._manager_source(Server.query.get(sid)) + return passexec_details_changed(pexec, source) def runTest(self): env = self._env(self.shared_id) @@ -120,6 +132,18 @@ def runTest(self): self.assertEqual(own['PGADMIN_PASSEXEC_SERVER_OWNER_AUTH_SOURCE'], OAUTH2) + # A cached command is rebuilt once the owner renames the group, + # so it neither reports the old name nor keeps a password chosen + # for it. + pexec = self._pexec(self.shared_id) + self.assertFalse(self._changed(pexec, self.shared_id)) + self.assertFalse(self._changed(None, self.shared_id)) + with self.app.app_context(): + group = db.session.get(ServerGroup, self.admin_group_id) + group.name = 'Renamed' + db.session.commit() + self.assertTrue(self._changed(pexec, self.shared_id)) + def tearDown(self): with self.app.app_context(): for obj in reversed(self.created): From 7924676bd3bcfaabb1de862dc62d14a7625afa3a Mon Sep 17 00:00:00 2001 From: Dave Page Date: Thu, 5 Nov 2026 13:55:31 +0000 Subject: [PATCH 3/4] Rebuild the password exec command before restoring a cached manager's connections. #10703 A reconnect during the restore can run the command, so it must already carry the current sharing state, owner and server group. --- web/pgadmin/utils/driver/psycopg3/__init__.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/web/pgadmin/utils/driver/psycopg3/__init__.py b/web/pgadmin/utils/driver/psycopg3/__init__.py index 2fb5da23c24..4910c091193 100644 --- a/web/pgadmin/utils/driver/psycopg3/__init__.py +++ b/web/pgadmin/utils/driver/psycopg3/__init__.py @@ -304,8 +304,6 @@ def connection_manager(self, sid=None): manager.update(manager_source) 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 @@ -318,13 +316,16 @@ def connection_manager(self, sid=None): # exec command is only rebuilt when sharing, or # the owner or server group it reports, changes, # so its cached password survives ordinary - # requests. + # requests. This runs before the connections are + # restored, since a reconnect may run the command. if manager.shared != server_data.shared or \ passexec_details_changed( manager.passexec, manager_source): manager.shared = server_data.shared manager.passexec = build_passexec( manager_source) + manager._restore_connections() + manager.update_session() managers['pinged'] = datetime.datetime.now() # The session is active again, so gc_timeout() should run the From 0ba5b6b189afb4dee09b55b1a806c11252d3fe95 Mon Sep 17 00:00:00 2001 From: Dave Page Date: Fri, 6 Nov 2026 09:36:06 +0000 Subject: [PATCH 4/4] Clarify where the password exec connection details come from. --- docs/en_US/server_dialog.rst | 21 ++++++++++----------- web/config.py | 13 +++++++------ web/pgadmin/utils/passexec.py | 2 +- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/docs/en_US/server_dialog.rst b/docs/en_US/server_dialog.rst index a05b5673b4b..851a0c38bf7 100644 --- a/docs/en_US/server_dialog.rst +++ b/docs/en_US/server_dialog.rst @@ -371,18 +371,17 @@ Use the fields in the *Advanced* tab to configure a connection: ``true``, the server is shared and owned by another user. Only an administrator can share a server, but a server shared before that restriction was introduced may be owned by any user, so this flag is not - proof that the owner is an administrator. Only the owner can change the - host, port and maintenance database, so a command that has checked that - the owner is one it trusts can rely on + proof that the owner is an administrator. On a shared server, ``PGADMIN_PASSEXEC_HOST``, ``PGADMIN_PASSEXEC_PORT`` and - ``PGADMIN_PASSEXEC_DATABASE``, and use them, the server id or the server - group to choose the password. ``PGADMIN_PASSEXEC_USERNAME`` is still set - by each user. Every user owns their own server groups and may give them - any name, including the name of an administrator's group, so the server - group proves nothing on its own: check ``PGADMIN_PASSEXEC_SHARED`` and - the owner as well. When ``PGADMIN_PASSEXEC_SHARED`` is ``false`` and the - owner is the user making the connection, every value was set by that - user. + ``PGADMIN_PASSEXEC_DATABASE`` are taken from the owner's server, not from + the copy kept for each user, whilst ``PGADMIN_PASSEXEC_USERNAME`` is each + user's own. Every user owns their own server groups and may give them any + name, including the name of an administrator's group, so the server group + proves nothing on its own: check ``PGADMIN_PASSEXEC_SHARED`` and the owner + as well. Commands should decide whether to issue a password based on the + pgAdmin user and the server owner. When ``PGADMIN_PASSEXEC_SHARED`` is + ``false`` and the owner is the user making the connection, every value + was set by that user. Click the *Post Connection SQL* tab to continue. diff --git a/web/config.py b/web/config.py index 4047d5acb02..95516b3ab4b 100644 --- a/web/config.py +++ b/web/config.py @@ -1126,12 +1126,13 @@ # # PGADMIN_PASSEXEC_SHARED does not prove that the owner is an # administrator, because a server shared before sharing was restricted to -# administrators may be owned by any user. Only the owner can set the host, -# port and maintenance database, so a command can rely on them once it has -# checked that the owner is one it trusts. Every user owns their own server -# groups and can give them any name, so the group name proves nothing unless -# the owner is checked too. PGADMIN_PASSEXEC_USERNAME remains untrusted on a -# shared server, because each user sets their own. +# administrators may be owned by any user. On a shared server, the host, +# port and maintenance database are taken from the owner's server, not from +# the copy kept for each user, whilst PGADMIN_PASSEXEC_USERNAME is each +# user's own. Every user owns their own server groups and can give them any +# name, so the group name proves nothing unless the owner is checked too. +# Commands should decide whether to issue a password based on the pgAdmin +# user and the server owner. # # Names may not start with "__". An empty dictionary (the default) turns the # feature off. This setting is applicable only for server mode. diff --git a/web/pgadmin/utils/passexec.py b/web/pgadmin/utils/passexec.py index 90efb97d81b..ea216b0a1f9 100644 --- a/web/pgadmin/utils/passexec.py +++ b/web/pgadmin/utils/passexec.py @@ -293,7 +293,7 @@ def __init__(self, name, argv, host, port, username, database, # The values below come from the server's own row and its # owner, which a user who does not own the server cannot # change; see the SERVER_PASSEXEC_COMMANDS comment in - # config.py for which of them a command can rely on. + # config.py for what each of them means. 'PGADMIN_PASSEXEC_SHARED': 'true' if shared else 'false', 'PGADMIN_PASSEXEC_SERVER_ID': str(server_id) if server_id is not None else '',