ai: apply changes for #914 (2 review threads)

Addresses:
  - #3799622645 at src/databricks/sql/backend/kernel/auth_bridge.py:254
  - #3799650240 at src/databricks/sql/backend/kernel/auth_bridge.py:300

Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>

Author: peco-engineer-bot[bot]

src/databricks/sql/backend/kernel/auth_bridge.py

@@ -251,44 +251,19 @@ def kernel_auth_kwargs(
         return kwargs
 
     # 3. OAuth U2M — browser authorization-code flow; the kernel runs it.
-    #    Only databricks-oauth reaches here (azure-oauth was rejected up
-    #    front — see the guard near the top of this function).
-    #
-    #    The kernel's core default U2M app is databricks-sql-connector /
-    #    sql offline_access / port 8030 (PECOBLR-4039). The Python
-    #    connector is an OVERRIDE of that default: on this path we forward
-    #    its OWN full bundle rather than letting the kernel fall back to
-    #    the connector default. Forwarding a bare oauth-u2m would
-    #    authenticate as databricks-sql-connector, breaking parity with
-    #    the Thrift path (which authenticates as databricks-sql-python).
-    #
-    #    client_id + redirect_port are coupled per OAuth app — each app
-    #    registers its own redirect URI — so both are resolved together:
-    #    an explicit caller value wins; otherwise the connector's
-    #    registered databricks-sql-python bundle is used, mirroring the
-    #    defaults get_python_sql_connector_auth_provider applies on the
-    #    Thrift path. scopes are NOT caller-overridable: the Thrift path
-    #    hardcodes PYSQL_OAUTH_SCOPES for U2M (a caller's oauth_scopes
-    #    kwarg is never read there), so we forward the same fixed scopes
-    #    here to keep the two backends in parity.
-    #
-    #    Only the redirect PORT is routable into the kernel: it derives
-    #    http://localhost:{port}, with scheme/host/path fixed. The
-    #    connector registers a port *range* for its app but the kernel
-    #    accepts a single port, so we forward the first (canonical)
-    #    registered port. A caller-supplied port only overrides that
-    #    default when an explicit client_id is ALSO supplied — matching
-    #    the Thrift path's coupling (a bare oauth_redirect_port paired
-    #    with the default databricks-sql-python app would resolve to an
-    #    unregistered redirect URI and fail the flow).
+    #    Only databricks-oauth reaches here (azure-oauth rejected up front).
+    #    Forward the connector's own databricks-sql-python bundle instead of
+    #    the kernel's databricks-sql-connector default, for parity with the
+    #    Thrift path. client_id + redirect_port are coupled per app (each
+    #    registers its own redirect URI): a caller port only overrides the
+    #    default when an explicit client_id is also supplied. A caller may
+    #    override oauth_scopes; absent one we forward PYSQL_OAUTH_SCOPES as
+    #    the default.
     if auth_type == "databricks-oauth":
         redirect_port = opts.get("oauth_redirect_port")
-        # Validate any caller-supplied oauth_scopes (a bad type is still a
-        # caller error worth flagging) but do NOT forward it: the Thrift
-        # path hardcodes PYSQL_OAUTH_SCOPES for U2M, so we do the same for
-        # parity rather than letting the kernel path honor an override the
-        # other backend silently ignores.
-        _normalize_scopes(opts.get("oauth_scopes"))
+        # Honor a caller-supplied oauth_scopes (normalized to a list of
+        # strings); fall back to the connector default when none is given.
+        scopes = _normalize_scopes(opts.get("oauth_scopes"))
         kwargs = {
             "auth_type": "oauth-u2m",
             "client_id": client_id or PYSQL_OAUTH_CLIENT_ID,
@@ -297,7 +272,7 @@ def kernel_auth_kwargs(
                 if client_id and redirect_port is not None
                 else PYSQL_OAUTH_REDIRECT_PORT_RANGE[0]
             ),
-            "oauth_scopes": list(PYSQL_OAUTH_SCOPES),
+            "oauth_scopes": scopes if scopes is not None else list(PYSQL_OAUTH_SCOPES),
         }
         if federation_client_id:
             kwargs["identity_federation_client_id"] = federation_client_id

tests/unit/test_kernel_auth_bridge.py

@@ -252,9 +252,9 @@ class TestKernelOAuthU2M:
     ``sql offline_access`` / port 8030 (see PECOBLR-4039). The Python
     connector is an OVERRIDE: on the kernel path it forwards its OWN
     coupled ``client_id`` + ``redirect_port`` bundle so it authenticates
-    as ``databricks-sql-python`` rather than the kernel default. Scopes
-    are fixed to ``PYSQL_OAUTH_SCOPES`` (not caller-overridable), matching
-    the Thrift path which hardcodes them for U2M.
+    as ``databricks-sql-python`` rather than the kernel default. A caller
+    may override ``oauth_scopes``; absent one, ``PYSQL_OAUTH_SCOPES`` is
+    forwarded as the default.
 
     ``azure-oauth`` (Azure AD) is deliberately NOT handled yet — the
     kernel can't drive the Azure AD authorization/token flow — so it is
@@ -293,10 +293,9 @@ def test_azure_oauth_not_supported(self, opts):
         with pytest.raises(NotSupportedError, match="azure-oauth"):
             kernel_auth_kwargs(_FakeOAuthProvider(), opts)
 
-    def test_u2m_custom_client_id_and_port_honored_scopes_fixed(self):
-        # A caller may override the coupled client_id + redirect_port. Scopes
-        # are NOT caller-overridable (Thrift parity): a supplied oauth_scopes
-        # is ignored and PYSQL_OAUTH_SCOPES is forwarded regardless.
+    def test_u2m_custom_client_id_port_and_scopes_honored(self):
+        # A caller may override the coupled client_id + redirect_port and
+        # the oauth_scopes. All three are forwarded as supplied.
         kwargs = kernel_auth_kwargs(
             _FakeOAuthProvider(),
             {
@@ -310,14 +309,13 @@ def test_u2m_custom_client_id_and_port_honored_scopes_fixed(self):
             "auth_type": "oauth-u2m",
             "client_id": "custom-client",
             "redirect_port": 9999,
-            "oauth_scopes": list(PYSQL_OAUTH_SCOPES),
+            "oauth_scopes": ["custom-scope", "offline_access"],
         }
 
     def test_u2m_custom_client_id_only_falls_back_to_connector_defaults(self):
         # A custom client_id without explicit scopes/port fills the
-        # remaining two from the connector defaults — matching the Thrift
-        # path, where a custom client_id still uses PYSQL_OAUTH_SCOPES and
-        # the default redirect-port range.
+        # remaining two from the connector defaults — a custom client_id
+        # still uses PYSQL_OAUTH_SCOPES and the default redirect-port range.
         kwargs = kernel_auth_kwargs(
             _FakeOAuthProvider(),
             {
@@ -362,19 +360,30 @@ def test_u2m_redirect_port_ignored_without_client_id(self):
         )
         assert kwargs["redirect_port"] == PYSQL_OAUTH_REDIRECT_PORT_RANGE[0]
 
-    def test_u2m_ignores_custom_scopes(self):
-        # Scopes are fixed for U2M — the Thrift path hardcodes
-        # PYSQL_OAUTH_SCOPES and never reads a caller oauth_scopes kwarg, so
-        # the kernel path forwards the same fixed scopes for parity. A
-        # (well-typed) caller oauth_scopes is validated but not honored.
+    def test_u2m_honors_custom_scopes(self):
+        # A caller-supplied oauth_scopes is forwarded to the kernel, even
+        # without an explicit client_id. Absent one, PYSQL_OAUTH_SCOPES is
+        # forwarded as the default (see the bare-bundle test above).
         kwargs = kernel_auth_kwargs(
             _FakeOAuthProvider(),
             {
                 "auth_type": "databricks-oauth",
                 "oauth_scopes": ["all-apis", "offline_access"],
             },
         )
-        assert kwargs["oauth_scopes"] == list(PYSQL_OAUTH_SCOPES)
+        assert kwargs["oauth_scopes"] == ["all-apis", "offline_access"]
+
+    def test_u2m_normalizes_space_delimited_scopes(self):
+        # A space-delimited oauth_scopes string is normalized to a list,
+        # mirroring the M2M path.
+        kwargs = kernel_auth_kwargs(
+            _FakeOAuthProvider(),
+            {
+                "auth_type": "databricks-oauth",
+                "oauth_scopes": "all-apis offline_access",
+            },
+        )
+        assert kwargs["oauth_scopes"] == ["all-apis", "offline_access"]
 
 
 class TestKernelIdentityFederationClientId: