feat: Fetch avatars using user id (#33214)

Co-authored-by: Marcos Spessatto Defendi <[email protected]>

.changeset/plenty-snakes-dream.md

@@ -0,0 +1,5 @@
+---
+"@rocket.chat/meteor": minor
+---
+
+Adds a new route to allow fetching avatars by the user's id `/avatar/uid/<UserID>`

apps/meteor/server/models/raw/Avatars.ts

@@ -1,11 +1,15 @@
-import type { IAvatar, RocketChatRecordDeleted } from '@rocket.chat/core-typings';
+import type { IAvatar, RocketChatRecordDeleted, IUser } from '@rocket.chat/core-typings';
 import type { IAvatarsModel } from '@rocket.chat/model-typings';
-import type { Collection, Db } from 'mongodb';
+import type { Collection, Db, FindOptions } from 'mongodb';
 
 import { BaseUploadModelRaw } from './BaseUploadModel';
 
 export class AvatarsRaw extends BaseUploadModelRaw implements IAvatarsModel {
 	constructor(db: Db, trash?: Collection<RocketChatRecordDeleted<IAvatar>>) {
 		super(db, 'avatars', trash);
 	}
+
+	findOneByUserId(userId: IUser['_id'], options?: FindOptions<IAvatar>) {
+		return this.findOne({ userId }, options);
+	}
 }

apps/meteor/server/models/raw/BaseUploadModel.ts

@@ -19,6 +19,7 @@ type T = IUpload;
 export abstract class BaseUploadModelRaw extends BaseRaw<T> implements IBaseUploadsModel<T> {
 	protected modelIndexes(): IndexDescription[] {
 		return [
+			{ key: { userId: 1 }, sparse: true },
 			{ key: { name: 1 }, sparse: true },
 			{ key: { rid: 1 }, sparse: true },
 			{ key: { expiresAt: 1 }, sparse: true },

apps/meteor/server/routes/avatar/index.ts

@@ -1,9 +1,10 @@
 import { WebApp } from 'meteor/webapp';
 
 import { roomAvatar } from './room';
-import { userAvatarByUsername } from './user';
+import { userAvatarByUsername, userAvatarById } from './user';
 
 import './middlewares';
 
 WebApp.connectHandlers.use('/avatar/room/', roomAvatar);
+WebApp.connectHandlers.use('/avatar/uid/', userAvatarById);
 WebApp.connectHandlers.use('/avatar/', userAvatarByUsername);

apps/meteor/server/routes/avatar/middlewares/auth.spec.ts

@@ -10,7 +10,7 @@ const mocks = {
 	},
 };
 
-const { protectAvatarsWithFallback } = proxyquire.noCallThru().load('./auth.ts', {
+const { protectAvatarsWithFallback, protectAvatars } = proxyquire.noCallThru().load('./auth.ts', {
 	'../utils': mocks.utils,
 });
 
@@ -54,33 +54,70 @@ describe('#protectAvatarsWithFallback()', () => {
 		expect(response.end.calledOnce).to.be.true;
 	});
 
-	it(`should write 200 to head and write fallback to body (user avatar)`, async () => {
+	it(`should write 200 to head and write fallback to body (room avatar)`, async () => {
 		mocks.utils.renderSVGLetters.returns('fallback');
 
-		await protectAvatarsWithFallback({ url: '/jon' }, response, next);
+		await protectAvatarsWithFallback({ url: '/room/jon' }, response, next);
 		expect(next.called).to.be.false;
 		expect(response.setHeader.called).to.be.false;
 		expect(response.writeHead.calledWith(200, { 'Content-Type': 'image/svg+xml' })).to.be.true;
 		expect(response.write.calledWith('fallback')).to.be.true;
 		expect(response.end.calledOnce).to.be.true;
 	});
 
-	it(`should write 200 to head and write fallback to body (room avatar)`, async () => {
-		mocks.utils.renderSVGLetters.returns('fallback');
+	it(`should call next if user can access avatar`, async () => {
+		mocks.utils.userCanAccessAvatar.returns(true);
+		const request = { url: '/jon' };
+
+		await protectAvatarsWithFallback(request, response, next);
+		expect(mocks.utils.userCanAccessAvatar.calledWith(request)).to.be.true;
+		expect(next.called).to.be.true;
+	});
+});
+
+describe('#protectAvatars()', () => {
+	const response = {
+		setHeader: sinon.spy(),
+		writeHead: sinon.spy(),
+		write: sinon.spy(),
+		end: sinon.spy(),
+	};
+	const next = sinon.spy();
+
+	afterEach(() => {
+		response.setHeader.resetHistory();
+		response.writeHead.resetHistory();
+		response.end.resetHistory();
+		next.resetHistory();
+
+		Object.values(mocks.utils).forEach((mock) => mock.reset());
+	});
+
+	it(`should write 404 to head if no url provided`, async () => {
+		await protectAvatars({}, response, next);
 
-		await protectAvatarsWithFallback({ url: '/room/jon' }, response, next);
 		expect(next.called).to.be.false;
 		expect(response.setHeader.called).to.be.false;
-		expect(response.writeHead.calledWith(200, { 'Content-Type': 'image/svg+xml' })).to.be.true;
-		expect(response.write.calledWith('fallback')).to.be.true;
+		expect(response.writeHead.calledWith(404)).to.be.true;
+		expect(response.end.calledOnce).to.be.true;
+	});
+
+	it(`should write 404 to head if access is denied`, async () => {
+		mocks.utils.userCanAccessAvatar.returns(false);
+
+		await protectAvatars({ url: '/room/jon' }, response, next);
+
+		expect(next.called).to.be.false;
+		expect(response.setHeader.called).to.be.false;
+		expect(response.writeHead.calledWith(404)).to.be.true;
 		expect(response.end.calledOnce).to.be.true;
 	});
 
 	it(`should call next if user can access avatar`, async () => {
 		mocks.utils.userCanAccessAvatar.returns(true);
 		const request = { url: '/jon' };
 
-		await protectAvatarsWithFallback(request, response, next);
+		await protectAvatars(request, response, next);
 		expect(mocks.utils.userCanAccessAvatar.calledWith(request)).to.be.true;
 		expect(next.called).to.be.true;
 	});

apps/meteor/server/routes/avatar/middlewares/auth.ts

@@ -42,3 +42,6 @@ const getProtectAvatars = (callback?: typeof renderFallback) => async (req: Inco
 
 // If unauthorized returns the SVG fallback (letter avatar)
 export const protectAvatarsWithFallback = getProtectAvatars(renderFallback);
+
+// Just returns 404
+export const protectAvatars = getProtectAvatars();

apps/meteor/server/routes/avatar/middlewares/index.ts

@@ -1,7 +1,8 @@
 import { WebApp } from 'meteor/webapp';
 
-import { protectAvatarsWithFallback } from './auth';
+import { protectAvatarsWithFallback, protectAvatars } from './auth';
 import { handleBrowserVersionCheck } from './browserVersion';
 
 WebApp.connectHandlers.use(handleBrowserVersionCheck);
+WebApp.connectHandlers.use('/avatar/uid/', protectAvatars);
 WebApp.connectHandlers.use('/avatar/', protectAvatarsWithFallback);

apps/meteor/server/routes/avatar/user.spec.ts

@@ -7,6 +7,7 @@ import sinon from 'sinon';
 const mocks = {
 	settingsGet: sinon.stub(),
 	findOneByUsernameIgnoringCase: sinon.stub(),
+	findOneById: sinon.stub(),
 	utils: {
 		serveSvgAvatarInRequestedFormat: sinon.spy(),
 		wasFallbackModified: sinon.stub(),
@@ -15,15 +16,18 @@ const mocks = {
 	},
 	serverFetch: sinon.stub(),
 	avatarFindOneByName: sinon.stub(),
+	avatarFindOneByUserId: sinon.stub(),
 };
 
-const { userAvatarByUsername } = proxyquire.noCallThru().load('./user', {
+const { userAvatarById, userAvatarByUsername } = proxyquire.noCallThru().load('./user', {
 	'@rocket.chat/models': {
 		Users: {
 			findOneByUsernameIgnoringCase: mocks.findOneByUsernameIgnoringCase,
+			findOneById: mocks.findOneById,
 		},
 		Avatars: {
 			findOneByName: mocks.avatarFindOneByName,
+			findOneByUserId: mocks.avatarFindOneByUserId,
 		},
 	},
 	'../../../app/settings/server': {
@@ -37,6 +41,138 @@ const { userAvatarByUsername } = proxyquire.noCallThru().load('./user', {
 	},
 });
 
+describe('#userAvatarById()', () => {
+	const response = {
+		setHeader: sinon.spy(),
+		writeHead: sinon.spy(),
+		end: sinon.spy(),
+	};
+	const next = sinon.spy();
+
+	afterEach(() => {
+		mocks.settingsGet.reset();
+		mocks.avatarFindOneByUserId.reset();
+
+		response.setHeader.resetHistory();
+		response.writeHead.resetHistory();
+		response.end.resetHistory();
+		next.resetHistory();
+
+		Object.values(mocks.utils).forEach((mock) => ('reset' in mock ? mock.reset() : mock.resetHistory()));
+	});
+
+	it(`should do nothing if url is not in request object`, async () => {
+		await userAvatarById({}, response, next);
+		expect(next.called).to.be.false;
+		expect(response.setHeader.called).to.be.false;
+		expect(response.writeHead.called).to.be.false;
+		expect(response.end.called).to.be.false;
+	});
+
+	it(`should write 404 if Id is not provided`, async () => {
+		await userAvatarById({ url: '/' }, response, next);
+		expect(next.called).to.be.false;
+		expect(response.setHeader.called).to.be.false;
+		expect(response.writeHead.calledWith(404)).to.be.true;
+		expect(response.end.calledOnce).to.be.true;
+	});
+
+	it(`should call external provider`, async () => {
+		const userId = 'xvf5Tr34';
+		const request = { url: `/${userId}` };
+
+		const pipe = sinon.spy();
+		const mockResponseHeaders = new Headers();
+		mockResponseHeaders.set('header1', 'true');
+		mockResponseHeaders.set('header2', 'false');
+
+		mocks.serverFetch.returns({
+			headers: mockResponseHeaders,
+			body: { pipe },
+		});
+
+		mocks.settingsGet.returns('test123/{username}');
+
+		mocks.findOneById.returns({ username: 'jon' });
+
+		await userAvatarById(request, response, next);
+
+		expect(mocks.utils.setCacheAndDispositionHeaders.calledWith(request, response)).to.be.true;
+		expect(mocks.findOneById.calledWith(userId)).to.be.true;
+		expect(mocks.serverFetch.calledWith('test123/jon')).to.be.true;
+		expect(response.setHeader.calledTwice).to.be.true;
+		expect(response.setHeader.getCall(0).calledWith('header1', 'true')).to.be.true;
+		expect(response.setHeader.getCall(1).calledWith('header2', 'false')).to.be.true;
+		expect(pipe.calledWith(response)).to.be.true;
+	});
+
+	it(`should serve avatar file if found`, async () => {
+		const request = { url: '/jon' };
+
+		const file = { uploadedAt: new Date(0), type: 'image/png', size: 100 };
+		mocks.avatarFindOneByUserId.returns(file);
+
+		await userAvatarById(request, response, next);
+
+		expect(mocks.utils.setCacheAndDispositionHeaders.calledWith(request, response)).to.be.true;
+		expect(mocks.utils.serveAvatarFile.calledWith(file, request, response, next)).to.be.true;
+	});
+
+	it(`should write 304 to head if content is not modified`, async () => {
+		const request = { url: '/xyzabc', headers: {} };
+
+		mocks.utils.wasFallbackModified.returns(false);
+
+		await userAvatarById(request, response, next);
+
+		expect(mocks.utils.setCacheAndDispositionHeaders.calledWith(request, response)).to.be.true;
+		expect(response.writeHead.calledWith(304)).to.be.true;
+		expect(response.end.calledOnce).to.be.true;
+	});
+
+	it(`should write 404 if userId is not found`, async () => {
+		mocks.utils.wasFallbackModified.returns(true);
+		mocks.findOneById.returns(null);
+
+		const userId = 'awdasdaw';
+		const request = { url: `/${userId}`, headers: {} };
+
+		await userAvatarById(request, response, next);
+		expect(mocks.utils.setCacheAndDispositionHeaders.calledWith(request, response)).to.be.true;
+
+		expect(response.writeHead.calledWith(404)).to.be.true;
+		expect(response.end.calledOnce).to.be.true;
+	});
+
+	it(`should fallback to SVG if no avatar found`, async () => {
+		const userId = '2apso9283';
+		const request = { url: `/${userId}`, headers: {} };
+
+		mocks.findOneById.returns({ username: 'jon' });
+		mocks.utils.wasFallbackModified.returns(true);
+
+		await userAvatarById(request, response, next);
+
+		expect(mocks.findOneById.calledWith(userId)).to.be.true;
+		expect(mocks.utils.setCacheAndDispositionHeaders.calledWith(request, response)).to.be.true;
+		expect(mocks.utils.serveSvgAvatarInRequestedFormat.calledWith({ nameOrUsername: 'jon', req: request, res: response })).to.be.true;
+	});
+
+	it(`should fallback to SVG with user name if UI_Use_Name_Avatar is true`, async () => {
+		const userId = '2apso9283';
+		const request = { url: `/${userId}`, headers: {} };
+
+		mocks.findOneById.returns({ username: 'jon', name: 'Doe' });
+		mocks.utils.wasFallbackModified.returns(true);
+		mocks.settingsGet.withArgs('UI_Use_Name_Avatar').returns(true);
+
+		await userAvatarById(request, response, next);
+
+		expect(mocks.utils.setCacheAndDispositionHeaders.calledWith(request, response)).to.be.true;
+		expect(mocks.utils.serveSvgAvatarInRequestedFormat.calledWith({ nameOrUsername: 'Doe', req: request, res: response })).to.be.true;
+	});
+});
+
 describe('#userAvatarByUsername()', () => {
 	const response = {
 		setHeader: sinon.spy(),

apps/meteor/server/routes/avatar/user.ts

@@ -1,5 +1,6 @@
 import type { IncomingMessage, ServerResponse } from 'http';
 
+import type { IUser } from '@rocket.chat/apps-engine/definition/users';
 import type { IIncomingMessage } from '@rocket.chat/core-typings';
 import { Avatars, Users } from '@rocket.chat/models';
 import { serverFetch as fetch } from '@rocket.chat/server-fetch';
@@ -72,3 +73,62 @@ export const userAvatarByUsername = async function (request: IncomingMessage, re
 
 	serveSvgAvatarInRequestedFormat({ nameOrUsername: requestUsername, req, res });
 };
+
+export const userAvatarById = async function (request: IncomingMessage, res: ServerResponse, next: NextFunction) {
+	const req = request as IIncomingMessage;
+
+	if (!req.url) {
+		return;
+	}
+
+	// replace removes the query string
+	const requestUserId = decodeURIComponent(req.url.slice(1).replace(/\?.*$/, ''));
+	if (!requestUserId) {
+		res.writeHead(404);
+		res.end();
+		return;
+	}
+
+	setCacheAndDispositionHeaders(req, res);
+
+	const externalProviderUrl = settings.get<string>('Accounts_AvatarExternalProviderUrl');
+	if (externalProviderUrl) {
+		const user = await Users.findOneById<Pick<IUser, 'username'>>(requestUserId, { projection: { username: 1 } });
+
+		if (!user?.username) {
+			res.writeHead(404);
+			res.end();
+			return;
+		}
+
+		void handleExternalProvider(externalProviderUrl, user.username, res);
+		return;
+	}
+
+	const file = await Avatars.findOneByUserId(requestUserId);
+	if (file) {
+		void serveAvatarFile(file, req, res, next);
+		return;
+	}
+
+	if (!wasFallbackModified(req.headers['if-modified-since'])) {
+		res.writeHead(304);
+		res.end();
+		return;
+	}
+
+	const user = await Users.findOneById<Pick<IUser, 'name' | 'username'>>(requestUserId, { projection: { username: 1, name: 1 } });
+	if (!user?.username) {
+		res.writeHead(404);
+		res.end();
+		return;
+	}
+
+	// Use real name for SVG letters
+	if (settings.get('UI_Use_Name_Avatar') && user?.name) {
+		serveSvgAvatarInRequestedFormat({ nameOrUsername: user.name, req, res });
+		return;
+	}
+
+	serveSvgAvatarInRequestedFormat({ nameOrUsername: user.username, req, res });
+};

packages/model-typings/src/models/IAvatarsModel.ts

@@ -1,5 +1,8 @@
-import type { IAvatar } from '@rocket.chat/core-typings';
+import type { IAvatar, IUser } from '@rocket.chat/core-typings';
+import type { FindOptions } from 'mongodb';
 
 import type { IBaseUploadsModel } from './IBaseUploadsModel';
 
-export type IAvatarsModel = IBaseUploadsModel<IAvatar>;
+export interface IAvatarsModel extends IBaseUploadsModel<IAvatar> {
+	findOneByUserId(userId: IUser['_id'], options?: FindOptions<IAvatarsModel>): Promise<IAvatar | null>;
+}