Skip to content

Fix FabAuthManager.get_url_login() dropping next_url, losing deep-link redirects after login - #73134

Merged
vincbeck merged 1 commit into
apache:mainfrom
TheoLauw:fix-fab-auth-manager-next-url-redirect
Sep 14, 2026
Merged

vincbeck merged 1 commit into
apache:mainfrom
TheoLauw:fix-fab-auth-manager-next-url-redirect

Conversation

@TheoLauw

Copy link
Copy Markdown
Contributor

Fixes #73133 (opened alongside this PR)

What

FabAuthManager.get_url_login() accepted **kwargs but never used them, so callers passing next_url=request.url (to redirect the user back to their originally requested page after login) had that value silently discarded. Every unauthenticated/expired-session redirect to /auth/login/ therefore lost the deep-link target, and the user landed on the homepage after logging in.

This is the same class of bug fixed for SimpleAuthManager in #67476 / #67483 / #67965, but that fix was explicitly scoped to SimpleAuthManager only. FabAuthManager (used for any FAB-backed auth: DB, LDAP, OAuth/SSO, etc.) had the identical bug and was left unpatched.

Why only one change is needed here (unlike the Simple auth manager fix)

SimpleAuthManager needed two PRs: one to propagate next_url into the login URL (#67965) and one to make the login route actually consume next on completion (#67483), because its login route is a bespoke FastAPI implementation with no built-in notion of "return to this page."

FabAuthManager doesn't need the second half: it delegates to flask_appbuilder's own AuthOAuthView/AuthDBView, whose login() view already reads next from the query string and encodes it into the OAuth state (or session, for non-OAuth backends), and whose redirect/get_safe_redirect are already monkey-patched by providers/fab/src/airflow/providers/fab/www/extensions/init_appbuilder.py to Airflow's own versions (providers/fab/src/airflow/providers/fab/www/views.py), which set the _token JWT cookie and forward to that URL. I traced this end-to-end and confirmed it already works correctly — the only missing link was next_url never making it into the initial /auth/login/ URL in the first place.

So this PR only needs to touch get_url_login(), mirroring the exact propagation fix from #67965:

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

Tests

Added test_get_url_login_with_next_url and test_get_url_login_without_next_url_kwarg to TestFabAuthManager, mirroring the tests added for SimpleAuthManager in #67965.

Verified manually against the released apache-airflow-providers-fab==3.2.0 package (not just against main) by patching the installed get_url_login and exercising it directly:

  • no next_url kwarg → unchanged bare login URL (backward compatible)
  • next_url provided → correctly appended as a url-encoded next query param
  • next_url=None → falls back to the bare URL
  • unrelated/unknown kwargs → ignored, no crash
  • next_url containing spaces/&/? → properly percent-encoded, not passed through raw

How I found this

Root-caused while investigating a real-world report from our Airflow 3 deployment (FabAuthManager + OAuth SSO): a data engineer reported that a deep link opened after session expiry always landed on the homepage, requiring a second click on the same link to actually reach the target. Filed as issue #73133 with full reproduction details and the trace of the surrounding call chain.

Checklist

  • I have performed a self-review of my own code
  • Unit tests added for the change
  • No breaking changes: when next_url is not supplied, get_url_login() returns exactly the same value as before

FabAuthManager.get_url_login() accepted **kwargs but never read
next_url from it, so callers redirecting an unauthenticated or
expired-session request to the login page (via
providers/fab/src/airflow/providers/fab/www/auth.py, which already
passes next_url=request.url) always lost the originally requested
URL. After completing login, the user landed on the homepage instead
of the deep link they opened, and had to open the same link a second
time to reach it.

This mirrors the fix already applied to SimpleAuthManager in apache#67965,
which was scoped to SimpleAuthManager only and left FabAuthManager
with the identical bug.

The rest of the redirect chain (flask_appbuilder's AuthOAuthView/
AuthDBView reading `next` from the query string into the OAuth state,
and the redirect/get_safe_redirect monkey-patch in
providers/fab/www/extensions/init_appbuilder.py that sets the JWT
cookie and forwards to that URL) already works correctly, so
propagating next_url into the login URL is the only change needed.
@boring-cyborg

boring-cyborg Bot commented Sep 14, 2026

Copy link
Copy Markdown

Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide
Here are some useful points:

  • Pay attention to the quality of your code (ruff, mypy and type annotations). Our prek-hooks will help you with that.
  • In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example Dag that shows how users should use it.
  • Consider using Breeze environment for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
  • Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
  • Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
  • Be sure to read the Airflow Coding style.
  • Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
    Apache Airflow is a community-driven project and together we are making it better 🚀.
    In case of doubts contact the developers at:
    Mailing List: dev@airflow.apache.org
    Slack: https://s.apache.org/airflow-slack

@vincbeck
vincbeck merged commit c902fea into apache:main Sep 14, 2026
79 checks passed
@boring-cyborg

boring-cyborg Bot commented Sep 14, 2026

Copy link
Copy Markdown

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FabAuthManager: deep links redirect to homepage on first login after session expiry (next_url ignored, same class of bug as #67476)

3 participants