From bbb5cc250bcdab53a497024322016a33dc256aa3 Mon Sep 17 00:00:00 2001 From: Dave Page Date: Thu, 15 Oct 2026 14:50:25 +0100 Subject: [PATCH] Connect the server when changing the Query Tool connection after it was disconnected. #8144 (#10588) --- web/pgadmin/tools/sqleditor/__init__.py | 45 ++++++ .../js/components/QueryToolComponent.jsx | 54 +++---- .../static/js/components/connectServer.js | 56 ++++++- .../tests/test_connect_server_user.py | 80 ++++++++++ .../sqleditor/connectServer.spec.js | 139 +++++++++++++++++- 5 files changed, 344 insertions(+), 30 deletions(-) create mode 100644 web/pgadmin/tools/sqleditor/tests/test_connect_server_user.py diff --git a/web/pgadmin/tools/sqleditor/__init__.py b/web/pgadmin/tools/sqleditor/__init__.py index f90cdf647..5768393f9 100644 --- a/web/pgadmin/tools/sqleditor/__init__.py +++ b/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 diff --git a/web/pgadmin/tools/sqleditor/static/js/components/QueryToolComponent.jsx b/web/pgadmin/tools/sqleditor/static/js/components/QueryToolComponent.jsx index 460ab539c..f89b5f9e2 100644 --- a/web/pgadmin/tools/sqleditor/static/js/components/QueryToolComponent.jsx +++ b/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(()=>{ diff --git a/web/pgadmin/tools/sqleditor/static/js/components/connectServer.js b/web/pgadmin/tools/sqleditor/static/js/components/connectServer.js index 3d8054ad8..f7053f0ba 100644 --- a/web/pgadmin/tools/sqleditor/static/js/components/connectServer.js +++ b/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); + }); +} diff --git a/web/pgadmin/tools/sqleditor/tests/test_connect_server_user.py b/web/pgadmin/tools/sqleditor/tests/test_connect_server_user.py new file mode 100644 index 000000000..903711ced --- /dev/null +++ b/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) diff --git a/web/regression/javascript/sqleditor/connectServer.spec.js b/web/regression/javascript/sqleditor/connectServer.spec.js index f06b77278..9e3eb4b3b 100644 --- a/web/regression/javascript/sqleditor/connectServer.spec.js +++ b/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); + }); +});