Merge pull request #3622 from SDA-SE/feat/group-photo

Add optional profile section to group entities

.changeset/dirty-melons-retire.md

@@ -0,0 +1,6 @@
+---
+'@backstage/catalog-model': patch
+---
+
+Introduce a `profile` section for group entities that can optional include a
+`displayName`, `email` and `picture`.

.changeset/dirty-wolves-wink.md

@@ -0,0 +1,7 @@
+---
+'@backstage/plugin-catalog-backend': patch
+---
+
+Support `profile` of groups including `displayName`, `email`, and `picture` in
+`LdapOrgReaderProcessor`. The source fields for them can be configured in the
+`ldapOrg` provider.

.changeset/odd-mugs-build.md

@@ -0,0 +1,6 @@
+---
+'@backstage/plugin-catalog-backend': patch
+---
+
+Support `profile` of groups including `displayName` and `picture` in
+`GithubOrgReaderProcessor`. Fixes the import of `description` for groups.

.changeset/tender-coats-unite.md

@@ -0,0 +1,11 @@
+---
+'@backstage/plugin-org': patch
+---
+
+Display the new `profile` fields (`displayName`, `email`, and `picture`) for
+groups on the `GroupProfileCard`.
+
+![Groups Profile Section](./groups_profile.png)
+
+This also resolves some cases where `profile` fields are missing for users or
+groups and for example falls back to displaying the entity name. Adds additional test data to the ACME Corp dataset.

.changeset/witty-scissors-divide.md

@@ -0,0 +1,7 @@
+---
+'@backstage/plugin-catalog-backend': patch
+---
+
+Support `profile` of groups including `displayName` and `email` in
+`MicrosoftGraphOrgReaderProcessor`. Importing `picture` doesn't work yet, as
+the Microsoft Graph API does not expose them correctly.

docs/features/software-catalog/descriptor-format.md

@@ -723,6 +723,10 @@ metadata:
   description: The infra business unit
 spec:
   type: business-unit
