ashb commented on a change in pull request #4973: [AIRFLOW-4155] Allow Public 
role access to /home
URL: https://github.com/apache/airflow/pull/4973#discussion_r270349029
 
 

 ##########
 File path: tests/www/test_security.py
 ##########
 @@ -308,3 +308,26 @@ def test_override_role_vm(self):
         test_security_manager = TestSecurityManager(appbuilder=self.appbuilder)
         self.assertEqual(len(test_security_manager.VIEWER_VMS), 1)
         self.assertEqual(test_security_manager.VIEWER_VMS, {'Airflow'})
+
+    def test_is_user_logged_in_returns_false_if_not_authenticated(self):
+        user = mock.MagicMock()
+        user.is_authenticated = False
+        self.assertFalse(self.security_manager.is_user_logged_in(user))
+
+    def test_is_user_logged_in_returns_true_if_authenticated(self):
+        user = mock.MagicMock()
+        user.is_authenticated = True
+        self.assertTrue(self.security_manager.is_user_logged_in(user))
+
+    def test_unauthenticated_user_is_public(self):
+        user = mock.MagicMock()
+        user.is_authenticated = False
+        self.assertTrue(self.security_manager.is_public_user(user))
 
 Review comment:
   The problem with using "Public" role is that that applies to signed in users 
_and_ not-signed in users. That feels I think is my cause for concern, as it's 
conflating two concepts.
   
   My concern is around "defaults" for an Airflow install - they should be, by 
default, locked down tight.
   
   Most people won't have  registration enabled, nor will they want it, so 
display the home page, even with no DAGs to an anon user just feels wrong and a 
confusing/worrying, if not _strictly_ a security problem.
   
   I think if we didn't use the Public role here the changes to the home-page 
etc are not a problem, though I'd like to get rid of `is_user_logged_in` being 
passed to the home page template, as I don't want that being shown to an 
not-logged-in user - I prefer the existing behaviour of redirecting to a login 
page.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to