Token exchange process starts but rise an InternalServerError exception.

[GitDruid]

The token exchange process starts but rise an InternalServerError exception (see attachment).

Exception on token exchange.txt

Originally posted by @GitDruid in #3579 (comment)

[tdurnford]

@Batta32 Can you take a look at this?

[Batta32]

Sure @tdurnford! We will be reviewing this issue and the PR #3655. As soon as we have any update, we will back here 😊.

[VictorGrycuk]

Hi @GitDruid, we were able to reproduce the InternalServerError exception.

So far the changes made on the PR #3655 seems to be right regarding the botbuilder SDK and following the sample sso-with-skills, but we want to validate that it does fix the error.

As soon as we have any update, we will back to you 😊.

InternalServerError exception

[Batta32]

@tdurnford & @GitDruid - after reviewing the changes of PR #3655, we confirmed that the changes are correct and follows the signature of the ExchangeTokenAsync of IExtendedUserTokenProvider for botbuilder-dotnet and the parameters had to be inverted. Also, in botbuilder-js the signature is the same and the sample sso-with-skills contains the same changes.

Last but not least, we did the following things apart from this Pull Request to have a consistency between TypeScript and C#:

• PR [C#][Virtual Assistant Sample] Fixing inverted parameters in TokenExchangeSkillHandler.cs #3667: which applies these changes to the C# Virtual Assistant Sample
• Commit of PR [TypeScript][Virtual Assistant] Implement changes in Virtual Assistant to 1.0 release #3584: which applies these changes in TypeScript for a parity with C#

[Batta32]

Hi @GitDruid - we are finalizing a plan to release new packages that will include the PR #3655 and PR #3667. As soon as we have any update we will back to you 😊.

[stevengum]

@Batta32, @ryanlengel, @peterinnesmsft any updates on this issue or the associated PRs? #3655 #3667

[Batta32]

Hi @stevengum, we created the PR #3686 that duplicates the changes of the PR #3655. As soon as it's merged, this issue will be closed.

Last but not least, the PR #3667 was already merged.