Avoid rewriting passwords loaded from keyring

AUTHORS

@@ -154,6 +154,7 @@ Contributors:
     * Diego
     * Chris (ChrisJr404)
     * Pieter Ouwerkerk (pouwerkerk)
+    * Melvin Cerba (MelvinCERBA)
 
 Creator:
 --------

changelog.rst

@@ -22,6 +22,9 @@ Features:
 
 Bug fixes:
 ----------
+* Avoid rewriting passwords loaded from the keyring after every successful
+  connection. On macOS, rewriting recreated Keychain items and restored Python
+  as an application allowed to access them without confirmation.
 * Fix special commands being broken while explain mode (F5) is on. Every input
   was prefixed with ``EXPLAIN (...)`` and sent to the server as SQL, including
   backslash commands and the bare words ``exit``/``quit``, so ``\q``, ``\d``,

pgcli/main.py

@@ -748,8 +748,10 @@ def connect(self, database="", host="", user="", port="", passwd="", dsn="", **k
 
         key = f"{user}@{host}@{port}"
 
+        password_loaded_from_keyring = False
         if not passwd and auth.keyring:
             passwd = auth.keyring_get_password(key)
+            password_loaded_from_keyring = bool(passwd)
 
         def should_ask_for_password(exc):
             # Prompt for a password after 1st attempt to connect
@@ -847,6 +849,7 @@ def should_ask_for_password(exc):
                         show_default=False,
                         type=str,
                     )
+                    password_loaded_from_keyring = False
                     pgexecute = PGExecute(
                         database,
                         user,
@@ -859,7 +862,7 @@ def should_ask_for_password(exc):
                     )
                 else:
                     raise e
-            if passwd and auth.keyring:
+            if passwd and auth.keyring and not password_loaded_from_keyring:
                 auth.keyring_set_password(key, passwd)
 
         except Exception as e:  # Connecting to a database could fail.

tests/test_main.py

@@ -7,6 +7,7 @@
 
 import pytest
 from click.testing import CliRunner
+from psycopg import OperationalError
 
 try:
     import setproctitle
@@ -644,6 +645,42 @@ def test_pg_service_file(tmpdir):
     del os.environ["PGSERVICEFILE"]
 
 
+def test_connect_does_not_resave_keyring_password(tmpdir):
+    cli = PGCli(pgclirc_file=str(tmpdir.join("rcfile")))
+
+    with (
+        mock.patch.dict(os.environ, {"PGPASSWORD": ""}),
+        mock.patch("pgcli.main.auth.keyring", True),
+        mock.patch("pgcli.main.auth.keyring_get_password", return_value="keyring-password") as get_password,
+        mock.patch("pgcli.main.auth.keyring_set_password") as set_password,
+        mock.patch("pgcli.main.PGExecute") as pgexecute,
+    ):
+        cli.connect(database="test", host="localhost", user="postgres", port=5432)
+
+    get_password.assert_called_once_with("postgres@localhost@5432")
+    assert pgexecute.call_args.args[2] == "keyring-password"
+    set_password.assert_not_called()
+
+
+def test_connect_saves_replacement_for_invalid_keyring_password(tmpdir):
+    cli = PGCli(pgclirc_file=str(tmpdir.join("rcfile")))
+
+    with (
+        mock.patch.dict(os.environ, {"PGPASSWORD": ""}),
+        mock.patch("pgcli.main.auth.keyring", True),
+        mock.patch("pgcli.main.auth.keyring_get_password", return_value="old-password"),
+        mock.patch("pgcli.main.auth.keyring_set_password") as set_password,
+        mock.patch("pgcli.main.click.prompt", return_value="new-password"),
+        mock.patch(
+            "pgcli.main.PGExecute",
+            side_effect=[OperationalError("password authentication failed"), mock.Mock()],
+        ),
+    ):
+        cli.connect(database="test", host="localhost", user="postgres", port=5432)
+
+    set_password.assert_called_once_with("postgres@localhost@5432", "new-password")
+
+
 def test_ssl_db_uri(tmpdir):
     with mock.patch.object(PGCli, "connect") as mock_connect:
         cli = PGCli(pgclirc_file=str(tmpdir.join("rcfile")))