Skip to content

Commit

Permalink
Merge pull request #191 from lsst/tickets/DM-21222
Browse files Browse the repository at this point in the history
DM-21222: Stop forcing existence of credentials in connection strings.
  • Loading branch information
DinoBektesevic committed Sep 14, 2019
2 parents 04626c5 + 5db9362 commit c901b73
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 37 deletions.
23 changes: 18 additions & 5 deletions python/lsst/daf/butler/core/connectionString.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
__all__ = ("DB_AUTH_ENVVAR", "DB_AUTH_PATH", "ConnectionStringFactory")

from sqlalchemy.engine import url
from lsst.daf.butler.core.dbAuth import DbAuth
from lsst.daf.butler.core.dbAuth import DbAuth, DbAuthError, DbAuthPermissionsError

DB_AUTH_ENVVAR = "LSST_DB_AUTH"
"""Default name of the environmental variable that will be used to locate DB
Expand Down Expand Up @@ -68,8 +68,7 @@ def fromConfig(cls, registryConfig):
Raises
------
DBAuthError
If the credentials file has incorrect permissions, doesn't exist at
the given location or is formatted incorrectly.
If the credentials file has incorrect permissions.
"""
# this import can not live on the top because of circular import issue
from lsst.daf.butler.core.registryConfig import RegistryConfig
Expand All @@ -80,11 +79,25 @@ def fromConfig(cls, registryConfig):
if getattr(conStr, key) is None:
setattr(conStr, key, regConf.get(key))

# sqlite with users and passwords not supported
if None in (conStr.username, conStr.password) and "sqlite" not in conStr.drivername:
# when host is None we cross our fingers and return
if conStr.host is None:
return conStr

# allow other mechanisms to insert username and password by not forcing
# the credentials to exist. If other mechanisms are used it's possible
# that credentials were never set-up, or that there would be no match
# in the credentials file. Both need to be ignored.
try:
dbAuth = DbAuth(DB_AUTH_PATH, DB_AUTH_ENVVAR)
auth = dbAuth.getAuth(conStr.drivername, conStr.username, conStr.host,
conStr.port, conStr.database)
except DbAuthPermissionsError:
# re-raise permission error for safety
raise
except DbAuthError:
# credentials file doesn't exist or no match was found
pass
else:
conStr.username = auth[0]
conStr.password = auth[1]

Expand Down
16 changes: 7 additions & 9 deletions python/lsst/daf/butler/core/dbAuth.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
import urllib.parse
import yaml

__all__ = ["DbAuth", "DbAuthError"]
__all__ = ["DbAuth", "DbAuthError", "DbAuthPermissionsError"]


class DbAuthError(RuntimeError):
Expand All @@ -34,6 +34,10 @@ class DbAuthError(RuntimeError):
pass


class DbAuthPermissionsError(DbAuthError):
"""Credentials file has incorrect permissions."""


class DbAuth:
"""Retrieves authentication information for database connections.
Expand Down Expand Up @@ -70,7 +74,7 @@ def __init__(self, path=None, envVar=None, authList=None):
raise DbAuthError("No DbAuth configuration file: " + secretPath)
mode = os.stat(secretPath).st_mode
if mode & (stat.S_IRWXG | stat.S_IRWXO) != 0:
raise DbAuthError(
raise DbAuthPermissionsError(
"DbAuth configuration file {} has incorrect permissions: "
"{:o}".format(secretPath, mode))

Expand Down Expand Up @@ -162,10 +166,6 @@ def getAuth(self, drivername, username, host, port, database):
# Check for mandatory entries
if "url" not in authDict:
raise DbAuthError("Missing URL in DbAuth configuration")
if "password" not in authDict:
raise DbAuthError(
"Missing password in DbAuth configuration "
"for URL: " + authDict["url"])

# Parse pseudo-URL from db-auth.yaml
components = urllib.parse.urlparse(authDict["url"])
Expand Down Expand Up @@ -205,9 +205,7 @@ def getAuth(self, drivername, username, host, port, database):
return (username, authDict["password"])
else:
if "username" not in authDict:
raise DbAuthError(
"Missing username in DbAuth configuration for "
"URL: " + authDict["url"])
return (None, authDict["password"])
return (authDict["username"], authDict["password"])

raise DbAuthError(
Expand Down
23 changes: 0 additions & 23 deletions tests/test_dbAuth.py
Original file line number Diff line number Diff line change
Expand Up @@ -246,13 +246,6 @@ def test_errors(self):
r"\(postgresql, None, example\.com, None, foo\)$"):
auth.getAuth("postgresql", None, "example.com", None, "foo")

auth = DbAuth(authList=[dict(url="postgresql://*.example.com")])
with self.assertRaisesRegex(
DbAuthError,
r"^Missing password in DbAuth configuration for URL: "
r"postgresql://\*\.example\.com$"):
auth.getAuth("postgresql", None, "example.com", None, "foo")

auth = DbAuth(authList=[dict(password="testing")])
with self.assertRaisesRegex(
DbAuthError,
Expand All @@ -272,14 +265,6 @@ def test_errors(self):
r"^Missing host in URL: postgresql:///foo$"):
auth.getAuth("postgresql", None, "example.com", None, "foo")

auth = DbAuth(authList=[
dict(url="postgresql://example.com/foo", password="testing")])
with self.assertRaisesRegex(
DbAuthError,
r"^Missing username in DbAuth configuration for URL: "
r"postgresql://example.com/foo$"):
auth.getAuth("postgresql", None, "example.com", None, "foo")

def test_getUrl(self):
"""Repeat relevant tests using getUrl interface."""
for matchPattern in [
Expand Down Expand Up @@ -424,14 +409,6 @@ def test_getUrl(self):
r"\(postgresql, None, example\.com, None, foo\)$"):
auth.getUrl("postgresql://example.com/foo")

auth = DbAuth(authList=[
dict(url="postgresql://example.com/foo", password="testing")])
with self.assertRaisesRegex(
DbAuthError,
r"^Missing username in DbAuth configuration for URL: "
r"postgresql://example.com/foo$"):
auth.getUrl("postgresql://example.com/foo")

def test_urlEncoding(self):
"""Test URL encoding of username and password."""
auth = DbAuth(authList=[
Expand Down

0 comments on commit c901b73

Please sign in to comment.