fix: persist rotated saved password and correct manager session state

web/pgadmin/browser/server_groups/servers/__init__.py

@@ -44,7 +44,7 @@
 from sqlalchemy.orm.attributes import flag_modified
 from pgadmin.utils.preferences import Preferences
 from .... import socketio as sio
-from pgadmin.utils import get_complete_file_path
+from pgadmin.utils import get_complete_file_path, str_to_bool
 from pgadmin.settings.utils import with_object_filters
 from pgadmin.utils.server_access import get_server, \
     get_user_server_query, get_server_group
@@ -1618,6 +1618,12 @@ def connect(self, gid, sid, is_qt=False, server=None):
         passfile = None
         tunnel_password = None
         save_password = False
+        # Distinguishes "the caller explicitly said false" from "the
+        # caller didn't mention save_password at all" -- only the former
+        # should clear an existing saved credential (see the success
+        # branch below); legacy callers that omit the field must not have
+        # a saved password silently wiped out from under them.
+        save_password_provided = False
         save_tunnel_password = False
         prompt_password = False
         prompt_tunnel_password = False
@@ -1685,8 +1691,15 @@ def connect(self, gid, sid, is_qt=False, server=None):
                 password = conn_passwd or server.password
         else:
             password = data['password'] if 'password' in data else None
-            save_password = data['save_password']\
-                if 'save_password' in data else False
+            # The password-prompt dialog seeds its checkbox from the
+            # server's current save_password setting (see
+            # get_response_for_password) and always sends its state, so
+            # this reflects the user's explicit choice -- including
+            # unchecking it for a server previously configured to save
+            # its password.
+            save_password_provided = 'save_password' in data
+            save_password = str_to_bool(
+                data['save_password'] if save_password_provided else False)
 
             try:
                 # Encrypt the password before saving with user's login
@@ -1737,6 +1750,12 @@ def connect(self, gid, sid, is_qt=False, server=None):
                     # 1 is True in SQLite as no boolean type
                     if _is_non_owner(server):
                         setattr(shared_server, 'save_password', 1)
+                        # `server` is a detached overlay (see
+                        # get_shared_server_properties) built before this
+                        # write, so it won't pick up the SharedServer
+                        # change on its own -- keep it in sync since the
+                        # connect response below reports its state.
+                        server.save_password = 1
                     else:
                         setattr(server, 'save_password', 1)
 
@@ -1754,6 +1773,28 @@ def connect(self, gid, sid, is_qt=False, server=None):
                     manager.release(database=server.maintenance_db)
                     conn = None
 
+                    return internal_server_error(errormsg=str(e))
+            elif save_password_provided and not save_password and \
+                    server.save_password and config.ALLOW_SAVE_PASSWORD:
+                # The user explicitly unticked "Save Password" on a server
+                # that had one saved -- clear it instead of leaving the
+                # now-stale credential and flag in place.
+                try:
+                    if _is_non_owner(server):
+                        setattr(shared_server, 'save_password', 0)
+                        setattr(shared_server, 'password', None)
+                        # Keep the detached overlay in sync -- see the
+                        # comment in the save_password branch above.
+                        server.save_password = 0
+                    else:
+                        setattr(server, 'save_password', 0)
+                        setattr(server, 'password', None)
+                    db.session.commit()
+                except Exception as e:
+                    current_app.logger.exception(e)
+                    manager.release(database=server.maintenance_db)
+                    conn = None
+
                     return internal_server_error(errormsg=str(e))
 
             if save_tunnel_password and config.ALLOW_SAVE_TUNNEL_PASSWORD:
