Honor AUTH_ROLE_PUBLIC in FastAPI API server by gaurav0107 · Pull Request #65685 · apache/airflow

gaurav0107 · 2026-04-22T17:43:02Z

Summary

The FastAPI API server's auth gate rejected anonymous users with 401 even when AUTH_ROLE_PUBLIC was set in webserver_config.py, because the gate only consulted Flask's config. The Flask legacy webserver honored it; FastAPI did not.

Approach

Per @jason810496's proposal (endorsed by @vincbeck in the issue thread):

New [fab] auth_role_public Airflow config — the canonical source going forward.
New BaseAuthManager.get_public_user() hook (returns None by default).
FabAuthManager.get_public_user() resolves the configured role, builds an AnonymousUser with _roles/_perms pre-populated inside app_context(), and returns it.
resolve_user_from_token() in the FastAPI security layer consults auth_manager.get_public_user() before raising 401.
Startup bridge in providers/fab/www/app.py copies [fab] auth_role_public into Flask's AUTH_ROLE_PUBLIC so the legacy webserver's existing code paths stay in sync — no behavior change for users on webserver_config.py only.

Why the `AnonymousUser._roles` direct-write?

FAB's AnonymousUser.roles is a lazy property that calls security_manager.get_public_role() on every access, which requires a Flask request context. get_public_user() runs under FastAPI where there is no request context, so we build the user inside a brief app_context(), resolve permissions eagerly, and write them directly to _roles/_perms to freeze the snapshot. This is the minimum surface needed to avoid touching FAB's property implementation while keeping the call safe outside Flask.

Type of change

Bug fix (non-breaking change which fixes an issue)

Testing

28 new tests added in `airflow-core/tests/unit/api_fastapi/core_api/test_security.py`
84 new tests added in `providers/fab/tests/unit/fab/auth_manager/test_fab_auth_manager.py`
Covers: happy path, unknown role name, precedence between `[fab] auth_role_public` and legacy `AUTH_ROLE_PUBLIC`, interaction with the existing bearer-token path, edge cases for FastAPI-without-Flask-request
Full suite: 476 targeted auth tests pass. `ruff check` and `ruff format` pass.

Backwards compatibility

Users relying on `AUTH_ROLE_PUBLIC = "Admin"` in `webserver_config.py` continue to work unchanged — the Flask webserver keeps consulting that value, and the FastAPI gate now also honors it via the startup bridge.

cc @vincbeck @jason810496

🤖 Generated with Claude Code

The FastAPI API server's bearer-token auth raised 401 for anonymous users even when AUTH_ROLE_PUBLIC was set, because the gate only honored Flask's webserver_config.py and the FastAPI stack had no equivalent. This adds a new [fab] auth_role_public Airflow config, surfaced via a new BaseAuthManager.get_public_user() hook. FabAuthManager populates an AnonymousUser with the public role's permissions pre-expanded so it works outside a Flask request context. The legacy AUTH_ROLE_PUBLIC setting in webserver_config.py keeps working via a startup-time bridge that copies [fab] auth_role_public into Flask's config. Closes apache#60897 Co-Authored-By: Claude <noreply@anthropic.com>

boring-cyborg · 2026-04-22T17:43:06Z

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 · 2026-04-22T18:09:04Z

            log.error("Couldn't deserialize user from token, JWT token is not valid: %s", e)
            raise InvalidTokenError(str(e))

+    def get_public_user(self) -> BaseUser | None:


I am not a big fan of this new method, with simple auth manager we went with another approach. We went wth creating a middleware which automatically create a token. Please look at airflow-core/src/airflow/api_fastapi/auth/managers/simple/middleware.py

Hey @vincbeck, thanks so much for the pointer! Really appreciate you taking the time to flag this — the middleware pattern is much cleaner and I should have looked at SimpleAllAdminMiddleware first.

I've just pushed a refactor (6041356) that follows the same approach:

New FabAuthRolePublicMiddleware in providers/fab/auth_manager/ — mirrors the structure of SimpleAllAdminMiddleware

It attaches an AnonymousUser to request.state.user on unauthenticated requests (the existing get_user dependency already short-circuits on request.state.user, so I skipped the JWT-minting step that simple does)

BaseAuthManager now has a small generic get_fastapi_middlewares() hook so any auth manager can plug in middleware without touching airflow-core

security.py is back to the original upstream 401 behavior — no opinions baked into the core auth gate anymore

The middleware only registers when [fab] auth_role_public is configured, same gating as before.

Let me know if you'd like me to adjust anything — happy to iterate. 🙏

@vincbeck

