Skip to content

Commit

Permalink
Authenticate media requests when loading avatars (#2856)
Browse files Browse the repository at this point in the history
* Load authenicated media

* lint

* Add tests

* Add widget behaviour and test.

* Update src/Avatar.test.tsx

Co-authored-by: Hugh Nimmo-Smith <[email protected]>

---------

Co-authored-by: Hugh Nimmo-Smith <[email protected]>
  • Loading branch information
Half-Shot and hughns authored Dec 6, 2024
1 parent 4fab1a2 commit 9d4cd21
Show file tree
Hide file tree
Showing 4 changed files with 236 additions and 21 deletions.
156 changes: 156 additions & 0 deletions src/Avatar.test.tsx
Original file line number Diff line number Diff line change
@@ -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 (
<ClientContextProvider
value={{
state: "valid",
disconnected: false,
supportedFeatures: {
reactions: true,
thumbnails: supportsThumbnails ?? true,
},
setClient: vi.fn(),
authenticated: {
client,
isPasswordlessUser: true,
changePassword: vi.fn(),
logout: vi.fn(),
},
}}
>
{children}
</ClientContextProvider>
);
};

afterEach(() => {
vi.unstubAllGlobals();
});

test("should just render a placeholder when the user has no avatar", () => {
const client = vi.mocked<MatrixClient>({
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(
<TestComponent client={client}>
<Avatar
id={member.userId}
name={displayName}
size={96}
src={member.getMxcAvatarUrl()}
/>
</TestComponent>,
);
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<MatrixClient>({
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(
<TestComponent client={client} supportsThumbnails={false}>
<Avatar
id={member.userId}
name={displayName}
size={96}
src={member.getMxcAvatarUrl()}
/>
</TestComponent>,
);
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<MatrixClient>({
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(
<TestComponent client={client}>
<Avatar
id={member.userId}
name={displayName}
size={96}
src={member.getMxcAvatarUrl()}
/>
</TestComponent>,
);

// 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}` },
});
});
80 changes: 71 additions & 9 deletions src/Avatar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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<Props> = ({
className,
id,
Expand All @@ -45,7 +67,7 @@ export const Avatar: FC<Props> = ({
style,
...props
}) => {
const { client } = useClient();
const clientState = useClientState();

const sizePx = useMemo(
() =>
Expand All @@ -55,18 +77,58 @@ export const Avatar: FC<Props> = ({
[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<string | undefined>(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 (
<CompoundAvatar
className={className}
id={id}
name={name}
size={`${sizePx}px`}
src={resolvedSrc}
src={avatarUrl}
style={style}
{...props}
/>
Expand Down
9 changes: 9 additions & 0 deletions src/ClientContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export type ValidClientState = {
disconnected: boolean;
supportedFeatures: {
reactions: boolean;
thumbnails: boolean;
};
setClient: (params?: SetClientParams) => void;
};
Expand All @@ -71,6 +72,8 @@ export type SetClientParams = {

const ClientContext = createContext<ClientState | undefined>(undefined);

export const ClientContextProvider = ClientContext.Provider;

export const useClientState = (): ClientState | undefined =>
useContext(ClientContext);

Expand Down Expand Up @@ -253,6 +256,7 @@ export const ClientProvider: FC<Props> = ({ children }) => {

const [isDisconnected, setIsDisconnected] = useState(false);
const [supportsReactions, setSupportsReactions] = useState(false);
const [supportsThumbnails, setSupportsThumbnails] = useState(false);

const state: ClientState | undefined = useMemo(() => {
if (alreadyOpenedErr) {
Expand All @@ -278,6 +282,7 @@ export const ClientProvider: FC<Props> = ({ children }) => {
disconnected: isDisconnected,
supportedFeatures: {
reactions: supportsReactions,
thumbnails: supportsThumbnails,
},
};
}, [
Expand All @@ -288,6 +293,7 @@ export const ClientProvider: FC<Props> = ({ children }) => {
setClient,
isDisconnected,
supportsReactions,
supportsThumbnails,
]);

const onSync = useCallback(
Expand All @@ -313,6 +319,8 @@ export const ClientProvider: FC<Props> = ({ 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",
);
Expand All @@ -334,6 +342,7 @@ export const ClientProvider: FC<Props> = ({ children }) => {
}
} else {
setSupportsReactions(true);
setSupportsThumbnails(true);
}

return (): void => {
Expand Down
12 changes: 0 additions & 12 deletions src/utils/matrix.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)!;
}

0 comments on commit 9d4cd21

Please sign in to comment.