From 1f58abdb16418640712de3fc3b07f1afd809229c Mon Sep 17 00:00:00 2001 From: Baptiste Gouhoury Date: Thu, 29 Oct 2026 08:16:20 +0100 Subject: [PATCH 1/5] Add browser login to airflowctl Users who sign in to Airflow through SSO or with a second factor have no password to give airflowctl, so they could only log in by copying a token from somewhere else. Auth managers can now advertise a browser login through the CLI login discovery route, and airflowctl can use it without knowing which auth manager serves it. Servers without discovery, or whose auth manager does not offer browser login, fail with a clear message instead of a silent wait for a browser redirect that never comes. Generated-by: Claude Code (Opus 5.5) Signed-off-by: Baptiste Gouhoury --- airflow-ctl/docs/images/command_hashes.txt | 2 +- airflow-ctl/docs/images/output_auth_login.svg | 140 +++++++++++------- airflow-ctl/docs/start.rst | 11 ++ airflow-ctl/src/airflowctl/api/operations.py | 36 +++++ airflow-ctl/src/airflowctl/ctl/cli_config.py | 33 ++++- .../airflowctl/ctl/commands/auth_command.py | 59 +++++++- .../src/airflowctl/ctl/utils/browser_login.py | 133 +++++++++++++++++ airflow-ctl/src/airflowctl/exceptions.py | 4 + .../tests/airflow_ctl/api/test_operations.py | 69 ++++++++- .../ctl/commands/test_auth_command.py | 106 ++++++++++++- .../tests/airflow_ctl/ctl/utils/__init__.py | 16 ++ .../ctl/utils/test_browser_login.py | 136 +++++++++++++++++ 12 files changed, 688 insertions(+), 57 deletions(-) create mode 100644 airflow-ctl/src/airflowctl/ctl/utils/browser_login.py create mode 100644 airflow-ctl/tests/airflow_ctl/ctl/utils/__init__.py create mode 100644 airflow-ctl/tests/airflow_ctl/ctl/utils/test_browser_login.py diff --git a/airflow-ctl/docs/images/command_hashes.txt b/airflow-ctl/docs/images/command_hashes.txt index 830500bf60b69..30ebffe3e6975 100644 --- a/airflow-ctl/docs/images/command_hashes.txt +++ b/airflow-ctl/docs/images/command_hashes.txt @@ -14,4 +14,4 @@ tasks:eb70701dfe1b9baeda39d5eea07fde5f variables:f8fc76d3d398b2780f4e97f7cd816646 version:31f4efdf8de0dbaaa4fac71ff7efecc3 plugins:4864fd8f356704bd2b3cd1aec3567e35 -auth login:963355e38ab1c133f2cf9495d00d99f6 +auth login:e068cc2df6160c58b93ef21b3d51abca diff --git a/airflow-ctl/docs/images/output_auth_login.svg b/airflow-ctl/docs/images/output_auth_login.svg index f722fb20eb3d3..ebc2556c4921d 100644 --- a/airflow-ctl/docs/images/output_auth_login.svg +++ b/airflow-ctl/docs/images/output_auth_login.svg @@ -1,4 +1,4 @@ - + - - + + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + + + + + + + + + + + + + + + + + + + + + + + + + + + + - + - + - - Usage:airflowctl auth login [-h] [--api-tokenAPI_TOKEN] -                             [--api-urlAPI_URL] [-eENV] -                             [--passwordPASSWORD] [--skip-keyring] -                             [--usernameUSERNAME] - -Log in to the Airflow API server and store the credentials for the selected -environment. Pass --api-token (or set AIRFLOW_CLI_TOKEN), or --username and ---password; missing credentials are prompted for when running in a  -terminal. - -Options: --h, --helpshow this help message and exit ---api-tokenAPI_TOKEN -The token to use for authentication ---api-urlAPI_URLThe URL of the Airflow API server --e, --envENVThe environment to run the command in ---passwordPASSWORDThe password to use for authentication ---skip-keyringSkip storing credentials in keyring ---usernameUSERNAMEThe username to use for authentication - + + Usage:airflowctl auth login [-h] [--api-tokenAPI_TOKEN] +                             [--api-urlAPI_URL] [-eENV] [--no-browser] +                             [--passwordPASSWORD] [--skip-keyring] +                             [--sso] [--timeoutTIMEOUT] +                             [--usernameUSERNAME] + +Log in to the Airflow API server and store the credentials for the selected +environment. Pass --api-token (or set AIRFLOW_CLI_TOKEN), or --username and +--password; missing credentials are prompted for when running in a  +terminal. Pass --sso to log in through the browser when the API server  +offers it. + +Options: +-h, --helpshow this help message and exit +--api-tokenAPI_TOKEN +The token to use for authentication +--api-urlAPI_URLThe URL of the Airflow API server +-e, --envENVThe environment to run the command in +--no-browserWith --sso, print the login URL instead of opening  +a browser. The browser must still reach this machine on 127.0.0.1, for  +example through 'ssh -L' when logged in remotely +--passwordPASSWORDThe password to use for authentication +--skip-keyringSkip storing credentials in keyring +--ssoLog in through the browser with the identity  +provider of the API server, if it offers it +--timeoutTIMEOUTWith --sso, seconds to wait for the browser login  +to complete +--usernameUSERNAMEThe username to use for authentication + diff --git a/airflow-ctl/docs/start.rst b/airflow-ctl/docs/start.rst index fc879092c8c48..d52f55527f938 100644 --- a/airflow-ctl/docs/start.rst +++ b/airflow-ctl/docs/start.rst @@ -44,6 +44,17 @@ OR export AIRFLOW_CLI_TOKEN= airflowctl auth login --api-url --env +OR, when the auth manager of the API server offers browser login (for example to sign in through SSO): + +.. code-block:: bash + + airflowctl auth login --sso --api-url --env + +This opens the login page of the API server in your browser and waits for the login to complete. +The browser redirects back to airflowctl on ``127.0.0.1``, so it must run on the same machine. +When logged in remotely, pass ``--no-browser`` to only print the login URL, and forward the port +of the printed ``redirect_uri`` from your local machine, for example with ``ssh -L``. + This command will create a new airflowctl environment with the specified username and password. You can then use the following command to start the airflowctl environment: diff --git a/airflow-ctl/src/airflowctl/api/operations.py b/airflow-ctl/src/airflowctl/api/operations.py index 8c62ed3d98e03..4d11a656bf7a2 100644 --- a/airflow-ctl/src/airflowctl/api/operations.py +++ b/airflow-ctl/src/airflowctl/api/operations.py @@ -48,6 +48,7 @@ BulkBodyVariableBody, BulkResponse, ClearTaskInstancesBody, + CliLoginMetadataResponse, Config, ConnectionBody, ConnectionCollectionResponse, @@ -346,6 +347,41 @@ def login_with_username_and_password(self, login: LoginBody) -> LoginResponse | self.client.post("/token/cli", json=login.model_dump(mode="json")).content ) + def get_cli_login_metadata(self, api_url: str) -> CliLoginMetadataResponse: + """Return the login methods the API server offers, or none for servers without CLI login discovery.""" + try: + response = self.client.get( + f"{api_url.rstrip('/')}/api/v2/auth/cli/metadata", + extensions={"airflowctl_suppress_error_log": True}, + ) + except httpx.HTTPStatusError as e: + if e.response.status_code != 404: + raise + return CliLoginMetadataResponse( + grant_types_supported=[], + code_challenge_methods_supported=[], + authorization_endpoint=None, + device_authorization_endpoint=None, + token_endpoint=None, + ) + return CliLoginMetadataResponse.model_validate_json(response.content) + + def login_with_authorization_code( + self, token_url: str, code: str, code_verifier: str, redirect_uri: str + ) -> LoginResponse: + """Redeem an authorization code from the browser login for a CLI token.""" + return LoginResponse.model_validate_json( + self.client.post( + token_url, + json={ + "grant_type": "authorization_code", + "code": code, + "code_verifier": code_verifier, + "redirect_uri": redirect_uri, + }, + ).content + ) + # Operations class AssetsOperations(BaseOperations): diff --git a/airflow-ctl/src/airflowctl/ctl/cli_config.py b/airflow-ctl/src/airflowctl/ctl/cli_config.py index 69fda36c76e2f..8d0e8de9294a8 100755 --- a/airflow-ctl/src/airflowctl/ctl/cli_config.py +++ b/airflow-ctl/src/airflowctl/ctl/cli_config.py @@ -344,6 +344,30 @@ def _load_help_texts_yaml() -> dict[str, dict[str, str]]: dest="password", help="The password to use for authentication", ) +ARG_AUTH_SSO = Arg( + flags=("--sso",), + dest="sso", + default=False, + action="store_true", + help="Log in through the browser with the identity provider of the API server, if it offers it", +) +ARG_AUTH_NO_BROWSER = Arg( + flags=("--no-browser",), + dest="no_browser", + default=False, + action="store_true", + help=( + "With --sso, print the login URL instead of opening a browser. The browser must still reach " + "this machine on 127.0.0.1, for example through 'ssh -L' when logged in remotely" + ), +) +ARG_AUTH_TIMEOUT = Arg( + flags=("--timeout",), + type=float, + default=300, + dest="timeout", + help="With --sso, seconds to wait for the browser login to complete", +) # Dag Commands Args ARG_DAG_ID = Arg( @@ -1238,7 +1262,8 @@ def merge_commands( description=( "Log in to the Airflow API server and store the credentials for the selected environment. " "Pass --api-token (or set AIRFLOW_CLI_TOKEN), or --username and --password; " - "missing credentials are prompted for when running in a terminal." + "missing credentials are prompted for when running in a terminal. " + "Pass --sso to log in through the browser when the API server offers it." ), func=lazy_load_command("airflowctl.ctl.commands.auth_command.login"), args=( @@ -1247,6 +1272,9 @@ def merge_commands( ARG_AUTH_USERNAME, ARG_AUTH_PASSWORD, ARG_AUTH_SKIP_KEYRING, + ARG_AUTH_SSO, + ARG_AUTH_NO_BROWSER, + ARG_AUTH_TIMEOUT, ), ), ActionCommand( @@ -1268,6 +1296,9 @@ def merge_commands( ARG_AUTH_URL, ARG_AUTH_USERNAME, ARG_AUTH_PASSWORD, + ARG_AUTH_SSO, + ARG_AUTH_NO_BROWSER, + ARG_AUTH_TIMEOUT, ), ), ) diff --git a/airflow-ctl/src/airflowctl/ctl/commands/auth_command.py b/airflow-ctl/src/airflowctl/ctl/commands/auth_command.py index cf521cbe7eea0..1eb49ad2f40a2 100644 --- a/airflow-ctl/src/airflowctl/ctl/commands/auth_command.py +++ b/airflow-ctl/src/airflowctl/ctl/commands/auth_command.py @@ -23,14 +23,43 @@ import json import os import sys +from urllib.parse import urljoin import keyring import rich from keyring.errors import NoKeyringError -from airflowctl.api.client import NEW_API_CLIENT, ClientKind, Credentials, provide_api_client +from airflowctl.api.client import NEW_API_CLIENT, Client, ClientKind, Credentials, provide_api_client from airflowctl.api.datamodels.auth_generated import LoginBody from airflowctl.ctl.console_formatting import AirflowConsole +from airflowctl.ctl.utils.browser_login import get_authorization_code +from airflowctl.exceptions import AirflowCtlBrowserLoginException + + +def _get_token_with_browser(args, api_client: Client) -> str: + """Log in through the browser and return the CLI token, if the API server offers it.""" + api_client.refresh_base_url(base_url=args.api_url, kind=ClientKind.AUTH) + metadata = api_client.login.get_cli_login_metadata(args.api_url) + if ( + "authorization_code" not in metadata.grant_types_supported + or "S256" not in metadata.code_challenge_methods_supported + or not metadata.authorization_endpoint + or not metadata.token_endpoint + ): + raise AirflowCtlBrowserLoginException(f"{args.api_url} does not offer browser login (--sso).") + + authorization_code = get_authorization_code( + api_url=args.api_url, + authorization_endpoint=metadata.authorization_endpoint, + open_browser=not args.no_browser, + timeout=args.timeout, + ) + return api_client.login.login_with_authorization_code( + urljoin(args.api_url, metadata.token_endpoint), + code=authorization_code.code, + code_verifier=authorization_code.code_verifier, + redirect_uri=authorization_code.redirect_uri, + ).access_token @provide_api_client(kind=ClientKind.AUTH) @@ -42,6 +71,26 @@ def login(args, api_client=NEW_API_CLIENT) -> None: password = args.password token = args.api_token or os.environ.get("AIRFLOW_CLI_TOKEN") + if args.sso: + if args.api_token or username or password or args.skip_keyring: + rich.print( + "[red]--sso is not compatible with --api-token, --username, --password or --skip-keyring." + ) + sys.exit(1) + try: + api_token = _get_token_with_browser(args, api_client) + except Exception as e: + rich.print(f"[red]Login failed: {e}") + sys.exit(1) + Credentials( + api_url=args.api_url, + api_token=api_token, + api_environment=args.env, + client_kind=ClientKind.AUTH, + ).save() + rich.print(success_message) + return + # If credentials are incomplete, prompt interactively on a real terminal if not token and not (username and password): if not sys.stdin.isatty(): @@ -107,6 +156,14 @@ def login(args, api_client=NEW_API_CLIENT) -> None: @provide_api_client(kind=ClientKind.AUTH) def get_token(args, api_client=NEW_API_CLIENT) -> None: """Generate and print a JWT token for the given credentials to stdout.""" + if args.sso: + try: + print(_get_token_with_browser(args, api_client)) + except Exception as e: + rich.print(f"[red]Token generation failed: {e}[/red]") + sys.exit(1) + return + username = args.username or input("Username: ") password = args.password or getpass.getpass("Password: ") diff --git a/airflow-ctl/src/airflowctl/ctl/utils/browser_login.py b/airflow-ctl/src/airflowctl/ctl/utils/browser_login.py new file mode 100644 index 0000000000000..7a687b7cf8152 --- /dev/null +++ b/airflow-ctl/src/airflowctl/ctl/utils/browser_login.py @@ -0,0 +1,133 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""Browser login with the authorization code flow and PKCE (RFC 7636), over a loopback redirect (RFC 8252).""" + +from __future__ import annotations + +import base64 +import hashlib +import secrets +import time +import webbrowser +from dataclasses import dataclass +from http.server import BaseHTTPRequestHandler, HTTPServer +from urllib.parse import parse_qs, urlencode, urljoin, urlsplit + +import rich + +from airflowctl.exceptions import AirflowCtlBrowserLoginException + +CALLBACK_PATH = "/callback" +_CALLBACK_PAGE = ( + b"airflowctl" + b"

