Merge branch 'main' into gcox/feature6627

Composer/packages/client/__tests__/pages/botProjectsSettings/PublishTarget.test.tsx

@@ -13,7 +13,7 @@ import { isShowAuthDialog } from '../../../src/utils/auth';
 jest.mock('../../../src/utils/auth', () => ({
   isShowAuthDialog: jest.fn(),
   getTokenFromCache: jest.fn(),
-  isGetTokenFromUser: jest.fn(),
+  userShouldProvideTokens: jest.fn(),
 }));
 
 const state = {

Composer/packages/client/src/components/ManageLuis/ManageLuis.tsx

@@ -23,7 +23,7 @@ import { ProvisionHandoff } from '@bfc/ui-shared';
 import { AuthClient } from '../../utils/authClient';
 import { AuthDialog } from '../../components/Auth/AuthDialog';
 import { armScopes } from '../../constants';
-import { getTokenFromCache, isShowAuthDialog, isGetTokenFromUser } from '../../utils/auth';
+import { getTokenFromCache, isShowAuthDialog, userShouldProvideTokens } from '../../utils/auth';
 import { LUIS_REGIONS } from '../../constants';
 import { dispatcherState } from '../../recoilModel/atoms';
 
@@ -94,7 +94,7 @@ export const ManageLuis = (props: ManageLuisProps) => {
 
   const hasAuth = async () => {
     let newtoken = '';
-    if (isGetTokenFromUser()) {
+    if (userShouldProvideTokens()) {
       if (isShowAuthDialog(false)) {
         setShowAuthDialog(true);
       }

Composer/packages/client/src/components/ManageQNA/ManageQNA.tsx

@@ -26,7 +26,7 @@ import { ProvisionHandoff } from '@bfc/ui-shared';
 import { AuthClient } from '../../utils/authClient';
 import { AuthDialog } from '../../components/Auth/AuthDialog';
 import { armScopes } from '../../constants';
-import { getTokenFromCache, isShowAuthDialog, isGetTokenFromUser } from '../../utils/auth';
+import { getTokenFromCache, isShowAuthDialog, userShouldProvideTokens } from '../../utils/auth';
 import { dispatcherState } from '../../recoilModel/atoms';
 
 type ManageQNAProps = {
@@ -102,7 +102,7 @@ export const ManageQNA = (props: ManageQNAProps) => {
 
   const hasAuth = async () => {
     let newtoken = '';
-    if (isGetTokenFromUser()) {
+    if (userShouldProvideTokens()) {
       if (isShowAuthDialog(false)) {
         setShowAuthDialog(true);
       }

Composer/packages/client/src/pages/botProject/adapters/ABSChannels.tsx

@@ -25,7 +25,7 @@ import { settingsState } from '../../../recoilModel';
 import { AuthClient } from '../../../utils/authClient';
 import { AuthDialog } from '../../../components/Auth/AuthDialog';
 import { armScopes } from '../../../constants';
-import { getTokenFromCache, isShowAuthDialog, isGetTokenFromUser } from '../../../utils/auth';
+import { getTokenFromCache, isShowAuthDialog, userShouldProvideTokens } from '../../../utils/auth';
 import httpClient from '../../../utils/httpUtil';
 import { dispatcherState } from '../../../recoilModel';
 import {
@@ -119,7 +119,7 @@ export const ABSChannels: React.FC<RuntimeSettingsProps> = (props) => {
       navigateTo(`/bot/${projectId}/botProjectsSettings/#addNewPublishProfile`);
     } else {
       let newtoken = '';
-      if (isGetTokenFromUser()) {
+      if (userShouldProvideTokens()) {
         if (isShowAuthDialog(false)) {
           setShowAuthDialog(true);
         }
@@ -315,7 +315,7 @@ export const ABSChannels: React.FC<RuntimeSettingsProps> = (props) => {
 
   const hasAuth = async () => {
     let newtoken = '';
-    if (isGetTokenFromUser()) {
+    if (userShouldProvideTokens()) {
       if (isShowAuthDialog(false)) {
         setShowAuthDialog(true);
       }

Composer/packages/client/src/pages/botProject/create-publish-profile/PublishProfileDialog.tsx

@@ -10,7 +10,7 @@ import { Dialog } from 'office-ui-fabric-react/lib/Dialog';
 import { Link } from 'office-ui-fabric-react/lib/Link';
 import { useRecoilValue } from 'recoil';
 
-import { getTokenFromCache, isGetTokenFromUser, setTenantId, getTenantIdFromCache } from '../../../utils/auth';
+import { getTokenFromCache, userShouldProvideTokens, setTenantId, getTenantIdFromCache } from '../../../utils/auth';
 import { PublishType } from '../../../recoilModel/types';
 import { PluginAPI } from '../../../plugins/api';
 import { PluginHost } from '../../../components/PluginHost/PluginHost';
@@ -91,8 +91,12 @@ export const PublishProfileDialog: React.FC<PublishProfileDialogProps> = (props)
         graphToken: getTokenFromCache('graphToken'),
       };
     };
+    /** @deprecated use `userShouldProvideTokens` instead */
     PluginAPI.publish.isGetTokenFromUser = () => {
-      return isGetTokenFromUser();
+      return userShouldProvideTokens();
+    };
+    PluginAPI.publish.userShouldProvideTokens = () => {
+      return userShouldProvideTokens();
     };
     PluginAPI.publish.setTitle = (value) => {
       setTitle(value);
@@ -160,7 +164,7 @@ export const PublishProfileDialog: React.FC<PublishProfileDialogProps> = (props)
       const fullConfig = { ...config, name: name, type: targetType };
 
       let arm, graph;
-      if (!isGetTokenFromUser()) {
+      if (!userShouldProvideTokens()) {
         const tenantId = getTenantIdFromCache();
         // require tenant id to be set by plugin (handles multiple tenant scenario)
         if (!tenantId) {

Composer/packages/client/src/pages/publish/Publish.tsx

@@ -17,7 +17,7 @@ import { getSensitiveProperties } from '../../recoilModel/dispatchers/utils/proj
 import {
   getTokenFromCache,
   isShowAuthDialog,
-  isGetTokenFromUser,
+  userShouldProvideTokens,
   setTenantId,
   getTenantIdFromCache,
 } from '../../utils/auth';
@@ -246,13 +246,13 @@ const Publish: React.FC<RouteComponentProps<{ projectId: string; targetName?: st
       if (target && isPublishingToAzure(target)) {
         const { tenantId } = JSON.parse(target.configuration);
 
-        if (tenantId) {
+        if (userShouldProvideTokens()) {
+          token = getTokenFromCache('accessToken');
+        } else if (tenantId) {
           token = tenantTokenMap.get(tenantId) ?? (await AuthClient.getARMTokenForTenant(tenantId));
           tenantTokenMap.set(tenantId, token);
-        } else if (isGetTokenFromUser()) {
-          token = getTokenFromCache('accessToken');
-          // old publish profile without tenant id
         } else {
+          // old publish profile without tenant id
           let tenant = getTenantIdFromCache();
           let tenants;
           if (!tenant) {

Composer/packages/client/src/plugins/api.ts

@@ -35,7 +35,9 @@ interface PublishAPI {
   getName?: () => string;
   savePublishConfig?: (config: PublishConfig) => void;
   getTokenFromCache?: () => { accessToken: string; graphToken: string };
+  /** @deprecated use `userShouldProvideTokens` instead */
   isGetTokenFromUser?: () => boolean;
+  userShouldProvideTokens?: () => boolean;
   getTenantIdFromCache?: () => string;
   setTenantId?: (value: string) => void;
 }
@@ -62,6 +64,7 @@ class API implements IAPI {
       savePublishConfig: undefined,
       getTokenFromCache: undefined,
       isGetTokenFromUser: undefined,
+      userShouldProvideTokens: undefined,
       getTenantIdFromCache: undefined,
       setTenantId: undefined,
     };

Composer/packages/client/src/utils/auth.ts

@@ -341,7 +341,7 @@ export function isShowAuthDialog(needGraph: boolean): boolean {
   }
 }
 
-export function isGetTokenFromUser(): boolean {
+export function userShouldProvideTokens(): boolean {
   if (isElectron()) {
     return false;
   } else if (authConfig.clientId && authConfig.redirectUrl && authConfig.tenantId) {

Composer/packages/electron-server/__tests__/auth/oneAuthService.test.ts

@@ -56,6 +56,7 @@ describe('OneAuth Serivce', () => {
   beforeEach(() => {
     jest.resetModules();
     oneAuthService = new OneAuthInstance();
+    // eslint-disable-next-line no-underscore-dangle
     (oneAuthService as any)._oneAuth = mockOneAuth;
     mockOneAuth.acquireCredentialInteractively.mockClear();
     mockOneAuth.acquireCredentialSilently.mockClear();
@@ -169,38 +170,82 @@ describe('OneAuth Serivce', () => {
     const result = await service.getAccessToken({});
 
     expect(result).toEqual({ accessToken: '', acquiredAt: 0, expiryTime: 99999999999 });
+    // reset node env
+    process.env.NODE_ENV = 'test';
   });
 
-  it('should get a list of tenants', async () => {
-    const mockTenants = [
-      {
-        tenantId: 'tenant1',
-      },
-      {
-        tenantId: 'tenant2',
-      },
-      {
-        tenantId: 'tenant3',
-      },
-    ];
-    mockFetch.mockResolvedValueOnce({
-      json: jest.fn().mockResolvedValue({ value: mockTenants }),
+  describe('#getTenants', () => {
+    it('should get a list of tenants', async () => {
+      const mockTenants = [
+        {
+          tenantId: 'tenant1',
+        },
+        {
+          tenantId: 'tenant2',
+        },
+        {
+          tenantId: 'tenant3',
+        },
+      ];
+      mockFetch.mockResolvedValueOnce({
+        json: jest.fn().mockResolvedValue({ value: mockTenants }),
+      });
+      const tenants = await oneAuthService.getTenants();
+
+      // it should have initialized
+      expect(mockOneAuth.setLogPiiEnabled).toHaveBeenCalled();
+      expect(mockOneAuth.setLogCallback).toHaveBeenCalled();
+      expect(mockOneAuth.initialize).toHaveBeenCalled();
+
+      // it should have signed in
+      expect(mockOneAuth.signInInteractively).toHaveBeenCalled();
+      expect((oneAuthService as any).signedInARMAccount).toEqual(mockAccount);
+
+      // it should have called the tenants API
+      expect(mockFetch).toHaveBeenCalledWith('https://management.azure.com/tenants?api-version=2020-01-01', {
+        headers: {
+          Authorization: 'Bearer someToken',
+        },
+      });
+
+      expect(tenants).toBe(mockTenants);
     });
-    const tenants = await oneAuthService.getTenants();
 
-    // it should have initialized
-    expect(mockOneAuth.setLogPiiEnabled).toHaveBeenCalled();
-    expect(mockOneAuth.setLogCallback).toHaveBeenCalled();
-    expect(mockOneAuth.initialize).toHaveBeenCalled();
-
-    // it should have signed in
-    expect(mockOneAuth.signInInteractively).toHaveBeenCalled();
-    expect((oneAuthService as any).signedInARMAccount).toEqual(mockAccount);
-
-    // it should have called the tenants API
-    expect(mockFetch.mock.calls[0][0]).toBe('https://management.azure.com/tenants?api-version=2020-01-01');
-
-    expect(tenants).toBe(mockTenants);
+    it('should not attempt to sign in if token already fetched', async () => {
+      const mockTenants = [
+        {
+          tenantId: 'tenant1',
+        },
+        {
+          tenantId: 'tenant2',
+        },
+        {
+          tenantId: 'tenant3',
+        },
+      ];
+      mockFetch.mockResolvedValueOnce({
+        json: jest.fn().mockResolvedValue({ value: mockTenants }),
+      });
+      (oneAuthService as any).signedInARMAccount = { some: 'account' };
+      (oneAuthService as any).tenantToken = 'cached-token';
+      const tenants = await oneAuthService.getTenants();
+
+      // it should have initialized
+      expect(mockOneAuth.setLogPiiEnabled).toHaveBeenCalled();
+      expect(mockOneAuth.setLogCallback).toHaveBeenCalled();
+      expect(mockOneAuth.initialize).toHaveBeenCalled();
+
+      expect(mockOneAuth.signInInteractively).not.toHaveBeenCalled();
+
+      // it should have called the tenants API
+      expect(mockFetch).toHaveBeenCalledWith('https://management.azure.com/tenants?api-version=2020-01-01', {
+        headers: {
+          Authorization: 'Bearer cached-token',
+        },
+      });
+
+      expect(tenants).toBe(mockTenants);
+    });
   });
 
   it('should throw an error if something goes wrong while getting a list of tenants', async () => {

Composer/packages/electron-server/__tests__/setupTests.js

@@ -1,4 +1,5 @@
-process.env.DEBUG = 'composer*';
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
 
 jest.mock('electron', () => ({
   app: {

Composer/packages/electron-server/src/auth/oneAuthService.ts

@@ -62,6 +62,8 @@ export class OneAuthInstance extends OneAuthBase {
   private _oneAuth: typeof OneAuth | null = null; //eslint-disable-line
   private signedInAccount: OneAuth.Account | undefined;
   private signedInARMAccount: OneAuth.Account | undefined;
+  /** Token solely used to fetch tenants */
+  private tenantToken: string | undefined;
 
   constructor() {
     super();
@@ -189,17 +191,19 @@ export class OneAuthInstance extends OneAuthBase {
       if (!this.initialized) {
         this.initialize();
       }
-      // log the user into the infrastructure tenant to get a token that can be used on the "tenants" API
-      log('Logging user into ARM...');
-      this.signedInARMAccount = undefined;
-      const signInParams = new this.oneAuth.AuthParameters(DEFAULT_AUTH_SCHEME, ARM_AUTHORITY, ARM_RESOURCE, '', '');
-      const result: OneAuth.AuthResult = await this.oneAuth.signInInteractively('', signInParams, '');
-      this.signedInARMAccount = result.account;
-      const token = result.credential.value;
+
+      if (!this.signedInARMAccount || !this.tenantToken) {
+        // log the user into the infrastructure tenant to get a token that can be used on the "tenants" API
+        log('Logging user into ARM...');
+        const signInParams = new this.oneAuth.AuthParameters(DEFAULT_AUTH_SCHEME, ARM_AUTHORITY, ARM_RESOURCE, '', '');
+        const result: OneAuth.AuthResult = await this.oneAuth.signInInteractively('', signInParams, '');
+        this.signedInARMAccount = result.account;
+        this.tenantToken = result.credential.value;
+      }
 
       // call the tenants API
       const tenantsResult = await fetch('https://management.azure.com/tenants?api-version=2020-01-01', {
-        headers: { Authorization: `Bearer ${token}` },
+        headers: { Authorization: `Bearer ${this.tenantToken}` },
       });
       const tenants = (await tenantsResult.json()) as GetTenantsResult;
       log('Got Azure tenants for user: %O', tenants.value);

Composer/packages/electron-server/src/main.ts

@@ -181,10 +181,6 @@ async function main(show = false) {
   const mainWindow = ElectronWindow.getInstance().browserWindow;
   initAppMenu(mainWindow);
   if (mainWindow) {
-    if (process.env.COMPOSER_DEV_TOOLS) {
-      mainWindow.webContents.openDevTools();
-    }
-
     await mainWindow.loadURL(getBaseUrl());
 
     if (show) {
@@ -296,6 +292,10 @@ async function run() {
 
     const mainWindow = getMainWindow();
     mainWindow?.webContents.send('session-update', 'session-started');
+
+    if (process.env.COMPOSER_DEV_TOOLS) {
+      mainWindow?.webContents.openDevTools();
+    }
   });
 
   // Quit when all windows are closed.

Composer/packages/extension-client/src/hooks/usePublishApi.ts

@@ -55,8 +55,12 @@ export function usePublishApi() {
   function setTenantId(value): void {
     window[ComposerGlobalName].setTenantId(value);
   }
+  function userShouldProvideTokens(): boolean {
+    return window[ComposerGlobalName].userShouldProvideTokens();
+  }
+  /** @deprecated use `userShouldProvideTokens` instead */
   function isGetTokenFromUser(): boolean {
-    return window[ComposerGlobalName].isGetTokenFromUser();
+    return window[ComposerGlobalName].userShouldProvideTokens();
   }
   return {
     publishConfig: getPublishConfig(),
@@ -71,6 +75,7 @@ export function usePublishApi() {
     savePublishConfig,
     getTokenFromCache,
     isGetTokenFromUser,
+    userShouldProvideTokens,
     getTenantIdFromCache,
     setTenantId,
   };

Composer/packages/server/src/locales/en-US.json

@@ -4025,4 +4025,4 @@
   "your_template_requires_qna_maker_to_access_content_a4ca6f76": {
     "message": "Your template requires QnA Maker to access content for your bot."
   }
-}
+}

extensions/azurePublish/src/components/ResourceGroupPicker.tsx

@@ -46,6 +46,7 @@ type ResourceGroupItemChoice = {
 };
 
 type Props = {
+  disabled: boolean;
   /**
    * The resource groups to choose from.
    * Set to undefined to disable this picker.
@@ -93,6 +94,7 @@ const onRenderLabel = (props) => {
 };
 
 export const ResourceGroupPicker = ({
+  disabled,
   resourceGroupNames,
   selectedResourceGroupName: controlledSelectedName,
   newResourceGroupName: controlledNewName,
@@ -169,7 +171,7 @@ export const ResourceGroupPicker = ({
         ariaLabel={formatMessage(
           'A resource group is a collection of resources that share the same lifecycle, permissions, and policies'
         )}
-        disabled={loading}
+        disabled={loading || disabled}
         label={formatMessage('Resource group')}
         options={options}
         placeholder={formatMessage('Select one')}