Tell server-mode password exec commands whether the server was shared, and by whom. #10703

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,26 @@ 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 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. On a shared server,
+    ``PGADMIN_PASSEXEC_HOST``, ``PGADMIN_PASSEXEC_PORT`` and
+    ``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.
 
 .. image:: images/server_post_connection_sql.png

web/config.py

@@ -1110,6 +1110,30 @@
 # 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.
+#
+# 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. 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.
 #

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',

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
@@ -303,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
@@ -314,13 +313,19 @@ 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. 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

web/pgadmin/utils/passexec.py

@@ -272,11 +272,16 @@ 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
         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 '',
@@ -285,6 +290,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 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 '',
+            '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 +366,38 @@ 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 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:
@@ -372,7 +420,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)

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)

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):