Merge branch 'trboehre/meetingalert' of https://github.com/microsoft/botbuilder-python into trboehre/meetingalert

Author: tracyboehrer

libraries/botbuilder-core/botbuilder/core/activity_handler.py

@@ -374,6 +374,36 @@ async def on_installation_update(  # pylint: disable=unused-argument
         Override this in a derived class to provide logic specific to
         ActivityTypes.InstallationUpdate activities.
 
+        :param turn_context: The context object for this turn
+        :type turn_context: :class:`botbuilder.core.TurnContext`
+        :returns: A task that represents the work queued to execute
+        """
+        if turn_context.activity.action == "add":
+            return await self.on_installation_update_add(turn_context)
+        if turn_context.activity.action == "remove":
+            return await self.on_installation_update_remove(turn_context)
+        return
+
+    async def on_installation_update_add(  # pylint: disable=unused-argument
+        self, turn_context: TurnContext
+    ):
+        """
+        Override this in a derived class to provide logic specific to
+        ActivityTypes.InstallationUpdate activities with 'action' set to 'add'.
+
+        :param turn_context: The context object for this turn
+        :type turn_context: :class:`botbuilder.core.TurnContext`
+        :returns: A task that represents the work queued to execute
+        """
+        return
+
+    async def on_installation_update_remove(  # pylint: disable=unused-argument
+        self, turn_context: TurnContext
+    ):
+        """
+        Override this in a derived class to provide logic specific to
+        ActivityTypes.InstallationUpdate activities with 'action' set to 'remove'.
+
         :param turn_context: The context object for this turn
         :type turn_context: :class:`botbuilder.core.TurnContext`
         :returns: A task that represents the work queued to execute

libraries/botbuilder-core/botbuilder/core/channel_service_handler.py

@@ -21,6 +21,7 @@
     ClaimsIdentity,
     CredentialProvider,
     JwtTokenValidation,
+    SkillValidation,
 )
 
 
