From 39f36a944251d056362702622d82eacd6ff54bb6 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 22 Jul 2024 14:06:27 +0200 Subject: [PATCH] auth-backend-module-microsoft-provider: update scope handling to define required scopes in backend Signed-off-by: Patrik Oldsberg --- .changeset/new-scissors-try.md | 5 ++ docs/auth/microsoft/provider.md | 4 +- .../src/authenticator.ts | 20 ++++-- .../src/module.test.ts | 65 ++++++++++++++++++- 4 files changed, 83 insertions(+), 11 deletions(-) create mode 100644 .changeset/new-scissors-try.md diff --git a/.changeset/new-scissors-try.md b/.changeset/new-scissors-try.md new file mode 100644 index 0000000000..a9e9a45ce7 --- /dev/null +++ b/.changeset/new-scissors-try.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-auth-backend-module-microsoft-provider': patch +--- + +Updated the Microsoft authenticator to accurately define required scopes, but to also omit the required and additional scopes when requesting resource scopes. diff --git a/docs/auth/microsoft/provider.md b/docs/auth/microsoft/provider.md index f799eeb627..60104b99b7 100644 --- a/docs/auth/microsoft/provider.md +++ b/docs/auth/microsoft/provider.md @@ -67,8 +67,6 @@ auth: clientSecret: ${AZURE_CLIENT_SECRET} tenantId: ${AZURE_TENANT_ID} domainHint: ${AZURE_TENANT_ID} - additionalScopes: - - Mail.Send signIn: resolvers: # typically you would pick one of these @@ -86,7 +84,7 @@ The Microsoft provider is a structure with three mandatory configuration keys: Leave blank if your app registration is multi tenant. When specified, this reduces login friction for users with accounts in multiple tenants by automatically filtering away accounts from other tenants. For more details, see [Home Realm Discovery](https://learn.microsoft.com/en-us/azure/active-directory/manage-apps/home-realm-discovery-policy) -- `additionalScopes` (optional): List of scopes for the App Registration. The default and mandatory value is ['user.read']. +- `additionalScopes` (optional): List of scopes for the App Registration, to be requested in addition to the required ones. ### Resolvers diff --git a/plugins/auth-backend-module-microsoft-provider/src/authenticator.ts b/plugins/auth-backend-module-microsoft-provider/src/authenticator.ts index 1b6eb87fae..fe7dc8f7e1 100644 --- a/plugins/auth-backend-module-microsoft-provider/src/authenticator.ts +++ b/plugins/auth-backend-module-microsoft-provider/src/authenticator.ts @@ -21,21 +21,30 @@ import { PassportProfile, } from '@backstage/plugin-auth-node'; import { ExtendedMicrosoftStrategy } from './strategy'; -import { union } from 'lodash'; /** @public */ export const microsoftAuthenticator = createOAuthAuthenticator({ defaultProfileTransform: PassportOAuthAuthenticatorHelper.defaultProfileTransform, + scopes: { + required: ['email', 'openid', 'offline_access', 'user.read'], + transform({ requested, granted, required, additional }) { + // Resources scopes are of the form `/`, and are handled + // separately from the normal scopes in the client. When request a + // resource scope we should only include forward the request scope along + // with offline_access. + const hasResourceScope = Array.from(requested).some(s => s.includes('/')); + if (hasResourceScope) { + return [...requested, 'offline_access']; + } + return [...requested, ...granted, ...required, ...additional]; + }, + }, initialize({ callbackUrl, config }) { const clientId = config.getString('clientId'); const clientSecret = config.getString('clientSecret'); const tenantId = config.getString('tenantId'); const domainHint = config.getOptionalString('domainHint'); - const scope = union( - ['user.read'], - config.getOptionalStringArray('additionalScopes'), - ); const helper = PassportOAuthAuthenticatorHelper.from( new ExtendedMicrosoftStrategy( @@ -44,7 +53,6 @@ export const microsoftAuthenticator = createOAuthAuthenticator({ clientSecret: clientSecret, callbackURL: callbackUrl, tenant: tenantId, - scope: scope, }, ( accessToken: string, diff --git a/plugins/auth-backend-module-microsoft-provider/src/module.test.ts b/plugins/auth-backend-module-microsoft-provider/src/module.test.ts index b50956b9eb..a072b99bde 100644 --- a/plugins/auth-backend-module-microsoft-provider/src/module.test.ts +++ b/plugins/auth-backend-module-microsoft-provider/src/module.test.ts @@ -67,7 +67,7 @@ describe('authModuleMicrosoftProvider', () => { expect(startUrl.pathname).toBe('/my-tenant-id/oauth2/v2.0/authorize'); expect(Object.fromEntries(startUrl.searchParams)).toEqual({ response_type: 'code', - scope: 'User.Read.All', + scope: 'email openid offline_access user.read User.Read.All', client_id: 'my-client-id', redirect_uri: `http://localhost:${server.port()}/api/auth/microsoft/handler/frame`, state: expect.any(String), @@ -97,6 +97,7 @@ describe('authModuleMicrosoftProvider', () => { clientSecret: 'another-client-secret', tenantId: 'another-tenant-id', domainHint: 'somedomain', + additionalScopes: ['some-extra-scope'], }, }, }, @@ -125,7 +126,7 @@ describe('authModuleMicrosoftProvider', () => { expect(startUrl.pathname).toBe('/another-tenant-id/oauth2/v2.0/authorize'); expect(Object.fromEntries(startUrl.searchParams)).toEqual({ response_type: 'code', - scope: 'user.read', + scope: 'email openid offline_access user.read some-extra-scope', client_id: 'another-client-id', redirect_uri: `http://localhost:${server.port()}/api/auth/microsoft/handler/frame`, state: expect.any(String), @@ -137,4 +138,64 @@ describe('authModuleMicrosoftProvider', () => { nonce: decodeURIComponent(nonceCookie.value), }); }); + + it('should not include required scopes for resources', async () => { + const { server } = await startTestBackend({ + features: [ + authPlugin, + authModuleMicrosoftProvider, + mockServices.rootConfig.factory({ + data: { + app: { + baseUrl: 'http://localhost:3000', + }, + auth: { + providers: { + microsoft: { + development: { + clientId: 'another-client-id', + clientSecret: 'another-client-secret', + tenantId: 'another-tenant-id', + additionalScopes: ['some-extra-scope'], + }, + }, + }, + }, + }, + }), + ], + }); + + const agent = request.agent(server); + + const res = await agent.get( + '/api/auth/microsoft/start?env=development&scope=some-resource/some-scope', + ); + + expect(res.status).toEqual(302); + + const nonceCookie = agent.jar.getCookie('microsoft-nonce', { + domain: 'localhost', + path: '/api/auth/microsoft/handler', + script: false, + secure: false, + }); + expect(nonceCookie).toBeDefined(); + + const startUrl = new URL(res.get('location')); + expect(startUrl.origin).toBe('https://login.microsoftonline.com'); + expect(startUrl.pathname).toBe('/another-tenant-id/oauth2/v2.0/authorize'); + expect(Object.fromEntries(startUrl.searchParams)).toEqual({ + response_type: 'code', + scope: 'some-resource/some-scope offline_access', + client_id: 'another-client-id', + redirect_uri: `http://localhost:${server.port()}/api/auth/microsoft/handler/frame`, + state: expect.any(String), + }); + + expect(decodeOAuthState(startUrl.searchParams.get('state')!)).toEqual({ + env: 'development', + nonce: decodeURIComponent(nonceCookie.value), + }); + }); });