From a88bba9532f458d08fbe841fc3c6b40843043999 Mon Sep 17 00:00:00 2001 From: Kundan Sable Date: Wed, 22 Jul 2026 16:00:00 +0530 Subject: [PATCH 1/5] fix: don't hard-fail on an undecryptable saved password MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Connection._decode_password() let UnicodeDecodeError from decrypt() propagate as a hard error. For OIDC/OAuth2 users whose crypt key can differ between sessions, a password saved under a previous key can never be decoded, and every connection attempt surfaced "Failed to decrypt the saved password" — even after deleting and recreating the pgAdmin user, since the corrupted ciphertext is what's stored, not anything tied to the user record. Catch the decode failure, log a warning, and treat it as "no saved password" instead of propagating the error. connect() then falls through to the normal password prompt, so the account is recoverable instead of permanently stuck. Fixes #10140 --- .../utils/driver/psycopg3/connection.py | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/web/pgadmin/utils/driver/psycopg3/connection.py b/web/pgadmin/utils/driver/psycopg3/connection.py index d07a16cefcd..ced8f627fc6 100644 --- a/web/pgadmin/utils/driver/psycopg3/connection.py +++ b/web/pgadmin/utils/driver/psycopg3/connection.py @@ -258,12 +258,20 @@ def _decode_password(self, encpass, manager, password, crypt_key): if isinstance(password, bytes): password = password.decode() except Exception as e: - manager.stop_ssh_tunnel() - current_app.logger.exception(e) - return True, \ - _( - "Failed to decrypt the saved password.\nError: {0}" - ).format(str(e)), password + # The saved password could not be decrypted. This happens + # when the stored ciphertext was encrypted with a different + # key (e.g. OIDC/OAuth2 logins where the derived encryption + # key changed between sessions), leaving un-decodable bytes + # (typically a "'utf-8' codec can't decode byte 0x.." error). + # Instead of failing every connection attempt permanently, + # discard the bad saved password and continue so the user is + # prompted for the password again. + current_app.logger.warning( + 'Ignoring the saved password as it could not be ' + 'decrypted. The user will be prompted for the password. ' + 'Error: {0}'.format(str(e)) + ) + return False, '', None return False, '', password def connect(self, **kwargs): From 64638cc0c5b038da9bf251e08911abfd4003c9a2 Mon Sep 17 00:00:00 2001 From: Kundan Sable Date: Fri, 24 Jul 2026 19:44:41 +0530 Subject: [PATCH 2/5] fix: clear cached ciphertext when a saved password fails to decrypt Per CodeRabbit review on PR #10174: _decode_password() discarded the undecryptable password for the current decode call, but the stale ciphertext was already cached on self.password (set earlier in connect(), before decoding) and never cleared. That meant: - Every subsequent connect() attempt re-decoded and re-warned on the same bad ciphertext instead of the "no saved password" state sticking. - If a server also had passexec_cmd configured, the passexec fallback in connect() (which only runs when neither password nor encpass is set) was skipped, since the stale encpass was still truthy. Clear self.password and manager.password alongside discarding the password in _decode_password's except branch, so the bad ciphertext doesn't linger. The SSH-tunnel prompt path in get_response_for_password() trusting `not server.save_password` (which can be true even for an undecryptable saved password) is a separate, pre-existing issue and is intentionally left out of this narrower fix. --- web/pgadmin/utils/driver/psycopg3/connection.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/web/pgadmin/utils/driver/psycopg3/connection.py b/web/pgadmin/utils/driver/psycopg3/connection.py index ced8f627fc6..466f0f7243a 100644 --- a/web/pgadmin/utils/driver/psycopg3/connection.py +++ b/web/pgadmin/utils/driver/psycopg3/connection.py @@ -271,6 +271,13 @@ def _decode_password(self, encpass, manager, password, crypt_key): 'decrypted. The user will be prompted for the password. ' 'Error: {0}'.format(str(e)) ) + # Clear the cached ciphertext so it isn't silently reused + # on the next connect() attempt, which would just hit + # this same decode failure again, and (since a stale but + # still-truthy encpass would otherwise remain cached) + # skip the passexec fallback below. + self.password = None + manager.password = None return False, '', None return False, '', password From d4a0c45b8a9b1ee7adb5678d9a1c96fa4fa6e441 Mon Sep 17 00:00:00 2001 From: Kundan Sable Date: Thu, 1 Oct 2026 15:20:10 +0530 Subject: [PATCH 3/5] Address review feedback: narrow decrypt handling, discard stale ciphertext, cover sibling decrypt sites. - Catch only decryption-related errors (UnicodeDecodeError, binascii.Error, ValueError) and keep the traceback and server id in the warning. - Have _decode_password() report that the ciphertext was discarded so that connect() drops its copy (restores the passexec fallback and avoids re-caching the bad value). - Handle undecryptable passwords in export_password_env(), reset(), query cancel and the SSH tunnel password. - Prompt for the server/tunnel password when a saved one was discarded, and explain why in the error message. - Add a unit test for _decode_password(). Co-Authored-By: Claude Sonnet 5.5 --- .../browser/server_groups/servers/__init__.py | 30 +++++++- web/pgadmin/utils/crypto.py | 9 +++ .../utils/driver/psycopg3/connection.py | 73 +++++++++++++++---- .../utils/driver/psycopg3/server_manager.py | 32 ++++++-- .../psycopg3/tests/test_decode_password.py | 60 +++++++++++++++ 5 files changed, 179 insertions(+), 25 deletions(-) create mode 100644 web/pgadmin/utils/driver/psycopg3/tests/test_decode_password.py diff --git a/web/pgadmin/browser/server_groups/servers/__init__.py b/web/pgadmin/browser/server_groups/servers/__init__.py index 5eef9970a50..16eb5667829 100644 --- a/web/pgadmin/browser/server_groups/servers/__init__.py +++ b/web/pgadmin/browser/server_groups/servers/__init__.py @@ -1738,7 +1738,11 @@ def connect(self, gid, sid, is_qt=False, server=None): ) except Exception as e: return self.get_response_for_password( - server, 401, not server.save_password, prompt_tunnel_password, + server, 401, + not server.save_password or self._saved_password_discarded( + conn), + prompt_tunnel_password or + self._saved_tunnel_password_discarded(conn), getattr(e, 'message', str(e))) if not status: @@ -1750,8 +1754,12 @@ def connect(self, gid, sid, is_qt=False, server=None): return internal_server_error(errmsg) return self.get_response_for_password( - server, 401, not server.save_password, - prompt_tunnel_password, errmsg) + server, 401, + not server.save_password or self._saved_password_discarded( + conn), + prompt_tunnel_password or + self._saved_tunnel_password_discarded(conn), + errmsg) else: if save_password and config.ALLOW_SAVE_PASSWORD: try: @@ -2184,6 +2192,22 @@ def check_pgpass(self, gid, sid): ) return internal_server_error(errormsg=str(e)) + @staticmethod + def _saved_password_discarded(conn): + """ + Whether the saved server password was undecryptable and dropped, in + which case the user must be prompted even if a password is "saved". + """ + return bool(getattr(conn, 'saved_password_discarded', False)) + + @staticmethod + def _saved_tunnel_password_discarded(conn): + """ + Whether the saved SSH tunnel password was undecryptable and dropped. + """ + manager = getattr(conn, 'manager', None) + return bool(getattr(manager, 'saved_tunnel_password_discarded', False)) + def get_response_for_password(self, server, status, prompt_password=False, prompt_tunnel_password=False, errmsg=None): diff --git a/web/pgadmin/utils/crypto.py b/web/pgadmin/utils/crypto.py index f49d2581b91..d9b5a044968 100644 --- a/web/pgadmin/utils/crypto.py +++ b/web/pgadmin/utils/crypto.py @@ -10,6 +10,7 @@ """This File Provides Cryptography.""" import base64 +import binascii import hashlib import os @@ -44,6 +45,14 @@ def encrypt(plaintext, key): encryptor.finalize()) +# Exceptions raised by decrypt() (or by decoding its result) when the +# ciphertext is malformed or was encrypted with a different key. Anything else +# (e.g. a TypeError from a missing key) is a programming error and should not +# be swallowed. UnicodeDecodeError and binascii.Error are ValueError +# subclasses, they are listed for clarity. +DECRYPT_ERRORS = (UnicodeDecodeError, binascii.Error, ValueError) + + def decrypt(ciphertext, key): """ Decrypt the AES encrypted string. diff --git a/web/pgadmin/utils/driver/psycopg3/connection.py b/web/pgadmin/utils/driver/psycopg3/connection.py index 466f0f7243a..982ce93f15a 100644 --- a/web/pgadmin/utils/driver/psycopg3/connection.py +++ b/web/pgadmin/utils/driver/psycopg3/connection.py @@ -22,7 +22,7 @@ from flask import g, current_app from flask_babel import gettext from flask_security import current_user -from pgadmin.utils.crypto import decrypt +from pgadmin.utils.crypto import decrypt, DECRYPT_ERRORS from psycopg._encodings import py_codecs as encodings import config @@ -186,6 +186,8 @@ def __init__(self, manager, conn_id, db, **kwargs): self.wasConnected = False # This flag indicates the connection reconnecting status. self.reconnecting = False + # Set when the saved password could not be decrypted and was dropped + self.saved_password_discarded = False self.use_binary_placeholder = use_binary_placeholder self.array_to_string = array_to_string self.qtLiteral = get_driver(config.PG_DEFAULT_DRIVER).qtLiteral @@ -245,19 +247,26 @@ def _check_user_password(self, kwargs): return password, encpass, is_update_password def _decode_password(self, encpass, manager, password, crypt_key): + """ + Decrypt the saved password. + + Returns a tuple of (is_error, errmsg, password, discarded). 'discarded' + is True when the saved password could not be decrypted and was thrown + away; the caller must then drop its copy of the ciphertext too. + """ if encpass: # Fetch Logged in User Details. user = User.query.filter_by(id=current_user.id).first() if user is None: - return True, self.UNAUTHORIZED_REQUEST, password + return True, self.UNAUTHORIZED_REQUEST, password, False try: password = decrypt(encpass, crypt_key) # password is in bytes, for python3 we need it in string if isinstance(password, bytes): password = password.decode() - except Exception as e: + except DECRYPT_ERRORS as e: # The saved password could not be decrypted. This happens # when the stored ciphertext was encrypted with a different # key (e.g. OIDC/OAuth2 logins where the derived encryption @@ -267,19 +276,23 @@ def _decode_password(self, encpass, manager, password, crypt_key): # discard the bad saved password and continue so the user is # prompted for the password again. current_app.logger.warning( - 'Ignoring the saved password as it could not be ' - 'decrypted. The user will be prompted for the password. ' - 'Error: {0}'.format(str(e)) + 'Ignoring the saved password of the server (#{0}) as it ' + 'could not be decrypted. The user will be prompted for ' + 'the password. Error: {1}'.format( + getattr(manager, 'sid', None), str(e)), + exc_info=True ) - # Clear the cached ciphertext so it isn't silently reused - # on the next connect() attempt, which would just hit - # this same decode failure again, and (since a stale but - # still-truthy encpass would otherwise remain cached) - # skip the passexec fallback below. + # Clear the cached ciphertext held by this connection and the + # manager. This only affects callers that reconnect without + # supplying a password (e.g. restoring connections); the main + # connect route reloads server.password from the + # configuration database through manager.update(), so the + # user is still prompted there. self.password = None manager.password = None - return False, '', None - return False, '', password + self.saved_password_discarded = True + return False, '', None, True + return False, '', password, False def connect(self, **kwargs): if self.conn: @@ -322,11 +335,17 @@ def connect(self, **kwargs): if not crypt_key_present: raise CryptKeyMissing() - is_error, errmsg, password = self._decode_password( + self.saved_password_discarded = False + is_error, errmsg, password, discarded = self._decode_password( encpass, manager, password, crypt_key) if is_error: return False, errmsg + if discarded: + # Don't let the stale ciphertext skip the passexec fallback, or + # be written back to the manager after a successful connection. + encpass = None + # If no password credential is found then connect request might come # from Query tool, ViewData grid, debugger, etc. In that case, fall # back to using the password returned from manager.passexec. @@ -414,6 +433,10 @@ async def connectdbserver(): msg=msg ) ) + if discarded: + msg = gettext( + "The saved password could not be decrypted; please enter " + "it again.\n{0}").format(msg) return False, msg # Overwrite connection notice attr to support @@ -1476,7 +1499,16 @@ def _decrypt_password(self, manager): if not crypt_key_present: return False, crypt_key, password - password = decrypt(password, crypt_key).decode() + try: + password = decrypt(password, crypt_key).decode() + except DECRYPT_ERRORS as e: + current_app.logger.warning( + 'Ignoring the saved password of the server (#{0}) as it ' + 'could not be decrypted. Error: {1}'.format( + getattr(manager, 'sid', None), str(e)), + exc_info=True + ) + password = None return True, '', password def reset(self): @@ -1714,7 +1746,16 @@ def cancel_transaction(self, conn_id, did=None): crypt_key_present, crypt_key = get_crypt_key() if not crypt_key_present: return False, crypt_key - password = decrypt(password, crypt_key).decode() + try: + password = decrypt(password, crypt_key).decode() + except DECRYPT_ERRORS as e: + current_app.logger.warning( + 'Ignoring the saved password of the server (#{0}) ' + 'as it could not be decrypted. Error: {1}'.format( + self.manager.sid, str(e)), + exc_info=True + ) + password = None try: with ConnectionLocker(self.manager.kerberos_conn): diff --git a/web/pgadmin/utils/driver/psycopg3/server_manager.py b/web/pgadmin/utils/driver/psycopg3/server_manager.py index 00738355ee4..d6f3c5e4c76 100644 --- a/web/pgadmin/utils/driver/psycopg3/server_manager.py +++ b/web/pgadmin/utils/driver/psycopg3/server_manager.py @@ -20,7 +20,7 @@ from werkzeug.exceptions import InternalServerError from pgadmin.utils import get_complete_file_path -from pgadmin.utils.crypto import decrypt +from pgadmin.utils.crypto import decrypt, DECRYPT_ERRORS from pgadmin.utils.master_password import process_masterpass_disabled from .connection import Connection from pgadmin.model import Server, User @@ -67,6 +67,7 @@ def update(self, server): self.server_cls = None self.password = None self.tunnel_password = None + self.saved_tunnel_password_discarded = False self.sid = server.id self.host = server.host @@ -545,7 +546,17 @@ def export_password_env(self, env): crypt_key_present, crypt_key = get_crypt_key() if not crypt_key_present: return False, crypt_key - password = decrypt(self.password, crypt_key).decode() + try: + password = decrypt(self.password, crypt_key).decode() + except DECRYPT_ERRORS as e: + # Skip the password, the utility will prompt/fail on its own. + current_app.logger.warning( + 'Ignoring the saved password of the server (#{0}) as it ' + 'could not be decrypted. Error: {1}'.format( + self.sid, str(e)), + exc_info=True + ) + return os.environ[str(env)] = password elif self.passexec: password = self.passexec.get() @@ -573,10 +584,19 @@ def create_ssh_tunnel(self, tunnel_password): # password is in bytes, for python3 we need it in string if isinstance(tunnel_password, bytes): tunnel_password = tunnel_password.decode() - except Exception as e: - current_app.logger.exception(e) - return False, gettext("Failed to decrypt the SSH tunnel " - "password.\nError: {0}").format(str(e)) + except DECRYPT_ERRORS as e: + # Same treatment as the server password: discard the + # undecryptable password and carry on, so that the user is + # prompted for it again when the tunnel fails to authenticate. + current_app.logger.warning( + 'Ignoring the saved SSH tunnel password of the server ' + '(#{0}) as it could not be decrypted. Error: {1}'.format( + self.sid, str(e)), + exc_info=True + ) + self.tunnel_password = None + self.saved_tunnel_password_discarded = True + tunnel_password = None try: # If authentication method is 1 then it uses identity file diff --git a/web/pgadmin/utils/driver/psycopg3/tests/test_decode_password.py b/web/pgadmin/utils/driver/psycopg3/tests/test_decode_password.py new file mode 100644 index 00000000000..348fcf05135 --- /dev/null +++ b/web/pgadmin/utils/driver/psycopg3/tests/test_decode_password.py @@ -0,0 +1,60 @@ +########################################################################## +# +# pgAdmin 4 - PostgreSQL Tools +# +# Copyright (C) 2013 - 2026, The pgAdmin Development Team +# This software is released under the PostgreSQL Licence +# +########################################################################## + +from unittest.mock import MagicMock, patch + +from pgadmin.utils.driver.psycopg3.connection import Connection +from pgadmin.utils.route import BaseTestGenerator + +MODULE = 'pgadmin.utils.driver.psycopg3.connection' + + +class TestDecodePassword(BaseTestGenerator): + """Saved password decryption failures must be non-fatal.""" + + scenarios = [ + ('Undecryptable ciphertext is discarded', dict( + side_effect=UnicodeDecodeError('utf-8', b'\xff', 0, 1, 'bad'), + discarded=True)), + ('Valid ciphertext is decrypted', dict( + return_value=b'secret', discarded=False, expected='secret')), + ('Unexpected errors still propagate', dict( + side_effect=TypeError('no key'), raises=True)), + ] + + def runTest(self): + conn = Connection.__new__(Connection) + conn.password = 'enc' + conn.saved_password_discarded = False + manager = MagicMock(sid=1, password='enc') + + with patch(MODULE + '.User') as user, \ + patch(MODULE + '.current_user'), \ + patch(MODULE + '.current_app'), \ + patch(MODULE + '.decrypt') as dec: + user.query.filter_by.return_value.first.return_value = object() + if hasattr(self, 'side_effect'): + dec.side_effect = self.side_effect + else: + dec.return_value = self.return_value + + if getattr(self, 'raises', False): + with self.assertRaises(TypeError): + conn._decode_password('enc', manager, None, 'key') + return + + result = conn._decode_password('enc', manager, None, 'key') + + if self.discarded: + self.assertEqual(result, (False, '', None, True)) + self.assertIsNone(conn.password) + self.assertIsNone(manager.password) + self.assertTrue(conn.saved_password_discarded) + else: + self.assertEqual(result, (False, '', self.expected, False)) From 843666cdfee2597230e4563e5c0901a92bf332f6 Mon Sep 17 00:00:00 2001 From: Kundan Sable Date: Thu, 1 Oct 2026 15:42:25 +0530 Subject: [PATCH 4/5] Let reset() continue when the saved password is undecryptable; fix the new unit test's mocks. Co-Authored-By: Claude Sonnet 5.5 --- web/pgadmin/utils/driver/psycopg3/connection.py | 4 +++- .../driver/psycopg3/tests/test_decode_password.py | 12 ++++++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/web/pgadmin/utils/driver/psycopg3/connection.py b/web/pgadmin/utils/driver/psycopg3/connection.py index 982ce93f15a..f1a3ca7ce06 100644 --- a/web/pgadmin/utils/driver/psycopg3/connection.py +++ b/web/pgadmin/utils/driver/psycopg3/connection.py @@ -1508,7 +1508,9 @@ def _decrypt_password(self, manager): getattr(manager, 'sid', None), str(e)), exc_info=True ) - password = None + # Recoverable: let reset() go on and try to connect without + # the password (e.g. through a passfile). + return False, '', None return True, '', password def reset(self): diff --git a/web/pgadmin/utils/driver/psycopg3/tests/test_decode_password.py b/web/pgadmin/utils/driver/psycopg3/tests/test_decode_password.py index 348fcf05135..f1ac7025e38 100644 --- a/web/pgadmin/utils/driver/psycopg3/tests/test_decode_password.py +++ b/web/pgadmin/utils/driver/psycopg3/tests/test_decode_password.py @@ -34,10 +34,14 @@ def runTest(self): conn.saved_password_discarded = False manager = MagicMock(sid=1, password='enc') - with patch(MODULE + '.User') as user, \ - patch(MODULE + '.current_user'), \ - patch(MODULE + '.current_app'), \ - patch(MODULE + '.decrypt') as dec: + user = MagicMock() + dec = MagicMock() + # Explicit replacement objects: patching with the default MagicMock + # would make mock inspect the flask proxies outside an app context. + with patch(MODULE + '.User', user), \ + patch(MODULE + '.current_user', MagicMock()), \ + patch(MODULE + '.current_app', MagicMock()), \ + patch(MODULE + '.decrypt', dec): user.query.filter_by.return_value.first.return_value = object() if hasattr(self, 'side_effect'): dec.side_effect = self.side_effect From 3cbd4d323e984325b2a5bc338ef19f51a6d18307 Mon Sep 17 00:00:00 2001 From: Kundan Sable Date: Thu, 1 Oct 2026 16:54:42 +0530 Subject: [PATCH 5/5] Use passexec after a discarded saved password in export_password_env(); reset the tunnel-discarded flag once the tunnel starts. Co-Authored-By: Claude Sonnet 5.5 --- web/pgadmin/utils/driver/psycopg3/server_manager.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/web/pgadmin/utils/driver/psycopg3/server_manager.py b/web/pgadmin/utils/driver/psycopg3/server_manager.py index d6f3c5e4c76..492db622d88 100644 --- a/web/pgadmin/utils/driver/psycopg3/server_manager.py +++ b/web/pgadmin/utils/driver/psycopg3/server_manager.py @@ -549,13 +549,15 @@ def export_password_env(self, env): try: password = decrypt(self.password, crypt_key).decode() except DECRYPT_ERRORS as e: - # Skip the password, the utility will prompt/fail on its own. current_app.logger.warning( 'Ignoring the saved password of the server (#{0}) as it ' 'could not be decrypted. Error: {1}'.format( self.sid, str(e)), exc_info=True ) + # Fall back to passexec, as when no password is saved. + if self.passexec: + os.environ[str(env)] = self.passexec.get() return os.environ[str(env)] = password elif self.passexec: @@ -629,6 +631,7 @@ def create_ssh_tunnel(self, tunnel_password): # flag tunnel threads in daemon mode to fix hang issue. self.tunnel_object.daemon_forward_servers = True self.tunnel_object.start() + self.saved_tunnel_password_discarded = False self.tunnel_created = True except BaseSSHTunnelForwarderError as e: current_app.logger.exception(e)