From 001dcd328c8d3b1c417fd7c7ee2aa20183b08eef Mon Sep 17 00:00:00 2001 From: TheCatLady <52870424+TheCatLady@users.noreply.github.com> Date: Sun, 14 Mar 2021 00:46:12 -0500 Subject: [PATCH] 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 --- server/routes/user/usersettings.ts | 20 ++++- src/components/UserList/index.tsx | 7 +- .../UserProfile/ProfileHeader/index.tsx | 48 +++++------ .../UserSettings/UserPermissions/index.tsx | 18 +++++ .../UserProfile/UserSettings/index.tsx | 79 ++++++++++++------- src/components/UserProfile/index.tsx | 3 +- src/i18n/locale/en.json | 4 + 7 files changed, 122 insertions(+), 57 deletions(-) diff --git a/server/routes/user/usersettings.ts b/server/routes/user/usersettings.ts index e20ee375..5b62362d 100644 --- 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', diff --git a/src/components/UserList/index.tsx b/src/components/UserList/index.tsx index a57a9056..d82ab9ff 100644 --- a/src/components/UserList/index.tsx +++ b/src/components/UserList/index.tsx @@ -559,6 +559,7 @@ const UserList: React.FC = () => { ) : ( - - - + isSettingsPage && ( + + + + ) )} diff --git a/src/components/UserProfile/UserSettings/UserPermissions/index.tsx b/src/components/UserProfile/UserSettings/UserPermissions/index.tsx index 18e222b3..63db8de9 100644 --- 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 ; } + if (currentUser?.id !== 1 && currentUser?.id === user?.id) { + return ( + <> +
+

+ {intl.formatMessage(messages.permissions)} +

+
+ + {intl.formatMessage(messages.unauthorizedDescription)} + + + ); + } + return ( <>
diff --git a/src/components/UserProfile/UserSettings/index.tsx b/src/components/UserProfile/UserSettings/index.tsx index 5e6dd5b1..392029d2 100644 --- 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 ( + <> + + +
+ + {intl.formatMessage(messages.unauthorizedDescription)} + +
+ + ); + } + 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 - ? hasPermission( - route.requiredPermission, - currentUser?.permissions ?? 0, - route.permissionType - ) - : true + .filter( + (route) => + !route.hidden && + (route.requiredPermission + ? hasPermission( + route.requiredPermission, + currentUser?.permissions ?? 0, + route.permissionType + ) + : true) ) .map((route, index) => ( {
diff --git a/src/i18n/locale/en.json b/src/i18n/locale/en.json index d13ef347..8c8694a0 100644 --- 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",