From 0a8513ed809db39dae51e26bd55f890f04f3d199 Mon Sep 17 00:00:00 2001 From: rjgoyln Date: Thu, 6 Aug 2026 01:54:00 +0800 Subject: [PATCH] Surface keyring failures instead of silently dropping the token MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit airflowctl auth login wrote the API url config and reported success even when the token never reached the keyring: the TypeError handler only converted the error for CLI clients, and every other client kind — plus any unrelated TypeError from the keyring backend — returned as if the save had worked. --- airflow-ctl/src/airflowctl/api/client.py | 5 +++-- .../tests/airflow_ctl/api/test_client.py | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/airflow-ctl/src/airflowctl/api/client.py b/airflow-ctl/src/airflowctl/api/client.py index 51e92d813750f..02c503ad17121 100644 --- a/airflow-ctl/src/airflowctl/api/client.py +++ b/airflow-ctl/src/airflowctl/api/client.py @@ -244,9 +244,10 @@ def save(self, skip_keyring: bool = False): "Use `airflowctl auth login --skip-keyring ...` to dismiss this error." ) from e except TypeError as e: - # This happens when the token is None, which is not allowed by keyring - if self.api_token is None and self.client_kind == ClientKind.CLI: + # keyring rejects a None password; any other TypeError is a real backend failure + if self.api_token is None: raise AirflowCtlCredentialNotFoundException("No API token found. Please login first.") from e + raise def load(self) -> Credentials: """Load the credentials from keyring and URL from disk file.""" diff --git a/airflow-ctl/tests/airflow_ctl/api/test_client.py b/airflow-ctl/tests/airflow_ctl/api/test_client.py index dc33092e78018..416b41abacf9a 100644 --- a/airflow-ctl/tests/airflow_ctl/api/test_client.py +++ b/airflow-ctl/tests/airflow_ctl/api/test_client.py @@ -216,6 +216,25 @@ def test_save_no_keyring(self, mock_keyring): with pytest.raises(AirflowCtlKeyringException, match="Keyring backend is not available"): Credentials(client_kind=cli_client).save() + @pytest.mark.parametrize("client_kind", [ClientKind.AUTH, ClientKind.NO_AUTH, None]) + @patch.dict(os.environ, {"AIRFLOW_CLI_ENVIRONMENT": "TEST_SAVE_NO_TOKEN"}) + @patch("airflowctl.api.client.keyring") + def test_save_without_token_raises_for_non_cli_client_kinds(self, mock_keyring, client_kind): + mock_keyring.set_password.side_effect = TypeError("password must be a string") + + with pytest.raises(AirflowCtlCredentialNotFoundException, match="No API token found"): + Credentials(api_url="http://localhost:8080", client_kind=client_kind).save() + + @patch.dict(os.environ, {"AIRFLOW_CLI_ENVIRONMENT": "TEST_SAVE_TYPE_ERROR"}) + @patch("airflowctl.api.client.keyring") + def test_save_reraises_type_error_unrelated_to_missing_token(self, mock_keyring): + mock_keyring.set_password.side_effect = TypeError("keyring backend blew up") + + with pytest.raises(TypeError, match="keyring backend blew up"): + Credentials( + api_url="http://localhost:8080", api_token="TEST_TOKEN", client_kind=ClientKind.CLI + ).save() + @patch.dict(os.environ, {"AIRFLOW_CLI_ENVIRONMENT": "TEST_SAVE_SKIP_KEYRING"}) @patch("airflowctl.api.client.keyring") def test_save_no_keyring_backend_skip_keyring(self, mock_keyring):