Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added middleware to address transitional location of tenant_id field #203

Merged
merged 3 commits into from
Jun 12, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions libraries/botbuilder-core/botbuilder/core/bot_adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from abc import ABC, abstractmethod
from typing import List, Callable, Awaitable
from botbuilder.schema import Activity, ConversationReference
from botframework.connector import Channels

from . import conversation_reference_extension
from .bot_assert import BotAssert
Expand Down Expand Up @@ -77,6 +78,19 @@ async def run_pipeline(self, context: TurnContext, callback: Callable[[TurnConte
BotAssert.context_not_none(context)

if context.activity is not None:

# Middleware to assign tenant_id from channelData to Conversation.tenant_id.
axelsrz marked this conversation as resolved.
Show resolved Hide resolved
# 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 middleware can be removed.
if (Channels.ms_teams == context.activity.channel_id and context.activity.conversation is not None
axelsrz marked this conversation as resolved.
Show resolved Hide resolved
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"])


try:
return await self._middleware.receive_activity_with_status(context, callback)
except Exception as error:
Expand Down
17 changes: 13 additions & 4 deletions libraries/botbuilder-core/botbuilder/core/bot_framework_adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,14 @@
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__})"

Expand Down Expand Up @@ -64,6 +65,14 @@ async def create_conversation(self, reference: ConversationReference, logic: Cal
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)
Expand Down Expand Up @@ -123,12 +132,12 @@ async def validate_activity(activity: Activity):
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
Expand Down
166 changes: 166 additions & 0 deletions libraries/botbuilder-core/tests/test_bot_framework_adapter.py
Original file line number Diff line number Diff line change
@@ -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)
8 changes: 8 additions & 0 deletions libraries/botbuilder-schema/botbuilder/schema/_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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):
Expand All @@ -647,6 +650,7 @@ def __init__(self, **kwargs):
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):
Expand Down Expand Up @@ -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 = {
Expand All @@ -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):
Expand All @@ -706,6 +713,7 @@ def __init__(self, **kwargs):
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):
Expand Down
12 changes: 10 additions & 2 deletions libraries/botbuilder-schema/botbuilder/schema/_models_py3.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -637,16 +639,18 @@ 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
self.id = id
self.name = name
self.aad_object_id = aad_object_id
self.role = role
self.tenant_id = tenant_id


class ConversationMembers(Model):
Expand Down Expand Up @@ -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 = {
Expand All @@ -696,16 +702,18 @@ 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
self.members = members
self.topic_name = topic_name
self.activity = activity
self.channel_data = channel_data
self.tenant_id = tenant_id


class ConversationReference(Model):
Expand Down