fix: do not allow editing of user settings under certain conditions (#1168)

* fix: do not allow editing of user settings under certain conditions * feat(lang): generate translation keys for new strings * refactor: modify owner check for clarity * fix(ui): hide buttons where appropriate and add missing translation string
2025-09-17 17:24:35 +02:00 · 2021-03-14 00:46:12 -05:00
parent 0bd0912613
commit 001dcd328c
7 changed files with 122 additions and 57 deletions
--- a/server/routes/user/usersettings.ts
+++ b/server/routes/user/usersettings.ts
@@ -73,6 +73,14 @@ userSettingsRoutes.post<
      return next({ status: 404, message: 'User not found.' });
    }

+    // "Owner" user settings cannot be modified by other users
+    if (user.id === 1 && req.user?.id !== 1) {
+      return next({
+        status: 403,
+        message: "You do not have permission to modify this user's settings.",
+      });
+    }
+
    user.username = req.body.username;
    if (!user.settings) {
      user.settings = new UserSettings({
@@ -240,6 +248,14 @@ userSettingsRoutes.post<
      return next({ status: 404, message: 'User not found.' });
    }

+    // "Owner" user settings cannot be modified by other users
+    if (user.id === 1 && req.user?.id !== 1) {
+      return next({
+        status: 403,
+        message: "You do not have permission to modify this user's settings.",
+      });
+    }
+
    if (!user.settings) {
      user.settings = new UserSettings({
        user: req.user,
@@ -309,8 +325,8 @@ userSettingsRoutes.post<
        return next({ status: 404, message: 'User not found.' });
      }

-      // Only let the owner user modify themselves
-      if (user.id === 1 && req.user?.id !== 1) {
+      // "Owner" user permissions cannot be modified, and users cannot set their own permissions
+      if (user.id === 1 || req.user?.id === user.id) {
        return next({
          status: 403,
          message: 'You do not have permission to modify this user',
--- a/src/components/UserList/index.tsx
+++ b/src/components/UserList/index.tsx
@@ -559,6 +559,7 @@ const UserList: React.FC = () => {
              <Table.TD alignText="right">
                <Button
                  buttonType="warning"
+                  disabled={user.id === 1 && currentUser?.id !== 1}
                  className="mr-2"
                  onClick={() =>
                    router.push(
@@ -571,7 +572,11 @@ const UserList: React.FC = () => {
                </Button>
                <Button
                  buttonType="danger"
-                  disabled={hasPermission(Permission.ADMIN, user.permissions)}
+                  disabled={
+                    user.id === 1 ||
+                    (currentUser?.id !== 1 &&
+                      hasPermission(Permission.ADMIN, user.permissions))
+                  }
                  onClick={() => setDeleteModal({ isOpen: true, user })}
                >
                  {intl.formatMessage(messages.delete)}
--- a/src/components/UserProfile/ProfileHeader/index.tsx
+++ b/src/components/UserProfile/ProfileHeader/index.tsx
@@ -82,7 +82,7 @@ const ProfileHeader: React.FC<ProfileHeaderProps> = ({
      </div>
      <div className="flex flex-col-reverse mt-6 space-y-4 space-y-reverse justify-stretch sm:flex-row-reverse sm:justify-end sm:space-x-reverse sm:space-y-0 sm:space-x-3 md:mt-0 md:flex-row md:space-x-3">
        {(loggedInUser?.id === user.id ||
-          hasPermission(Permission.MANAGE_USERS)) &&
+          (user.id !== 1 && hasPermission(Permission.MANAGE_USERS))) &&
        !isSettingsPage ? (
          <Link
            href={
@@ -109,6 +109,7 @@ const ProfileHeader: React.FC<ProfileHeaderProps> = ({
            </Button>
          </Link>
        ) : (
+          isSettingsPage && (
            <Link
              href={
                loggedInUser?.id === user.id ? `/profile` : `/users/${user.id}`
@@ -131,6 +132,7 @@ const ProfileHeader: React.FC<ProfileHeaderProps> = ({
                {intl.formatMessage(messages.profile)}
              </Button>
            </Link>
+          )
        )}
      </div>
    </div>
--- a/src/components/UserProfile/UserSettings/UserPermissions/index.tsx
+++ b/src/components/UserProfile/UserSettings/UserPermissions/index.tsx
@@ -10,6 +10,7 @@ import Error from '../../../../pages/_error';
 import Button from '../../../Common/Button';
 import LoadingSpinner from '../../../Common/LoadingSpinner';
 import PermissionEdit from '../../../PermissionEdit';
+import Alert from '../../../Common/Alert';

 const messages = defineMessages({
  displayName: 'Display Name',
@@ -20,6 +21,8 @@ const messages = defineMessages({
  toastSettingsSuccess: 'Settings successfully saved!',
  toastSettingsFailure: 'Something went wrong while saving settings.',
  permissions: 'Permissions',
+  unauthorized: 'Unauthorized',
+  unauthorizedDescription: 'You cannot modify your own permissions.',
 });

 const UserPermissions: React.FC = () => {
@@ -40,6 +43,21 @@ const UserPermissions: React.FC = () => {
    return <Error statusCode={500} />;
  }

+  if (currentUser?.id !== 1 && currentUser?.id === user?.id) {
+    return (
+      <>
+        <div className="mb-6">
+          <h3 className="heading">
+            {intl.formatMessage(messages.permissions)}
+          </h3>
+        </div>
+        <Alert title={intl.formatMessage(messages.unauthorized)} type="error">
+          {intl.formatMessage(messages.unauthorizedDescription)}
+        </Alert>
+      </>
+    );
+  }
+
  return (
    <>
      <div className="mb-6">
--- a/src/components/UserProfile/UserSettings/index.tsx
+++ b/src/components/UserProfile/UserSettings/index.tsx
@@ -9,6 +9,7 @@ import LoadingSpinner from '../../Common/LoadingSpinner';
 import PageTitle from '../../Common/PageTitle';
 import ProfileHeader from '../ProfileHeader';
 import useSettings from '../../../hooks/useSettings';
+import Alert from '../../Common/Alert';

 const messages = defineMessages({
  settings: 'User Settings',
@@ -16,6 +17,9 @@ const messages = defineMessages({
  menuChangePass: 'Password',
  menuNotifications: 'Notifications',
  menuPermissions: 'Permissions',
+  unauthorized: 'Unauthorized',
+  unauthorizedDescription:
+    "You do not have permission to modify this user's settings.",
 });

 interface SettingsRoute {
@@ -24,6 +28,7 @@ interface SettingsRoute {
  regex: RegExp;
  requiredPermission?: Permission | Permission[];
  permissionType?: { type: 'and' | 'or' };
+  hidden?: boolean;
 }

 const UserSettings: React.FC = ({ children }) => {
@@ -51,6 +56,15 @@ const UserSettings: React.FC = ({ children }) => {
      text: intl.formatMessage(messages.menuChangePass),
      route: '/settings/password',
      regex: /\/settings\/password/,
+      hidden:
+        (!settings.currentSettings.localLogin &&
+          !hasPermission(
+            Permission.MANAGE_SETTINGS,
+            currentUser?.permissions ?? 0
+          )) ||
+        (currentUser?.id !== 1 &&
+          currentUser?.id !== user?.id &&
+          hasPermission(Permission.ADMIN, user?.permissions ?? 0)),
    },
    {
      text: intl.formatMessage(messages.menuNotifications),
@@ -62,6 +76,7 @@ const UserSettings: React.FC = ({ children }) => {
      route: '/settings/permissions',
      regex: /\/settings\/permissions/,
      requiredPermission: Permission.MANAGE_USERS,
+      hidden: currentUser?.id !== 1 && currentUser?.id === user.id,
    },
  ];

@@ -76,20 +91,6 @@ const UserSettings: React.FC = ({ children }) => {
    regex: RegExp;
    isMobile?: boolean;
  }> = ({ children, route, regex, isMobile = false }) => {
-    if (
-      route === '/settings/password' &&
-      ((!settings.currentSettings.localLogin &&
-        !hasPermission(
-          Permission.MANAGE_SETTINGS,
-          currentUser?.permissions ?? 0
-        )) ||
-        (currentUser?.id !== 1 &&
-          currentUser?.id !== user?.id &&
-          hasPermission(Permission.ADMIN, user?.permissions ?? 0)))
-    ) {
-      return null;
-    }
-
    const finalRoute = router.asPath.includes('/profile')
      ? `/profile${route}`
      : `/users/${user.id}${route}`;
@@ -111,6 +112,20 @@ const UserSettings: React.FC = ({ children }) => {
    );
  };

+  if (currentUser?.id !== 1 && user.id === 1) {
+    return (
+      <>
+        <PageTitle title={intl.formatMessage(messages.settings)} />
+        <ProfileHeader user={user} isSettingsPage />
+        <div className="mt-6">
+          <Alert title={intl.formatMessage(messages.unauthorized)} type="error">
+            {intl.formatMessage(messages.unauthorizedDescription)}
+          </Alert>
+        </div>
+      </>
+    );
+  }
+
  const currentRoute = settingsRoutes.find(
    (route) => !!router.pathname.match(route.regex)
  )?.route;
@@ -136,14 +151,16 @@ const UserSettings: React.FC = ({ children }) => {
            aria-label="Selected tab"
          >
            {settingsRoutes
-              .filter((route) =>
-                route.requiredPermission
+              .filter(
+                (route) =>
+                  !route.hidden &&
+                  (route.requiredPermission
                    ? hasPermission(
                        route.requiredPermission,
                        currentUser?.permissions ?? 0,
                        route.permissionType
                      )
-                  : true
+                    : true)
              )
              .map((route, index) => (
                <SettingsLink
@@ -161,14 +178,16 @@ const UserSettings: React.FC = ({ children }) => {
          <div className="border-b border-gray-600">
            <nav className="flex -mb-px">
              {settingsRoutes
-                .filter((route) =>
-                  route.requiredPermission
+                .filter(
+                  (route) =>
+                    !route.hidden &&
+                    (route.requiredPermission
                      ? hasPermission(
                          route.requiredPermission,
                          currentUser?.permissions ?? 0,
                          route.permissionType
                        )
-                    : true
+                      : true)
                )
                .map((route, index) => (
                  <SettingsLink
--- a/src/components/UserProfile/index.tsx
+++ b/src/components/UserProfile/index.tsx
@@ -16,6 +16,7 @@ import { defineMessages, useIntl } from 'react-intl';

 const messages = defineMessages({
  recentrequests: 'Recent Requests',
+  norequests: 'No Requests',
 });

 type MediaTitle = MovieDetails | TvDetails;
@@ -95,7 +96,7 @@ const UserProfile: React.FC = () => {
            />
          ))}
          placeholder={<RequestCard.Placeholder />}
-          emptyMessage={'No Requests'}
+          emptyMessage={intl.formatMessage(messages.norequests)}
        />
      </div>
    </>
--- a/src/i18n/locale/en.json
+++ b/src/i18n/locale/en.json
@@ -773,11 +773,15 @@
  "components.UserProfile.UserSettings.UserPermissions.saving": "Saving…",
  "components.UserProfile.UserSettings.UserPermissions.toastSettingsFailure": "Something went wrong while saving settings.",
  "components.UserProfile.UserSettings.UserPermissions.toastSettingsSuccess": "Settings successfully saved!",
+  "components.UserProfile.UserSettings.UserPermissions.unauthorized": "Unauthorized",
+  "components.UserProfile.UserSettings.UserPermissions.unauthorizedDescription": "You cannot modify your own permissions.",
  "components.UserProfile.UserSettings.menuChangePass": "Password",
  "components.UserProfile.UserSettings.menuGeneralSettings": "General Settings",
  "components.UserProfile.UserSettings.menuNotifications": "Notifications",
  "components.UserProfile.UserSettings.menuPermissions": "Permissions",
  "components.UserProfile.UserSettings.settings": "User Settings",
+  "components.UserProfile.UserSettings.unauthorized": "Unauthorized",
+  "components.UserProfile.UserSettings.unauthorizedDescription": "You do not have permission to modify this user's settings.",
  "components.UserProfile.recentrequests": "Recent Requests",
  "i18n.advanced": "Advanced",
  "i18n.approve": "Approve",