paulononato/backstage
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

fix(catalog-backend-module-msgraph): filter disabled users client-side

Browse Source 
Revert the server-side accountEnabled base filter from #34165 which
broke the group members endpoint. Filter disabled users client-side
in both the /users and group members paths. Restore the mutual
exclusivity check between userFilter and userGroupMemberFilter.

Fixes #34422

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Fredrik Adelöw <freben@gmail.com>
This commit is contained in:
Fredrik Adelöw
2026-05-28 15:24:48 +02:00
parent 3346845548
commit 8930d77157
7 changed files with 88 additions and 65 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.changeset/fix-msgraph-group-member-filter.md
+5
View File
@@ -0,0 +1,5 @@
---
'@backstage/plugin-catalog-backend-module-msgraph': patch
---
Reverted the server-side `accountEnabled eq true` base filter that was added in v1.51.0, which broke the `userGroupMember` path because the group members endpoint doesn't support `$filter` on that property. Disabled users are now filtered client-side in both the `/users` and group members paths. The mutual exclusivity check between `userFilter` and `userGroupMemberFilter` has been restored.
.patches/pr-34425.txt
+1
View File
@@ -0,0 +1 @@
Fix msgraph userGroupMember filter error by filtering disabled users client-side
plugins/catalog-backend-module-msgraph/config.d.ts
Vendored
+1 -2
View File
@@ -60,9 +60,8 @@ export interface Config {
/**
* The filter to apply to extract users.
* Combined with the base `accountEnabled eq true` filter.
*
* E.g. "userType eq 'member'"
* E.g. "accountEnabled eq true and userType eq 'member'"
*/
userFilter?: string;
/**
plugins/catalog-backend-module-msgraph/src/microsoftGraph/config.test.ts
+4 -25
View File
@@ -34,7 +34,7 @@ describe('readMicrosoftGraphConfig', () => {
id: 'target',
target: 'target',
tenantId: 'tenantId',
userFilter: 'accountEnabled eq true',
userFilter: undefined,
},
];
expect(actual).toEqual(expected);
@@ -69,7 +69,7 @@ describe('readMicrosoftGraphConfig', () => {
clientSecret: 'clientSecret',
authority: 'https://login.example.com/',
userExpand: 'manager',
userFilter: "accountEnabled eq true and (userType eq 'member')",
userFilter: "userType eq 'member'",
userSelect: ['id', 'displayName', 'department'],
groupExpand: 'member',
groupSelect: ['id', 'displayName', 'description'],
@@ -123,7 +123,7 @@ describe('readProviderConfigs', () => {
id: 'customProviderId',
target: 'https://graph.microsoft.com/v1.0',
tenantId: 'tenantId',
userFilter: 'accountEnabled eq true',
userFilter: undefined,
userPath: 'users',
groupPath: 'groups',
},
@@ -178,7 +178,7 @@ describe('readProviderConfigs', () => {
authority: 'https://login.example.com/',
queryMode: 'advanced',
userExpand: 'manager',
userFilter: "accountEnabled eq true and (userType eq 'member')",
userFilter: "userType eq 'member'",
userSelect: ['id', 'displayName', 'department'],
userPath: '/groups/{groupId}/members',
groupExpand: 'member',
@@ -197,27 +197,6 @@ describe('readProviderConfigs', () => {
expect(actual).toEqual(expected);
});
it('should combine custom filter with accountEnabled filter by default', () => {
const config = {
catalog: {
providers: {
microsoftGraphOrg: {
customProviderId: {
tenantId: 'tenantId',
user: {
filter: "userType eq 'member'",
},
},
},
},
},
};
const [result] = readProviderConfigs(new ConfigReader(config));
expect(result.userFilter).toBe(
"accountEnabled eq true and (userType eq 'member')",
);
});
it('should fail if clientId is set without clientSecret', () => {
const config = {
catalog: {
plugins/catalog-backend-module-msgraph/src/microsoftGraph/config.ts
+26 -15
View File
@@ -61,10 +61,9 @@ export type MicrosoftGraphProviderConfig = {
*/
clientSecret?: string;
/**
* The filter to apply to extract users. This is combined with the base
* `accountEnabled eq true` filter that is always applied automatically.
* The filter to apply to extract users.
*
* E.g. "userType eq 'member'"
* E.g. "accountEnabled eq true and userType eq 'member'"
*/
userFilter?: string;
/**
@@ -193,9 +192,7 @@ export function readMicrosoftGraphConfig(
const clientSecret = providerConfig.getOptionalString('clientSecret');
const userExpand = providerConfig.getOptionalString('userExpand');
const userFilter = buildUserFilter(
providerConfig.getOptionalString('userFilter'),
);
const userFilter = providerConfig.getOptionalString('userFilter');
const userSelect = providerConfig.getOptionalStringArray('userSelect');
const userGroupMemberFilter = providerConfig.getOptionalString(
'userGroupMemberFilter',
@@ -207,6 +204,17 @@ export function readMicrosoftGraphConfig(
const groupFilter = providerConfig.getOptionalString('groupFilter');
const groupSearch = providerConfig.getOptionalString('groupSearch');
if (userFilter && userGroupMemberFilter) {
throw new Error(
`userFilter and userGroupMemberFilter are mutually exclusive, only one can be specified.`,
);
}
if (userFilter && userGroupMemberSearch) {
throw new Error(
`userGroupMemberSearch cannot be specified when userFilter is defined.`,
);
}
const groupSelect = providerConfig.getOptionalStringArray('groupSelect');
const queryMode = providerConfig.getOptionalString('queryMode');
if (
@@ -304,7 +312,7 @@ export function readProviderConfig(
const clientSecret = config.getOptionalString('clientSecret');
const userExpand = config.getOptionalString('user.expand');
const userFilter = buildUserFilter(config.getOptionalString('user.filter'));
const userFilter = config.getOptionalString('user.filter');
const userSelect = config.getOptionalStringArray('user.select');
const userPath = config.getOptionalString('user.path') ?? 'users';
const loadUserPhotos = config.getOptionalBoolean('user.loadPhotos');
@@ -335,6 +343,17 @@ export function readProviderConfig(
);
const userGroupMemberPath = config.getOptionalString('userGroupMember.path');
if (userFilter && userGroupMemberFilter) {
throw new Error(
`userFilter and userGroupMemberFilter are mutually exclusive, only one can be specified.`,
);
}
if (userFilter && userGroupMemberSearch) {
throw new Error(
`userGroupMemberSearch cannot be specified when userFilter is defined.`,
);
}
if (clientId && !clientSecret) {
throw new Error(`clientSecret must be provided when clientId is defined.`);
}
@@ -374,11 +393,3 @@ export function readProviderConfig(
schedule,
};
}
function buildUserFilter(rawFilter: string | undefined): string {
const base = 'accountEnabled eq true';
if (rawFilter) {
return `${base} and (${rawFilter})`;
}
return base;
}
plugins/catalog-backend-module-msgraph/src/microsoftGraph/read.test.ts
+1 -1
View File
@@ -1390,7 +1390,7 @@ describe('read microsoft graph', () => {
expect(client.getUsers).toHaveBeenCalledTimes(1);
expect(client.getUsers).toHaveBeenCalledWith(
{
select: ['mail'],
select: ['mail', 'accountEnabled'],
top: 999,
},
undefined,
plugins/catalog-backend-module-msgraph/src/microsoftGraph/read.ts
+50 -22
View File
@@ -42,6 +42,27 @@ import { LoggerService } from '@backstage/backend-plugin-api';
const PAGE_SIZE = 999;
function ensureSelectContains(select: string[], field: string): string[] {
if (
select.some(
s => s.toLocaleLowerCase('en-US') === field.toLocaleLowerCase('en-US'),
)
) {
return select;
}
return [...select, field];
}
async function* filterDisabledUsers(
users: AsyncIterable<MicrosoftGraph.User>,
): AsyncIterable<MicrosoftGraph.User> {
for await (const user of users) {
if (user.accountEnabled !== false) {
yield user;
}
}
}
export async function readMicrosoftGraphUsers(
client: MicrosoftGraphClient,
options: {
@@ -58,16 +79,21 @@ export async function readMicrosoftGraphUsers(
): Promise<{
users: UserEntity[]; // With all relations empty
}> {
const users = client.getUsers(
{
filter: options.userFilter,
expand: options.userExpand,
select: options.userSelect,
top: PAGE_SIZE,
},
options.queryMode,
options.userPath,
options.signal,
const select = options.userSelect
? ensureSelectContains(options.userSelect, 'accountEnabled')
: undefined;
const users = filterDisabledUsers(
client.getUsers(
{
filter: options.userFilter,
expand: options.userExpand,
select,
top: PAGE_SIZE,
},
options.queryMode,
options.userPath,
options.signal,
),
);
return {
@@ -86,7 +112,6 @@ export async function readMicrosoftGraphUsersInGroups(
options: {
queryMode?: 'basic' | 'advanced';
userExpand?: string;
userFilter?: string;
userSelect?: string[];
loadUserPhotos?: boolean;
userGroupMemberSearch?: string;
@@ -121,16 +146,20 @@ export async function readMicrosoftGraphUsersInGroups(
userGroupMemberPromises.push(
limiter(async () => {
let groupMemberCount = 0;
for await (const user of client.getGroupUserMembers(
group.id!,
{
expand: options.userExpand,
filter: options.userFilter,
select: options.userSelect,
top: PAGE_SIZE,
},
options.queryMode,
options.signal,
const memberSelect = options.userSelect
? ensureSelectContains(options.userSelect, 'accountEnabled')
: undefined;
for await (const user of filterDisabledUsers(
client.getGroupUserMembers(
group.id!,
{
expand: options.userExpand,
select: memberSelect,
top: PAGE_SIZE,
},
options.queryMode,
options.signal,
),
)) {
userGroupMembers.set(user.id!, user);
groupMemberCount++;
@@ -455,7 +484,6 @@ export async function readMicrosoftGraphOrg(
{
queryMode: options.queryMode,
userExpand: options.userExpand,
userFilter: options.userFilter,
userSelect: options.userSelect,
userGroupMemberFilter: options.userGroupMemberFilter,
userGroupMemberSearch: options.userGroupMemberSearch,