Tighten password security by removing `URL.__str__`

For improved security, the :class:`_url.URL` object will now use password
obfuscation by default when ``str(url)`` is called. To stringify a URL with
cleartext password, the :meth:`_url.URL.render_as_string` may be used,
passing the :paramref:`_url.URL.render_as_string.hide_password` parameter
as ``False``. Thanks to our contributors for this pull request.

Fixes: #8567
Closes: #8563
Pull-request: https://github.com/sqlalchemy/sqlalchemy/pull/8563
Pull-request-sha: d1f1127f753849eb70b8d6cc64badf34e1b9219b

Change-Id: If756c8073ff99ac83876d9833c8fe1d7c76211f9

diff --git a/doc/build/changelog/unreleased_20/8567.rst b/doc/build/changelog/unreleased_20/8567.rst
new file mode 100644 (file)
index 0000000..e6c37c2
--- /dev/null
+++ b/doc/build/changelog/unreleased_20/8567.rst
@@ -0,0 +1,13 @@
+.. change::
+    :tags: bug, engine
+    :tickets: 8567
+
+    For improved security, the :class:`_url.URL` object will now use password
+    obfuscation by default when ``str(url)`` is called. To stringify a URL with
+    cleartext password, the :meth:`_url.URL.render_as_string` may be used,
+    passing the :paramref:`_url.URL.render_as_string.hide_password` parameter
+    as ``False``. Thanks to our contributors for this pull request.
+
+    .. seealso::
+
+        :ref:`change_8567`

diff --git a/doc/build/changelog/whatsnew_20.rst b/doc/build/changelog/whatsnew_20.rst
index 794f6d3cc30e58ffb2b4278b1f1e164845fe29e5..7ac8a7cf9db8a1bdcfcced3dafd749c4f4f1393b 100644 (file)
--- a/doc/build/changelog/whatsnew_20.rst
+++ b/doc/build/changelog/whatsnew_20.rst
@@ -1038,6 +1038,43 @@ not expected to have significant effects on backwards compatibility.
 
 .. _Cython: https://cython.org/
 
+.. _change_8567:
+
+``str(engine.url)`` will obfuscate the password by default
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+To avoid leakage of database passwords, calling ``str()`` on a
+:class:`.URL` will now enable the password obfuscation feature by default.
+Previously, this obfuscation would be in place for ``__repr__()`` calls
+but not ``__str__()``.   This change will impact applications and test suites
+that attempt to invoke :func:`_sa.create_engine` given the stringified URL
+from another engine, such as::
+
+    >>> e1 = create_engine("postgresql+psycopg2://scott:tiger@localhost/test")
+    >>> e2 = create_engine(str(e1.url))
+
+The above engine ``e2`` will not have the correct password; it will have the
+obfuscated string ``"***"``.
+
+The preferred approach for the above pattern is to pass the
+:class:`.URL` object directly, there's no need to stringify::
+
+    >>> e1 = create_engine("postgresql+psycopg2://scott:tiger@localhost/test")
+    >>> e2 = create_engine(e1.url)
+
+Otherwise, for a stringified URL with cleartext password, use the
+:meth:`_url.URL.render_as_string` method, passing the
+:paramref:`_url.URL.render_as_string.hide_password` parameter
+as ``False``::
+
+    >>> e1 = create_engine("postgresql+psycopg2://scott:tiger@localhost/test")
+    >>> url_string = e1.url.render_as_string(hide_password=False)
+    >>> e2 = create_engine(url_string)
+
+
+:ticket:`8567`
+
+
 .. _change_6980:
 
 "with_variant()" clones the original TypeEngine rather than changing the type

diff --git a/lib/sqlalchemy/engine/url.py b/lib/sqlalchemy/engine/url.py
index 8d80cfd1c8770c73836ecf5bc8dd6bb858b4f880..cca31edd0f833233aa8cd5d4b5d25b86951af4df 100644 (file)
--- a/lib/sqlalchemy/engine/url.py
+++ b/lib/sqlalchemy/engine/url.py
@@ -642,9 +642,6 @@ class URL(NamedTuple):
             )
         return s
 
-    def __str__(self) -> str:
-        return self.render_as_string(hide_password=False)
-
     def __repr__(self) -> str:
         return self.render_as_string()
 

diff --git a/lib/sqlalchemy/testing/plugin/plugin_base.py b/lib/sqlalchemy/testing/plugin/plugin_base.py
index 494e7d5ab5b9b20f469897d48b24228fe9464508..b90a2ec58c0b0eb57db54867f710772b4603086d 100644 (file)
--- a/lib/sqlalchemy/testing/plugin/plugin_base.py
+++ b/lib/sqlalchemy/testing/plugin/plugin_base.py
@@ -436,7 +436,10 @@ def _engine_uri(options, file_config):
 
         if options.write_idents and provision.FOLLOWER_IDENT:
             with open(options.write_idents, "a") as file_:
