Merge commit from fork

This PR fixes a security issue where authenticated users could access and modify data belonging to
other users. The isOwnProfileOrAdmin() middleware was missing from several push subscription API
routes. As a result, any authenticated user on the instance could manipulate the userId parameter in
the URL to view or delete the push subscriptions of other users.

server/routes/user/index.ts

@@ -22,6 +22,7 @@ import { getSettings } from '@server/lib/settings';
 import logger from '@server/logger';
 import { isAuthenticated } from '@server/middleware/auth';
 import { getHostname } from '@server/utils/getHostname';
+import { isOwnProfileOrAdmin } from '@server/utils/profileMiddleware';
 import { Router } from 'express';
 import gravatarUrl from 'gravatar-url';
 import { findIndex, sortBy } from 'lodash';
@@ -275,15 +276,16 @@ router.post<
   }
 });
 
-router.get<{ userId: number }>(
+router.get<{ userId: string }>(
   '/:userId/pushSubscriptions',
+  isOwnProfileOrAdmin(),
   async (req, res, next) => {
     try {
       const userPushSubRepository = getRepository(UserPushSubscription);
 
       const userPushSubs = await userPushSubRepository.find({
         relations: { user: true },
-        where: { user: { id: req.params.userId } },
+        where: { user: { id: Number(req.params.userId) } },
       });
 
       return res.status(200).json(userPushSubs);
@@ -293,8 +295,9 @@ router.get<{ userId: number }>(
   }
 );
 
-router.get<{ userId: number; endpoint: string }>(
+router.get<{ userId: string; endpoint: string }>(
   '/:userId/pushSubscription/:endpoint',
+  isOwnProfileOrAdmin(),
   async (req, res, next) => {
     try {
       const userPushSubRepository = getRepository(UserPushSubscription);
@@ -304,7 +307,7 @@ router.get<{ userId: number; endpoint: string }>(
           user: true,
         },
         where: {
-          user: { id: req.params.userId },
+          user: { id: Number(req.params.userId) },
           endpoint: req.params.endpoint,
         },
       });
@@ -316,8 +319,9 @@ router.get<{ userId: number; endpoint: string }>(
   }
 );
 
-router.delete<{ userId: number; endpoint: string }>(
+router.delete<{ userId: string; endpoint: string }>(
   '/:userId/pushSubscription/:endpoint',
+  isOwnProfileOrAdmin(),
   async (req, res, next) => {
     try {
       const userPushSubRepository = getRepository(UserPushSubscription);
@@ -325,7 +329,7 @@ router.delete<{ userId: number; endpoint: string }>(
       const userPushSub = await userPushSubRepository.findOne({
         relations: { user: true },
         where: {
-          user: { id: req.params.userId },
+          user: { id: Number(req.params.userId) },
           endpoint: req.params.endpoint,
         },
       });
@@ -747,18 +751,8 @@ router.get<{ id: string }, QuotaResponse>(
 
 router.get<{ id: string }, UserWatchDataResponse>(
   '/:id/watch_data',
+  isOwnProfileOrAdmin(),
   async (req, res, next) => {
-    if (
-      Number(req.params.id) !== req.user?.id &&
-      !req.user?.hasPermission(Permission.ADMIN)
-    ) {
-      return next({
-        status: 403,
-        message:
-          "You do not have permission to view this user's recently watched media.",
-      });
-    }
-
     const settings = getSettings().tautulli;
 
     if (!settings.hostname || !settings.port || !settings.apiKey) {

server/routes/user/usersettings.ts

@@ -16,40 +16,15 @@ import logger from '@server/logger';
 import { isAuthenticated } from '@server/middleware/auth';
 import { ApiError } from '@server/types/error';
 import { getHostname } from '@server/utils/getHostname';
+import {
+  isOwnProfile,
+  isOwnProfileOrAdmin,
+} from '@server/utils/profileMiddleware';
 import { Router } from 'express';
 import net from 'net';
 import { Not } from 'typeorm';
 import { canMakePermissionsChange } from '.';
 
-const isOwnProfile = (): Middleware => {
-  return (req, res, next) => {
-    if (req.user?.id !== Number(req.params.id)) {
-      return next({
-        status: 403,
-        message: "You do not have permission to view this user's settings.",
-      });
-    }
-    next();
-  };
-};
-
-const isOwnProfileOrAdmin = (): Middleware => {
-  const authMiddleware: Middleware = (req, res, next) => {
-    if (
-      !req.user?.hasPermission(Permission.MANAGE_USERS) &&
-      req.user?.id !== Number(req.params.id)
-    ) {
-      return next({
-        status: 403,
-        message: "You do not have permission to view this user's settings.",
-      });
-    }
-
-    next();
-  };
-  return authMiddleware;
-};
-
 const userSettingsRoutes = Router({ mergeParams: true });
 
 userSettingsRoutes.get<{ id: string }, UserSettingsGeneralResponse>(

server/utils/profileMiddleware.ts

@@ -0,0 +1,30 @@
+import { Permission } from '@server/lib/permissions';
+
+export const isOwnProfile = (): Middleware => {
+  return (req, res, next) => {
+    if (req.user?.id !== Number(req.params.id)) {
+      return next({
+        status: 403,
+        message: "You do not have permission to view this user's settings.",
+      });
+    }
+    next();
+  };
+};
+
+export const isOwnProfileOrAdmin = (): Middleware => {
+  const authMiddleware: Middleware = (req, res, next) => {
+    if (
+      !req.user?.hasPermission(Permission.MANAGE_USERS) &&
+      req.user?.id !== Number(req.params.id)
+    ) {
+      return next({
+        status: 403,
+        message: "You do not have permission to view this user's settings.",
+      });
+    }
+
+    next();
+  };
+  return authMiddleware;
+};