From 8021fc16cb851b5c880c515bb766740e4480dc35 Mon Sep 17 00:00:00 2001 From: Jacky Hu Date: Thu, 4 Jul 2024 16:17:18 -0700 Subject: [PATCH] [PECO-1715] Remove username/password (BasicAuth) auth option Signed-off-by: Jacky Hu --- src/databricks/sql/auth/auth.py | 17 ++++------ src/databricks/sql/auth/authenticators.py | 15 --------- src/databricks/sql/client.py | 4 +-- tests/unit/test_auth.py | 40 +++++------------------ tests/unit/test_client.py | 8 ----- tests/unit/test_oauth_persistence.py | 2 -- 6 files changed, 17 insertions(+), 69 deletions(-) diff --git a/src/databricks/sql/auth/auth.py b/src/databricks/sql/auth/auth.py index 40376b90..2a6c9d4d 100755 --- a/src/databricks/sql/auth/auth.py +++ b/src/databricks/sql/auth/auth.py @@ -4,7 +4,6 @@ from databricks.sql.auth.authenticators import ( AuthProvider, AccessTokenAuthProvider, - BasicAuthProvider, ExternalAuthProvider, DatabricksOAuthProvider, ) @@ -13,7 +12,7 @@ class AuthType(Enum): DATABRICKS_OAUTH = "databricks-oauth" AZURE_OAUTH = "azure-oauth" - # other supported types (access_token, user/pass) can be inferred + # other supported types (access_token) can be inferred # we can add more types as needed later @@ -21,8 +20,6 @@ class ClientContext: def __init__( self, hostname: str, - username: Optional[str] = None, - password: Optional[str] = None, access_token: Optional[str] = None, auth_type: Optional[str] = None, oauth_scopes: Optional[List[str]] = None, @@ -34,8 +31,6 @@ def __init__( credentials_provider=None, ): self.hostname = hostname - self.username = username - self.password = password self.access_token = access_token self.auth_type = auth_type self.oauth_scopes = oauth_scopes @@ -65,8 +60,6 @@ def get_auth_provider(cfg: ClientContext): ) elif cfg.access_token is not None: return AccessTokenAuthProvider(cfg.access_token) - elif cfg.username is not None and cfg.password is not None: - return BasicAuthProvider(cfg.username, cfg.password) elif cfg.use_cert_as_auth and cfg.tls_client_cert_file: # no op authenticator. authentication is performed using ssl certificate outside of headers return AuthProvider() @@ -100,12 +93,16 @@ def get_python_sql_connector_auth_provider(hostname: str, **kwargs): (client_id, redirect_port_range) = get_client_id_and_redirect_port( auth_type == AuthType.AZURE_OAUTH.value ) + if kwargs.get("username") or kwargs.get("password"): + raise ValueError( + "Username/password authentication is no longer supported. " + "Please use OAuth or access token instead." + ) + cfg = ClientContext( hostname=normalize_host_name(hostname), auth_type=auth_type, access_token=kwargs.get("access_token"), - username=kwargs.get("_username"), - password=kwargs.get("_password"), use_cert_as_auth=kwargs.get("_use_cert_as_auth"), tls_client_cert_file=kwargs.get("_tls_client_cert_file"), oauth_scopes=PYSQL_OAUTH_SCOPES, diff --git a/src/databricks/sql/auth/authenticators.py b/src/databricks/sql/auth/authenticators.py index e89c2bd5..64eb91bb 100644 --- a/src/databricks/sql/auth/authenticators.py +++ b/src/databricks/sql/auth/authenticators.py @@ -43,21 +43,6 @@ def add_headers(self, request_headers: Dict[str, str]): request_headers["Authorization"] = self.__authorization_header_value -# Private API: this is an evolving interface and it will change in the future. -# Please must not depend on it in your applications. -class BasicAuthProvider(AuthProvider): - def __init__(self, username: str, password: str): - auth_credentials = f"{username}:{password}".encode("UTF-8") - auth_credentials_base64 = base64.standard_b64encode(auth_credentials).decode( - "UTF-8" - ) - - self.__authorization_header_value = f"Basic {auth_credentials_base64}" - - def add_headers(self, request_headers: Dict[str, str]): - request_headers["Authorization"] = self.__authorization_header_value - - # Private API: this is an evolving interface and it will change in the future. # Please must not depend on it in your applications. class DatabricksOAuthProvider(AuthProvider): diff --git a/src/databricks/sql/client.py b/src/databricks/sql/client.py index c5962147..e56d22f6 100755 --- a/src/databricks/sql/client.py +++ b/src/databricks/sql/client.py @@ -163,10 +163,8 @@ def read(self) -> Optional[OAuthToken]: # Internal arguments in **kwargs: # _user_agent_entry # Tag to add to User-Agent header. For use by partners. - # _username, _password - # Username and password Basic authentication (no official support) # _use_cert_as_auth - # Use a TLS cert instead of a token or username / password (internal use only) + # Use a TLS cert instead of a token # _enable_ssl # Connect over HTTP instead of HTTPS # _port diff --git a/tests/unit/test_auth.py b/tests/unit/test_auth.py index 5b81f2b7..a1b36bf7 100644 --- a/tests/unit/test_auth.py +++ b/tests/unit/test_auth.py @@ -5,7 +5,6 @@ from databricks.sql.auth.auth import ( AccessTokenAuthProvider, - BasicAuthProvider, AuthProvider, ExternalAuthProvider, AuthType, @@ -33,21 +32,6 @@ def test_access_token_provider(self): self.assertEqual(len(http_request.keys()), 2) self.assertEqual(http_request["myKey"], "myVal") - def test_basic_auth_provider(self): - username = "moderakh" - password = "Elevate Databricks 123!!!" - auth = BasicAuthProvider(username=username, password=password) - - http_request = {"myKey": "myVal"} - auth.add_headers(http_request) - - self.assertEqual( - http_request["Authorization"], - "Basic bW9kZXJha2g6RWxldmF0ZSBEYXRhYnJpY2tzIDEyMyEhIQ==", - ) - self.assertEqual(len(http_request.keys()), 2) - self.assertEqual(http_request["myKey"], "myVal") - def test_noop_auth_provider(self): auth = AuthProvider() @@ -175,21 +159,6 @@ def __call__(self, *args, **kwargs) -> HeaderFactory: auth_provider.add_headers(headers) self.assertEqual(headers["foo"], "bar") - def test_get_python_sql_connector_auth_provider_username_password(self): - username = "moderakh" - password = "Elevate Databricks 123!!!" - hostname = "moderakh-test.cloud.databricks.com" - kwargs = {"_username": username, "_password": password} - auth_provider = get_python_sql_connector_auth_provider(hostname, **kwargs) - self.assertTrue(type(auth_provider).__name__, "BasicAuthProvider") - - headers = {} - auth_provider.add_headers(headers) - self.assertEqual( - headers["Authorization"], - "Basic bW9kZXJha2g6RWxldmF0ZSBEYXRhYnJpY2tzIDEyMyEhIQ==", - ) - def test_get_python_sql_connector_auth_provider_noop(self): tls_client_cert_file = "fake.cert" use_cert_as_auth = "abc" @@ -200,3 +169,12 @@ def test_get_python_sql_connector_auth_provider_noop(self): } auth_provider = get_python_sql_connector_auth_provider(hostname, **kwargs) self.assertTrue(type(auth_provider).__name__, "CredentialProvider") + + def test_get_python_sql_connector_basic_auth(self): + kwargs = { + "username": "username", + "password": "password", + } + with self.assertRaises(ValueError) as e: + get_python_sql_connector_auth_provider("foo.cloud.databricks.com", **kwargs) + self.assertIn("Username/password authentication is no longer supported", str(e.exception)) diff --git a/tests/unit/test_client.py b/tests/unit/test_client.py index 43574160..68e8b830 100644 --- a/tests/unit/test_client.py +++ b/tests/unit/test_client.py @@ -103,7 +103,6 @@ def test_close_uses_the_correct_session_id(self, mock_client_class): def test_auth_args(self, mock_client_class): # Test that the following auth args work: # token = foo, - # token = None, _username = foo, _password = bar # token = None, _tls_client_cert_file = something, _use_cert_as_auth = True connection_args = [ { @@ -111,13 +110,6 @@ def test_auth_args(self, mock_client_class): "http_path": None, "access_token": "tok", }, - { - "server_hostname": "foo", - "http_path": None, - "_username": "foo", - "_password": "bar", - "access_token": None, - }, { "server_hostname": "foo", "http_path": None, diff --git a/tests/unit/test_oauth_persistence.py b/tests/unit/test_oauth_persistence.py index 10677c16..28b3cab3 100644 --- a/tests/unit/test_oauth_persistence.py +++ b/tests/unit/test_oauth_persistence.py @@ -1,8 +1,6 @@ import unittest -from databricks.sql.auth.auth import AccessTokenAuthProvider, BasicAuthProvider, AuthProvider -from databricks.sql.auth.auth import get_python_sql_connector_auth_provider from databricks.sql.experimental.oauth_persistence import DevOnlyFilePersistence, OAuthToken import tempfile import os