From 1fc84c8df87275942daf1eec07d460410fb3594b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Thu, 28 May 2026 17:24:37 +0200 Subject: [PATCH] Always request accountEnabled in $select MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Microsoft Graph API does NOT include accountEnabled in the default user projection — it requires an explicit $select. Without it, the client-side filterDisabledUsers check cannot work because the field is undefined in the response. Add a DEFAULT_USER_SELECT constant with all default Graph API user fields plus accountEnabled. ensureSelectContains now falls back to this list when no custom userSelect is configured. Co-Authored-By: Claude Opus 4.6 (1M context) Signed-off-by: Fredrik Adelöw --- .../src/microsoftGraph/read.test.ts | 21 ++++++++---- .../src/microsoftGraph/read.ts | 32 +++++++++++++++---- 2 files changed, 40 insertions(+), 13 deletions(-) diff --git a/plugins/catalog-backend-module-msgraph/src/microsoftGraph/read.test.ts b/plugins/catalog-backend-module-msgraph/src/microsoftGraph/read.test.ts index db10f65f9a..7c697fd837 100644 --- a/plugins/catalog-backend-module-msgraph/src/microsoftGraph/read.test.ts +++ b/plugins/catalog-backend-module-msgraph/src/microsoftGraph/read.test.ts @@ -137,6 +137,7 @@ describe('read microsoft graph', () => { expect(client.getUsers).toHaveBeenCalledWith( { filter: 'accountEnabled eq true', + select: expect.arrayContaining(['accountEnabled']), top: 999, }, undefined, @@ -186,6 +187,7 @@ describe('read microsoft graph', () => { expect(client.getUsers).toHaveBeenCalledWith( { filter: 'accountEnabled eq true', + select: expect.arrayContaining(['accountEnabled']), top: 999, }, 'advanced', @@ -231,6 +233,7 @@ describe('read microsoft graph', () => { { expand: 'manager', filter: 'accountEnabled eq true', + select: expect.arrayContaining(['accountEnabled']), top: 999, }, undefined, @@ -280,6 +283,7 @@ describe('read microsoft graph', () => { expect(client.getUsers).toHaveBeenCalledWith( { filter: 'accountEnabled eq true', + select: expect.arrayContaining(['accountEnabled']), top: 999, }, undefined, @@ -325,7 +329,7 @@ describe('read microsoft graph', () => { ]); }); - it('should not pass select when userSelect is not configured', async () => { + it('should request default fields including accountEnabled when userSelect is not configured', async () => { client.getUsers.mockImplementation(getExampleUsers); client.getUserPhotoWithSizeLimit.mockResolvedValue(undefined); @@ -334,7 +338,7 @@ describe('read microsoft graph', () => { }); expect(client.getUsers).toHaveBeenCalledWith( - { top: 999 }, + { select: expect.arrayContaining(['accountEnabled']), top: 999 }, undefined, undefined, undefined, @@ -411,7 +415,7 @@ describe('read microsoft graph', () => { expect(client.getGroupUserMembers).toHaveBeenCalledTimes(1); expect(client.getGroupUserMembers).toHaveBeenCalledWith( 'groupid', - { top: 999 }, + { select: expect.arrayContaining(['accountEnabled']), top: 999 }, undefined, undefined, ); @@ -477,7 +481,7 @@ describe('read microsoft graph', () => { ); }); - it('should not pass select when userSelect is not configured', async () => { + it('should request default fields including accountEnabled when userSelect is not configured', async () => { client.getGroups.mockImplementation(getExampleGroups); client.getGroupUserMembers.mockImplementation(getExampleUsers); client.getUserPhotoWithSizeLimit.mockResolvedValue(undefined); @@ -489,7 +493,7 @@ describe('read microsoft graph', () => { expect(client.getGroupUserMembers).toHaveBeenCalledWith( 'groupid', - { top: 999 }, + { select: expect.arrayContaining(['accountEnabled']), top: 999 }, undefined, undefined, ); @@ -544,7 +548,7 @@ describe('read microsoft graph', () => { expect(client.getGroupUserMembers).toHaveBeenCalledTimes(1); expect(client.getGroupUserMembers).toHaveBeenCalledWith( 'groupid', - { top: 999 }, + { select: expect.arrayContaining(['accountEnabled']), top: 999 }, 'advanced', undefined, ); @@ -604,6 +608,7 @@ describe('read microsoft graph', () => { 'groupid', { expand: 'manager', + select: expect.arrayContaining(['accountEnabled']), top: 999, }, undefined, @@ -1407,6 +1412,7 @@ describe('read microsoft graph', () => { expect(client.getUsers).toHaveBeenCalledWith( { filter: undefined, + select: expect.arrayContaining(['accountEnabled']), top: 999, }, undefined, @@ -1451,6 +1457,7 @@ describe('read microsoft graph', () => { { expand: 'manager', filter: 'accountEnabled eq true', + select: expect.arrayContaining(['accountEnabled']), top: 999, }, undefined, @@ -1493,6 +1500,7 @@ describe('read microsoft graph', () => { expect(client.getUsers).toHaveBeenCalledTimes(1); expect(client.getUsers).toHaveBeenCalledWith( { + select: expect.arrayContaining(['accountEnabled']), top: 999, }, undefined, @@ -1625,6 +1633,7 @@ describe('read microsoft graph', () => { expect(client.getUsers).toHaveBeenCalledTimes(1); expect(client.getUsers).toHaveBeenCalledWith( { + select: expect.arrayContaining(['accountEnabled']), top: 999, }, undefined, diff --git a/plugins/catalog-backend-module-msgraph/src/microsoftGraph/read.ts b/plugins/catalog-backend-module-msgraph/src/microsoftGraph/read.ts index fc6d78fea3..678c762c4a 100644 --- a/plugins/catalog-backend-module-msgraph/src/microsoftGraph/read.ts +++ b/plugins/catalog-backend-module-msgraph/src/microsoftGraph/read.ts @@ -42,21 +42,39 @@ import { LoggerService } from '@backstage/backend-plugin-api'; const PAGE_SIZE = 999; +// The default properties returned by the Microsoft Graph API for User +// objects when no $select is specified. accountEnabled is NOT included +// in this set — it requires an explicit $select. We always request it +// so that filterDisabledUsers can work. +// https://learn.microsoft.com/en-us/graph/api/user-list#optional-query-parameters +const DEFAULT_USER_SELECT = [ + 'accountEnabled', + 'businessPhones', + 'displayName', + 'givenName', + 'id', + 'jobTitle', + 'mail', + 'mobilePhone', + 'officeLocation', + 'preferredLanguage', + 'surname', + 'userPrincipalName', +]; + function ensureSelectContains( select: string[] | undefined, field: string, -): string[] | undefined { - if (!select) { - return undefined; - } +): string[] { + const base = select ?? DEFAULT_USER_SELECT; if ( - select.some( + base.some( s => s.toLocaleLowerCase('en-US') === field.toLocaleLowerCase('en-US'), ) ) { - return select; + return base; } - return [...select, field]; + return [...base, field]; } async function* filterDisabledUsers(