Login complete. You can close this window and return to the terminal.

" +) + + +@dataclass +class AuthorizationCode: + """Authorization code returned to the loopback redirect, with what the token endpoint needs to redeem it.""" + + code: str + code_verifier: str + redirect_uri: str + + +def build_pkce_pair() -> tuple[str, str]: + """Return a PKCE ``(code_verifier, code_challenge)`` pair for the ``S256`` method.""" + code_verifier = secrets.token_urlsafe(64) + digest = hashlib.sha256(code_verifier.encode("ascii")).digest() + code_challenge = base64.urlsafe_b64encode(digest).rstrip(b"=").decode("ascii") + return code_verifier, code_challenge + + +class _CallbackServer(HTTPServer): + callback_params: dict[str, str] | None = None + + +class _CallbackHandler(BaseHTTPRequestHandler): + server: _CallbackServer + + def do_GET(self) -> None: + url = urlsplit(self.path) + # Browsers also ask for /favicon.ico; only the callback ends the wait. + if url.path != CALLBACK_PATH or self.server.callback_params is not None: + self.send_error(404) + return + self.server.callback_params = {key: values[0] for key, values in parse_qs(url.query).items()} + self.send_response(200) + self.send_header("Content-Type", "text/html; charset=utf-8") + self.end_headers() + self.wfile.write(_CALLBACK_PAGE) + + def log_message(self, format, *args) -> None: + # The default handler logs every request to stderr, including the code in the query string. + pass + + +def get_authorization_code( + *, api_url: str, authorization_endpoint: str, open_browser: bool, timeout: float +) -> AuthorizationCode: + """ + Run the browser part of the login and return the authorization code it produced. + + :param api_url: URL of the Airflow API server + :param authorization_endpoint: ``authorization_endpoint`` from the CLI login metadata + :param open_browser: open the login page in the default browser, otherwise only print its URL + :param timeout: seconds to wait for the user to finish logging in + """ + code_verifier, code_challenge = build_pkce_pair() + state = secrets.token_urlsafe(32) + + with _CallbackServer(("127.0.0.1", 0), _CallbackHandler) as server: + redirect_uri = f"http://127.0.0.1:{server.server_address[1]}{CALLBACK_PATH}" + # Endpoints in the metadata are absolute paths, which keeps any path prefix of the API server. + login_url = ( + urljoin(api_url, authorization_endpoint) + + "?" + + urlencode( + { + "redirect_uri": redirect_uri, + "state": state, + "code_challenge": code_challenge, + "code_challenge_method": "S256", + } + ) + ) + rich.print(f"[blue]Log in from your browser:[/blue] {login_url}") + if open_browser: + webbrowser.open(login_url) + + deadline = time.monotonic() + timeout + while server.callback_params is None: + remaining = deadline - time.monotonic() + if remaining <= 0: + raise AirflowCtlBrowserLoginException(f"No login completed within {timeout:g} seconds.") + server.timeout = remaining + server.handle_request() + params = server.callback_params + + if not secrets.compare_digest(params.get("state", ""), state): + raise AirflowCtlBrowserLoginException("The login response does not match this login attempt.") + if "error" in params: + raise AirflowCtlBrowserLoginException( + f"Login refused: {params.get('error_description') or params['error']}" + ) + if not params.get("code"): + raise AirflowCtlBrowserLoginException("The login response contains no authorization code.") + return AuthorizationCode(code=params["code"], code_verifier=code_verifier, redirect_uri=redirect_uri) diff --git a/airflow-ctl/src/airflowctl/exceptions.py b/airflow-ctl/src/airflowctl/exceptions.py index 0af8b9fa593cc..835167f61815b 100644 --- a/airflow-ctl/src/airflowctl/exceptions.py +++ b/airflow-ctl/src/airflowctl/exceptions.py @@ -44,3 +44,7 @@ class AirflowCtlConnectionException(AirflowCtlException): class AirflowCtlKeyringException(AirflowCtlException): """Raise when a keyring error occurs while performing an operation.""" + + +class AirflowCtlBrowserLoginException(AirflowCtlException): + """Raise when the browser login of ``auth login --sso`` cannot complete.""" diff --git a/airflow-ctl/tests/airflow_ctl/api/test_operations.py b/airflow-ctl/tests/airflow_ctl/api/test_operations.py index 91ebe43b17bcf..28593aa60c29d 100644 --- a/airflow-ctl/tests/airflow_ctl/api/test_operations.py +++ b/airflow-ctl/tests/airflow_ctl/api/test_operations.py @@ -117,7 +117,7 @@ XComResponse, XComResponseNative, ) -from airflowctl.api.operations import BaseOperations, _build_query_params, _tolerant_model +from airflowctl.api.operations import BaseOperations, ServerResponseError, _build_query_params, _tolerant_model from airflowctl.exceptions import AirflowCtlConnectionException if TYPE_CHECKING: @@ -2780,6 +2780,73 @@ def handle_request(request: httpx.Request) -> httpx.Response: ) assert response.access_token == "NO_TOKEN" + def test_get_cli_login_metadata(self): + metadata = { + "grant_types_supported": ["password", "authorization_code"], + "code_challenge_methods_supported": ["S256"], + "authorization_endpoint": "/auth/login/cli", + "device_authorization_endpoint": None, + "token_endpoint": "/auth/token/cli", + } + + def handle_request(request: httpx.Request) -> httpx.Response: + assert request.url.path == "/prefix/api/v2/auth/cli/metadata" + return httpx.Response(200, json=metadata) + + client = make_api_client(transport=httpx.MockTransport(handle_request), kind=ClientKind.AUTH) + + response = client.login.get_cli_login_metadata("http://server/prefix/") + + assert response.model_dump() == metadata + + @pytest.mark.parametrize( + "response", + [ + pytest.param(httpx.Response(404, json={"detail": "Not Found"}), id="json"), + pytest.param(httpx.Response(404, text="Not Found"), id="text"), + ], + ) + def test_get_cli_login_metadata_without_discovery(self, response): + client = make_api_client( + transport=httpx.MockTransport(lambda request: response), kind=ClientKind.AUTH + ) + + metadata = client.login.get_cli_login_metadata("http://server") + + assert metadata.grant_types_supported == [] + assert metadata.token_endpoint is None + + def test_get_cli_login_metadata_server_error(self): + client = make_api_client( + transport=httpx.MockTransport(lambda request: httpx.Response(403, json={"detail": "Forbidden"})), + kind=ClientKind.AUTH, + ) + + with pytest.raises(ServerResponseError): + client.login.get_cli_login_metadata("http://server") + + def test_login_with_authorization_code(self): + def handle_request(request: httpx.Request) -> httpx.Response: + assert str(request.url) == "http://server/auth/token/cli" + assert json.loads(request.content) == { + "grant_type": "authorization_code", + "code": "code", + "code_verifier": "verifier", + "redirect_uri": "http://127.0.0.1:1234/callback", + } + return httpx.Response(201, json={"access_token": "TOKEN"}) + + client = make_api_client(transport=httpx.MockTransport(handle_request), kind=ClientKind.AUTH) + + response = client.login.login_with_authorization_code( + "http://server/auth/token/cli", + code="code", + code_verifier="verifier", + redirect_uri="http://127.0.0.1:1234/callback", + ) + + assert response.access_token == "TOKEN" + class TestXComOperations: """Test suite for XCom operations.""" diff --git a/airflow-ctl/tests/airflow_ctl/ctl/commands/test_auth_command.py b/airflow-ctl/tests/airflow_ctl/ctl/commands/test_auth_command.py index 9f442f40b9016..7a9a6150cbdc9 100644 --- a/airflow-ctl/tests/airflow_ctl/ctl/commands/test_auth_command.py +++ b/airflow-ctl/tests/airflow_ctl/ctl/commands/test_auth_command.py @@ -22,12 +22,41 @@ from unittest import mock from unittest.mock import patch +import httpx import pytest -from airflowctl.api.client import ClientKind +from airflowctl.api.client import Client, ClientKind from airflowctl.api.datamodels.auth_generated import LoginResponse from airflowctl.ctl import cli_parser from airflowctl.ctl.commands import auth_command +from airflowctl.ctl.utils.browser_login import AuthorizationCode + +SSO_METADATA = { + "grant_types_supported": ["password", "refresh_token", "authorization_code"], + "code_challenge_methods_supported": ["S256"], + "authorization_endpoint": "/auth/login/cli", + "device_authorization_endpoint": None, + "token_endpoint": "/auth/token/cli", +} +AUTHORIZATION_CODE = AuthorizationCode( + code="code", code_verifier="verifier", redirect_uri="http://127.0.0.1:1234/callback" +) + + +def _make_sso_api_client(metadata: dict) -> Client: + def handle_request(request: httpx.Request) -> httpx.Response: + if request.url.path == "/api/v2/auth/cli/metadata": + return httpx.Response(200, json=metadata) + assert str(request.url) == "http://localhost:8080/auth/token/cli" + assert json.loads(request.content)["code_verifier"] == "verifier" + return httpx.Response(201, json={"access_token": "SSO_TOKEN"}) + + return Client( + base_url="test://server", + transport=httpx.MockTransport(handle_request), + token="", + kind=ClientKind.AUTH, + ) class TestCliAuthCommands: @@ -259,6 +288,81 @@ def test_login_with_username_and_password_no_keyring_backend(self, mock_keyring, class TestListEnvs: parser = cli_parser.get_parser() + @patch("airflowctl.ctl.commands.auth_command.get_authorization_code", autospec=True) + @patch("airflowctl.api.client.keyring") + def test_login_with_sso(self, mock_keyring, mock_get_authorization_code): + mock_get_authorization_code.return_value = AUTHORIZATION_CODE + mock_keyring.get_password.return_value = None + + auth_command.login( + self.parser.parse_args( + [ + "auth", + "login", + "--api-url", + "http://localhost:8080", + "--sso", + "--no-browser", + "--timeout", + "30", + ] + ), + api_client=_make_sso_api_client(SSO_METADATA), + ) + + mock_get_authorization_code.assert_called_once_with( + api_url="http://localhost:8080", + authorization_endpoint="/auth/login/cli", + open_browser=False, + timeout=30, + ) + mock_keyring.set_password.assert_called_once_with("airflowctl", "api_token_production", "SSO_TOKEN") + + @pytest.mark.parametrize( + "metadata", + [ + pytest.param({**SSO_METADATA, "grant_types_supported": ["password"]}, id="no-authorization-code"), + pytest.param({**SSO_METADATA, "code_challenge_methods_supported": ["plain"]}, id="no-s256"), + pytest.param({**SSO_METADATA, "authorization_endpoint": None}, id="no-authorization-endpoint"), + ], + ) + @patch("airflowctl.ctl.commands.auth_command.get_authorization_code", autospec=True) + def test_login_with_sso_not_offered(self, mock_get_authorization_code, metadata): + with pytest.raises(SystemExit): + auth_command.login( + self.parser.parse_args(["auth", "login", "--api-url", "http://localhost:8080", "--sso"]), + api_client=_make_sso_api_client(metadata), + ) + + mock_get_authorization_code.assert_not_called() + + @pytest.mark.parametrize( + "extra_args", + [["--username", "user"], ["--password", "pass"], ["--api-token", "token"], ["--skip-keyring"]], + ) + @patch("airflowctl.ctl.commands.auth_command.get_authorization_code", autospec=True) + def test_login_with_sso_incompatible_args(self, mock_get_authorization_code, extra_args): + with pytest.raises(SystemExit): + auth_command.login( + self.parser.parse_args( + ["auth", "login", "--api-url", "http://localhost:8080", "--sso", *extra_args] + ), + api_client=_make_sso_api_client(SSO_METADATA), + ) + + mock_get_authorization_code.assert_not_called() + + @patch("airflowctl.ctl.commands.auth_command.get_authorization_code", autospec=True) + def test_get_token_with_sso(self, mock_get_authorization_code, capsys): + mock_get_authorization_code.return_value = AUTHORIZATION_CODE + + auth_command.get_token( + self.parser.parse_args(["auth", "token", "--api-url", "http://localhost:8080", "--sso"]), + api_client=_make_sso_api_client(SSO_METADATA), + ) + + assert capsys.readouterr().out == "SSO_TOKEN\n" + def test_list_envs_empty_airflow_home(self, monkeypatch): """Test list-envs with no AIRFLOW_HOME directory.""" with ( diff --git a/airflow-ctl/tests/airflow_ctl/ctl/utils/__init__.py b/airflow-ctl/tests/airflow_ctl/ctl/utils/__init__.py new file mode 100644 index 0000000000000..13a83393a9124 --- /dev/null +++ b/airflow-ctl/tests/airflow_ctl/ctl/utils/__init__.py @@ -0,0 +1,16 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. diff --git a/airflow-ctl/tests/airflow_ctl/ctl/utils/test_browser_login.py b/airflow-ctl/tests/airflow_ctl/ctl/utils/test_browser_login.py new file mode 100644 index 0000000000000..92eb2fe1423ac --- /dev/null +++ b/airflow-ctl/tests/airflow_ctl/ctl/utils/test_browser_login.py @@ -0,0 +1,136 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from __future__ import annotations + +import base64 +import hashlib +import threading +import urllib.error +import urllib.request +from unittest.mock import patch +from urllib.parse import parse_qs, urlencode, urlsplit + +import pytest + +from airflowctl.ctl.utils.browser_login import build_pkce_pair, get_authorization_code +from airflowctl.exceptions import AirflowCtlBrowserLoginException + + +def _fake_browser(build_callback_query): + """Return a ``webbrowser.open`` stand-in that calls back the loopback redirect like Keycloak would.""" + + def open_login_page(login_url): + params = {key: values[0] for key, values in parse_qs(urlsplit(login_url).query).items()} + callback_url = f"{params['redirect_uri']}?{urlencode(build_callback_query(params))}" + + def call_back(): + # A stray request must not end the wait. + redirect = urlsplit(params["redirect_uri"]) + with pytest.raises(urllib.error.HTTPError): + urllib.request.urlopen(f"http://{redirect.netloc}/favicon.ico") + urllib.request.urlopen(callback_url).read() + + threading.Thread(target=call_back, daemon=True).start() + return True + + return open_login_page + + +def test_build_pkce_pair(): + code_verifier, code_challenge = build_pkce_pair() + + assert 43 <= len(code_verifier) <= 128 + expected = base64.urlsafe_b64encode(hashlib.sha256(code_verifier.encode()).digest()).rstrip(b"=").decode() + assert code_challenge == expected + + +@patch("airflowctl.ctl.utils.browser_login.webbrowser.open", autospec=True) +def test_get_authorization_code(mock_open): + seen_params = {} + + def build_callback_query(params): + seen_params.update(params) + return {"code": "the-code", "state": params["state"]} + + mock_open.side_effect = _fake_browser(build_callback_query) + + result = get_authorization_code( + api_url="https://airflow.example.com/prefix", + authorization_endpoint="/prefix/auth/login/cli", + open_browser=True, + timeout=10, + ) + + login_url = urlsplit(mock_open.call_args.args[0]) + assert (login_url.netloc, login_url.path) == ("airflow.example.com", "/prefix/auth/login/cli") + assert seen_params["code_challenge_method"] == "S256" + assert result.code == "the-code" + assert result.redirect_uri == seen_params["redirect_uri"] + assert urlsplit(result.redirect_uri).hostname == "127.0.0.1" + challenge = base64.urlsafe_b64encode(hashlib.sha256(result.code_verifier.encode()).digest()) + assert challenge.rstrip(b"=").decode() == seen_params["code_challenge"] + + +@pytest.mark.parametrize( + ("build_callback_query", "message"), + [ + pytest.param( + lambda params: {"code": "the-code", "state": "forged"}, + "does not match this login attempt", + id="state-mismatch", + ), + pytest.param( + lambda params: { + "error": "access_denied", + "error_description": "User refused", + "state": params["state"], + }, + "Login refused: User refused", + id="error", + ), + pytest.param( + lambda params: {"state": params["state"]}, + "contains no authorization code", + id="no-code", + ), + ], +) +@patch("airflowctl.ctl.utils.browser_login.webbrowser.open", autospec=True) +def test_get_authorization_code_rejects_callback(mock_open, build_callback_query, message): + mock_open.side_effect = _fake_browser(build_callback_query) + + with pytest.raises(AirflowCtlBrowserLoginException, match=message): + get_authorization_code( + api_url="http://localhost:8080", + authorization_endpoint="/auth/login/cli", + open_browser=True, + timeout=10, + ) + + +@patch("airflowctl.ctl.utils.browser_login.webbrowser.open", autospec=True) +def test_get_authorization_code_no_browser_times_out(mock_open): + with pytest.raises(AirflowCtlBrowserLoginException, match="No login completed within 0.2 seconds"): + get_authorization_code( + api_url="http://localhost:8080", + authorization_endpoint="/auth/login/cli", + open_browser=False, + timeout=0.2, + ) + + mock_open.assert_not_called() From a78b2c1b0f2f78ed38f6c88370ca71089f00ba5a Mon Sep 17 00:00:00 2001 From: Baptiste Gouhoury Date: Thu, 29 Oct 2026 08:37:57 +0100 Subject: [PATCH 2/5] Keep the airflowctl browser login URL usable The login URL was printed to stdout, so capturing the output of `auth token --sso` also captured the URL and the user never saw it. It was also wrapped at the terminal width, which breaks copying it with --no-browser. Generated-by: Claude Code (Opus 5.5) Signed-off-by: Baptiste Gouhoury --- airflow-ctl/src/airflowctl/ctl/utils/browser_login.py | 8 ++++++-- .../tests/airflow_ctl/ctl/utils/test_browser_login.py | 5 ++++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/airflow-ctl/src/airflowctl/ctl/utils/browser_login.py b/airflow-ctl/src/airflowctl/ctl/utils/browser_login.py index 7a687b7cf8152..130f887aa76ab 100644 --- a/airflow-ctl/src/airflowctl/ctl/utils/browser_login.py +++ b/airflow-ctl/src/airflowctl/ctl/utils/browser_login.py @@ -28,7 +28,8 @@ from http.server import BaseHTTPRequestHandler, HTTPServer from urllib.parse import parse_qs, urlencode, urljoin, urlsplit -import rich +from rich.console import Console +from rich.markup import escape from airflowctl.exceptions import AirflowCtlBrowserLoginException @@ -109,7 +110,10 @@ def get_authorization_code( } ) ) - rich.print(f"[blue]Log in from your browser:[/blue] {login_url}") + # stderr keeps the URL out of `auth token --sso` output; soft_wrap keeps it on one line for copying. + Console(stderr=True, soft_wrap=True).print( + f"[blue]Log in from your browser:[/blue] {escape(login_url)}" + ) if open_browser: webbrowser.open(login_url) diff --git a/airflow-ctl/tests/airflow_ctl/ctl/utils/test_browser_login.py b/airflow-ctl/tests/airflow_ctl/ctl/utils/test_browser_login.py index 92eb2fe1423ac..5f183de0c9e94 100644 --- a/airflow-ctl/tests/airflow_ctl/ctl/utils/test_browser_login.py +++ b/airflow-ctl/tests/airflow_ctl/ctl/utils/test_browser_login.py @@ -60,7 +60,7 @@ def test_build_pkce_pair(): @patch("airflowctl.ctl.utils.browser_login.webbrowser.open", autospec=True) -def test_get_authorization_code(mock_open): +def test_get_authorization_code(mock_open, capsys): seen_params = {} def build_callback_query(params): @@ -76,6 +76,9 @@ def build_callback_query(params): timeout=10, ) + printed = capsys.readouterr() + assert printed.out == "" + assert printed.err == f"Log in from your browser: {mock_open.call_args.args[0]}\n" login_url = urlsplit(mock_open.call_args.args[0]) assert (login_url.netloc, login_url.path) == ("airflow.example.com", "/prefix/auth/login/cli") assert seen_params["code_challenge_method"] == "S256" From 409023e3057d336b89874ab3b6b263ed64d647cc Mon Sep 17 00:00:00 2001 From: Baptiste Gouhoury Date: Fri, 30 Oct 2026 12:40:13 +0100 Subject: [PATCH 3/5] Harden the airflowctl browser login The login metadata comes from the network, so an endpoint outside the API server would have received the code, the PKCE verifier and any stored token. Authorization codes are single use and identity providers can treat a replay as an attack, so the exchange must not be retried. Reviewers also pointed out that the browser reported success on failed logins and that an idle connection defeated --timeout. Error text from the server could crash the output, and auth token ignored the arguments that --sso is documented to refuse. --- airflow-ctl/docs/start.rst | 6 +- airflow-ctl/src/airflowctl/api/operations.py | 22 ++--- .../airflowctl/ctl/commands/auth_command.py | 18 ++-- .../src/airflowctl/ctl/utils/browser_login.py | 78 +++++++++++------- .../tests/airflow_ctl/api/test_operations.py | 19 +++++ .../ctl/commands/test_auth_command.py | 47 +++++++++-- .../ctl/utils/test_browser_login.py | 82 ++++++++++++++++--- 7 files changed, 204 insertions(+), 68 deletions(-) diff --git a/airflow-ctl/docs/start.rst b/airflow-ctl/docs/start.rst index d52f55527f938..d3f1d96c71307 100644 --- a/airflow-ctl/docs/start.rst +++ b/airflow-ctl/docs/start.rst @@ -48,14 +48,14 @@ OR, when the auth manager of the API server offers browser login (for example to .. code-block:: bash - airflowctl auth login --sso --api-url --env + airflowctl auth login --sso --api-url This opens the login page of the API server in your browser and waits for the login to complete. The browser redirects back to airflowctl on ``127.0.0.1``, so it must run on the same machine. -When logged in remotely, pass ``--no-browser`` to only print the login URL, and forward the port +On a remote machine, pass ``--no-browser`` to only print the login URL, and forward the port of the printed ``redirect_uri`` from your local machine, for example with ``ssh -L``. -This command will create a new airflowctl environment with the specified username and password. +Each of these commands creates a new airflowctl environment with the given credentials. You can then use the following command to start the airflowctl environment: .. code-block:: bash diff --git a/airflow-ctl/src/airflowctl/api/operations.py b/airflow-ctl/src/airflowctl/api/operations.py index 4d11a656bf7a2..11960907b8c1b 100644 --- a/airflow-ctl/src/airflowctl/api/operations.py +++ b/airflow-ctl/src/airflowctl/api/operations.py @@ -370,17 +370,19 @@ def login_with_authorization_code( self, token_url: str, code: str, code_verifier: str, redirect_uri: str ) -> LoginResponse: """Redeem an authorization code from the browser login for a CLI token.""" - return LoginResponse.model_validate_json( - self.client.post( - token_url, - json={ - "grant_type": "authorization_code", - "code": code, - "code_verifier": code_verifier, - "redirect_uri": redirect_uri, - }, - ).content + request = self.client.build_request( + "POST", + token_url, + json={ + "grant_type": "authorization_code", + "code": code, + "code_verifier": code_verifier, + "redirect_uri": redirect_uri, + }, ) + # send() skips the retries of Client.request(): a code is single use, and identity providers can + # treat a replayed code as an attack and revoke the session. + return LoginResponse.model_validate_json(self.client.send(request).content) # Operations diff --git a/airflow-ctl/src/airflowctl/ctl/commands/auth_command.py b/airflow-ctl/src/airflowctl/ctl/commands/auth_command.py index 1eb49ad2f40a2..f79d257d59c7b 100644 --- a/airflow-ctl/src/airflowctl/ctl/commands/auth_command.py +++ b/airflow-ctl/src/airflowctl/ctl/commands/auth_command.py @@ -23,16 +23,16 @@ import json import os import sys -from urllib.parse import urljoin import keyring import rich from keyring.errors import NoKeyringError +from rich.markup import escape from airflowctl.api.client import NEW_API_CLIENT, Client, ClientKind, Credentials, provide_api_client from airflowctl.api.datamodels.auth_generated import LoginBody from airflowctl.ctl.console_formatting import AirflowConsole -from airflowctl.ctl.utils.browser_login import get_authorization_code +from airflowctl.ctl.utils.browser_login import get_authorization_code, resolve_endpoint from airflowctl.exceptions import AirflowCtlBrowserLoginException @@ -48,14 +48,15 @@ def _get_token_with_browser(args, api_client: Client) -> str: ): raise AirflowCtlBrowserLoginException(f"{args.api_url} does not offer browser login (--sso).") + authorization_url = resolve_endpoint(args.api_url, metadata.authorization_endpoint) + token_url = resolve_endpoint(args.api_url, metadata.token_endpoint) authorization_code = get_authorization_code( - api_url=args.api_url, - authorization_endpoint=metadata.authorization_endpoint, + authorization_url=authorization_url, open_browser=not args.no_browser, timeout=args.timeout, ) return api_client.login.login_with_authorization_code( - urljoin(args.api_url, metadata.token_endpoint), + token_url, code=authorization_code.code, code_verifier=authorization_code.code_verifier, redirect_uri=authorization_code.redirect_uri, @@ -80,7 +81,7 @@ def login(args, api_client=NEW_API_CLIENT) -> None: try: api_token = _get_token_with_browser(args, api_client) except Exception as e: - rich.print(f"[red]Login failed: {e}") + rich.print(f"[red]Login failed: {escape(str(e))}") sys.exit(1) Credentials( api_url=args.api_url, @@ -157,10 +158,13 @@ def login(args, api_client=NEW_API_CLIENT) -> None: def get_token(args, api_client=NEW_API_CLIENT) -> None: """Generate and print a JWT token for the given credentials to stdout.""" if args.sso: + if args.username or args.password: + rich.print("[red]--sso is not compatible with --username or --password.") + sys.exit(1) try: print(_get_token_with_browser(args, api_client)) except Exception as e: - rich.print(f"[red]Token generation failed: {e}[/red]") + rich.print(f"[red]Token generation failed: {escape(str(e))}[/red]") sys.exit(1) return diff --git a/airflow-ctl/src/airflowctl/ctl/utils/browser_login.py b/airflow-ctl/src/airflowctl/ctl/utils/browser_login.py index 130f887aa76ab..292d03398be9b 100644 --- a/airflow-ctl/src/airflowctl/ctl/utils/browser_login.py +++ b/airflow-ctl/src/airflowctl/ctl/utils/browser_login.py @@ -38,6 +38,9 @@ b"airflowctl" b"

Login complete. You can close this window and return to the terminal.

" ) +_CALLBACK_FAILED_PAGE = ( + b"airflowctl

Login failed. Return to the terminal for details.

" +) @dataclass @@ -57,38 +60,63 @@ def build_pkce_pair() -> tuple[str, str]: return code_verifier, code_challenge +def resolve_endpoint(api_url: str, endpoint: str) -> str: + """Return the URL of a login metadata endpoint, which must be on the API server.""" + url = urljoin(api_url, endpoint) + if urlsplit(url)[:2] != urlsplit(api_url)[:2]: + raise AirflowCtlBrowserLoginException(f"{endpoint} is not an endpoint of {api_url}.") + return url + + +def _find_login_error(params: dict[str, str], state: str) -> str | None: + """Return why the callback parameters do not complete this login, or ``None`` if they do.""" + # compare_digest raises TypeError on non-ASCII str. + if not secrets.compare_digest(params.get("state", "").encode(), state.encode()): + return "The login response does not match this login attempt." + if "error" in params: + return f"Login refused: {params.get('error_description') or params['error']}" + if not params.get("code"): + return "The login response contains no authorization code." + return None + + class _CallbackServer(HTTPServer): + state: str callback_params: dict[str, str] | None = None + login_error: str | None = None class _CallbackHandler(BaseHTTPRequestHandler): server: _CallbackServer + # server.timeout only bounds accept(), so an idle connection such as a browser preconnect would block. + timeout = 2 def do_GET(self) -> None: url = urlsplit(self.path) - # Browsers also ask for /favicon.ico; only the callback ends the wait. if url.path != CALLBACK_PATH or self.server.callback_params is not None: self.send_error(404) return - self.server.callback_params = {key: values[0] for key, values in parse_qs(url.query).items()} - self.send_response(200) + params = {key: values[0] for key, values in parse_qs(url.query).items()} + login_error = _find_login_error(params, self.server.state) + self.server.callback_params = params + self.server.login_error = login_error + self.send_response(400 if login_error else 200) self.send_header("Content-Type", "text/html; charset=utf-8") self.end_headers() - self.wfile.write(_CALLBACK_PAGE) + self.wfile.write(_CALLBACK_FAILED_PAGE if login_error else _CALLBACK_PAGE) def log_message(self, format, *args) -> None: - # The default handler logs every request to stderr, including the code in the query string. + # Request lines contain the authorization code. pass def get_authorization_code( - *, api_url: str, authorization_endpoint: str, open_browser: bool, timeout: float + *, authorization_url: str, open_browser: bool, timeout: float ) -> AuthorizationCode: """ Run the browser part of the login and return the authorization code it produced. - :param api_url: URL of the Airflow API server - :param authorization_endpoint: ``authorization_endpoint`` from the CLI login metadata + :param authorization_url: URL of the ``authorization_endpoint`` from the CLI login metadata :param open_browser: open the login page in the default browser, otherwise only print its URL :param timeout: seconds to wait for the user to finish logging in """ @@ -96,21 +124,18 @@ def get_authorization_code( state = secrets.token_urlsafe(32) with _CallbackServer(("127.0.0.1", 0), _CallbackHandler) as server: + server.state = state redirect_uri = f"http://127.0.0.1:{server.server_address[1]}{CALLBACK_PATH}" - # Endpoints in the metadata are absolute paths, which keeps any path prefix of the API server. - login_url = ( - urljoin(api_url, authorization_endpoint) - + "?" - + urlencode( - { - "redirect_uri": redirect_uri, - "state": state, - "code_challenge": code_challenge, - "code_challenge_method": "S256", - } - ) + query = urlencode( + { + "redirect_uri": redirect_uri, + "state": state, + "code_challenge": code_challenge, + "code_challenge_method": "S256", + } ) - # stderr keeps the URL out of `auth token --sso` output; soft_wrap keeps it on one line for copying. + login_url = f"{authorization_url}?{query}" + # `auth token --sso` prints the token on stdout. Console(stderr=True, soft_wrap=True).print( f"[blue]Log in from your browser:[/blue] {escape(login_url)}" ) @@ -125,13 +150,8 @@ def get_authorization_code( server.timeout = remaining server.handle_request() params = server.callback_params + login_error = server.login_error - if not secrets.compare_digest(params.get("state", ""), state): - raise AirflowCtlBrowserLoginException("The login response does not match this login attempt.") - if "error" in params: - raise AirflowCtlBrowserLoginException( - f"Login refused: {params.get('error_description') or params['error']}" - ) - if not params.get("code"): - raise AirflowCtlBrowserLoginException("The login response contains no authorization code.") + if login_error: + raise AirflowCtlBrowserLoginException(login_error) return AuthorizationCode(code=params["code"], code_verifier=code_verifier, redirect_uri=redirect_uri) diff --git a/airflow-ctl/tests/airflow_ctl/api/test_operations.py b/airflow-ctl/tests/airflow_ctl/api/test_operations.py index 28593aa60c29d..5a0877bf7a51f 100644 --- a/airflow-ctl/tests/airflow_ctl/api/test_operations.py +++ b/airflow-ctl/tests/airflow_ctl/api/test_operations.py @@ -2847,6 +2847,25 @@ def handle_request(request: httpx.Request) -> httpx.Response: assert response.access_token == "TOKEN" + def test_login_with_authorization_code_is_not_retried(self): + requests = [] + + def handle_request(request: httpx.Request) -> httpx.Response: + requests.append(request) + return httpx.Response(503, json={"detail": "Unavailable"}) + + client = make_api_client(transport=httpx.MockTransport(handle_request), kind=ClientKind.AUTH) + + with pytest.raises(ServerResponseError): + client.login.login_with_authorization_code( + "http://server/auth/token/cli", + code="code", + code_verifier="verifier", + redirect_uri="http://127.0.0.1:1234/callback", + ) + + assert len(requests) == 1 + class TestXComOperations: """Test suite for XCom operations.""" diff --git a/airflow-ctl/tests/airflow_ctl/ctl/commands/test_auth_command.py b/airflow-ctl/tests/airflow_ctl/ctl/commands/test_auth_command.py index 7a9a6150cbdc9..cc198a1651723 100644 --- a/airflow-ctl/tests/airflow_ctl/ctl/commands/test_auth_command.py +++ b/airflow-ctl/tests/airflow_ctl/ctl/commands/test_auth_command.py @@ -30,6 +30,7 @@ from airflowctl.ctl import cli_parser from airflowctl.ctl.commands import auth_command from airflowctl.ctl.utils.browser_login import AuthorizationCode +from airflowctl.exceptions import AirflowCtlBrowserLoginException SSO_METADATA = { "grant_types_supported": ["password", "refresh_token", "authorization_code"], @@ -284,10 +285,6 @@ def test_login_with_username_and_password_no_keyring_backend(self, mock_keyring, api_client=api_client, ) - -class TestListEnvs: - parser = cli_parser.get_parser() - @patch("airflowctl.ctl.commands.auth_command.get_authorization_code", autospec=True) @patch("airflowctl.api.client.keyring") def test_login_with_sso(self, mock_keyring, mock_get_authorization_code): @@ -311,8 +308,7 @@ def test_login_with_sso(self, mock_keyring, mock_get_authorization_code): ) mock_get_authorization_code.assert_called_once_with( - api_url="http://localhost:8080", - authorization_endpoint="/auth/login/cli", + authorization_url="http://localhost:8080/auth/login/cli", open_browser=False, timeout=30, ) @@ -324,6 +320,14 @@ def test_login_with_sso(self, mock_keyring, mock_get_authorization_code): pytest.param({**SSO_METADATA, "grant_types_supported": ["password"]}, id="no-authorization-code"), pytest.param({**SSO_METADATA, "code_challenge_methods_supported": ["plain"]}, id="no-s256"), pytest.param({**SSO_METADATA, "authorization_endpoint": None}, id="no-authorization-endpoint"), + pytest.param( + {**SSO_METADATA, "authorization_endpoint": "//evil.example.com/login"}, + id="foreign-authorization-endpoint", + ), + pytest.param( + {**SSO_METADATA, "token_endpoint": "https://evil.example.com/token"}, + id="foreign-token-endpoint", + ), ], ) @patch("airflowctl.ctl.commands.auth_command.get_authorization_code", autospec=True) @@ -352,6 +356,33 @@ def test_login_with_sso_incompatible_args(self, mock_get_authorization_code, ext mock_get_authorization_code.assert_not_called() + @pytest.mark.parametrize("extra_args", [["--username", "user"], ["--password", "pass"]]) + @patch("airflowctl.ctl.commands.auth_command.get_authorization_code", autospec=True) + def test_get_token_with_sso_incompatible_args(self, mock_get_authorization_code, extra_args): + with pytest.raises(SystemExit): + auth_command.get_token( + self.parser.parse_args( + ["auth", "token", "--api-url", "http://localhost:8080", "--sso", *extra_args] + ), + api_client=_make_sso_api_client(SSO_METADATA), + ) + + mock_get_authorization_code.assert_not_called() + + @pytest.mark.parametrize( + ("command", "subcommand"), + [pytest.param(auth_command.login, "login", id="login"), (auth_command.get_token, "token")], + ) + @patch("airflowctl.ctl.commands.auth_command.get_authorization_code", autospec=True) + def test_sso_error_is_not_read_as_markup(self, mock_get_authorization_code, command, subcommand): + mock_get_authorization_code.side_effect = AirflowCtlBrowserLoginException("refused [/oops]") + + with pytest.raises(SystemExit): + command( + self.parser.parse_args(["auth", subcommand, "--api-url", "http://localhost:8080", "--sso"]), + api_client=_make_sso_api_client(SSO_METADATA), + ) + @patch("airflowctl.ctl.commands.auth_command.get_authorization_code", autospec=True) def test_get_token_with_sso(self, mock_get_authorization_code, capsys): mock_get_authorization_code.return_value = AUTHORIZATION_CODE @@ -363,6 +394,10 @@ def test_get_token_with_sso(self, mock_get_authorization_code, capsys): assert capsys.readouterr().out == "SSO_TOKEN\n" + +class TestListEnvs: + parser = cli_parser.get_parser() + def test_list_envs_empty_airflow_home(self, monkeypatch): """Test list-envs with no AIRFLOW_HOME directory.""" with ( diff --git a/airflow-ctl/tests/airflow_ctl/ctl/utils/test_browser_login.py b/airflow-ctl/tests/airflow_ctl/ctl/utils/test_browser_login.py index 5f183de0c9e94..4fefece729490 100644 --- a/airflow-ctl/tests/airflow_ctl/ctl/utils/test_browser_login.py +++ b/airflow-ctl/tests/airflow_ctl/ctl/utils/test_browser_login.py @@ -19,6 +19,8 @@ import base64 import hashlib +import queue +import socket import threading import urllib.error import urllib.request @@ -27,23 +29,26 @@ import pytest -from airflowctl.ctl.utils.browser_login import build_pkce_pair, get_authorization_code +from airflowctl.ctl.utils.browser_login import build_pkce_pair, get_authorization_code, resolve_endpoint from airflowctl.exceptions import AirflowCtlBrowserLoginException -def _fake_browser(build_callback_query): - """Return a ``webbrowser.open`` stand-in that calls back the loopback redirect like Keycloak would.""" +def _fake_browser(build_callback_query, pages): + """Return a ``webbrowser.open`` stand-in that calls back and puts ``(status, body)`` on ``pages``.""" def open_login_page(login_url): params = {key: values[0] for key, values in parse_qs(urlsplit(login_url).query).items()} callback_url = f"{params['redirect_uri']}?{urlencode(build_callback_query(params))}" def call_back(): - # A stray request must not end the wait. redirect = urlsplit(params["redirect_uri"]) with pytest.raises(urllib.error.HTTPError): urllib.request.urlopen(f"http://{redirect.netloc}/favicon.ico") - urllib.request.urlopen(callback_url).read() + try: + response = urllib.request.urlopen(callback_url) + except urllib.error.HTTPError as e: + response = e + pages.put((response.status, response.read())) threading.Thread(target=call_back, daemon=True).start() return True @@ -51,6 +56,13 @@ def call_back(): return open_login_page +def test_resolve_endpoint(): + assert ( + resolve_endpoint("https://airflow.example.com/prefix", "/prefix/auth/token/cli") + == "https://airflow.example.com/prefix/auth/token/cli" + ) + + def test_build_pkce_pair(): code_verifier, code_challenge = build_pkce_pair() @@ -62,16 +74,16 @@ def test_build_pkce_pair(): @patch("airflowctl.ctl.utils.browser_login.webbrowser.open", autospec=True) def test_get_authorization_code(mock_open, capsys): seen_params = {} + pages = queue.Queue() def build_callback_query(params): seen_params.update(params) return {"code": "the-code", "state": params["state"]} - mock_open.side_effect = _fake_browser(build_callback_query) + mock_open.side_effect = _fake_browser(build_callback_query, pages) result = get_authorization_code( - api_url="https://airflow.example.com/prefix", - authorization_endpoint="/prefix/auth/login/cli", + authorization_url="https://airflow.example.com/prefix/auth/login/cli", open_browser=True, timeout=10, ) @@ -87,6 +99,9 @@ def build_callback_query(params): assert urlsplit(result.redirect_uri).hostname == "127.0.0.1" challenge = base64.urlsafe_b64encode(hashlib.sha256(result.code_verifier.encode()).digest()) assert challenge.rstrip(b"=").decode() == seen_params["code_challenge"] + status, page = pages.get(timeout=5) + assert status == 200 + assert b"Login complete" in page @pytest.mark.parametrize( @@ -97,6 +112,11 @@ def build_callback_query(params): "does not match this login attempt", id="state-mismatch", ), + pytest.param( + lambda params: {"code": "the-code", "state": "forg\u00e9"}, + "does not match this login attempt", + id="non-ascii-state", + ), pytest.param( lambda params: { "error": "access_denied", @@ -115,23 +135,59 @@ def build_callback_query(params): ) @patch("airflowctl.ctl.utils.browser_login.webbrowser.open", autospec=True) def test_get_authorization_code_rejects_callback(mock_open, build_callback_query, message): - mock_open.side_effect = _fake_browser(build_callback_query) + pages = queue.Queue() + mock_open.side_effect = _fake_browser(build_callback_query, pages) with pytest.raises(AirflowCtlBrowserLoginException, match=message): get_authorization_code( - api_url="http://localhost:8080", - authorization_endpoint="/auth/login/cli", + authorization_url="http://localhost:8080/auth/login/cli", open_browser=True, timeout=10, ) + status, page = pages.get(timeout=5) + assert status == 400 + assert b"Login failed" in page + assert b"Login complete" not in page + + +@patch("airflowctl.ctl.utils.browser_login.webbrowser.open", autospec=True) +def test_get_authorization_code_drops_idle_connection(mock_open): + closed_by_server = [] + + def open_login_page(login_url): + params = {key: values[0] for key, values in parse_qs(urlsplit(login_url).query).items()} + redirect = urlsplit(params["redirect_uri"]) + + def call_back(): + with socket.create_connection((redirect.hostname, redirect.port), timeout=10) as idle: + try: + closed_by_server.append(idle.recv(1) == b"") + except TimeoutError: + closed_by_server.append(False) + query = urlencode({"code": "the-code", "state": params["state"]}) + urllib.request.urlopen(f"{params['redirect_uri']}?{query}").read() + + threading.Thread(target=call_back, daemon=True).start() + return True + + mock_open.side_effect = open_login_page + + result = get_authorization_code( + authorization_url="http://localhost:8080/auth/login/cli", + open_browser=True, + timeout=30, + ) + + assert closed_by_server == [True] + assert result.code == "the-code" + @patch("airflowctl.ctl.utils.browser_login.webbrowser.open", autospec=True) def test_get_authorization_code_no_browser_times_out(mock_open): with pytest.raises(AirflowCtlBrowserLoginException, match="No login completed within 0.2 seconds"): get_authorization_code( - api_url="http://localhost:8080", - authorization_endpoint="/auth/login/cli", + authorization_url="http://localhost:8080/auth/login/cli", open_browser=False, timeout=0.2, ) From c0f78a566c763a3ea683f462cf3a4bcdd9b31248 Mon Sep 17 00:00:00 2001 From: Baptiste Gouhoury Date: Sat, 31 Oct 2026 10:10:12 +0100 Subject: [PATCH 4/5] Stop the airflowctl browser page from claiming the login worked The page is sent before the authorization code is exchanged for a token, so the exchange can still fail after the browser said the login was complete, and the browser and the terminal then disagree. Generated-by: Claude Code (Opus 5.5) Signed-off-by: Baptiste Gouhoury --- airflow-ctl/src/airflowctl/ctl/utils/browser_login.py | 3 ++- airflow-ctl/tests/airflow_ctl/api/test_operations.py | 7 ++++++- .../tests/airflow_ctl/ctl/utils/test_browser_login.py | 3 +-- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/airflow-ctl/src/airflowctl/ctl/utils/browser_login.py b/airflow-ctl/src/airflowctl/ctl/utils/browser_login.py index 292d03398be9b..c80f351f4e9b6 100644 --- a/airflow-ctl/src/airflowctl/ctl/utils/browser_login.py +++ b/airflow-ctl/src/airflowctl/ctl/utils/browser_login.py @@ -34,9 +34,10 @@ from airflowctl.exceptions import AirflowCtlBrowserLoginException CALLBACK_PATH = "/callback" +# Sent before the code is exchanged for a token, so it must not claim that the login worked. _CALLBACK_PAGE = ( b"airflowctl" - b"

Login complete. You can close this window and return to the terminal.

" + b"

You can close this window and return to the terminal to see the result of the login.

" ) _CALLBACK_FAILED_PAGE = ( b"airflowctl

Login failed. Return to the terminal for details.

" diff --git a/airflow-ctl/tests/airflow_ctl/api/test_operations.py b/airflow-ctl/tests/airflow_ctl/api/test_operations.py index 5a0877bf7a51f..c1c09a51e0cb4 100644 --- a/airflow-ctl/tests/airflow_ctl/api/test_operations.py +++ b/airflow-ctl/tests/airflow_ctl/api/test_operations.py @@ -117,7 +117,12 @@ XComResponse, XComResponseNative, ) -from airflowctl.api.operations import BaseOperations, ServerResponseError, _build_query_params, _tolerant_model +from airflowctl.api.operations import ( + BaseOperations, + ServerResponseError, + _build_query_params, + _tolerant_model, +) from airflowctl.exceptions import AirflowCtlConnectionException if TYPE_CHECKING: diff --git a/airflow-ctl/tests/airflow_ctl/ctl/utils/test_browser_login.py b/airflow-ctl/tests/airflow_ctl/ctl/utils/test_browser_login.py index 4fefece729490..8d51135ebdf0a 100644 --- a/airflow-ctl/tests/airflow_ctl/ctl/utils/test_browser_login.py +++ b/airflow-ctl/tests/airflow_ctl/ctl/utils/test_browser_login.py @@ -101,7 +101,7 @@ def build_callback_query(params): assert challenge.rstrip(b"=").decode() == seen_params["code_challenge"] status, page = pages.get(timeout=5) assert status == 200 - assert b"Login complete" in page + assert b"return to the terminal to see the result" in page @pytest.mark.parametrize( @@ -148,7 +148,6 @@ def test_get_authorization_code_rejects_callback(mock_open, build_callback_query status, page = pages.get(timeout=5) assert status == 400 assert b"Login failed" in page - assert b"Login complete" not in page @patch("airflowctl.ctl.utils.browser_login.webbrowser.open", autospec=True) From 60a8bf08be9d2462927b36b834aaef59c04aa9c8 Mon Sep 17 00:00:00 2001 From: Baptiste Gouhoury Date: Sat, 31 Oct 2026 10:17:39 +0100 Subject: [PATCH 5/5] Let airflowctl browser login work without a keyring Headless machines often have no keyring, and those are the users --no-browser is for. The keyring error told them to pass --skip-keyring, which --sso refused, so they could not log in at all. Generated-by: Claude Code (Opus 5.5) Signed-off-by: Baptiste Gouhoury --- airflow-ctl/docs/start.rst | 2 ++ .../airflowctl/ctl/commands/auth_command.py | 16 ++++++++++----- .../ctl/commands/test_auth_command.py | 20 ++++++++++++++++++- docs/spelling_wordlist.txt | 1 + 4 files changed, 33 insertions(+), 6 deletions(-) diff --git a/airflow-ctl/docs/start.rst b/airflow-ctl/docs/start.rst index d3f1d96c71307..ce8596bb9ffe2 100644 --- a/airflow-ctl/docs/start.rst +++ b/airflow-ctl/docs/start.rst @@ -54,6 +54,8 @@ This opens the login page of the API server in your browser and waits for the lo The browser redirects back to airflowctl on ``127.0.0.1``, so it must run on the same machine. On a remote machine, pass ``--no-browser`` to only print the login URL, and forward the port of the printed ``redirect_uri`` from your local machine, for example with ``ssh -L``. +On a machine without a keyring, also pass ``--skip-keyring``: the token is printed instead of +stored, and you can set ``AIRFLOW_CLI_TOKEN`` to it. Each of these commands creates a new airflowctl environment with the given credentials. You can then use the following command to start the airflowctl environment: diff --git a/airflow-ctl/src/airflowctl/ctl/commands/auth_command.py b/airflow-ctl/src/airflowctl/ctl/commands/auth_command.py index f79d257d59c7b..0ab4cd14187ff 100644 --- a/airflow-ctl/src/airflowctl/ctl/commands/auth_command.py +++ b/airflow-ctl/src/airflowctl/ctl/commands/auth_command.py @@ -27,6 +27,7 @@ import keyring import rich from keyring.errors import NoKeyringError +from rich.console import Console from rich.markup import escape from airflowctl.api.client import NEW_API_CLIENT, Client, ClientKind, Credentials, provide_api_client @@ -73,10 +74,8 @@ def login(args, api_client=NEW_API_CLIENT) -> None: token = args.api_token or os.environ.get("AIRFLOW_CLI_TOKEN") if args.sso: - if args.api_token or username or password or args.skip_keyring: - rich.print( - "[red]--sso is not compatible with --api-token, --username, --password or --skip-keyring." - ) + if args.api_token or username or password: + rich.print("[red]--sso is not compatible with --api-token, --username or --password.") sys.exit(1) try: api_token = _get_token_with_browser(args, api_client) @@ -88,7 +87,14 @@ def login(args, api_client=NEW_API_CLIENT) -> None: api_token=api_token, api_environment=args.env, client_kind=ClientKind.AUTH, - ).save() + ).save(args.skip_keyring) + if args.skip_keyring: + # Nothing stores the token, so this is the only way to use the login that just happened. + print(api_token) + Console(stderr=True).print( + "[yellow]The token was not stored. Set AIRFLOW_CLI_TOKEN to the printed token to use it.[/yellow]" + ) + return rich.print(success_message) return diff --git a/airflow-ctl/tests/airflow_ctl/ctl/commands/test_auth_command.py b/airflow-ctl/tests/airflow_ctl/ctl/commands/test_auth_command.py index cc198a1651723..d4a904615e081 100644 --- a/airflow-ctl/tests/airflow_ctl/ctl/commands/test_auth_command.py +++ b/airflow-ctl/tests/airflow_ctl/ctl/commands/test_auth_command.py @@ -314,6 +314,24 @@ def test_login_with_sso(self, mock_keyring, mock_get_authorization_code): ) mock_keyring.set_password.assert_called_once_with("airflowctl", "api_token_production", "SSO_TOKEN") + @patch("airflowctl.ctl.commands.auth_command.get_authorization_code", autospec=True) + @patch("airflowctl.api.client.keyring") + def test_login_with_sso_skip_keyring_prints_token( + self, mock_keyring, mock_get_authorization_code, capsys + ): + mock_get_authorization_code.return_value = AUTHORIZATION_CODE + mock_keyring.get_password.return_value = None + + auth_command.login( + self.parser.parse_args( + ["auth", "login", "--api-url", "http://localhost:8080", "--sso", "--skip-keyring"] + ), + api_client=_make_sso_api_client(SSO_METADATA), + ) + + assert capsys.readouterr().out == "SSO_TOKEN\n" + mock_keyring.set_password.assert_not_called() + @pytest.mark.parametrize( "metadata", [ @@ -342,7 +360,7 @@ def test_login_with_sso_not_offered(self, mock_get_authorization_code, metadata) @pytest.mark.parametrize( "extra_args", - [["--username", "user"], ["--password", "pass"], ["--api-token", "token"], ["--skip-keyring"]], + [["--username", "user"], ["--password", "pass"], ["--api-token", "token"]], ) @patch("airflowctl.ctl.commands.auth_command.get_authorization_code", autospec=True) def test_login_with_sso_incompatible_args(self, mock_get_authorization_code, extra_args): diff --git a/docs/spelling_wordlist.txt b/docs/spelling_wordlist.txt index ec0b8690b3986..3cce7b958847d 100644 --- a/docs/spelling_wordlist.txt +++ b/docs/spelling_wordlist.txt @@ -1685,6 +1685,7 @@ sslkey sslmode sslrootcert ssm +sso Stackdriver stackdriver stacklevel