Per @vincbeck's review, replace the BaseAuthManager.get_public_user() method with a middleware approach mirroring SimpleAllAdminMiddleware. - Add generic BaseAuthManager.get_fastapi_middlewares() extension hook that auth managers override to register their own middlewares. - New FabAuthRolePublicMiddleware attaches an AnonymousUser to request.state.user for unauthenticated requests when [fab] auth_role_public is configured. The FastAPI get_user dependency short-circuits on request.state.user, so no JWT minting is needed. - Revert security.py to upstream behavior (401 on missing token). - Rename FabAuthManager.get_public_user -> build_public_user and mark as :meta private: since it's an implementation detail consumed only by the middleware. Addresses apache#65685 (comment) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

vincbeck · 2026-04-22T19:27:09Z

-in ``$AIRFLOW_HOME/webserver_config.py`` needs to be set with the desired role that the Anonymous
-user will have by default:
+To deactivate the authentication and allow users to be identified as Anonymous, set the
+``[fab] auth_role_public`` Airflow configuration to the desired role that anonymous users will have


That is fab specific, it should not be in airflow core documentation

Good call — reverted the doc expansion entirely. The AUTH_ROLE_PUBLIC paragraph is back to the upstream wording, and the new [fab] auth_role_public config is documented via provider.yaml (which shows up in the generated config reference). Let me know if you'd rather I add a short sentence somewhere pointing users at the new key.

vincbeck · 2026-04-22T19:27:22Z

@@ -0,0 +1 @@
+Honor ``AUTH_ROLE_PUBLIC`` in the FastAPI-based API server. Add a new ``[fab] auth_role_public`` Airflow configuration that is used consistently by both the FastAPI and legacy Flask FAB auth stacks so that setting a public role actually removes the login prompt. Setting ``AUTH_ROLE_PUBLIC`` in ``webserver_config.py`` keeps working for backwards compatibility.


Removed the newsfragment.

vincbeck · 2026-04-22T19:30:06Z

+            return None
+
+        user = AnonymousUser()
+        flask_app = self.flask_app or (self.appbuilder.app if self.appbuilder else None)


self.appbuilder.app should be the only way to access the app. self.flask_app should be removed

Done — build_public_user no longer touches self.flask_app. It resolves the role via a SQLAlchemy session and sets _roles/_perms directly on the AnonymousUser, so no Flask context is opened in the auth manager.

vincbeck · 2026-04-22T19:33:57Z

+            with flask_app.app_context():
+                flask_app.config["AUTH_ROLE_PUBLIC"] = public_role_name
+                role = flask_app.appbuilder.sm.find_role(public_role_name)
+                if role is not None:
+                    # ``AnonymousUser.roles`` is a lazy property that calls
+                    # ``security_manager.get_public_role()`` on every access, which needs a Flask
+                    # request context we do not have under FastAPI. Writing ``_roles``/``_perms``
+                    # directly freezes a snapshot of the public role's permissions for the
+                    # lifetime of a single FastAPI authorization check (see #60897).
+                    user._roles = {role}
+                    user._perms = {(perm.action.name, perm.resource.name) for perm in role.permissions}


Instead of doing this, I think it would be simpler to create a User object by and (like simple auth manager). Ideally I do not want to create a Flask context in this file

Switched to the same shape as simple auth manager — no Flask context, no app_context(), no find_role hop through the security manager. The new version queries the Role via @provide_session (with permissions joined eager-loaded on the model) and snapshots _roles/_perms on the AnonymousUser before the session closes, so downstream _is_authorized checks only need user.perms (already materialized as tuples) and don't hit Flask at all.

One small caveat: unlike the simple auth manager, FAB's User is a SQLAlchemy model and its real login flow expects DB-backed roles, so I'm still using the existing AnonymousUser class rather than constructing a User(...) by hand — the pre-populated _roles/_perms path is exactly what AnonymousUser was designed for. Happy to go further if you'd prefer a purpose-built anonymous model though.

vincbeck · 2026-04-22T19:35:43Z

+    auth_role_public_conf = conf.get("fab", "auth_role_public", fallback="") or ""
+    if auth_role_public_conf:
+        flask_app.config["AUTH_ROLE_PUBLIC"] = auth_role_public_conf


Nice, since we are doing that, there is no need to check for conf.get("fab", "auth_role_public") in other places?

Yes — cleaned up. The only other caller was _get_auth_role_public in the FAB auth manager, and it now reads appbuilder.app.config["AUTH_ROLE_PUBLIC"] directly. The www/app.py bridge is the single place that reads the Airflow config, and everything else downstream reads the Flask config. There were no other conf.get("fab", "auth_role_public") call sites.

vincbeck · 2026-04-22T19:36:27Z

@@ -174,6 +175,9 @@ def init_middlewares(app: FastAPI) -> None:

        app.add_middleware(SimpleAllAdminMiddleware)


This then should be removed and simple auth manager should override get_fastapi_middlewares

Done — removed the hardcoded SimpleAllAdminMiddleware branch from init_middlewares and pushed the registration into SimpleAuthManager.get_fastapi_middlewares. Both FAB and simple now go through the same generic hook. Added unit tests for the new override too.

