You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by el...@apache.org on 2023/01/10 05:52:00 UTC

[airflow] branch main updated: Fix UIAlert should_show when AUTH_ROLE_PUBLIC set (#28781)

This is an automated email from the ASF dual-hosted git repository.

eladkal pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git


The following commit(s) were added to refs/heads/main by this push:
     new f17e2ba48b Fix UIAlert should_show when AUTH_ROLE_PUBLIC set (#28781)
f17e2ba48b is described below

commit f17e2ba48b59525655a92e04684db664a672918f
Author: Victor Chiapaikeo <vc...@gmail.com>
AuthorDate: Tue Jan 10 00:51:53 2023 -0500

    Fix UIAlert should_show when AUTH_ROLE_PUBLIC set (#28781)
    
    * Fix UIAlert should_show when AUTH_ROLE_PUBLIC set
    
    Co-authored-by: Tzu-ping Chung <ur...@gmail.com>
---
 airflow/www/utils.py               | 22 +++++++++++++++++++---
 tests/test_utils/www.py            |  7 +++++++
 tests/www/views/conftest.py        |  7 ++++++-
 tests/www/views/test_views_home.py |  5 +++++
 4 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/airflow/www/utils.py b/airflow/www/utils.py
index 35fa3278ed..e56fc159c3 100644
--- a/airflow/www/utils.py
+++ b/airflow/www/utils.py
@@ -56,6 +56,8 @@ if TYPE_CHECKING:
     from sqlalchemy.orm.session import Session
     from sqlalchemy.sql.operators import ColumnOperators
 
+    from airflow.www.fab_security.sqla.manager import SecurityManager
+
 
 def datetime_to_string(value: DateTime | None) -> str | None:
     if value is None:
@@ -820,10 +822,24 @@ class UIAlert:
         self.html = html
         self.message = Markup(message) if html else message
 
-    def should_show(self, securitymanager) -> bool:
-        """Determine if the user should see the message based on their role membership"""
+    def should_show(self, securitymanager: SecurityManager) -> bool:
+        """Determine if the user should see the message.
+
+        The decision is based on the user's role. If ``AUTH_ROLE_PUBLIC`` is
+        set in ``webserver_config.py``, An anonymous user would have the
+        ``AUTH_ROLE_PUBLIC`` role.
+        """
         if self.roles:
-            user_roles = {r.name for r in securitymanager.current_user.roles}
+            current_user = securitymanager.current_user
+            if current_user is not None:
+                user_roles = {r.name for r in securitymanager.current_user.roles}
+            elif "AUTH_ROLE_PUBLIC" in securitymanager.appbuilder.get_app.config:
+                # If the current_user is anonymous, assign AUTH_ROLE_PUBLIC role (if it exists) to them
+                user_roles = {securitymanager.appbuilder.get_app.config["AUTH_ROLE_PUBLIC"]}
+            else:
+                # Unable to obtain user role - default to not showing
+                return False
+
             if not user_roles.intersection(set(self.roles)):
                 return False
         return True
diff --git a/tests/test_utils/www.py b/tests/test_utils/www.py
index 8491d54094..55699c7051 100644
--- a/tests/test_utils/www.py
+++ b/tests/test_utils/www.py
@@ -32,6 +32,13 @@ def client_with_login(app, **kwargs):
     return client
 
 
+def client_without_login(app):
+    # Anonymous users can only view if AUTH_ROLE_PUBLIC is set to non-Public
+    app.config["AUTH_ROLE_PUBLIC"] = "Viewer"
+    client = app.test_client()
+    return client
+
+
 def check_content_in_response(text, resp, resp_code=200):
     resp_html = resp.data.decode("utf-8")
     assert resp_code == resp.status_code
diff --git a/tests/www/views/conftest.py b/tests/www/views/conftest.py
index 4166b1c961..e04b5ec96e 100644
--- a/tests/www/views/conftest.py
+++ b/tests/www/views/conftest.py
@@ -29,7 +29,7 @@ from airflow.models import DagBag
 from airflow.www.app import create_app
 from tests.test_utils.api_connexion_utils import delete_user
 from tests.test_utils.decorators import dont_initialize_flask_app_submodules
-from tests.test_utils.www import client_with_login
+from tests.test_utils.www import client_with_login, client_without_login
 
 
 @pytest.fixture(autouse=True, scope="module")
@@ -123,6 +123,11 @@ def user_client(app):
     return client_with_login(app, username="test_user", password="test_user")
 
 
+@pytest.fixture()
+def anonymous_client(app):
+    return client_without_login(app)
+
+
 class _TemplateWithContext(NamedTuple):
     template: jinja2.environment.Template
     context: dict[str, Any]
diff --git a/tests/www/views/test_views_home.py b/tests/www/views/test_views_home.py
index 1041a71117..4f6e354a7b 100644
--- a/tests/www/views/test_views_home.py
+++ b/tests/www/views/test_views_home.py
@@ -203,6 +203,11 @@ def test_home_robots_header_in_response(user_client):
 @pytest.mark.parametrize(
     "client, flash_message, expected",
     [
+        ("anonymous_client", UIAlert("hello world"), True),
+        ("anonymous_client", UIAlert("hello world", roles=["Viewer"]), True),
+        ("anonymous_client", UIAlert("hello world", roles=["User"]), False),
+        ("anonymous_client", UIAlert("hello world", roles=["Viewer", "User"]), True),
+        ("anonymous_client", UIAlert("hello world", roles=["Admin"]), False),
         ("user_client", UIAlert("hello world"), True),
         ("user_client", UIAlert("hello world", roles=["User"]), True),
         ("user_client", UIAlert("hello world", roles=["User", "Admin"]), True),