From 66223ca50001e522dbd2919c7ba478fc297419ca Mon Sep 17 00:00:00 2001 From: Dave Page Date: Wed, 10 Jun 2026 15:40:55 +0100 Subject: [PATCH 1/2] Fix Query Tool password re-prompt loop for unsaved passwords. (#9091) When a tool (Query Tool, View/Edit Data, etc.) opens a connection for a server whose password was not saved, it relies on the password cached on the server manager. If that cached password is unavailable, the tool prompts for it. The entered password was POSTed to the connect_server endpoint, which short-circuited with "Server connected" whenever the server's primary connection was already established -- silently discarding the entered password. The tool's own connection therefore still had no password and re-prompted immediately, producing an infinite prompt loop in which the re-entered password appeared to be rejected. Cache the entered password on the server manager (encrypted) in that short-circuit path so the tool's connection can reuse it. The password overwrites any cached value, so a regenerated short-lived cloud auth token (AWS RDS IAM / Azure Entra) takes effect immediately. --- docs/en_US/release_notes_9_16.rst | 1 + web/pgadmin/tools/sqleditor/__init__.py | 45 ++++++++ .../tests/test_cache_manager_password.py | 100 ++++++++++++++++++ 3 files changed, 146 insertions(+) create mode 100644 web/pgadmin/tools/sqleditor/utils/tests/test_cache_manager_password.py diff --git a/docs/en_US/release_notes_9_16.rst b/docs/en_US/release_notes_9_16.rst index 4f0740cdff0..33ed6690dec 100644 --- a/docs/en_US/release_notes_9_16.rst +++ b/docs/en_US/release_notes_9_16.rst @@ -40,6 +40,7 @@ Bug fixes ********* | `Issue #6308 `_ - Fix the infinite loading spinner after an idle database connection is silently dropped, by detecting stale connections and offering a reconnect dialog. + | `Issue #9091 `_ - Fix the Query Tool re-prompting for an unsaved password in a loop and rejecting the re-entered password, by caching the entered password on the server manager when the primary connection is already established. | `Issue #9595 `_ - Fix missing ALTER ... SET DEFAULT statements for inherited columns in the generated table SQL/EDIT script. | `Issue #9677 `_ - Fix the Unlogged table toggle in table properties not generating any ALTER TABLE ... SET LOGGED/UNLOGGED statement. | `Issue #9828 `_ - Fix tool calls failing against OpenAI-compatible providers that emit empty/null name, arguments, or id fields in streaming continuation deltas. diff --git a/web/pgadmin/tools/sqleditor/__init__.py b/web/pgadmin/tools/sqleditor/__init__.py index c9e26df2f00..082affd105d 100644 --- a/web/pgadmin/tools/sqleditor/__init__.py +++ b/web/pgadmin/tools/sqleditor/__init__.py @@ -46,6 +46,8 @@ from pgadmin.utils.ajax import make_json_response, bad_request, \ success_return, internal_server_error, service_unavailable, gone from pgadmin.utils.driver import get_driver +from pgadmin.utils.crypto import encrypt +from pgadmin.utils.master_password import get_crypt_key from pgadmin.utils.exception import ConnectionLost, SSHTunnelConnectionLost, \ CryptKeyMissing, ObjectGone from pgadmin.browser.utils import underscore_escape @@ -2681,6 +2683,16 @@ def connect_server(sid): conn = manager.connection() if conn.connected(): + # The server's primary connection is already established. However, + # individual tools (Query Tool, View/Edit Data, etc.) open their own + # connections and, when the password is not saved, rely on the + # password cached on the server manager. If that cached password is + # missing (e.g. it was never persisted, or the tab was restored from + # a workspace) the tool prompts for the password. Make sure the + # password the user just entered at that prompt is cached here so the + # tool's connection can use it, instead of being discarded and + # re-prompted in a loop. + _cache_manager_password_from_request(manager) return make_json_response( success=1, info=gettext("Server connected."), @@ -2693,6 +2705,39 @@ def connect_server(sid): ) +def _cache_manager_password_from_request(manager): + """ + Cache the password supplied with the current request (from a tool's + password prompt) onto the server manager, so that connections opened by + tools such as the Query Tool can reuse it without prompting again. + + This is a no-op when no password is supplied or when the encryption key + is unavailable. When a password is supplied it overwrites any cached + password, so a freshly entered credential (e.g. a regenerated, short-lived + cloud auth token) takes effect immediately. + """ + if request.form: + data = request.form + elif request.data: + data = json.loads(request.data) + else: + return + + password = data.get('password', None) + if not password: + return + + crypt_key_present, crypt_key = get_crypt_key() + if not crypt_key_present: + return + + try: + manager._update_password(encrypt(password, crypt_key)) + manager.update_session() + except Exception as e: + current_app.logger.exception(e) + + @blueprint.route( '/filter_dialog/', methods=["PUT"], endpoint='set_filter_data' diff --git a/web/pgadmin/tools/sqleditor/utils/tests/test_cache_manager_password.py b/web/pgadmin/tools/sqleditor/utils/tests/test_cache_manager_password.py new file mode 100644 index 00000000000..01443ced69c --- /dev/null +++ b/web/pgadmin/tools/sqleditor/utils/tests/test_cache_manager_password.py @@ -0,0 +1,100 @@ +########################################################################## +# +# pgAdmin 4 - PostgreSQL Tools +# +# Copyright (C) 2013 - 2026, The pgAdmin Development Team +# This software is released under the PostgreSQL Licence +# +########################################################################## + +from pgadmin.utils.route import BaseTestGenerator +from pgadmin.utils.crypto import decrypt +from unittest.mock import patch, MagicMock + +import pgadmin.tools.sqleditor as sqleditor + +CRYPT_KEY = 'test-crypt-key' +LONG_TOKEN = ( + 'abc.rds.amazonaws.com:5432/?Action=connect&' + 'X-Amz-Algorithm=AWS4-HMAC-SHA256&' + 'X-Amz-Credential=ABC%2F20250820%2Fus-east-1&' + 'X-Amz-Signature=deadbeef+slashes%2F%2F' +) * 8 + + +class CacheManagerPasswordTest(BaseTestGenerator): + """ + Regression test for issue #9091. + + When a tool (Query Tool, View/Edit Data, etc.) prompts for a password + that was not saved, the entered password is POSTed to the connect_server + endpoint. If the server's primary connection is already connected the + endpoint short-circuits; _cache_manager_password_from_request makes sure + the entered password is still cached on the manager (encrypted), so the + tool's connection can reuse it instead of being re-prompted in a loop. + """ + + scenarios = [ + ('When a password is supplied it is encrypted and cached', dict( + form_data={'password': LONG_TOKEN}, + crypt_key_present=True, + expect_cached=True, + )), + ('When an existing cached password is overwritten', dict( + form_data={'password': LONG_TOKEN}, + crypt_key_present=True, + existing_password=b'stale-encrypted-token', + expect_cached=True, + )), + ('When no password is supplied it is a no-op', dict( + form_data={}, + crypt_key_present=True, + expect_cached=False, + )), + ('When an empty password is supplied it is a no-op', dict( + form_data={'password': ''}, + crypt_key_present=True, + expect_cached=False, + )), + ('When the crypt key is missing it is a no-op', dict( + form_data={'password': LONG_TOKEN}, + crypt_key_present=False, + expect_cached=False, + )), + ] + + def runTest(self): + manager = MagicMock() + manager.password = getattr(self, 'existing_password', None) + + def _update_password(passwd): + manager.password = passwd + + manager._update_password.side_effect = _update_password + + mock_request = MagicMock() + mock_request.form = self.form_data + mock_request.data = None + + crypt_key = CRYPT_KEY if self.crypt_key_present else None + + with patch.object(sqleditor, 'request', mock_request), \ + patch.object(sqleditor, 'get_crypt_key', + return_value=(self.crypt_key_present, crypt_key)): + sqleditor._cache_manager_password_from_request(manager) + + if self.expect_cached: + self.assertTrue(manager._update_password.called) + self.assertTrue(manager.update_session.called) + # The cached value must be the encrypted form of the supplied + # password and must decrypt back to the original token intact. + cached = manager.password + self.assertIsNotNone(cached) + self.assertNotEqual(cached, self.form_data['password']) + decrypted = decrypt(cached, CRYPT_KEY) + if isinstance(decrypted, bytes): + decrypted = decrypted.decode() + self.assertEqual(decrypted, self.form_data['password']) + else: + self.assertFalse(manager._update_password.called) + self.assertFalse(manager.update_session.called) From 02881424763c26f0616217b610f7de04ba8995f5 Mon Sep 17 00:00:00 2001 From: Dave Page Date: Wed, 10 Jun 2026 16:02:57 +0100 Subject: [PATCH 2/2] Make _cache_manager_password_from_request fully best-effort. Move request-body JSON parsing inside the try/except so malformed request data is treated as a silent no-op (logged and swallowed) rather than propagating a JSONDecodeError and turning the connect_server endpoint's 'Server connected' response into a 500. Add a regression test scenario for the malformed-JSON case. Addresses CodeRabbit review feedback on #10073. --- web/pgadmin/tools/sqleditor/__init__.py | 30 +++++++++++-------- .../tests/test_cache_manager_password.py | 9 +++++- 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/web/pgadmin/tools/sqleditor/__init__.py b/web/pgadmin/tools/sqleditor/__init__.py index 082affd105d..d09d479cc21 100644 --- a/web/pgadmin/tools/sqleditor/__init__.py +++ b/web/pgadmin/tools/sqleditor/__init__.py @@ -2715,23 +2715,27 @@ def _cache_manager_password_from_request(manager): is unavailable. When a password is supplied it overwrites any cached password, so a freshly entered credential (e.g. a regenerated, short-lived cloud auth token) takes effect immediately. + + This is best-effort: any failure (including malformed request data) is + logged and swallowed so it never turns the caller's "Server connected" + response into a 500 error. """ - if request.form: - data = request.form - elif request.data: - data = json.loads(request.data) - else: - return + try: + if request.form: + data = request.form + elif request.data: + data = json.loads(request.data) + else: + return - password = data.get('password', None) - if not password: - return + password = data.get('password', None) + if not password: + return - crypt_key_present, crypt_key = get_crypt_key() - if not crypt_key_present: - return + crypt_key_present, crypt_key = get_crypt_key() + if not crypt_key_present: + return - try: manager._update_password(encrypt(password, crypt_key)) manager.update_session() except Exception as e: diff --git a/web/pgadmin/tools/sqleditor/utils/tests/test_cache_manager_password.py b/web/pgadmin/tools/sqleditor/utils/tests/test_cache_manager_password.py index 01443ced69c..7491883079f 100644 --- a/web/pgadmin/tools/sqleditor/utils/tests/test_cache_manager_password.py +++ b/web/pgadmin/tools/sqleditor/utils/tests/test_cache_manager_password.py @@ -61,6 +61,12 @@ class CacheManagerPasswordTest(BaseTestGenerator): crypt_key_present=False, expect_cached=False, )), + ('When the request body is malformed JSON it is a silent no-op', dict( + form_data={}, + request_data=b'{not-valid-json', + crypt_key_present=True, + expect_cached=False, + )), ] def runTest(self): @@ -74,11 +80,12 @@ def _update_password(passwd): mock_request = MagicMock() mock_request.form = self.form_data - mock_request.data = None + mock_request.data = getattr(self, 'request_data', None) crypt_key = CRYPT_KEY if self.crypt_key_present else None with patch.object(sqleditor, 'request', mock_request), \ + patch.object(sqleditor, 'current_app', MagicMock()), \ patch.object(sqleditor, 'get_crypt_key', return_value=(self.crypt_key_present, crypt_key)): sqleditor._cache_manager_password_from_request(manager)