Repository: microsoft/botbuilder-python
Pull request #203 (2019-06-07 23:59:02)

Added middleware to address transitional location of tenant_id field 
Added middleware to address transitional location of tenant_id field for teams in BotFrameworkAdapter. More testing on adapter pending.

*This PR address issues #139 and #140 


Patch:
diff --git a/libraries/botbuilder-core/botbuilder/core/bot_framework_adapter.py b/libraries/botbuilder-core/botbuilder/core/bot_framework_adapter.py
index bbebb3e2..24e18b74 100644
--- a/libraries/botbuilder-core/botbuilder/core/bot_framework_adapter.py
+++ b/libraries/botbuilder-core/botbuilder/core/bot_framework_adapter.py
@@ -7,13 +7,14 @@ from botbuilder.schema import (Activity, ChannelAccount,
                                ConversationAccount,
                                ConversationParameters, ConversationReference,
                                ConversationsResult, ConversationResourceResponse)
-from botframework.connector import ConnectorClient
+from botframework.connector import ConnectorClient, Channels
 from botframework.connector.auth import (MicrosoftAppCredentials,
                                          JwtTokenValidation, SimpleCredentialProvider)
 
 from . import __version__
 from .bot_adapter import BotAdapter
 from .turn_context import TurnContext
+from .middleware_set import Middleware
 
 USER_AGENT = f"Microsoft-BotFramework/3.1 (BotBuilder Python/{__version__})"
 
@@ -64,6 +65,14 @@ class BotFrameworkAdapter(BotAdapter):
             parameters = ConversationParameters(bot=reference.bot)
             client = self.create_connector_client(reference.service_url)
 
+            # Mix in the tenant ID if specified. This is required for MS Teams.
+            if reference.conversation is not None and reference.conversation.tenant_id:
+                # Putting tenant_id in channel_data is a temporary while we wait for the Teams API to be updated
+                parameters.channel_data = {'tenant': {'id': reference.conversation.tenant_id}}
+
+                # Permanent solution is to put tenant_id in parameters.tenant_id
+                parameters.tenant_id = reference.conversation.tenant_id
+
             resource_response = await client.conversations.create_conversation(parameters)
             request = TurnContext.apply_conversation_reference(Activity(), reference, is_incoming=True)
             request.conversation = ConversationAccount(id=resource_response.id)
@@ -85,13 +94,25 @@ class BotFrameworkAdapter(BotAdapter):
         :param auth_header:
         :param logic:
         :return:
-        """
+        """       
         activity = await self.parse_request(req)
         auth_header = auth_header or ''
 
         await self.authenticate_request(activity, auth_header)
         context = self.create_context(activity)
 
+        # Fix to assign tenant_id from channelData to Conversation.tenant_id.
+        # MS Teams currently sends the tenant ID in channelData and the correct behavior is to expose 
+        # this value in Activity.Conversation.tenant_id.
+        # This code copies the tenant ID from channelData to Activity.Conversation.tenant_id.
+        # Once MS Teams sends the tenant_id in the Conversation property, this code can be removed.
+        if (Channels.ms_teams == context.activity.channel_id and context.activity.conversation is not None
+                and not context.activity.conversation.tenant_id):
+            teams_channel_data = context.activity.channel_data
+            if teams_channel_data.get("tenant", {}).get("id", None):
+                context.activity.conversation.tenant_id = str(teams_channel_data["tenant"]["id"])
+        
+        
         return await self.run_pipeline(context, logic)
 
     async def authenticate_request(self, request: Activity, auth_header: str):
@@ -123,12 +144,12 @@ class BotFrameworkAdapter(BotAdapter):
             if not isinstance(activity.type, str):
                 raise TypeError('BotFrameworkAdapter.parse_request(): invalid or missing activity type.')
             return True
-
         if not isinstance(req, Activity):
             # If the req is a raw HTTP Request, try to deserialize it into an Activity and return the Activity.
-            if hasattr(req, 'body'):
+            if getattr(req, 'body_exists', False):
                 try:
-                    activity = Activity().deserialize(req.body)
+                    body = await req.json()
+                    activity = Activity().deserialize(body)
                     is_valid_activity = await validate_activity(activity)
                     if is_valid_activity:
                         return activity
