[PECO-1715] Remove username/password (BasicAuth) auth option (databricks#409)

Signed-off-by: Jacky Hu <[email protected]>

Author: jackyhu-db

src/databricks/sql/auth/auth.py

@@ -4,7 +4,6 @@
 from databricks.sql.auth.authenticators import (
     AuthProvider,
     AccessTokenAuthProvider,
-    BasicAuthProvider,
     ExternalAuthProvider,
     DatabricksOAuthProvider,
 )
@@ -13,16 +12,14 @@
 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
 
 
 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,

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):

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

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))

tests/unit/test_client.py

@@ -103,21 +103,13 @@ 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 = [
             {
                 "server_hostname": "foo",
                 "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,

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