microsoft · axelsrz · May 6, 2020 · May 6, 2020
diff --git a/libraries/botbuilder-core/botbuilder/core/bot_framework_adapter.py b/libraries/botbuilder-core/botbuilder/core/bot_framework_adapter.py
@@ -481,14 +481,8 @@ async def process_activity_with_identity(
 
         await self.run_pipeline(context, logic)
 
-        if activity.type == ActivityTypes.invoke:
-            invoke_response = context.turn_state.get(
-                BotFrameworkAdapter._INVOKE_RESPONSE_KEY  # pylint: disable=protected-access
-            )
-            if invoke_response is None:
-                return InvokeResponse(status=int(HTTPStatus.NOT_IMPLEMENTED))
-            return invoke_response.value
-
+        # Handle ExpectedReplies scenarios where the all the activities have been buffered and sent back at once
+        # in an invoke response.
         # Return the buffered activities in the response.  In this case, the invoker
         # should deserialize accordingly:
         #    activities = ExpectedReplies().deserialize(response.body).activities
@@ -498,6 +492,16 @@ async def process_activity_with_identity(
             ).serialize()
             return InvokeResponse(status=int(HTTPStatus.OK), body=expected_replies)
 
+        # Handle Invoke scenarios, which deviate from the request/request model in that
+        # the Bot will return a specific body and return code.
+        if activity.type == ActivityTypes.invoke:
+            invoke_response = context.turn_state.get(
+                BotFrameworkAdapter._INVOKE_RESPONSE_KEY  # pylint: disable=protected-access
+            )
+            if invoke_response is None:
+                return InvokeResponse(status=int(HTTPStatus.NOT_IMPLEMENTED))
+            return invoke_response.value
+
         return None
 
     async def __generate_callerid(self, claims_identity: ClaimsIdentity) -> str:

diff --git a/libraries/botbuilder-core/botbuilder/core/invoke_response.py b/libraries/botbuilder-core/botbuilder/core/invoke_response.py
@@ -24,3 +24,11 @@ def __init__(self, status: int = None, body: object = None):
         """
         self.status = status
         self.body = body
+
+    def is_successful_status_code(self) -> bool:
+        """
+        Gets a value indicating whether the invoke response was successful.
+        :return: A value that indicates if the HTTP response was successful. true if status is in
+        the Successful range (200-299); otherwise false.
+        """
+        return 200 <= self.status <= 299
diff --git a/libraries/botbuilder-dialogs/botbuilder/dialogs/skills/begin_skill_dialog_options.py b/libraries/botbuilder-dialogs/botbuilder/dialogs/skills/begin_skill_dialog_options.py
@@ -5,19 +5,13 @@
 
 
 class BeginSkillDialogOptions:
-    def __init__(
-        self, activity: Activity, connection_name: str = None
-    ):  # pylint: disable=unused-argument
+    def __init__(self, activity: Activity):
         self.activity = activity
-        self.connection_name = connection_name
 
     @staticmethod
     def from_object(obj: object) -> "BeginSkillDialogOptions":
         if isinstance(obj, dict) and "activity" in obj:
-            return BeginSkillDialogOptions(obj["activity"], obj.get("connection_name"))
+            return BeginSkillDialogOptions(obj["activity"])
         if hasattr(obj, "activity"):
-            return BeginSkillDialogOptions(
-                obj.activity,
-                obj.connection_name if hasattr(obj, "connection_name") else None,
-            )
+            return BeginSkillDialogOptions(obj.activity)
         return None
diff --git a/libraries/botbuilder-dialogs/botbuilder/dialogs/skills/skill_dialog.py b/libraries/botbuilder-dialogs/botbuilder/dialogs/skills/skill_dialog.py
@@ -36,15 +36,14 @@ def __init__(self, dialog_options: SkillDialogOptions, dialog_id: str):
 
         self.dialog_options = dialog_options
         self._deliver_mode_state_key = "deliverymode"
-        self._sso_connection_name_key = "SkillDialog.SSOConnectionName"
 
     async def begin_dialog(self, dialog_context: DialogContext, options: object = None):
         """
         Method called when a new dialog has been pushed onto the stack and is being activated.
         :param dialog_context: The dialog context for the current turn of conversation.
         :param options: (Optional) additional argument(s) to pass to the dialog being started.
         """
-        dialog_args = SkillDialog._validate_begin_dialog_args(options)
+        dialog_args = self._validate_begin_dialog_args(options)
 
         await dialog_context.context.send_trace_activity(
             f"{SkillDialog.__name__}.BeginDialogAsync()",
@@ -61,24 +60,22 @@ async def begin_dialog(self, dialog_context: DialogContext, options: object = No
             is_incoming=True,
         )
 
+        # Store delivery mode in dialog state for later use.
         dialog_context.active_dialog.state[
             self._deliver_mode_state_key
         ] = dialog_args.activity.delivery_mode
 
-        dialog_context.active_dialog.state[
-            self._sso_connection_name_key
-        ] = dialog_args.connection_name
-
         # Send the activity to t
8000
he skill.
-        eoc_activity = await self._send_to_skill(
-            dialog_context.context, skill_activity, dialog_args.connection_name
-        )
+        eoc_activity = await self._send_to_skill(dialog_context.context, skill_activity)
         if eoc_activity:
             return await dialog_context.end_dialog(eoc_activity.value)
 
         return self.end_of_turn
 
     async def continue_dialog(self, dialog_context: DialogContext):
+        if not self._on_validate_activity(dialog_context.context.activity):
+            return self.end_of_turn
+
         await dialog_context.context.send_trace_activity(
             f"{SkillDialog.__name__}.continue_dialog()",
             label=f"ActivityType: {dialog_context.context.activity.type}",
@@ -98,17 +95,13 @@ async def continue_dialog(self, dialog_context: DialogContext):
 
         # Create deep clone of the original activity to avoid altering it before forwarding it.
         skill_activity = deepcopy(dialog_context.context.activity)
+
         skill_activity.delivery_mode = dialog_context.active_dialog.state[
             self._deliver_mode_state_key
         ]
-        connection_name = dialog_context.active_dialog.state[
-            self._sso_connection_name_key
-        ]
 
         # Just forward to the remote skill
-        eoc_activity = await self._send_to_skill(
-            dialog_context.context, skill_activity, connection_name
-        )
+        eoc_activity = await self._send_to_skill(dialog_context.context, skill_activity)
         if eoc_activity:
             return await dialog_context.end_dialog(eoc_activity.value)
 
@@ -163,8 +156,7 @@ async def end_dialog(
 
         await super().end_dialog(context, instance, reason)
 
-    @staticmethod
-    def _validate_begin_dialog_args(options: object) -> BeginSkillDialogOptions:
+    def _validate_begin_dialog_args(self, options: object) -> BeginSkillDialogOptions:
         if not options:
             raise TypeError("options cannot be None.")
 
@@ -182,26 +174,36 @@ def _validate_begin_dialog_args(options: object) -> BeginSkillDialogOptions:
 
         return dialog_args
 
+    def _on_validate_activity(
+        self, activity: Activity  # pylint: disable=unused-argument
+    ) -> bool:
+        """
+        Validates the activity sent during continue_dialog.
+
+        Override this method to implement a custom validator for the activity being sent during continue_dialog.
+        This method can be used to ignore activities of a certain type if needed.
+        If this method returns false, the dialog will end the turn without processing the activity.
+        """
+        return True
+
     async def _send_to_skill(
-        self, context: TurnContext, activity: Activity, connection_name: str = None
+        self, context: TurnContext, activity: Activity
     ) -> Activity:
-        # Create a conversationId to interact with the skill and send the activity
-        conversation_id_factory_options = SkillConversationIdFactoryOptions(
-            from_bot_oauth_scope=context.turn_state.get(BotAdapter.BOT_OAUTH_SCOPE_KEY),
-            from_bot_id=self.dialog_options.bot_id,
-            activity=activity,
-            bot_framework_skill=self.dialog_options.skill,
-        )
-
-        skill_conversation_id = await self.dialog_options.conversation_id_factory.create_skill_conversation_id(
-            conversation_id_factory_options
+        if activity.type == ActivityTypes.invoke:
+            # Force ExpectReplies for invoke activities so we can get the replies right away and send
+            # them back to the channel if needed. This makes sure that the dialog will receive the Invoke
+            # response from the skill and any other activities sent, including EoC.
+            activity.delivery_mode = DeliveryModes.expect_replies
+
+        skill_conversation_id = await self._create_skill_conversation_id(
+            context, activity
         )
 
         # Always save state before forwarding
         # (the dialog stack won't get updated with the skillDialog and things won't work if you don't)
-        skill_info = self.dialog_options.skill
         await self.dialog_options.conversation_state.save_changes(context, True)
 
+        skill_info = self.dialog_options.skill
         response = await self.dialog_options.skill_client.post_activity(
             self.dialog_options.bot_id,
             skill_info.app_id,
@@ -229,7 +231,7 @@ async def _send_to_skill(
                     # Capture the EndOfConversation activity if it was sent from skill
                     eoc_activity = from_skill_activity
                 elif await self._intercept_oauth_cards(
-                    context, from_skill_activity, connection_name
+                    context, from_skill_activity, self.dialog_options.connection_name
                 ):
                     # do nothing. Token exchange succeeded, so no oauthcard needs to be shown to the user
                     pass
@@ -239,6 +241,21 @@ async def _send_to_skill(
 
         return eoc_activity
 
+    async def _create_skill_conversation_id(
+        self, context: TurnContext, activity: Activity
+    ) -> str:
+        # Create a conversationId to interact with the skill and send the activity
+        conversation_id_factory_options = SkillConversationIdFactoryOptions(
+            from_bot_oauth_scope=context.turn_state.get(BotAdapter.BOT_OAUTH_SCOPE_KEY),
+            from_bot_id=self.dialog_options.bot_id,
+            activity=activity,
+            bot_framework_skill=self.dialog_options.skill,
+        )
+        skill_conversation_id = await self.dialog_options.conversation_id_factory.create_skill_conversation_id(
+            conversation_id_factory_options
+        )
+        return skill_conversation_id
+
     async def _intercept_oauth_cards(
         self, context: TurnContext, activity: Activity, connection_name: str
     ):
@@ -248,6 +265,8 @@ async def _intercept_oauth_cards(
         if not connection_name or not isinstance(
             context.adapter, ExtendedUserTokenProvider
         ):
+            # The adapter may choose not to support token exchange, in which case we fallback to
+            # showing an oauth card to the user.
             return False
 
         oauth_card_attachment = next(
@@ -273,13 +292,17 @@ async def _intercept_oauth_cards(
                     )
 
                     if result and result.token:
+                        # If token above is null, then SSO has failed and hence we return false.
+                        # If not, send an invoke to the skill with the token.
                         return await self._send_token_exchange_invoke_to_skill(
                             activity,
                             oauth_card.token_exchange_resource.id,
                             oauth_card.connection_name,
                             result.token,
                         )
                 except:
+                    # Failures in token exchange are not fatal. They simply mean that the user needs
+                    # to be shown the OAuth card.
                     return False
 
         return False
@@ -310,4 +333,4 @@ async def _send_token_exchange_invoke_to_skill(
         )
 
         # Check response status: true if success, false if failure
-        return response.status / 100 == 2
+        return response.is_successful_status_code()
diff --git a/libraries/botbuilder-dialogs/botbuilder/dialogs/skills/skill_dialog_options.py b/libraries/botbuilder-dialogs/botbuilder/dialogs/skills/skill_dialog_options.py
@@ -18,10 +18,12 @@ def __init__(
         skill: BotFrameworkSkill = None,
         conversation_id_factory: ConversationIdFactoryBase = None,
        conversation_state: ConversationState = None,
+        connection_name: str = None,
     ):
         self.bot_id = bot_id
         self.skill_client = skill_client
         self.skill_host_endpoint = skill_host_endpoint
         self.skill = skill
         self.conversation_id_factory = conversation_id_factory
         self.conversation_state = conversation_state
+        self.connection_name = connection_name