From 9d4cd211ed6965b58adb5d0a1990040c8cd8d6af Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Fri, 6 Dec 2024 18:30:05 +0000 Subject: [PATCH] Authenticate media requests when loading avatars (#2856) * Load authenicated media * lint * Add tests * Add widget behaviour and test. * Update src/Avatar.test.tsx Co-authored-by: Hugh Nimmo-Smith --------- Co-authored-by: Hugh Nimmo-Smith --- src/Avatar.test.tsx | 156 ++++++++++++++++++++++++++++++++++++++++++ src/Avatar.tsx | 80 +++++++++++++++++++--- src/ClientContext.tsx | 9 +++ src/utils/matrix.ts | 12 ---- 4 files changed, 236 insertions(+), 21 deletions(-) create mode 100644 src/Avatar.test.tsx diff --git a/src/Avatar.test.tsx b/src/Avatar.test.tsx new file mode 100644 index 000000000..7eee2e909 --- /dev/null +++ b/src/Avatar.test.tsx @@ -0,0 +1,156 @@ +/* +Copyright 2024 New Vector Ltd. + +SPDX-License-Identifier: AGPL-3.0-only +Please see LICENSE in the repository root for full details. +*/ + +import { afterEach, expect, test, vi } from "vitest"; +import { render, screen } from "@testing-library/react"; +import { MatrixClient } from "matrix-js-sdk/src/client"; +import { FC, PropsWithChildren } from "react"; + +import { ClientContextProvider } from "./ClientContext"; +import { Avatar } from "./Avatar"; +import { mockMatrixRoomMember, mockRtcMembership } from "./utils/test"; + +const TestComponent: FC< + PropsWithChildren<{ client: MatrixClient; supportsThumbnails?: boolean }> +> = ({ client, children, supportsThumbnails }) => { + return ( + + {children} + + ); +}; + +afterEach(() => { + vi.unstubAllGlobals(); +}); + +test("should just render a placeholder when the user has no avatar", () => { + const client = vi.mocked({ + getAccessToken: () => "my-access-token", + mxcUrlToHttp: () => vi.fn(), + } as unknown as MatrixClient); + + vi.spyOn(client, "mxcUrlToHttp"); + const member = mockMatrixRoomMember( + mockRtcMembership("@alice:example.org", "AAAA"), + { + getMxcAvatarUrl: () => undefined, + }, + ); + const displayName = "Alice"; + render( + + + , + ); + const element = screen.getByRole("img", { name: "@alice:example.org" }); + expect(element.tagName).toEqual("SPAN"); + expect(client.mxcUrlToHttp).toBeCalledTimes(0); +}); + +test("should just render a placeholder when thumbnails are not supported", () => { + const client = vi.mocked({ + getAccessToken: () => "my-access-token", + mxcUrlToHttp: () => vi.fn(), + } as unknown as MatrixClient); + + vi.spyOn(client, "mxcUrlToHttp"); + const member = mockMatrixRoomMember( + mockRtcMembership("@alice:example.org", "AAAA"), + { + getMxcAvatarUrl: () => "mxc://example.org/alice-avatar", + }, + ); + const displayName = "Alice"; + render( + + + , + ); + const element = screen.getByRole("img", { name: "@alice:example.org" }); + expect(element.tagName).toEqual("SPAN"); + expect(client.mxcUrlToHttp).toBeCalledTimes(0); +}); + +test("should attempt to fetch authenticated media", async () => { + const expectedAuthUrl = "http://example.org/media/alice-avatar"; + const expectedObjectURL = "my-object-url"; + const accessToken = "my-access-token"; + const theBlob = new Blob([]); + + // vitest doesn't have a implementation of create/revokeObjectURL, so we need + // to delete the property. It's a bit odd, but it works. + Reflect.deleteProperty(global.window.URL, "createObjectURL"); + globalThis.URL.createObjectURL = vi.fn().mockReturnValue(expectedObjectURL); + Reflect.deleteProperty(global.window.URL, "revokeObjectURL"); + globalThis.URL.revokeObjectURL = vi.fn(); + + const fetchFn = vi.fn().mockResolvedValue({ + blob: async () => Promise.resolve(theBlob), + }); + vi.stubGlobal("fetch", fetchFn); + + const client = vi.mocked({ + getAccessToken: () => accessToken, + mxcUrlToHttp: () => vi.fn(), + } as unknown as MatrixClient); + + vi.spyOn(client, "mxcUrlToHttp").mockReturnValue(expectedAuthUrl); + const member = mockMatrixRoomMember( + mockRtcMembership("@alice:example.org", "AAAA"), + { + getMxcAvatarUrl: () => "mxc://example.org/alice-avatar", + }, + ); + const displayName = "Alice"; + render( + + + , + ); + + // Fetch is asynchronous, so wait for this to resolve. + await vi.waitUntil(() => + document.querySelector(`img[src='${expectedObjectURL}']`), + ); + + expect(client.mxcUrlToHttp).toBeCalledTimes(1); + expect(globalThis.fetch).toBeCalledWith(expectedAuthUrl, { + headers: { Authorization: `Bearer ${accessToken}` }, + }); +}); diff --git a/src/Avatar.tsx b/src/Avatar.tsx index 29ab52362..f3fe6cd87 100644 --- a/src/Avatar.tsx +++ b/src/Avatar.tsx @@ -5,11 +5,11 @@ SPDX-License-Identifier: AGPL-3.0-only Please see LICENSE in the repository root for full details. */ -import { useMemo, FC, CSSProperties } from "react"; +import { useMemo, FC, CSSProperties, useState, useEffect } from "react"; import { Avatar as CompoundAvatar } from "@vector-im/compound-web"; +import { MatrixClient } from "matrix-js-sdk/src/client"; -import { getAvatarUrl } from "./utils/matrix"; -import { useClient } from "./ClientContext"; +import { useClientState } from "./ClientContext"; export enum Size { XS = "xs", @@ -36,6 +36,28 @@ interface Props { style?: CSSProperties; } +export function getAvatarUrl( + client: MatrixClient, + mxcUrl: string | null, + avatarSize = 96, +): string | null { + const width = Math.floor(avatarSize * window.devicePixelRatio); + const height = Math.floor(avatarSize * window.devicePixelRatio); + // scale is more suitable for larger sizes + const resizeMethod = avatarSize <= 96 ? "crop" : "scale"; + return mxcUrl + ? client.mxcUrlToHttp( + mxcUrl, + width, + height, + resizeMethod, + false, + true, + true, + ) + : null; +} + export const Avatar: FC = ({ className, id, @@ -45,7 +67,7 @@ export const Avatar: FC = ({ style, ...props }) => { - const { client } = useClient(); + const clientState = useClientState(); const sizePx = useMemo( () => @@ -55,10 +77,50 @@ export const Avatar: FC = ({ [size], ); - const resolvedSrc = useMemo(() => { - if (!client || !src || !sizePx) return undefined; - return src.startsWith("mxc://") ? getAvatarUrl(client, src, sizePx) : src; - }, [client, src, sizePx]); + const [avatarUrl, setAvatarUrl] = useState(undefined); + + useEffect(() => { + if (clientState?.state !== "valid") { + return; + } + const { authenticated, supportedFeatures } = clientState; + const client = authenticated?.client; + + if (!client || !src || !sizePx || !supportedFeatures.thumbnails) { + return; + } + + const token = client.getAccessToken(); + if (!token) { + return; + } + const resolveSrc = getAvatarUrl(client, src, sizePx); + if (!resolveSrc) { + setAvatarUrl(undefined); + return; + } + + let objectUrl: string | undefined; + fetch(resolveSrc, { + headers: { + Authorization: `Bearer ${token}`, + }, + }) + .then(async (req) => req.blob()) + .then((blob) => { + objectUrl = URL.createObjectURL(blob); + setAvatarUrl(objectUrl); + }) + .catch((ex) => { + setAvatarUrl(undefined); + }); + + return (): void => { + if (objectUrl) { + URL.revokeObjectURL(objectUrl); + } + }; + }, [clientState, src, sizePx]); return ( = ({ id={id} name={name} size={`${sizePx}px`} - src={resolvedSrc} + src={avatarUrl} style={style} {...props} /> diff --git a/src/ClientContext.tsx b/src/ClientContext.tsx index 8b5589d50..7a37e7504 100644 --- a/src/ClientContext.tsx +++ b/src/ClientContext.tsx @@ -48,6 +48,7 @@ export type ValidClientState = { disconnected: boolean; supportedFeatures: { reactions: boolean; + thumbnails: boolean; }; setClient: (params?: SetClientParams) => void; }; @@ -71,6 +72,8 @@ export type SetClientParams = { const ClientContext = createContext(undefined); +export const ClientContextProvider = ClientContext.Provider; + export const useClientState = (): ClientState | undefined => useContext(ClientContext); @@ -253,6 +256,7 @@ export const ClientProvider: FC = ({ children }) => { const [isDisconnected, setIsDisconnected] = useState(false); const [supportsReactions, setSupportsReactions] = useState(false); + const [supportsThumbnails, setSupportsThumbnails] = useState(false); const state: ClientState | undefined = useMemo(() => { if (alreadyOpenedErr) { @@ -278,6 +282,7 @@ export const ClientProvider: FC = ({ children }) => { disconnected: isDisconnected, supportedFeatures: { reactions: supportsReactions, + thumbnails: supportsThumbnails, }, }; }, [ @@ -288,6 +293,7 @@ export const ClientProvider: FC = ({ children }) => { setClient, isDisconnected, supportsReactions, + supportsThumbnails, ]); const onSync = useCallback( @@ -313,6 +319,8 @@ export const ClientProvider: FC = ({ children }) => { } if (initClientState.widgetApi) { + // There is currently no widget API for authenticated media thumbnails. + setSupportsThumbnails(false); const reactSend = initClientState.widgetApi.hasCapability( "org.matrix.msc2762.send.event:m.reaction", ); @@ -334,6 +342,7 @@ export const ClientProvider: FC = ({ children }) => { } } else { setSupportsReactions(true); + setSupportsThumbnails(true); } return (): void => { diff --git a/src/utils/matrix.ts b/src/utils/matrix.ts index d3821a3ff..63b6ef67a 100644 --- a/src/utils/matrix.ts +++ b/src/utils/matrix.ts @@ -333,15 +333,3 @@ export function getRelativeRoomUrl( : ""; return `/room/#${roomPart}?${generateUrlSearchParams(roomId, encryptionSystem, viaServers).toString()}`; } - -export function getAvatarUrl( - client: MatrixClient, - mxcUrl: string, - avatarSize = 96, -): string { - const width = Math.floor(avatarSize * window.devicePixelRatio); - const height = Math.floor(avatarSize * window.devicePixelRatio); - // scale is more suitable for larger sizes - const resizeMethod = avatarSize <= 96 ? "crop" : "scale"; - return mxcUrl && client.mxcUrlToHttp(mxcUrl, width, height, resizeMethod)!; -}