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

Add support in ActivityHandler for Teams signin (invoke) response #1963

Closed

Conversation

garypretty
Copy link
Contributor

Added support for Invoke Activities to ActivityHandler, with specific support for enabling forwarding of Teams Sign-in response to current dialog.

Currently ActivityHandler allows a developer to override the OnTokenResponseEventAsync activity to forward incoming token responses to the current dialog.

This change

  • Adds Invoke activity support to the ActivityHandler
  • Checks for an incoming signin/verifyState invoke activity from teams and sends an appropriate Invoke response back to Teams
  • Allows a developer to override new method OnMsTeamsSigninResponseInvokeAsync to forward to the current dialog (the OAuthPrompt already contains a check for an MSTeams Invoke Activity of this type).

e.g.

protected override async Task OnMsTeamsSigninResponseInvokeAsync(ITurnContext<IInvokeActivity> turnContext, CancellationToken cancellationToken)
        {
            Logger.LogInformation("Running dialog with MsTeams Token Response Event Activity.");

            // Run the Dialog with the new MsTeams Token Response Event Activity.
            await Dialog.Run(turnContext, ConversationState.CreateProperty<DialogState>(nameof(DialogState)), cancellationToken);
        }

@garypretty
Copy link
Contributor Author

@cleemullins @Jeffders open question - In this PR I am sending an invoke response to the invoke activity from Teams within the Activity Handler. This response could in theory be moved into the OAuth prompt, so the ActivityHandler is just responsible for routing. Thoughts?

@johnataylor
Copy link
Member

We could consider adding this Invoke routing to the ActivityHandler, however, we did that in 4.3 but pulled it because we thought we need to understand what the other Invoke's Teams needed were. We decided the way forward was a TeamsActivityHandler in samples, pending more definition around the problem. Basuically we decided to go with a minimal but extensible model when we introduced the feature.

Anyhow, is getting the activity into the Dialog the only issue here? Does the OAuth prompt do the right thing once its handed the activity?

@garypretty
Copy link
Contributor Author

garypretty commented May 28, 2019

@johnataylor I think the only thing the OAuthPrompt doesn't do is send an Invoke Response (it is checking for the incoming Invoke) - therefore, at the very least, we should be able to move the one line invoke response into the Teams handling method in the OAuthPrompt and simply have the ActivityHandler route the signin/vefifyState Invoke to the dialog as it does in the PR.

The TeamsActivityHandler makes a lot of sense, but I think, as a minimum, the routing of the token / signin responses should be consistently handled regardless of channel, if that makes sense. Then, maybe use a TeamsActivityHandler for other functionality, just as you would need additional code to handle special Facebook events. I just don't think it will make sense to a developer to have this glaring omission for such an important channel.

@stevengum stevengum added the on hold Progress is halted at the moment. label Jun 5, 2019
@johnataylor
Copy link
Member

@garypretty my understanding was this Invoke was MS Teams specific. I'll check with @Jeffders

@garypretty
Copy link
Contributor Author

@johnataylor sorry, I wasn't clear, yes it is teams specific. I was arguing for us adopting a default for routing the teams auth code (as per this pr) within the default ActivityHandler, as signin is something you want to work consistently across channels. Then, having a Teams specific handler for functionality specific to Teams.
Apologies for not being clearer - let me know if I am still not 😊

@cleemullins
Copy link
Contributor

This is an area where we're going to evolve quite a bit in the next iteration. We've been working with the Teams folks and have a plan in place. In addition, there is a set of samples (JS + C#) that we've prototyped a bit of this work in, found here.

@garypretty garypretty deleted the teams-auth-invoke-response branch August 6, 2019 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
on hold Progress is halted at the moment.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants