SNOW-2057503 allow only whitelisted schemes for OAuth url parameters (#2292)

Author: sfc-gh-mmishchenko

src/snowflake/connector/connection.py

@@ -93,6 +93,7 @@
     ER_INVALID_WIF_SETTINGS,
     ER_NO_ACCOUNT_NAME,
     ER_NO_CLIENT_ID,
+    ER_NO_CLIENT_SECRET,
     ER_NO_NUMPY,
     ER_NO_PASSWORD,
     ER_NO_USER,
@@ -1204,7 +1205,7 @@ def __open_connection(self):
                 )
             elif self._authenticator == OAUTH_AUTHORIZATION_CODE:
                 self._check_experimental_authentication_flag()
-                self._check_oauth_required_parameters()
+                self._check_oauth_parameters()
                 if self._role and (self._oauth_scope == ""):
                     # if role is known then let's inject it into scope
                     self._oauth_scope = _OAUTH_DEFAULT_SCOPE.format(role=self._role)
@@ -1232,7 +1233,7 @@ def __open_connection(self):
                 )
             elif self._authenticator == OAUTH_CLIENT_CREDENTIALS:
                 self._check_experimental_authentication_flag()
-                self._check_oauth_required_parameters()
+                self._check_oauth_parameters()
                 if self._role and (self._oauth_scope == ""):
                     # if role is known then let's inject it into scope
                     self._oauth_scope = _OAUTH_DEFAULT_SCOPE.format(role=self._role)
@@ -2237,7 +2238,7 @@ def _check_experimental_authentication_flag(self) -> None:
                 },
             )
 
-    def _check_oauth_required_parameters(self) -> None:
+    def _check_oauth_parameters(self) -> None:
         if self._oauth_client_id is None:
             Error.errorhandler_wrapper(
                 self,
@@ -2255,6 +2256,32 @@ def _check_oauth_required_parameters(self) -> None:
                 ProgrammingError,
                 {
                     "msg": "Oauth code flow requirement 'client_secret' is empty",
-                    "errno": ER_NO_CLIENT_ID,
+                    "errno": ER_NO_CLIENT_SECRET,
+                },
+            )
+        if (
+            self._oauth_authorization_url
+            and not self._oauth_authorization_url.startswith("https://")
+        ):
+            Error.errorhandler_wrapper(
+                self,
+                None,
+                ProgrammingError,
+                {
+                    "msg": "OAuth supports only authorization urls that use 'https' scheme",
+                    "errno": ER_INVALID_VALUE,
+                },
+            )
+        if self._oauth_redirect_uri and not (
+            self._oauth_redirect_uri.startswith("http://")
+            or self._oauth_redirect_uri.startswith("https://")
+        ):
+            Error.errorhandler_wrapper(
+                self,
+                None,
+                ProgrammingError,
+                {
+                    "msg": "OAuth supports only authorization urls that use 'http(s)' scheme",
+                    "errno": ER_INVALID_VALUE,
                 },
             )

src/snowflake/connector/errorcode.py

@@ -34,6 +34,7 @@
 ER_INVALID_WIF_SETTINGS = 251017
 ER_WIF_CREDENTIALS_NOT_FOUND = 251018
 ER_EXPERIMENTAL_AUTHENTICATION_NOT_SUPPORTED = 251019
+ER_NO_CLIENT_SECRET = 251020
 
 # cursor
 ER_FAILED_TO_REWRITE_MULTI_ROW_INSERT = 252001

test/unit/test_oauth_token.py

@@ -119,6 +119,15 @@ def remove(self, key: TokenKey) -> None:
         yield tmp_cache
 
 
