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 for OAuthCards to WebChat #954

Merged
merged 4 commits into from
Apr 26, 2018
Merged

Conversation

Jeffders
Copy link
Member

Adds support for OAuthCards to WebChat:

  1. Add support for rendering OAuthCard attachments in a similar way to SignInCards, however, using Submit actions instead of just opening the URL because of Timestamp #2
  2. Add support for calling getSessionId from IBotConnection just prior to opening the OAuthCard link. When used with the DirectLine service, this method sets a security token in the browser to secure WebChat during OAuthCard sign-ins; basically this helps ensure the webchat browser where the OAuthCard was received is the same one where the link was traversed.

@compulim
Copy link
Contributor

Can you add some tests for your feature?

You can start by:

  • Modify test/commands_map.ts
    • For quick turnaround time, temporary modify the very last lines var testOnly to include your new tests only
    • Add new commands
      • server code is what the server should send when the command is received
      • do code should use Nightmare to do extra client UI stuff
      • client code run inside Chrome (sandboxed), to assert on the UI
      • You can learn more from the command hostconfig, it's the simplest amongst them all
  • npm run build-test
  • npm run test

If you want to run your command manually:

src/Chat.tsx Outdated
window.open(text);
break;
case "signin":
let loginWindow = window.open();
if(botConnection.getSessionId) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space between if and ( please :-)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem.

@compulim compulim merged commit 617de0a into microsoft:master Apr 26, 2018
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

Successfully merging this pull request may close these issues.

4 participants