There's one issue in current Authentication support, which may results in signinVerify invoke be processed by wrong OAuth Connection, thus blocks user sign in.
The root cause is:
Each BotAuthentication instance will register a handler for signinVerify invoke. And the Application class will execute the signinVerify invoke handlers based on the registration order and finish the activity processing after executed the first handler. So the first BotAuthentication will handle all the signinVerify invokes, which will result in error if user wants to signin with another OAuth Connection settings.
To Reproduce
Add following authentication options to 06.auth.botSSO:
{
settings: {
sharepoint: {
connectionName: process.env.SharepointConnectionName ?? '',
title: 'Sign in',
text: 'Please sign in to use the bot.',
endOnInvalidMessage: true
},
graph: {
connectionName: process.env.ConnectionName ?? '',
title: 'Sign in',
text: 'Please sign in to use the bot.',
endOnInvalidMessage: true
}
},
default: "graph"
}
Some thoughts about the solution
Here's something in my mind for open discussion:
The signinVerify activity only has a state (magic code) payload, so there's no information for us to determine which signinVerify handler should be executed in the route selector. One possible solution is making the runDialog a global operation (it's currently per OAuthPromptSetting). So we only need to have one signinVerify handler and get rid of this problem. Because there should be only one authentication happens at the same time, having a global runDialog should be technically feasible. This may involve many refactors though.
Some other things we need to consider
I haven't got time to verify but the tokenExchange activity should has similar issue. Though we used FilteredTeamsSSOTokenExchangeMiddleware in the app, the runDialog method of the first BotAuthentication will still be executed after the middleware finishes. But since tokenExchange activity provides connection name, we can filter based on the connection name in the route selector to avoid this issue. I already made some refactor to make this easy to happen.
singhk97
commented
4 months ago
Describe the bug
There's one issue in current Authentication support, which may results in
signinVerifyinvoke be processed by wrong OAuth Connection, thus blocks user sign in.The root cause is: Each
BotAuthenticationinstance will register a handler forsigninVerifyinvoke. And theApplicationclass will execute thesigninVerifyinvoke handlers based on the registration order and finish the activity processing after executed the first handler. So the firstBotAuthenticationwill handle all thesigninVerifyinvokes, which will result in error if user wants to signin with another OAuth Connection settings.To Reproduce
06.auth.botSSO:I have created a branch with updated bot SSO sample to repro this issue: https://github.com/microsoft/teams-ai/tree/chyuan/multi-connection-issue/js/samples/06.auth.botSSO
Expected behavior
The signin should success
Some thoughts about the solution Here's something in my mind for open discussion: The
signinVerifyactivity only has astate(magic code) payload, so there's no information for us to determine whichsigninVerifyhandler should be executed in the route selector. One possible solution is making therunDialoga global operation (it's currently per OAuthPromptSetting). So we only need to have onesigninVerifyhandler and get rid of this problem. Because there should be only one authentication happens at the same time, having a globalrunDialogshould be technically feasible. This may involve many refactors though.Some other things we need to consider I haven't got time to verify but the
tokenExchangeactivity should has similar issue. Though we usedFilteredTeamsSSOTokenExchangeMiddlewarein the app, therunDialogmethod of the firstBotAuthenticationwill still be executed after the middleware finishes. But sincetokenExchangeactivity provides connection name, we can filter based on the connection name in the route selector to avoid this issue. I already made some refactor to make this easy to happen.