From 779d7a230442fe1abc70f2359c02f3d1902767ee Mon Sep 17 00:00:00 2001
From: Tim Jacomb <tim.jacomb@hmcts.net>
Date: Thu, 4 Nov 2021 17:07:18 +0000
Subject: [PATCH] Tweak logic for msgraph catalog ingesting for display names
 with security groups

Signed-off-by: Tim Jacomb <tim.jacomb@hmcts.net>
---
 .changeset/early-trains-hammer.md             |  7 ++
 .../src/microsoftGraph/read.test.ts           | 86 +++++++++++++++++++
 .../src/microsoftGraph/read.ts                |  9 +-
 3 files changed, 101 insertions(+), 1 deletion(-)
 create mode 100644 .changeset/early-trains-hammer.md

diff --git a/.changeset/early-trains-hammer.md b/.changeset/early-trains-hammer.md
new file mode 100644
index 0000000000..853788e015
--- /dev/null
+++ b/.changeset/early-trains-hammer.md
@@ -0,0 +1,7 @@
+---
+'@backstage/plugin-catalog-backend-module-msgraph': patch
+---
+
+Tweak logic for msgraph catalog ingesting for display names with security groups
+
+Previously security groups that weren't mail enabled were imported with UUIDs, now they use the display name.
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 fa86d77b27..b0273e1f58 100644
--- a/plugins/catalog-backend-module-msgraph/src/microsoftGraph/read.test.ts
+++ b/plugins/catalog-backend-module-msgraph/src/microsoftGraph/read.test.ts
@@ -445,6 +445,92 @@ describe('read microsoft graph', () => {
       // expect(client.getGroupPhotoWithSizeLimit).toBeCalledTimes(1);
       // expect(client.getGroupPhotoWithSizeLimit).toBeCalledWith('groupid', 120);
     });
+
+    it('should read security groups', async () => {
+      async function* getExampleGroups() {
+        yield {
+          id: 'groupid',
+          displayName: 'Group Name',
+          description: 'Group Description',
+          mail: 'group@example.com',
+          mailNickname: 'df546d53-4f5f-4462-b371-d4a855787047',
+          mailEnabled: false,
+          securityEnabled: true,
+        };
+      }
+
+      async function* getExampleGroupMembers(): AsyncIterable<GroupMember> {
+        yield {
+          '@odata.type': '#microsoft.graph.group',
+          id: 'childgroupid',
+        };
+        yield {
+          '@odata.type': '#microsoft.graph.user',
+          id: 'userid',
+        };
+      }
+
+      client.getGroups.mockImplementation(getExampleGroups);
+      client.getGroupMembers.mockImplementation(getExampleGroupMembers);
+      client.getOrganization.mockResolvedValue({
+        id: 'tenantid',
+        displayName: 'Organization Name',
+      });
+      client.getGroupPhotoWithSizeLimit.mockResolvedValue(
+        'data:image/jpeg;base64,...',
+      );
+
+      const { groups, rootGroup } = await readMicrosoftGraphGroups(
+        client,
+        'tenantid',
+        {
+          groupFilter: 'securityEnabled eq true',
+        },
+      );
+
+      const expectedRootGroup = group({
+        metadata: {
+          annotations: {
+            'graph.microsoft.com/tenant-id': 'tenantid',
+          },
+          name: 'organization_name',
+          description: 'Organization Name',
+        },
+        spec: {
+          type: 'root',
+          profile: {
+            displayName: 'Organization Name',
+          },
+          children: [],
+        },
+      });
+      expect(groups).toEqual([
+        expectedRootGroup,
+        group({
+          metadata: {
+            annotations: {
+              'graph.microsoft.com/group-id': 'groupid',
+            },
+            name: 'group_name',
+            description: 'Group Description',
+          },
+          spec: {
+            type: 'team',
+            profile: {
+              displayName: 'Group Name',
+              email: 'group@example.com',
+            },
+            children: [],
+          },
+        }),
+      ]);
+      expect(rootGroup).toEqual(expectedRootGroup);
+      expect(client.getGroups).toBeCalledWith({
+        filter: 'securityEnabled eq true',
+      });
+      expect(client.getGroupMembers).toBeCalledTimes(1);
+      expect(client.getGroupMembers).toBeCalledWith('groupid');
+    });
   });
 
   describe('resolveRelations', () => {
diff --git a/plugins/catalog-backend-module-msgraph/src/microsoftGraph/read.ts b/plugins/catalog-backend-module-msgraph/src/microsoftGraph/read.ts
index fa229ca956..77bea268ee 100644
--- a/plugins/catalog-backend-module-msgraph/src/microsoftGraph/read.ts
+++ b/plugins/catalog-backend-module-msgraph/src/microsoftGraph/read.ts
@@ -251,6 +251,13 @@ export async function readMicrosoftGraphOrganization(
   return { rootGroup };
 }
 
+function extractGroupName(group: MicrosoftGraph.Group): string {
+  if (group.securityEnabled && !group.mailEnabled) {
+    return group.displayName as string;
+  }
+  return (group.mailNickname || group.displayName) as string;
+}
+
 export async function defaultGroupTransformer(
   group: MicrosoftGraph.Group,
   groupPhoto?: string,
@@ -259,7 +266,7 @@ export async function defaultGroupTransformer(
     return undefined;
   }
 
-  const name = normalizeEntityName(group.mailNickname || group.displayName);
+  const name = normalizeEntityName(extractGroupName(group));
   const entity: GroupEntity = {
     apiVersion: 'backstage.io/v1alpha1',
     kind: 'Group',