fix: add correct permission checks to modifying user password/permissions

pull/1050/head
sct 4 years ago
parent 2771376ecc
commit ddfc5e6aa8

@ -167,7 +167,10 @@ router.get<{ id: string }, UserRequestsResponse>(
} }
); );
const canMakePermissionsChange = (permissions: number, user?: User) => export const canMakePermissionsChange = (
permissions: number,
user?: User
): boolean =>
// Only let the owner grant admin privileges // Only let the owner grant admin privileges
!(hasPermission(Permission.ADMIN, permissions) && user?.id !== 1) || !(hasPermission(Permission.ADMIN, permissions) && user?.id !== 1) ||
// Only let users with the manage settings permission, grant the same permission // Only let users with the manage settings permission, grant the same permission

@ -1,5 +1,6 @@
import { Router } from 'express'; import { Router } from 'express';
import { getRepository } from 'typeorm'; import { getRepository } from 'typeorm';
import { canMakePermissionsChange } from '.';
import { User } from '../../entity/User'; import { User } from '../../entity/User';
import { UserSettings } from '../../entity/UserSettings'; import { UserSettings } from '../../entity/UserSettings';
import { import {
@ -21,6 +22,7 @@ const isOwnProfileOrAdmin = (): Middleware => {
message: "You do not have permission to view this user's settings.", message: "You do not have permission to view this user's settings.",
}); });
} }
next(); next();
}; };
return authMiddleware; return authMiddleware;
@ -137,7 +139,19 @@ userSettingsRoutes.post<
if (req.body.newPassword.length < 8) { if (req.body.newPassword.length < 8) {
return next({ return next({
status: 400, status: 400,
message: 'Password must be at least 8 characters', message: 'Password must be at least 8 characters.',
});
}
if (
(user.id === 1 && req.user?.id !== 1) ||
(user.hasPermission(Permission.ADMIN) &&
user.id !== req.user?.id &&
req.user?.id !== 1)
) {
return next({
status: 403,
message: "You do not have permission to modify this user's password.",
}); });
} }
@ -283,13 +297,20 @@ userSettingsRoutes.post<
return next({ status: 404, message: 'User not found.' }); return next({ status: 404, message: 'User not found.' });
} }
if (user.id === 1) { // Only let the owner user modify themselves
if (user.id === 1 && req.user?.id !== 1) {
return next({ return next({
status: 500, status: 403,
message: 'Permissions for user with ID 1 cannot be modified', message: 'You do not have permission to modify this user',
}); });
} }
if (!canMakePermissionsChange(req.body.permissions, req.user)) {
return next({
status: 403,
message: 'You do not have permission to grant this level of access',
});
}
user.permissions = req.body.permissions; user.permissions = req.body.permissions;
await userRepository.save(user); await userRepository.save(user);

@ -5,7 +5,7 @@ import React from 'react';
import { defineMessages, useIntl } from 'react-intl'; import { defineMessages, useIntl } from 'react-intl';
import { useToasts } from 'react-toast-notifications'; import { useToasts } from 'react-toast-notifications';
import useSWR from 'swr'; import useSWR from 'swr';
import { useUser } from '../../../../hooks/useUser'; import { Permission, useUser } from '../../../../hooks/useUser';
import Error from '../../../../pages/_error'; import Error from '../../../../pages/_error';
import Alert from '../../../Common/Alert'; import Alert from '../../../Common/Alert';
import Button from '../../../Common/Button'; import Button from '../../../Common/Button';
@ -33,6 +33,9 @@ const messages = defineMessages({
nopasswordsetDescription: nopasswordsetDescription:
'This user account currently does not have a password specifically for {applicationTitle}.\ 'This user account currently does not have a password specifically for {applicationTitle}.\
Configure a password below to enable this account to sign in as a "local user."', Configure a password below to enable this account to sign in as a "local user."',
nopermission: 'No Permission',
nopermissionDescription:
"You do not have permission to modify this user's password.",
}); });
const UserPasswordChange: React.FC = () => { const UserPasswordChange: React.FC = () => {
@ -41,14 +44,14 @@ const UserPasswordChange: React.FC = () => {
const { addToast } = useToasts(); const { addToast } = useToasts();
const router = useRouter(); const router = useRouter();
const { user: currentUser } = useUser(); const { user: currentUser } = useUser();
const { user } = useUser({ id: Number(router.query.userId) }); const { user, hasPermission } = useUser({ id: Number(router.query.userId) });
const { data, error, revalidate } = useSWR<{ hasPassword: boolean }>( const { data, error, revalidate } = useSWR<{ hasPassword: boolean }>(
user ? `/api/v1/user/${user?.id}/settings/password` : null user ? `/api/v1/user/${user?.id}/settings/password` : null
); );
const PasswordChangeSchema = Yup.object().shape({ const PasswordChangeSchema = Yup.object().shape({
currentPassword: Yup.lazy(() => currentPassword: Yup.lazy(() =>
data?.hasPassword data?.hasPassword && currentUser?.id === user?.id
? Yup.string().required( ? Yup.string().required(
intl.formatMessage(messages.validationCurrentPassword) intl.formatMessage(messages.validationCurrentPassword)
) )
@ -73,6 +76,23 @@ const UserPasswordChange: React.FC = () => {
return <Error statusCode={500} />; return <Error statusCode={500} />;
} }
if (
currentUser?.id !== user?.id &&
hasPermission(Permission.ADMIN) &&
currentUser?.id !== 1
) {
return (
<>
<div className="mb-6">
<h3 className="heading">{intl.formatMessage(messages.password)}</h3>
</div>
<Alert title={intl.formatMessage(messages.nopermission)} type="error">
{intl.formatMessage(messages.nopermissionDescription)}
</Alert>
</>
);
}
return ( return (
<> <>
<div className="mb-6"> <div className="mb-6">

@ -708,6 +708,8 @@
"components.UserProfile.UserSettings.UserPasswordChange.newpassword": "New Password", "components.UserProfile.UserSettings.UserPasswordChange.newpassword": "New Password",
"components.UserProfile.UserSettings.UserPasswordChange.nopasswordset": "No Password Set", "components.UserProfile.UserSettings.UserPasswordChange.nopasswordset": "No Password Set",
"components.UserProfile.UserSettings.UserPasswordChange.nopasswordsetDescription": "This user account currently does not have a password specifically for {applicationTitle}. Configure a password below to enable this account to sign in as a \"local user.\"", "components.UserProfile.UserSettings.UserPasswordChange.nopasswordsetDescription": "This user account currently does not have a password specifically for {applicationTitle}. Configure a password below to enable this account to sign in as a \"local user.\"",
"components.UserProfile.UserSettings.UserPasswordChange.nopermission": "No Permission",
"components.UserProfile.UserSettings.UserPasswordChange.nopermissionDescription": "You do not have permission to modify this user's password.",
"components.UserProfile.UserSettings.UserPasswordChange.password": "Password", "components.UserProfile.UserSettings.UserPasswordChange.password": "Password",
"components.UserProfile.UserSettings.UserPasswordChange.save": "Save Changes", "components.UserProfile.UserSettings.UserPasswordChange.save": "Save Changes",
"components.UserProfile.UserSettings.UserPasswordChange.saving": "Saving…", "components.UserProfile.UserSettings.UserPasswordChange.saving": "Saving…",

Loading…
Cancel
Save