Skip to content

Commit 60b3f98

Browse files
sw-joelmutsw-joelmut
authored
fix: [#4449] CloudAdapter always builds Connector with MicrosoftAppCredentials (never CertificateAppCredentials) -- certificate auth flow broken (#4457)
* Add support for certificates * Fix typos
1 parent ebb0e7e commit 60b3f98

6 files changed

+220
-7
lines changed

‎libraries/botbuilder-core/src/configurationBotFrameworkAuthentication.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,16 @@ const TypedOptions = z
8888
* A value for the CallerId.
8989
*/
9090
CallerId: z.string(),
91+
92+
/**
93+
* Certificate thumbprint to authenticate the appId against AAD.
94+
*/
95+
[AuthenticationConstants.CertificateThumbprint]: z.string(),
96+
97+
/**
98+
* Certificate key to authenticate the appId against AAD.
99+
*/
100+
[AuthenticationConstants.CertificatePrivateKey]: z.string(),
91101
})
92102
.partial();
93103

‎libraries/botbuilder-core/src/configurationServiceClientCredentialFactory.ts

Lines changed: 59 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import * as z from 'zod';
55
import { ok } from 'assert';
66
import { Configuration } from 'botbuilder-dialogs-adaptive-runtime-core';
77
import {
8+
AuthenticationConstants,
9+
CertificateServiceClientCredentialsFactory,
810
JwtTokenProviderFactory,
911
ManagedIdentityServiceClientCredentialsFactory,
1012
PasswordServiceClientCredentialFactory,
@@ -37,6 +39,16 @@ const TypedConfig = z
3739
* The tenant id assigned to your bot in the [Bot Framework Portal](https://dev.botframework.com/).
3840
*/
3941
MicrosoftAppTenantId: z.string(),
42+
43+
/**
44+
* Certificate thumbprint to authenticate the appId against AAD.
45+
*/
46+
[AuthenticationConstants.CertificateThumbprint]: z.string(),
47+
48+
/**
49+
* Certificate key to authenticate the appId against AAD.
50+
*/
51+
[AuthenticationConstants.CertificatePrivateKey]: z.string(),
4052
})
4153
.partial();
4254

@@ -62,10 +74,24 @@ export class ConfigurationServiceClientCredentialFactory extends PasswordService
6274
MicrosoftAppPassword = null,
6375
MicrosoftAppType = null,
6476
MicrosoftAppTenantId = null,
77+
[AuthenticationConstants.CertificateThumbprint]: CertificateThumbprint = null,
78+
[AuthenticationConstants.CertificatePrivateKey]: CertificatePrivateKey = null,
6579
} = TypedConfig.nonstrict().parse(factoryOptions);
6680
super(MicrosoftAppId, MicrosoftAppPassword, MicrosoftAppTenantId);
6781

6882
const appType = MicrosoftAppType?.trim() ?? MultiTenant;
83+
const withCertificate = CertificateThumbprint || CertificatePrivateKey;
84+
85+
if (withCertificate) {
86+
ok(
87+
CertificateThumbprint?.trim(),
88+
'CertificateThumbprint is required when using a Certificate in configuration.'
89+
);
90+
ok(
91+
CertificatePrivateKey?.trim(),
92+
'CertificatePrivateKey is required when using a Certificate in configuration.'
93+
);
94+
}
6995

7096
switch (appType.toLocaleLowerCase()) {
7197
case UserAssignedMsi.toLocaleLowerCase():
@@ -80,18 +106,44 @@ export class ConfigurationServiceClientCredentialFactory extends PasswordService
80106
break;
81107
case SingleTenant.toLocaleLowerCase():
82108
ok(MicrosoftAppId?.trim(), 'MicrosoftAppId is required for SingleTenant in configuration.');
83-
ok(MicrosoftAppPassword?.trim(), 'MicrosoftAppPassword is required for SingleTenant in configuration.');
84109
ok(MicrosoftAppTenantId?.trim(), 'MicrosoftAppTenantId is required for SingleTenant in configuration.');
85110

86-
this.inner = new PasswordServiceClientCredentialFactory(
87-
MicrosoftAppId,
88-
MicrosoftAppPassword,
89-
MicrosoftAppTenantId
90-
);
111+
if (withCertificate) {
112+
this.inner = new CertificateServiceClientCredentialsFactory(
113+
MicrosoftAppId,
114+
CertificateThumbprint,
115+
CertificatePrivateKey,
116+
MicrosoftAppTenantId
117+
);
118+
} else {
119+
ok(
120+
MicrosoftAppPassword?.trim(),
121+
'MicrosoftAppPassword is required for SingleTenant in configuration.'
122+
);
123+
124+
this.inner = new PasswordServiceClientCredentialFactory(
125+
MicrosoftAppId,
126+
MicrosoftAppPassword,
127+
MicrosoftAppTenantId
128+
);
129+
}
91130
break;
92131
default:
93132
//MultiTenant
94-
this.inner = new PasswordServiceClientCredentialFactory(MicrosoftAppId, MicrosoftAppPassword, '');
133+
if (withCertificate) {
134+
ok(
135+
MicrosoftAppId?.trim(),
136+
'MicrosoftAppId is required for MultiTenant when using a Certificate in configuration.'
137+
);
138+
139+
this.inner = new CertificateServiceClientCredentialsFactory(
140+
MicrosoftAppId,
141+
CertificateThumbprint,
142+
CertificatePrivateKey
143+
);
144+
} else {
145+
this.inner = new PasswordServiceClientCredentialFactory(MicrosoftAppId, MicrosoftAppPassword, '');
146+
}
95147
break;
96148
}
97149
}