@@ -469,18 +470,28 @@ async def on_upload_attachment(
         raise BotActionNotImplementedError()
 
     async def _authenticate(self, auth_header: str) -> ClaimsIdentity:
+        """
+        Helper to authenticate the header.
+
+        This code is very similar to the code in JwtTokenValidation.authenticate_request,
+        we should move this code somewhere in that library when we refactor auth,
+        for now we keep it private to avoid adding more public static functions that we will need to deprecate later.
+        """
         if not auth_header:
             is_auth_disabled = (
                 await self._credential_provider.is_authentication_disabled()
             )
-            if is_auth_disabled:
-                # In the scenario where Auth is disabled, we still want to have the
-                # IsAuthenticated flag set in the ClaimsIdentity. To do this requires
-                # adding in an empty claim.
-                return ClaimsIdentity({}, True)
+            if not is_auth_disabled:
+                # No auth header. Auth is required. Request is not authorized.
+                raise PermissionError()
 
-            raise PermissionError()
+            # In the scenario where Auth is disabled, we still want to have the
+            # IsAuthenticated flag set in the ClaimsIdentity. To do this requires
+            # adding in an empty claim.
+            # Since ChannelServiceHandler calls are always a skill callback call, we set the skill claim too.
+            return SkillValidation.create_anonymous_skill_claim()
 
+        # Validate the header and extract claims.
         return await JwtTokenValidation.validate_auth_header(
             auth_header,
             self._credential_provider,

libraries/botbuilder-core/tests/skills/test_skill_handler.py

@@ -2,11 +2,16 @@
 import json
 from uuid import uuid4
 from asyncio import Future
-from typing import Dict, List
+from typing import Dict, List, Callable
 
 from unittest.mock import Mock, MagicMock
 import aiounittest
 
+from botframework.connector.auth import (
+    AuthenticationConfiguration,
+    AuthenticationConstants,
+    ClaimsIdentity,
+)
 from botbuilder.core import (
     TurnContext,
     BotActionNotImplementedError,
@@ -28,11 +33,6 @@
     Transcript,
     CallerIdConstants,
 )
-from botframework.connector.auth import (
-    AuthenticationConfiguration,
-    AuthenticationConstants,
-    ClaimsIdentity,
-)
 
 
 class ConversationIdFactoryForTest(ConversationIdFactoryBase):
@@ -206,10 +206,30 @@ async def test_on_send_to_conversation(self):
         )
 
         mock_adapter = Mock()
-        mock_adapter.continue_conversation = MagicMock(return_value=Future())
-        mock_adapter.continue_conversation.return_value.set_result(Mock())
-        mock_adapter.send_activities = MagicMock(return_value=Future())
-        mock_adapter.send_activities.return_value.set_result([])
+
+        async def continue_conversation(
+            reference: ConversationReference,
+            callback: Callable,
+            bot_id: str = None,
+            claims_identity: ClaimsIdentity = None,
+            audience: str = None,
+        ):  # pylint: disable=unused-argument
+            await callback(
+                TurnContext(
+                    mock_adapter,
+                    conversation_reference_extension.get_continuation_activity(
+                        self._conversation_reference
+                    ),
+                )
+            )
+
+        async def send_activities(
+            context: TurnContext, activities: List[Activity]
+        ):  # pylint: disable=unused-argument
+            return [ResourceResponse(id="resourceId")]
+
+        mock_adapter.continue_conversation = continue_conversation
+        mock_adapter.send_activities = send_activities
 
         sut = self.create_skill_handler_for_testing(mock_adapter)
 
@@ -218,25 +238,12 @@ async def test_on_send_to_conversation(self):
 
         assert not activity.caller_id
 
-        await sut.test_on_send_to_conversation(
+        resource_response = await sut.test_on_send_to_conversation(
             self._claims_identity, self._conversation_id, activity
         )
 
-        args, kwargs = mock_adapter.continue_conversation.call_args_list[0]
-
-        assert isinstance(args[0], ConversationReference)
-        assert callable(args[1])
-        assert isinstance(kwargs["claims_identity"], ClaimsIdentity)
-
-        await args[1](
-            TurnContext(
-                mock_adapter,
-                conversation_reference_extension.get_continuation_activity(
-                    self._conversation_reference
-                ),
-            )
-        )
         assert activity.caller_id is None
+        assert resource_response.id == "resourceId"
 
     async def test_forwarding_on_send_to_conversation(self):
         self._conversation_id = await self._test_id_factory.create_skill_conversation_id(

libraries/botbuilder-core/tests/test_activity_handler.py

@@ -77,6 +77,14 @@ async def on_installation_update(self, turn_context: TurnContext):
         self.record.append("on_installation_update")
         return await super().on_installation_update(turn_context)
 
+    async def on_installation_update_add(self, turn_context: TurnContext):
+        self.record.append("on_installation_update_add")
+        return await super().on_installation_update_add(turn_context)
+
+    async def on_installation_update_remove(self, turn_context: TurnContext):
+        self.record.append("on_installation_update_remove")
+        return await super().on_installation_update_remove(turn_context)
+
     async def on_unrecognized_activity_type(self, turn_context: TurnContext):
         self.record.append("on_unrecognized_activity_type")
         return await super().on_unrecognized_activity_type(turn_context)
@@ -246,6 +254,34 @@ async def test_on_installation_update(self):
         assert len(bot.record) == 1
         assert bot.record[0] == "on_installation_update"
 
+    async def test_on_installation_update_add(self):
+        activity = Activity(type=ActivityTypes.installation_update, action="add")
+
+        adapter = TestInvokeAdapter()
+        turn_context = TurnContext(adapter, activity)
+
+        # Act
+        bot = TestingActivityHandler()
+        await bot.on_turn(turn_context)
+
+        assert len(bot.record) == 2
+        assert bot.record[0] == "on_installation_update"
+        assert bot.record[1] == "on_installation_update_add"
+
+    async def test_on_installation_update_add_remove(self):
+        activity = Activity(type=ActivityTypes.installation_update, action="remove")
+
+        adapter = TestInvokeAdapter()
+        turn_context = TurnContext(adapter, activity)
+
+        # Act
+        bot = TestingActivityHandler()
+        await bot.on_turn(turn_context)
+
+        assert len(bot.record) == 2
+        assert bot.record[0] == "on_installation_update"
+        assert bot.record[1] == "on_installation_update_remove"
+
     async def test_healthcheck(self):
         activity = Activity(type=ActivityTypes.invoke, name="healthcheck",)
 

libraries/botbuilder-core/tests/test_channel_service_handler.py

@@ -0,0 +1,46 @@
+# Copyright (c) Microsoft Corporation. All rights reserved.
+# Licensed under the MIT License.
+
+import aiounittest
+from botbuilder.core import ChannelServiceHandler
+from botframework.connector.auth import (
+    AuthenticationConfiguration,
+    ClaimsIdentity,
+    SimpleCredentialProvider,
+    JwtTokenValidation,
+    AuthenticationConstants,
+)
+import botbuilder.schema
+
+
+class TestChannelServiceHandler(ChannelServiceHandler):
+    def __init__(self):
+        self.claims_identity = None
+        ChannelServiceHandler.__init__(
+            self, SimpleCredentialProvider("", ""), AuthenticationConfiguration()
+        )
+
+    async def on_reply_to_activity(
+        self,
+        claims_identity: ClaimsIdentity,
+        conversation_id: str,
+        activity_id: str,
+        activity: botbuilder.schema.Activity,
+    ) -> botbuilder.schema.ResourceResponse:
+        self.claims_identity = claims_identity
+        return botbuilder.schema.ResourceResponse()
+
+
+class ChannelServiceHandlerTests(aiounittest.AsyncTestCase):
+    async def test_should_authenticate_anonymous_skill_claim(self):
+        sut = TestChannelServiceHandler()
+        await sut.handle_reply_to_activity(None, "123", "456", {})
+
+        assert (
+            sut.claims_identity.authentication_type
+            == AuthenticationConstants.ANONYMOUS_AUTH_TYPE
+        )
+        assert (
+            JwtTokenValidation.get_app_id_from_claims(sut.claims_identity.claims)
+            == AuthenticationConstants.ANONYMOUS_SKILL_APP_ID
+        )

libraries/botbuilder-integration-aiohttp/botbuilder/integration/aiohttp/bot_framework_http_client.py

@@ -15,6 +15,7 @@
     ConversationReference,
     ConversationAccount,
     ChannelAccount,
+    RoleTypes,
 )
 from botframework.connector.auth import (
     ChannelProvider,
@@ -97,7 +98,9 @@ async def post_activity(
             activity.conversation.id = conversation_id
             activity.service_url = service_url
             if not activity.recipient:
-                activity.recipient = ChannelAccount()
+                activity.recipient = ChannelAccount(role=RoleTypes.skill)
+            else:
+                activity.recipient.role = RoleTypes.skill
 
             status, content = await self._post_content(to_url, token, activity)
 

libraries/botbuilder-integration-aiohttp/tests/test_bot_framework_http_client.py

@@ -1,7 +1,7 @@
 from unittest.mock import Mock
 
 import aiounittest
-from botbuilder.schema import ConversationAccount, ChannelAccount
+from botbuilder.schema import ConversationAccount, ChannelAccount, RoleTypes
 from botbuilder.integration.aiohttp import BotFrameworkHttpClient
 from botframework.connector.auth import CredentialProvider, Activity
 
@@ -69,3 +69,4 @@ async def _mock_post_content(
         )
 
         assert activity.recipient.id == skill_recipient_id
+        assert activity.recipient.role is RoleTypes.skill

libraries/botbuilder-schema/botbuilder/schema/_connector_client_enums.py

@@ -8,6 +8,7 @@ class RoleTypes(str, Enum):
 
     user = "user"
     bot = "bot"
+    skill = "skill"
 
 
 class ActivityTypes(str, Enum):

libraries/botbuilder-schema/botbuilder/schema/_models_py3.py

@@ -1307,8 +1307,8 @@ class ConversationAccount(Model):
     :param aad_object_id: This account's object ID within Azure Active
      Directory (AAD)
     :type aad_object_id: str
-    :param role: Role of the entity behind the account (Example: User, Bot,
-     etc.). Possible values include: 'user', 'bot'
+    :param role: Role of the entity behind the account (Example: User, Bot, Skill
+     etc.). Possible values include: 'user', 'bot', 'skill'
     :type role: str or ~botframework.connector.models.RoleTypes
     :param tenant_id: This conversation's tenant ID
     :type tenant_id: str

libraries/botframework-connector/botframework/connector/auth/app_credentials.py

@@ -104,7 +104,11 @@ def signed_session(self, session: requests.Session = None) -> requests.Session:
     def _should_authorize(
         self, session: requests.Session  # pylint: disable=unused-argument
     ) -> bool:
-        return True
+        # We don't set the token if the AppId is not set, since it means that we are in an un-authenticated scenario.
+        return (
+            self.microsoft_app_id != AuthenticationConstants.ANONYMOUS_SKILL_APP_ID
+            and self.microsoft_app_id is not None
+        )
 
     def get_access_token(self, force_refresh: bool = False) -> str:
         """