From 28b1f4c82215c8898582d4c90e4aff62028bc223 Mon Sep 17 00:00:00 2001 From: TJ Date: Fri, 27 Jan 2023 07:48:00 -0700 Subject: [PATCH 1/5] fix: Update azure provisioning to use arm token for creating app registration --- .../server/src/controllers/provision.ts | 3 +- Composer/packages/types/src/publish.ts | 4 ++- extensions/azurePublish/src/node/index.ts | 33 ++++++++++++++----- extensions/azurePublish/src/node/provision.ts | 11 +++++-- 4 files changed, 38 insertions(+), 13 deletions(-) diff --git a/Composer/packages/server/src/controllers/provision.ts b/Composer/packages/server/src/controllers/provision.ts index 3e9cbd777b..14688a3aa9 100644 --- a/Composer/packages/server/src/controllers/provision.ts +++ b/Composer/packages/server/src/controllers/provision.ts @@ -49,7 +49,8 @@ export const ProvisionController = { { ...req.body }, currentProject, user, - authService.getAccessToken.bind(authService) + authService.getAccessToken.bind(authService), + authService.getArmAccessToken.bind(authService) ); // set status and return value as json res.status(result.status).json(result); diff --git a/Composer/packages/types/src/publish.ts b/Composer/packages/types/src/publish.ts index a75a7b873f..caae493ee0 100644 --- a/Composer/packages/types/src/publish.ts +++ b/Composer/packages/types/src/publish.ts @@ -59,6 +59,7 @@ export type PullResponse = { }; type GetAccessToken = (params: AuthParameters) => Promise; +type GetArmAccessToken = (tenantId: string) => Promise; // TODO: Add types for project, metadata export type PublishPlugin = { @@ -101,7 +102,8 @@ export type PublishPlugin = { config: Config, project: IBotProject, user?: UserIdentity, - getAccessToken?: GetAccessToken + getAccessToken?: GetAccessToken, + getArmAccessToken?: GetArmAccessToken ) => Promise; getProvisionStatus?: ( target: string, diff --git a/extensions/azurePublish/src/node/index.ts b/extensions/azurePublish/src/node/index.ts index b3bf71a1c9..1a1027f235 100644 --- a/extensions/azurePublish/src/node/index.ts +++ b/extensions/azurePublish/src/node/index.ts @@ -268,7 +268,13 @@ export default async (composer: IExtensionRegistration): Promise => { /*******************************************************************************************************************************/ /* These methods provision resources to azure async */ /*******************************************************************************************************************************/ - asyncProvision = async (jobId: string, config: ProvisionConfig, project: IBotProject, user): Promise => { + asyncProvision = async ( + jobId: string, + config: ProvisionConfig, + project: IBotProject, + user, + getArmAccessToken: (tenantId: string) => Promise + ): Promise => { const { runtimeLanguage } = parseRuntimeKey(project.settings?.runtime?.key); // map runtime language/platform to worker runtime @@ -286,13 +292,16 @@ export default async (composer: IExtensionRegistration): Promise => { const { name } = provisionConfig; // Create the object responsible for actually taking the provision actions. - const azureProvisioner = new BotProjectProvision({ - ...provisionConfig, - logger: (msg: any) => { - this.logger(msg); - BackgroundProcessManager.updateProcess(jobId, 202, msg.message); + const azureProvisioner = new BotProjectProvision( + { + ...provisionConfig, + logger: (msg: any) => { + this.logger(msg); + BackgroundProcessManager.updateProcess(jobId, 202, msg.message); + }, }, - }); + getArmAccessToken + ); // perform the provision using azureProvisioner.create. // this will start the process, then return. @@ -512,9 +521,15 @@ export default async (composer: IExtensionRegistration): Promise => { /************************************************************************************************** * plugin methods for provision *************************************************************************************************/ - provision = async (config: any, project: IBotProject, user, getAccessToken): Promise => { + provision = async ( + config: any, + project: IBotProject, + user, + getAccessToken, + getArmAccessToken + ): Promise => { const jobId = BackgroundProcessManager.startProcess(202, project.id, config.name, 'Creating Azure resources...'); - this.asyncProvision(jobId, config, project, user); + this.asyncProvision(jobId, config, project, user, getArmAccessToken); return BackgroundProcessManager.getStatus(jobId); }; diff --git a/extensions/azurePublish/src/node/provision.ts b/extensions/azurePublish/src/node/provision.ts index 0b98396df5..cf7b32e8e5 100644 --- a/extensions/azurePublish/src/node/provision.ts +++ b/extensions/azurePublish/src/node/provision.ts @@ -48,7 +48,7 @@ export class BotProjectProvision { // Will be assigned by create or deploy private tenantId = ''; - constructor(config: ProvisionConfig) { + constructor(config: ProvisionConfig, private getArmAccessToken?: (tenantId: string) => Promise) { this.subscriptionId = config.subscription; this.logger = config.logger; this.accessToken = config.accessToken; @@ -97,8 +97,15 @@ export class BotProjectProvision { message: '> Creating App Registration ...', }); + const armAccessToken = await this.getArmAccessToken?.(this.tenantId); + + this.logger({ + status: BotProjectDeployLoggerType.PROVISION_INFO, + message: `> Retrieved arm access token ...: ${armAccessToken}`, + }); + const appCreateOptions: AxiosRequestConfig = { - headers: { Authorization: `Bearer ${this.graphToken}` }, + headers: { Authorization: `Bearer ${armAccessToken ?? this.graphToken}` }, }; // This call if successful returns an object in the form From 749ff6564ab26c30596ff9d677f24a9e7c85cd7b Mon Sep 17 00:00:00 2001 From: TJ Date: Thu, 9 Mar 2023 09:14:03 -0700 Subject: [PATCH 2/5] maybe --- .../packages/server/src/controllers/provision.ts | 3 +-- Composer/packages/types/src/publish.ts | 2 -- extensions/azurePublish/src/node/index.ts | 15 +++++---------- extensions/azurePublish/src/node/provision.ts | 13 +++++++++---- 4 files changed, 15 insertions(+), 18 deletions(-) diff --git a/Composer/packages/server/src/controllers/provision.ts b/Composer/packages/server/src/controllers/provision.ts index 14688a3aa9..3e9cbd777b 100644 --- a/Composer/packages/server/src/controllers/provision.ts +++ b/Composer/packages/server/src/controllers/provision.ts @@ -49,8 +49,7 @@ export const ProvisionController = { { ...req.body }, currentProject, user, - authService.getAccessToken.bind(authService), - authService.getArmAccessToken.bind(authService) + authService.getAccessToken.bind(authService) ); // set status and return value as json res.status(result.status).json(result); diff --git a/Composer/packages/types/src/publish.ts b/Composer/packages/types/src/publish.ts index caae493ee0..60ce68974f 100644 --- a/Composer/packages/types/src/publish.ts +++ b/Composer/packages/types/src/publish.ts @@ -59,7 +59,6 @@ export type PullResponse = { }; type GetAccessToken = (params: AuthParameters) => Promise; -type GetArmAccessToken = (tenantId: string) => Promise; // TODO: Add types for project, metadata export type PublishPlugin = { @@ -103,7 +102,6 @@ export type PublishPlugin = { project: IBotProject, user?: UserIdentity, getAccessToken?: GetAccessToken, - getArmAccessToken?: GetArmAccessToken ) => Promise; getProvisionStatus?: ( target: string, diff --git a/extensions/azurePublish/src/node/index.ts b/extensions/azurePublish/src/node/index.ts index 1a1027f235..f6867d81e4 100644 --- a/extensions/azurePublish/src/node/index.ts +++ b/extensions/azurePublish/src/node/index.ts @@ -16,6 +16,7 @@ import { PublishResult, SDKKinds, PublishProfile, + AuthParameters, } from '@botframework-composer/types'; import { parseRuntimeKey, applyPublishingProfileToSettings } from '@bfc/shared'; import { indexer } from '@bfc/indexers'; @@ -273,7 +274,7 @@ export default async (composer: IExtensionRegistration): Promise => { config: ProvisionConfig, project: IBotProject, user, - getArmAccessToken: (tenantId: string) => Promise + geAccessToken: (params: AuthParameters) => Promise ): Promise => { const { runtimeLanguage } = parseRuntimeKey(project.settings?.runtime?.key); @@ -300,7 +301,7 @@ export default async (composer: IExtensionRegistration): Promise => { BackgroundProcessManager.updateProcess(jobId, 202, msg.message); }, }, - getArmAccessToken + geAccessToken ); // perform the provision using azureProvisioner.create. @@ -521,15 +522,9 @@ export default async (composer: IExtensionRegistration): Promise => { /************************************************************************************************** * plugin methods for provision *************************************************************************************************/ - provision = async ( - config: any, - project: IBotProject, - user, - getAccessToken, - getArmAccessToken - ): Promise => { + provision = async (config: any, project: IBotProject, user, getAccessToken): Promise => { const jobId = BackgroundProcessManager.startProcess(202, project.id, config.name, 'Creating Azure resources...'); - this.asyncProvision(jobId, config, project, user, getArmAccessToken); + this.asyncProvision(jobId, config, project, user, getAccessToken); return BackgroundProcessManager.getStatus(jobId); }; diff --git a/extensions/azurePublish/src/node/provision.ts b/extensions/azurePublish/src/node/provision.ts index cf7b32e8e5..961bf0a06d 100644 --- a/extensions/azurePublish/src/node/provision.ts +++ b/extensions/azurePublish/src/node/provision.ts @@ -3,6 +3,7 @@ import { TokenCredentials } from '@azure/ms-rest-js'; import axios, { AxiosRequestConfig } from 'axios'; +import { AuthParameters } from '@botframework-composer/types'; import { BotProjectDeployLoggerType } from './types'; import { AzureResourceManangerConfig } from './azureResourceManager/azureResourceManagerConfig'; @@ -48,7 +49,7 @@ export class BotProjectProvision { // Will be assigned by create or deploy private tenantId = ''; - constructor(config: ProvisionConfig, private getArmAccessToken?: (tenantId: string) => Promise) { + constructor(config: ProvisionConfig, private getAccessToken?: (params: AuthParameters) => Promise) { this.subscriptionId = config.subscription; this.logger = config.logger; this.accessToken = config.accessToken; @@ -97,15 +98,19 @@ export class BotProjectProvision { message: '> Creating App Registration ...', }); - const armAccessToken = await this.getArmAccessToken?.(this.tenantId); + const accessToken = await this.getAccessToken?.({ + scopes: ['https://graph.microsoft.com/Application.ReadWrite.All'], + targetResource: 'https://graph.microsoft.com/', + authority: `https://login.microsoftonline.com/${this.tenantId}/oauth2/v2.0/authorize`, + }); this.logger({ status: BotProjectDeployLoggerType.PROVISION_INFO, - message: `> Retrieved arm access token ...: ${armAccessToken}`, + message: `> Retrieved graph access token ...: ${accessToken}`, }); const appCreateOptions: AxiosRequestConfig = { - headers: { Authorization: `Bearer ${armAccessToken ?? this.graphToken}` }, + headers: { Authorization: `Bearer ${accessToken ?? this.graphToken}` }, }; // This call if successful returns an object in the form From 907c72cf2db08687303e151cf1c5523651c72aae Mon Sep 17 00:00:00 2001 From: TJ Date: Thu, 9 Mar 2023 11:55:56 -0700 Subject: [PATCH 3/5] Cache --- .../server/src/services/auth/electronAuthProvider.ts | 2 +- extensions/azurePublish/src/node/provision.ts | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/Composer/packages/server/src/services/auth/electronAuthProvider.ts b/Composer/packages/server/src/services/auth/electronAuthProvider.ts index b137853b40..c004ba7a21 100644 --- a/Composer/packages/server/src/services/auth/electronAuthProvider.ts +++ b/Composer/packages/server/src/services/auth/electronAuthProvider.ts @@ -140,6 +140,6 @@ export class ElectronAuthProvider extends AuthProvider { } private getTokenHash(params: AuthParameters): string { - return params.targetResource || ''; + return JSON.stringify(params); } } diff --git a/extensions/azurePublish/src/node/provision.ts b/extensions/azurePublish/src/node/provision.ts index 961bf0a06d..44bf3b1f79 100644 --- a/extensions/azurePublish/src/node/provision.ts +++ b/extensions/azurePublish/src/node/provision.ts @@ -98,6 +98,11 @@ export class BotProjectProvision { message: '> Creating App Registration ...', }); + this.logger({ + status: BotProjectDeployLoggerType.PROVISION_INFO, + message: `> TenantId: ${this.tenantId}`, + }); + const accessToken = await this.getAccessToken?.({ scopes: ['https://graph.microsoft.com/Application.ReadWrite.All'], targetResource: 'https://graph.microsoft.com/', From 8c3d344cb7c4b154fed05685d6895b97b192374a Mon Sep 17 00:00:00 2001 From: Eugene Olonov Date: Tue, 4 Apr 2023 10:27:22 -0700 Subject: [PATCH 4/5] fix: ensure parameters match cached and pass authority --- .../server/src/services/auth/electronAuthProvider.ts | 9 +++++---- extensions/azurePublish/src/node/provision.ts | 6 ++++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/Composer/packages/server/src/services/auth/electronAuthProvider.ts b/Composer/packages/server/src/services/auth/electronAuthProvider.ts index c004ba7a21..76756585fd 100644 --- a/Composer/packages/server/src/services/auth/electronAuthProvider.ts +++ b/Composer/packages/server/src/services/auth/electronAuthProvider.ts @@ -34,7 +34,8 @@ export class ElectronAuthProvider extends AuthProvider { async getAccessToken(params: AuthParameters): Promise { const { getAccessToken } = this.electronContext; - const { targetResource = '' } = params; + const { targetResource = '', authority } = params; + const authParams = { targetResource, authority }; if (isLinux()) { log('Auth login flow is currently unsupported in Linux.'); @@ -44,7 +45,7 @@ export class ElectronAuthProvider extends AuthProvider { log('Getting access token.'); // try to get a cached token - const cachedToken = this.getCachedToken(params); + const cachedToken = this.getCachedToken(authParams); if (!!cachedToken && Date.now() <= cachedToken.expiryTime.valueOf()) { log('Returning cached token.'); return cachedToken.accessToken; @@ -53,8 +54,8 @@ export class ElectronAuthProvider extends AuthProvider { try { // otherwise get a fresh token log('Did not find cached token. Getting fresh token.'); - const { accessToken, acquiredAt, expiryTime } = await getAccessToken({ targetResource }); - this.cacheTokens({ accessToken, acquiredAt, expiryTime }, params); + const { accessToken, acquiredAt, expiryTime } = await getAccessToken(authParams); + this.cacheTokens({ accessToken, acquiredAt, expiryTime }, authParams); return accessToken; } catch (e) { diff --git a/extensions/azurePublish/src/node/provision.ts b/extensions/azurePublish/src/node/provision.ts index 44bf3b1f79..38da14ae35 100644 --- a/extensions/azurePublish/src/node/provision.ts +++ b/extensions/azurePublish/src/node/provision.ts @@ -104,14 +104,16 @@ export class BotProjectProvision { }); const accessToken = await this.getAccessToken?.({ - scopes: ['https://graph.microsoft.com/Application.ReadWrite.All'], + scopes: ['https://graph.microsoft.com/Application.ReadWrite.All'], // TODO: figure out if we need to pass scopes targetResource: 'https://graph.microsoft.com/', authority: `https://login.microsoftonline.com/${this.tenantId}/oauth2/v2.0/authorize`, }); this.logger({ status: BotProjectDeployLoggerType.PROVISION_INFO, - message: `> Retrieved graph access token ...: ${accessToken}`, + message: `> Retrieved tenant specific graph access token ...: ${accessToken?.length} ${ + accessToken === this.graphToken + }`, }); const appCreateOptions: AxiosRequestConfig = { From 3af991dcf761af9058995a375f81c32c7b9f1e65 Mon Sep 17 00:00:00 2001 From: Eugene Olonov Date: Tue, 4 Apr 2023 11:11:28 -0700 Subject: [PATCH 5/5] reuse auth headers for password --- extensions/azurePublish/src/node/provision.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/azurePublish/src/node/provision.ts b/extensions/azurePublish/src/node/provision.ts index 38da14ae35..524f588c6f 100644 --- a/extensions/azurePublish/src/node/provision.ts +++ b/extensions/azurePublish/src/node/provision.ts @@ -171,7 +171,7 @@ export class BotProjectProvision { }, }; const setSecretOptions: AxiosRequestConfig = { - headers: { Authorization: `Bearer ${this.graphToken}` }, + headers: { Authorization: appCreateOptions.headers.Authorization }, }; let passwordSet;