Connect the server when changing the Query Tool connection after it was disconnected. #8144 (#10588)

web/pgadmin/tools/sqleditor/__init__.py

@@ -2821,6 +2821,18 @@ def connect_server(sid):
             status=410, success=0,
             errormsg=gettext("Could not find the required server.")
         )
+
+    # When the Query Tool switches to a connection made as a different user
+    # or role, connecting the server itself would prompt for the server's
+    # own user's password, which that user may not know, so leave the
+    # server as it is.
+    if not _connects_as_server_user(server):
+        return make_json_response(
+            success=1,
+            info=gettext("Server connection not required."),
+            data={}
+        )
+
     driver = get_driver(PG_DEFAULT_DRIVER)
     manager = driver.connection_manager(sid)
 
@@ -2848,6 +2860,39 @@ def connect_server(sid):
     )
 
 
+def _connects_as_server_user(server):
+    """
+    Return whether the user and role supplied with the current request (if
+    any) are the ones the server itself connects with.  A request that does
+    not supply a user is treated as connecting as the server's own user, so
+    callers that do not pass one behave as they always have.
+
+    An empty role matches, since a connection opened without a role falls
+    back to the server's configured role.
+    """
+    data = request.form
+    if not data and request.data:
+        try:
+            data = json.loads(request.data)
+        except ValueError:
+            data = {}
+    if not hasattr(data, 'get') or 'user' not in data:
+        return True
+
+    if server.shared and server.user_id != current_user.id:
+        # Import here to avoid circular dependency
+        from pgadmin.browser.server_groups.servers import ServerModule
+        shared_server = ServerModule.get_shared_server(
+            server, server.servergroup_id)
+        server = ServerModule.get_shared_server_properties(
+            server, shared_server)
+
+    user = data.get('user') or None
+    role = data.get('role') or None
+    return user == (server.username or None) and \
+        role in (None, server.role or None)
+
+
 def _cache_manager_password_from_request(manager):
     """
     Cache the password supplied with the current request (from a tool's

web/pgadmin/tools/sqleditor/static/js/components/QueryToolComponent.jsx

@@ -38,7 +38,7 @@ import { retrieveNodeName } from '../show_view_data';
 import { useModal } from '../../../../../static/js/helpers/ModalProvider';
 import usePreferences from '../../../../../preferences/static/js/store';
 import { useApplicationState } from '../../../../../settings/static/ApplicationStateProvider';
-import { connectServer, connectServerModal } from './connectServer';
+import { connectServer, connectServerModal, connectionCancelledError, ensureServerConnected } from './connectServer';
 import { FileManagerUtils  } from '../../../../../misc/file_manager/static/js/components/FileManager';
 
 export const QueryToolContext = React.createContext();
@@ -115,15 +115,6 @@ const FIXED_PREF = {
   },
 };
 
-/* Raised when the user dismisses the password prompt rather than supplying
- * credentials. The flag lets callers tell a deliberate cancellation from a
- * genuine failure without matching on the (translated) message text. */
-function connectionCancelledError() {
-  const error = new Error(gettext('Connection cancelled'));
-  error.cancelled = true;
-  return error;
-}
-
 export default function QueryToolComponent({params, pgWindow, pgAdmin, selectedNodeInfo, qtPanelDocker, qtPanelId, eventBusObj}) {
   const containerRef = React.useRef(null);
   const preferencesStore = usePreferences();
@@ -773,6 +764,16 @@ export default function QueryToolComponent({params, pgWindow, pgAdmin, selectedN
         };
       });
     };
+    /* Put the previous selection back if the change does not go ahead */
+    const revertSelection = ()=>{
+      if(!isNew) {
+        selectConn(currSelectedConn, currConnected, false);
+      } else {
+        setQtStatePartial({
+          obtaining_conn: false,
+        });
+      }
+    };
     /* If not new, select it initially to show loading */
     if(!isNew) {
       selectConn(connectionData);
@@ -782,7 +783,20 @@ export default function QueryToolComponent({params, pgWindow, pgAdmin, selectedN
       });
     }
 
-    return new Promise((resolve, reject)=>{
+    /* Connect the server itself first, so that the Object Explorer reflects
+     * the connection if the server had been disconnected. The backend only
+     * does so when this connection uses the server's own user and role. */
+    return ensureServerConnected(api, modal, connectionData.sid, (isConnecting)=>{
+      setQtStatePartial({
+        obtaining_conn: isConnecting,
+      });
+    }, {
+      user: connectionData.user,
+      role: connectionData.role,
+    }).catch((error)=>{
+      revertSelection();
+      throw error;
+    }).then(()=>new Promise((resolve, reject)=>{
       api.post(url_for('sqleditor.update_sqleditor_connection', {
         trans_id: qtState.params.trans_id,
         sgid: connectionData.sgid,
@@ -823,13 +837,7 @@ export default function QueryToolComponent({params, pgWindow, pgAdmin, selectedN
               obtaining_conn: false,
             });
             const onCancel = ()=>{
-              if(!isNew) {
-                selectConn(currSelectedConn, currConnected, false);
-              } else {
-                setQtStatePartial({
-                  obtaining_conn: false,
-                });
-              }
+              revertSelection();
               reject(connectionCancelledError());
             };
             connectServerModal(modal, error.response?.data?.result, (passwordData)=>{
@@ -844,17 +852,11 @@ export default function QueryToolComponent({params, pgWindow, pgAdmin, selectedN
               );
             }, onCancel);
           } else {
-            if(!isNew) {
-              selectConn(currSelectedConn, currConnected, false);
-            } else {
-              setQtStatePartial({
-                obtaining_conn: false,
-              });
-            }
+            revertSelection();
             reject(error instanceof Error ? error : Error(gettext('Something went wrong')));
           }
         });
-    });
+    }));
   };
 
   const onNewConnClick = useCallback(()=>{

web/pgadmin/tools/sqleditor/static/js/components/connectServer.js

@@ -10,6 +10,15 @@ import gettext from 'sources/gettext';
 import url_for from 'sources/url_for';
 import ConnectServerContent from '../../../../../static/js/Dialogs/ConnectServerContent';
 
+/* Raised when the user dismisses the password prompt rather than supplying
+ * credentials. The flag lets callers tell a deliberate cancellation from a
+ * genuine failure without matching on the (translated) message text. */
+export function connectionCancelledError() {
+  const error = new Error(gettext('Connection cancelled'));
+  error.cancelled = true;
+  return error;
+}
+
 export function connectServerModal(modal, modalData, connectCallback, cancelCallback) {
   modal.showModal(gettext('Connect to server'), (closeModal)=>{
     return (
@@ -30,22 +39,63 @@ export function connectServerModal(modal, modalData, connectCallback, cancelCall
   });
 }
 
-export async function connectServer(api, modal, sid, user, formData, connectCallback, cancelCallback, promptCallback) {
+/* The connect_server endpoint answers 428 when it needs a password, and 401
+ * (with the prompt data) when the one supplied was rejected. */
+function isCredentialChallenge(error) {
+  return [401, 428].includes(error?.response?.status);
+}
+
+/* errorCallback is optional: when given, any failure that is not a request
+ * for credentials is passed to it instead of raising the password prompt.
+ * extraData is optional: when given, it is sent with every request, including
+ * those made after a password prompt. */
+export async function connectServer(api, modal, sid, user, formData, connectCallback, cancelCallback, promptCallback, errorCallback, extraData) {
   try {
     let {data: respData} = await api({
       method: 'POST',
       url: url_for('sqleditor.connect_server', {'sid': sid}),
       headers: { 'Content-Type': 'application/x-www-form-urlencoded' },
-      data: formData
+      data: extraData ? {...formData, ...extraData} : formData
     });
     connectCallback?.(respData.data);
   } catch (error) {
+    if(errorCallback && !isCredentialChallenge(error)) {
+      errorCallback(error);
+      return;
+    }
     promptCallback?.(false);
     connectServerModal(modal, error.response?.data?.result, async (data)=>{
       promptCallback?.(true);
-      connectServer(api, modal, sid, user, data, connectCallback, cancelCallback, promptCallback);
+      connectServer(api, modal, sid, user, data, connectCallback, cancelCallback, promptCallback, errorCallback, extraData);
     }, ()=>{
       cancelCallback?.();
     });
   }
 }
+
+/* Make sure the server's own connection (the one the Object Explorer shows)
+ * is established, prompting for its password if required. The Query Tool's
+ * connection is separate, so without this a server that was disconnected
+ * stays disconnected in the Object Explorer even though the Query Tool has
+ * reconnected to it. Resolves once connected, rejects with a cancelled error
+ * if the user dismisses the prompt, and rejects with the server's error
+ * message for any other failure.
+ *
+ * connectAs is optional: when given, its user and role are those of the
+ * connection being switched to, and the server only connects if they are the
+ * server's own, so nobody is prompted for the password of a user they are
+ * not connecting as. */
+export function ensureServerConnected(api, modal, sid, promptCallback, connectAs) {
+  const extraData = connectAs ? {
+    user: connectAs.user ?? '',
+    role: connectAs.role ?? '',
+  } : undefined;
+  return new Promise((resolve, reject)=>{
+    connectServer(api, modal, sid, null, null, resolve, ()=>{
+      reject(connectionCancelledError());
+    }, promptCallback, (error)=>{
+      const errmsg = error?.response?.data?.errormsg;
+      reject(errmsg ? new Error(errmsg) : error);
+    }, extraData);
+  });
+}

web/pgadmin/tools/sqleditor/tests/test_connect_server_user.py

@@ -0,0 +1,80 @@
+##########################################################################
+#
+# pgAdmin 4 - PostgreSQL Tools
+#
+# Copyright (C) 2013 - 2026, The pgAdmin Development Team
+# This software is released under the PostgreSQL Licence
+#
+##########################################################################
+
+from unittest.mock import patch, MagicMock
+
+from pgadmin.model import Server
+from pgadmin.utils.ajax import make_json_response
+from pgadmin.utils.route import BaseTestGenerator
+from regression import parent_node_dict
+
+
+class TestConnectServerUser(BaseTestGenerator):
+    """
+    The Query Tool's connection bar asks sqleditor.connect_server to connect
+    the server only when the selected connection uses the server's own user
+    and role; for any other user or role it must leave the server alone, so
+    that nobody is prompted for a password they may not know (issue #8144).
+    """
+    scenarios = [
+        ('No user supplied connects the server, as before',
+         dict(form_data=None, expect_connect=True)),
+        ('Server user without a role connects the server',
+         dict(form_data={'user': '{server_user}', 'role': ''},
+              expect_connect=True)),
+        ('Server user with the server role connects the server',
+         dict(form_data={'user': '{server_user}', 'role': '{server_role}'},
+              expect_connect=True)),
+        ('Server user with a different role skips the server connect',
+         dict(form_data={'user': '{server_user}',
+                         'role': 'pga_test_other_role'},
+              expect_connect=False)),
+        ('A different user skips the server connect',
+         dict(form_data={'user': 'pga_test_other_user', 'role': ''},
+              expect_connect=False)),
+    ]
+
+    def setUp(self):
+        self.sid = parent_node_dict["server"][-1]["server_id"]
+        with self.app.app_context():
+            server = Server.query.filter_by(id=self.sid).first()
+            self.server_user = server.username or ''
+            self.server_role = server.role or ''
+
+    def runTest(self):
+        data = {}
+        if self.form_data is not None:
+            data = {
+                key: value.format(server_user=self.server_user,
+                                  server_role=self.server_role)
+                for key, value in self.form_data.items()
+            }
+
+        # Report the server as disconnected, and stand in for the server
+        # node's connect view, so the test can see whether a connect was
+        # attempted without needing the server's password.
+        manager = MagicMock()
+        manager.connection.return_value.connected.return_value = False
+        driver = MagicMock()
+        driver.connection_manager.return_value = manager
+        view = MagicMock()
+        view.connect.side_effect = lambda *args, **kwargs: \
+            make_json_response(success=1, info='connected', data={})
+
+        with patch('pgadmin.tools.sqleditor.get_driver',
+                   return_value=driver), \
+            patch('pgadmin.tools.sqleditor.SchemaDiffRegistry.get_node_view',
+                  return_value=view):
+            response = self.tester.post(
+                '/sqleditor/connect_server/{0}'.format(self.sid),
+                data=data,
+            )
+
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(view.connect.called, self.expect_connect)

web/regression/javascript/sqleditor/connectServer.spec.js

@@ -20,7 +20,7 @@ jest.mock('../../../pgadmin/static/js/Dialogs/ConnectServerContent', () => ({
 }));
 
 import url_for from 'sources/url_for';
-import { connectServer } from '../../../pgadmin/tools/sqleditor/static/js/components/connectServer';
+import { connectServer, ensureServerConnected } from '../../../pgadmin/tools/sqleditor/static/js/components/connectServer';
 
 /* Captures the props of each password prompt raised, so that a test can drive
  * the prompt by calling onOK or closeModal on it. */
@@ -117,3 +117,140 @@ describe('connectServer', ()=>{
     expect(()=>modal.prompts[0].closeModal()).not.toThrow();
   });
 });
+
+describe('ensureServerConnected', ()=>{
+  it('connects the server itself and resolves without prompting', async ()=>{
+    const api = jest.fn().mockResolvedValue({data: {data: {connected: true}}});
+    const modal = createFakeModal();
+    url_for.mockClear();
+
+    await expect(ensureServerConnected(api, modal, 1)).resolves.toEqual({connected: true});
+
+    /* Changing the Query Tool connection must connect the server, otherwise a
+     * disconnected server stays disconnected in the Object Explorer whilst the
+     * Query Tool reconnects (issue #8144). */
+    expect(url_for).toHaveBeenCalledWith('sqleditor.connect_server', {'sid': 1});
+    expect(modal.prompts).toHaveLength(0);
+  });
+
+  it('resolves once the password prompt is answered', async ()=>{
+    const api = jest.fn()
+      .mockRejectedValueOnce({response: {status: 428, data: {result: 'need password'}}})
+      .mockResolvedValueOnce({data: {data: {connected: true}}});
+    const modal = createFakeModal();
+    const promptCallback = jest.fn();
+
+    const result = ensureServerConnected(api, modal, 1, promptCallback);
+    await new Promise(process.nextTick);
+    expect(modal.prompts).toHaveLength(1);
+    expect(promptCallback).toHaveBeenLastCalledWith(false);
+
+    modal.prompts[0].onOK(passwordFormData);
+
+    await expect(result).resolves.toEqual({connected: true});
+    expect(promptCallback).toHaveBeenLastCalledWith(true);
+    expect(api).toHaveBeenLastCalledWith(expect.objectContaining({data: {password: 'secret'}}));
+  });
+
+  it('rejects with a cancelled error when the prompt is dismissed', async ()=>{
+    const api = jest.fn().mockRejectedValue({response: {status: 428, data: {result: 'need password'}}});
+    const modal = createFakeModal();
+
+    const result = ensureServerConnected(api, modal, 1);
+    await new Promise(process.nextTick);
+    modal.prompts[0].closeModal();
+
+    await expect(result).rejects.toMatchObject({cancelled: true});
+  });
+
+  it('prompts again when the supplied password is rejected', async ()=>{
+    const api = jest.fn()
+      .mockRejectedValueOnce({response: {status: 428, data: {result: 'need password'}}})
+      .mockRejectedValueOnce({response: {status: 401, data: {result: 'wrong password'}}})
+      .mockResolvedValueOnce({data: {data: {connected: true}}});
+    const modal = createFakeModal();
+
+    const result = ensureServerConnected(api, modal, 1);
+    await new Promise(process.nextTick);
+    modal.prompts[0].onOK(passwordFormData);
+    await new Promise(process.nextTick);
+
+    expect(modal.prompts).toHaveLength(2);
+    expect(modal.prompts[1].data).toBe('wrong password');
+    modal.prompts[1].onOK(passwordFormData);
+
+    await expect(result).resolves.toEqual({connected: true});
+  });
+
+  it('rejects with the server error, without prompting, for other failures', async ()=>{
+    const api = jest.fn().mockRejectedValue(
+      {response: {status: 410, data: {errormsg: 'Could not find the required server.'}}});
+    const modal = createFakeModal();
+
+    const error = await ensureServerConnected(api, modal, 1).catch((e)=>e);
+
+    expect(modal.prompts).toHaveLength(0);
+    expect(error).toBeInstanceOf(Error);
+    expect(error.message).toBe('Could not find the required server.');
+    expect(error.cancelled).toBeUndefined();
+  });
+
+  it('sends no user or role unless asked to', async ()=>{
+    const api = jest.fn().mockResolvedValue({data: {data: {}}});
+
+    await ensureServerConnected(api, createFakeModal(), 1);
+
+    expect(api).toHaveBeenCalledWith(expect.objectContaining({data: null}));
+  });
+
+  it('sends the user and role of the connection being switched to', async ()=>{
+    const api = jest.fn().mockResolvedValue({data: {data: {}}});
+
+    await ensureServerConnected(api, createFakeModal(), 1, undefined,
+      {user: 'jane', role: 'reporting'});
+
+    /* The backend only connects the server for its own user and role, so a
+     * connection made as anyone else is not held up by a prompt for the
+     * server's own user's password (issue #8144). */
+    expect(api).toHaveBeenCalledWith(expect.objectContaining({
+      data: {user: 'jane', role: 'reporting'},
+    }));
+  });
+
+  it('sends an empty role when the connection has none', async ()=>{
+    const api = jest.fn().mockResolvedValue({data: {data: {}}});
+
+    await ensureServerConnected(api, createFakeModal(), 1, undefined,
+      {user: 'postgres', role: undefined});
+
+    expect(api).toHaveBeenCalledWith(expect.objectContaining({
+      data: {user: 'postgres', role: ''},
+    }));
+  });
+
+  it('keeps sending the user and role after a password prompt', async ()=>{
+    const api = jest.fn()
+      .mockRejectedValueOnce({response: {status: 428, data: {result: 'need password'}}})
+      .mockResolvedValueOnce({data: {data: {connected: true}}});
+    const modal = createFakeModal();
+
+    const result = ensureServerConnected(api, modal, 1, undefined,
+      {user: 'postgres', role: ''});
+    await new Promise(process.nextTick);
+    modal.prompts[0].onOK(passwordFormData);
+
+    await expect(result).resolves.toEqual({connected: true});
+    expect(api).toHaveBeenLastCalledWith(expect.objectContaining({
+      data: {password: 'secret', user: 'postgres', role: ''},
+    }));
+  });
+
+  it('passes on a failure that has no server response unchanged', async ()=>{
+    const networkError = new Error('Network Error');
+    const api = jest.fn().mockRejectedValue(networkError);
+    const modal = createFakeModal();
+
+    await expect(ensureServerConnected(api, modal, 1)).rejects.toBe(networkError);
+    expect(modal.prompts).toHaveLength(0);
+  });
+});