potiuk · 2026-04-22T20:04:56Z

@gaurav0107 This PR has been converted to draft because it does not yet meet our Pull Request quality criteria.

Issues found:

❌ Unresolved review comments (7 threads from maintainers): please walk through each unresolved review thread. Even if a suggestion looks incorrect or irrelevant — and some of them will be, especially any comments left by automated reviewers like GitHub Copilot — it is still the author's responsibility to respond: apply the fix, reply in-thread with a brief explanation of why the suggestion does not apply, or resolve the thread if the feedback is no longer relevant. Leaving threads unaddressed for weeks blocks the PR from moving forward.

What to do next:

Walk through each unresolved review thread and respond as described above.
Make sure static checks pass locally: prek run --from-ref main --stage pre-commit.
Mark the PR as "Ready for review" when you're done.

Converting a PR to draft is not a rejection — it is an invitation to bring the PR up to the project's standards so that maintainer review time is spent productively. There is no rush — take your time and work at your own pace. We appreciate your contribution and are happy to wait for updates. If you have questions, feel free to ask on the Airflow Slack.

@vincbeck

…iring Follow-up review from @vincbeck on PR apache#65685. Shrinks the diff and aligns the FAB public-access path with the simple auth manager's patterns. - ``FabAuthManager.build_public_user`` no longer opens a Flask app context or uses ``self.flask_app``. It resolves the public role via a SQLAlchemy session (``@provide_session``), eager-loaded joined permissions, and writes ``_roles``/``_perms`` directly on the ``AnonymousUser``. No Flask context is created in the auth manager. - ``FabAuthManager._get_auth_role_public`` reads only the Flask app config. The ``providers/fab/www/app.py`` bridge already copies ``[fab] auth_role_public`` into ``AUTH_ROLE_PUBLIC`` at app creation, so there's a single source of truth and no duplicate ``conf.get``. - ``SimpleAuthManager`` now overrides ``get_fastapi_middlewares`` to register ``SimpleAllAdminMiddleware`` when ``[core] simple_auth_manager_all_admins`` is set. The hardcoded registration in ``init_middlewares`` is removed so all auth managers go through the same generic hook. - Revert the ``webserver-authentication.rst`` expansion; the existing ``AUTH_ROLE_PUBLIC`` paragraph is enough and the new config key is documented via ``provider.yaml``. - Drop the 65685 newsfragment per maintainer feedback. Tests updated to match the new behavior (Flask config is the single source of truth read by the auth manager) and extended with two new tests for ``SimpleAuthManager.get_fastapi_middlewares``. Co-Authored-By: Claude <noreply@anthropic.com>

potiuk · 2026-04-22T22:19:29Z

Quick follow-up to the triage comment above — one clarification on the "Unresolved review comments" item:

Once you believe a thread has been addressed — whether by pushing a fix, or by replying in-thread with an explanation of why the suggestion doesn't apply — please mark the thread as resolved yourself by clicking the "Resolve conversation" button at the bottom of each thread. Reviewers don't auto-close their own threads, so an addressed-but-unresolved thread reads as "still waiting on the author" and keeps the PR from moving forward. The author doing the resolve-click is the expected convention on this project.

gaurav0107 requested review from bugraoz93, choo121600, ephraimbuddy, jason810496, pierrejeambrun, rawwar, shubhamraj-git and vincbeck as code owners April 22, 2026 17:43

boring-cyborg Bot added area:API Airflow's REST/HTTP API area:providers kind:documentation provider:fab labels Apr 22, 2026

gaurav0107 mentioned this pull request Apr 22, 2026

AUTH_ROLE_PUBLIC = 'Admin' # doesn't remove the login dialog #60897

Open

2 tasks

vincbeck reviewed Apr 22, 2026

View reviewed changes

potiuk marked this pull request as draft April 22, 2026 20:04

		@@ -0,0 +1 @@
		Honor ``AUTH_ROLE_PUBLIC`` in the FastAPI-based API server. Add a new ``[fab] auth_role_public`` Airflow configuration that is used consistently by both the FastAPI and legacy Flask FAB auth stacks so that setting a public role actually removes the login prompt. Setting ``AUTH_ROLE_PUBLIC`` in ``webserver_config.py`` keeps working for backwards compatibility.

		@@ -174,6 +175,9 @@ def init_middlewares(app: FastAPI) -> None:

		app.add_middleware(SimpleAllAdminMiddleware)

Conversation

gaurav0107 commented Apr 22, 2026

Summary

Approach

Why the AnonymousUser._roles direct-write?

Type of change

Testing

Backwards compatibility

Uh oh!

boring-cyborg Bot commented Apr 22, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

potiuk commented Apr 22, 2026

Uh oh!

potiuk commented Apr 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Why the `AnonymousUser._roles` direct-write?