diff --git a/libraries/botbuilder-schema/botbuilder/schema/_models.py b/libraries/botbuilder-schema/botbuilder/schema/_models.py
index d1fd5966..00673fd4 100644
--- a/libraries/botbuilder-schema/botbuilder/schema/_models.py
+++ b/libraries/botbuilder-schema/botbuilder/schema/_models.py
@@ -628,6 +628,8 @@ class ConversationAccount(Model):
     :param role: Role of the entity behind the account (Example: User, Bot,
      etc.). Possible values include: 'user', 'bot'
     :type role: str or ~botframework.connector.models.RoleTypes
+    :param tenant_id: This conversation's tenant ID
+    :type tenant_id: str
     """
 
     _attribute_map = {
@@ -637,6 +639,7 @@ class ConversationAccount(Model):
         'name': {'key': 'name', 'type': 'str'},
         'aad_object_id': {'key': 'aadObjectId', 'type': 'str'},
         'role': {'key': 'role', 'type': 'str'},
+        'tenant_id': {'key': 'tenantID', 'type': 'str'},
     }
 
     def __init__(self, **kwargs):
@@ -647,6 +650,7 @@ class ConversationAccount(Model):
         self.name = kwargs.get('name', None)
         self.aad_object_id = kwargs.get('aad_object_id', None)
         self.role = kwargs.get('role', None)
+        self.tenant_id = kwargs.get('tenant_id', None)
 
 
 class ConversationMembers(Model):
@@ -687,6 +691,8 @@ class ConversationParameters(Model):
     :param channel_data: Channel specific payload for creating the
      conversation
     :type channel_data: object
+    :param tenant_id: (Optional) The tenant ID in which the conversation should be created
+    :type tenant_id: str
     """
 
     _attribute_map = {
@@ -696,6 +702,7 @@ class ConversationParameters(Model):
         'topic_name': {'key': 'topicName', 'type': 'str'},
         'activity': {'key': 'activity', 'type': 'Activity'},
         'channel_data': {'key': 'channelData', 'type': 'object'},
+        'tenant_id': {'key': 'tenantID', 'type': 'str'},
     }
 
     def __init__(self, **kwargs):
@@ -706,6 +713,7 @@ class ConversationParameters(Model):
         self.topic_name = kwargs.get('topic_name', None)
         self.activity = kwargs.get('activity', None)
         self.channel_data = kwargs.get('channel_data', None)
+        self.tenant_id = kwargs.get('tenant_id', None)
 
 
 class ConversationReference(Model):
diff --git a/libraries/botbuilder-schema/botbuilder/schema/_models_py3.py b/libraries/botbuilder-schema/botbuilder/schema/_models_py3.py
index e6c0f9ac..dfe40781 100644
--- a/libraries/botbuilder-schema/botbuilder/schema/_models_py3.py
+++ b/libraries/botbuilder-schema/botbuilder/schema/_models_py3.py
@@ -628,6 +628,8 @@ class ConversationAccount(Model):
     :param role: Role of the entity behind the account (Example: User, Bot,
      etc.). Possible values include: 'user', 'bot'
     :type role: str or ~botframework.connector.models.RoleTypes
+    :param tenant_id: This conversation's tenant ID
+    :type tenant_id: str
     """
 
     _attribute_map = {
@@ -637,9 +639,10 @@ class ConversationAccount(Model):
         'name': {'key': 'name', 'type': 'str'},
         'aad_object_id': {'key': 'aadObjectId', 'type': 'str'},
         'role': {'key': 'role', 'type': 'str'},
+        'tenant_id': {'key': 'tenantID', 'type': 'str'},
     }
 
-    def __init__(self, *, is_group: bool=None, conversation_type: str=None, id: str=None, name: str=None, aad_object_id: str=None, role=None, **kwargs) -> None:
+    def __init__(self, *, is_group: bool=None, conversation_type: str=None, id: str=None, name: str=None, aad_object_id: str=None, role=None, tenant_id=None, **kwargs) -> None:
         super(ConversationAccount, self).__init__(**kwargs)
         self.is_group = is_group
         self.conversation_type = conversation_type
@@ -647,6 +650,7 @@ class ConversationAccount(Model):
         self.name = name
         self.aad_object_id = aad_object_id
         self.role = role
+        self.tenant_id = tenant_id
 
 
 class ConversationMembers(Model):
@@ -687,6 +691,8 @@ class ConversationParameters(Model):
     :param channel_data: Channel specific payload for creating the
      conversation
     :type channel_data: object
+    :param tenant_id: (Optional) The tenant ID in which the conversation should be created
+    :type tenant_id: str
     """
 
     _attribute_map = {
@@ -696,9 +702,10 @@ class ConversationParameters(Model):
         'topic_name': {'key': 'topicName', 'type': 'str'},
         'activity': {'key': 'activity', 'type': 'Activity'},
         'channel_data': {'key': 'channelData', 'type': 'object'},
+        'tenant_id': {'key': 'tenantID', 'type': 'str'},
     }
 
