microsoft / botframework-sdk

Bot Framework provides the most comprehensive experience for building conversation applications.
MIT License
7.49k stars 2.44k forks source link

AADv2 Magic Code Not Returning Token From GetUserTokenAsync #4880

Closed adamhockemeyer closed 5 years ago

adamhockemeyer commented 6 years ago

Bot Info

Issue Description

I followed the AADv2bot guide and created a bot, have it connected to AADv2, published to both Azure App Service and testing it locally. The bot is also successfully connected to the Twilio channel.

The bot both locally and via Azure App Service with the web channel will present the "Sign In" button card, prompt for me to enter my credentials and will successfully call the MS Graph and return my name.

Twilio however will send a long url for me to copy and paste into a browser, I then sign in via the browser and receive a 6 digit 'magic code' and enter that magic code back into the sms message back to the bot. However I am never authenticated with the magic code. I tried debugging this locally and manually calling await context.GetUserTokenAsync(ConnectionName, MagicCode) and it just results null.

In a breakpont that I set, the GetTokenResponse has null for Token and the magic code value I entered in as the NonTokenRepsonse. It appears the GetUserTokenAsync with the magic code is not working correctly.

Here I believe is the line of code returning the magic code back to me and not a token.

Code Example

private async Task ListMe(IDialogContext context, IAwaitable<GetTokenResponse> tokenResponse)
        {
            var token = await tokenResponse;

            // Twilio SMS using Magic Code hits below but will not return a token.
            if(token.NonTokenResponse != null)
            {
               var result =  await context.GetUserTokenAsync(ConnectionName, token.NonTokenResponse);
                // result is null when using magic code, the 'GetTokenDialog' calls this itself, but I was just verifying the value
                if(result != null)
                {
                    token.Token = result.Token;
                }

            }

            var client = new SimpleGraphClient(token.Token);

            var me = await client.GetMe();
            //var manager = await client.GetManager();

            await context.PostAsync($"Hello {me.DisplayName}!  You are signed in!");
        }

Reproduction Steps

  1. Run this example: https://github.com/Microsoft/BotBuilder/tree/master/CSharp/Samples/AadV2Bot
  2. Connect it to Twilio as an additional channel
  3. Send 'me' in a text message to the SMS number setup and connected to the bot framework.
  4. You will receive a long url to copy and put into your browser to authenticate
  5. After authentication you will receive a 6 digit code
  6. Enter the code back into SMS to the Twilio SMS number.
  7. The GetTokenResponse will not contain a token, just a NonTokenRepsonse which is the code that was entered in.
  8. Unable to call the MS Graph client without a token.

Expected Behavior

I would expect that the GetUserTokenAsync which accepts the connection name and magic code would return a token per this line (https://github.com/Microsoft/BotBuilder/blob/master/CSharp/Library/Microsoft.Bot.Builder/Dialogs/GetTokenDialog.cs#L116).

Actual Results

Instead of returning a token when using the magic code a GetTokenRepsonse is returned where Token is null and NonTokenResponse contains the magic code that was entered into the text message.

JasonSowers commented 6 years ago

@adamhockemeyer thanks for reporting this I have not tried Twilio myself yet, I will work on getting a reproduction set up and will report back with any information or possible workaround I find.

mkonkolowicz commented 6 years ago

I am having the same issue, but on the twilio channel only. When using web chat (via portal) this scenario works fine for me. It’s worth noting I am using the generic oauth mechanism, not aad

JasonSowers commented 6 years ago

Thanks for reporting the issue @mkonkolowicz. I am looking into this now.

JasonSowers commented 6 years ago

We have been able to reproduce this and are looking into a fix.

JasonSowers commented 6 years ago

@adamhockemeyer @mkonkolowicz We figured out that this is being caused by the '+' sign in the userId (which is the phone number) of a Twilio account. As a workaround, I was able to test successfully by just removing the '+' from the Activity.From.Id property when it first hits the bot in the messages controller. I did not extensively test this, but I was able to have success with the AADv2 sample. Here is the simple code I was using.

 if (activity.Type == ActivityTypes.Message)
            {
                if (activity.ChannelId == ChannelIds.Sms && activity.From.Id.Contains("+"))
                {
                    activity.From.Id = activity.From.Id.Substring(1, activity.From.Id.Length - 1);
                }
                await Conversation.SendAsync(activity, () => new Dialogs.RootDialog());
            }

We will look into a more permanent fix, but for now, use a similar workaround to this.

mkonkolowicz commented 6 years ago

Thanks, will test tomorrow.

mkonkolowicz commented 6 years ago

The workaround was successful. I am able to log in via the sms channel. Please update this thread once the fix for this problem is in place, and then I will be able to remove the custom logic I've implemented as suggested above.

mkonkolowicz commented 6 years ago

@JasonSowers seems the same problem is happening on the Slack channel. After sending the magic string, a null token is returned. Can you please try and repro?

mkonkolowicz commented 6 years ago

FYI: During the call GetUserTokenAsync(connectionName,activity.Text), where activity.Text is the magic string I notice the following exceptions in Debug:

Exception thrown: 'Microsoft.Rest.TransientFaultHandling.HttpRequestWithStatusException' in Microsoft.Rest.ClientRuntime.dll Exception thrown: 'Microsoft.Rest.TransientFaultHandling.HttpRequestWithStatusException' in mscorlib.dll

JasonSowers commented 6 years ago

Looking into this, thank you for reporting.

JasonSowers commented 6 years ago

@mkonkolowicz I have created a new issue to track this #4914, please follow the progress there.

stevengum commented 5 years ago

Thank you for opening an issue against the Bot Framework SDK v3. As part of the Bot Framework v4 release, we’ve moved all v3 work to a new repo located at https://github.com/microsoft/botbuilder-v3. We will continue to support and offer maintenance updates to v3 via this new repo.

From now on, https://github.com/microsoft/botbuilder repo will be used as hub, with pointers to all the different SDK languages, tools and samples repos.

As part of this restructuring, we are closing all tickets in this repo.

For defects or feature requests, please create a new issue in the new Bot Framework v3 repo found here: https://github.com/microsoft/botbuilder-v3/issues

For Azure Bot Service Channel specific defects or feature requests (e.g. Facebook, Twilio, Teams, Slack, etc.), please create a new issue in the new Bot Framework Channel repo found here: https://github.com/microsoft/botframework-services/issues

For product behavior, how-to, or general understanding questions, please use Stackoverflow. https://stackoverflow.com/search?q=bot+framework

Thank you.

The Bot Framework Team