diff --git a/.changeset/fix-msgraph-group-member-filter.md b/.changeset/fix-msgraph-group-member-filter.md index 73dc41b9b8..a56b30a9af 100644 --- a/.changeset/fix-msgraph-group-member-filter.md +++ b/.changeset/fix-msgraph-group-member-filter.md @@ -2,4 +2,4 @@ '@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. +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 (`accountEnabled !== true`) are now filtered client-side in both the `/users` and group members paths. The mutual exclusivity checks between `userFilter` and `userGroupMemberFilter`/`userGroupMemberSearch` have been restored. diff --git a/plugins/catalog-backend-module-msgraph/config.d.ts b/plugins/catalog-backend-module-msgraph/config.d.ts index bddec4b658..6553484bda 100644 --- a/plugins/catalog-backend-module-msgraph/config.d.ts +++ b/plugins/catalog-backend-module-msgraph/config.d.ts @@ -59,9 +59,11 @@ export interface Config { // they could also be configured "in code". /** - * The filter to apply to extract users. + * The filter to apply to extract users. Disabled users + * (accountEnabled !== true) are always filtered out client-side + * regardless of this setting. * - * E.g. "accountEnabled eq true and userType eq 'member'" + * E.g. "userType eq 'member'" */ userFilter?: string; /** diff --git a/plugins/catalog-backend-module-msgraph/src/microsoftGraph/config.ts b/plugins/catalog-backend-module-msgraph/src/microsoftGraph/config.ts index 5feabcb9b2..4f82bcd1f3 100644 --- a/plugins/catalog-backend-module-msgraph/src/microsoftGraph/config.ts +++ b/plugins/catalog-backend-module-msgraph/src/microsoftGraph/config.ts @@ -61,9 +61,11 @@ export type MicrosoftGraphProviderConfig = { */ clientSecret?: string; /** - * The filter to apply to extract users. + * The filter to apply to extract users. Disabled users (accountEnabled + * !== true) are always filtered out client-side regardless of this + * setting. * - * E.g. "accountEnabled eq true and userType eq 'member'" + * E.g. "userType eq 'member'" */ userFilter?: string; /** 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 74d31fd482..af21e0aa20 100644 --- a/plugins/catalog-backend-module-msgraph/src/microsoftGraph/read.test.ts +++ b/plugins/catalog-backend-module-msgraph/src/microsoftGraph/read.test.ts @@ -76,6 +76,7 @@ describe('read microsoft graph', () => { id: 'userid', displayName: 'User Name', mail: 'user.name@example.com', + accountEnabled: true, }; } async function* getExampleGroups() { @@ -354,6 +355,60 @@ describe('read microsoft graph', () => { ); }); + it('should skip disabled users fetched via group membership', async () => { + client.getGroups.mockImplementation(getExampleGroups); + client.getGroupUserMembers.mockImplementation(async function* () { + yield { + id: 'enabled-user', + displayName: 'Enabled User', + mail: 'enabled@example.com', + accountEnabled: true, + }; + yield { + id: 'disabled-user', + displayName: 'Disabled User', + mail: 'disabled@example.com', + accountEnabled: false, + }; + yield { + id: 'unset-user', + displayName: 'Unset User', + mail: 'unset@example.com', + }; + }); + client.getUserPhotoWithSizeLimit.mockResolvedValue(undefined); + + const { users } = await readMicrosoftGraphUsersInGroups(client, { + userGroupMemberFilter: 'securityEnabled eq true', + logger: mockServices.logger.mock(), + }); + + const names = users.map(u => u.metadata.name); + expect(names).toEqual(['enabled_example.com']); + }); + + it('should include accountEnabled in select when userSelect is set', async () => { + client.getGroups.mockImplementation(getExampleGroups); + client.getGroupUserMembers.mockImplementation(getExampleUsers); + client.getUserPhotoWithSizeLimit.mockResolvedValue(undefined); + + await readMicrosoftGraphUsersInGroups(client, { + userGroupMemberFilter: 'securityEnabled eq true', + userSelect: ['id', 'displayName', 'mail'], + logger: mockServices.logger.mock(), + }); + + expect(client.getGroupUserMembers).toHaveBeenCalledWith( + 'groupid', + { + select: ['id', 'displayName', 'mail', 'accountEnabled'], + top: 999, + }, + undefined, + undefined, + ); + }); + it('should read users from Groups with advanced query mode', async () => { client.getGroups.mockImplementation(getExampleGroups); client.getGroupUserMembers.mockImplementation(getExampleUsers); @@ -1232,6 +1287,7 @@ describe('read microsoft graph', () => { async function* getExampleUsersEmail() { yield { mail: 'user.name@example.com', + accountEnabled: true, }; } @@ -1460,6 +1516,7 @@ describe('read microsoft graph', () => { id: `userid-${i}`, displayName: 'User Name', mail: 'user.name@example.com', + accountEnabled: true, }; } } diff --git a/plugins/catalog-backend-module-msgraph/src/microsoftGraph/read.ts b/plugins/catalog-backend-module-msgraph/src/microsoftGraph/read.ts index 20e48d5388..efb1f737ad 100644 --- a/plugins/catalog-backend-module-msgraph/src/microsoftGraph/read.ts +++ b/plugins/catalog-backend-module-msgraph/src/microsoftGraph/read.ts @@ -57,7 +57,7 @@ async function* filterDisabledUsers( users: AsyncIterable, ): AsyncIterable { for await (const user of users) { - if (user.accountEnabled !== false) { + if (user.accountEnabled === true) { yield user; } }