From afaf5d918c3dd161cdb4cea5b446342734009a04 Mon Sep 17 00:00:00 2001 From: Tony Anziano Date: Sun, 28 Feb 2021 20:01:40 -0800 Subject: [PATCH] feature: Add support for MSA authentication (#5909) * Implemented server-side endpoints for ARM auth * Removed old debug code * Exposed getTenants and getARMTokenForTenant to extensions * Added CSRF protection to ARM routes * Added extra logging to ARM flow * Fixed tests * Added try-catch blocks and better logging * Added tests Co-authored-by: Dong Lei --- Composer/packages/client/src/plugins/api.ts | 14 ++-- .../packages/client/src/utils/authClient.ts | 46 ++++++++++- .../__tests__/auth/oneAuthService.test.ts | 76 ++++++++++++++++++- .../src/auth/oneAuthService.ts | 69 ++++++++++++++++- .../electron-server/src/auth/oneAuthShim.ts | 8 +- Composer/packages/electron-server/src/main.ts | 2 + .../extension-client/src/auth/index.ts | 17 ++++- .../packages/server/src/controllers/auth.ts | 30 ++++++++ Composer/packages/server/src/router/api.ts | 2 + .../server/src/utility/electronContext.ts | 4 +- Composer/packages/types/src/azure.ts | 13 ++++ Composer/packages/types/src/index.ts | 1 + 12 files changed, 267 insertions(+), 15 deletions(-) create mode 100644 Composer/packages/types/src/azure.ts diff --git a/Composer/packages/client/src/plugins/api.ts b/Composer/packages/client/src/plugins/api.ts index 12147d4948..ece74de137 100644 --- a/Composer/packages/client/src/plugins/api.ts +++ b/Composer/packages/client/src/plugins/api.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { AuthParameters } from '@botframework-composer/types'; +import { AuthParameters, AzureTenant } from '@botframework-composer/types'; import { AuthClient } from '../utils/authClient'; @@ -18,6 +18,8 @@ interface PublishConfig { interface AuthAPI { getAccessToken: (options: AuthParameters) => Promise; // returns an access token + getARMTokenForTenant: (tenantId: string) => Promise; // returns an ARM access token for specified tenant + getTenants: () => Promise; // signs a user in and returns available Azure tenants for the user logOut: () => Promise; } @@ -41,12 +43,10 @@ class API implements IAPI { constructor() { this.auth = { - getAccessToken: (params: AuthParameters) => { - return AuthClient.getAccessToken(params); - }, - logOut: () => { - return AuthClient.logOut(); - }, + getAccessToken: (params: AuthParameters) => AuthClient.getAccessToken(params), + getARMTokenForTenant: (tenantId: string) => AuthClient.getARMTokenForTenant(tenantId), + getTenants: () => AuthClient.getTenants(), + logOut: () => AuthClient.logOut(), }; this.publish = { getPublishConfig: undefined, diff --git a/Composer/packages/client/src/utils/authClient.ts b/Composer/packages/client/src/utils/authClient.ts index 4bd160e357..06dd73ad61 100644 --- a/Composer/packages/client/src/utils/authClient.ts +++ b/Composer/packages/client/src/utils/authClient.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { AuthParameters } from '@botframework-composer/types'; +import { AuthParameters, AzureTenant } from '@botframework-composer/types'; import { authConfig } from '../constants'; @@ -105,7 +105,51 @@ async function logOut() { } } +/** + * Will retrieve an ARM token for the desired Azure tenant. + * NOTE: Should call getTenants() beforehand to retrieve a list of + * available tenants. + * @param tenantId The Azure tenant to get an ARM token for + */ +async function getARMTokenForTenant(tenantId: string): Promise { + const options = { + method: 'GET', + headers: {}, + }; + if (isElectron()) { + const { __csrf__ = '' } = window; + options.headers['X-CSRF-Token'] = __csrf__; + } + + const result = await fetch(`/api/auth/getARMTokenForTenant?tenantId=${tenantId}`, options); + const { accessToken = '' } = await result.json(); + return accessToken; +} + +/** + * Will log the user into ARM and return a list of available Azure tenants for the user. + * This should then be used to display the list of tenants in the UI, allowing the user + * to select a tenant. The selected tenant ID should then be passed to getARMTokenForTenant() + * to get an ARM token for reading / writing Azure resources in that tenant. + */ +async function getTenants(): Promise { + const options = { + method: 'GET', + headers: {}, + }; + if (isElectron()) { + const { __csrf__ = '' } = window; + options.headers['X-CSRF-Token'] = __csrf__; + } + + const result = await fetch('/api/auth/getTenants', options); + const { tenants = [] } = await result.json(); + return tenants; +} + export const AuthClient = { getAccessToken, + getARMTokenForTenant, + getTenants, logOut, }; diff --git a/Composer/packages/electron-server/__tests__/auth/oneAuthService.test.ts b/Composer/packages/electron-server/__tests__/auth/oneAuthService.test.ts index 9e896129c0..b01f9af661 100644 --- a/Composer/packages/electron-server/__tests__/auth/oneAuthService.test.ts +++ b/Composer/packages/electron-server/__tests__/auth/oneAuthService.test.ts @@ -1,3 +1,6 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + import { OneAuthInstance } from '../../src/auth/oneAuthService'; jest.mock('../../src/electronWindow', () => ({ @@ -13,6 +16,9 @@ jest.mock('../../src/utility/platform', () => ({ isMac: () => false, })); +const mockFetch = jest.fn().mockResolvedValue(true); +jest.mock('node-fetch', () => async (...args) => mockFetch(...args)); + describe('OneAuth Serivce', () => { const INTERACTION_REQUIRED = 'interactionRequired'; const mockAccount = { @@ -29,7 +35,7 @@ describe('OneAuth Serivce', () => { initialize: jest.fn(), setLogCallback: jest.fn(), setLogPiiEnabled: jest.fn(), - signInInteractively: jest.fn().mockResolvedValue({ account: mockAccount }), + signInInteractively: jest.fn().mockResolvedValue({ account: mockAccount, credential: mockCredential }), shutdown: jest.fn(), AadConfiguration: class AAD {}, AppConfiguration: class App {}, @@ -37,9 +43,11 @@ describe('OneAuth Serivce', () => { Status: { InteractionRequired: INTERACTION_REQUIRED, }, + MsaConfiguration: class MSA {}, + setFlights: jest.fn(), }; let oneAuthService = new OneAuthInstance(); // bypass the shim logic - let processEnvBackup = { ...process.env }; + const processEnvBackup = { ...process.env }; afterEach(() => { process.env = processEnvBackup; @@ -56,8 +64,10 @@ describe('OneAuth Serivce', () => { mockOneAuth.setLogPiiEnabled.mockClear(); mockOneAuth.signInInteractively.mockClear(); mockOneAuth.shutdown.mockClear(); + mockFetch.mockClear(); (oneAuthService as any).initialized = false; (oneAuthService as any).signedInAccount = undefined; + (oneAuthService as any).signedInARMAccount = undefined; }); it('should sign in and get an access token (happy path)', async () => { @@ -147,4 +157,66 @@ describe('OneAuth Serivce', () => { expect(result).toEqual({ accessToken: '', acquiredAt: 0, expiryTime: 99999999999 }); }); + + 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.mock.calls[0][0]).toBe('https://management.azure.com/tenants?api-version=2020-01-01'); + + expect(tenants).toBe(mockTenants); + }); + + it('should throw an error if something goes wrong while getting a list of tenants', async () => { + mockFetch.mockRejectedValueOnce({ error: 'could not get a list of tenants' }); + + await expect(oneAuthService.getTenants()).rejects.toEqual({ error: 'could not get a list of tenants' }); + }); + + it('should get an ARM token for a tenant', async () => { + mockOneAuth.acquireCredentialSilently.mockReturnValueOnce({ credential: { value: 'someARMToken' } }); + (oneAuthService as any).signedInARMAccount = {}; + const result = await oneAuthService.getARMTokenForTenant('someTenant'); + + expect(result).toBe('someARMToken'); + }); + + it('should return an empty string if there is no signed in ARM account', async () => { + (oneAuthService as any).signedInARMAccount = undefined; + const result = await oneAuthService.getARMTokenForTenant('someTenant'); + + expect(result).toBe(''); + }); + + it('should throw an error if something goes wrong while getting an ARM token', async () => { + mockOneAuth.acquireCredentialSilently.mockRejectedValueOnce({ error: 'Could not get an ARM token' }); + (oneAuthService as any).signedInARMAccount = {}; + + await expect(oneAuthService.getARMTokenForTenant('someTenant')).rejects.toEqual({ + error: 'Could not get an ARM token', + }); + }); }); diff --git a/Composer/packages/electron-server/src/auth/oneAuthService.ts b/Composer/packages/electron-server/src/auth/oneAuthService.ts index 293c7fcfd7..cc27080fd4 100644 --- a/Composer/packages/electron-server/src/auth/oneAuthService.ts +++ b/Composer/packages/electron-server/src/auth/oneAuthService.ts @@ -2,8 +2,9 @@ // Licensed under the MIT License. import path from 'path'; -import { ElectronAuthParameters } from '@botframework-composer/types'; +import { AzureTenant, ElectronAuthParameters } from '@botframework-composer/types'; import { app } from 'electron'; +import fetch from 'node-fetch'; import ElectronWindow from '../electronWindow'; import { isLinux, isMac } from '../utility/platform'; @@ -26,11 +27,18 @@ const GRAPH_RESOURCE = 'https://graph.microsoft.com'; const DEFAULT_LOCALE = 'en'; // TODO: get this from settings? const DEFAULT_AUTH_SCHEME = 2; // bearer token const DEFAULT_AUTH_AUTHORITY = 'https://login.microsoftonline.com/common'; // work and school accounts +const ARM_AUTHORITY = 'https://login.microsoftonline.com/organizations'; +const ARM_RESOURCE = 'https://management.core.windows.net'; + +type GetTenantsResult = { + value: AzureTenant[]; +}; export class OneAuthInstance extends OneAuthBase { private initialized: boolean; private _oneAuth: typeof OneAuth | null = null; //eslint-disable-line private signedInAccount: OneAuth.Account | undefined; + private signedInARMAccount: OneAuth.Account | undefined; constructor() { super(); @@ -57,13 +65,22 @@ export class OneAuthInstance extends OneAuthBase { 'Please login', window.getNativeWindowHandle() ); + const msaConfig = new this.oneAuth.MsaConfiguration( + COMPOSER_CLIENT_ID, + COMPOSER_REDIRECT_URI, + GRAPH_RESOURCE + '/Application.ReadWrite.All', + undefined + ); const aadConfig = new this.oneAuth.AadConfiguration( COMPOSER_CLIENT_ID, COMPOSER_REDIRECT_URI, GRAPH_RESOURCE, false // prefer broker ); - this.oneAuth.initialize(appConfig, undefined, aadConfig, undefined); + this.oneAuth.setFlights([ + 2, // UseMsalforMsa + ]); + this.oneAuth.initialize(appConfig, msaConfig, aadConfig, undefined); this.initialized = true; log('Service initialized.'); } else { @@ -137,6 +154,54 @@ export class OneAuthInstance extends OneAuthBase { } } + public async getTenants(): Promise { + try { + 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; + + // call the tenants API + const tenantsResult = await fetch('https://management.azure.com/tenants?api-version=2020-01-01', { + headers: { Authorization: `Bearer ${token}` }, + }); + const tenants = (await tenantsResult.json()) as GetTenantsResult; + log('Got Azure tenants for user: %O', tenants.value); + return tenants.value; + } catch (e) { + log('There was an error trying to log the user into ARM: %O', e); + throw e; + } + } + + public async getARMTokenForTenant(tenantId: string): Promise { + if (this.signedInARMAccount) { + try { + log('Getting an ARM token for tenant %s', tenantId); + const tokenParams = new this.oneAuth.AuthParameters( + DEFAULT_AUTH_SCHEME, + `https://login.microsoftonline.com/${tenantId}`, + ARM_RESOURCE, + '', + '' + ); + const result = await this.oneAuth.acquireCredentialSilently(this.signedInARMAccount.id, tokenParams, ''); + log('Acquired ARM token for tenant: %s', result.credential.value); + return result.credential.value; + } catch (e) { + log('There was an error trying to get an ARM token for tenant %s: %O', tenantId, e); + throw e; + } + } + return ''; + } + public shutdown() { log('Shutting down...'); this.oneAuth.shutdown(); diff --git a/Composer/packages/electron-server/src/auth/oneAuthShim.ts b/Composer/packages/electron-server/src/auth/oneAuthShim.ts index 4667522b3e..afc87dfc50 100644 --- a/Composer/packages/electron-server/src/auth/oneAuthShim.ts +++ b/Composer/packages/electron-server/src/auth/oneAuthShim.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { ElectronAuthParameters } from '@botframework-composer/types'; +import { AzureTenant, ElectronAuthParameters } from '@botframework-composer/types'; import logger from '../utility/logger'; @@ -21,4 +21,10 @@ export class OneAuthShim extends OneAuthBase { } public shutdown() {} public signOut() {} + public async getARMTokenForTenant(tenantId: string): Promise { + return ''; + } + public async getTenants(): Promise { + return []; + } } diff --git a/Composer/packages/electron-server/src/main.ts b/Composer/packages/electron-server/src/main.ts index 9ee2951b16..553419e4be 100644 --- a/Composer/packages/electron-server/src/main.ts +++ b/Composer/packages/electron-server/src/main.ts @@ -160,6 +160,8 @@ async function loadServer() { const { start } = await import('@bfc/server'); serverPort = await start({ getAccessToken: OneAuthService.getAccessToken.bind(OneAuthService), + getARMTokenForTenant: OneAuthService.getARMTokenForTenant.bind(OneAuthService), + getTenants: OneAuthService.getTenants.bind(OneAuthService), logOut: OneAuthService.signOut.bind(OneAuthService), machineId, sessionId, diff --git a/Composer/packages/extension-client/src/auth/index.ts b/Composer/packages/extension-client/src/auth/index.ts index d62924ef81..bf9d25c4b1 100644 --- a/Composer/packages/extension-client/src/auth/index.ts +++ b/Composer/packages/extension-client/src/auth/index.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { AuthParameters } from '@botframework-composer/types'; +import { AuthParameters, AzureTenant } from '@botframework-composer/types'; import { ComposerGlobalName } from '../common/constants'; @@ -19,3 +19,18 @@ export function logOut(): Promise { export function getAccessToken(params: AuthParameters): Promise { return window[ComposerGlobalName].getAccessToken(params); } + +/** + * Logs the user in and returns a list of available Azure tenants. + */ +export function getTenants(): Promise { + return window[ComposerGlobalName].getTenants(); +} + +/** + * Returns an ARM token for the specified tenant. Should be called AFTER getTenants(). + * @param tenantId Tenant to retrieve an ARM token for + */ +export function getARMTokenForTenant(tenantId: string): Promise { + return window[ComposerGlobalName].getARMTokenForTenant(tenantId); +} diff --git a/Composer/packages/server/src/controllers/auth.ts b/Composer/packages/server/src/controllers/auth.ts index 68c75f8d99..161f26c7ed 100644 --- a/Composer/packages/server/src/controllers/auth.ts +++ b/Composer/packages/server/src/controllers/auth.ts @@ -4,6 +4,7 @@ import { Request, Response } from 'express'; import { authService } from '../services/auth/auth'; +import { useElectronContext } from '../utility/electronContext'; import { isElectron } from '../utility/isElectron'; type GetAccessTokenRequest = Request & { @@ -17,6 +18,12 @@ type GetAccessTokenRequest = Request & { }; }; +type GetARMTokenForTenantRequest = Request & { + query: { + tenantId: string; + }; +}; + async function getAccessToken(req: GetAccessTokenRequest, res: Response) { const { clientId, targetResource, scopes = '[]' } = req.query; if (isElectron && !targetResource) { @@ -44,7 +51,30 @@ async function logOut(req, res) { res.status(200); } +async function getTenants(req: Request, res: Response) { + try { + const { getTenants } = useElectronContext(); + const tenants = await getTenants(); + res.status(200).json({ tenants }); + } catch (e) { + return res.status(500).json(e); + } +} + +async function getARMTokenForTenant(req: GetARMTokenForTenantRequest, res: Response) { + try { + const { tenantId } = req.query; + const { getARMTokenForTenant } = useElectronContext(); + const accessToken = await getARMTokenForTenant(tenantId); + res.status(200).json({ accessToken }); + } catch (e) { + return res.status(500).json(e); + } +} + export const AuthController = { getAccessToken, + getTenants, + getARMTokenForTenant, logOut, }; diff --git a/Composer/packages/server/src/router/api.ts b/Composer/packages/server/src/router/api.ts index 7ca97028dd..f3ae8d7e4b 100644 --- a/Composer/packages/server/src/router/api.ts +++ b/Composer/packages/server/src/router/api.ts @@ -112,6 +112,8 @@ router.post('/extensions/proxy/:url', ExtensionsController.performExtensionFetch // authentication from client router.get('/auth/getAccessToken', csrfProtection, AuthController.getAccessToken); router.get('/auth/logOut', AuthController.logOut); +router.get('/auth/getTenants', csrfProtection, AuthController.getTenants); +router.get('/auth/getARMTokenForTenant', csrfProtection, AuthController.getARMTokenForTenant); // FeatureFlags router.get('/featureFlags', FeatureFlagController.getFeatureFlags); diff --git a/Composer/packages/server/src/utility/electronContext.ts b/Composer/packages/server/src/utility/electronContext.ts index 0adc854e86..1dbdd3da8f 100644 --- a/Composer/packages/server/src/utility/electronContext.ts +++ b/Composer/packages/server/src/utility/electronContext.ts @@ -1,12 +1,14 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { ElectronAuthParameters } from '@botframework-composer/types'; +import { AzureTenant, ElectronAuthParameters } from '@botframework-composer/types'; export type ElectronContext = { getAccessToken: ( params: ElectronAuthParameters ) => Promise<{ accessToken: string; acquiredAt: number; expiryTime: number }>; + getARMTokenForTenant: (tenantId: string) => Promise; + getTenants: () => Promise; logOut: () => void; machineId: string; sessionId: string; diff --git a/Composer/packages/types/src/azure.ts b/Composer/packages/types/src/azure.ts new file mode 100644 index 0000000000..57c53dc9c6 --- /dev/null +++ b/Composer/packages/types/src/azure.ts @@ -0,0 +1,13 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +export type AzureTenant = { + countryCode: string; + defaultDomain: string; + displayName: string; + domains: string[]; + id: string; + tenantCategory: string; + tenantId: string; + tenantType: string; +}; diff --git a/Composer/packages/types/src/index.ts b/Composer/packages/types/src/index.ts index 0972541e98..7c4deb0901 100644 --- a/Composer/packages/types/src/index.ts +++ b/Composer/packages/types/src/index.ts @@ -2,6 +2,7 @@ // Licensed under the MIT License. export * from './auth'; +export * from './azure'; export * from './diagnostic'; export * from './dialogUtils'; export * from './extension';