TheoLauw opened a new issue, #73133: URL: https://github.com/apache/airflow/issues/73133
### Under which category would you file this issue? Airflow Core ### Apache Airflow version Other Airflow 3 version (please specify below) ### If "Other Airflow 3 version" selected, which one? Reproduced on 3.2.1. Verified the same code is still present (unpatched) in the latest `apache-airflow-providers-fab` release (3.7.0), so this affects the current `main` branch as well. ### What happened? When a user's session has expired (or they open a deep link while unauthenticated) with `FabAuthManager` configured (e.g. `AUTH_TYPE = AUTH_OAUTH` / SSO via `webserver_config.py`), they are redirected to the homepage instead of the originally requested URL after completing login. Reopening the same link a second time resolves correctly, because a valid session/JWT cookie now exists. This is the exact same class of bug as #67476, which was reported and fixed for `SimpleAuthManager` in #67483 / #67965 (merged for 3.3.0). Those fixes were explicitly scoped to `SimpleAuthManager` only — `FabAuthManager.get_url_login()` has the identical bug and was never patched. ### Root cause `FabAuthManager.get_url_login()` accepts `**kwargs` but never reads `next_url` from it: https://github.com/apache/airflow/blob/main/providers/fab/src/airflow/providers/fab/auth_manager/fab_auth_manager.py ```python def get_url_login(self, **kwargs) -> str: """Return the login page url.""" return urljoin(self.apiserver_endpoint, f"{AUTH_MANAGER_FASTAPI_APP_PREFIX}/login/") ``` The callers that trigger this redirect already pass `next_url=request.url` correctly: - `providers/fab/src/airflow/providers/fab/www/auth.py` (`has_access_with_pk`, `_has_access`) — both call `redirect(get_auth_manager().get_url_login(next_url=request.url))` Because `get_url_login()` discards `next_url`, the browser is sent to the bare `.../auth/login/` URL with **no** `next` query parameter at all. I traced the rest of the chain and confirmed it is otherwise intact and does not need any change: - `flask_appbuilder.security.views.AuthOAuthView.login()` reads `request.args.get('next')` (via the `login_oauth.html` template's `next` JS variable, or directly on `/login/<provider>`) and embeds it in the `state` JWT sent to the IdP. - `providers/fab/src/airflow/providers/fab/www/extensions/init_appbuilder.py` monkey-patches `flask_appbuilder.security.views.redirect` and `get_safe_redirect` with Airflow's own versions (`providers/fab/src/airflow/providers/fab/www/views.py`), which correctly set the `_token` JWT cookie and forward to whatever URL FAB decided to redirect to. So the *only* missing link is `next_url` being dropped at the very first step, before any of that machinery ever sees it. ### What you think should happen instead? `FabAuthManager.get_url_login()` should append the `next_url` it's given as a `next` query parameter, mirroring the fix already applied to `SimpleAuthManager` in #67965: ```python def get_url_login(self, **kwargs) -> str: """Return the login page url.""" login_url = urljoin(self.apiserver_endpoint, f"{AUTH_MANAGER_FASTAPI_APP_PREFIX}/login/") next_url = kwargs.get("next_url") if next_url: return f"{login_url}?{urlencode({'next': next_url})}" return login_url ``` Unlike the `SimpleAuthManager` fix, this looks like it only needs the "propagation" half (the equivalent of #67965) — the "consumption" half (the equivalent of #67483) already exists for FAB via `flask_appbuilder`'s own OAuth `state` handling plus Airflow's `redirect`/`get_safe_redirect` monkey-patch, so once `next` reaches `/login/`, the rest of the chain should already carry it through. I'm opening a PR with this fix plus unit tests mirroring the ones added in #67965. ### How to reproduce 1. Configure `AUTH_TYPE = AUTH_OAUTH` (or any FAB auth backend) via `webserver_config.py`, i.e. `FabAuthManager` is the active `[core] auth_manager`. 2. Log in, then let the session/JWT cookie expire (or open the link in a fresh private window). 3. Open a deep link, e.g. `https://<airflow-host>/dags/<dag_id>/runs/<run_id>/tasks/<task_id>`. 4. Complete login. 5. **Observe:** you land on `/` instead of the deep link. 6. Open the same URL again. 7. **Observe:** it now resolves correctly (valid session already exists). ### Deployment Other Docker-based deployment ### Deployment details Self-hosted Airflow 3.2.1 on Kubernetes (official Helm chart), `FabAuthManager` with `AUTH_TYPE = AUTH_OAUTH` against an external OIDC/Auth0-style provider. ### Anything else? - Confirmed the bug is still present by inspecting `fab_auth_manager.py` in `apache-airflow-providers-fab` versions 3.2.0, 3.3.0, 3.5.0, and 3.7.0 (latest available at time of writing) — all identical, unpatched. - Related: #67476 / #67483 / #67965 (SimpleAuthManager fix), #36832 (older, unrelated AF2-era `next`/`next_url` kwarg mismatch bug), #63521 (a different symptom — a transient "Unauthorized" flash during the OAuth race, addressed for FAB by #61287). ### Are you willing to submit a PR? - [x] Yes I am willing to submit a PR! ### Code of Conduct - [x] I agree to follow this project's [Code of Conduct](https://github.com/apache/airflow/blob/main/CODE_OF_CONDUCT.md) -- 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]
