Address review: use === true, update docs, add tests

- Use accountEnabled === true instead of !== false (stricter,
  excludes users with unset accountEnabled)
- Update doc comments to mention client-side filtering and remove
  accountEnabled from the filter example
- Update changeset to mention userGroupMemberSearch exclusivity
- Add accountEnabled: true to test mocks
- Add regression tests for disabled user filtering

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Fredrik Adelöw <freben@gmail.com>

.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.

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;
           /**

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;
   /**

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,
           };
         }
       }

plugins/catalog-backend-module-msgraph/src/microsoftGraph/read.ts

@@ -57,7 +57,7 @@ async function* filterDisabledUsers(
   users: AsyncIterable<MicrosoftGraph.User>,
 ): AsyncIterable<MicrosoftGraph.User> {
   for await (const user of users) {
-    if (user.accountEnabled !== false) {
+    if (user.accountEnabled === true) {
       yield user;
     }
   }