Fix security-sensitive testcase enumeration in issue redirector by en…

src/appengine/handlers/issue_redirector.py

@@ -16,6 +16,7 @@
 
 from clusterfuzz._internal.issue_management import issue_tracker_utils
 from handlers import base_handler
+from libs import access
 from libs import helpers
 
 
@@ -24,7 +25,7 @@ class Handler(base_handler.Handler):
 
   def get(self, testcase_id=None):
     """Redirect user to the correct URL."""
-    testcase = helpers.get_testcase(testcase_id)
+    testcase = access.check_access_and_get_testcase(testcase_id)
     issue_url = helpers.get_or_exit(
         lambda: issue_tracker_utils.get_issue_url(testcase),
         'Issue tracker for testcase (id=%s) is not found.' % testcase_id,

src/clusterfuzz/_internal/tests/appengine/handlers/issue_redirector_test.py

@@ -19,7 +19,11 @@
 import webtest
 
 from clusterfuzz._internal.datastore import data_types
+from clusterfuzz._internal.datastore import ndb_init
+from clusterfuzz._internal.issue_management import issue_tracker_utils
 from clusterfuzz._internal.tests.test_libs import helpers as test_helpers
+from libs import access
+from libs import helpers
 
 
 class HandlerTest(unittest.TestCase):
@@ -28,33 +32,66 @@ class HandlerTest(unittest.TestCase):
   def setUp(self):
     test_helpers.patch(self, [
         'clusterfuzz._internal.issue_management.issue_tracker_utils.get_issue_url',
-        'libs.helpers.get_testcase',
+        'libs.access.check_access_and_get_testcase',
         'clusterfuzz._internal.system.environment.is_running_on_app_engine',
+        'libs.helpers.get_user_email',
+        'clusterfuzz._internal.config.db_config.get_value',
+        'logging.exception',
+        'libs.form.generate_csrf_token',
     ])
     self.mock.is_running_on_app_engine.return_value = True
+    self.mock.get_user_email.return_value = ''
+    self.mock.get_value.return_value = 'contact@example.com'
+    self.mock.generate_csrf_token.return_value = 'dummy_csrf_token'
+
+    self.ndb_context = ndb_init.context()
+    self.ndb_context.__enter__()
 
     import server
     self.app = webtest.TestApp(server.app)
 
+  def tearDown(self):
+    self.ndb_context.__exit__(None, None, None)
+
   def test_succeed(self):
     """Test redirection succeeds."""
     testcase = data_types.Testcase()
     testcase.bug_information = '456789'
-    self.mock.get_testcase.return_value = testcase
+    self.mock.check_access_and_get_testcase.return_value = testcase
     self.mock.get_issue_url.return_value = 'http://google.com/456789'
 
     response = self.app.get('/issue/12345')
 
     self.assertEqual(302, response.status_int)
     self.assertEqual('http://google.com/456789', response.headers['Location'])
 
-    self.mock.get_testcase.assert_has_calls([mock.call('12345')])
+    self.mock.check_access_and_get_testcase.assert_has_calls(
+        [mock.call('12345')])
     self.mock.get_issue_url.assert_has_calls([mock.call(testcase)])
 
   def test_no_issue_url(self):
     """Test no issue url."""
-    self.mock.get_testcase.return_value = data_types.Testcase()
+    self.mock.check_access_and_get_testcase.return_value = data_types.Testcase()
     self.mock.get_issue_url.return_value = ''
 
     response = self.app.get('/issue/12345', expect_errors=True)
     self.assertEqual(404, response.status_int)
+
+  def test_access_denied(self):
+    """Test access denied error."""
+    self.mock.get_user_email.return_value = 'test@user.com'
+    self.mock.check_access_and_get_testcase.side_effect = (
+        helpers.AccessDeniedError())
+
+    response = self.app.get('/issue/12345', expect_errors=True)
+    self.assertEqual(403, response.status_int)
+
+  def test_unauthorized(self):
+    """Test unauthorized error."""
+    self.mock.get_user_email.return_value = ''
+    self.mock.check_access_and_get_testcase.side_effect = (
+        helpers.UnauthorizedError())
+
+    response = self.app.get('/issue/12345', expect_errors=True)
+    self.assertEqual(302, response.status_int)
+    self.assertIn('/login', response.headers['Location'])