Skip to content

Fix '/auth/token' 500 when FAB auth manager is recreated#65710

Merged
vincbeck merged 13 commits into
apache:mainfrom
snowlaxc:fix/fab-auth-token-flask-app
Apr 24, 2026
Merged

Fix '/auth/token' 500 when FAB auth manager is recreated#65710
vincbeck merged 13 commits into
apache:mainfrom
snowlaxc:fix/fab-auth-token-flask-app

Conversation

@snowlaxc
Copy link
Copy Markdown
Contributor

Fix '/auth/token' 500 when FAB auth manager is recreated

'init_auth_manager()' initializes the global auth manager and
'FabAuthManager.get_fastapi_app()' stores the Flask app used by the
FAB-backed auth routes on that instance.

Later, 'init_appbuilder()' calls 'create_auth_manager()' again. That
replaces the initialized global auth manager with a fresh instance that
does not have 'flask_app' set, causing '/auth/token' to fail with:

'Flask app is not initialized. Check that FabAuthManager started up correctly.'

This change reuses the already initialized auth manager when available
and only falls back to 'create_auth_manager()' if initialization has
not happened yet.

This was reproduced on Airflow 3.1.2 deployments using
'FabAuthManager', where:

  • 'POST /auth/token' returned '500' before the patch
  • empty request bodies return '400' after the patch
  • valid username/password requests return '201' and a JWT after the patch

Validation

Manually reproduced before the patch:

  • 'POST /auth/token' returned '500'

Manually validated after the patch:

  • empty request body returns '400'
  • valid username/password returns '201'

Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)

  • Read the Pull Request Guidelines for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
  • For fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
  • When adding dependency, check compliance with the ASF 3rd Party License Policy.
  • For significant user-facing changes create newsfragment: '{pr_number}.significant.rst', in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.

Prevent FAB auth manager recreation from breaking /auth/token

When FAB auth manager is initialized for the FastAPI app, the auth
manager instance stores the Flask app used by the FAB-backed auth
routes.

Later, init_appbuilder() recreates the global auth manager instance.
That replacement loses the initialized flask_app reference, so
/auth/token fails with:

"Flask app is not initialized. Check that FabAuthManager started up correctly."

Reuse the already initialized auth manager when present and only fall
back to create_auth_manager() if initialization has not happened yet.
@boring-cyborg
Copy link
Copy Markdown

boring-cyborg Bot commented Apr 23, 2026

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
Copy link
Copy Markdown
Contributor

CI is failing

Only reuse initialized FAB auth managers in init_appbuilder

The previous change reused the global auth manager whenever one was already initialized. That was too broad for test environments, where the global auth manager may exist but not be a FabAuthManager.

This updates the logic to reuse the existing auth manager only when it is already FAB-backed, and to fall back to  reate_auth_manager() otherwise.

This preserves the /auth/token fix while avoiding FAB CLI and auth manager test failures caused by reusing a non-FAB auth manager.
@snowlaxc snowlaxc requested a review from vincbeck April 24, 2026 01:59
Remove trailing whitespace from the blank line after the local FabAuthManager import in AppBuilder initialization. The import remains visually separated from the executable setup code, while matching the format expected by pre-commit hooks so CI does not fail with hook-made changes.
@snowlaxc
Copy link
Copy Markdown
Contributor Author

@vincbeck CI is now passing after the latest updates. Could you please take another look when you have a chance?

Comment thread providers/fab/src/airflow/providers/fab/www/extensions/init_appbuilder.py Outdated
Replace the generic auth-manager lookup in init_appbuilder with get_fab_auth_manager(), which already guarantees a FabAuthManager instance or raises when the configured manager is not FAB-based.
add import get_fab_auth_manager
@snowlaxc snowlaxc closed this Apr 24, 2026
@snowlaxc snowlaxc reopened this Apr 24, 2026
@vincbeck vincbeck merged commit 77fa348 into apache:main Apr 24, 2026
89 checks passed
@boring-cyborg
Copy link
Copy Markdown

boring-cyborg Bot commented Apr 24, 2026

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.

2 participants