Skip to content

URL encode username and password values for DB connections #7486

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wadesdev wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

URL encode username and password values for DB connections #7486

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 11 additions & 6 deletions src/fides/api/service/connectors/mysql_connector.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from typing import Dict, List
from urllib.parse import quote

from sqlalchemy.engine import Engine, LegacyCursorResult, create_engine # type: ignore

Expand All @@ -24,13 +25,15 @@ class MySQLConnector(SQLConnector):
secrets_schema = MySQLSchema

def build_uri(self) -> str:
"""Build URI of format mysql+pymysql://[user[:password]@][netloc][:port][/dbname]"""
"""Build URI of format mysql+pymysql://[user[:password]@][netloc][:port][/dbname].
Username and password are URL-encoded so that reserved characters (e.g. @, :) do not
break URI parsing."""
config = self.secrets_schema(**self.configuration.secrets or {})

user_password = ""
if config.username:
user = config.username
password = f":{config.password}" if config.password else ""
user = quote(str(config.username), safe="")
password = f":{quote(str(config.password), safe='')}" if config.password else ""
user_password = f"{user}{password}@"

netloc = config.host
Expand All @@ -40,13 +43,15 @@ def build_uri(self) -> str:
return url

def build_ssh_uri(self, local_address: tuple) -> str:
"""Build URI of format mysql+pymysql://[user[:password]@][ssh_host][:ssh_port][/dbname]"""
"""Build URI of format mysql+pymysql://[user[:password]@][ssh_host][:ssh_port][/dbname].
Username and password are URL-encoded so that reserved characters (e.g. @, :) do not
break URI parsing."""
config = self.secrets_schema(**self.configuration.secrets or {})

user_password = ""
if config.username:
user = config.username
password = f":{config.password}" if config.password else ""
user = quote(str(config.username), safe="")
password = f":{quote(str(config.password), safe='')}" if config.password else ""
user_password = f"{user}{password}@"

local_host, local_port = local_address
Expand Down
18 changes: 12 additions & 6 deletions src/fides/api/service/connectors/postgres_connector.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from urllib.parse import quote

from loguru import logger
from sqlalchemy import text
from sqlalchemy.engine import Connection, Engine, create_engine # type: ignore
Expand Down Expand Up @@ -25,13 +27,15 @@ def requires_primary_keys(self) -> bool:
return False

def build_uri(self) -> str:
"""Build URI of format postgresql://[user[:password]@][netloc][:port][/dbname]"""
"""Build URI of format postgresql://[user[:password]@][netloc][:port][/dbname].
Username and password are URL-encoded so that reserved characters (e.g. @, :) do not
break URI parsing."""
config = self.secrets_schema(**self.configuration.secrets or {})

user_password = ""
if config.username:
user = config.username
password = f":{config.password}" if config.password else ""
user = quote(str(config.username), safe="")
password = f":{quote(str(config.password), safe='')}" if config.password else ""
user_password = f"{user}{password}@"

netloc = config.host
Expand All @@ -41,13 +45,15 @@ def build_uri(self) -> str:
return f"postgresql://{user_password}{netloc}{port}{dbname}{query}"

def build_ssh_uri(self, local_address: tuple) -> str:
"""Build URI of format postgresql://[user[:password]@][ssh_host][:ssh_port][/dbname]"""
"""Build URI of format postgresql://[user[:password]@][ssh_host][:ssh_port][/dbname].
Username and password are URL-encoded so that reserved characters (e.g. @, :) do not
break URI parsing."""
config = self.secrets_schema(**self.configuration.secrets or {})

user_password = ""
if config.username:
user = config.username
password = f":{config.password}" if config.password else ""
user = quote(str(config.username), safe="")
password = f":{quote(str(config.password), safe='')}" if config.password else ""
user_password = f"{user}{password}@"

local_host, local_port = local_address
Expand Down
19 changes: 13 additions & 6 deletions src/fides/api/service/connectors/redshift_connector.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from typing import Dict, Union
from urllib.parse import quote_plus
from urllib.parse import quote

from loguru import logger
from sqlalchemy import text
Expand All @@ -22,25 +22,32 @@ class RedshiftConnector(SQLConnector):
secrets_schema = RedshiftSchema

def build_ssh_uri(self, local_address: tuple) -> str:
"""Build SSH URI of format redshift+psycopg2://[user[:password]@][ssh_host][:ssh_port][/dbname]"""
"""Build SSH URI of format redshift+psycopg2://[user[:password]@][ssh_host][:ssh_port][/dbname].
Username and password are URL-encoded so that reserved characters (e.g. @, :) do not
break URI parsing."""
local_host, local_port = local_address

config = self.secrets_schema(**self.configuration.secrets or {})

port = f":{local_port}" if local_port else ""
database = f"/{config.database}" if config.database else ""
url = f"redshift+psycopg2://{config.user}:{config.password}@{local_host}{port}{database}"
user = quote(str(config.user), safe="")
password = quote(str(config.password), safe="")
Copy link
Contributor

@vcruces vcruces Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about adding a check to ensure the credential exists before applying str?

If config.password is None, str(None) converts this to the literal string "None", resulting in URLs like: redshift+psycopg2://user:None@...

This differs from the MySQL/Postgres implementation, which conditionally includes credentials only when they exist

url = f"redshift+psycopg2://{user}:{password}@{local_host}{port}{database}"
return url

# Overrides BaseConnector.build_uri
def build_uri(self) -> str:
"""Build URI of format redshift+psycopg2://user:password@[host][:port][/database]"""
"""Build URI of format redshift+psycopg2://user:password@[host][:port][/database].
Username and password are URL-encoded so that reserved characters (e.g. @, :) do not
break URI parsing."""
config = self.secrets_schema(**self.configuration.secrets or {})

url_encoded_password = quote_plus(config.password)
user = quote(str(config.user), safe="")
Copy link
Contributor

@vcruces vcruces Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed this changed from quote_plus to quote. I don’t see an issue with using quote here, if anything, it may even be preferable, but I’m wondering whether the original use of quote_plus was intentional 🤔

Not raising this as a change request, just something to keep in mind in case we see any related behavior changes down the line

password = quote(str(config.password), safe="")
port = f":{config.port}" if config.port else ""
database = f"/{config.database}" if config.database else ""
url = f"redshift+psycopg2://{config.user}:{url_encoded_password}@{config.host}{port}{database}"
url = f"redshift+psycopg2://{user}:{password}@{config.host}{port}{database}"
return url

# Overrides SQLConnector.create_client
Expand Down
Loading