‎libraries/botbuilder-core/tests/configurationServiceClientCredentialFactory.test.js

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ const {
66
ConfigurationServiceClientCredentialFactory,
77
createServiceClientCredentialFactoryFromConfiguration,
88
} = require('../');
9+
const { CertificateAppCredentials } = require('botframework-connector');
910

1011
describe('ConfigurationServiceClientCredentialFactory', function () {
1112
class TestConfiguration {
@@ -96,6 +97,52 @@ describe('ConfigurationServiceClientCredentialFactory', function () {
9697
createServiceClientCredentialFactoryFromConfiguration(config);
9798
});
9899

100+
it('createServiceClientCredentialFactoryFromConfiguration() with certificate', async function () {
101+
const config = new TestConfiguration({
102+
CertificateThumbprint: 'CertificateThumbprint',
103+
CertificatePrivateKey: 'CertificatePrivateKey',
104+
});
105+
106+
const serviceClient = createServiceClientCredentialFactoryFromConfiguration(config);
107+
const credentialClient = await serviceClient.createCredentials(TestConfiguration.DefaultConfig.MicrosoftAppId);
108+
assert(credentialClient instanceof CertificateAppCredentials);
109+
});
110+
111+
it('createServiceClientCredentialFactoryFromConfiguration() with certificate and no MicrosoftAppId', async function () {
112+
const config = new TestConfiguration({
113+
MicrosoftAppId: undefined,
114+
CertificateThumbprint: 'CertificateThumbprint',
115+
CertificatePrivateKey: 'CertificatePrivateKey',
116+
});
117+
118+
assert.throws(() => createServiceClientCredentialFactoryFromConfiguration(config), {
119+
name: 'AssertionError',
120+
message: 'MicrosoftAppId is required for MultiTenant when using a Certificate in configuration.',
121+
});
122+
});
123+
124+
it('createServiceClientCredentialFactoryFromConfiguration() with certificate and no CertificatePrivateKey', async function () {
125+
const config = new TestConfiguration({
126+
CertificateThumbprint: 'CertificateThumbprint',
127+
});
128+
129+
assert.throws(() => createServiceClientCredentialFactoryFromConfiguration(config), {
130+
name: 'AssertionError',
131+
message: 'CertificatePrivateKey is required when using a Certificate in configuration.',
132+
});
133+
});
134+
135+
it('createServiceClientCredentialFactoryFromConfiguration() with certificate and no CertificateThumbprint', async function () {
136+
const config = new TestConfiguration({
137+
CertificatePrivateKey: 'CertificatePrivateKey',
138+
});
139+
140+
assert.throws(() => createServiceClientCredentialFactoryFromConfiguration(config), {
141+
name: 'AssertionError',
142+
message: 'CertificateThumbprint is required when using a Certificate in configuration.',
143+
});
144+
});
145+
99146
it('createServiceClientCredentialFactoryFromConfiguration() with casing-string MicrosoftAppType configuration should work', function () {
100147
const config = new TestConfiguration({ ...SingleTenantConfig, MicrosoftAppType: 'singletenant' });
101148

@@ -137,6 +184,18 @@ describe('ConfigurationServiceClientCredentialFactory', function () {
137184
});
138185
});
139186

187+
it('createServiceClientCredentialFactoryFromConfiguration() singleTenant with certificate', async function () {
188+
const config = new TestConfiguration({
189+
...SingleTenantConfig,
190+
CertificateThumbprint: 'CertificateThumbprint',
191+
CertificatePrivateKey: 'CertificatePrivateKey',
192+
});
193+
194+
const serviceClient = createServiceClientCredentialFactoryFromConfiguration(config);
195+
const credentialClient = await serviceClient.createCredentials(SingleTenantConfig.MicrosoftAppId);
196+
assert(credentialClient instanceof CertificateAppCredentials);
197+
});
198+
140199
it('createServiceClientCredentialFactoryFromConfiguration() manageIdentityApp with config should work', function () {
141200
const config = new TestConfiguration(MSIConfig);
142201
const bfAuth = createServiceClientCredentialFactoryFromConfiguration(config);

‎libraries/botframework-connector/src/auth/authenticationConstants.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,4 +181,14 @@ export namespace AuthenticationConstants {
181181
* Indicates that bot identity is anonymous (no appId and password were provided).
182182
*/
183183
export const AnonymousAuthType = 'anonymous';
184+
185+
/**
186+
* Certificate thumbprint to authenticate the appId against AAD.
187+
*/
188+
export const CertificateThumbprint = 'CertificateThumbprint';
189+
190+
/**
191+
* Certificate key to authenticate the appId against AAD.
192+
*/
193+
export const CertificatePrivateKey = 'CertificatePrivateKey';
184194
}

‎libraries/botframework-connector/src/auth/certificateServiceClientCredentialsFactory.ts

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
/**
2+
* @module botframework-connector
3+
*/
4+
/**
5+
* Copyright (c) Microsoft Corporation. All rights reserved.
6+
* Licensed under the MIT License.
7+
*/
8+
9+
import type { ServiceClientCredentials } from '@azure/ms-rest-js';
10+
import { ServiceClientCredentialsFactory } from './serviceClientCredentialsFactory';
11+
import { ok } from 'assert';
12+
import { CertificateAppCredentials } from './certificateAppCredentials';
13+
14+
/**
15+
* A Certificate implementation of the [ServiceClientCredentialsFactory](xref:botframework-connector.ServiceClientCredentialsFactory) abstract class.
16+
*/
17+
export class CertificateServiceClientCredentialsFactory extends ServiceClientCredentialsFactory {
18+
private readonly appId: string;
19+
private readonly certificateThumbprint: string;
20+
private readonly certificatePrivateKey: string;
21+
private readonly tenantId: string | null;
22+
23+
/**
24+
* Initializes a new instance of the CertificateServiceClientCredentialsFactory class.
25+
*
26+
* @param appId Microsoft application Id related to the certificate.
27+
* @param certificateThumbprint A hex encoded thumbprint of the certificate.
28+
* @param certificatePrivateKey A PEM encoded certificate private key.
29+
* @param tenantId Optional. The oauth token tenant.
30+
*/
31+
constructor(appId: string, certificateThumbprint: string, certificatePrivateKey: string, tenantId?: string) {
32+
super();
33+
ok(appId?.trim(), 'CertificateServiceClientCredentialsFactory.constructor(): missing appId.');
34+
ok(
35+
certificateThumbprint?.trim(),
36+
'CertificateServiceClientCredentialsFactory.constructor(): missing certificateThumbprint.'
37+
);
38+
ok(
39+
certificatePrivateKey?.trim(),
40+
'CertificateServiceClientCredentialsFactory.constructor(): missing certificatePrivateKey.'
41+
);
42+
43+
this.appId = appId;
44+
this.certificateThumbprint = certificateThumbprint;
45+
this.certificatePrivateKey = certificatePrivateKey;
46+
this.tenantId = tenantId;
47+
}
48+
49+
/**
50+
* @inheritdoc
51+
*/
52+
async isValidAppId(appId: string): Promise<boolean> {
53+
return appId === this.appId;
54+
}
55+
56+
/**
57+
* @inheritdoc
58+
*/
59+
async isAuthenticationDisabled(): Promise<boolean> {
60+
// Auth is always enabled for Certificate.
61+
return;
62+
}
63+
64+
/**
65+
* @inheritdoc
66+
*/
67+
async createCredentials(appId: string, audience: string): Promise<ServiceClientCredentials> {
68+
ok(
69+
await this.isValidAppId(appId),
70+
'CertificateServiceClientCredentialsFactory.createCredentials(): Invalid Managed ID.'
71+
);
72+
73+
return new CertificateAppCredentials(
74+
this.appId,
75+
this.certificateThumbprint,
76+
this.certificatePrivateKey,
77+
this.tenantId,
78+
audience
79+
);
80+
}
81+
}

‎libraries/botframework-connector/src/auth/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ export * from './authenticateRequestResult';
1515
export * from './botFrameworkAuthentication';
1616
export * from './botFrameworkAuthenticationFactory';
1717
export * from './certificateAppCredentials';
18+
export * from './certificateServiceClientCredentialsFactory';
1819
export * from './channelValidation';
1920
export * from './claimsIdentity';
2021
export * from './connectorFactory';

0 commit comments

Comments
 (0)