@@ -2195,6 +2236,7 @@ def get_response_for_password(self, server, status, prompt_password=False,
                 "service": server.service,
                 "prompt_tunnel_password": prompt_tunnel_password,
                 "prompt_password": prompt_password,
+                "save_password": bool(server.save_password),
                 "allow_save_password":
                     True if config.ALLOW_SAVE_PASSWORD and
                     'allow_save_password' in session and
@@ -2217,6 +2259,7 @@ def get_response_for_password(self, server, status, prompt_password=False,
                 "errmsg": errmsg,
                 "service": server.service,
                 "prompt_password": True,
+                "save_password": bool(server.save_password),
                 "allow_save_password":
                     True if config.ALLOW_SAVE_PASSWORD and
                     'allow_save_password' in session and

web/pgadmin/static/js/Dialogs/ConnectServerContent.jsx

@@ -26,7 +26,10 @@ export default function ConnectServerContent({closeModal, data, onOK, setHeight,
     tunnel_password: '',
     save_tunnel_password: false,
     password: '',
-    save_password: false,
+    // Seed the checkbox from the server's current setting so that, for a
+    // server already configured to save its password, the checkbox
+    // reflects that instead of always defaulting to unchecked.
+    save_password: Boolean(data?.save_password),
   });
 
   const onTextChange = (e, id) => {
@@ -119,8 +122,10 @@ export default function ConnectServerContent({closeModal, data, onOK, setHeight,
             }
             if(data.prompt_password) {
               postFormData.append('password', formData.password);
-              formData.save_password &&
-                postFormData.append('save_password', formData.save_password);
+              // Always send the checkbox state (rather than only when
+              // checked) so the backend can tell "explicitly unchecked"
+              // apart from "field not sent".
+              postFormData.append('save_password', formData.save_password);
             }
             onOK?.(postFormData);
             closeModal();

web/pgadmin/tools/sqleditor/__init__.py

@@ -42,7 +42,7 @@
 from pgadmin.tools.sqleditor.utils.update_session_grid_transaction import \
     update_session_grid_transaction
 from pgadmin.utils import PgAdminModule
-from pgadmin.utils import get_storage_directory
+from pgadmin.utils import get_storage_directory, str_to_bool
 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
@@ -267,6 +267,7 @@ def initialize_viewdata(trans_id, cmd_type, obj_type, sgid, sid, did, obj_id):
                         "username": user or server.username,
                         "errmsg": msg,
                         "prompt_password": True,
+                        "save_password": bool(server.save_password),
                         "allow_save_password": True
                         if ALLOW_SAVE_PASSWORD and
                         session.get('allow_save_password', None)
@@ -592,6 +593,7 @@ def _init_sqleditor(trans_id, connect, sgid, sid, did, dbname=None, **kwargs):
                             "username": user or server.username,
                             "errmsg": msg,
                             "prompt_password": True,
+                            "save_password": bool(server.save_password),
                             "allow_save_password": True
                             if ALLOW_SAVE_PASSWORD and
                             session.get('allow_save_password', None)
@@ -2713,7 +2715,7 @@ def connect_server(sid):
         # 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)
+        _cache_manager_password_from_request(manager, server)
         return make_json_response(
             success=1,
             info=gettext("Server connected."),
@@ -2726,7 +2728,7 @@ def connect_server(sid):
     )
 
 
-def _cache_manager_password_from_request(manager):
+def _cache_manager_password_from_request(manager, server=None):
     """
     Cache the password supplied with the current request (from a tool's
     password prompt) onto the server manager, so that connections opened by
@@ -2737,6 +2739,13 @@ def _cache_manager_password_from_request(manager):
     password, so a freshly entered credential (e.g. a regenerated, short-lived
     cloud auth token) takes effect immediately.
 
+    When "Save Password" is requested and allowed, the freshly entered
+    password is also persisted to the server record (overwriting any stale
+    stored ciphertext).  Without this, a rotated/regenerated password entered
+    at the prompt would work for the current session only and the tool would
+    keep re-using the stale saved password and re-prompt on the next
+    connection.
+
     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.
@@ -2757,12 +2766,116 @@ def _cache_manager_password_from_request(manager):
         if not crypt_key_present:
             return
 
-        manager._update_password(encrypt(password, crypt_key))
+        # This request never actually uses `password` to open a connection
+        # (the manager's primary connection was already established
+        # beforehand), so it must be validated against the server before
+        # caching it on the manager or persisting it -- otherwise a typo at
+        # the prompt would silently replace a working password, for the
+        # current session as well as in durable storage.
+        if not _password_is_valid(manager, password):
+            return
+
+        enc_password = encrypt(password, crypt_key)
+        manager._update_password(enc_password)
         manager.update_session()
+
+        if server is None or not ALLOW_SAVE_PASSWORD:
+            return
+
+        save_password_provided = 'save_password' in data
+        save_password = str_to_bool(data.get('save_password', False))
+
+        # Persist the freshly entered password if the user asked to save
+        # it, so the stale stored ciphertext is replaced. An explicit
+        # false instead clears any previously saved credential -- mirrors
+        # the same "Save Password" opt-out handling in
+        # browser.server_groups.servers.ServerNode.connect -- so
+        # unchecking the box here doesn't leave a stale saved password.
+        if save_password:
+            _persist_saved_password(server, enc_password)
+        elif save_password_provided:
+            _clear_saved_password(server)
     except Exception as e:
         current_app.logger.exception(e)
 
 
+def _password_is_valid(manager, password):
+    """
+    Verify that `password` (plaintext) actually authenticates against the
+    server, using a standalone connection that is closed immediately
+    afterwards -- it is never registered with the manager.
+    """
+    import psycopg
+    try:
+        conn_string = manager.create_connection_string(
+            manager.db, manager.user, password)
+        test_conn = psycopg.Connection.connect(
+            conn_string, connect_timeout=10)
+        test_conn.close()
+        return True
+    except psycopg.Error as e:
+        current_app.logger.info(
+            'Not persisting the re-entered password: it failed '
+            f'validation against the server.\nError: {e}'
+        )
+        return False
+
+
+def _get_save_password_target(server):
+    """
+    Return the record ("save_password"/"password" live on the owned Server
+    row, or on the current user's SharedServer row for a shared server they
+    don't own).
+    """
+    from pgadmin.browser.server_groups.servers import (
+        ServerModule, _is_non_owner)
+
+    if _is_non_owner(server):
+        shared_server = ServerModule.get_shared_server(
+            server, server.servergroup_id)
+        if shared_server is not None:
+            return shared_server
+    return server
+
+
+def _persist_saved_password(server, enc_password):
+    """
+    Persist the encrypted password to the server record (owned or shared),
+    replacing any stale stored ciphertext.
+    """
+    from pgadmin.model import db
+
+    target = _get_save_password_target(server)
+    setattr(target, 'save_password', 1)
+    setattr(target, 'password', enc_password)
+    try:
+        db.session.commit()
+    except Exception:
+        db.session.rollback()
+        raise
+
+
+def _clear_saved_password(server):
+    """
+    Clear a previously saved password on the owned or shared server record,
+    so an explicit "Save Password" opt-out doesn't leave a stale saved
+    credential behind.
+    """
+    from pgadmin.model import db
+
+    target = _get_save_password_target(server)
+    if not target.save_password:
+        return
+
+    setattr(target, 'save_password', 0)
+    setattr(target, 'password', None)
+    try:
+        db.session.commit()
+    except Exception:
+        db.session.rollback()
+        raise
+
+
 @blueprint.route(
     '/filter_dialog/<int:trans_id>',
     methods=["PUT"], endpoint='set_filter_data'