Skip to content

Commit

Permalink
port:[#3906] Port Managed Identity (MSI) + Single Tenant from DotNet (#…
Browse files Browse the repository at this point in the history
…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>
  • Loading branch information
5 people authored Sep 27, 2021
1 parent eb58d15 commit edb0f90
Show file tree
Hide file tree
Showing 22 changed files with 1,067 additions and 72 deletions.
7 changes: 7 additions & 0 deletions libraries/botbuilder-core/etc/botbuilder-core.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import { O365ConnectorCard } from 'botframework-schema';
import { PasswordServiceClientCredentialFactory } from 'botframework-connector';
import { ReceiptCard } from 'botframework-schema';
import { ResourceResponse } from 'botframework-schema';
import { ServiceClientCredentials } from 'botframework-connector';
import { ServiceClientCredentialsFactory } from 'botframework-connector';
import { ServiceCollection } from 'botbuilder-dialogs-adaptive-runtime-core';
import { SignInUrlResponse } from 'botframework-schema';
Expand Down Expand Up @@ -353,6 +354,12 @@ export type ConfigurationBotFrameworkAuthenticationOptions = z.infer<typeof Type
// @public
export class ConfigurationServiceClientCredentialFactory extends PasswordServiceClientCredentialFactory {
constructor(factoryOptions?: ConfigurationServiceClientCredentialFactoryOptions);
// (undocumented)
createCredentials(microsoftAppId: string, audience: string, loginEndpoint: string, validateAuthority: boolean): Promise<ServiceClientCredentials>;
// (undocumented)
isAuthenticationDisabled(): Promise<boolean>;
// (undocumented)
isValidAppId(microsoftAppId: string): Promise<boolean>;
}

// Warning: (ae-forgotten-export) The symbol "TypedConfig" needs to be exported by the entry point index.d.ts
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,19 @@
// Licensed under the MIT License.

import * as z from 'zod';
import { ok } from 'assert';
import { Configuration } from 'botbuilder-dialogs-adaptive-runtime-core';
import { PasswordServiceClientCredentialFactory } from 'botframework-connector';
import {
JwtTokenProviderFactory,
ManagedIdentityServiceClientCredentialsFactory,
PasswordServiceClientCredentialFactory,
ServiceClientCredentials,
ServiceClientCredentialsFactory,
} from 'botframework-connector';

const MultiTenant = 'MultiTenant';
const SingleTenant = 'SingleTenant';
const UserAssignedMsi = 'UserAssignedMsi';

const TypedConfig = z
.object({
Expand All @@ -16,6 +27,16 @@ const TypedConfig = z
* The password assigned to your bot in the [Bot Framework Portal](https://dev.botframework.com/).
*/
MicrosoftAppPassword: z.string(),

/**
* The type of app id assigned to your bot in the [Bot Framework Portal](https://dev.botframework.com/).
*/
MicrosoftAppType: z.string(),

/**
* The tenant id assigned to your bot in the [Bot Framework Portal](https://dev.botframework.com/).
*/
MicrosoftAppTenantId: z.string(),
})
.partial();

Expand All @@ -28,14 +49,77 @@ export type ConfigurationServiceClientCredentialFactoryOptions = z.infer<typeof
* ServiceClientCredentialsFactory that uses a [ConfigurationServiceClientCredentialFactoryOptions](xref:botbuilder-core.ConfigurationServiceClientCredentialFactoryOptions) or a [Configuration](xref:botbuilder-dialogs-adaptive-runtime-core.Configuration) instance to build ServiceClientCredentials with an AppId and App Password.
*/
export class ConfigurationServiceClientCredentialFactory extends PasswordServiceClientCredentialFactory {
private readonly inner: ServiceClientCredentialsFactory;

/**
* Initializes a new instance of the [ConfigurationServiceClientCredentialFactory](xref:botbuilder-core.ConfigurationServiceClientCredentialFactory) class.
*
* @param factoryOptions A [ConfigurationServiceClientCredentialFactoryOptions](xref:botbuilder-core.ConfigurationServiceClientCredentialFactoryOptions) object.
*/
constructor(factoryOptions: ConfigurationServiceClientCredentialFactoryOptions = {}) {
const { MicrosoftAppId = null, MicrosoftAppPassword = null } = TypedConfig.nonstrict().parse(factoryOptions);
super(MicrosoftAppId, MicrosoftAppPassword);
const {
MicrosoftAppId = null,
MicrosoftAppPassword = null,
MicrosoftAppType = null,
MicrosoftAppTenantId = null,
} = TypedConfig.nonstrict().parse(factoryOptions);
super(MicrosoftAppId, MicrosoftAppPassword, MicrosoftAppTenantId);

const appType = MicrosoftAppType?.trim() ?? MultiTenant;

switch (appType.toLocaleLowerCase()) {
case UserAssignedMsi.toLocaleLowerCase():
ok(MicrosoftAppId?.trim(), 'MicrosoftAppId is required for MSI in configuration.');
ok(MicrosoftAppTenantId?.trim(), 'MicrosoftAppTenantId is required for MSI in configuration.');
ok(!MicrosoftAppPassword?.trim(), 'MicrosoftAppPassword must not be set for MSI in configuration.');

this.inner = new ManagedIdentityServiceClientCredentialsFactory(
MicrosoftAppId,
new JwtTokenProviderFactory()
);
break;
case SingleTenant.toLocaleLowerCase():
ok(MicrosoftAppId?.trim(), 'MicrosoftAppId is required for SingleTenant in configuration.');
ok(MicrosoftAppPassword?.trim(), 'MicrosoftAppPassword is required for SingleTenant in configuration.');
ok(MicrosoftAppTenantId?.trim(), 'MicrosoftAppTenantId is required for SingleTenant in configuration.');

this.inner = new PasswordServiceClientCredentialFactory(
MicrosoftAppId,
MicrosoftAppPassword,
MicrosoftAppTenantId
);
break;
default:
//MultiTenant
this.inner = new PasswordServiceClientCredentialFactory(MicrosoftAppId, MicrosoftAppPassword, '');
break;
}
}

/**
* @inheritdoc
*/
isValidAppId(microsoftAppId: string): Promise<boolean> {
return this.inner.isValidAppId(microsoftAppId);
}

/**
* @inheritdoc
*/
isAuthenticationDisabled(): Promise<boolean> {
return this.inner.isAuthenticationDisabled();
}

/**
* @inheritdoc
*/
createCredentials(
microsoftAppId: string,
audience: string,
loginEndpoint: string,
validateAuthority: boolean
): Promise<ServiceClientCredentials> {
return this.inner.createCredentials(microsoftAppId, audience, loginEndpoint, validateAuthority);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ describe('ConfigurationServiceClientCredentialFactory', function () {
static DefaultConfig = {
MicrosoftAppId: 'appId',
MicrosoftAppPassword: 'appPassword',
MicrosoftAppType: undefined,
MicrosoftAppTenantId: undefined,
};

constructor(config = {}) {
Expand All @@ -25,20 +27,34 @@ describe('ConfigurationServiceClientCredentialFactory', function () {
set(_path, _val) {}
}

it('constructor should work', function () {
const SingleTenantConfig = {
MicrosoftAppId: 'singleAppId',
MicrosoftAppPassword: 'singleAppPassword',
MicrosoftAppType: 'SingleTenant',
MicrosoftAppTenantId: 'singleAppTenantId',
};

const MSIConfig = {
MicrosoftAppId: 'msiAppId',
MicrosoftAppType: 'UserAssignedMsi',
MicrosoftAppTenantId: 'msiAppTenantId',
MicrosoftAppPassword: undefined,
};

it('constructor() should work', function () {
const bfAuth = new ConfigurationServiceClientCredentialFactory(TestConfiguration.DefaultConfig);
assert.strictEqual(bfAuth.appId, TestConfiguration.DefaultConfig.MicrosoftAppId);
assert.strictEqual(bfAuth.password, TestConfiguration.DefaultConfig.MicrosoftAppPassword);
});

it('createServiceClientCredentialFactoryFromConfiguration should work', function () {
it('createServiceClientCredentialFactoryFromConfiguration() should work', function () {
const config = new TestConfiguration();
const bfAuth = createServiceClientCredentialFactoryFromConfiguration(config);
assert.strictEqual(bfAuth.appId, TestConfiguration.DefaultConfig.MicrosoftAppId);
assert.strictEqual(bfAuth.password, TestConfiguration.DefaultConfig.MicrosoftAppPassword);
});

it('undefined or null configuration values should result in null values', function () {
it('createServiceClientCredentialFactoryFromConfiguration() with undefined or null configuration values should result in null values', function () {
const config = new TestConfiguration({
MicrosoftAppId: undefined,
MicrosoftAppPassword: undefined,
Expand All @@ -48,7 +64,7 @@ describe('ConfigurationServiceClientCredentialFactory', function () {
assert.strictEqual(bfAuth.password, null);
});

it('non-string values should fail', function () {
it('createServiceClientCredentialFactoryFromConfiguration() with non-string values should fail', function () {
const config = new TestConfiguration({
MicrosoftAppId: 1,
MicrosoftAppPassword: true,
Expand All @@ -63,4 +79,96 @@ describe('ConfigurationServiceClientCredentialFactory', function () {
}
);
});

it('createServiceClientCredentialFactoryFromConfiguration() with non-defined MicrosoftAppType configuration should work', function () {
const config = new TestConfiguration();

// eslint-disable-next-line @typescript-eslint/no-unused-vars
const { MicrosoftAppType, ...configuration } = config.configuration;
config.configuration = configuration;

createServiceClientCredentialFactoryFromConfiguration(config);
});

it('createServiceClientCredentialFactoryFromConfiguration() with empty-string MicrosoftAppType configuration should work', function () {
const config = new TestConfiguration({ MicrosoftAppType: '' });

createServiceClientCredentialFactoryFromConfiguration(config);
});

it('createServiceClientCredentialFactoryFromConfiguration() with casing-string MicrosoftAppType configuration should work', function () {
const config = new TestConfiguration({ ...SingleTenantConfig, MicrosoftAppType: 'singletenant' });

createServiceClientCredentialFactoryFromConfiguration(config);
});

it('createServiceClientCredentialFactoryFromConfiguration() singleTenant with config should work', function () {
const config = new TestConfiguration(SingleTenantConfig);
const bfAuth = createServiceClientCredentialFactoryFromConfiguration(config);
assert.strictEqual(bfAuth.appId, config.configuration.MicrosoftAppId);
assert.strictEqual(bfAuth.password, config.configuration.MicrosoftAppPassword);
assert.strictEqual(bfAuth.tenantId, config.configuration.MicrosoftAppTenantId);
});

it('createServiceClientCredentialFactoryFromConfiguration() singleTenant without tenantId should throw', function () {
const config = new TestConfiguration({ ...SingleTenantConfig, MicrosoftAppTenantId: undefined });

assert.throws(() => createServiceClientCredentialFactoryFromConfiguration(config), {
name: 'AssertionError',
message: 'MicrosoftAppTenantId is required for SingleTenant in configuration.',
});
});

it('createServiceClientCredentialFactoryFromConfiguration() singleTenant without appId should throw', function () {
const config = new TestConfiguration({ ...SingleTenantConfig, MicrosoftAppId: undefined });

assert.throws(() => createServiceClientCredentialFactoryFromConfiguration(config), {
name: 'AssertionError',
message: 'MicrosoftAppId is required for SingleTenant in configuration.',
});
});

it('createServiceClientCredentialFactoryFromConfiguration() singleTenant without appPassword should throw', function () {
const config = new TestConfiguration({ ...SingleTenantConfig, MicrosoftAppPassword: undefined });

assert.throws(() => createServiceClientCredentialFactoryFromConfiguration(config), {
name: 'AssertionError',
message: 'MicrosoftAppPassword is required for SingleTenant in configuration.',
});
});

it('createServiceClientCredentialFactoryFromConfiguration() manageIdentityApp with config should work', function () {
const config = new TestConfiguration(MSIConfig);
const bfAuth = createServiceClientCredentialFactoryFromConfiguration(config);

assert.strictEqual(bfAuth.appId, config.configuration.MicrosoftAppId);
assert.strictEqual(bfAuth.tenantId, config.configuration.MicrosoftAppTenantId);
});

it('createServiceClientCredentialFactoryFromConfiguration() manageIdentityApp without tenantId should throw', function () {
const config = new TestConfiguration({ ...MSIConfig, MicrosoftAppTenantId: undefined });

assert.throws(() => createServiceClientCredentialFactoryFromConfiguration(config), {
name: 'AssertionError',
message: 'MicrosoftAppTenantId is required for MSI in configuration.',
});
});

it('createServiceClientCredentialFactoryFromConfiguration() manageIdentityApp without appId should throw', function () {
const config = new TestConfiguration({ ...MSIConfig, MicrosoftAppId: undefined });

assert.throws(() => createServiceClientCredentialFactoryFromConfiguration(config), {
name: 'AssertionError',
message: 'MicrosoftAppId is required for MSI in configuration.',
});
});

it('createServiceClientCredentialFactoryFromConfiguration() manageIdentityApp with appPassword should throw', function () {
const config = new TestConfiguration({ ...MSIConfig, MicrosoftAppPassword: 'msiAppPassword' });

assert.throws(() => createServiceClientCredentialFactoryFromConfiguration(config), {
name: 'AssertionError',
message: 'MicrosoftAppPassword must not be set for MSI in configuration.',
});
});
});
21 changes: 19 additions & 2 deletions libraries/botbuilder-dialogs-adaptive-runtime/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { ServiceCollection } from 'botbuilder-dialogs-adaptive-runtime-core';

import {
AuthenticationConfiguration,
AuthenticationConstants,
allowedCallersClaimsValidator,
BotFrameworkAuthentication,
ServiceClientCredentialsFactory,
Expand Down Expand Up @@ -226,6 +227,20 @@ function addSkills(services: ServiceCollection, configuration: Configuration): v
({ storage }) => new SkillConversationIdFactory(storage)
);

// If TenantId is specified in config, add the tenant as a valid JWT token issuer for Bot to Skill conversation.
// The token issuer for MSI and single tenant scenarios will be the tenant where the bot is registered.
const validTokenIssuers: string[] = [];
const tenantId = configuration.type(['runtimeSettings', 'MicrosoftAppTenantIdKey'], z.string()) ?? '';

if (tenantId) {
// For SingleTenant/MSI auth, the JWT tokens will be issued from the bot's home tenant.
// So, these issuers need to be added to the list of valid token issuers for authenticating activity requests.
validTokenIssuers.push(`${AuthenticationConstants.ValidTokenIssuerUrlTemplateV1}${tenantId}/`);
validTokenIssuers.push(`${AuthenticationConstants.ValidTokenIssuerUrlTemplateV2}${tenantId}/v2.0`);
validTokenIssuers.push(`${AuthenticationConstants.ValidGovernmentTokenIssuerUrlTemplateV1}${tenantId}/`);
validTokenIssuers.push(`${AuthenticationConstants.ValidGovernmentTokenIssuerUrlTemplateV2}${tenantId}/v2.0`);
}

services.addFactory<AuthenticationConfiguration>('authenticationConfiguration', () => {
const allowedCallers =
configuration.type(['runtimeSettings', 'skills', 'allowedCallers'], z.array(z.string())) ?? [];
Expand All @@ -248,14 +263,16 @@ function addSkills(services: ServiceCollection, configuration: Configuration): v
// runtimeSettings.sills are ignored
return new AuthenticationConfiguration(
undefined,
allowedCallersClaimsValidator(skills.map((skill) => skill.msAppId))
allowedCallersClaimsValidator(skills.map((skill) => skill.msAppId)),
validTokenIssuers
);
} else {
// If the config entry for runtimeSettings.skills.allowedCallers contains entries, then we are a skill and
// we validate caller against this list
return new AuthenticationConfiguration(
undefined,
allowedCallers.length ? allowedCallersClaimsValidator(allowedCallers) : undefined
allowedCallers.length ? allowedCallersClaimsValidator(allowedCallers) : undefined,
validTokenIssuers
);
}
});
Expand Down
3 changes: 2 additions & 1 deletion libraries/botframework-connector/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
}
},
"dependencies": {
"@azure/identity": "2.0.0-beta.6",
"@azure/ms-rest-js": "1.9.1",
"@types/jsonwebtoken": "7.2.8",
"@types/node": "^10.17.27",
Expand Down Expand Up @@ -54,7 +55,7 @@
"build:browserify:run": "browserify -s BFC --debug lib/browser.js | exorcist lib/browser.js.map | sponge lib/browser.js",
"build:downlevel-dts": "downlevel-dts lib _ts3.4/lib --checksum",
"clean": "rimraf _ts3.4 lib tsconfig.tsbuildinfo",
"depcheck": "depcheck --config ../../.depcheckrc --ignores azure",
"depcheck": "depcheck --config ../../.depcheckrc --ignores azure,sinon",
"lint": "eslint . --ext .js,.ts",
"postbuild": "npm-run-all -p build:browserify build:downlevel-dts",
"test": "yarn build && yarn test:mocha",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ export class AuthenticationConfiguration {
* @param {string[]} requiredEndorsements An array of JWT endorsements.
* @param {(claims: Claim[]) => Promise<void>} validateClaims Function that validates a list of Claims
* and should throw an exception if the validation fails.
* @param {string[]} validTokenIssuers An array of valid JWT token issuers.
*/
constructor(public requiredEndorsements: string[] = [], public validateClaims?: ValidateClaims) {}
constructor(
public requiredEndorsements: string[] = [],
public validateClaims?: ValidateClaims,
public validTokenIssuers?: string[]
) {}
}
Loading

0 comments on commit edb0f90

Please sign in to comment.