vincbeck commented on code in PR #75913:
URL: https://github.com/apache/airflow/pull/75913#discussion_r4412860428
##########
providers/keycloak/src/airflow/providers/keycloak/auth_manager/services/token.py:
##########
@@ -37,6 +42,48 @@
from airflow.providers.keycloak.auth_manager.keycloak_auth_manager import
KeycloakAuthManager
from airflow.providers.keycloak.auth_manager.user import
KeycloakAuthManagerUser
+# RFC 8252 section 7.3 loopback redirects. ``localhost`` is left out because
it can resolve
+# to a non-loopback interface.
+CLI_LOOPBACK_HOSTS = frozenset({"127.0.0.1", "::1"})
+CLI_CALLBACK_PATH = "/callback"
+
+
+def is_cli_authorization_code_enabled() -> bool:
+ return conf.getboolean(CONF_SECTION_NAME,
CONF_CLI_AUTHORIZATION_CODE_ENABLED_KEY, fallback=False)
+
+
+def is_cli_loopback_redirect_uri(redirect_uri: str) -> bool:
+ """Return whether ``redirect_uri`` is a loopback callback a command-line
client listens on."""
+ try:
+ parts = urlsplit(redirect_uri)
+ port = parts.port
+ except ValueError:
+ return False
+ return (
Review Comment:
From my agent:
> The loopback redirect_uri check can be bypassed. services/token.py:66 only
checks parts.username is None, so a URL like
http://:x&code_challenge=EVIL&[email protected]:5/callback
passes. The URL then goes unencoded into the Keycloak auth URL, which lets an
attacker add their own PKCE parameters. Fix: reject any @ in the netloc, and
URL-encode redirect_uri.
##########
providers/keycloak/src/airflow/providers/keycloak/auth_manager/keycloak_auth_manager.py:
##########
@@ -234,6 +241,17 @@ def get_url_logout(self) -> str | None:
base_url = conf.get("api", "base_url", fallback="/")
return urljoin(base_url, f"{AUTH_MANAGER_FASTAPI_APP_PREFIX}/logout")
+ def get_cli_login_methods(self) -> CliLoginMethods:
+ methods = CliLoginMethods(
+ grant_types_supported=["password", "refresh_token"],
+ token_endpoint=f"{AUTH_MANAGER_FASTAPI_APP_PREFIX}/token/cli",
Review Comment:
You probably want to use the `base_url` from config?
##########
providers/keycloak/src/airflow/providers/keycloak/auth_manager/datamodels/token.py:
##########
@@ -98,3 +130,21 @@ def default_grant_type(cls, data):
if "grant_type" not in data:
data["grant_type"] = "password"
return data
+
+
+TokenCliUnion = Annotated[
+ TokenPasswordBody | TokenRefreshBody | TokenAuthorizationCodeBody,
+ Field(discriminator="grant_type"),
+]
+
+
+class TokenCliBody(RootModel[TokenCliUnion]):
+ """Token request body for command-line clients."""
+
+ @model_validator(mode="before")
+ @classmethod
+ def default_grant_type(cls, data):
Review Comment:
A non JSON body will result in a 500
##########
providers/keycloak/src/airflow/providers/keycloak/auth_manager/services/token.py:
##########
@@ -210,3 +257,66 @@ def create_client_credentials_token(
)
return get_auth_manager().generate_api_jwt(user,
expiration_time_in_seconds=expiration_time_in_seconds)
+
+
+def create_token_from_authorization_code(
+ code: str,
+ code_verifier: str,
+ redirect_uri: str,
+ expiration_time_in_seconds: int = conf.getint("api_auth",
"jwt_cli_expiration_time"),
+) -> str:
+ """
+ Create a CLI token from an authorization code obtained through
``/login/cli``.
+
+ Keycloak checks ``code_verifier`` against the challenge ``/login/cli``
forwarded, so
+ the code is useless to anyone who intercepted it on the loopback redirect.
+ """
+ if not is_cli_authorization_code_enabled():
+ raise HTTPException(
+ status_code=status.HTTP_400_BAD_REQUEST,
+ detail="The authorization_code grant is not enabled for
command-line clients",
+ )
+ if not is_cli_loopback_redirect_uri(redirect_uri):
+ raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST,
detail="Invalid redirect_uri")
+
+ client = KeycloakAuthManager.get_keycloak_client()
+ try:
+ tokens = client.token(
+ grant_type="authorization_code",
+ code=code,
+ redirect_uri=redirect_uri,
+ code_verifier=code_verifier,
+ scope="openid",
+ )
+ except KeycloakError:
+ raise HTTPException(status_code=status.HTTP_403_FORBIDDEN,
detail="Invalid authorization code")
+
+ return _create_api_jwt_from_keycloak_tokens(client, tokens,
expiration_time_in_seconds)
+
+
+def refresh_cli_token(token: str) -> str:
+ """
+ Create a CLI token with fresh Keycloak tokens from a CLI token that is
still valid.
+
+ The new token expires with the old one, so refreshing never extends a CLI
session.
+ Expired and revoked tokens are refused by ``get_user_from_token``.
+ """
+ auth_manager = get_auth_manager()
+ try:
+ user = cast("KeycloakAuthManagerUser | None",
async_to_sync(auth_manager.get_user_from_token)(token))
+ except jwt.PyJWTError:
+ user = None
+ if not user or not user.refresh_token:
+ raise HTTPException(status_code=status.HTTP_403_FORBIDDEN,
detail="Invalid token")
+
+ client = KeycloakAuthManager.get_keycloak_client()
+ try:
+ tokens = client.refresh_token(user.refresh_token)
+ except KeycloakError:
+ raise HTTPException(status_code=status.HTTP_403_FORBIDDEN,
detail="Invalid token")
+
+ user.access_token = tokens["access_token"]
+ user.refresh_token = tokens["refresh_token"]
Review Comment:
Some Keycloak envs have no refresh token, this will fail
##########
providers/keycloak/src/airflow/providers/keycloak/auth_manager/services/token.py:
##########
@@ -210,3 +257,66 @@ def create_client_credentials_token(
)
return get_auth_manager().generate_api_jwt(user,
expiration_time_in_seconds=expiration_time_in_seconds)
+
+
+def create_token_from_authorization_code(
+ code: str,
+ code_verifier: str,
+ redirect_uri: str,
+ expiration_time_in_seconds: int = conf.getint("api_auth",
"jwt_cli_expiration_time"),
+) -> str:
+ """
+ Create a CLI token from an authorization code obtained through
``/login/cli``.
+
+ Keycloak checks ``code_verifier`` against the challenge ``/login/cli``
forwarded, so
+ the code is useless to anyone who intercepted it on the loopback redirect.
+ """
+ if not is_cli_authorization_code_enabled():
+ raise HTTPException(
+ status_code=status.HTTP_400_BAD_REQUEST,
+ detail="The authorization_code grant is not enabled for
command-line clients",
+ )
+ if not is_cli_loopback_redirect_uri(redirect_uri):
+ raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST,
detail="Invalid redirect_uri")
+
+ client = KeycloakAuthManager.get_keycloak_client()
+ try:
+ tokens = client.token(
+ grant_type="authorization_code",
+ code=code,
+ redirect_uri=redirect_uri,
+ code_verifier=code_verifier,
+ scope="openid",
+ )
+ except KeycloakError:
+ raise HTTPException(status_code=status.HTTP_403_FORBIDDEN,
detail="Invalid authorization code")
+
+ return _create_api_jwt_from_keycloak_tokens(client, tokens,
expiration_time_in_seconds)
+
+
+def refresh_cli_token(token: str) -> str:
+ """
+ Create a CLI token with fresh Keycloak tokens from a CLI token that is
still valid.
+
+ The new token expires with the old one, so refreshing never extends a CLI
session.
+ Expired and revoked tokens are refused by ``get_user_from_token``.
+ """
+ auth_manager = get_auth_manager()
+ try:
+ user = cast("KeycloakAuthManagerUser | None",
async_to_sync(auth_manager.get_user_from_token)(token))
+ except jwt.PyJWTError:
+ user = None
+ if not user or not user.refresh_token:
+ raise HTTPException(status_code=status.HTTP_403_FORBIDDEN,
detail="Invalid token")
+
+ client = KeycloakAuthManager.get_keycloak_client()
+ try:
+ tokens = client.refresh_token(user.refresh_token)
+ except KeycloakError:
+ raise HTTPException(status_code=status.HTTP_403_FORBIDDEN,
detail="Invalid token")
+
+ user.access_token = tokens["access_token"]
+ user.refresh_token = tokens["refresh_token"]
Review Comment:
On the same topic. `refresh_token` is listed as a supported grant even when
the Keycloak client issues no refresh tokens. The CLI then gets a confusing 403
"Invalid token"
##########
providers/keycloak/src/airflow/providers/keycloak/auth_manager/services/token.py:
##########
@@ -210,3 +257,66 @@ def create_client_credentials_token(
)
return get_auth_manager().generate_api_jwt(user,
expiration_time_in_seconds=expiration_time_in_seconds)
+
+
+def create_token_from_authorization_code(
+ code: str,
+ code_verifier: str,
+ redirect_uri: str,
+ expiration_time_in_seconds: int = conf.getint("api_auth",
"jwt_cli_expiration_time"),
+) -> str:
+ """
+ Create a CLI token from an authorization code obtained through
``/login/cli``.
+
+ Keycloak checks ``code_verifier`` against the challenge ``/login/cli``
forwarded, so
+ the code is useless to anyone who intercepted it on the loopback redirect.
+ """
+ if not is_cli_authorization_code_enabled():
+ raise HTTPException(
+ status_code=status.HTTP_400_BAD_REQUEST,
+ detail="The authorization_code grant is not enabled for
command-line clients",
+ )
+ if not is_cli_loopback_redirect_uri(redirect_uri):
+ raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST,
detail="Invalid redirect_uri")
+
+ client = KeycloakAuthManager.get_keycloak_client()
+ try:
+ tokens = client.token(
+ grant_type="authorization_code",
+ code=code,
+ redirect_uri=redirect_uri,
+ code_verifier=code_verifier,
+ scope="openid",
+ )
+ except KeycloakError:
+ raise HTTPException(status_code=status.HTTP_403_FORBIDDEN,
detail="Invalid authorization code")
+
+ return _create_api_jwt_from_keycloak_tokens(client, tokens,
expiration_time_in_seconds)
+
+
+def refresh_cli_token(token: str) -> str:
+ """
+ Create a CLI token with fresh Keycloak tokens from a CLI token that is
still valid.
+
+ The new token expires with the old one, so refreshing never extends a CLI
session.
+ Expired and revoked tokens are refused by ``get_user_from_token``.
+ """
+ auth_manager = get_auth_manager()
+ try:
+ user = cast("KeycloakAuthManagerUser | None",
async_to_sync(auth_manager.get_user_from_token)(token))
+ except jwt.PyJWTError:
+ user = None
+ if not user or not user.refresh_token:
+ raise HTTPException(status_code=status.HTTP_403_FORBIDDEN,
detail="Invalid token")
+
+ client = KeycloakAuthManager.get_keycloak_client()
+ try:
+ tokens = client.refresh_token(user.refresh_token)
+ except KeycloakError:
+ raise HTTPException(status_code=status.HTTP_403_FORBIDDEN,
detail="Invalid token")
+
+ user.access_token = tokens["access_token"]
+ user.refresh_token = tokens["refresh_token"]
+ # The signature and expiry were verified by get_user_from_token above.
+ expires_at = jwt.decode(token, options={"verify_signature": False})["exp"]
+ return auth_manager.generate_api_jwt(user,
expiration_time_in_seconds=int(expires_at - time.time()))
Review Comment:
A refresh can return a token that is already expired. services/token.py:322
computes the lifetime as int(exp - time.time()), but validation allows
jwt_leeway (10s by default). A token that expired 5 seconds ago passes
validation and comes back with a lifetime of -5. Fix: reject a remaining
lifetime of 0 or less.
##########
providers/keycloak/src/airflow/providers/keycloak/auth_manager/services/token.py:
##########
@@ -37,6 +42,48 @@
from airflow.providers.keycloak.auth_manager.keycloak_auth_manager import
KeycloakAuthManager
from airflow.providers.keycloak.auth_manager.user import
KeycloakAuthManagerUser
+# RFC 8252 section 7.3 loopback redirects. ``localhost`` is left out because
it can resolve
+# to a non-loopback interface.
+CLI_LOOPBACK_HOSTS = frozenset({"127.0.0.1", "::1"})
+CLI_CALLBACK_PATH = "/callback"
+
+
+def is_cli_authorization_code_enabled() -> bool:
+ return conf.getboolean(CONF_SECTION_NAME,
CONF_CLI_AUTHORIZATION_CODE_ENABLED_KEY, fallback=False)
+
+
+def is_cli_loopback_redirect_uri(redirect_uri: str) -> bool:
+ """Return whether ``redirect_uri`` is a loopback callback a command-line
client listens on."""
+ try:
+ parts = urlsplit(redirect_uri)
+ port = parts.port
+ except ValueError:
+ return False
+ return (
+ parts.scheme == "http"
+ and parts.hostname in CLI_LOOPBACK_HOSTS
+ and port is not None
+ and parts.username is None
+ and parts.path == CLI_CALLBACK_PATH
+ and not parts.query
+ and not parts.fragment
+ )
+
+
+def _create_api_jwt_from_keycloak_tokens(
Review Comment:
Is not it the same as `create_token_for`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
