diff --git a/.changeset/dirty-melons-retire.md b/.changeset/dirty-melons-retire.md new file mode 100644 index 0000000000..2b92b44678 --- /dev/null +++ b/.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`. diff --git a/.changeset/dirty-wolves-wink.md b/.changeset/dirty-wolves-wink.md new file mode 100644 index 0000000000..77f1a62aff --- /dev/null +++ b/.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. diff --git a/.changeset/groups_profile.png b/.changeset/groups_profile.png new file mode 100644 index 0000000000..417faaa4af Binary files /dev/null and b/.changeset/groups_profile.png differ diff --git a/.changeset/odd-mugs-build.md b/.changeset/odd-mugs-build.md new file mode 100644 index 0000000000..bf95d23410 --- /dev/null +++ b/.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. diff --git a/.changeset/tender-coats-unite.md b/.changeset/tender-coats-unite.md new file mode 100644 index 0000000000..018660bcfa --- /dev/null +++ b/.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. diff --git a/.changeset/witty-scissors-divide.md b/.changeset/witty-scissors-divide.md new file mode 100644 index 0000000000..f288088ef2 --- /dev/null +++ b/.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. diff --git a/docs/features/software-catalog/descriptor-format.md b/docs/features/software-catalog/descriptor-format.md index 72e6388273..e17c5ca36e 100644 --- a/docs/features/software-catalog/descriptor-format.md +++ b/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 diff --git a/packages/catalog-model/examples-relative/acme/backstage-group.yaml b/packages/catalog-model/examples-relative/acme/backstage-group.yaml index f992d155bc..1cfd469e99 100644 --- a/packages/catalog-model/examples-relative/acme/backstage-group.yaml +++ b/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] diff --git a/packages/catalog-model/examples-relative/acme/boxoffice-group.yaml b/packages/catalog-model/examples-relative/acme/boxoffice-group.yaml index 0be1fe58cb..4bb742bdb1 100644 --- a/packages/catalog-model/examples-relative/acme/boxoffice-group.yaml +++ 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] diff --git a/packages/catalog-model/examples-relative/acme/infrastructure-group.yaml b/packages/catalog-model/examples-relative/acme/infrastructure-group.yaml index 2341782944..acfeef642c 100644 --- a/packages/catalog-model/examples-relative/acme/infrastructure-group.yaml +++ b/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] diff --git a/packages/catalog-model/examples-relative/acme/org.yaml b/packages/catalog-model/examples-relative/acme/org.yaml index fe368962b1..58cce827ff 100644 --- a/packages/catalog-model/examples-relative/acme/org.yaml +++ b/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: diff --git a/packages/catalog-model/examples-relative/acme/team-a-group.yaml b/packages/catalog-model/examples-relative/acme/team-a-group.yaml index 8dbf0bb839..53d4bb8d80 100644 --- a/packages/catalog-model/examples-relative/acme/team-a-group.yaml +++ b/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] diff --git a/packages/catalog-model/examples-relative/acme/team-b-group.yaml b/packages/catalog-model/examples-relative/acme/team-b-group.yaml index 00e9e41d80..a59a24317d 100644 --- a/packages/catalog-model/examples-relative/acme/team-b-group.yaml +++ b/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: [] diff --git a/packages/catalog-model/examples-relative/acme/team-c-group.yaml b/packages/catalog-model/examples-relative/acme/team-c-group.yaml index 0f97f69cb3..31f67ac362 100644 --- a/packages/catalog-model/examples-relative/acme/team-c-group.yaml +++ b/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: [] diff --git a/packages/catalog-model/examples-relative/acme/team-d-group.yaml b/packages/catalog-model/examples-relative/acme/team-d-group.yaml index 5a1a53a2e6..c4f87b3ac3 100644 --- a/packages/catalog-model/examples-relative/acme/team-d-group.yaml +++ b/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: [] diff --git a/packages/catalog-model/src/kinds/GroupEntityV1alpha1.test.ts b/packages/catalog-model/src/kinds/GroupEntityV1alpha1.test.ts index 755a68cede..b25795cd31 100644 --- a/packages/catalog-model/src/kinds/GroupEntityV1alpha1.test.ts +++ b/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/); diff --git a/packages/catalog-model/src/kinds/GroupEntityV1alpha1.ts b/packages/catalog-model/src/kinds/GroupEntityV1alpha1.ts index 5f73d00e66..6f2664af77 100644 --- a/packages/catalog-model/src/kinds/GroupEntityV1alpha1.ts +++ b/packages/catalog-model/src/kinds/GroupEntityV1alpha1.ts @@ -27,6 +27,13 @@ const schema = yup.object>({ 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[]; }; diff --git a/plugins/catalog-backend/src/ingestion/processors/github/github.test.ts b/plugins/catalog-backend/src/ingestion/processors/github/github.test.ts index d10aba8acb..8911e84d99 100644 --- a/plugins/catalog-backend/src/ingestion/processors/github/github.test.ts +++ b/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: [], }, diff --git a/plugins/catalog-backend/src/ingestion/processors/github/github.ts b/plugins/catalog-backend/src/ingestion/processors/github/github.ts index 8be96c41de..2b33d72f65 100644 --- a/plugins/catalog-backend/src/ingestion/processors/github/github.ts +++ b/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; }; @@ -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); diff --git a/plugins/catalog-backend/src/ingestion/processors/ldap/config.test.ts b/plugins/catalog-backend/src/ingestion/processors/ldap/config.test.ts index 7e6036668f..cc43827d87 100644 --- a/plugins/catalog-backend/src/ingestion/processors/ldap/config.test.ts +++ b/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', }, diff --git a/plugins/catalog-backend/src/ingestion/processors/ldap/config.ts b/plugins/catalog-backend/src/ingestion/processors/ldap/config.ts index 2a63b20619..7608f247d4 100644 --- a/plugins/catalog-backend/src/ingestion/processors/ldap/config.ts +++ b/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'), }; diff --git a/plugins/catalog-backend/src/ingestion/processors/ldap/read.test.ts b/plugins/catalog-backend/src/ingestion/processors/ldap/read.test.ts index e964a03ce0..4155c2124e 100644 --- a/plugins/catalog-backend/src/ingestion/processors/ldap/read.test.ts +++ b/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: [], }, }), diff --git a/plugins/catalog-backend/src/ingestion/processors/ldap/read.ts b/plugins/catalog-backend/src/ingestion/processors/ldap/read.ts index b7315fd291..f0fb19ac02 100644 --- a/plugins/catalog-backend/src/ingestion/processors/ldap/read.ts +++ b/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); diff --git a/plugins/catalog-backend/src/ingestion/processors/microsoftGraph/client.test.ts b/plugins/catalog-backend/src/ingestion/processors/microsoftGraph/client.test.ts index e51a7753af..5ef82432bb 100644 --- a/plugins/catalog-backend/src/ingestion/processors/microsoftGraph/client.test.ts +++ b/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) => diff --git a/plugins/catalog-backend/src/ingestion/processors/microsoftGraph/client.ts b/plugins/catalog-backend/src/ingestion/processors/microsoftGraph/client.ts index 9db76542f4..bdb5918b2f 100644 --- a/plugins/catalog-backend/src/ingestion/processors/microsoftGraph/client.ts +++ b/plugins/catalog-backend/src/ingestion/processors/microsoftGraph/client.ts @@ -117,59 +117,34 @@ export class MicrosoftGraphClient { userId: string, maxSize: number, ): Promise { - 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 { - 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 { yield* this.requestCollection(`users`, query); } + async getGroupPhotoWithSizeLimit( + groupId: string, + maxSize: number, + ): Promise { + return await this.getPhotoWithSizeLimit('groups', groupId, maxSize); + } + + async getGroupPhoto( + groupId: string, + sizeId?: string, + ): Promise { + return await this.getPhoto('groups', groupId, sizeId); + } + async *getGroups(query?: ODataQuery): AsyncIterable { yield* this.requestCollection(`groups`, query); } @@ -190,6 +165,61 @@ export class MicrosoftGraphClient { return await response.json(); } + private async getPhotoWithSizeLimit( + entityName: string, + id: string, + maxSize: number, + ): Promise { + 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 { + 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 { const result = await response.json(); const error = result.error as MicrosoftGraph.PublicError; diff --git a/plugins/catalog-backend/src/ingestion/processors/microsoftGraph/read.test.ts b/plugins/catalog-backend/src/ingestion/processors/microsoftGraph/read.test.ts index b0d446c417..07d540c848 100644 --- a/plugins/catalog-backend/src/ingestion/processors/microsoftGraph/read.test.ts +++ b/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); }); }); diff --git a/plugins/catalog-backend/src/ingestion/processors/microsoftGraph/read.ts b/plugins/catalog-backend/src/ingestion/processors/microsoftGraph/read.ts index 532e1c3777..4409422645 100644 --- a/plugins/catalog-backend/src/ingestion/processors/microsoftGraph/read.ts +++ b/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[] = []; + const promises: Promise[] = []; 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()); groups.push(rootGroup); - const groupMemberPromises: Promise[] = []; + const promises: Promise[] = []; 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, diff --git a/plugins/org/src/components/Cards/Group/GroupProfile/GroupProfileCard.tsx b/plugins/org/src/components/Cards/Group/GroupProfile/GroupProfileCard.tsx index a2582195b9..779cb1d92d 100644 --- a/plugins/org/src/components/Cards/Group/GroupProfile/GroupProfileCard.tsx +++ b/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 User not found; return ( } + title={} subheader={description} variant={variant} > - + + + + + + + {profile?.email && ( + + + + + + + + {profile.email} + + + + )} {parent ? ( diff --git a/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.test.tsx b/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.test.tsx index 0c188afc57..535b2acd0e 100644 --- a/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.test.tsx +++ b/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: [], }, }; diff --git a/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx b/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx index 6f1986d2b4..579db9d9a5 100644 --- a/plugins/org/src/components/Cards/Group/MembersList/MembersListCard.tsx +++ b/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 ( - + { 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 = ({ {members && members.length ? ( diff --git a/plugins/org/src/components/Cards/User/UserProfileCard/UserProfileCard.tsx b/plugins/org/src/components/Cards/User/UserProfileCard/UserProfileCard.tsx index c984d347e7..99048c0cd8 100644 --- a/plugins/org/src/components/Cards/User/UserProfileCard/UserProfileCard.tsx +++ b/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 = ({ - - - - - - {profile?.email && ( + {profile?.email && ( + + + + + + {profile.email} - )} - - + + + )}