+@pytest.fixture()
+def omit_oauth_urls_check():
+    with mock.patch(
+        "snowflake.connector.SnowflakeConnection._check_oauth_parameters",
+        return_value=None,
+    ):
+        yield
+
+
 @pytest.mark.skipolddriver
 @patch("snowflake.connector.auth._http_server.AuthHttpServer.DEFAULT_TIMEOUT", 30)
 def test_oauth_code_successful_flow(
@@ -127,6 +136,7 @@ def test_oauth_code_successful_flow(
     wiremock_generic_mappings_dir,
     webbrowser_mock,
     monkeypatch,
+    omit_oauth_urls_check,
 ) -> None:
     monkeypatch.setenv("SF_ENABLE_EXPERIMENTAL_AUTHENTICATION", "true")
     monkeypatch.setenv("SNOWFLAKE_AUTH_SOCKET_REUSE_PORT", "true")
@@ -169,6 +179,7 @@ def test_oauth_code_invalid_state(
     wiremock_oauth_authorization_code_dir,
     webbrowser_mock,
     monkeypatch,
+    omit_oauth_urls_check,
 ) -> None:
     monkeypatch.setenv("SF_ENABLE_EXPERIMENTAL_AUTHENTICATION", "true")
     monkeypatch.setenv("SNOWFLAKE_AUTH_SOCKET_REUSE_PORT", "true")
@@ -241,6 +252,7 @@ def test_oauth_code_token_request_error(
     wiremock_oauth_authorization_code_dir,
     webbrowser_mock,
     monkeypatch,
+    omit_oauth_urls_check,
 ) -> None:
     monkeypatch.setenv("SF_ENABLE_EXPERIMENTAL_AUTHENTICATION", "true")
     monkeypatch.setenv("SNOWFLAKE_AUTH_SOCKET_REUSE_PORT", "true")
@@ -279,6 +291,7 @@ def test_oauth_code_browser_timeout(
     wiremock_oauth_authorization_code_dir,
     webbrowser_mock,
     monkeypatch,
+    omit_oauth_urls_check,
 ) -> None:
     monkeypatch.setenv("SF_ENABLE_EXPERIMENTAL_AUTHENTICATION", "true")
     monkeypatch.setenv("SNOWFLAKE_AUTH_SOCKET_REUSE_PORT", "true")
@@ -320,6 +333,7 @@ def test_oauth_code_custom_urls(
     wiremock_generic_mappings_dir,
     webbrowser_mock,
     monkeypatch,
+    omit_oauth_urls_check,
 ) -> None:
     monkeypatch.setenv("SF_ENABLE_EXPERIMENTAL_AUTHENTICATION", "true")
     monkeypatch.setenv("SNOWFLAKE_AUTH_SOCKET_REUSE_PORT", "true")
@@ -363,6 +377,7 @@ def test_oauth_code_successful_refresh_token_flow(
     wiremock_generic_mappings_dir,
     monkeypatch,
     temp_cache,
+    omit_oauth_urls_check,
 ) -> None:
     monkeypatch.setenv("SF_ENABLE_EXPERIMENTAL_AUTHENTICATION", "true")
     monkeypatch.setenv("SNOWFLAKE_AUTH_SOCKET_REUSE_PORT", "true")
@@ -423,6 +438,7 @@ def test_oauth_code_expired_refresh_token_flow(
     webbrowser_mock,
     monkeypatch,
     temp_cache,
+    omit_oauth_urls_check,
 ) -> None:
     monkeypatch.setenv("SF_ENABLE_EXPERIMENTAL_AUTHENTICATION", "true")
     monkeypatch.setenv("SNOWFLAKE_AUTH_SOCKET_REUSE_PORT", "true")
@@ -526,7 +542,6 @@ def test_client_creds_successful_flow(
             protocol="http",
             role="ANALYST",
             oauth_token_request_url=f"http://{wiremock_client.wiremock_host}:{wiremock_client.wiremock_http_port}/oauth/token-request",
-            oauth_authorization_url=f"http://{wiremock_client.wiremock_host}:{wiremock_client.wiremock_http_port}/oauth/authorize",
             host=wiremock_client.wiremock_host,
             port=wiremock_client.wiremock_http_port,
         )