From aefa5eae01ebe5fd5db6bb4898ed5faee53b5db1 Mon Sep 17 00:00:00 2001 From: gaurav0107 Date: Wed, 22 Apr 2026 23:07:42 +0530 Subject: [PATCH 1/3] Honor AUTH_ROLE_PUBLIC in FastAPI API server 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 #60897 Co-Authored-By: Claude --- .../auth/managers/base_auth_manager.py | 14 ++++ .../airflow/api_fastapi/core_api/security.py | 9 +- .../api_fastapi/core_api/test_security.py | 28 +++++++ .../auth-manager/webserver-authentication.rst | 24 +++++- providers/fab/newsfragments/60897.bugfix.rst | 1 + providers/fab/provider.yaml | 16 ++++ .../fab/auth_manager/fab_auth_manager.py | 48 ++++++++++- .../fab/src/airflow/providers/fab/www/app.py | 9 ++ .../fab/auth_manager/test_fab_auth_manager.py | 84 +++++++++++++++++++ 9 files changed, 226 insertions(+), 7 deletions(-) create mode 100644 providers/fab/newsfragments/60897.bugfix.rst diff --git a/airflow-core/src/airflow/api_fastapi/auth/managers/base_auth_manager.py b/airflow-core/src/airflow/api_fastapi/auth/managers/base_auth_manager.py index a26a4413aadf3..655f3ffffca6e 100644 --- a/airflow-core/src/airflow/api_fastapi/auth/managers/base_auth_manager.py +++ b/airflow-core/src/airflow/api_fastapi/auth/managers/base_auth_manager.py @@ -158,6 +158,20 @@ async def get_user_from_token(self, token: str) -> BaseUser: 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: + """ + Return a user representing anonymous/public access, or ``None`` if not supported. + + Auth managers that support unauthenticated access (for example, the FAB auth manager's + ``[fab] auth_role_public`` configuration) should override this method to return a user + object when public access is enabled. When a user is returned, the API server will use it + for requests that do not carry any authentication token instead of returning 401. + + By default this method returns ``None``, meaning the auth manager does not permit + anonymous access and a valid token is always required. + """ + return None + def generate_jwt( self, user: T, *, expiration_time_in_seconds: int = conf.getint("api_auth", "jwt_expiration_time") ) -> str: diff --git a/airflow-core/src/airflow/api_fastapi/core_api/security.py b/airflow-core/src/airflow/api_fastapi/core_api/security.py index 774cfa9ed6a74..72043c833a0c7 100644 --- a/airflow-core/src/airflow/api_fastapi/core_api/security.py +++ b/airflow-core/src/airflow/api_fastapi/core_api/security.py @@ -114,11 +114,18 @@ def auth_manager_from_app(request: Request) -> BaseAuthManager: async def resolve_user_from_token(token_str: str | None) -> BaseUser: + auth_manager = get_auth_manager() if not token_str: + # When the auth manager supports anonymous/public access (e.g. FAB's + # ``[fab] auth_role_public`` setting), fall back to the public user instead of + # rejecting the request with 401. + public_user = auth_manager.get_public_user() + if public_user is not None: + return public_user raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Not authenticated") try: - return await get_auth_manager().get_user_from_token(token_str) + return await auth_manager.get_user_from_token(token_str) except ExpiredSignatureError: raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Token Expired") except InvalidTokenError: diff --git a/airflow-core/tests/unit/api_fastapi/core_api/test_security.py b/airflow-core/tests/unit/api_fastapi/core_api/test_security.py index e5cb5f17cf98e..1dce97696599a 100644 --- a/airflow-core/tests/unit/api_fastapi/core_api/test_security.py +++ b/airflow-core/tests/unit/api_fastapi/core_api/test_security.py @@ -110,6 +110,34 @@ async def test_get_user_expired_token(self, mock_get_auth_manager): auth_manager.get_user_from_token.assert_called_once_with(token_str) + @patch("airflow.api_fastapi.core_api.security.get_auth_manager") + async def test_resolve_user_from_token_no_token_raises_when_no_public_user(self, mock_get_auth_manager): + """No token and auth manager has no public user → 401.""" + auth_manager = AsyncMock() + auth_manager.get_public_user = Mock(return_value=None) + mock_get_auth_manager.return_value = auth_manager + + with pytest.raises(HTTPException, match="Not authenticated"): + await resolve_user_from_token(None) + + auth_manager.get_public_user.assert_called_once_with() + auth_manager.get_user_from_token.assert_not_called() + + @patch("airflow.api_fastapi.core_api.security.get_auth_manager") + async def test_resolve_user_from_token_no_token_returns_public_user(self, mock_get_auth_manager): + """No token but auth manager exposes a public user → return it instead of 401.""" + public_user = SimpleAuthManagerUser(username="public", role="admin") + + auth_manager = AsyncMock() + auth_manager.get_public_user = Mock(return_value=public_user) + mock_get_auth_manager.return_value = auth_manager + + result = await resolve_user_from_token(None) + + assert result is public_user + auth_manager.get_public_user.assert_called_once_with() + auth_manager.get_user_from_token.assert_not_called() + @patch("airflow.api_fastapi.core_api.security.resolve_user_from_token") async def test_get_user_with_request_state(self, mock_resolve_user_from_token): user = Mock() diff --git a/providers/fab/docs/auth-manager/webserver-authentication.rst b/providers/fab/docs/auth-manager/webserver-authentication.rst index e950560a94aac..7a00a64a188ff 100644 --- a/providers/fab/docs/auth-manager/webserver-authentication.rst +++ b/providers/fab/docs/auth-manager/webserver-authentication.rst @@ -42,9 +42,27 @@ following CLI commands to create an account: --role Admin \ --email spiderman@superhero.org -To deactivate the authentication and allow users to be identified as Anonymous, the following entry -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 +by default. For example, in ``airflow.cfg``: + +.. code-block:: ini + + [fab] + auth_role_public = Admin + +or via the equivalent environment variable: + +.. code-block:: bash + + export AIRFLOW__FAB__AUTH_ROLE_PUBLIC=Admin + +This makes both the FastAPI-based API server and the legacy Flask-based views honor anonymous +access consistently. + +For backwards compatibility, setting ``AUTH_ROLE_PUBLIC`` in ``$AIRFLOW_HOME/webserver_config.py`` +is still supported, but the ``[fab] auth_role_public`` Airflow config takes precedence when both +are set and is the recommended configuration going forward: .. code-block:: ini diff --git a/providers/fab/newsfragments/60897.bugfix.rst b/providers/fab/newsfragments/60897.bugfix.rst new file mode 100644 index 0000000000000..0792fdada92d5 --- /dev/null +++ b/providers/fab/newsfragments/60897.bugfix.rst @@ -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. diff --git a/providers/fab/provider.yaml b/providers/fab/provider.yaml index 7a6d72fa20922..0c0d244287da4 100644 --- a/providers/fab/provider.yaml +++ b/providers/fab/provider.yaml @@ -263,6 +263,22 @@ config: type: integer example: ~ default: "30" + auth_role_public: + description: | + Role that Anonymous (unauthenticated) users are granted. When set, the FAB auth manager + will allow access to the API server and UI without requiring a login, and anonymous + requests will be treated as members of the given role. Leave empty (the default) to + require authentication. + + This replaces the previous ``AUTH_ROLE_PUBLIC`` setting in ``webserver_config.py``. When + both are set, this ``[fab] auth_role_public`` config takes precedence. Setting this + config also applies the equivalent ``AUTH_ROLE_PUBLIC`` to the Flask app used by the FAB + auth manager, so all FAB auth code paths (FastAPI-based API server and legacy Flask + views) honor it consistently. + version_added: 3.6.2 + type: string + example: "Admin" + default: "" auth-managers: - airflow.providers.fab.auth_manager.fab_auth_manager.FabAuthManager diff --git a/providers/fab/src/airflow/providers/fab/auth_manager/fab_auth_manager.py b/providers/fab/src/airflow/providers/fab/auth_manager/fab_auth_manager.py index 61bbf142dc2e2..d1066e8a18f29 100644 --- a/providers/fab/src/airflow/providers/fab/auth_manager/fab_auth_manager.py +++ b/providers/fab/src/airflow/providers/fab/auth_manager/fab_auth_manager.py @@ -300,11 +300,53 @@ def is_logged_in(self) -> bool: """Return whether the user is logged in.""" user = self.get_user() return bool( - self.appbuilder - and self.appbuilder.app.config.get("AUTH_ROLE_PUBLIC", None) - or (not user.is_anonymous and user.is_active) + (self.appbuilder and self._get_auth_role_public()) or (not user.is_anonymous and user.is_active) ) + def _get_auth_role_public(self) -> str | None: + """ + Return the role granted to anonymous users, or ``None`` if public access is disabled. + + Prefers the Airflow ``[fab] auth_role_public`` configuration and falls back to the legacy + ``AUTH_ROLE_PUBLIC`` entry in ``webserver_config.py`` so existing deployments continue to + work. + """ + role = conf.get("fab", "auth_role_public", fallback="") or "" + if role: + return role + if self.appbuilder is not None: + return self.appbuilder.app.config.get("AUTH_ROLE_PUBLIC", None) + return None + + def get_public_user(self) -> AnonymousUser | None: + """ + Return an :class:`AnonymousUser` when public access is enabled, else ``None``. + + Public access is enabled when the Airflow ``[fab] auth_role_public`` config (or, for + backwards compatibility, ``AUTH_ROLE_PUBLIC`` in ``webserver_config.py``) is set. The + returned user inherits the role configured there and is used by the FastAPI API server + to authorize requests that do not carry a JWT token. + """ + public_role_name = self._get_auth_role_public() + if not public_role_name: + return None + + user = AnonymousUser() + flask_app = self.flask_app or (self.appbuilder.app if self.appbuilder else None) + if flask_app is not None: + 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: + # FAB's ``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} + return user + def create_token(self, headers: dict[str, str], body: dict[str, Any]) -> User | None: """ Create a new token from a payload. diff --git a/providers/fab/src/airflow/providers/fab/www/app.py b/providers/fab/src/airflow/providers/fab/www/app.py index 765d942ccc7a4..28bdae3418983 100644 --- a/providers/fab/src/airflow/providers/fab/www/app.py +++ b/providers/fab/src/airflow/providers/fab/www/app.py @@ -84,6 +84,15 @@ def remove_duplicate_date_header(response): with flask_app.app_context(): flask_app.config.from_pyfile(webserver_config, silent=True) + # Bridge ``[fab] auth_role_public`` Airflow config into the Flask app config so legacy FAB + # code paths that read ``AUTH_ROLE_PUBLIC`` from ``current_app.config`` (e.g. + # ``AnonymousUser.roles``, basic_auth, kerberos_auth, security manager) stay in sync with + # the FastAPI-based auth flow. The Airflow config takes precedence over + # ``webserver_config.py`` when both are set so there is a single source of truth. + 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 + url = make_url(flask_app.config["SQLALCHEMY_DATABASE_URI"]) if url.drivername == "sqlite" and url.database and not isabs(url.database): raise AirflowConfigException( diff --git a/providers/fab/tests/unit/fab/auth_manager/test_fab_auth_manager.py b/providers/fab/tests/unit/fab/auth_manager/test_fab_auth_manager.py index 99f7dc24edc23..b87b523980d90 100644 --- a/providers/fab/tests/unit/fab/auth_manager/test_fab_auth_manager.py +++ b/providers/fab/tests/unit/fab/auth_manager/test_fab_auth_manager.py @@ -262,6 +262,90 @@ def test_is_logged_in_with_inactive_user(self, mock_get_user, auth_manager_with_ assert auth_manager_with_appbuilder.is_logged_in() is False + @mock.patch.object(FabAuthManager, "get_user") + def test_is_logged_in_with_auth_role_public_conf(self, mock_get_user, auth_manager_with_appbuilder): + """``[fab] auth_role_public`` Airflow config is enough to be 'logged in'.""" + user = Mock() + user.is_anonymous.return_value = True + user.is_active.return_value = False + mock_get_user.return_value = user + + with conf_vars({("fab", "auth_role_public"): "Admin"}): + assert auth_manager_with_appbuilder.is_logged_in() is True + + def test_get_public_user_returns_none_when_not_configured(self, auth_manager_with_appbuilder): + """Without ``[fab] auth_role_public`` or ``AUTH_ROLE_PUBLIC``, there is no public user.""" + flask_app = auth_manager_with_appbuilder.flask_app or (auth_manager_with_appbuilder.appbuilder.app) + previous = flask_app.config.get("AUTH_ROLE_PUBLIC") + flask_app.config["AUTH_ROLE_PUBLIC"] = None + try: + with conf_vars({("fab", "auth_role_public"): ""}): + assert auth_manager_with_appbuilder.get_public_user() is None + finally: + flask_app.config["AUTH_ROLE_PUBLIC"] = previous + + def test_get_public_user_returns_anonymous_user_from_conf(self, auth_manager_with_appbuilder): + """``[fab] auth_role_public`` yields an :class:`AnonymousUser` with the role resolved.""" + from airflow.providers.fab.auth_manager.models.anonymous_user import AnonymousUser + + with conf_vars({("fab", "auth_role_public"): "Admin"}): + user = auth_manager_with_appbuilder.get_public_user() + + assert isinstance(user, AnonymousUser) + assert len(user.roles) == 1 + assert user.roles[0].name == "Admin" + # ``perms`` must be pre-populated so FastAPI can evaluate authorization outside a + # Flask request/app context. + assert user._perms, "Expected permissions to be pre-populated on the public user" + + def test_get_public_user_falls_back_to_flask_config(self, auth_manager_with_appbuilder): + """Legacy ``AUTH_ROLE_PUBLIC`` in ``webserver_config.py`` is still honored.""" + from airflow.providers.fab.auth_manager.models.anonymous_user import AnonymousUser + + flask_app = auth_manager_with_appbuilder.flask_app or (auth_manager_with_appbuilder.appbuilder.app) + previous = flask_app.config.get("AUTH_ROLE_PUBLIC") + flask_app.config["AUTH_ROLE_PUBLIC"] = "Admin" + try: + with conf_vars({("fab", "auth_role_public"): ""}): + user = auth_manager_with_appbuilder.get_public_user() + finally: + flask_app.config["AUTH_ROLE_PUBLIC"] = previous + + assert isinstance(user, AnonymousUser) + assert len(user.roles) == 1 + assert user.roles[0].name == "Admin" + + def test_get_public_user_with_unknown_role_returns_user_with_empty_perms( + self, auth_manager_with_appbuilder + ): + """A misconfigured role name still yields an :class:`AnonymousUser` with empty perms.""" + from airflow.providers.fab.auth_manager.models.anonymous_user import AnonymousUser + + with conf_vars({("fab", "auth_role_public"): "DoesNotExist"}): + user = auth_manager_with_appbuilder.get_public_user() + + assert isinstance(user, AnonymousUser) + # No pre-populated perms means the user has no authorization; the role will also not + # be resolved since ``find_role`` returned None for a non-existent role. + assert user._perms == set() + + def test_get_public_user_airflow_conf_takes_precedence(self, auth_manager_with_appbuilder): + """When both ``[fab] auth_role_public`` and the Flask config are set, Airflow config wins.""" + from airflow.providers.fab.auth_manager.models.anonymous_user import AnonymousUser + + flask_app = auth_manager_with_appbuilder.flask_app or (auth_manager_with_appbuilder.appbuilder.app) + previous = flask_app.config.get("AUTH_ROLE_PUBLIC") + flask_app.config["AUTH_ROLE_PUBLIC"] = "Viewer" + try: + with conf_vars({("fab", "auth_role_public"): "Admin"}): + user = auth_manager_with_appbuilder.get_public_user() + finally: + flask_app.config["AUTH_ROLE_PUBLIC"] = previous + + assert isinstance(user, AnonymousUser) + assert len(user.roles) == 1 + assert user.roles[0].name == "Admin" + @pytest.mark.parametrize( ("auth_type", "method"), [ From 6041356ee4462d50398db49f41c1fa49129aefa2 Mon Sep 17 00:00:00 2001 From: gaurav0107 Date: Thu, 23 Apr 2026 00:46:24 +0530 Subject: [PATCH 2/3] Refactor AUTH_ROLE_PUBLIC support to use middleware pattern 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 https://github.com/apache/airflow/pull/65685#discussion_r3125995253 Co-Authored-By: Claude Opus 4.7 (1M context) --- .../auth/managers/base_auth_manager.py | 18 ++-- .../src/airflow/api_fastapi/core_api/app.py | 4 + .../airflow/api_fastapi/core_api/security.py | 9 +- .../api_fastapi/core_api/test_security.py | 27 +----- .../{60897.bugfix.rst => 65685.bugfix.rst} | 0 .../fab/auth_manager/fab_auth_manager.py | 22 +++-- .../providers/fab/auth_manager/middleware.py | 54 +++++++++++ .../fab/auth_manager/test_fab_auth_manager.py | 40 ++++++-- .../unit/fab/auth_manager/test_middleware.py | 96 +++++++++++++++++++ 9 files changed, 210 insertions(+), 60 deletions(-) rename providers/fab/newsfragments/{60897.bugfix.rst => 65685.bugfix.rst} (100%) create mode 100644 providers/fab/src/airflow/providers/fab/auth_manager/middleware.py create mode 100644 providers/fab/tests/unit/fab/auth_manager/test_middleware.py diff --git a/airflow-core/src/airflow/api_fastapi/auth/managers/base_auth_manager.py b/airflow-core/src/airflow/api_fastapi/auth/managers/base_auth_manager.py index 655f3ffffca6e..9d139c883b399 100644 --- a/airflow-core/src/airflow/api_fastapi/auth/managers/base_auth_manager.py +++ b/airflow-core/src/airflow/api_fastapi/auth/managers/base_auth_manager.py @@ -158,19 +158,17 @@ async def get_user_from_token(self, token: str) -> BaseUser: 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: + def get_fastapi_middlewares(self) -> list[tuple[type, dict[str, Any]]]: """ - Return a user representing anonymous/public access, or ``None`` if not supported. + Return middlewares the auth manager wants registered on the main FastAPI app. - Auth managers that support unauthenticated access (for example, the FAB auth manager's - ``[fab] auth_role_public`` configuration) should override this method to return a user - object when public access is enabled. When a user is returned, the API server will use it - for requests that do not carry any authentication token instead of returning 401. - - By default this method returns ``None``, meaning the auth manager does not permit - anonymous access and a valid token is always required. + Each entry is a ``(middleware_class, kwargs)`` tuple and is registered via + ``app.add_middleware`` by the API server. Auth managers that need to intercept or + augment incoming requests (for example, the FAB auth manager attaching an + anonymous user to unauthenticated requests when ``[fab] auth_role_public`` is + configured) should override this method. """ - return None + return [] def generate_jwt( self, user: T, *, expiration_time_in_seconds: int = conf.getint("api_auth", "jwt_expiration_time") diff --git a/airflow-core/src/airflow/api_fastapi/core_api/app.py b/airflow-core/src/airflow/api_fastapi/core_api/app.py index 16719e3f6d5bd..b55f78b890e97 100644 --- a/airflow-core/src/airflow/api_fastapi/core_api/app.py +++ b/airflow-core/src/airflow/api_fastapi/core_api/app.py @@ -165,6 +165,7 @@ def init_error_handlers(app: FastAPI) -> None: def init_middlewares(app: FastAPI) -> None: + from airflow.api_fastapi.app import get_auth_manager from airflow.api_fastapi.auth.middlewares.refresh_token import JWTRefreshMiddleware from airflow.api_fastapi.common.http_access_log import HttpAccessLogMiddleware @@ -174,6 +175,9 @@ def init_middlewares(app: FastAPI) -> None: app.add_middleware(SimpleAllAdminMiddleware) + for middleware_cls, middleware_kwargs in get_auth_manager().get_fastapi_middlewares(): + app.add_middleware(middleware_cls, **middleware_kwargs) + # GZipMiddleware must be inside HttpAccessLogMiddleware so that access logs capture # the full end-to-end duration including compression time. # See https://github.com/apache/airflow/issues/60165 diff --git a/airflow-core/src/airflow/api_fastapi/core_api/security.py b/airflow-core/src/airflow/api_fastapi/core_api/security.py index 72043c833a0c7..774cfa9ed6a74 100644 --- a/airflow-core/src/airflow/api_fastapi/core_api/security.py +++ b/airflow-core/src/airflow/api_fastapi/core_api/security.py @@ -114,18 +114,11 @@ def auth_manager_from_app(request: Request) -> BaseAuthManager: async def resolve_user_from_token(token_str: str | None) -> BaseUser: - auth_manager = get_auth_manager() if not token_str: - # When the auth manager supports anonymous/public access (e.g. FAB's - # ``[fab] auth_role_public`` setting), fall back to the public user instead of - # rejecting the request with 401. - public_user = auth_manager.get_public_user() - if public_user is not None: - return public_user raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Not authenticated") try: - return await auth_manager.get_user_from_token(token_str) + return await get_auth_manager().get_user_from_token(token_str) except ExpiredSignatureError: raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Token Expired") except InvalidTokenError: diff --git a/airflow-core/tests/unit/api_fastapi/core_api/test_security.py b/airflow-core/tests/unit/api_fastapi/core_api/test_security.py index 1dce97696599a..0d05a0fdc78c3 100644 --- a/airflow-core/tests/unit/api_fastapi/core_api/test_security.py +++ b/airflow-core/tests/unit/api_fastapi/core_api/test_security.py @@ -110,34 +110,11 @@ async def test_get_user_expired_token(self, mock_get_auth_manager): auth_manager.get_user_from_token.assert_called_once_with(token_str) - @patch("airflow.api_fastapi.core_api.security.get_auth_manager") - async def test_resolve_user_from_token_no_token_raises_when_no_public_user(self, mock_get_auth_manager): - """No token and auth manager has no public user → 401.""" - auth_manager = AsyncMock() - auth_manager.get_public_user = Mock(return_value=None) - mock_get_auth_manager.return_value = auth_manager - + async def test_resolve_user_from_token_no_token_raises(self): + """No token always produces 401; public-user fallback is handled by middleware, not here.""" with pytest.raises(HTTPException, match="Not authenticated"): await resolve_user_from_token(None) - auth_manager.get_public_user.assert_called_once_with() - auth_manager.get_user_from_token.assert_not_called() - - @patch("airflow.api_fastapi.core_api.security.get_auth_manager") - async def test_resolve_user_from_token_no_token_returns_public_user(self, mock_get_auth_manager): - """No token but auth manager exposes a public user → return it instead of 401.""" - public_user = SimpleAuthManagerUser(username="public", role="admin") - - auth_manager = AsyncMock() - auth_manager.get_public_user = Mock(return_value=public_user) - mock_get_auth_manager.return_value = auth_manager - - result = await resolve_user_from_token(None) - - assert result is public_user - auth_manager.get_public_user.assert_called_once_with() - auth_manager.get_user_from_token.assert_not_called() - @patch("airflow.api_fastapi.core_api.security.resolve_user_from_token") async def test_get_user_with_request_state(self, mock_resolve_user_from_token): user = Mock() diff --git a/providers/fab/newsfragments/60897.bugfix.rst b/providers/fab/newsfragments/65685.bugfix.rst similarity index 100% rename from providers/fab/newsfragments/60897.bugfix.rst rename to providers/fab/newsfragments/65685.bugfix.rst diff --git a/providers/fab/src/airflow/providers/fab/auth_manager/fab_auth_manager.py b/providers/fab/src/airflow/providers/fab/auth_manager/fab_auth_manager.py index d1066e8a18f29..110ac7c18b3d4 100644 --- a/providers/fab/src/airflow/providers/fab/auth_manager/fab_auth_manager.py +++ b/providers/fab/src/airflow/providers/fab/auth_manager/fab_auth_manager.py @@ -318,14 +318,14 @@ def _get_auth_role_public(self) -> str | None: return self.appbuilder.app.config.get("AUTH_ROLE_PUBLIC", None) return None - def get_public_user(self) -> AnonymousUser | None: + def build_public_user(self) -> AnonymousUser | None: """ - Return an :class:`AnonymousUser` when public access is enabled, else ``None``. + Build an :class:`AnonymousUser` pre-populated with the configured public role. - Public access is enabled when the Airflow ``[fab] auth_role_public`` config (or, for - backwards compatibility, ``AUTH_ROLE_PUBLIC`` in ``webserver_config.py``) is set. The - returned user inherits the role configured there and is used by the FastAPI API server - to authorize requests that do not carry a JWT token. + Returns ``None`` when ``[fab] auth_role_public`` (or the legacy + ``AUTH_ROLE_PUBLIC`` entry in ``webserver_config.py``) is not set. + + :meta private: """ public_role_name = self._get_auth_role_public() if not public_role_name: @@ -338,7 +338,7 @@ def get_public_user(self) -> AnonymousUser | None: flask_app.config["AUTH_ROLE_PUBLIC"] = public_role_name role = flask_app.appbuilder.sm.find_role(public_role_name) if role is not None: - # FAB's ``AnonymousUser.roles`` is a lazy property that calls + # ``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 @@ -347,6 +347,14 @@ def get_public_user(self) -> AnonymousUser | None: user._perms = {(perm.action.name, perm.resource.name) for perm in role.permissions} return user + def get_fastapi_middlewares(self) -> list[tuple[type, dict[str, Any]]]: + """Register the FAB public-access middleware when public access is configured.""" + if not self._get_auth_role_public(): + return [] + from airflow.providers.fab.auth_manager.middleware import FabAuthRolePublicMiddleware + + return [(FabAuthRolePublicMiddleware, {})] + def create_token(self, headers: dict[str, str], body: dict[str, Any]) -> User | None: """ Create a new token from a payload. diff --git a/providers/fab/src/airflow/providers/fab/auth_manager/middleware.py b/providers/fab/src/airflow/providers/fab/auth_manager/middleware.py new file mode 100644 index 0000000000000..8a160453f1f16 --- /dev/null +++ b/providers/fab/src/airflow/providers/fab/auth_manager/middleware.py @@ -0,0 +1,54 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from __future__ import annotations + +from typing import TYPE_CHECKING + +from starlette.middleware.base import BaseHTTPMiddleware + +from airflow.api_fastapi.app import get_auth_manager +from airflow.api_fastapi.auth.managers.base_auth_manager import COOKIE_NAME_JWT_TOKEN + +if TYPE_CHECKING: + from fastapi import Request + + +class FabAuthRolePublicMiddleware(BaseHTTPMiddleware): + """ + Attach an anonymous user to unauthenticated requests when public access is enabled. + + When ``[fab] auth_role_public`` (or the legacy ``AUTH_ROLE_PUBLIC`` entry in + ``webserver_config.py``) is set, requests that do not carry any authentication + token get a pre-populated :class:`AnonymousUser` assigned to + ``request.state.user``. The FastAPI ``get_user`` dependency picks it up before + attempting JWT validation, which would otherwise reject the request with 401. + """ + + async def dispatch(self, request: Request, call_next): + if not self._has_auth_token(request): + public_user = get_auth_manager().build_public_user() + if public_user is not None: + request.state.user = public_user + return await call_next(request) + + @staticmethod + def _has_auth_token(request: Request) -> bool: + auth_header = request.headers.get("authorization") + if auth_header: + return True + return bool(request.cookies.get(COOKIE_NAME_JWT_TOKEN)) diff --git a/providers/fab/tests/unit/fab/auth_manager/test_fab_auth_manager.py b/providers/fab/tests/unit/fab/auth_manager/test_fab_auth_manager.py index b87b523980d90..93958189c3fbc 100644 --- a/providers/fab/tests/unit/fab/auth_manager/test_fab_auth_manager.py +++ b/providers/fab/tests/unit/fab/auth_manager/test_fab_auth_manager.py @@ -273,23 +273,23 @@ def test_is_logged_in_with_auth_role_public_conf(self, mock_get_user, auth_manag with conf_vars({("fab", "auth_role_public"): "Admin"}): assert auth_manager_with_appbuilder.is_logged_in() is True - def test_get_public_user_returns_none_when_not_configured(self, auth_manager_with_appbuilder): + def test_build_public_user_returns_none_when_not_configured(self, auth_manager_with_appbuilder): """Without ``[fab] auth_role_public`` or ``AUTH_ROLE_PUBLIC``, there is no public user.""" flask_app = auth_manager_with_appbuilder.flask_app or (auth_manager_with_appbuilder.appbuilder.app) previous = flask_app.config.get("AUTH_ROLE_PUBLIC") flask_app.config["AUTH_ROLE_PUBLIC"] = None try: with conf_vars({("fab", "auth_role_public"): ""}): - assert auth_manager_with_appbuilder.get_public_user() is None + assert auth_manager_with_appbuilder.build_public_user() is None finally: flask_app.config["AUTH_ROLE_PUBLIC"] = previous - def test_get_public_user_returns_anonymous_user_from_conf(self, auth_manager_with_appbuilder): + def test_build_public_user_returns_anonymous_user_from_conf(self, auth_manager_with_appbuilder): """``[fab] auth_role_public`` yields an :class:`AnonymousUser` with the role resolved.""" from airflow.providers.fab.auth_manager.models.anonymous_user import AnonymousUser with conf_vars({("fab", "auth_role_public"): "Admin"}): - user = auth_manager_with_appbuilder.get_public_user() + user = auth_manager_with_appbuilder.build_public_user() assert isinstance(user, AnonymousUser) assert len(user.roles) == 1 @@ -298,7 +298,7 @@ def test_get_public_user_returns_anonymous_user_from_conf(self, auth_manager_wit # Flask request/app context. assert user._perms, "Expected permissions to be pre-populated on the public user" - def test_get_public_user_falls_back_to_flask_config(self, auth_manager_with_appbuilder): + def test_build_public_user_falls_back_to_flask_config(self, auth_manager_with_appbuilder): """Legacy ``AUTH_ROLE_PUBLIC`` in ``webserver_config.py`` is still honored.""" from airflow.providers.fab.auth_manager.models.anonymous_user import AnonymousUser @@ -307,7 +307,7 @@ def test_get_public_user_falls_back_to_flask_config(self, auth_manager_with_appb flask_app.config["AUTH_ROLE_PUBLIC"] = "Admin" try: with conf_vars({("fab", "auth_role_public"): ""}): - user = auth_manager_with_appbuilder.get_public_user() + user = auth_manager_with_appbuilder.build_public_user() finally: flask_app.config["AUTH_ROLE_PUBLIC"] = previous @@ -315,21 +315,21 @@ def test_get_public_user_falls_back_to_flask_config(self, auth_manager_with_appb assert len(user.roles) == 1 assert user.roles[0].name == "Admin" - def test_get_public_user_with_unknown_role_returns_user_with_empty_perms( + def test_build_public_user_with_unknown_role_returns_user_with_empty_perms( self, auth_manager_with_appbuilder ): """A misconfigured role name still yields an :class:`AnonymousUser` with empty perms.""" from airflow.providers.fab.auth_manager.models.anonymous_user import AnonymousUser with conf_vars({("fab", "auth_role_public"): "DoesNotExist"}): - user = auth_manager_with_appbuilder.get_public_user() + user = auth_manager_with_appbuilder.build_public_user() assert isinstance(user, AnonymousUser) # No pre-populated perms means the user has no authorization; the role will also not # be resolved since ``find_role`` returned None for a non-existent role. assert user._perms == set() - def test_get_public_user_airflow_conf_takes_precedence(self, auth_manager_with_appbuilder): + def test_build_public_user_airflow_conf_takes_precedence(self, auth_manager_with_appbuilder): """When both ``[fab] auth_role_public`` and the Flask config are set, Airflow config wins.""" from airflow.providers.fab.auth_manager.models.anonymous_user import AnonymousUser @@ -338,7 +338,7 @@ def test_get_public_user_airflow_conf_takes_precedence(self, auth_manager_with_a flask_app.config["AUTH_ROLE_PUBLIC"] = "Viewer" try: with conf_vars({("fab", "auth_role_public"): "Admin"}): - user = auth_manager_with_appbuilder.get_public_user() + user = auth_manager_with_appbuilder.build_public_user() finally: flask_app.config["AUTH_ROLE_PUBLIC"] = previous @@ -346,6 +346,26 @@ def test_get_public_user_airflow_conf_takes_precedence(self, auth_manager_with_a assert len(user.roles) == 1 assert user.roles[0].name == "Admin" + def test_get_fastapi_middlewares_disabled(self, auth_manager_with_appbuilder): + """No middleware is registered when public access is not configured.""" + flask_app = auth_manager_with_appbuilder.flask_app or (auth_manager_with_appbuilder.appbuilder.app) + previous = flask_app.config.get("AUTH_ROLE_PUBLIC") + flask_app.config["AUTH_ROLE_PUBLIC"] = None + try: + with conf_vars({("fab", "auth_role_public"): ""}): + assert auth_manager_with_appbuilder.get_fastapi_middlewares() == [] + finally: + flask_app.config["AUTH_ROLE_PUBLIC"] = previous + + def test_get_fastapi_middlewares_enabled(self, auth_manager_with_appbuilder): + """``FabAuthRolePublicMiddleware`` is registered when public access is configured.""" + from airflow.providers.fab.auth_manager.middleware import FabAuthRolePublicMiddleware + + with conf_vars({("fab", "auth_role_public"): "Admin"}): + middlewares = auth_manager_with_appbuilder.get_fastapi_middlewares() + + assert middlewares == [(FabAuthRolePublicMiddleware, {})] + @pytest.mark.parametrize( ("auth_type", "method"), [ diff --git a/providers/fab/tests/unit/fab/auth_manager/test_middleware.py b/providers/fab/tests/unit/fab/auth_manager/test_middleware.py new file mode 100644 index 0000000000000..d710f982bb740 --- /dev/null +++ b/providers/fab/tests/unit/fab/auth_manager/test_middleware.py @@ -0,0 +1,96 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from __future__ import annotations + +from unittest.mock import AsyncMock, Mock, patch + +import pytest + +from airflow.api_fastapi.auth.managers.base_auth_manager import COOKIE_NAME_JWT_TOKEN +from airflow.providers.fab.auth_manager.middleware import FabAuthRolePublicMiddleware + + +@pytest.mark.asyncio +class TestFabAuthRolePublicMiddleware: + def _make_request(self, headers: dict | None = None, cookies: dict | None = None) -> Mock: + request = Mock() + request.headers = headers or {} + request.cookies = cookies or {} + request.state = Mock(spec=[]) + return request + + @patch("airflow.providers.fab.auth_manager.middleware.get_auth_manager") + async def test_sets_public_user_when_no_auth_present(self, mock_get_auth_manager): + public_user = Mock(name="public_user") + auth_manager = Mock() + auth_manager.build_public_user = Mock(return_value=public_user) + mock_get_auth_manager.return_value = auth_manager + + middleware = FabAuthRolePublicMiddleware(app=Mock()) + request = self._make_request() + call_next = AsyncMock(return_value=Mock(name="response")) + + await middleware.dispatch(request, call_next) + + assert request.state.user is public_user + auth_manager.build_public_user.assert_called_once_with() + call_next.assert_awaited_once_with(request) + + @patch("airflow.providers.fab.auth_manager.middleware.get_auth_manager") + async def test_skips_when_authorization_header_present(self, mock_get_auth_manager): + auth_manager = Mock() + auth_manager.build_public_user = Mock() + mock_get_auth_manager.return_value = auth_manager + + middleware = FabAuthRolePublicMiddleware(app=Mock()) + request = self._make_request(headers={"authorization": "Bearer real-token"}) + call_next = AsyncMock(return_value=Mock(name="response")) + + await middleware.dispatch(request, call_next) + + auth_manager.build_public_user.assert_not_called() + assert not hasattr(request.state, "user") + + @patch("airflow.providers.fab.auth_manager.middleware.get_auth_manager") + async def test_skips_when_jwt_cookie_present(self, mock_get_auth_manager): + auth_manager = Mock() + auth_manager.build_public_user = Mock() + mock_get_auth_manager.return_value = auth_manager + + middleware = FabAuthRolePublicMiddleware(app=Mock()) + request = self._make_request(cookies={COOKIE_NAME_JWT_TOKEN: "cookie-token"}) + call_next = AsyncMock(return_value=Mock(name="response")) + + await middleware.dispatch(request, call_next) + + auth_manager.build_public_user.assert_not_called() + + @patch("airflow.providers.fab.auth_manager.middleware.get_auth_manager") + async def test_does_nothing_when_auth_manager_has_no_public_user(self, mock_get_auth_manager): + auth_manager = Mock() + auth_manager.build_public_user = Mock(return_value=None) + mock_get_auth_manager.return_value = auth_manager + + middleware = FabAuthRolePublicMiddleware(app=Mock()) + request = self._make_request() + call_next = AsyncMock(return_value=Mock(name="response")) + + await middleware.dispatch(request, call_next) + + assert not hasattr(request.state, "user") + call_next.assert_awaited_once_with(request) + From d69a963a0dad5497450fbc14cdfad1f4f48d129f Mon Sep 17 00:00:00 2001 From: gaurav0107 Date: Thu, 23 Apr 2026 01:48:42 +0530 Subject: [PATCH 3/3] Address maintainer review: simpler user construction and middleware wiring Follow-up review from @vincbeck on PR #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 --- .../managers/simple/simple_auth_manager.py | 8 ++ .../src/airflow/api_fastapi/core_api/app.py | 5 -- .../simple/test_simple_auth_manager.py | 10 +++ .../auth-manager/webserver-authentication.rst | 24 +----- providers/fab/newsfragments/65685.bugfix.rst | 1 - .../fab/auth_manager/fab_auth_manager.py | 40 ++++----- .../fab/auth_manager/test_fab_auth_manager.py | 83 +++++++------------ .../unit/fab/auth_manager/test_middleware.py | 1 - 8 files changed, 70 insertions(+), 102 deletions(-) delete mode 100644 providers/fab/newsfragments/65685.bugfix.rst diff --git a/airflow-core/src/airflow/api_fastapi/auth/managers/simple/simple_auth_manager.py b/airflow-core/src/airflow/api_fastapi/auth/managers/simple/simple_auth_manager.py index a40915d5da81c..e84e18c42e978 100644 --- a/airflow-core/src/airflow/api_fastapi/auth/managers/simple/simple_auth_manager.py +++ b/airflow-core/src/airflow/api_fastapi/auth/managers/simple/simple_auth_manager.py @@ -319,6 +319,14 @@ def is_authorized_hitl_task(self, *, assigned_users: set[str], user: SimpleAuthM # Delegate to parent class for the actual authorization check return super().is_authorized_hitl_task(assigned_users=assigned_users, user=user) + def get_fastapi_middlewares(self) -> list[tuple[type, dict[str, Any]]]: + """Register the all-admins middleware when ``[core] simple_auth_manager_all_admins`` is set.""" + if not conf.getboolean("core", "simple_auth_manager_all_admins"): + return [] + from airflow.api_fastapi.auth.managers.simple.middleware import SimpleAllAdminMiddleware + + return [(SimpleAllAdminMiddleware, {})] + def get_fastapi_app(self) -> FastAPI | None: """ Specify a sub FastAPI application specific to the auth manager. diff --git a/airflow-core/src/airflow/api_fastapi/core_api/app.py b/airflow-core/src/airflow/api_fastapi/core_api/app.py index b55f78b890e97..c36bb6c978ffc 100644 --- a/airflow-core/src/airflow/api_fastapi/core_api/app.py +++ b/airflow-core/src/airflow/api_fastapi/core_api/app.py @@ -29,7 +29,6 @@ from fastapi.templating import Jinja2Templates from airflow.api_fastapi.auth.tokens import get_signing_key -from airflow.configuration import conf from airflow.exceptions import AirflowException log = logging.getLogger(__name__) @@ -170,10 +169,6 @@ def init_middlewares(app: FastAPI) -> None: from airflow.api_fastapi.common.http_access_log import HttpAccessLogMiddleware app.add_middleware(JWTRefreshMiddleware) - if conf.getboolean("core", "simple_auth_manager_all_admins"): - from airflow.api_fastapi.auth.managers.simple.middleware import SimpleAllAdminMiddleware - - app.add_middleware(SimpleAllAdminMiddleware) for middleware_cls, middleware_kwargs in get_auth_manager().get_fastapi_middlewares(): app.add_middleware(middleware_cls, **middleware_kwargs) diff --git a/airflow-core/tests/unit/api_fastapi/auth/managers/simple/test_simple_auth_manager.py b/airflow-core/tests/unit/api_fastapi/auth/managers/simple/test_simple_auth_manager.py index 1e9f195c8b715..b11fec3e508d2 100644 --- a/airflow-core/tests/unit/api_fastapi/auth/managers/simple/test_simple_auth_manager.py +++ b/airflow-core/tests/unit/api_fastapi/auth/managers/simple/test_simple_auth_manager.py @@ -367,3 +367,13 @@ def test_is_authorized_hitl_task(self, auth_manager, all_admins, user_id, assign def test_get_teams(self, auth_manager): teams = auth_manager._get_teams() assert teams == {"test", "marketing"} + + @conf_vars({("core", "simple_auth_manager_all_admins"): "false"}) + def test_get_fastapi_middlewares_disabled(self, auth_manager): + assert auth_manager.get_fastapi_middlewares() == [] + + @conf_vars({("core", "simple_auth_manager_all_admins"): "true"}) + def test_get_fastapi_middlewares_enabled(self, auth_manager): + from airflow.api_fastapi.auth.managers.simple.middleware import SimpleAllAdminMiddleware + + assert auth_manager.get_fastapi_middlewares() == [(SimpleAllAdminMiddleware, {})] diff --git a/providers/fab/docs/auth-manager/webserver-authentication.rst b/providers/fab/docs/auth-manager/webserver-authentication.rst index 7a00a64a188ff..e950560a94aac 100644 --- a/providers/fab/docs/auth-manager/webserver-authentication.rst +++ b/providers/fab/docs/auth-manager/webserver-authentication.rst @@ -42,27 +42,9 @@ following CLI commands to create an account: --role Admin \ --email spiderman@superhero.org -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 -by default. For example, in ``airflow.cfg``: - -.. code-block:: ini - - [fab] - auth_role_public = Admin - -or via the equivalent environment variable: - -.. code-block:: bash - - export AIRFLOW__FAB__AUTH_ROLE_PUBLIC=Admin - -This makes both the FastAPI-based API server and the legacy Flask-based views honor anonymous -access consistently. - -For backwards compatibility, setting ``AUTH_ROLE_PUBLIC`` in ``$AIRFLOW_HOME/webserver_config.py`` -is still supported, but the ``[fab] auth_role_public`` Airflow config takes precedence when both -are set and is the recommended configuration going forward: +To deactivate the authentication and allow users to be identified as Anonymous, the following entry +in ``$AIRFLOW_HOME/webserver_config.py`` needs to be set with the desired role that the Anonymous +user will have by default: .. code-block:: ini diff --git a/providers/fab/newsfragments/65685.bugfix.rst b/providers/fab/newsfragments/65685.bugfix.rst deleted file mode 100644 index 0792fdada92d5..0000000000000 --- a/providers/fab/newsfragments/65685.bugfix.rst +++ /dev/null @@ -1 +0,0 @@ -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. diff --git a/providers/fab/src/airflow/providers/fab/auth_manager/fab_auth_manager.py b/providers/fab/src/airflow/providers/fab/auth_manager/fab_auth_manager.py index 110ac7c18b3d4..c9fbf16281aa3 100644 --- a/providers/fab/src/airflow/providers/fab/auth_manager/fab_auth_manager.py +++ b/providers/fab/src/airflow/providers/fab/auth_manager/fab_auth_manager.py @@ -307,23 +307,25 @@ def _get_auth_role_public(self) -> str | None: """ Return the role granted to anonymous users, or ``None`` if public access is disabled. - Prefers the Airflow ``[fab] auth_role_public`` configuration and falls back to the legacy - ``AUTH_ROLE_PUBLIC`` entry in ``webserver_config.py`` so existing deployments continue to - work. + ``providers/fab/www/app.py`` copies ``[fab] auth_role_public`` from the Airflow config + into ``AUTH_ROLE_PUBLIC`` on the Flask app when the app is created, so reading the + Flask config here covers both the new config key and the legacy + ``AUTH_ROLE_PUBLIC`` entry in ``webserver_config.py``. """ - role = conf.get("fab", "auth_role_public", fallback="") or "" - if role: - return role if self.appbuilder is not None: return self.appbuilder.app.config.get("AUTH_ROLE_PUBLIC", None) return None - def build_public_user(self) -> AnonymousUser | None: + @provide_session + def build_public_user(self, *, session: Session = NEW_SESSION) -> AnonymousUser | None: """ Build an :class:`AnonymousUser` pre-populated with the configured public role. - Returns ``None`` when ``[fab] auth_role_public`` (or the legacy - ``AUTH_ROLE_PUBLIC`` entry in ``webserver_config.py``) is not set. + Returns ``None`` when neither ``[fab] auth_role_public`` nor the legacy + ``AUTH_ROLE_PUBLIC`` entry in ``webserver_config.py`` is set. + + The role and its permissions are resolved via the database so the caller does not + need a Flask app/request context to use the returned user (see #60897). :meta private: """ @@ -332,19 +334,13 @@ def build_public_user(self) -> AnonymousUser | None: return None user = AnonymousUser() - flask_app = self.flask_app or (self.appbuilder.app if self.appbuilder else None) - if flask_app is not None: - 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} + role = session.scalar(select(Role).where(Role.name == public_role_name)) + if role is not None: + # ``AnonymousUser.roles``/``perms`` normally resolve lazily through Flask's + # ``current_app``. Writing ``_roles``/``_perms`` directly freezes a snapshot for + # the lifetime of a single FastAPI authorization check. + user._roles = {role} + user._perms = {(perm.action.name, perm.resource.name) for perm in role.permissions} return user def get_fastapi_middlewares(self) -> list[tuple[type, dict[str, Any]]]: diff --git a/providers/fab/tests/unit/fab/auth_manager/test_fab_auth_manager.py b/providers/fab/tests/unit/fab/auth_manager/test_fab_auth_manager.py index 93958189c3fbc..6b253207079a7 100644 --- a/providers/fab/tests/unit/fab/auth_manager/test_fab_auth_manager.py +++ b/providers/fab/tests/unit/fab/auth_manager/test_fab_auth_manager.py @@ -263,106 +263,85 @@ def test_is_logged_in_with_inactive_user(self, mock_get_user, auth_manager_with_ assert auth_manager_with_appbuilder.is_logged_in() is False @mock.patch.object(FabAuthManager, "get_user") - def test_is_logged_in_with_auth_role_public_conf(self, mock_get_user, auth_manager_with_appbuilder): - """``[fab] auth_role_public`` Airflow config is enough to be 'logged in'.""" + def test_is_logged_in_with_auth_role_public(self, mock_get_user, flask_app, auth_manager_with_appbuilder): + """When ``AUTH_ROLE_PUBLIC`` is set on the Flask app, anonymous users are 'logged in'.""" user = Mock() user.is_anonymous.return_value = True user.is_active.return_value = False mock_get_user.return_value = user - with conf_vars({("fab", "auth_role_public"): "Admin"}): + previous = flask_app.config.get("AUTH_ROLE_PUBLIC") + flask_app.config["AUTH_ROLE_PUBLIC"] = "Admin" + try: assert auth_manager_with_appbuilder.is_logged_in() is True + finally: + flask_app.config["AUTH_ROLE_PUBLIC"] = previous - def test_build_public_user_returns_none_when_not_configured(self, auth_manager_with_appbuilder): - """Without ``[fab] auth_role_public`` or ``AUTH_ROLE_PUBLIC``, there is no public user.""" - flask_app = auth_manager_with_appbuilder.flask_app or (auth_manager_with_appbuilder.appbuilder.app) + def test_build_public_user_returns_none_when_not_configured( + self, flask_app, auth_manager_with_appbuilder + ): + """Without ``AUTH_ROLE_PUBLIC`` set on the Flask app, there is no public user.""" previous = flask_app.config.get("AUTH_ROLE_PUBLIC") flask_app.config["AUTH_ROLE_PUBLIC"] = None try: - with conf_vars({("fab", "auth_role_public"): ""}): - assert auth_manager_with_appbuilder.build_public_user() is None + assert auth_manager_with_appbuilder.build_public_user() is None finally: flask_app.config["AUTH_ROLE_PUBLIC"] = previous - def test_build_public_user_returns_anonymous_user_from_conf(self, auth_manager_with_appbuilder): - """``[fab] auth_role_public`` yields an :class:`AnonymousUser` with the role resolved.""" - from airflow.providers.fab.auth_manager.models.anonymous_user import AnonymousUser - - with conf_vars({("fab", "auth_role_public"): "Admin"}): - user = auth_manager_with_appbuilder.build_public_user() - - assert isinstance(user, AnonymousUser) - assert len(user.roles) == 1 - assert user.roles[0].name == "Admin" - # ``perms`` must be pre-populated so FastAPI can evaluate authorization outside a - # Flask request/app context. - assert user._perms, "Expected permissions to be pre-populated on the public user" - - def test_build_public_user_falls_back_to_flask_config(self, auth_manager_with_appbuilder): - """Legacy ``AUTH_ROLE_PUBLIC`` in ``webserver_config.py`` is still honored.""" + def test_build_public_user_returns_anonymous_user(self, flask_app, auth_manager_with_appbuilder): + """``AUTH_ROLE_PUBLIC`` yields an :class:`AnonymousUser` with the role resolved from the DB.""" from airflow.providers.fab.auth_manager.models.anonymous_user import AnonymousUser - flask_app = auth_manager_with_appbuilder.flask_app or (auth_manager_with_appbuilder.appbuilder.app) previous = flask_app.config.get("AUTH_ROLE_PUBLIC") flask_app.config["AUTH_ROLE_PUBLIC"] = "Admin" try: - with conf_vars({("fab", "auth_role_public"): ""}): - user = auth_manager_with_appbuilder.build_public_user() + user = auth_manager_with_appbuilder.build_public_user() finally: flask_app.config["AUTH_ROLE_PUBLIC"] = previous assert isinstance(user, AnonymousUser) assert len(user.roles) == 1 assert user.roles[0].name == "Admin" + # ``perms`` must be pre-populated so FastAPI can evaluate authorization outside a + # Flask request/app context. + assert user._perms, "Expected permissions to be pre-populated on the public user" def test_build_public_user_with_unknown_role_returns_user_with_empty_perms( - self, auth_manager_with_appbuilder + self, flask_app, auth_manager_with_appbuilder ): """A misconfigured role name still yields an :class:`AnonymousUser` with empty perms.""" from airflow.providers.fab.auth_manager.models.anonymous_user import AnonymousUser - with conf_vars({("fab", "auth_role_public"): "DoesNotExist"}): - user = auth_manager_with_appbuilder.build_public_user() - - assert isinstance(user, AnonymousUser) - # No pre-populated perms means the user has no authorization; the role will also not - # be resolved since ``find_role`` returned None for a non-existent role. - assert user._perms == set() - - def test_build_public_user_airflow_conf_takes_precedence(self, auth_manager_with_appbuilder): - """When both ``[fab] auth_role_public`` and the Flask config are set, Airflow config wins.""" - from airflow.providers.fab.auth_manager.models.anonymous_user import AnonymousUser - - flask_app = auth_manager_with_appbuilder.flask_app or (auth_manager_with_appbuilder.appbuilder.app) previous = flask_app.config.get("AUTH_ROLE_PUBLIC") - flask_app.config["AUTH_ROLE_PUBLIC"] = "Viewer" + flask_app.config["AUTH_ROLE_PUBLIC"] = "DoesNotExist" try: - with conf_vars({("fab", "auth_role_public"): "Admin"}): - user = auth_manager_with_appbuilder.build_public_user() + user = auth_manager_with_appbuilder.build_public_user() finally: flask_app.config["AUTH_ROLE_PUBLIC"] = previous assert isinstance(user, AnonymousUser) - assert len(user.roles) == 1 - assert user.roles[0].name == "Admin" + # Role not found in the DB, so no pre-populated perms. + assert user._perms == set() - def test_get_fastapi_middlewares_disabled(self, auth_manager_with_appbuilder): + def test_get_fastapi_middlewares_disabled(self, flask_app, auth_manager_with_appbuilder): """No middleware is registered when public access is not configured.""" - flask_app = auth_manager_with_appbuilder.flask_app or (auth_manager_with_appbuilder.appbuilder.app) previous = flask_app.config.get("AUTH_ROLE_PUBLIC") flask_app.config["AUTH_ROLE_PUBLIC"] = None try: - with conf_vars({("fab", "auth_role_public"): ""}): - assert auth_manager_with_appbuilder.get_fastapi_middlewares() == [] + assert auth_manager_with_appbuilder.get_fastapi_middlewares() == [] finally: flask_app.config["AUTH_ROLE_PUBLIC"] = previous - def test_get_fastapi_middlewares_enabled(self, auth_manager_with_appbuilder): + def test_get_fastapi_middlewares_enabled(self, flask_app, auth_manager_with_appbuilder): """``FabAuthRolePublicMiddleware`` is registered when public access is configured.""" from airflow.providers.fab.auth_manager.middleware import FabAuthRolePublicMiddleware - with conf_vars({("fab", "auth_role_public"): "Admin"}): + previous = flask_app.config.get("AUTH_ROLE_PUBLIC") + flask_app.config["AUTH_ROLE_PUBLIC"] = "Admin" + try: middlewares = auth_manager_with_appbuilder.get_fastapi_middlewares() + finally: + flask_app.config["AUTH_ROLE_PUBLIC"] = previous assert middlewares == [(FabAuthRolePublicMiddleware, {})] diff --git a/providers/fab/tests/unit/fab/auth_manager/test_middleware.py b/providers/fab/tests/unit/fab/auth_manager/test_middleware.py index d710f982bb740..e4117a0fe9198 100644 --- a/providers/fab/tests/unit/fab/auth_manager/test_middleware.py +++ b/providers/fab/tests/unit/fab/auth_manager/test_middleware.py @@ -93,4 +93,3 @@ async def test_does_nothing_when_auth_manager_has_no_public_user(self, mock_get_ assert not hasattr(request.state, "user") call_next.assert_awaited_once_with(request) -