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 ase channel validation #6712

Closed
wants to merge 1 commit into from
Closed

Add ase channel validation #6712

wants to merge 1 commit into from

Conversation

fangyangci
Copy link
Contributor

@fangyangci fangyangci commented Dec 11, 2023

Description

To resolve the connection issue between DL_ASE and bot, we built DL_ASE v2.0. In v2.0, we used WebSocket/HTTPS instead of named pipe to connect between DL_ASE and bot.

Therefore, we need to add a special header validation for AseChannel.

Specific Changes

  1. Add AseChannel channelId check in ConfigurationBotFrameworkAuthentication.
  2. Fix a potential bug with USGov OAuthEndpoint while using SingleTenant.

Testing

Adding unit tests here is not straightforward, similar to EmulatorValidation and ChannelValidation.
I tested this feature locally and also using end-to-end tests in DL_ASE 2.0 within SingleTenant/MultiTenant/UMSI in Public/US Gov Cloud.

@fangyangci fangyangci requested a review from a team as a code owner December 11, 2023 07:25
@Danieladu
Copy link
Contributor

Please decouple the OAuthEndpoint fix and new ASE channel support. I think they are two things, right?

@tracyboehrer tracyboehrer added the Automation: Parity with js The PR needs to be ported to JS label Dec 11, 2023
@Danieladu
Copy link
Contributor

Danieladu commented Dec 12, 2023

It is better to add the unit test or functional test.
Such as the tests in JwtTokenExtractorTests, BotFrameworkAdapterTests, JwtTokenValidationTests

@fangyangci
Copy link
Contributor Author

Please decouple the OAuthEndpoint fix and new ASE channel support. I think they are two things, right?

I split the USGov SingleTenant bug to a new pr.
#6714

@fangyangci fangyangci closed this Dec 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Automation: Parity with js The PR needs to be ported to JS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants