-
Notifications
You must be signed in to change notification settings - Fork 283
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
feat: cloud adapter #3790
Merged
Merged
feat: cloud adapter #3790
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ter (#3698) * add additional BotFrameworkAuthentication classes for CloudAdapter * Add Alexa to Channels enum for #3603 * fix export, USER_AGENT and style issue * make path in Configuration.get optional, apply other PR feedback * address additional PR feedback, minor cleanup * update adaptive-runtime.Configuration to reflect contract * cleanup typing, add unit tests, refactor existing code * fix linting error in types.ts
* remove Cortana from Channels enum * clean up linted choices_* tests
* port: cloud adapter base - remove unnecessary emitter type - streaming syntax issues - introduce runtypes helpers - bot framework adapter implements http adapter interface * add created status code, remove dupe status codes Not sure how the duplicate status codes piece was introduced, seems unnecessary though. * add continue conv w/ claims to bot adapter This enables us to properly pass claims to cloud adapter without plumbing them all the way through turn context. This is handled in .NET via overloads, however we can't achieve the same thing due to backwards-compat requirements in BotAdapter. See .NET overloads: https://github.com/microsoft/botbuilder-dotnet/blob/3c335046f95deeac50fbb0b48c7c8c42051d4f6d/libraries/Microsoft.Bot.Builder/CloudAdapterBase.cs#L147-L163 * connectory factory create -> audience? * cloud adapter + associated runtime changes - properly handle streaming requests - include retries for named pipes * cloud skill handler changes - better http errors for channel service routes - clean up channel service handler tests This is essentially a port of microsoft/botbuilder-dotnet#5304 * better docs/comments all around * plumb errors through to http responses - req + res in turn context, use in adapter error handler - fix several errors that should have yielded a 401 * port builtin auth handler logic This accounts for the no-auth (Emulator) skills scenario. The logic exists in .NET here: https://github.com/microsoft/botbuilder-dotnet/blob/3c335046f95deeac50fbb0b48c7c8c42051d4f6d/libraries/Microsoft.Bot.Connector/Authentication/BuiltinBotFrameworkAuthentication.cs#L72-L87 * fix anonymous auth handling The default scenario after creating a bot using Composer is a settings file where `MicrosoftAppId` is set to `""`, aka the empty string. When Emulator sends requests without an app ID/password, the auth header is missing and thus produces a null app ID (identity.getClaims returns null). This commit properly accounts for this anonymous auth scenario. * fix runtime constructor exception The default runtime config does not set any of the configuration values related to auth. As a result, runtypes checking fails. This change coerces the runtype into a partial (to match the contructor signature) and assigns the checked value. Also, - fix boolean coercian to stringify value - use import type * small runtypes cleanup Also remove unnecessary "as" casting * remove extra call to isValidAppId This call does not exist in the .NET counterpart: https://github.com/microsoft/botbuilder-dotnet/blob/main/libraries/Microsoft.Bot.Connector/Authentication/PasswordServiceClientCredentialFactory.cs#L76 * fix dialog context error message issue * cloud adapter base -> core Also extract response error handling to integrations as base no longer knows about HTTP specifics * user token access helper Also, refactor adaptive oauth input using oauth prompt * reconcile core bot adapter changes * handle streaming version request Note that token handling also does not exist in .NET: https://github.com/microsoft/botbuilder-dotnet/blob/main/libraries/Microsoft.Bot.Builder/Streaming/StreamingRequestHandler.cs#L517 * support service client credentials factory in runtime * restore appId validation to pwServiceCredentialFactory.createCredentials() - remove "only-warn" plugin - add tests * quick PR feedback fixes * remove duplicate call to create connector client * continueConversationAsync with overloads * finishing touches - remaining PR items - remove BotLogic type - test:compat fixes Co-authored-by: stevengum <[email protected]>
stevengum
approved these changes
Jun 23, 2021
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
All code is reviewed save for #3789, leave any feedback on that PR and I will address retroactively here.
Fixes several issues, all linked inline to the commits.
#minor