diff --git a/plugins/catalog-backend-module-msgraph-incremental/src/MicrosoftGraphIncrementalEntityProvider.test.ts b/plugins/catalog-backend-module-msgraph-incremental/src/MicrosoftGraphIncrementalEntityProvider.test.ts index 7e65d8a728..954c8d43c0 100644 --- a/plugins/catalog-backend-module-msgraph-incremental/src/MicrosoftGraphIncrementalEntityProvider.test.ts +++ b/plugins/catalog-backend-module-msgraph-incremental/src/MicrosoftGraphIncrementalEntityProvider.test.ts @@ -205,6 +205,25 @@ describe('MicrosoftGraphIncrementalEntityProvider', () => { ), ); }); + + it('warns when groupIncludeSubGroups is set', async () => { + const provider = new MicrosoftGraphIncrementalEntityProvider({ + id: 'default', + provider: { + ...baseProviderConfig, + groupIncludeSubGroups: true, + } as any, + logger, + }); + + await provider.around(async () => {}); + + expect(logger.warn).toHaveBeenCalledWith( + expect.stringContaining( + 'groupIncludeSubGroups is not supported', + ), + ); + }); }); describe('next — users phase', () => { @@ -416,6 +435,29 @@ describe('MicrosoftGraphIncrementalEntityProvider', () => { expect(mockGetUserPhotoGated).toHaveBeenCalledWith(mockClient, 'u1', 120); }); + it('skips photo fetch when user has no id to avoid requesting users/undefined/photo', async () => { + mockRequestOnePage.mockResolvedValueOnce({ + items: [ + { + // No id field — Graph can theoretically omit it + displayName: 'No-ID User', + userPrincipalName: 'noid@example.com', + }, + ], + nextLink: undefined, + }); + + const provider = new MicrosoftGraphIncrementalEntityProvider({ + id: 'default', + provider: baseProviderConfig as any, + logger, + }); + + await provider.next(makeContext()); + + expect(mockGetUserPhotoGated).not.toHaveBeenCalled(); + }); + it('continues processing remaining users when a photo load fails and logs the error', async () => { mockRequestOnePage.mockResolvedValueOnce({ items: [ @@ -640,6 +682,50 @@ describe('MicrosoftGraphIncrementalEntityProvider', () => { expect(parentEntity!.entity.spec?.children).toHaveLength(1); }); + it('omits child group refs when groupFilter is active to avoid dangling references', async () => { + const providerWithFilter = { + ...baseProviderConfig, + groupFilter: 'securityEnabled eq true', + }; + (mockClient.getOrganization as jest.Mock).mockResolvedValue({ + id: 'org-id', + displayName: 'My Org', + }); + mockRequestOnePage.mockResolvedValueOnce({ + items: [ + { id: 'grp-parent', displayName: 'Parent', mail: 'parent@example.com' }, + ], + nextLink: undefined, + }); + (mockClient.getGroupMembers as jest.Mock).mockReturnValue( + asyncYield({ + '@odata.type': '#microsoft.graph.group', + id: 'grp-child', + displayName: 'Child Group', + mail: 'child@example.com', + }), + ); + + const provider = new MicrosoftGraphIncrementalEntityProvider({ + id: 'default', + provider: providerWithFilter as any, + logger, + }); + + const result = await provider.next( + { client: mockClient, provider: providerWithFilter as any }, + groupsCursor, + ); + + const parentEntity = result.entities!.find( + e => + e.entity.metadata.annotations?.[ + MICROSOFT_GRAPH_GROUP_ID_ANNOTATION + ] === 'grp-parent', + ); + expect(parentEntity!.entity.spec?.children).toHaveLength(0); + }); + it('returns done:false with groups cursor when nextLink is present', async () => { const nextLink = 'https://graph.microsoft.com/v1.0/groups?$skiptoken=page2'; diff --git a/plugins/catalog-backend-module-msgraph-incremental/src/MicrosoftGraphIncrementalEntityProvider.ts b/plugins/catalog-backend-module-msgraph-incremental/src/MicrosoftGraphIncrementalEntityProvider.ts index 642cf88f44..dc333a924f 100644 --- a/plugins/catalog-backend-module-msgraph-incremental/src/MicrosoftGraphIncrementalEntityProvider.ts +++ b/plugins/catalog-backend-module-msgraph-incremental/src/MicrosoftGraphIncrementalEntityProvider.ts @@ -274,6 +274,14 @@ export class MicrosoftGraphIncrementalEntityProvider ); } + if (provider.groupIncludeSubGroups) { + this.options.logger.warn( + `${this.getProviderName()}: groupIncludeSubGroups is not supported by ` + + `MicrosoftGraphIncrementalEntityProvider and will be ignored. ` + + `Switch to MicrosoftGraphOrgEntityProvider if you require this option.`, + ); + } + const client = MicrosoftGraphClient.create(provider); await burst({ client, provider }); } @@ -321,9 +329,9 @@ export class MicrosoftGraphIncrementalEntityProvider rawUsers.map(user => limiter(async () => { let userPhoto: string | undefined; - if (provider.loadUserPhotos !== false) { + if (user.id && provider.loadUserPhotos !== false) { try { - userPhoto = await getUserPhotoGated(client, user.id!, 120); + userPhoto = await getUserPhotoGated(client, user.id, 120); } catch (e) { this.options.logger.debug( `${this.getProviderName()}: failed to load photo for user ${ @@ -469,14 +477,19 @@ export class MicrosoftGraphIncrementalEntityProvider ); } } else if (member['@odata.type'] === '#microsoft.graph.group') { - const childEntity = await groupTransformer( - member as MicrosoftGraph.Group, - ); - if (childEntity) { - childEntity.metadata.name = capEntityName( - childEntity.metadata.name, + // Only emit child refs when no group filter/search is active. + // With a filter, child groups may not be ingested themselves, + // which would produce dangling spec.children references. + if (!provider.groupFilter && !provider.groupSearch) { + const childEntity = await groupTransformer( + member as MicrosoftGraph.Group, ); - childRefs.push(stringifyEntityRef(childEntity)); + if (childEntity) { + childEntity.metadata.name = capEntityName( + childEntity.metadata.name, + ); + childRefs.push(stringifyEntityRef(childEntity)); + } } } }