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

AADv2 Magic Code Not Returning Token From GetUserTokenAsync #4880

Closed
adamhockemeyer opened this issue Jul 19, 2018 · 12 comments
Closed

AADv2 Magic Code Not Returning Token From GetUserTokenAsync #4880

adamhockemeyer opened this issue Jul 19, 2018 · 12 comments
Assignees

Comments

@adamhockemeyer
Copy link

Bot Info

  • SDK Platform: .Net
  • SDK Version: 3.15.3
  • Active Channels: Web Chat, Twilio
  • Deployment Environment: Azure App Service, Local Development

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
Copy link
Contributor

@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
Copy link

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
Copy link
Contributor

JasonSowers commented Jul 20, 2018

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

@JasonSowers
Copy link
Contributor

JasonSowers commented Jul 23, 2018

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

@JasonSowers
Copy link
Contributor

JasonSowers commented Jul 23, 2018

@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
Copy link

Thanks, will test tomorrow.

@mkonkolowicz
Copy link

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
Copy link

@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
Copy link

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
Copy link
Contributor

Looking into this, thank you for reporting.

@JasonSowers
Copy link
Contributor

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

@stevengum
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants