summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorYassen Damyanov <yd@itlabs.bg>2022-09-22 12:12:28 -0400
committerMike Bayer <mike_mp@zzzcomputing.com>2022-09-23 17:27:30 -0400
commit3333c6623fa45bcbc7fabd061184a79b7b7f2fa6 (patch)
tree6c262c52683c544470d68a0ae40c5c0ed16b1722
parentd50bbd56740f86bb363b405f7d8e5df9667bb4e3 (diff)
downloadsqlalchemy-3333c6623fa45bcbc7fabd061184a79b7b7f2fa6.tar.gz
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
-rw-r--r--doc/build/changelog/unreleased_20/8567.rst13
-rw-r--r--doc/build/changelog/whatsnew_20.rst37
-rw-r--r--lib/sqlalchemy/engine/url.py3
-rw-r--r--lib/sqlalchemy/testing/plugin/plugin_base.py5
-rw-r--r--lib/sqlalchemy/testing/provision.py4
-rw-r--r--test/dialect/oracle/test_dialect.py2
-rw-r--r--test/engine/test_parseconnect.py76
-rw-r--r--test/ext/asyncio/test_engine_py3k.py4
8 files changed, 119 insertions, 25 deletions
diff --git a/doc/build/changelog/unreleased_20/8567.rst b/doc/build/changelog/unreleased_20/8567.rst
new file mode 100644
index 000000000..e6c37c2b2
--- /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 794f6d3cc..7ac8a7cf9 100644
--- 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 8d80cfd1c..cca31edd0 100644
--- 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 494e7d5ab..b90a2ec58 100644
--- 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 12448b2fe..7ba89b505 100644
--- 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 e3101840a..9c2496b05 100644
--- 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 23be61aaf..f571b4bab 100644
--- 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 0269aaaee..cdf70ca67 100644
--- 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)