-                file_.write(provision.FOLLOWER_IDENT + " " + db_url + "\n")
+                file_.write(
+                    f"{provision.FOLLOWER_IDENT} "
+                    f"{db_url.render_as_string(hide_password=False)}\n"
+                )
 
         cfg = provision.setup_config(
             db_url, options, file_config, provision.FOLLOWER_IDENT

diff --git a/lib/sqlalchemy/testing/provision.py b/lib/sqlalchemy/testing/provision.py
index 12448b2fe11fc7b7cb4e61ed86bbd594c734217d..7ba89b505a2eb2a0372fb1f43ec466801fc65233 100644 (file)
--- a/lib/sqlalchemy/testing/provision.py
+++ b/lib/sqlalchemy/testing/provision.py
@@ -167,7 +167,7 @@ def _generate_driver_urls(url, extra_drivers):
     extra_drivers.discard(main_driver)
 
     url = generate_driver_url(url, main_driver, "")
-    yield str(url)
+    yield url
 
     for drv in list(extra_drivers):
 
@@ -183,7 +183,7 @@ def _generate_driver_urls(url, extra_drivers):
         if new_url:
             extra_drivers.remove(drv)
 
-            yield str(new_url)
+            yield new_url
 
 
 @register.init

diff --git a/test/dialect/oracle/test_dialect.py b/test/dialect/oracle/test_dialect.py
index e3101840af6a55c10d23149a739a962c2308a6ac..9c2496b05d1126f1d92092c7bcee75a73cadd061 100644 (file)
--- a/test/dialect/oracle/test_dialect.py
+++ b/test/dialect/oracle/test_dialect.py
@@ -100,7 +100,7 @@ class OracledbMode(fixtures.TestBase):
     def _run_in_process(self, fn):
         ctx = get_context("spawn")
         queue = ctx.Queue()
-        process = ctx.Process(target=fn, args=(str(config.db_url), queue))
+        process = ctx.Process(target=fn, args=(config.db_url, queue))
         try:
             process.start()
             process.join(10)

diff --git a/test/engine/test_parseconnect.py b/test/engine/test_parseconnect.py
index 23be61aaf8bb54860197462bc132e0e4054f9624..f571b4bab74b469a171e7bc972a738c70c0e24aa 100644 (file)
--- a/test/engine/test_parseconnect.py
+++ b/test/engine/test_parseconnect.py
@@ -83,36 +83,57 @@ class URLTest(fixtures.TestBase):
             "E:/work/src/LEM/db/hello.db",
             None,
         ), u.database
-        eq_(str(u), text)
+        eq_(u.render_as_string(hide_password=False), text)
 
     def test_rfc1738_password(self):
         u = url.make_url("dbtype://user:pass word + other%3Awords@host/dbname")
         eq_(u.password, "pass word + other:words")
-        eq_(str(u), "dbtype://user:pass word + other%3Awords@host/dbname")
+        eq_(str(u), "dbtype://user:***@host/dbname")
+        eq_(
+            u.render_as_string(hide_password=False),
+            "dbtype://user:pass word + other%3Awords@host/dbname",
+        )
 
         u = url.make_url(
             "dbtype://username:apples%2Foranges@hostspec/database"
         )
         eq_(u.password, "apples/oranges")
-        eq_(str(u), "dbtype://username:apples%2Foranges@hostspec/database")
+        eq_(str(u), "dbtype://username:***@hostspec/database")
+        eq_(
+            u.render_as_string(hide_password=False),
+            "dbtype://username:apples%2Foranges@hostspec/database",
+        )
 
         u = url.make_url(
             "dbtype://username:apples%40oranges%40%40@hostspec/database"
         )
         eq_(u.password, "apples@oranges@@")
+        eq_(str(u), "dbtype://username:***@hostspec/database")
         eq_(
-            str(u),
+            u.render_as_string(hide_password=False),
             "dbtype://username:apples%40oranges%40%40@hostspec/database",
         )
 
         u = url.make_url("dbtype://username%40:@hostspec/database")
         eq_(u.password, "")
         eq_(u.username, "username@")
-        eq_(str(u), "dbtype://username%40:@hostspec/database")
+        eq_(
+            # Do not reveal an empty password
+            str(u),
+            "dbtype://username%40:***@hostspec/database",
+        )
+        eq_(
+            u.render_as_string(hide_password=False),
+            "dbtype://username%40:@hostspec/database",
+        )
 
         u = url.make_url("dbtype://username:pass%2Fword@hostspec/database")
         eq_(u.password, "pass/word")
-        eq_(str(u), "dbtype://username:pass%2Fword@hostspec/database")
+        eq_(str(u), "dbtype://username:***@hostspec/database")
+        eq_(
+            u.render_as_string(hide_password=False),
+            "dbtype://username:pass%2Fword@hostspec/database",
+        )
 
     def test_password_custom_obj(self):
         class SecurePassword(str):
@@ -128,58 +149,76 @@ class URLTest(fixtures.TestBase):
         )
 
         eq_(u.password, "secured_password")
