Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: Local avatars prioritized over external avatar provider and remove remnant references on client of Accounts_AvatarExternalProviderUrl #33296

Merged
merged 6 commits into from
Sep 18, 2024
Merged

fix: Local avatars prioritized over external avatar provider and remove remnant references on client of Accounts_AvatarExternalProviderUrl #33296

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f8dd991
Remove remnant references of `Accounts_AvatarExternalProviderUrl` on …
tassoevan Sep 16, 2024
817bc7e
Remove redundant CDN handling
tassoevan Sep 16, 2024
b23ec89
fix: Avatar external provider URL wrong priority
gabriellsh Sep 16, 2024
bf13e15
Update CS
gabriellsh Sep 16, 2024
42c43e0
Merge branch 'develop' into fix/external-avatars
kodiakhq[bot] Sep 17, 2024
7261aaf
Merge branch 'develop' into fix/external-avatars
kodiakhq[bot] Sep 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .changeset/strong-grapes-brake.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
'@rocket.chat/meteor': patch
---

Fixed remaining direct references to external user avatar URLs

Fixed local avatars having priority over external provider

It mainly corrects the behavior of E2E encryption messages and desktop notifications.
5 changes: 0 additions & 5 deletions apps/meteor/app/utils/client/getUserAvatarURL.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
import { settings } from '../../settings/client';
import { getAvatarURL } from './getAvatarURL';

export const getUserAvatarURL = function (username: string, cache = ''): string | undefined {
const externalSource = (settings.get<string>('Accounts_AvatarExternalProviderUrl') || '').trim().replace(/\/$/, '');
if (externalSource !== '') {
return externalSource.replace('{username}', username);
}
if (username == null) {
return;
}
Expand Down
5 changes: 0 additions & 5 deletions apps/meteor/app/utils/server/getUserAvatarURL.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
import { settings } from '../../settings/server';
import { getAvatarURL } from './getAvatarURL';

export const getUserAvatarURL = function (username: string, cache = ''): string | undefined {
const externalSource = (settings.get<string>('Accounts_AvatarExternalProviderUrl') || '').trim().replace(/\/$/, '');
if (externalSource !== '') {
return externalSource.replace('{username}', username);
}
if (username == null) {
return;
}
Expand Down
8 changes: 2 additions & 6 deletions apps/meteor/client/providers/AvatarUrlProvider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { AvatarUrlContext, useSetting } from '@rocket.chat/ui-contexts';
import { AvatarUrlContext } from '@rocket.chat/ui-contexts';
import type { ReactNode } from 'react';
import React, { useMemo } from 'react';

Expand All @@ -10,19 +10,15 @@ type AvatarUrlProviderProps = {
};

const AvatarUrlProvider = ({ children }: AvatarUrlProviderProps) => {
const cdnAvatarUrl = String(useSetting('CDN_PREFIX') || '');
const contextValue = useMemo(
() => ({
getUserPathAvatar: ((): ((uid: string, etag?: string) => string) => {
if (cdnAvatarUrl) {
return (uid: string, etag?: string): string => `${cdnAvatarUrl}/avatar/${uid}${etag ? `?etag=${etag}` : ''}`;
}
return (uid: string, etag?: string): string => getURL(`/avatar/${uid}${etag ? `?etag=${etag}` : ''}`);
})(),
getRoomPathAvatar: ({ type, ...room }: any): string =>
roomCoordinator.getRoomDirectives(type || room.t).getAvatarPath({ username: room._id, ...room }) || '',
}),
[cdnAvatarUrl],
[],
);

return <AvatarUrlContext.Provider children={children} value={contextValue} />;
Expand Down
14 changes: 7 additions & 7 deletions apps/meteor/server/routes/avatar/user.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,13 @@ export const userAvatar = async function (req, res) {
return;
}

if (settings.get('Accounts_AvatarExternalProviderUrl')) {
const response = await fetch(settings.get('Accounts_AvatarExternalProviderUrl').replace('{username}', requestUsername));
response.headers.forEach((value, key) => res.setHeader(key, value));
response.body.pipe(res);
return;
}

const reqModifiedHeader = req.headers['if-modified-since'];

const file = await Avatars.findOneByName(requestUsername);
Expand All @@ -52,13 +59,6 @@ export const userAvatar = async function (req, res) {
return FileUpload.get(file, req, res);
}

if (settings.get('Accounts_AvatarExternalProviderUrl')) {
const response = await fetch(settings.get('Accounts_AvatarExternalProviderUrl').replace('{username}', requestUsername));
response.headers.forEach((value, key) => res.setHeader(key, value));
response.body.pipe(res);
return;
}

// if still using "letters fallback"
if (!wasFallbackModified(reqModifiedHeader, res)) {
res.writeHead(304);
Expand Down
Loading