From 72050720b2976f67b4b3cdb06d3b22c6e7dbaca1 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Fri, 2 Jun 2023 11:09:16 +0300 Subject: [PATCH] [Cases] Fix bug where the Cases users API will thrown an error when `imageUrl` is set to `null` (#158815) ## Summary As part of the Serverless work in https://github.com/elastic/kibana/issues/153726, we validate all of our responses returned by the Cases APIs. The GET case users internal API validates the schema of the `avatar` persisted in the `data` of the user profile. It may be possible to set the `imageUrl` to `null` from within the user profile page. This PR fixes this bug. It also updates the type of the `imageUrl` in `UserProfileAvatarData`. ## Testing 1. Go to your user profile and press save 2. Go to a case 3. You should see an error toaster. The fix in this RP should eliminate the error toaster and the Case view page should be shown properly 4. Go to your user profile and change the initials the color and set a profile image. Go again to a case and verify that the avatar is shown as expected ### Checklist Delete any items that are not applicable to this PR. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios ### For maintainers - [x] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --- .../src/user_profile.ts | 2 +- .../avatar/user_profile/impl/user_profile.ts | 2 +- x-pack/plugins/cases/common/api/user.test.ts | 11 ++++ x-pack/plugins/cases/common/api/user.ts | 8 ++- .../cases/server/client/user_actions/users.ts | 21 ++++++- .../security/common/model/user_profile.ts | 2 +- .../common/internal/user_actions_get_users.ts | 61 +++++++++++++++++++ 7 files changed, 101 insertions(+), 6 deletions(-) diff --git a/packages/kbn-user-profile-components/src/user_profile.ts b/packages/kbn-user-profile-components/src/user_profile.ts index bafe2d2184b42..4d0803ad6dbec 100644 --- a/packages/kbn-user-profile-components/src/user_profile.ts +++ b/packages/kbn-user-profile-components/src/user_profile.ts @@ -80,7 +80,7 @@ export interface UserProfileAvatarData { /** * Base64 data URL for the user avatar image. */ - imageUrl?: string; + imageUrl?: string | null; } export const USER_AVATAR_FALLBACK_CODE_POINT = 97; // code point for lowercase "a" diff --git a/packages/shared-ux/avatar/user_profile/impl/user_profile.ts b/packages/shared-ux/avatar/user_profile/impl/user_profile.ts index f82816df3d3e3..a278e2bc61a9c 100644 --- a/packages/shared-ux/avatar/user_profile/impl/user_profile.ts +++ b/packages/shared-ux/avatar/user_profile/impl/user_profile.ts @@ -84,7 +84,7 @@ export interface UserProfileAvatarData { /** * Base64 data URL for the user avatar image. */ - imageUrl?: string; + imageUrl?: string | null; } export const USER_AVATAR_FALLBACK_CODE_POINT = 97; // code point for lowercase "a" diff --git a/x-pack/plugins/cases/common/api/user.test.ts b/x-pack/plugins/cases/common/api/user.test.ts index 3fd8e9e4e8b98..a24d9df98af9e 100644 --- a/x-pack/plugins/cases/common/api/user.test.ts +++ b/x-pack/plugins/cases/common/api/user.test.ts @@ -5,6 +5,7 @@ * 2.0. */ +import { set } from 'lodash'; import { UserRt, UserWithProfileInfoRt, UsersRt, GetCaseUsersResponseRt } from './user'; describe('User', () => { @@ -61,6 +62,16 @@ describe('User', () => { }); }); + it.each(['initials', 'color', 'imageUrl'])('does not returns an error if %s is null', (key) => { + const reqWithNullImage = set(defaultRequest, `avatar.${key}`, null); + const query = UserWithProfileInfoRt.decode(reqWithNullImage); + + expect(query).toStrictEqual({ + _tag: 'Right', + right: reqWithNullImage, + }); + }); + it('removes foo:bar attributes from request', () => { const query = UserWithProfileInfoRt.decode({ ...defaultRequest, diff --git a/x-pack/plugins/cases/common/api/user.ts b/x-pack/plugins/cases/common/api/user.ts index c832e71d2a33d..fe7ad34cab58a 100644 --- a/x-pack/plugins/cases/common/api/user.ts +++ b/x-pack/plugins/cases/common/api/user.ts @@ -25,7 +25,13 @@ export const UserWithProfileInfoRt = rt.intersection([ rt.exact(rt.partial({ uid: rt.string })), rt.exact( rt.partial({ - avatar: rt.exact(rt.partial({ initials: rt.string, color: rt.string, imageUrl: rt.string })), + avatar: rt.exact( + rt.partial({ + initials: rt.union([rt.string, rt.null]), + color: rt.union([rt.string, rt.null]), + imageUrl: rt.union([rt.string, rt.null]), + }) + ), }) ), ]); diff --git a/x-pack/plugins/cases/server/client/user_actions/users.ts b/x-pack/plugins/cases/server/client/user_actions/users.ts index 60eafcc2812bb..52084d5d0c412 100644 --- a/x-pack/plugins/cases/server/client/user_actions/users.ts +++ b/x-pack/plugins/cases/server/client/user_actions/users.ts @@ -5,7 +5,8 @@ * 2.0. */ -import type { UserProfileWithAvatar } from '@kbn/user-profile-components'; +import { isString } from 'lodash'; +import type { UserProfileAvatarData, UserProfileWithAvatar } from '@kbn/user-profile-components'; import type { GetCaseUsersResponse, User, UserWithProfileInfo } from '../../../common/api'; import { decodeOrThrow, GetCaseUsersResponseRt } from '../../../common/api'; import type { OwnerEntity } from '../../authorization'; @@ -137,11 +138,27 @@ const getUserInformation = ( full_name: userProfile?.user.full_name ?? userInfo?.full_name ?? null, username: userProfile?.user.username ?? userInfo?.username ?? null, }, - avatar: userProfile?.data.avatar, + avatar: getUserProfileAvatar(userProfile?.data.avatar), uid: userProfile?.uid ?? uid ?? userInfo?.profile_uid, }; }; +const getUserProfileAvatar = ( + avatar?: UserProfileAvatarData | undefined +): UserWithProfileInfo['avatar'] | undefined => { + if (!avatar) { + return avatar; + } + + const res = { + ...(isString(avatar.initials) ? { initials: avatar.initials } : {}), + ...(isString(avatar.color) ? { color: avatar.color } : {}), + ...(isString(avatar.imageUrl) ? { imageUrl: avatar.imageUrl } : {}), + }; + + return res; +}; + const removeAllFromSet = (originalSet: Set, values: string[]) => { const newSet = new Set(originalSet); values.forEach((value) => newSet.delete(value)); diff --git a/x-pack/plugins/security/common/model/user_profile.ts b/x-pack/plugins/security/common/model/user_profile.ts index 943934cdf8abd..a39f56a4bff96 100644 --- a/x-pack/plugins/security/common/model/user_profile.ts +++ b/x-pack/plugins/security/common/model/user_profile.ts @@ -87,7 +87,7 @@ export interface UserProfileAvatarData { /** * Base64 data URL for the user avatar image. */ - imageUrl?: string; + imageUrl?: string | null; } /** diff --git a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/internal/user_actions_get_users.ts b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/internal/user_actions_get_users.ts index 35a07a4727a1b..030f828baf3d7 100644 --- a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/internal/user_actions_get_users.ts +++ b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/internal/user_actions_get_users.ts @@ -431,6 +431,67 @@ export default ({ getService }: FtrProviderContext): void => { expect(assignees).to.eql([]); expect(unassignedUsers).to.eql([]); }); + + it('does not throw with imageUrl set to null', async () => { + await updateUserProfileAvatar({ + supertest, + req: { + // @ts-expect-error: types are not correct + initials: null, + // @ts-expect-error: types are not correct + color: null, + imageUrl: null, + }, + headers: superUserHeaders, + }); + + const postedCase = await createCase( + supertestWithoutAuth, + getPostCaseRequest(), + 200, + null, + superUserHeaders + ); + + const res = await getCaseUsers({ + caseId: postedCase.id, + supertest, + }); + + expect(res.participants[0].avatar).to.eql({}); + expect(res.reporter.avatar).to.eql({}); + }); + + it('does not return any avatar data if they are not a string', async () => { + await updateUserProfileAvatar({ + supertest, + req: { + // @ts-expect-error: types are not correct + initials: 4, + // @ts-expect-error: types are not correct + color: true, + // @ts-expect-error: types are not correct + imageUrl: [], + }, + headers: superUserHeaders, + }); + + const postedCase = await createCase( + supertestWithoutAuth, + getPostCaseRequest(), + 200, + null, + superUserHeaders + ); + + const res = await getCaseUsers({ + caseId: postedCase.id, + supertest, + }); + + expect(res.participants[0].avatar).to.eql({}); + expect(res.reporter.avatar).to.eql({}); + }); }); });