-    def __init__(self, *, is_group: bool=None, bot=None, members=None, topic_name: str=None, activity=None, channel_data=None, **kwargs) -> None:
+    def __init__(self, *, is_group: bool=None, bot=None, members=None, topic_name: str=None, activity=None, channel_data=None, tenant_id=None, **kwargs) -> None:
         super(ConversationParameters, self).__init__(**kwargs)
         self.is_group = is_group
         self.bot = bot
@@ -706,6 +713,7 @@ class ConversationParameters(Model):
         self.topic_name = topic_name
         self.activity = activity
         self.channel_data = channel_data
+        self.tenant_id = tenant_id
 
 
 class ConversationReference(Model):


Test patch:
diff --git a/libraries/botbuilder-core/tests/test_bot_framework_adapter.py b/libraries/botbuilder-core/tests/test_bot_framework_adapter.py
new file mode 100644
index 00000000..a6aedee5
--- /dev/null
+++ b/libraries/botbuilder-core/tests/test_bot_framework_adapter.py
@@ -0,0 +1,166 @@
+# Copyright (c) Microsoft Corporation. All rights reserved.
+# Licensed under the MIT License.
+
+import aiounittest
+import unittest
+from copy import copy
+from unittest.mock import Mock
+
+from botbuilder.core import BotFrameworkAdapter, BotFrameworkAdapterSettings, TurnContext
+from botbuilder.schema import Activity, ActivityTypes, ConversationAccount, ConversationReference, ChannelAccount
+from botframework.connector import ConnectorClient
+from botframework.connector.auth import ClaimsIdentity
+
+reference = ConversationReference(
+    activity_id='1234',
+    channel_id='test',
+    service_url='https://example.org/channel',
+    user=ChannelAccount(
+        id='user',
+        name='User Name'
+    ),
+    bot=ChannelAccount(
+        id='bot',
+        name='Bot Name'
+    ),
+    conversation=ConversationAccount(
+        id='convo1'
+    )
+)
+
+test_activity = Activity(text='test', type=ActivityTypes.message)
+
+incoming_message = TurnContext.apply_conversation_reference(copy(test_activity), reference, True)
+outgoing_message = TurnContext.apply_conversation_reference(copy(test_activity), reference)
+incoming_invoke = TurnContext.apply_conversation_reference(Activity(type=ActivityTypes.invoke), reference, True)
+
+
+class AdapterUnderTest(BotFrameworkAdapter):
+
+    def __init__(self, settings=None):
+        super().__init__(settings)
+        self.tester = aiounittest.AsyncTestCase()
+        self.fail_auth = False
+        self.fail_operation = False
+        self.expect_auth_header = ''
+        self.new_service_url = None
+
+    def aux_test_authenticate_request(self, request: Activity, auth_header: str):
+        return super().authenticate_request(request, auth_header)
+
+    def aux_test_create_connector_client(self, service_url: str):
+        return super().create_connector_client(service_url)
+
+    async def authenticate_request(self, request: Activity, auth_header: str):
+        self.tester.assertIsNotNone(request, 'authenticate_request() not passed request.')
+        self.tester.assertEqual(auth_header, self.expect_auth_header, 'authenticateRequest() not passed expected authHeader.')
+        return not self.fail_auth
+
+    def create_connector_client(self, service_url: str) -> ConnectorClient:
+        self.tester.assertIsNotNone(service_url, 'create_connector_client() not passed service_url.')
+        connector_client_mock = Mock()
+
+        def mock_reply_to_activity(conversation_id, activity_id, activity):
+            nonlocal self
+            self.tester.assertIsNotNone(conversation_id, 'reply_to_activity not passed conversation_id')
+            self.tester.assertIsNotNone(activity_id, 'reply_to_activity not passed activity_id')
+            self.tester.assertIsNotNone(activity, 'reply_to_activity not passed activity')
+            return not self.fail_auth
+
+        def mock_send_to_conversation(conversation_id, activity):
+            nonlocal self
+            self.tester.assertIsNotNone(conversation_id, 'send_to_conversation not passed conversation_id')
+            self.tester.assertIsNotNone(activity, 'send_to_conversation not passed activity')
+            return not self.fail_auth
+
+        def mock_update_activity(conversation_id, activity_id, activity):
+            nonlocal self
+            self.tester.assertIsNotNone(conversation_id, 'update_activity not passed conversation_id')
+            self.tester.assertIsNotNone(activity_id, 'update_activity not passed activity_id')
+            self.tester.assertIsNotNone(activity, 'update_activity not passed activity')
+            return not self.fail_auth
+
+        def mock_delete_activity(conversation_id, activity_id):
+            nonlocal self
+            self.tester.assertIsNotNone(conversation_id, 'delete_activity not passed conversation_id')
+            self.tester.assertIsNotNone(activity_id, 'delete_activity not passed activity_id')
+            return not self.fail_auth
+
+        def mock_create_conversation(parameters):
+            nonlocal self
+            self.tester.assertIsNotNone(parameters, 'create_conversation not passed parameters')
+            return not self.fail_auth
+
+        connector_client_mock.conversations.reply_to_activity.side_effect = mock_reply_to_activity
+        connector_client_mock.conversations.send_to_conversation.side_effect = mock_send_to_conversation
+        connector_client_mock.conversations.update_activity.side_effect = mock_update_activity
+        connector_client_mock.conversations.delete_activity.side_effect = mock_delete_activity
+        connector_client_mock.conversations.create_conversation.side_effect = mock_create_conversation
+
+        return connector_client_mock
+
+
+async def process_activity(channel_id: str, channel_data_tenant_id: str, conversation_tenant_id: str):
+    activity = None
+    mock_claims = unittest.mock.create_autospec(ClaimsIdentity)
+    mock_credential_provider = unittest.mock.create_autospec(BotFrameworkAdapterSettings)
+
+    sut = BotFrameworkAdapter(mock_credential_provider)
+
+    async def aux_func(context):
+        nonlocal activity
+        activity = context.Activity
+
+    await sut.process_activity(
+        Activity(
+            channel_id=channel_id,
+            service_url="https://smba.trafficmanager.net/amer/",
+            channel_data={"tenant": {"id": channel_data_tenant_id}},
+            conversation=ConversationAccount(
+                tenant_id=conversation_tenant_id
+            ),
+        ),
+        mock_claims,
+        aux_func)
+    return activity
+
+
+class TestBotFrameworkAdapter(aiounittest.AsyncTestCase):
+
+    def test_should_create_connector_client(self):
+        adapter = AdapterUnderTest()
+        client = adapter.aux_test_create_connector_client(reference.service_url)
+        self.assertIsNotNone(client, 'client not returned.')
+        self.assertIsNotNone(client.conversations, 'invalid client returned.')
+
+    async def test_should_process_activity(self):
+        called = False
+        adapter = AdapterUnderTest()
+
+        async def aux_func_assert_context(context):
+            self.assertIsNotNone(context, 'context not passed.')
+            nonlocal called
+            called = True
+
+        await adapter.process_activity(incoming_message, '', aux_func_assert_context)
+        self.assertTrue(called, 'bot logic not called.')
+
+    async def test_should_migrate_tenant_id_for_msteams(self):
+        incoming = TurnContext.apply_conversation_reference(
+            activity=Activity(
+                type=ActivityTypes.message,
+                text='foo',
+                channel_data={'tenant': {'id': '1234'}}
+            ),
+            reference=reference,
+            is_incoming=True
+        )
+
+        incoming.channel_id = 'msteams'
+        adapter = AdapterUnderTest()
+
+        async def aux_func_assert_tenant_id_copied(context):
+            self.assertEquals(context.activity.conversation.tenant_id, '1234', 'should have copied tenant id from '
+                                                                               'channel_data to conversation address')
+
+        await adapter.process_activity(incoming, '', aux_func_assert_tenant_id_copied)