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

port: Managed Identity (MSI) + Single Tenant support for Bot apps (#5829) #3906

Closed
github-actions bot opened this issue Aug 20, 2021 · 1 comment · Fixed by #3923
Closed

port: Managed Identity (MSI) + Single Tenant support for Bot apps (#5829) #3906

github-actions bot opened this issue Aug 20, 2021 · 1 comment · Fixed by #3923
Assignees
Labels
ExemptFromDailyDRIReport Use this label to exclude the issue from the DRI report. P0 Must Fix. Release-blocker parity The issue describes a gap in parity between two or more platforms.
Milestone

Comments

@github-actions
Copy link

The changes in Managed Identity (MSI) + Single Tenant support for Bot apps (#5829) may need to be ported to maintain parity with microsoft/botbuilder-dotnet.

Fixes #5830

Description

  • Enable Azure's Managed Identity (MSI) for bot apps, so that bot authors don't have to manage app secrets.
  • Support for hosting bots as single tenant apps, which enables acquiring AAD access tokens from the bot's host tenant, as opposed to, botframework.com tenant.

Below are the 3 ways a bot app can now be hosted:

1. Multi-Tenant (Default):

{
  "MicrosoftAppId": "",
  "MicrosoftAppPassword": ""
}

2. Single Tenant:

{
  "MicrosoftAppType": "SingleTenant",
  "MicrosoftAppId": "",
  "MicrosoftAppPassword": "",
  "MicrosoftAppTenantId": ""
}

3. Managed Identity (MSI):

{
  "MicrosoftAppType": "UserAssignedMSI",
  "MicrosoftAppId": "",
  "MicrosoftAppTenantId": ""
}

Specific Changes

  • Added classes for Managed Identity credentials management and token acquisition.
  • Updated ConfigurationServiceClientCredentialFactory to instantiate the appropriate credential mechanism (MultiTenant/SingleTenant/MSI) based on appsettings.
  • Added the bot's host tenant as a valid JWT token issuer since the tokens will be issued from the host tenant when using SingleTenant/MSI hosting model. This was done by adding a property to the AuthenticationConfiguration class which would be used in the skills scenarios.
  • Added an overload in PasswordServiceClientCredentialsFactory to accept TenantId which can be used to acquire token using the bot's host tenant for SingleTenant scenario.

Testing

Following scenarios were verified:

1. Bot to Channel scenarios:

  • MultiTenant
  • SingleTenant
  • MSI

2. Bot to bot (skills) scenarios:

MultiTenant SkillSingleTenant SkillMSI Skill
MultiTenant HostY--
SingleTenant Host-YY
MSI Host-YY
NOTE: When one of the bots is MultiTenant, the other cannot be SingleTenant/MSI since the token acquisition by the MultiTenant bot will fail due to not being able to find the SingleTenant/MSI bot resource registered in the `botframework.com` tenant. So, those scenarios are not supported.

Please review and, if necessary, port the changes.

@github-actions github-actions bot added ExemptFromDailyDRIReport Use this label to exclude the issue from the DRI report. needs-triage The issue has just been created and it has not been reviewed by the team. parity The issue describes a gap in parity between two or more platforms. labels Aug 20, 2021
@mrivera-ms mrivera-ms added P0 Must Fix. Release-blocker and removed needs-triage The issue has just been created and it has not been reviewed by the team. labels Aug 25, 2021
@mrivera-ms mrivera-ms added this to the R15 milestone Aug 25, 2021
@stevengum
Copy link
Member

Note that for the SingleTenant Skill-MSI Skill scenarios to work, the bots must be in the same tenant.

stevengum pushed a commit that referenced this issue Sep 27, 2021
…3923)

* Port MSI and SingleTenant support

* Add unit tests for botframework-connector

* Restore yarn.lock

* Test nohoist for azure/identity

* Fix dependencies issues

* Connect ManagedIdentityAuthenticator.getToken

* Add remaining tests and fix the already added

* Update yarn.lock

* Fixing depcheck errors in PR

* Fixing depcheck, adjusting versions and ignores

* Fix parity port with DotNet causing SingleTenant to fail at authentication

* Apppliying feedback

* Applying fixes for tests, lint and zod related feedback

* Fixing asserts import and extra parameter

* Fix tests failing due to wrong link and fixing case-wise comparison failures

* Fixing lint

* Add ignore-casing to MicrosoftAppType based on DotNet code and add unit tests to validate

* Improve issuer array assignation

* fix: export ms-rest-js type and remove added dep

* Improve issuers, assertions, unit tests and many other small changes

* fix: skillValidation validTokenIssuers

* Update @azure/identity to 2.0.0-beta.6

* Change logical operator for nullable operator

* Improve SingleTenant and MSI implementation from feedback

* fix: IJwtTokenProviderFactory

* fix: reintroduced typo

Co-authored-by: CeciliaAvila <cecilia.avila@southworks.com>
Co-authored-by: Martin Luccanera <18757147+MartinLuccanera@users.noreply.github.com>
Co-authored-by: Cecilia Avila <44245136+ceciliaavila@users.noreply.github.com>
Co-authored-by: Josh Gummersall <jgummersall@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ExemptFromDailyDRIReport Use this label to exclude the issue from the DRI report. P0 Must Fix. Release-blocker parity The issue describes a gap in parity between two or more platforms.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants