baptistegh commented on issue #75560: URL: https://github.com/apache/airflow/issues/75560#issuecomment-6418424363
Thanks @stegololz, this is very helpful. I agree with all four blockers and the spec details, and I'll take the core hook route for discovery. Here is the PR split I'd follow: 1. **Core:** `BaseAuthManager.get_cli_login_methods()` (empty by default) and a metadata route next to `/auth/login`. 2. **Keycloak provider:** a dedicated body model for `/token/cli` (`password` by default) and a `refresh_token` grant. This also fixes the 403 after 5 minutes for password login today. 3. **Keycloak provider:** the browser flow, with S256 PKCE enforced by Airflow, loopback-only redirects and a config option that is off by default. 4. **Keycloak provider:** the device code flow, with its own config option, off by default. 5. **airflowctl:** discovery, `--sso`, `--no-browser` (prints the URL, still uses the loopback callback) and `--timeout`, plus a single refresh retry on 401/403. 6. **airflowctl:** an explicit `--device-code`. airflowctl never falls back to a flow the server doesn't advertise. 7. **Later:** SSO by default on a TTY, as you described. 1 and 2 don't depend on each other. 5 needs 1 and 3. A few questions before I start on the provider side: 1. **Refresh contract.** My plan is for the `refresh_token` grant to take the current Airflow CLI token: verify its signature, refresh with the Keycloak refresh token from its claims, and mint a new CLI token. I'd refuse it once the CLI token's own `exp` has passed, so a session never outlives `jwt_cli_expiration_time`. Does that work for you? 2. **Refresh token in the claims.** `generate_api_jwt` already puts the Keycloak refresh token in the signed but unencrypted claims. That's fine for short-lived tokens, but the CLI token lives longer and sits in the keyring or a file. Is that acceptable, or should the refresh token stay server-side? 3. **Discovery route.** Is `GET /api/v2/auth/cli/metadata` OK? 4. **Older cores.** On Airflow versions without the route, airflowctl would print a clear "SSO login is not supported by this server" error rather than trying the flow. The provider would only add its methods when running on a core that has the hook. OK? 5. **`python-keycloak>=6.0`.** Should the bump go in the browser-flow PR? 6. **Config option names.** I'd suggest `cli_authorization_code_enabled` and `cli_device_code_enabled` under `[keycloak_auth_manager]`, both `False` by default. Do they look right to you? If this works for you, I'll start with the core PR. -- 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]
