Skip to content

Commit

Permalink
fix: rewrite avatarproxy and CachedImage (#1016)
Browse files Browse the repository at this point in the history
* fix: rewrite avatarproxy and CachedImage

Avatar proxy was allowing every request to be proxied, no matter the original ressource's origin or
filetype. This PR fixes it be allowing only relevant resources to be cached, i.e. Jellyfin/Emby
images and TMDB images.

fix #1012, #1013

* fix: resolve CodeQL error

* fix: resolve CodeQL error

* fix: resolve review comments

* fix: resolve review comment

* fix: resolve CodeQL error

* fix: update imageproxy path
  • Loading branch information
gauthier-th authored Oct 17, 2024
1 parent a351264 commit 4e48fdf
Show file tree
Hide file tree
Showing 29 changed files with 97 additions and 40 deletions.
15 changes: 4 additions & 11 deletions server/routes/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -262,8 +262,6 @@ authRoutes.post('/jellyfin', async (req, res, next) => {
urlBase: body.urlBase,
});

const { externalHostname } = getSettings().jellyfin;

// Try to find deviceId that corresponds to jellyfin user, else generate a new one
let user = await userRepository.findOne({
where: { jellyfinUsername: body.username },
Expand All @@ -281,11 +279,6 @@ authRoutes.post('/jellyfin', async (req, res, next) => {
// First we need to attempt to log the user in to jellyfin
const jellyfinserver = new JellyfinAPI(hostname ?? '', undefined, deviceId);

const jellyfinHost =
externalHostname && externalHostname.length > 0
? externalHostname
: hostname;

const ip = req.ip;
let clientIp;

Expand Down Expand Up @@ -336,7 +329,7 @@ authRoutes.post('/jellyfin', async (req, res, next) => {
jellyfinAuthToken: account.AccessToken,
permissions: Permission.ADMIN,
avatar: account.User.PrimaryImageTag
? `${jellyfinHost}/Users/${account.User.Id}/Images/Primary/?tag=${account.User.PrimaryImageTag}&quality=90`
? `/Users/${account.User.Id}/Images/Primary/?tag=${account.User.PrimaryImageTag}&quality=90`
: gravatarUrl(body.email || account.User.Name, {
default: 'mm',
size: 200,
Expand All @@ -355,7 +348,7 @@ authRoutes.post('/jellyfin', async (req, res, next) => {
jellyfinAuthToken: account.AccessToken,
permissions: Permission.ADMIN,
avatar: account.User.PrimaryImageTag
? `${jellyfinHost}/Users/${account.User.Id}/Images/Primary/?tag=${account.User.PrimaryImageTag}&quality=90`
? `/Users/${account.User.Id}/Images/Primary/?tag=${account.User.PrimaryImageTag}&quality=90`
: gravatarUrl(body.email || account.User.Name, {
default: 'mm',
size: 200,
Expand Down Expand Up @@ -410,7 +403,7 @@ authRoutes.post('/jellyfin', async (req, res, next) => {
);
// Update the users avatar with their jellyfin profile pic (incase it changed)
if (account.User.PrimaryImageTag) {
const avatar = `${jellyfinHost}/Users/${account.User.Id}/Images/Primary/?tag=${account.User.PrimaryImageTag}&quality=90`;
const avatar = `/Users/${account.User.Id}/Images/Primary/?tag=${account.User.PrimaryImageTag}&quality=90`;
if (avatar !== user.avatar) {
const avatarProxy = new ImageProxy('avatar', '');
avatarProxy.clearCachedImage(user.avatar);
Expand Down Expand Up @@ -467,7 +460,7 @@ authRoutes.post('/jellyfin', async (req, res, next) => {
jellyfinDeviceId: deviceId,
permissions: settings.main.defaultPermissions,
avatar: account.User.PrimaryImageTag
? `${jellyfinHost}/Users/${account.User.Id}/Images/Primary/?tag=${account.User.PrimaryImageTag}&quality=90`
? `/Users/${account.User.Id}/Images/Primary/?tag=${account.User.PrimaryImageTag}&quality=90`
: gravatarUrl(body.email || account.User.Name, {
default: 'mm',
size: 200,
Expand Down
23 changes: 21 additions & 2 deletions server/routes/avatarproxy.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,34 @@
import { MediaServerType } from '@server/constants/server';
import ImageProxy from '@server/lib/imageproxy';
import { getSettings } from '@server/lib/settings';
import logger from '@server/logger';
import { getHostname } from '@server/utils/getHostname';
import { Router } from 'express';

const router = Router();

const avatarImageProxy = new ImageProxy('avatar', '');
// Proxy avatar images
router.get('/*', async (req, res) => {
const imagePath = req.url.startsWith('/') ? req.url.slice(1) : req.url;

let imagePath = '';
try {
const jellyfinAvatar = req.url.match(
/(\/Users\/\w+\/Images\/Primary\/?\?tag=\w+&quality=90)$/
)?.[1];
if (!jellyfinAvatar) {
const mediaServerType = getSettings().main.mediaServerType;
throw new Error(
`Provided URL is not ${
mediaServerType === MediaServerType.JELLYFIN
? 'a Jellyfin'
: 'an Emby'
} avatar.`
);
}

const imageUrl = new URL(jellyfinAvatar, getHostname());
imagePath = imageUrl.toString();

const imageData = await avatarImageProxy.getImage(imagePath);

res.writeHead(200, {
Expand Down
7 changes: 1 addition & 6 deletions server/routes/settings/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -377,11 +377,6 @@ settingsRoutes.get('/jellyfin/library', async (req, res, next) => {

settingsRoutes.get('/jellyfin/users', async (req, res) => {
const settings = getSettings();
const { externalHostname } = settings.jellyfin;
const jellyfinHost =
externalHostname && externalHostname.length > 0
? externalHostname
: getHostname();

const userRepository = getRepository(User);
const admin = await userRepository.findOneOrFail({
Expand All @@ -401,7 +396,7 @@ settingsRoutes.get('/jellyfin/users', async (req, res) => {
username: user.Name,
id: user.Id,
thumb: user.PrimaryImageTag
? `${jellyfinHost}/Users/${user.Id}/Images/Primary/?tag=${user.PrimaryImageTag}&quality=90`
? `/Users/${user.Id}/Images/Primary/?tag=${user.PrimaryImageTag}&quality=90`
: gravatarUrl(user.Name, { default: 'mm', size: 200 }),
email: user.Name,
}));
Expand Down
8 changes: 1 addition & 7 deletions server/routes/user/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -516,12 +516,6 @@ router.post(

//const jellyfinUsersResponse = await jellyfinClient.getUsers();
const createdUsers: User[] = [];
const { externalHostname } = getSettings().jellyfin;

const jellyfinHost =
externalHostname && externalHostname.length > 0
? externalHostname
: hostname;

jellyfinClient.setUserId(admin.jellyfinUserId ?? '');
const jellyfinUsers = await jellyfinClient.getUsers();
Expand All @@ -546,7 +540,7 @@ router.post(
email: jellyfinUser?.Name,
permissions: settings.main.defaultPermissions,
avatar: jellyfinUser?.PrimaryImageTag
? `${jellyfinHost}/Users/${jellyfinUser.Id}/Images/Primary/?tag=${jellyfinUser.PrimaryImageTag}&quality=90`
? `/Users/${jellyfinUser.Id}/Images/Primary/?tag=${jellyfinUser.PrimaryImageTag}&quality=90`
: gravatarUrl(jellyfinUser?.Name ?? '', {
default: 'mm',
size: 200,
Expand Down
3 changes: 3 additions & 0 deletions src/components/Blacklist/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ const BlacklistedItem = ({ item, revalidateList }: BlacklistedItemProps) => {
{title && title.backdropPath && (
<div className="absolute inset-0 z-0 w-full bg-cover bg-center xl:w-2/3">
<CachedImage
type="tmdb"
src={`https://image.tmdb.org/t/p/w1920_and_h800_multi_faces/${title.backdropPath}`}
alt=""
style={{ width: '100%', height: '100%', objectFit: 'cover' }}
Expand All @@ -293,6 +294,7 @@ const BlacklistedItem = ({ item, revalidateList }: BlacklistedItemProps) => {
className="relative h-auto w-12 flex-shrink-0 scale-100 transform-gpu overflow-hidden rounded-md transition duration-300 hover:scale-105"
>
<CachedImage
type="tmdb"
src={
title?.posterPath
? `https://image.tmdb.org/t/p/w600_and_h900_bestv2${title.posterPath}`
Expand Down Expand Up @@ -355,6 +357,7 @@ const BlacklistedItem = ({ item, revalidateList }: BlacklistedItemProps) => {
<Link href={`/users/${item.user.id}`}>
<span className="group flex items-center truncate">
<CachedImage
type="avatar"
src={item.user.avatar}
alt=""
className="avatar-sm ml-1.5"
Expand Down
2 changes: 2 additions & 0 deletions src/components/CollectionDetails/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ const CollectionDetails = ({ collection }: CollectionDetailsProps) => {
{data.backdropPath && (
<div className="media-page-bg-image">
<CachedImage
type="tmdb"
alt=""
src={`https://image.tmdb.org/t/p/w1920_and_h800_multi_faces/${data.backdropPath}`}
style={{ width: '100%', height: '100%', objectFit: 'cover' }}
Expand Down Expand Up @@ -228,6 +229,7 @@ const CollectionDetails = ({ collection }: CollectionDetailsProps) => {
<div className="media-header">
<div className="media-poster">
<CachedImage
type="tmdb"
src={
data.posterPath
? `https://image.tmdb.org/t/p/w600_and_h900_bestv2${data.posterPath}`
Expand Down
32 changes: 21 additions & 11 deletions src/components/Common/CachedImage/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,24 +4,34 @@ import Image from 'next/image';

const imageLoader: ImageLoader = ({ src }) => src;

export type CachedImageProps = ImageProps & {
src: string;
type: 'tmdb' | 'avatar';
};

/**
* The CachedImage component should be used wherever
* we want to offer the option to locally cache images.
**/
const CachedImage = ({ src, ...props }: ImageProps) => {
const CachedImage = ({ src, type, ...props }: CachedImageProps) => {
const { currentSettings } = useSettings();

let imageUrl = src;

if (typeof imageUrl === 'string' && imageUrl.startsWith('http')) {
const parsedUrl = new URL(imageUrl);
let imageUrl: string;

if (parsedUrl.host === 'image.tmdb.org') {
if (currentSettings.cacheImages)
imageUrl = imageUrl.replace('https://image.tmdb.org', '/imageproxy');
} else if (parsedUrl.host !== 'gravatar.com') {
imageUrl = '/avatarproxy/' + imageUrl;
}
if (type === 'tmdb') {
// tmdb stuff
imageUrl =
currentSettings.cacheImages && !src.startsWith('/')
? src.replace(/^https:\/\/image\.tmdb\.org\//, '/imageproxy/')
: src;
} else if (type === 'avatar') {
// jellyfin avatar (in any)
const jellyfinAvatar = src.match(
/(\/Users\/\w+\/Images\/Primary\/?\?tag=\w+&quality=90)$/
)?.[1];
imageUrl = jellyfinAvatar ? `/avatarproxy` + jellyfinAvatar : src;
} else {
return null;
}

return <Image unoptimized loader={imageLoader} src={imageUrl} {...props} />;
Expand Down
1 change: 1 addition & 0 deletions src/components/Common/ImageFader/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ const ImageFader: ForwardRefRenderFunction<HTMLDivElement, ImageFaderProps> = (
{...props}
>
<CachedImage
type="tmdb"
className="absolute inset-0 h-full w-full"
alt=""
src={imageUrl}
Expand Down
1 change: 1 addition & 0 deletions src/components/Common/Modal/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ const Modal = React.forwardRef<HTMLDivElement, ModalProps>(
{backdrop && (
<div className="absolute top-0 left-0 right-0 z-0 h-64 max-h-full w-full">
<CachedImage
type="tmdb"
alt=""
src={backdrop}
style={{ width: '100%', height: '100%', objectFit: 'cover' }}
Expand Down
1 change: 1 addition & 0 deletions src/components/CompanyCard/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ const CompanyCard = ({ image, url, name }: CompanyCardProps) => {
>
<div className="relative h-full w-full">
<CachedImage
type="tmdb"
src={image}
alt={name}
className="relative z-40 h-full w-full"
Expand Down
1 change: 1 addition & 0 deletions src/components/GenreCard/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ const GenreCard = ({ image, url, name, canExpand = false }: GenreCardProps) => {
tabIndex={0}
>
<CachedImage
type="tmdb"
src={image}
alt=""
style={{ width: '100%', height: '100%', objectFit: 'cover' }}
Expand Down
3 changes: 2 additions & 1 deletion src/components/IssueDetails/IssueComment/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,8 @@ const IssueComment = ({
</Transition>
<Link href={isActiveUser ? '/profile' : `/users/${comment.user.id}`}>
<CachedImage
src={`${comment.user.avatar}`}
type="avatar"
src={comment.user.avatar}
alt=""
className="h-10 w-10 scale-100 transform-gpu rounded-full object-cover ring-1 ring-gray-500 transition duration-300 hover:scale-105"
width={40}
Expand Down
5 changes: 4 additions & 1 deletion src/components/IssueDetails/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,7 @@ const IssueDetails = () => {
{data.backdropPath && (
<div className="media-page-bg-image">
<CachedImage
type="tmdb"
alt=""
src={`https://image.tmdb.org/t/p/w1920_and_h800_multi_faces/${data.backdropPath}`}
style={{ width: '100%', height: '100%', objectFit: 'cover' }}
Expand All @@ -235,6 +236,7 @@ const IssueDetails = () => {
<div className="media-header">
<div className="media-poster">
<CachedImage
type="tmdb"
src={
data.posterPath
? `https://image.tmdb.org/t/p/w600_and_h900_bestv2${data.posterPath}`
Expand Down Expand Up @@ -287,7 +289,8 @@ const IssueDetails = () => {
className="group ml-1 inline-flex h-full items-center xl:ml-1.5"
>
<CachedImage
src={`${issueData.createdBy.avatar}`}
type="avatar"
src={issueData.createdBy.avatar}
alt=""
className="mr-0.5 h-5 w-5 scale-100 transform-gpu rounded-full object-cover transition duration-300 group-hover:scale-105 xl:mr-1 xl:h-6 xl:w-6"
width={20}
Expand Down
5 changes: 4 additions & 1 deletion src/components/IssueList/IssueItem/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ const IssueItem = ({ issue }: IssueItemProps) => {
{title.backdropPath && (
<div className="absolute inset-0 z-0 w-full bg-cover bg-center xl:w-2/3">
<CachedImage
type="tmdb"
src={`https://image.tmdb.org/t/p/w1920_and_h800_multi_faces/${title.backdropPath}`}
alt=""
style={{ width: '100%', height: '100%', objectFit: 'cover' }}
Expand All @@ -137,6 +138,7 @@ const IssueItem = ({ issue }: IssueItemProps) => {
className="relative h-auto w-12 flex-shrink-0 scale-100 transform-gpu overflow-hidden rounded-md transition duration-300 hover:scale-105"
>
<CachedImage
type="tmdb"
src={
title.posterPath
? `https://image.tmdb.org/t/p/w600_and_h900_bestv2${title.posterPath}`
Expand Down Expand Up @@ -226,7 +228,8 @@ const IssueItem = ({ issue }: IssueItemProps) => {
className="group flex items-center truncate"
>
<CachedImage
src={'/avatarproxy/' + issue.createdBy.avatar}
type="avatar"
src={issue.createdBy.avatar}
alt=""
className="avatar-sm ml-1.5 object-cover"
width={20}
Expand Down
2 changes: 2 additions & 0 deletions src/components/Layout/UserDropdown/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ const UserDropdown = () => {
data-testid="user-menu"
>
<CachedImage
type="avatar"
className="h-8 w-8 rounded-full object-cover sm:h-10 sm:w-10"
src={user ? user.avatar : ''}
alt=""
Expand All @@ -80,6 +81,7 @@ const UserDropdown = () => {
<div className="flex flex-col space-y-4 px-4 py-4">
<div className="flex items-center space-x-2">
<CachedImage
type="avatar"
className="h-8 w-8 rounded-full object-cover sm:h-10 sm:w-10"
src={user ? user.avatar : ''}
alt=""
Expand Down
2 changes: 2 additions & 0 deletions src/components/ManageSlideOver/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,7 @@ const ManageSlideOver = ({
content={user.displayName}
>
<CachedImage
type="avatar"
src={user.avatar}
alt={user.displayName}
className="h-8 w-8 scale-100 transform-gpu rounded-full object-cover ring-1 ring-gray-500 transition duration-300 hover:scale-105"
Expand Down Expand Up @@ -530,6 +531,7 @@ const ManageSlideOver = ({
content={user.displayName}
>
<CachedImage
type="avatar"
src={user.avatar}
alt={user.displayName}
className="h-8 w-8 scale-100 transform-gpu rounded-full object-cover ring-1 ring-gray-500 transition duration-300 hover:scale-105"
Expand Down
3 changes: 3 additions & 0 deletions src/components/MovieDetails/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,7 @@ const MovieDetails = ({ movie }: MovieDetailsProps) => {
{data.backdropPath && (
<div className="media-page-bg-image">
<CachedImage
type="tmdb"
alt=""
src={`https://image.tmdb.org/t/p/w1920_and_h800_multi_faces/${data.backdropPath}`}
style={{ width: '100%', height: '100%', objectFit: 'cover' }}
Expand Down Expand Up @@ -494,6 +495,7 @@ const MovieDetails = ({ movie }: MovieDetailsProps) => {
<div className="media-header">
<div className="media-poster">
<CachedImage
type="tmdb"
src={
data.posterPath
? `https://image.tmdb.org/t/p/w600_and_h900_bestv2${data.posterPath}`
Expand Down Expand Up @@ -741,6 +743,7 @@ const MovieDetails = ({ movie }: MovieDetailsProps) => {
<div className="group relative z-0 scale-100 transform-gpu cursor-pointer overflow-hidden rounded-lg bg-gray-800 bg-cover bg-center shadow-md ring-1 ring-gray-700 transition duration-300 hover:scale-105 hover:ring-gray-500">
<div className="absolute inset-0 z-0">
<CachedImage
type="tmdb"
src={`https://image.tmdb.org/t/p/w1440_and_h320_multi_faces/${data.collection.backdropPath}`}
alt=""
style={{
Expand Down
1 change: 1 addition & 0 deletions src/components/PersonCard/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ const PersonCard = ({
{profilePath ? (
<div className="relative h-full w-3/4 overflow-hidden rounded-full ring-1 ring-gray-700">
<CachedImage
type="tmdb"
src={`https://image.tmdb.org/t/p/w600_and_h900_bestv2${profilePath}`}
alt=""
style={{
Expand Down
1 change: 1 addition & 0 deletions src/components/PersonDetails/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ const PersonDetails = () => {
{data.profilePath && (
<div className="relative mb-6 mr-0 h-36 w-36 flex-shrink-0 overflow-hidden rounded-full ring-1 ring-gray-700 lg:mb-0 lg:mr-6 lg:h-44 lg:w-44">
<CachedImage
type="tmdb"
src={`https://image.tmdb.org/t/p/w600_and_h900_bestv2${data.profilePath}`}
alt=""
style={{ width: '100%', height: '100%', objectFit: 'cover' }}
Expand Down
Loading

0 comments on commit 4e48fdf

Please sign in to comment.