-        eq_(str(u), "dbtype://x:secured_password@localhost")
+        eq_(
+            u.render_as_string(hide_password=False),
+            "dbtype://x:secured_password@localhost",
+        )
 
         # test in-place modification
         sp.value = "new_secured_password"
         eq_(u.password, sp)
-        eq_(str(u), "dbtype://x:new_secured_password@localhost")
+        eq_(
+            u.render_as_string(hide_password=False),
+            "dbtype://x:new_secured_password@localhost",
+        )
 
         u = u.set(password="hi")
 
         eq_(u.password, "hi")
-        eq_(str(u), "dbtype://x:hi@localhost")
+        eq_(u.render_as_string(hide_password=False), "dbtype://x:hi@localhost")
 
         u = u._replace(password=None)
 
         is_(u.password, None)
-        eq_(str(u), "dbtype://x@localhost")
+        eq_(u.render_as_string(hide_password=False), "dbtype://x@localhost")
 
     def test_query_string(self):
         u = url.make_url("dialect://user:pass@host/db?arg1=param1&arg2=param2")
         eq_(u.query, {"arg1": "param1", "arg2": "param2"})
-        eq_(str(u), "dialect://user:pass@host/db?arg1=param1&arg2=param2")
+        eq_(
+            u.render_as_string(hide_password=False),
+            "dialect://user:pass@host/db?arg1=param1&arg2=param2",
+        )
 
         u = url.make_url("dialect://user:pass@host/?arg1=param1&arg2=param2")
         eq_(u.query, {"arg1": "param1", "arg2": "param2"})
         eq_(u.database, "")
-        eq_(str(u), "dialect://user:pass@host/?arg1=param1&arg2=param2")
+        eq_(
+            u.render_as_string(hide_password=False),
+            "dialect://user:pass@host/?arg1=param1&arg2=param2",
+        )
 
         u = url.make_url("dialect://user:pass@host?arg1=param1&arg2=param2")
         eq_(u.query, {"arg1": "param1", "arg2": "param2"})
         eq_(u.database, None)
-        eq_(str(u), "dialect://user:pass@host?arg1=param1&arg2=param2")
+        eq_(
+            u.render_as_string(hide_password=False),
+            "dialect://user:pass@host?arg1=param1&arg2=param2",
+        )
 
         u = url.make_url(
             "dialect://user:pass@host:450?arg1=param1&arg2=param2"
         )
         eq_(u.port, 450)
         eq_(u.query, {"arg1": "param1", "arg2": "param2"})
-        eq_(str(u), "dialect://user:pass@host:450?arg1=param1&arg2=param2")
+        eq_(
+            u.render_as_string(hide_password=False),
+            "dialect://user:pass@host:450?arg1=param1&arg2=param2",
+        )
 
         u = url.make_url(
             "dialect://user:pass@host/db?arg1=param1&arg2=param2&arg2=param3"
         )
         eq_(u.query, {"arg1": "param1", "arg2": ("param2", "param3")})
         eq_(
-            str(u),
+            u.render_as_string(hide_password=False),
             "dialect://user:pass@host/db?arg1=param1&arg2=param2&arg2=param3",
         )
 
         test_url = "dialect://user:pass@host/db?arg1%3D=param1&arg2=param+2"
         u = url.make_url(test_url)
         eq_(u.query, {"arg1=": "param1", "arg2": "param 2"})
-        eq_(str(u), test_url)
+        eq_(u.render_as_string(hide_password=False), test_url)
 
     def test_comparison(self):
         common_url = (
@@ -727,7 +766,10 @@ class CreateEngineTest(fixtures.TestBase):
         assert e.url.drivername == e2.url.drivername == "mysql+mysqldb"
         assert e.url.username == e2.url.username == "scott"
         assert e2.url is u
-        assert str(u) == "mysql+mysqldb://scott:tiger@localhost/test"
+        eq_(
+            u.render_as_string(hide_password=False),
+            "mysql+mysqldb://scott:tiger@localhost/test",
+        )
         assert repr(u) == "mysql+mysqldb://scott:***@localhost/test"
         assert repr(e) == "Engine(mysql+mysqldb://scott:***@localhost/test)"
         assert repr(e2) == "Engine(mysql+mysqldb://scott:***@localhost/test)"

diff --git a/test/ext/asyncio/test_engine_py3k.py b/test/ext/asyncio/test_engine_py3k.py
index 0269aaaeecfba959c3a2f44de8051f4b45367b82..cdf70ca678592c5f7a0540989b9f059533c0a9e5 100644 (file)
--- a/test/ext/asyncio/test_engine_py3k.py
+++ b/test/ext/asyncio/test_engine_py3k.py
@@ -635,7 +635,9 @@ class AsyncEngineTest(EngineFixture):
 
     def test_async_engine_from_config(self):
         config = {
-            "sqlalchemy.url": str(testing.db.url),
+            "sqlalchemy.url": testing.db.url.render_as_string(
+                hide_password=False
+            ),
             "sqlalchemy.echo": "true",
         }
         engine = async_engine_from_config(config)