Merge pull request #897 from GitGuardian/salomevoltz/scrt-4543-ggshie…

…ld-auth-login-flow-errors-are-not-handled-when

fix(auth_login): error handling with invalid instance URL

changelog.d/20240527_155007_salome.voltz_scrt_4543_ggshield_auth_login_flow_errors_are_not_handled_when.md

@@ -0,0 +1,3 @@
+### Fixed
+
+- Errors thrown during `ggshield auth login` flow with an invalid instance URL are handled and the stack trace is no longer displayed on the console.

ggshield/cmd/auth/login.py

@@ -2,6 +2,7 @@
 from typing import Any, List, Optional, Tuple
 
 import click
+import requests
 
 from ggshield.cmd.utils.common_options import add_common_options
 from ggshield.cmd.utils.context_obj import ContextObj
@@ -189,7 +190,14 @@ def token_login(config: Config, instance: Optional[str]) -> None:
 
     # enforce using the token (and not use config default)
     client = create_client(api_key=token, api_url=config.api_url)
-    response = client.get(endpoint="token")
+    try:
+        response = client.get(endpoint="token")
+    except requests.exceptions.ConnectionError as e:
+        if "Failed to resolve" in str(e):
+            raise click.UsageError(f"Invalid instance: {instance}.")
+        else:
+            raise UnexpectedError(f"Failed to connect to {instance}.") from e
+
     if not response.ok:
         raise UnexpectedError("Authentication failed with token.")
 

ggshield/core/config/config.py

@@ -13,6 +13,7 @@
     api_to_dashboard_url,
     clean_url,
     dashboard_to_api_url,
+    validate_instance_url,
 )
 
 
@@ -68,6 +69,7 @@ def instance_name(self) -> str:
         except KeyError:
             pass
         else:
+            validate_instance_url(url)
             return remove_url_trailing_slash(url)
 
         try:

ggshield/core/url_utils.py

@@ -24,24 +24,33 @@ def clean_url(url: str, warn: bool = False) -> ParseResult:
     return parsed_url
 
 
-def dashboard_to_api_url(dashboard_url: str, warn: bool = False) -> str:
+def validate_instance_url(url: str, warn: bool = False) -> ParseResult:
     """
-    Convert a dashboard URL to an API URL.
-    handles the SaaS edge case where the host changes instead of the path
+    Validate a dashboard URL
     """
-    parsed_url = clean_url(dashboard_url, warn=warn)
+    parsed_url = clean_url(url, warn=warn)
     if parsed_url.scheme != "https" and not (
         parsed_url.netloc.startswith("localhost")
         or parsed_url.netloc.startswith("127.0.0.1")
     ):
-        raise UsageError(
-            f"Invalid scheme for dashboard URL '{dashboard_url}', expected HTTPS"
-        )
+        raise UsageError(f"Invalid scheme for dashboard URL '{url}', expected HTTPS")
     if any(parsed_url.netloc.endswith("." + domain) for domain in GITGUARDIAN_DOMAINS):
         if parsed_url.path:
             raise UsageError(
-                f"Invalid dashboard URL '{dashboard_url}', got an unexpected path '{parsed_url.path}'"
+                f"Invalid dashboard URL '{url}', got an unexpected path '{parsed_url.path}'"
             )
+
+    return parsed_url
+
+
+def dashboard_to_api_url(dashboard_url: str, warn: bool = False) -> str:
+    """
+    Convert a dashboard URL to an API URL.
+    handles the SaaS edge case where the host changes instead of the path
+    """
+    parsed_url = validate_instance_url(dashboard_url, warn=warn)
+
+    if any(parsed_url.netloc.endswith("." + domain) for domain in GITGUARDIAN_DOMAINS):
         parsed_url = parsed_url._replace(
             netloc=parsed_url.netloc.replace("dashboard", "api")
         )

tests/unit/cmd/auth/test_login.py

@@ -930,3 +930,22 @@ def test_sso_url(
         assert exit_code == ExitCode.SUCCESS, output
         self._webbrowser_open_mock.assert_called()
         self._assert_open_url(host=expected_web_host)
+
+    @pytest.mark.parametrize(
+        "instance_url",
+        [
+            "https://dashboard.gitguardian.com/abc",
+        ],
+    )
+    def test_invalid_instance_url(self, instance_url, cli_fs_runner, monkeypatch):
+        """
+        GIVEN an invalid instance URL
+        WHEN running the login command
+        THEN it fails
+        """
+        monkeypatch.setenv("GITGUARDIAN_INSTANCE", instance_url)
+
+        self.prepare_mocks(monkeypatch)
+        exit_code, output = self.run_cmd(cli_fs_runner)
+        assert exit_code == ExitCode.USAGE_ERROR, output
+        self._webbrowser_open_mock.assert_not_called()

tests/unit/core/config/test_config.py

@@ -6,6 +6,7 @@
 from typing import Optional
 
 import pytest
+from click import UsageError
 
 from ggshield.core.config import AccountConfig, Config, InstanceConfig
 from ggshield.core.config.utils import get_auth_config_filepath, load_yaml_dict
@@ -312,3 +313,19 @@ def test_updating_config_not_from_default_local_config_path(
 
         dct = load_yaml_dict(local_config_path)
         assert dct["instance"] == "https://after.com"
+
+    @pytest.mark.parametrize(
+        "instance_url",
+        ["", "http://api.gitguardian.com/", "https://api.gitguardian.com/abc"],
+    )
+    def test_invalid_instance_url(self, monkeypatch, instance_url):
+        """
+        GIVEN an invalid instance url
+        WHEN loading the config
+        THEN it raises a UsageError
+        """
+
+        monkeypatch.setitem(os.environ, "GITGUARDIAN_API_URL", instance_url)
+        with pytest.raises(UsageError):
+            config = Config()
+            config.instance_name