From b8ebecd100ac0f4b3fa00cccb91d45d02add729f Mon Sep 17 00:00:00 2001 From: Alex Crome Date: Wed, 6 Jul 2022 23:20:28 +0100 Subject: [PATCH] Use Azure Identity to Authenticate with Microsoft Graph API Replaced MSAL with Azure Identity to support additional authentication methods with Microsoft Graph - in particular Managed Identity. If `clientId` and clientSecret` are configured, the plugin will continue to behave as before using those credentials. If those values are ommitted, then `DefaultAzureCredential` is choose the best method of authentication. This is particularly helpful locally as if the Azure CLI or Azure Powershell are installed, Azure Identity will use those to authenticate, without the developer needing to configure anything. In those cases, the following config will be sufficient for a working graph plugin. ```yaml catalog: provider: microsoftGraphOrg: default: tenantId: ${AZURE_TENANT_ID} user: filter: accountEnabled eq true and userType eq 'member' group: filter: securityEnabled eq false and mailEnabled eq true and groupTypes/any(c:c+eq+'Unified') ``` Signed-off-by: Alex Crome --- .changeset/polite-keys-confess.md | 12 +++ app-config.yaml | 9 ++ .../catalog-backend-module-msgraph/README.md | 14 ++- .../package.json | 6 +- .../src/microsoftGraph/client.test.ts | 30 +++--- .../src/microsoftGraph/client.ts | 43 +++++---- .../src/microsoftGraph/config.test.ts | 93 ++++++++++++------- .../src/microsoftGraph/config.ts | 49 +++++++--- yarn.lock | 2 +- 9 files changed, 169 insertions(+), 89 deletions(-) create mode 100644 .changeset/polite-keys-confess.md diff --git a/.changeset/polite-keys-confess.md b/.changeset/polite-keys-confess.md new file mode 100644 index 0000000000..c83bd20b3a --- /dev/null +++ b/.changeset/polite-keys-confess.md @@ -0,0 +1,12 @@ +--- +'@backstage/plugin-catalog-backend-module-msgraph': minor +--- + +Microsoft Graph plugin can supports many more options for authenticating with the Microsoft Graph API. +Previously only ClientId/ClientSecret was supported, but now all the authentication options of `DefaultAzureCredential` from `@azure/identity` are supported. +Including Managed Identity, Client Certificate, Azure CLI and VS Code. + +If `clientId` and `clientSecret` are specified in configuration, the plugin behaves the same way as before. +If these fields are omitted, the plugin uses `DefaultAzureCredential` to determine + +This is particularly useful for local development environments - the default configuration will try to use existing credentials from Visual Studio Code, Azure CLI and Azure PowerShell, without the user needing to configure any credentials in app-config.yaml diff --git a/app-config.yaml b/app-config.yaml index 9a3206026e..67a55582c8 100644 --- a/app-config.yaml +++ b/app-config.yaml @@ -216,6 +216,15 @@ catalog: - Domain - Location + providers: + microsoftGraphOrg: + default: + tenantId: ${AZURE_TENANT_ID} + user: + filter: accountEnabled eq true and userType eq 'member' + group: + filter: securityEnabled eq false and mailEnabled eq true and groupTypes/any(c:c+eq+'Unified') + processors: ldapOrg: ### Example for how to add your enterprise LDAP server diff --git a/plugins/catalog-backend-module-msgraph/README.md b/plugins/catalog-backend-module-msgraph/README.md index 6132d5abc6..d9f004af03 100644 --- a/plugins/catalog-backend-module-msgraph/README.md +++ b/plugins/catalog-backend-module-msgraph/README.md @@ -6,13 +6,17 @@ This provider is useful if you want to import users and groups from Azure Active ## Getting Started -1. Create or use an existing App registration in the [Microsoft Azure Portal](https://portal.azure.com/). +1. Choose your authentication method + + - If you have a + +1. Create or use an existing App registration or Managed Identity in the [Microsoft Azure Portal](https://portal.azure.com/). The App registration requires at least the API permissions `Group.Read.All`, `GroupMember.Read.All`, `User.Read` and `User.Read.All` for Microsoft Graph (if you still run into errors about insufficient privileges, add `Team.ReadBasic.All` and `TeamMember.Read.All` too). -2. Configure the entity provider: +1. Configure the entity provider: ```yaml # app-config.yaml @@ -159,3 +163,9 @@ export async function myGroupTransformer( }), ); ``` + +## Troubleshooting + +### Authentication Errors + +If you're having problems authenticating, take a look at (Troubleshooting Azure Identity Authentication Issues)[https://github.com/Azure/azure-sdk-for-js/blob/main/sdk/identity/identity/TROUBLESHOOTING.md] diff --git a/plugins/catalog-backend-module-msgraph/package.json b/plugins/catalog-backend-module-msgraph/package.json index 921d198b8b..7609b4c083 100644 --- a/plugins/catalog-backend-module-msgraph/package.json +++ b/plugins/catalog-backend-module-msgraph/package.json @@ -33,7 +33,7 @@ "start": "backstage-cli package start" }, "dependencies": { - "@azure/msal-node": "^1.1.0", + "@azure/identity": "^2.0.4", "@backstage/backend-tasks": "^0.3.3-next.2", "@backstage/catalog-model": "^1.1.0-next.2", "@backstage/config": "^1.0.1", @@ -43,9 +43,9 @@ "lodash": "^4.17.21", "node-fetch": "^2.6.7", "p-limit": "^3.0.2", + "qs": "^6.9.4", "uuid": "^8.0.0", - "winston": "^3.2.1", - "qs": "^6.9.4" + "winston": "^3.2.1" }, "devDependencies": { "@backstage/backend-common": "^0.14.1-next.2", diff --git a/plugins/catalog-backend-module-msgraph/src/microsoftGraph/client.test.ts b/plugins/catalog-backend-module-msgraph/src/microsoftGraph/client.test.ts index 5ff4fa4e0e..94502b9e7d 100644 --- a/plugins/catalog-backend-module-msgraph/src/microsoftGraph/client.test.ts +++ b/plugins/catalog-backend-module-msgraph/src/microsoftGraph/client.test.ts @@ -14,30 +14,26 @@ * limitations under the License. */ -import * as msal from '@azure/msal-node'; +import { TokenCredential } from '@azure/identity'; import { setupRequestMockHandlers } from '@backstage/backend-test-utils'; import { rest } from 'msw'; import { setupServer } from 'msw/node'; import { MicrosoftGraphClient } from './client'; describe('MicrosoftGraphClient', () => { - const confidentialClientApplication: jest.Mocked = - { - acquireTokenByClientCredential: jest.fn(), - } as any; + const tokenCredential: jest.Mocked = { + getToken: jest.fn(), + } as any; let client: MicrosoftGraphClient; const worker = setupServer(); setupRequestMockHandlers(worker); beforeEach(() => { - confidentialClientApplication.acquireTokenByClientCredential.mockResolvedValue( - { token: 'ACCESS_TOKEN' } as any, - ); - client = new MicrosoftGraphClient( - 'https://example.com', - confidentialClientApplication, - ); + tokenCredential.getToken.mockResolvedValue({ + token: 'ACCESS_TOKEN', + } as any); + client = new MicrosoftGraphClient('https://example.com', tokenCredential); }); afterEach(() => { @@ -55,12 +51,10 @@ describe('MicrosoftGraphClient', () => { expect(response.status).toBe(200); expect(await response.json()).toEqual({ value: 'example' }); - expect( - confidentialClientApplication.acquireTokenByClientCredential, - ).toBeCalledTimes(1); - expect( - confidentialClientApplication.acquireTokenByClientCredential, - ).toBeCalledWith({ scopes: ['https://graph.microsoft.com/.default'] }); + expect(tokenCredential.getToken).toBeCalledTimes(1); + expect(tokenCredential.getToken).toBeCalledWith( + 'https://graph.microsoft.com/.default', + ); }); it('should perform simple api request', async () => { diff --git a/plugins/catalog-backend-module-msgraph/src/microsoftGraph/client.ts b/plugins/catalog-backend-module-msgraph/src/microsoftGraph/client.ts index 20e791c5d5..26da0cb64b 100644 --- a/plugins/catalog-backend-module-msgraph/src/microsoftGraph/client.ts +++ b/plugins/catalog-backend-module-msgraph/src/microsoftGraph/client.ts @@ -14,7 +14,11 @@ * limitations under the License. */ -import * as msal from '@azure/msal-node'; +import { + TokenCredential, + DefaultAzureCredential, + ClientSecretCredential, +} from '@azure/identity'; import * as MicrosoftGraph from '@microsoft/microsoft-graph-types'; import fetch, { Response } from 'node-fetch'; import qs from 'qs'; @@ -77,25 +81,32 @@ export class MicrosoftGraphClient { * @param config - Configuration for Interacting with Graph API */ static create(config: MicrosoftGraphProviderConfig): MicrosoftGraphClient { - const clientConfig: msal.Configuration = { - auth: { - clientId: config.clientId, - clientSecret: config.clientSecret, - authority: `${config.authority}/${config.tenantId}`, - }, + const options = { + authorityHost: config.authority, + tenantId: config.tenantId, }; - const pca = new msal.ConfidentialClientApplication(clientConfig); - return new MicrosoftGraphClient(config.target, pca); + + const credential = + config.clientId && config.clientSecret + ? new ClientSecretCredential( + config.tenantId, + config.clientId, + config.clientSecret, + options, + ) + : new DefaultAzureCredential(options); + + return new MicrosoftGraphClient(config.target, credential); } /** * @param baseUrl - baseUrl of Graph API {@link MicrosoftGraphProviderConfig.target} - * @param pca - instance of `msal.ConfidentialClientApplication` that is used to acquire token for Graph API calls + * @param tokenCredential - instance of `TokenCredential` that is used to acquire token for Graph API calls * */ constructor( private readonly baseUrl: string, - private readonly pca: msal.ConfidentialClientApplication, + private readonly tokenCredential: TokenCredential, ) {} /** @@ -202,18 +213,18 @@ export class MicrosoftGraphClient { headers?: Record, ): Promise { // Make sure that we always have a valid access token (might be cached) - const token = await this.pca.acquireTokenByClientCredential({ - scopes: ['https://graph.microsoft.com/.default'], - }); + const token = await this.tokenCredential.getToken( + 'https://graph.microsoft.com/.default', + ); if (!token) { - throw new Error('Error while requesting token for Microsoft Graph'); + throw new Error('Failed to obtain token from Azure Identity'); } return await fetch(url, { headers: { ...headers, - Authorization: `Bearer ${token.accessToken}`, + Authorization: `Bearer ${token.token}`, }, }); } diff --git a/plugins/catalog-backend-module-msgraph/src/microsoftGraph/config.test.ts b/plugins/catalog-backend-module-msgraph/src/microsoftGraph/config.test.ts index 8239090249..8e2866df65 100644 --- a/plugins/catalog-backend-module-msgraph/src/microsoftGraph/config.test.ts +++ b/plugins/catalog-backend-module-msgraph/src/microsoftGraph/config.test.ts @@ -25,8 +25,6 @@ describe('readMicrosoftGraphConfig', () => { id: 'target', target: 'target', tenantId: 'tenantId', - clientId: 'clientId', - clientSecret: 'clientSecret', }, ], }; @@ -36,11 +34,6 @@ describe('readMicrosoftGraphConfig', () => { id: 'target', target: 'target', tenantId: 'tenantId', - clientId: 'clientId', - clientSecret: 'clientSecret', - authority: 'https://login.microsoftonline.com', - userFilter: undefined, - groupFilter: undefined, }, ]; expect(actual).toEqual(expected); @@ -72,7 +65,7 @@ describe('readMicrosoftGraphConfig', () => { tenantId: 'tenantId', clientId: 'clientId', clientSecret: 'clientSecret', - authority: 'https://login.example.com', + authority: 'https://login.example.com/', userExpand: 'manager', userFilter: 'accountEnabled eq true', groupExpand: 'member', @@ -87,12 +80,7 @@ describe('readMicrosoftGraphConfig', () => { const config = { providers: [ { - id: 'target', - target: 'target', tenantId: 'tenantId', - clientId: 'clientId', - clientSecret: 'clientSecret', - authority: 'https://login.example.com/', userFilter: 'accountEnabled eq true', userGroupMemberFilter: 'any', }, @@ -105,12 +93,7 @@ describe('readMicrosoftGraphConfig', () => { const config = { providers: [ { - id: 'target', - target: 'target', tenantId: 'tenantId', - clientId: 'clientId', - clientSecret: 'clientSecret', - authority: 'https://login.example.com/', userFilter: 'accountEnabled eq true', userGroupMemberSearch: 'any', }, @@ -118,6 +101,30 @@ describe('readMicrosoftGraphConfig', () => { }; expect(() => readMicrosoftGraphConfig(new ConfigReader(config))).toThrow(); }); + + it('should fail if clientId is set without clientSecret', () => { + const config = { + providers: [ + { + tenantId: 'tenantId', + clientId: 'clientId', + }, + ], + }; + expect(() => readMicrosoftGraphConfig(new ConfigReader(config))).toThrow(); + }); + + it('should fail if clientSecret is set without clientId', () => { + const config = { + providers: [ + { + tenantId: 'tenantId', + clientSecret: 'clientId', + }, + ], + }; + expect(() => readMicrosoftGraphConfig(new ConfigReader(config))).toThrow(); + }); }); describe('readProviderConfigs', () => { @@ -127,10 +134,7 @@ describe('readProviderConfigs', () => { providers: { microsoftGraphOrg: { customProviderId: { - target: 'target', tenantId: 'tenantId', - clientId: 'clientId', - clientSecret: 'clientSecret', }, }, }, @@ -140,11 +144,8 @@ describe('readProviderConfigs', () => { const expected = [ { id: 'customProviderId', - target: 'target', + target: 'https://graph.microsoft.com/v1.0', tenantId: 'tenantId', - clientId: 'clientId', - clientSecret: 'clientSecret', - authority: 'https://login.microsoftonline.com', }, ]; expect(actual).toEqual(expected); @@ -183,7 +184,7 @@ describe('readProviderConfigs', () => { tenantId: 'tenantId', clientId: 'clientId', clientSecret: 'clientSecret', - authority: 'https://login.example.com', + authority: 'https://login.example.com/', userExpand: 'manager', userFilter: 'accountEnabled eq true', groupExpand: 'member', @@ -200,11 +201,7 @@ describe('readProviderConfigs', () => { providers: { microsoftGraphOrg: { customProviderId: { - target: 'target', tenantId: 'tenantId', - clientId: 'clientId', - clientSecret: 'clientSecret', - authority: 'https://login.example.com/', user: { filter: 'accountEnabled eq true', }, @@ -225,11 +222,7 @@ describe('readProviderConfigs', () => { providers: { microsoftGraphOrg: { customProviderId: { - target: 'target', tenantId: 'tenantId', - clientId: 'clientId', - clientSecret: 'clientSecret', - authority: 'https://login.example.com/', user: { filter: 'accountEnabled eq true', }, @@ -243,4 +236,36 @@ describe('readProviderConfigs', () => { }; expect(() => readProviderConfigs(new ConfigReader(config))).toThrow(); }); + + it('should fail if clientId is set without clientSecret', () => { + const config = { + catalog: { + providers: { + microsoftGraphOrg: { + customProviderId: { + tenantId: 'tenantId', + clientId: 'id', + }, + }, + }, + }, + }; + expect(() => readProviderConfigs(new ConfigReader(config))).toThrow(); + }); + + it('should fail if clientSecret is set without clientId', () => { + const config = { + catalog: { + providers: { + microsoftGraphOrg: { + customProviderId: { + tenantId: 'tenantId', + clientSecret: 'clientSecret', + }, + }, + }, + }, + }; + expect(() => readProviderConfigs(new ConfigReader(config))).toThrow(); + }); }); diff --git a/plugins/catalog-backend-module-msgraph/src/microsoftGraph/config.ts b/plugins/catalog-backend-module-msgraph/src/microsoftGraph/config.ts index 30e3a9a4d2..f7675acf42 100644 --- a/plugins/catalog-backend-module-msgraph/src/microsoftGraph/config.ts +++ b/plugins/catalog-backend-module-msgraph/src/microsoftGraph/config.ts @@ -17,7 +17,6 @@ import { Config } from '@backstage/config'; import { trimEnd } from 'lodash'; -const DEFAULT_AUTHORITY = 'https://login.microsoftonline.com'; const DEFAULT_PROVIDER_ID = 'default'; const DEFAULT_TARGET = 'https://graph.microsoft.com/v1.0'; @@ -49,12 +48,14 @@ export type MicrosoftGraphProviderConfig = { tenantId: string; /** * The OAuth client ID to use for authenticating requests. + * If specified, ClientSecret must also be specified */ - clientId: string; + clientId?: string; /** * The OAuth client secret to use for authenticating requests. + * If specified, ClientId must also be specified */ - clientSecret: string; + clientSecret?: string; /** * The filter to apply to extract users. * @@ -131,14 +132,15 @@ export function readMicrosoftGraphConfig( const providerConfigs = config.getOptionalConfigArray('providers') ?? []; for (const providerConfig of providerConfigs) { - const target = trimEnd(providerConfig.getString('target'), '/'); + const target = trimEnd( + providerConfig.getOptionalString('target') ?? DEFAULT_TARGET, + '/', + ); + const authority = providerConfig.getOptionalString('authority'); - const authority = providerConfig.getOptionalString('authority') - ? trimEnd(providerConfig.getOptionalString('authority'), '/') - : DEFAULT_AUTHORITY; const tenantId = providerConfig.getString('tenantId'); - const clientId = providerConfig.getString('clientId'); - const clientSecret = providerConfig.getString('clientSecret'); + const clientId = providerConfig.getOptionalString('clientId'); + const clientSecret = providerConfig.getOptionalString('clientSecret'); const userExpand = providerConfig.getOptionalString('userExpand'); const userFilter = providerConfig.getOptionalString('userFilter'); @@ -173,6 +175,18 @@ export function readMicrosoftGraphConfig( throw new Error(`queryMode must be one of: basic, advanced`); } + if (clientId && !clientSecret) { + throw new Error( + `clientSecret must be provided when clientId is defined.`, + ); + } + + if (clientSecret && !clientId) { + throw new Error( + `clientId must be provided when clientSecret is defined.`, + ); + } + providers.push({ id: target, target, @@ -225,14 +239,11 @@ export function readProviderConfig( config.getOptionalString('target') ?? DEFAULT_TARGET, '/', ); - const authority = trimEnd( - config.getOptionalString('authority') ?? DEFAULT_AUTHORITY, - '/', - ); + const authority = config.getOptionalString('authority'); - const clientId = config.getString('clientId'); - const clientSecret = config.getString('clientSecret'); const tenantId = config.getString('tenantId'); + const clientId = config.getOptionalString('clientId'); + const clientSecret = config.getOptionalString('clientSecret'); const userExpand = config.getOptionalString('user.expand'); const userFilter = config.getOptionalString('user.filter'); @@ -260,6 +271,14 @@ export function readProviderConfig( ); } + if (clientId && !clientSecret) { + throw new Error(`clientSecret must be provided when clientId is defined.`); + } + + if (clientSecret && !clientId) { + throw new Error(`clientId must be provided when clientSecret is defined.`); + } + return { id, target, diff --git a/yarn.lock b/yarn.lock index 4393aedfb8..88993d0568 100644 --- a/yarn.lock +++ b/yarn.lock @@ -349,7 +349,7 @@ resolved "https://registry.npmjs.org/@azure/msal-common/-/msal-common-7.1.0.tgz#b77dbf9ae581f1ed254f81d56422e3cdd6664b32" integrity sha512-WyfqE5mY/rggjqvq0Q5DxLnA33KSb0vfsUjxa95rycFknI03L5GPYI4HTU9D+g0PL5TtsQGnV3xzAGq9BFCVJQ== -"@azure/msal-node@^1.1.0", "@azure/msal-node@^1.3.0": +"@azure/msal-node@^1.3.0": version "1.11.0" resolved "https://registry.npmjs.org/@azure/msal-node/-/msal-node-1.11.0.tgz#d8bd3f15c1f05bf806ba6f9479c48c2eddd6a98d" integrity sha512-KW/XEexfCrPzdYbjY7NVmhq9okZT3Jvck55CGXpz9W5asxeq3EtrP45p+ZXtQVEfko0YJdolpCNqWUyXvanWZg==