You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "vandonr-amz (via GitHub)" <gi...@apache.org> on 2023/07/07 23:43:13 UTC

[GitHub] [airflow] vandonr-amz commented on a diff in pull request #32428: Add AuthManager.is_logged_in and use it to determine whether a user is logged in

vandonr-amz commented on code in PR #32428:
URL: https://github.com/apache/airflow/pull/32428#discussion_r1256611114


##########
airflow/auth/managers/base_auth_manager.py:
##########
@@ -33,3 +33,8 @@ class BaseAuthManager(LoggingMixin):
     def get_user_name(self) -> str:
         """Return the username associated to the user in session."""
         ...
+
+    @abstractmethod
+    def is_logged_in(self) -> bool:
+        """Return whether the user is logged in."""
+        ...

Review Comment:
   should this be abstract ? Or maybe we could implement it using `get_user_name` ? If it returns None if user is not logged ?



##########
airflow/www/auth.py:
##########
@@ -46,7 +46,7 @@ def decorated(*args, **kwargs):
             )
             if appbuilder.sm.check_authorization(permissions, dag_id):
                 return func(*args, **kwargs)
-            elif not g.user.is_anonymous and not g.user.perms:
+            elif auth_manager.is_logged_in() and not g.user.perms:

Review Comment:
   I suppose `g.user.perms` is something we'll touch too, but later on in the project ?



##########
airflow/auth/managers/fab/fab_auth_manager.py:
##########
@@ -39,3 +39,7 @@ def get_user_name(self) -> str:
         first_name = current_user.first_name or ""
         last_name = current_user.last_name or ""
         return f"{first_name} {last_name}".strip()
+
+    def is_logged_in(self) -> bool:
+        """Return whether the user is logged in."""
+        return not current_user.is_anonymous

Review Comment:
   should we check if `current_user` is None before ? Given the fact that it was done in https://github.com/apache/airflow/pull/32428/files#diff-8075dbc4ae06e7b83cd163eeb206322a7977c227bb57398e7343c9e09c80ae59L71



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org