+  profile:
+    displayName: Infrastructure
+    email: infrastructure@example.com
+    picture: https://example.com/groups/bu-infrastructure.jpeg
   parent: ops
   children: [backstage, other]
 ```
@@ -747,6 +751,14 @@ Some common values for this field could be:
 - `product-area`
 - `root` - as a common virtual root of the hierarchy, if desired
 
+### `spec.profile` [optional]
+
+Optional profile information about the group, mainly for display purposes. All
+fields of this structure are also optional. The email would be a group email of
+some form, that the group may wish to be used for contacting them. The picture
+is expected to be a URL pointing to an image that's representative of the group,
+and that a browser could fetch and render on a group page or similar.
+
 ### `spec.parent` [optional]
 
 The immediate parent group in the hierarchy, if any. Not all groups must have a

packages/catalog-model/examples-relative/acme/backstage-group.yaml

@@ -5,6 +5,10 @@ metadata:
   description: The backstage sub-department
 spec:
   type: sub-department
+  profile:
+    displayName: Backstage
+    email: backstage@example.com
+    picture: https://example.com/groups/backstage.jpeg
   parent: infrastructure
   ancestors: [infrastructure, acme-corp]
   children: [team-a, team-b]

packages/catalog-model/examples-relative/acme/boxoffice-group.yaml

@@ -5,6 +5,10 @@ metadata:
   description: The boxoffice sub-department
 spec:
   type: sub-department
+  profile:
+    displayName: Box Office
+    email: boxoffice@example.com
+    # Intentional no picture for testing
   parent: infrastructure
   ancestors: [infrastructure, acme-corp]
   children: [team-c, team-d]

packages/catalog-model/examples-relative/acme/infrastructure-group.yaml

@@ -5,6 +5,7 @@ metadata:
   description: The infra department
 spec:
   type: department
+  # Intentional no profile for testing
   parent: acme-corp
   ancestors: [acme-corp]
   children: [backstage, boxoffice]

packages/catalog-model/examples-relative/acme/org.yaml

@@ -5,6 +5,10 @@ metadata:
   description: The acme-corp organization
 spec:
   type: organization
+  profile:
+    displayName: ACME Corp
+    email: info@example.com
+    picture: https://example.com/logo.jpeg
   ancestors: []
   children: [infrastructure]
   descendants:

packages/catalog-model/examples-relative/acme/team-a-group.yaml

@@ -5,6 +5,10 @@ metadata:
   description: Team A
 spec:
   type: team
+  profile:
+    # Intentional no displayName for testing
+    email: team-a@example.com
+    picture: https://example.com/groups/team-a.jpeg
   parent: backstage
   ancestors: [backstage, infrastructure, acme-corp]
   children: []
@@ -16,7 +20,7 @@ metadata:
   name: breanna.davison
 spec:
   profile:
-    displayName: Breanna Davison
+    # Intentional no displayName for testing
     email: breanna-davison@example.com
     picture: https://example.com/staff/breanna.jpeg
   memberOf: [team-a]

packages/catalog-model/examples-relative/acme/team-b-group.yaml

@@ -5,6 +5,10 @@ metadata:
   description: Team B
 spec:
   type: team
+  profile:
+    displayName: Team B
+    email: team-b@example.com
+    picture: https://example.com/groups/team-b.jpeg
   parent: backstage
   ancestors: [backstage, infrastructure, acme-corp]
   children: []

packages/catalog-model/examples-relative/acme/team-c-group.yaml

@@ -5,6 +5,10 @@ metadata:
   description: Team C
 spec:
   type: team
+  profile:
+    displayName: Team C
+    email: team-c@example.com
+    picture: https://example.com/groups/team-c.jpeg
   parent: boxoffice
   ancestors: [boxoffice, infrastructure, acme-corp]
   children: []

packages/catalog-model/examples-relative/acme/team-d-group.yaml

@@ -5,6 +5,10 @@ metadata:
   description: Team D
 spec:
   type: team
+  profile:
+    displayName: Team D
+    email: team-d@example.com
+    picture: https://example.com/groups/team-d.jpeg
   parent: boxoffice
   ancestors: [boxoffice, infrastructure, acme-corp]
   children: []

packages/catalog-model/src/kinds/GroupEntityV1alpha1.test.ts

@@ -28,11 +28,15 @@ describe('GroupV1alpha1Validator', () => {
       kind: 'Group',
       metadata: {
         name: 'doe-squad',
-        title: 'Doe Squad',
         description: 'A squad for John and Jane',
       },
       spec: {
         type: 'squad',
+        profile: {
+          displayName: 'Doe Squad',
+          email: 'doe@doe.org',
+          picture: 'https://doe.org/doe',
+        },
         parent: 'group-a',
         children: ['child-a', 'child-b'],
       },
@@ -73,6 +77,70 @@ describe('GroupV1alpha1Validator', () => {
     await expect(validator.check(entity)).rejects.toThrow(/type/);
   });
 
+  // profile
+
+  it('accepts missing profile', async () => {
+    delete (entity as any).spec.profile;
+    await expect(validator.check(entity)).resolves.toBe(true);
+  });
+
+  it('rejects wrong profile', async () => {
+    (entity as any).spec.profile = 7;
+    await expect(validator.check(entity)).rejects.toThrow(/profile/);
+  });
+
+  it('profile accepts missing displayName', async () => {
+    delete (entity as any).spec.profile.displayName;
+    await expect(validator.check(entity)).resolves.toBe(true);
+  });
+
+  it('profile rejects wrong displayName', async () => {
+    (entity as any).spec.profile.displayName = 7;
+    await expect(validator.check(entity)).rejects.toThrow(/displayName/);
+  });
+
+  it('profile rejects empty displayName', async () => {
+    (entity as any).spec.profile.displayName = '';
+    await expect(validator.check(entity)).rejects.toThrow(/displayName/);
+  });
+
+  it('profile accepts missing email', async () => {
+    delete (entity as any).spec.profile.email;
+    await expect(validator.check(entity)).resolves.toBe(true);
+  });
+
+  it('profile rejects wrong email', async () => {
+    (entity as any).spec.profile.email = 7;
+    await expect(validator.check(entity)).rejects.toThrow(/email/);
+  });
+
+  it('profile rejects empty email', async () => {
+    (entity as any).spec.profile.email = '';
+    await expect(validator.check(entity)).rejects.toThrow(/email/);
+  });
+
+  it('profile accepts missing picture', async () => {
+    delete (entity as any).spec.profile.picture;
+    await expect(validator.check(entity)).resolves.toBe(true);
+  });
+
+  it('profile rejects wrong picture', async () => {
+    (entity as any).spec.profile.picture = 7;
+    await expect(validator.check(entity)).rejects.toThrow(/picture/);
+  });
+
+  it('profile rejects empty picture', async () => {
+    (entity as any).spec.profile.picture = '';
+    await expect(validator.check(entity)).rejects.toThrow(/picture/);
+  });
+
+  it('profile accepts unknown additional fields', async () => {
+    (entity as any).spec.profile.foo = 'data';
+    await expect(validator.check(entity)).resolves.toBe(true);
+  });
+
+  // parent
+
   it('accepts missing parent', async () => {
     delete (entity as any).spec.parent;
     await expect(validator.check(entity)).resolves.toBe(true);
@@ -83,6 +151,8 @@ describe('GroupV1alpha1Validator', () => {
     await expect(validator.check(entity)).rejects.toThrow(/parent/);
   });
 
+  // children
+
   it('rejects missing children', async () => {
     delete (entity as any).spec.children;
     await expect(validator.check(entity)).rejects.toThrow(/children/);

packages/catalog-model/src/kinds/GroupEntityV1alpha1.ts

@@ -27,6 +27,13 @@ const schema = yup.object<Partial<GroupEntityV1alpha1>>({
   spec: yup
     .object({
       type: yup.string().required().min(1),
+      profile: yup
+        .object({
+          displayName: yup.string().min(1).notRequired(),
+          email: yup.string().min(1).notRequired(),
+          picture: yup.string().min(1).notRequired(),
+        })
+        .notRequired(),
       parent: yup.string().notRequired().min(1),
       // Use these manual tests because yup .required() requires at least
       // one element and there is no simple workaround -_-
@@ -46,6 +53,11 @@ export interface GroupEntityV1alpha1 extends Entity {
   kind: typeof KIND;
   spec: {
     type: string;
+    profile?: {
+      displayName?: string;
+      email?: string;
+      picture?: string;
+    };
     parent?: string;
     children: string[];
   };

plugins/catalog-backend/src/ingestion/processors/github/github.test.ts

@@ -78,6 +78,9 @@ describe('github', () => {
               {
                 slug: 'team',
                 combinedSlug: 'blah/team',
+                name: 'Team',
+                description: 'The one and only team',
+                avatarUrl: 'http://example.com/team.jpeg',
                 parentTeam: {
                   slug: 'parent',
                   combinedSlug: '',
@@ -96,9 +99,16 @@ describe('github', () => {
       const output = {
         groups: [
           expect.objectContaining({
-            metadata: expect.objectContaining({ name: 'team' }),
+            metadata: expect.objectContaining({
+              name: 'team',
+              description: 'The one and only team',
+            }),
             spec: {
               type: 'team',
+              profile: {
+                displayName: 'Team',
+                picture: 'http://example.com/team.jpeg',
+              },
               parent: 'parent',
               children: [],
             },

plugins/catalog-backend/src/ingestion/processors/github/github.ts

@@ -45,7 +45,9 @@ export type User = {
 export type Team = {
   slug: string;
   combinedSlug: string;
+  name?: string;
   description?: string;
+  avatarUrl?: string;
   parentTeam?: Team;
   members: Connection<User>;
 };
@@ -137,6 +139,9 @@ export async function getOrganizationTeams(
           nodes {
             slug
             combinedSlug
+            name
+            description
+            avatarUrl
             parentTeam { slug }
             members(first: 100, membership: IMMEDIATE) {
               pageInfo { hasNextPage }
@@ -162,12 +167,23 @@ export async function getOrganizationTeams(
       },
       spec: {
         type: 'team',
+        profile: {},
         children: [],
       },
     };
 
-    if (team.description) entity.metadata.description = team.description;
-    if (team.parentTeam) entity.spec.parent = team.parentTeam.slug;
+    if (team.description) {
+      entity.metadata.description = team.description;
+    }
+    if (team.name) {
+      entity.spec.profile!.displayName = team.name;
+    }
+    if (team.avatarUrl) {
+      entity.spec.profile!.picture = team.avatarUrl;
+    }
+    if (team.parentTeam) {
+      entity.spec.parent = team.parentTeam.slug;
+    }
 
     const memberNames: string[] = [];
     groupMemberUsers.set(team.slug, memberNames);

plugins/catalog-backend/src/ingestion/processors/ldap/config.test.ts

@@ -66,6 +66,7 @@ describe('readLdapConfig', () => {
             name: 'cn',
             description: 'description',
             type: 'groupType',
+            displayName: 'cn',
             memberOf: 'memberOf',
             members: 'member',
           },
@@ -114,6 +115,9 @@ describe('readLdapConfig', () => {
               name: 'v',
               description: 'd',
               type: 't',
+              displayName: 'c',
+              email: 'm',
+              picture: 'p',
               memberOf: 'm',
               members: 'n',
             },
@@ -161,6 +165,9 @@ describe('readLdapConfig', () => {
             name: 'v',
             description: 'd',
             type: 't',
+            displayName: 'c',
+            email: 'm',
+            picture: 'p',
             memberOf: 'm',
             members: 'n',
           },

plugins/catalog-backend/src/ingestion/processors/ldap/config.ts

@@ -109,6 +109,15 @@ export type GroupConfig = {
     // The name of the attribute that shall be used for the value of the
     // spec.type field of the entity. Defaults to "groupType".
     type: string;
+    // The name of the attribute that shall be used for the value of the
+    // spec.profile.displayName field of the entity. Defaults to "cn".
+    displayName: string;
+    // The name of the attribute that shall be used for the value of the
+    // spec.profile.email field of the entity.
+    email?: string;
+    // The name of the attribute that shall be used for the value of the
+    // spec.profile.picture field of the entity.
+    picture?: string;
     // The name of the attribute that shall be used for the values of the
     // spec.parent field of the entity. Defaults to "memberOf".
     memberOf: string;
@@ -141,6 +150,7 @@ const defaultConfig = {
       rdn: 'cn',
       name: 'cn',
       description: 'description',
+      displayName: 'cn',
       type: 'groupType',
       memberOf: 'memberOf',
       members: 'member',
@@ -220,6 +230,9 @@ export function readLdapConfig(config: Config): LdapProviderConfig[] {
       name: c.getOptionalString('name'),
       description: c.getOptionalString('description'),
       type: c.getOptionalString('type'),
+      displayName: c.getOptionalString('displayName'),
+      email: c.getOptionalString('email'),
+      picture: c.getOptionalString('picture'),
       memberOf: c.getOptionalString('memberOf'),
       members: c.getOptionalString('members'),
     };

plugins/catalog-backend/src/ingestion/processors/ldap/read.test.ts

@@ -138,6 +138,8 @@ describe('readLdapGroups', () => {
         cn: ['cn-value'],
         description: ['description-value'],
         tt: ['type-value'],
+        mail: ['mail-value'],
+        avatarUrl: ['avatarUrl-value'],
         memberOf: ['x', 'y', 'z'],
         member: ['e', 'f', 'g'],
         entryDN: ['dn-value'],
@@ -151,6 +153,9 @@ describe('readLdapGroups', () => {
         rdn: 'cn',
         name: 'cn',
         description: 'description',
+        displayName: 'cn',
+        email: 'mail',
+        picture: 'avatarUrl',
         type: 'tt',
         memberOf: 'memberOf',
         members: 'member',
@@ -173,6 +178,11 @@ describe('readLdapGroups', () => {
         },
         spec: {
           type: 'type-value',
+          profile: {
+            displayName: 'cn-value',
+            email: 'mail-value',
+            picture: 'avatarUrl-value',
+          },
           children: [],
         },
       }),

plugins/catalog-backend/src/ingestion/processors/ldap/read.ts

@@ -150,6 +150,7 @@ export async function readLdapGroups(
       },
       spec: {
         type: 'unknown',
+        profile: {},
         children: [],
       },
     };
@@ -178,6 +179,15 @@ export async function readLdapGroups(
     mapStringAttr(attributes, map.type, v => {
       entity.spec.type = v;
     });
+    mapStringAttr(attributes, map.displayName, v => {
+      entity.spec.profile!.displayName = v;
+    });
+    mapStringAttr(attributes, map.email, v => {
+      entity.spec.profile!.email = v;
+    });
+    mapStringAttr(attributes, map.picture, v => {
+      entity.spec.profile!.picture = v;
+    });
 
     mapReferencesAttr(attributes, map.memberOf, (myDn, vs) => {
       ensureItems(groupMemberOf, myDn, vs);

plugins/catalog-backend/src/ingestion/processors/microsoftGraph/client.test.ts

@@ -210,7 +210,7 @@ describe('MicrosoftGraphClient', () => {
     expect(photo).toBeFalsy();
   });
 
-  it('should load profile photo', async () => {
+  it('should load user profile photo', async () => {
     worker.use(
       rest.get('https://example.com/users/user-id/photo/*', (_, res, ctx) =>
         res(ctx.status(200), ctx.text('911')),
@@ -222,7 +222,7 @@ describe('MicrosoftGraphClient', () => {
     expect(photo).toEqual('data:image/jpeg;base64,OTEx');
   });
 
-  it('should load profile photo for size 120', async () => {
+  it('should load user profile photo for size 120', async () => {
     worker.use(
       rest.get(
         'https://example.com/users/user-id/photos/120/*',
@@ -252,6 +252,46 @@ describe('MicrosoftGraphClient', () => {
     expect(values).toEqual([{ surname: 'Example' }]);
   });
 
+  it('should load group profile photo with max size of 120', async () => {
+    worker.use(
+      rest.get('https://example.com/groups/group-id/photos', (_, res, ctx) =>
+        res(
+          ctx.status(200),
+          ctx.json({
+            value: [
+              {
+                height: 120,
+                id: 120,
+              },
+            ],
+          }),
+        ),
+      ),
+    );
+    worker.use(
+      rest.get(
+        'https://example.com/groups/group-id/photos/120/*',
+        (_, res, ctx) => res(ctx.status(200), ctx.text('911')),
+      ),
+    );
+
+    const photo = await client.getGroupPhotoWithSizeLimit('group-id', 120);
+
+    expect(photo).toEqual('data:image/jpeg;base64,OTEx');
+  });
+
+  it('should load group profile photo', async () => {
+    worker.use(
+      rest.get('https://example.com/groups/group-id/photo/*', (_, res, ctx) =>
+        res(ctx.status(200), ctx.text('911')),
+      ),
+    );
+
+    const photo = await client.getGroupPhoto('group-id');
+
+    expect(photo).toEqual('data:image/jpeg;base64,OTEx');
+  });
+
   it('should load groups', async () => {
     worker.use(
       rest.get('https://example.com/groups', (_, res, ctx) =>

plugins/catalog-backend/src/ingestion/processors/microsoftGraph/client.ts

@@ -117,59 +117,34 @@ export class MicrosoftGraphClient {
     userId: string,
     maxSize: number,
   ): Promise<string | undefined> {
-    const response = await this.requestApi(`users/${userId}/photos`);
-
-    if (response.status === 404) {
-      return undefined;
-    } else if (response.status !== 200) {
-      await this.handleError('user photos', response);
-    }
-
-    const result = await response.json();
-    const photos = result.value as MicrosoftGraph.ProfilePhoto[];
-    let selectedPhoto: MicrosoftGraph.ProfilePhoto | undefined = undefined;
-
-    // Find the biggest picture that is smaller than the max size
-    for (const p of photos) {
-      if (
-        !selectedPhoto ||
-        (p.height! >= selectedPhoto.height! && p.height! <= maxSize)
-      ) {
-        selectedPhoto = p;
-      }
-    }
-
-    if (!selectedPhoto) {
-      return undefined;
-    }
-
-    return await this.getUserPhoto(userId, selectedPhoto.id!);
+    return await this.getPhotoWithSizeLimit('users', userId, maxSize);
   }
 
   async getUserPhoto(
     userId: string,
     sizeId?: string,
   ): Promise<string | undefined> {
-    const path = sizeId
-      ? `users/${userId}/photos/${sizeId}/$value`
-      : `users/${userId}/photo/$value`;
-    const response = await this.requestApi(path);
-
-    if (response.status === 404) {
-      return undefined;
-    } else if (response.status !== 200) {
-      await this.handleError('photo', response);
-    }
-
-    return `data:image/jpeg;base64,${Buffer.from(
-      await response.arrayBuffer(),
-    ).toString('base64')}`;
+    return await this.getPhoto('users', userId, sizeId);
   }
 
   async *getUsers(query?: ODataQuery): AsyncIterable<MicrosoftGraph.User> {
     yield* this.requestCollection<MicrosoftGraph.User>(`users`, query);
   }
 
+  async getGroupPhotoWithSizeLimit(
+    groupId: string,
+    maxSize: number,
+  ): Promise<string | undefined> {
+    return await this.getPhotoWithSizeLimit('groups', groupId, maxSize);
+  }
+
+  async getGroupPhoto(
+    groupId: string,
+    sizeId?: string,
+  ): Promise<string | undefined> {
+    return await this.getPhoto('groups', groupId, sizeId);
+  }
+
   async *getGroups(query?: ODataQuery): AsyncIterable<MicrosoftGraph.Group> {
     yield* this.requestCollection<MicrosoftGraph.Group>(`groups`, query);
   }
@@ -190,6 +165,61 @@ export class MicrosoftGraphClient {
     return await response.json();
   }
 
+  private async getPhotoWithSizeLimit(
+    entityName: string,
+    id: string,
+    maxSize: number,
+  ): Promise<string | undefined> {
+    const response = await this.requestApi(`${entityName}/${id}/photos`);
+
+    if (response.status === 404) {
+      return undefined;
+    } else if (response.status !== 200) {
+      await this.handleError(`${entityName} photos`, response);
+    }
+
+    const result = await response.json();
+    const photos = result.value as MicrosoftGraph.ProfilePhoto[];
+    let selectedPhoto: MicrosoftGraph.ProfilePhoto | undefined = undefined;
+
+    // Find the biggest picture that is smaller than the max size
+    for (const p of photos) {
+      if (
+        !selectedPhoto ||
+        (p.height! >= selectedPhoto.height! && p.height! <= maxSize)
+      ) {
+        selectedPhoto = p;
+      }
+    }
+
+    if (!selectedPhoto) {
+      return undefined;
+    }
+
+    return await this.getPhoto(entityName, id, selectedPhoto.id!);
+  }
+
+  private async getPhoto(
+    entityName: string,
+    id: string,
+    sizeId?: string,
+  ): Promise<string | undefined> {
+    const path = sizeId
+      ? `${entityName}/${id}/photos/${sizeId}/$value`
+      : `${entityName}/${id}/photo/$value`;
+    const response = await this.requestApi(path);
+
+    if (response.status === 404) {
+      return undefined;
+    } else if (response.status !== 200) {
+      await this.handleError('photo', response);
+    }
+
+    return `data:image/jpeg;base64,${Buffer.from(
+      await response.arrayBuffer(),
+    ).toString('base64')}`;
+  }
+
   private async handleError(path: string, response: Response): Promise<void> {
     const result = await response.json();
     const error = result.error as MicrosoftGraph.PublicError;

plugins/catalog-backend/src/ingestion/processors/microsoftGraph/read.test.ts

@@ -63,6 +63,7 @@ describe('read microsoft graph', () => {
     getGroups: jest.fn(),
     getGroupMembers: jest.fn(),
     getUserPhotoWithSizeLimit: jest.fn(),
+    getGroupPhotoWithSizeLimit: jest.fn(),
     getOrganization: jest.fn(),
   } as any;
 
@@ -150,6 +151,9 @@ describe('read microsoft graph', () => {
           },
           spec: {
             type: 'root',
+            profile: {
+              displayName: 'Organization Name',
+            },
           },
         }),
       );
@@ -165,6 +169,8 @@ describe('read microsoft graph', () => {
         yield {
           id: 'groupid',
           displayName: 'Group Name',
+          description: 'Group Description',
+          mail: 'group@example.com',
         };
       }
 
@@ -185,6 +191,9 @@ describe('read microsoft graph', () => {
         id: 'tenantid',
         displayName: 'Organization Name',
       });
+      client.getGroupPhotoWithSizeLimit.mockResolvedValue(
+        'data:image/jpeg;base64,...',
+      );
 
       const {
         groups,
@@ -205,6 +214,9 @@ describe('read microsoft graph', () => {
         },
         spec: {
           type: 'root',
+          profile: {
+            displayName: 'Organization Name',
+          },
         },
       });
       expect(groups).toEqual([
@@ -215,10 +227,17 @@ describe('read microsoft graph', () => {
               'graph.microsoft.com/group-id': 'groupid',
             },
             name: 'group_name',
-            description: 'Group Name',
+            description: 'Group Description',
           },
           spec: {
             type: 'team',
+            profile: {
+              displayName: 'Group Name',
+              email: 'group@example.com',
+              // TODO: Loading groups doesn't work right now as Microsoft Graph
+              // doesn't allows this yet
+              /* picture: 'data:image/jpeg;base64,...',*/
+            },
           },
         }),
       ]);
@@ -230,10 +249,14 @@ describe('read microsoft graph', () => {
       expect(client.getGroups).toBeCalledTimes(1);
       expect(client.getGroups).toBeCalledWith({
         filter: 'securityEnabled eq false',
-        select: ['id', 'displayName', 'mailNickname'],
+        select: ['id', 'displayName', 'description', 'mail', 'mailNickname'],
       });
       expect(client.getGroupMembers).toBeCalledTimes(1);
       expect(client.getGroupMembers).toBeCalledWith('groupid');
+      // TODO: Loading groups doesn't work right now as Microsoft Graph
+      // doesn't allows this yet
+      // expect(client.getGroupPhotoWithSizeLimit).toBeCalledTimes(1);
+      // expect(client.getGroupPhotoWithSizeLimit).toBeCalledWith('groupid', 120);
     });
   });
 

plugins/catalog-backend/src/ingestion/processors/microsoftGraph/read.ts

@@ -37,7 +37,7 @@ export async function readMicrosoftGraphUsers(
   users: UserEntity[]; // With all relations empty
 }> {
   const entities: UserEntity[] = [];
-  const picturePromises: Promise<void>[] = [];
+  const promises: Promise<void>[] = [];
   const limiter = limiterFactory(10);
 
   for await (const user of client.getUsers({
@@ -82,12 +82,12 @@ export async function readMicrosoftGraphUsers(
       );
     });
 
-    picturePromises.push(loadPhoto);
+    promises.push(loadPhoto);
     entities.push(entity);
   }
 
   // Wait for all photos to be downloaded
-  await Promise.all(picturePromises);
+  await Promise.all(promises);
 
   return { users: entities };
 }
@@ -113,6 +113,9 @@ export async function readMicrosoftGraphOrganization(
     },
     spec: {
       type: 'root',
+      profile: {
+        displayName: organization.displayName!,
+      },
       children: [],
     },
   };
@@ -139,11 +142,11 @@ export async function readMicrosoftGraphGroups(
   groupMember.set(rootGroup.metadata.name, new Set<string>());
   groups.push(rootGroup);
 
-  const groupMemberPromises: Promise<void>[] = [];
+  const promises: Promise<void>[] = [];
 
   for await (const group of client.getGroups({
     filter: options?.groupFilter,
-    select: ['id', 'displayName', 'mailNickname'],
+    select: ['id', 'displayName', 'description', 'mail', 'mailNickname'],
   })) {
     if (!group.id || !group.displayName) {
       continue;
@@ -155,18 +158,27 @@ export async function readMicrosoftGraphGroups(
       kind: 'Group',
       metadata: {
         name: name,
-        description: group.displayName,
         annotations: {
           [MICROSOFT_GRAPH_GROUP_ID_ANNOTATION]: group.id,
         },
       },
       spec: {
         type: 'team',
-        // TODO: We could include a group email and picture
+        profile: {},
         children: [],
       },
     };
 
+    if (group.description) {
+      entity.metadata.description = group.description;
+    }
+    if (group.displayName) {
+      entity.spec.profile!.displayName = group.displayName;
+    }
+    if (group.mail) {
+      entity.spec.profile!.email = group.mail;
+    }
+
     // Download the members in parallel, otherwise it can take quite some time
     const loadGroupMembers = limiter(async () => {
       for await (const member of client.getGroupMembers(group.id!)) {
@@ -184,12 +196,25 @@ export async function readMicrosoftGraphGroups(
       }
     });
 
-    groupMemberPromises.push(loadGroupMembers);
+    // TODO: Loading groups doesn't work right now as Microsoft Graph doesn't
+    // allows this yet: https://microsoftgraph.uservoice.com/forums/920506-microsoft-graph-feature-requests/suggestions/37884922-allow-application-to-set-or-update-a-group-s-photo
+    /*/ / Download the photos in parallel, otherwise it can take quite some time
+    const loadPhoto = limiter(async () => {
+      entity.spec.profile!.picture = await client.getGroupPhotoWithSizeLimit(
+        group.id!,
+        // We are limiting the photo size, as groups with full resolution photos
+        // can make the Backstage API slow
+        120,
+      );
+    });
+
+    promises.push(loadPhoto);*/
+    promises.push(loadGroupMembers);
     groups.push(entity);
   }
 
-  // Wait for all group members to be loaded
-  await Promise.all(groupMemberPromises);
+  // Wait for all group members and photos to be loaded
+  await Promise.all(promises);
 
   return {
     groups,

plugins/org/src/components/Cards/Group/GroupProfile/GroupProfileCard.tsx

@@ -14,20 +14,21 @@
  * limitations under the License.
  */
 
-import React from 'react';
-import { Box, Grid, Link, Tooltip, Typography } from '@material-ui/core';
-import Alert from '@material-ui/lab/Alert';
-import { InfoCard } from '@backstage/core';
-import { entityRouteParams } from '@backstage/plugin-catalog';
 import {
   Entity,
   GroupEntity,
   RELATION_CHILD_OF,
   RELATION_PARENT_OF,
 } from '@backstage/catalog-model';
+import { Avatar, InfoCard } from '@backstage/core';
+import { entityRouteParams } from '@backstage/plugin-catalog';
+import { Box, Grid, Link, Tooltip, Typography } from '@material-ui/core';
 import AccountTreeIcon from '@material-ui/icons/AccountTree';
+import EmailIcon from '@material-ui/icons/Email';
 import GroupIcon from '@material-ui/icons/Group';
-import { Link as RouterLink, generatePath } from 'react-router-dom';
+import Alert from '@material-ui/lab/Alert';
+import React from 'react';
+import { generatePath, Link as RouterLink } from 'react-router-dom';
 
 const GroupLink = ({
   groupName,
@@ -68,6 +69,7 @@ export const GroupProfileCard = ({
 }) => {
   const {
     metadata: { name, description },
+    spec: { profile },
   } = group;
   const parent = group?.relations
     ?.filter(r => r.type === RELATION_CHILD_OF)
@@ -78,16 +80,42 @@ export const GroupProfileCard = ({
     ?.filter(r => r.type === RELATION_PARENT_OF)
     ?.map(group => group.target.name);
 
+  const displayName = profile?.displayName ?? name;
+
   if (!group) return <Alert severity="error">User not found</Alert>;
 
   return (
     <InfoCard
-      title={<CardTitle title={name} />}
+      title={<CardTitle title={displayName} />}
       subheader={description}
       variant={variant}
     >
       <Grid container spacing={3}>
-        <Grid item>
+        <Grid item xs={12} sm={2} xl={1}>
+          <Box
+            display="flex"
+            alignItems="flex-start"
+            justifyContent="center"
+            height="100%"
+            width="100%"
+          >
+            <Avatar displayName={displayName} picture={profile?.picture} />
+          </Box>
+        </Grid>
+        <Grid item md={10} xl={11}>
+          {profile?.email && (
+            <Typography variant="subtitle1">
+              <Box display="flex" alignItems="center">
+                <Tooltip title="Email">
+                  <EmailIcon fontSize="inherit" />
+                </Tooltip>
+
+                <Box ml={1} display="inline">
+                  {profile.email}
+                </Box>
+              </Box>
+            </Typography>
+          )}
           {parent ? (
             <Typography variant="subtitle1">
               <Box display="flex" alignItems="center">

plugins/org/src/components/Cards/Group/MembersList/MembersListCard.test.tsx

@@ -18,12 +18,12 @@ import { renderWithEffects, wrapInTestApp } from '@backstage/test-utils';
 import React from 'react';
 import { ApiProvider, ApiRegistry } from '@backstage/core';
 import { CatalogApi, catalogApiRef } from '@backstage/plugin-catalog';
-import { Entity } from '@backstage/catalog-model';
+import { Entity, GroupEntity } from '@backstage/catalog-model';
 import { MembersListCard } from './MembersListCard';
 
 describe('MemberTab Test', () => {
-  const groupEntity = {
-    apiVersion: 'v1',
+  const groupEntity: GroupEntity = {
+    apiVersion: 'backstage.io/v1alpha1',
     kind: 'Group',
     metadata: {
       name: 'team-d',
@@ -33,9 +33,7 @@ describe('MemberTab Test', () => {
     spec: {
       type: 'team',
       parent: 'boxoffice',
-      ancestors: ['boxoffice', 'acme-corp'],
       children: [],
-      descendants: [],
     },
   };
 

plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx

@@ -15,6 +15,7 @@
  */
 import {
   Entity,
+  GroupEntity,
   RELATION_MEMBER_OF,
   UserEntity,
 } from '@backstage/catalog-model';
@@ -42,7 +43,9 @@ const useStyles = makeStyles((theme: Theme) =>
       borderRadius: '4px',
       overflow: 'visible',
       position: 'relative',
-      margin: theme.spacing(3, 0, 0),
+      margin: theme.spacing(3, 0, 1),
+      flex: '1',
+      minWidth: '0px',
     },
   }),
 );
@@ -55,12 +58,14 @@ const MemberComponent = ({
   groupEntity: Entity;
 }) => {
   const classes = useStyles();
-  const { name: metaName } = member.metadata;
-  const { profile } = member.spec;
+  const {
+    metadata: { name: metaName },
+    spec: { profile },
+  } = member;
   const displayName = profile?.displayName ?? metaName;
 
   return (
-    <Grid item xs={12} sm={6} md={3} xl={2}>
+    <Grid item container xs={12} sm={6} md={3} xl={2}>
       <Box className={classes.card}>
         <Box
           display="flex"
@@ -100,13 +105,16 @@ const MemberComponent = ({
 export const MembersListCard = ({
   entity: groupEntity,
 }: {
-  entity: Entity;
+  entity: GroupEntity;
 }) => {
   const {
     metadata: { name: groupName },
+    spec: { profile },
   } = groupEntity;
   const catalogApi = useApi(catalogApiRef);
 
+  const displayName = profile?.displayName ?? groupName;
+
   const { loading, error, value: members } = useAsync(async () => {
     const membersList = await catalogApi.getEntities({
       filter: {
@@ -133,7 +141,7 @@ export const MembersListCard = ({
     <Grid item>
       <InfoCard
         title={`Members (${members?.length || 0})`}
-        subheader={`of ${groupName}`}
+        subheader={`of ${displayName}`}
       >
         <Grid container spacing={3}>
           {members && members.length ? (

plugins/org/src/components/Cards/User/UserProfileCard/UserProfileCard.tsx

@@ -74,7 +74,6 @@ export const UserProfileCard = ({
     user?.relations
       ?.filter(r => r.type === RELATION_MEMBER_OF)
       ?.map(group => group.target.name) || [];
-
   const displayName = profile?.displayName ?? metaName;
 
   if (!user) {
@@ -96,18 +95,19 @@ export const UserProfileCard = ({
           </Box>
         </Grid>
         <Grid item md={10} xl={11}>
-          <Typography variant="subtitle1">
-            <Box display="flex" alignItems="center">
-              <Tooltip title="Email">
-                <EmailIcon fontSize="inherit" />
-              </Tooltip>
-              {profile?.email && (
+          {profile?.email && (
+            <Typography variant="subtitle1">
+              <Box display="flex" alignItems="center">
+                <Tooltip title="Email">
+                  <EmailIcon fontSize="inherit" />
+                </Tooltip>
+
                 <Box ml={1} display="inline">
                   {profile.email}
                 </Box>
-              )}
-            </Box>
-          </Typography>
+              </Box>
+            </Typography>
+          )}
           <Typography variant="subtitle1">
             <Box display="flex" alignItems="center">
               <Tooltip title="Member of">