Jelajahi Sumber

[slack] Add document read permission check for Slack users as Hue users(#1947)

* [slack] Add doc read perm check for link  and update UTs
- Better editor type check with id and uuid
- Removed Document2.objects.get mocking and instead creating doc in testserver (to check doc access)
- Simplify query_id into a dict and separate user and perm check
- Pass user_id to get slack user email
- Retrieve email prefix from slack user for Hue user

* [slack] Simplify permission check with only can_read_or_exception method  and update UTs
- Access check only for Slack User who is Hue user
- Update unit test with different link share in Slack flow
- Test slack_email_prefix and mock Slack users_info API call instead
- Update unit test with removing unrelated checks and testing slack user not hue user separately
- Better exception messages
Harsh Gupta 4 tahun lalu
induk
melakukan
f2ea2479d1

+ 28 - 7
desktop/core/src/desktop/lib/botserver/views.py

@@ -82,11 +82,13 @@ def parse_events(event):
 
   """
   channel_id = event.get('channel')
+  user_id = event.get('user')
+
   if event.get('type') == 'message':
-    handle_on_message(channel_id, event.get('bot_id'), event.get('text'), event.get('user'))
+    handle_on_message(channel_id, event.get('bot_id'), event.get('text'), user_id)
 
   if event.get('type') == 'link_shared':
-    handle_on_link_shared(channel_id, event.get('message_ts'), event.get('links'))
+    handle_on_link_shared(channel_id, event.get('message_ts'), event.get('links'), user_id)
 
 
 def handle_on_message(channel_id, bot_id, text, user_id):
@@ -99,26 +101,35 @@ def handle_on_message(channel_id, bot_id, text, user_id):
       send_hi_user(channel_id, user_id)
 
 
-def handle_on_link_shared(channel_id, message_ts, links):
+def handle_on_link_shared(channel_id, message_ts, links, user_id):
   for item in links:
     path = urlsplit(item['url'])[2]
     id_type, qid = urlsplit(item['url'])[3].split('=')
+    query_id = {'id': qid} if qid.isdigit() else {'uuid': qid}
 
     try:
       if path == '/hue/editor' and id_type == 'editor':
-        doc = Document2.objects.get(id=qid)
+        doc = Document2.objects.get(**query_id)
         doc_type = 'Query'
       elif path == '/hue/gist' and id_type == 'uuid':
-        doc = _get_gist_document(uuid=qid)
+        doc = _get_gist_document(**query_id)
         doc_type = 'Gist'
       else:
         raise PopupException(_("Cannot unfurl link"))
     except Document2.DoesNotExist:
-      msg = "Document with {key}={value} does not exist".format(key='uuid' if id_type == 'uuid' else 'id', value=qid)
+      msg = "Document with {key} does not exist".format(key=query_id)
       raise PopupException(_(msg))
 
+    # Permission check for Slack user to be Hue user
+    try:
+      user = User.objects.get(username=slack_email_prefix(user_id))
+    except User.DoesNotExist:
+      raise PopupException(_("Slack user does not have access to the query"))
+
+    doc.can_read_or_exception(user)
+
     # Mock request for query execution and fetch result
-    user = rewrite_user(User.objects.get(username=doc.owner.username))
+    user = rewrite_user(user)
     request = MockRequest(user=user)
 
     payload = _make_unfurl_payload(request, item['url'], id_type, doc, doc_type)
@@ -132,6 +143,16 @@ def handle_on_link_shared(channel_id, message_ts, links):
       send_result_file(request, channel_id, message_ts, doc, 'xls')
 
 
+def slack_email_prefix(user_id):
+  try:
+    slack_user = slack_client.users_info(user=user_id)
+  except Exception as e:
+    raise PopupException(_("Cannot find query owner in Slack"), detail=e)
+  
+  if slack_user['ok']:
+    return slack_user['user']['profile']['email'].split('@')[0]
+
+
 def send_result_file(request, channel_id, message_ts, doc, file_format):
   notebook = json.loads(doc.data)
   snippet = notebook['snippets'][0]

+ 103 - 43
desktop/core/src/desktop/lib/botserver/views_tests.py

@@ -44,6 +44,15 @@ class TestBotServer(unittest.TestCase):
   def setUpClass(cls):
     if not conf.SLACK.IS_ENABLED.get():
       raise SkipTest
+  
+  def setUp(self):
+    # Slack user email: test@example.com
+    self.client = make_logged_in_client(username="test", groupname="default", recreate=True, is_superuser=False)
+    self.user = User.objects.get(username="test")
+
+    # Other slack user email: test_not_me@example.com
+    self.client_not_me = make_logged_in_client(username="test_not_me", groupname="default", recreate=True, is_superuser=False)
+    self.user_not_me = User.objects.get(username="test_not_me")
 
   def test_send_hi_user(self):
     with patch('desktop.lib.botserver.views.slack_client.api_call') as api_call:
@@ -71,64 +80,115 @@ class TestBotServer(unittest.TestCase):
 
       handle_on_message("channel", None, "hello hue test", "user_id")
       assert_true(say_hi_user.called)
+  
+  def test_handle_query_history_link(self):
+    with patch('desktop.lib.botserver.views.slack_client.chat_unfurl') as chat_unfurl:
+      with patch('desktop.lib.botserver.views._make_unfurl_payload') as mock_unfurl_payload:
+        with patch('desktop.lib.botserver.views.send_result_file') as send_result_file:
+          with patch('desktop.lib.botserver.views.slack_client.users_info') as users_info:
+
+            channel_id = "channel"
+            message_ts = "12.1"
+            user_id = "<@user_id>"
+
+            links = [{"url": "https://demo.gethue.com/hue/editor?editor=12345"}]
+            doc_data = {
+              "dialect": "mysql",
+              "snippets": [{
+                "database": "hue",
+                "statement_raw": "SELECT 5000",
+              }]
+            }
+            doc = Document2.objects.create(id=12345, data=json.dumps(doc_data), owner=self.user)
+            mock_unfurl_payload.return_value = {
+              'payload': {},
+              'file_status': True,
+            }
+
+            # Slack user is Hue user but without read access sends link
+            users_info.return_value = {
+              "ok": True,
+              "user": {
+                "profile": {
+                  "email": "test_not_me@example.com"
+                }
+              }
+            }
+            assert_raises(PopupException, handle_on_link_shared, "channel", "12.1", links, "<@user_id>")
 
-  def test_handle_on_link_shared(self):
+            # Slack user is Hue user with read access sends link
+            doc.update_permission(self.user, is_link_on=True)
+            handle_on_link_shared(channel_id, message_ts, links, user_id)
+
+            assert_true(chat_unfurl.called)
+            assert_true(send_result_file.called)
+
+            # Document does not exist
+            qhistory_url = "https://demo.gethue.com/hue/editor?editor=109644"
+            assert_raises(PopupException, handle_on_link_shared, "channel", "12.1", [{"url": qhistory_url}], "<@user_id>")
+
+            # Cannot unfurl link with invalid query link
+            inv_qhistory_url = "https://demo.gethue.com/hue/editor/?type=4"
+            assert_raises(PopupException, handle_on_link_shared, "channel", "12.1", [{"url": inv_qhistory_url}], "<@user_id>")
+
+  def test_handle_gist_link(self):
     with patch('desktop.lib.botserver.views.slack_client.chat_unfurl') as chat_unfurl:
       with patch('desktop.lib.botserver.views._make_unfurl_payload') as mock_unfurl_payload:
-        with patch('desktop.lib.botserver.views.Document2.objects.get') as document2_objects_get:
-          with patch('desktop.lib.botserver.views._get_gist_document') as _get_gist_document:
+        with patch('desktop.lib.botserver.views._get_gist_document') as _get_gist_document:
+          with patch('desktop.lib.botserver.views.slack_client.users_info') as users_info:
             with patch('desktop.lib.botserver.views.send_result_file') as send_result_file:
 
-              client = make_logged_in_client(username="test", groupname="default", recreate=True, is_superuser=False)
-              user = User.objects.get(username="test")
               channel_id = "channel"
               message_ts = "12.1"
+              user_id = "<@user_id>"
 
-              # qhistory link
-              links = [{"url": "https://demo.gethue.com/hue/editor?editor=123456"}]
-              doc_data = {
-                "dialect": "mysql",
-                "snippets": [{
-                  "database": "hue",
-                  "statement_raw": "SELECT 5000",
-                }]
-              }
-              document2_objects_get.return_value = Mock(data=json.dumps(doc_data), owner=user)
-              mock_unfurl_payload.return_value = {
-                'payload': {},
-                'file_status': True,
-              }
-              handle_on_link_shared(channel_id, message_ts, links)
-              assert_true(chat_unfurl.called)
-              assert_true(send_result_file.called)
-
-              # gist link
               doc_data = {"statement_raw": "SELECT 98765"}
-              _get_gist_document.return_value = Mock(data=json.dumps(doc_data), owner=user, extra='mysql')
-              links = [{"url": "http://demo.gethue.com/hue/gist?uuid=random"}]
-
+              links = [{"url": "http://demo.gethue.com/hue/gist?uuid=some_uuid"}]
+              _get_gist_document.return_value = Mock(data=json.dumps(doc_data), owner=self.user, extra='mysql')
               mock_unfurl_payload.return_value = {
                 'payload': {},
                 'file_status': False,
               }
-              handle_on_link_shared(channel_id, message_ts, links)
-              assert_true(chat_unfurl.called)
 
-              # Cannot unfurl link with invalid links
-              inv_qhistory_url = "https://demo.gethue.com/hue/editor/?type=4"
-              inv_gist_url = "http://demo.gethue.com/hue/gist?uuids/=xyz"
-              assert_raises(PopupException, handle_on_link_shared, "channel", "12.1", [{"url": inv_qhistory_url}])
-              assert_raises(PopupException, handle_on_link_shared, "channel", "12.1", [{"url": inv_gist_url}])
+              # Slack user who is Hue user sends link
+              users_info.return_value = {
+                "ok": True,
+                "user": {
+                  "profile": {
+                    "email": "test_not_me@example.com"
+                  }
+                }
+              }
+              handle_on_link_shared(channel_id, message_ts, links, user_id)
 
-              # Document does not exist
-              document2_objects_get.side_effect = PopupException('Query document does not exist')
-              _get_gist_document.side_effect = PopupException('Gist does not exist')
+              assert_true(chat_unfurl.called)
+              assert_false(send_result_file.called)
 
-              qhistory_url = "https://demo.gethue.com/hue/editor?editor=109644"
+              # Gist document does not exist
+              _get_gist_document.side_effect = PopupException('Gist does not exist')
               gist_url = "https://demo.gethue.com/hue/gist?uuid=6d1c407b-d999-4dfd-ad23-d3a46c19a427"
-              assert_raises(PopupException, handle_on_link_shared, "channel", "12.1", [{"url": qhistory_url}])
-              assert_raises(PopupException, handle_on_link_shared, "channel", "12.1", [{"url": gist_url}])
 
-              # chat_unfurl exception
-              chat_unfurl.side_effect = PopupException('Cannot unfurl link')
-              assert_raises(PopupException, handle_on_link_shared, "channel", "12.1", links)
+              assert_raises(PopupException, handle_on_link_shared, "channel", "12.1", [{"url": gist_url}], "<@user_id>")
+
+              # Cannot unfurl with invalid gist link
+              inv_gist_url = "http://demo.gethue.com/hue/gist?uuids/=invalid_link"
+              assert_raises(PopupException, handle_on_link_shared, "channel", "12.1", [{"url": inv_gist_url}], "<@user_id>")
+
+  def test_slack_user_not_hue_user(self):
+    with patch('desktop.lib.botserver.views.slack_client.users_info') as users_info:
+      with patch('desktop.lib.botserver.views._get_gist_document') as _get_gist_document:
+        
+        # Can be checked similarly with query link too
+        doc_data = {"statement_raw": "SELECT 98765"}
+        links = [{"url": "http://demo.gethue.com/hue/gist?uuid=some_uuid"}]
+        _get_gist_document.return_value = Mock(data=json.dumps(doc_data), owner=self.user, extra='mysql')
+
+        users_info.return_value = {
+          "ok": True,
+          "user": {
+            "profile": {
+              "email": "test_user_not_exist@example.com"
+            }
+          }
+        }
+        assert_raises(PopupException, handle_on_link_shared, "channel", "12.1", links, "<@user_id>")