Fixed AdminX user details modal behaviour (#18153)

refs https://github.com/TryGhost/Product/issues/3832

---

This pull request improves the user management and Stripe integration
features in the admin-x-settings app. It adds a new `ChangePasswordForm`
component for changing user passwords, refactors the error handling and
response handling logic in the `StripeConnectModal` and `apiRequests`
files, and fixes a bug in the `UserDetailModal` component. It also
creates new files for the custom error classes and the response handling
function.
This commit is contained in:
Jono M 2023-09-15 08:50:09 +01:00 committed by GitHub
parent b483432867
commit 2310e9d93f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 342 additions and 194 deletions

View File

@ -1,5 +1,5 @@
import APIKeys from '../advanced/integrations/APIKeys';
import Button from '../../../admin-x-ds/global/Button';
import ChangePasswordForm from './users/ChangePasswordForm';
import ConfirmationModal from '../../../admin-x-ds/global/modal/ConfirmationModal';
import Heading from '../../../admin-x-ds/global/Heading';
import Icon from '../../../admin-x-ds/global/Icon';
@ -9,7 +9,7 @@ import Menu, {MenuItem} from '../../../admin-x-ds/global/Menu';
import Modal from '../../../admin-x-ds/global/modal/Modal';
import NiceModal, {useModal} from '@ebay/nice-modal-react';
import Radio from '../../../admin-x-ds/global/form/Radio';
import React, {useCallback, useEffect, useRef, useState} from 'react';
import React, {useCallback, useEffect, useState} from 'react';
import SettingGroup from '../../../admin-x-ds/settings/SettingGroup';
import SettingGroupContent from '../../../admin-x-ds/settings/SettingGroupContent';
import TextField from '../../../admin-x-ds/global/form/TextField';
@ -23,7 +23,7 @@ import validator from 'validator';
import {DetailsInputs} from './DetailsInputs';
import {HostLimitError, useLimiter} from '../../../hooks/useLimiter';
import {RoutingModalProps} from '../../providers/RoutingProvider';
import {User, canAccessSettings, hasAdminAccess, isAdminUser, isOwnerUser, useDeleteUser, useEditUser, useMakeOwner, useUpdatePassword} from '../../../api/users';
import {User, canAccessSettings, hasAdminAccess, isAdminUser, isOwnerUser, useDeleteUser, useEditUser, useMakeOwner} from '../../../api/users';
import {genStaffToken, getStaffToken} from '../../../api/staffToken';
import {getImageUrl, useUploadImage} from '../../../api/images';
import {getSettingValues} from '../../../api/settings';
@ -254,139 +254,6 @@ const EmailNotifications: React.FC<UserDetailProps> = ({user, setUserData}) => {
);
};
function passwordValidation({password, confirmPassword}: {password: string; confirmPassword: string}) {
const errors: {
newPassword?: string;
confirmNewPassword?: string;
} = {};
if (password !== confirmPassword) {
errors.newPassword = 'Your new passwords do not match';
errors.confirmNewPassword = 'Your new passwords do not match';
}
if (password.length < 10) {
errors.newPassword = 'Password must be at least 10 characters';
}
//ToDo: add more validations
return errors;
}
const Password: React.FC<UserDetailProps> = ({user}) => {
const [editPassword, setEditPassword] = useState(false);
const [newPassword, setNewPassword] = useState('');
const [confirmNewPassword, setConfirmNewPassword] = useState('');
const [saveState, setSaveState] = useState<'saving'|'saved'|'error'|''>('');
const [errors, setErrors] = useState<{
newPassword?: string;
confirmNewPassword?: string;
}>({});
const newPasswordRef = useRef<HTMLInputElement>(null);
const confirmNewPasswordRef = useRef<HTMLInputElement>(null);
const {mutateAsync: updatePassword} = useUpdatePassword();
useEffect(() => {
if (saveState === 'saved') {
setTimeout(() => {
setSaveState('');
setEditPassword(false);
}, 2500);
}
}, [saveState]);
const showPasswordInputs = () => {
setEditPassword(true);
};
const view = (
<Button
color='grey'
label='Change password'
onClick={showPasswordInputs}
/>
);
let buttonLabel = 'Change password';
if (saveState === 'saving') {
buttonLabel = 'Updating...';
} else if (saveState === 'saved') {
buttonLabel = 'Updated';
} else if (saveState === 'error') {
buttonLabel = 'Retry';
}
const form = (
<>
<TextField
error={!!errors.newPassword}
hint={errors.newPassword}
inputRef={newPasswordRef}
title="New password"
type="password"
value={newPassword}
onChange={(e) => {
setNewPassword(e.target.value);
}}
/>
<TextField
error={!!errors.confirmNewPassword}
hint={errors.confirmNewPassword}
inputRef={confirmNewPasswordRef}
title="Verify password"
type="password"
value={confirmNewPassword}
onChange={(e) => {
setConfirmNewPassword(e.target.value);
}}
/>
<Button
color='red'
label={buttonLabel}
onClick={async () => {
setSaveState('saving');
const validationErrros = passwordValidation({password: newPassword, confirmPassword: confirmNewPassword});
setErrors(validationErrros);
if (Object.keys(validationErrros).length > 0) {
// show errors
setNewPassword('');
setConfirmNewPassword('');
if (newPasswordRef.current) {
newPasswordRef.current.value = '';
}
if (confirmNewPasswordRef.current) {
confirmNewPasswordRef.current.value = '';
}
setSaveState('');
return;
}
try {
await updatePassword({
newPassword,
confirmNewPassword,
oldPassword: '',
userId: user?.id
});
setSaveState('saved');
} catch (e) {
setSaveState('error');
// show errors
}
}}
/>
</>
);
return (
<SettingGroup
border={false}
customHeader={<CustomHeader>Password</CustomHeader>}
title='Password'
>
{editPassword ? form : view}
</SettingGroup>
);
};
const StaffToken: React.FC<UserDetailProps> = () => {
const {refetch: apiKey} = getStaffToken({
enabled: false
@ -615,11 +482,9 @@ const UserDetailModalContent: React.FC<{user: User}> = ({user}) => {
}
};
let suspendUserLabel = userData?.status === 'inactive' ? 'Un-suspend user' : 'Suspend user';
let menuItems: MenuItem[] = [];
if (isAdminUser(userData)) {
if (isOwnerUser(currentUser) && isAdminUser(userData) && userData.status !== 'inactive') {
menuItems.push({
id: 'make-owner',
label: 'Make owner',
@ -627,30 +492,32 @@ const UserDetailModalContent: React.FC<{user: User}> = ({user}) => {
});
}
menuItems = menuItems.concat([
{
if (userData.id !== currentUser.id) {
let suspendUserLabel = userData.status === 'inactive' ? 'Un-suspend user' : 'Suspend user';
menuItems.push({
id: 'delete-user',
label: 'Delete user',
onClick: () => {
confirmDelete(user, {owner: ownerUser});
}
},
{
}, {
id: 'suspend-user',
label: suspendUserLabel,
onClick: () => {
confirmSuspend(userData);
}
},
{
id: 'view-user-activity',
label: 'View user activity',
onClick: () => {
mainModal.remove();
updateRoute(`history/view/${userData.id}`);
}
});
}
menuItems.push({
id: 'view-user-activity',
label: 'View user activity',
onClick: () => {
mainModal.remove();
updateRoute(`history/view/${userData.id}`);
}
]);
});
let okLabel = saveState === 'saved' ? 'Saved' : 'Save & close';
@ -661,7 +528,7 @@ const UserDetailModalContent: React.FC<{user: User}> = ({user}) => {
}
const coverButtonContainerClassName = clsx(
canAccessSettings(currentUser) ? (
hasAdminAccess(currentUser) ? (
userData.cover_image ? 'relative ml-10 mr-[106px] flex translate-y-[-80px] gap-3 md:ml-0 md:justify-end' : 'relative -mb-8 ml-10 mr-[106px] flex translate-y-[358px] md:ml-0 md:translate-y-[268px] md:justify-end'
) : (
userData.cover_image ? 'relative ml-10 flex max-w-4xl translate-y-[-80px] gap-3 md:mx-auto md:justify-end' : 'relative -mb-8 ml-10 flex max-w-4xl translate-y-[358px] md:mx-auto md:translate-y-[268px] md:justify-end'
@ -771,7 +638,7 @@ const UserDetailModalContent: React.FC<{user: User}> = ({user}) => {
handleImageUpload('cover_image', file);
}}
>Upload cover image</ImageUpload>
{canAccessSettings(currentUser) && <div className="absolute bottom-12 right-12 z-10">
{hasAdminAccess(currentUser) && <div className="absolute bottom-12 right-12 z-10">
<Menu items={menuItems} position='right' trigger={<UserMenuTrigger />}></Menu>
</div>}
<div className={`${!canAccessSettings(currentUser) ? 'mx-10 pl-0 md:max-w-[50%] min-[920px]:ml-[calc((100vw-920px)/2)] min-[920px]:max-w-[460px]' : 'max-w-[50%] pl-12'} relative flex flex-col items-start gap-4 pb-60 pt-10 md:flex-row md:items-center md:pb-7 md:pt-60`}>
@ -819,7 +686,7 @@ const UserDetailModalContent: React.FC<{user: User}> = ({user}) => {
<StaffToken user={userData} />
</div>
<EmailNotifications setUserData={setUserData} user={userData} />
<Password user={userData} />
<ChangePasswordForm user={userData} />
</div>
</div>
</Modal>

View File

@ -0,0 +1,244 @@
import Button from '../../../../admin-x-ds/global/Button';
import Heading from '../../../../admin-x-ds/global/Heading';
import SettingGroup from '../../../../admin-x-ds/settings/SettingGroup';
import TextField from '../../../../admin-x-ds/global/form/TextField';
import {User, useUpdatePassword} from '../../../../api/users';
import {ValidationError} from '../../../../utils/errors';
import {showToast} from '../../../../admin-x-ds/global/Toast';
import {useEffect, useRef, useState} from 'react';
import {useGlobalData} from '../../../providers/GlobalDataProvider';
const BAD_PASSWORDS = [
'1234567890',
'qwertyuiop',
'qwertzuiop',
'asdfghjkl;',
'abcdefghij',
'0987654321',
'1q2w3e4r5t',
'12345asdfg'
];
const DISALLOWED_PASSWORDS = ['ghost', 'password', 'passw0rd'];
/**
* Counts repeated characters if a string. When 50% or more characters are the same,
* we return false and therefore invalidate the string.
*/
const validateCharacterOccurrance = (stringToTest: string) => {
let chars: Record<string, number> = {};
let allowedOccurancy;
let valid = true;
allowedOccurancy = stringToTest.length / 2;
// Loop through string and accumulate character counts
for (let i = 0; i < stringToTest.length; i += 1) {
if (!chars[stringToTest[i]]) {
chars[stringToTest[i]] = 1;
} else {
chars[stringToTest[i]] += 1;
}
}
// check if any of the accumulated chars exceed the allowed occurancy
// of 50% of the words' length.
for (let charCount in chars) {
if (chars[charCount] >= allowedOccurancy) {
valid = false;
return valid;
}
}
return valid;
};
const ChangePasswordForm: React.FC<{user: User}> = ({user}) => {
const {currentUser, config, siteData} = useGlobalData();
const [editPassword, setEditPassword] = useState(false);
const [oldPassword, setOldPassword] = useState('');
const [newPassword, setNewPassword] = useState('');
const [confirmNewPassword, setConfirmNewPassword] = useState('');
const [saveState, setSaveState] = useState<'saving'|'saved'|'error'|''>('');
const [errors, setErrors] = useState<{
oldPassword?: string;
newPassword?: string;
confirmNewPassword?: string;
}>({});
const newPasswordRef = useRef<HTMLInputElement>(null);
const confirmNewPasswordRef = useRef<HTMLInputElement>(null);
const {mutateAsync: updatePassword} = useUpdatePassword();
const isCurrentUser = user.id === currentUser.id;
const validate = ({password, confirmPassword}: {password: string; confirmPassword: string}) => {
if (isCurrentUser && !oldPassword) {
return {oldPassword: 'Your current password is required to set a new one'};
}
if (password !== confirmPassword) {
return {
newPassword: 'Your new passwords do not match',
confirmNewPassword: 'Your new passwords do not match'
};
}
let blogUrl = config.blogUrl || window.location.host;
let blogTitle = siteData.title;
let blogUrlWithSlash;
blogUrl = blogUrl.replace(/^http(s?):\/\//, '');
blogUrlWithSlash = blogUrl.match(/\/$/) ? blogUrl : `${blogUrl}/`;
blogTitle = blogTitle ? blogTitle.trim().toLowerCase() : blogTitle;
if (password.length < 10) {
return {newPassword: 'Password must be at least 10 characters long.'};
}
password = password.toString();
// disallow password from badPasswords list (e. g. '1234567890')
for (const badPassword of BAD_PASSWORDS) {
if (badPassword === password) {
return {newPassword: 'Sorry, you cannot use an insecure password.'};
}
};
// password must not match with users' email
if (password.toLowerCase() === user.email.toLowerCase()) {
return {newPassword: 'Sorry, you cannot use an insecure password.'};
}
// password must not contain the words 'ghost', 'password', or 'passw0rd'
for (const disallowedPassword of DISALLOWED_PASSWORDS) {
if (password.toLowerCase().indexOf(disallowedPassword) >= 0) {
return {newPassword: 'Sorry, you cannot use an insecure password.'};
}
};
// password must not match with blog title
if (password.toLowerCase() === blogTitle) {
return {newPassword: 'Sorry, you cannot use an insecure password.'};
}
// password must not match with blog URL (without protocol, with or without trailing slash)
if (password.toLowerCase() === blogUrl || password.toLowerCase() === blogUrlWithSlash) {
return {newPassword: 'Sorry, you cannot use an insecure password.'};
}
// disallow passwords where 50% or more of characters are the same
if (!validateCharacterOccurrance(password)) {
return {newPassword: 'Sorry, you cannot use an insecure password.'};
}
return {};
};
useEffect(() => {
if (saveState === 'saved') {
setTimeout(() => {
setSaveState('');
setEditPassword(false);
}, 2500);
}
}, [saveState]);
const showPasswordInputs = () => {
setEditPassword(true);
};
const view = (
<Button
color='grey'
label='Change password'
onClick={showPasswordInputs}
/>
);
let buttonLabel = 'Change password';
if (saveState === 'saving') {
buttonLabel = 'Updating...';
} else if (saveState === 'saved') {
buttonLabel = 'Updated';
}
const form = (
<>
{isCurrentUser && <TextField
error={!!errors.oldPassword}
hint={errors.oldPassword}
title="Old password"
type="password"
value={oldPassword}
onChange={(e) => {
setOldPassword(e.target.value);
}}
/>}
<TextField
error={!!errors.newPassword}
hint={errors.newPassword}
inputRef={newPasswordRef}
title="New password"
type="password"
value={newPassword}
onChange={(e) => {
setNewPassword(e.target.value);
}}
/>
<TextField
error={!!errors.confirmNewPassword}
hint={errors.confirmNewPassword}
inputRef={confirmNewPasswordRef}
title="Verify password"
type="password"
value={confirmNewPassword}
onChange={(e) => {
setConfirmNewPassword(e.target.value);
}}
/>
<Button
color='red'
label={buttonLabel}
onClick={async () => {
setSaveState('saving');
const validationErrors = validate({password: newPassword, confirmPassword: confirmNewPassword});
setErrors(validationErrors);
if (Object.keys(validationErrors).length > 0) {
setOldPassword('');
setNewPassword('');
setConfirmNewPassword('');
setSaveState('');
return;
}
try {
await updatePassword({
newPassword,
confirmNewPassword,
oldPassword,
userId: user?.id
});
setSaveState('saved');
} catch (e) {
setSaveState('');
showToast({
type: 'pageError',
message: e instanceof ValidationError ? e.message : `Couldn't update password. Please try again.`
});
}
}}
/>
</>
);
return (
<SettingGroup
border={false}
customHeader={<Heading level={4}>Password</Heading>}
title='Password'
>
{editPassword ? form : view}
</SettingGroup>
);
};
export default ChangePasswordForm;

View File

@ -15,7 +15,7 @@ import TextField from '../../../../admin-x-ds/global/form/TextField';
import Toggle from '../../../../admin-x-ds/global/form/Toggle';
import useRouting from '../../../../hooks/useRouting';
import useSettingGroup from '../../../../hooks/useSettingGroup';
import {ApiError} from '../../../../utils/apiRequests';
import {JSONError} from '../../../../utils/errors';
import {ReactComponent as StripeVerified} from '../../../../assets/images/stripe-verified.svg';
import {checkStripeEnabled, getSettingValue, getSettingValues, useDeleteStripeSettings, useEditSettings} from '../../../../api/settings';
import {getGhostPaths} from '../../../../utils/helpers';
@ -81,7 +81,7 @@ const Connect: React.FC = () => {
await editTier(tier);
break;
} catch (e) {
if (e instanceof ApiError && e.data?.errors?.[0].code === 'STRIPE_NOT_CONFIGURED') {
if (e instanceof JSONError && e.data?.errors?.[0].code === 'STRIPE_NOT_CONFIGURED') {
pollTimeout += RETRY_PRODUCT_SAVE_POLL_LENGTH;
// no-op: will try saving again as stripe is not ready
continue;
@ -108,7 +108,7 @@ const Connect: React.FC = () => {
{key: 'portal_plans', value: JSON.stringify(['free', 'monthly', 'yearly'])}
]);
} catch (e) {
if (e instanceof ApiError && e.data?.errors) {
if (e instanceof JSONError && e.data?.errors) {
setError('Invalid secure key');
return;
}
@ -228,7 +228,7 @@ const Direct: React.FC<{onClose: () => void}> = ({onClose}) => {
await handleSave();
onClose();
} catch (e) {
if (e instanceof ApiError) {
if (e instanceof JSONError) {
showToast({
type: 'pageError',
message: 'Failed to save settings. Please check you copied both keys correctly.'

View File

@ -1,3 +1,4 @@
import handleResponse from './handleResponse';
import {QueryClient, UseInfiniteQueryOptions, UseQueryOptions, useInfiniteQuery, useMutation, useQuery, useQueryClient} from '@tanstack/react-query';
import {getGhostPaths} from './helpers';
import {useMemo} from 'react';
@ -24,27 +25,6 @@ interface RequestOptions {
credentials?: 'include' | 'omit' | 'same-origin';
}
export class ApiError extends Error {
constructor(
public readonly response: Response,
public readonly data?: {
errors?: Array<{
code: string | null;
context: string | null;
details: string | null;
ghostErrorCode: string | null;
help: string | null;
id: string;
message: string;
property: string | null;
type: string;
}>
}
) {
super(data?.errors?.[0]?.message || response.statusText);
}
}
export const useFetchApi = () => {
const {ghostVersion} = useServices();
@ -69,16 +49,7 @@ export const useFetchApi = () => {
...options
});
if (response.status > 299) {
const data = response.headers.get('content-type')?.includes('application/json') ? await response.json() : undefined;
throw new ApiError(response, data);
} else if (response.status === 204) {
return;
} else if (response.headers.get('content-type')?.includes('text/csv')) {
return await response.text();
} else {
return await response.json();
}
return handleResponse(response);
};
};

View File

@ -0,0 +1,39 @@
interface ErrorResponse {
errors: Array<{
code: string
context: string | null
details: string | null
ghostErrorCode: string | null
help: string
id: string
message: string
property: string | null
type: string
}>
}
export class APIError extends Error {
constructor(
public readonly response: Response,
public readonly data?: unknown,
message?: string
) {
super(message || response.statusText);
}
}
export class JSONError extends APIError {
constructor(
response: Response,
public readonly data?: ErrorResponse,
message?: string
) {
super(response, data, message);
}
}
export class ValidationError extends JSONError {
constructor(response: Response, data: ErrorResponse) {
super(response, data, data.errors[0].message);
}
}

View File

@ -0,0 +1,27 @@
import {APIError, JSONError, ValidationError} from './errors';
const handleResponse = async (response: Response) => {
if (response.status === 422) {
const data = await response.json();
if (data.errors?.[0]?.type === 'ValidationError') {
throw new ValidationError(response, data);
} else {
throw new JSONError(response, data);
}
} else if (response.status > 299) {
if (response.headers.get('content-type')?.includes('json')) {
throw new JSONError(response, await response.json());
} else {
throw new APIError(response, await response.text());
}
} else if (response.status === 204) {
return;
} else if (response.headers.get('content-type')?.includes('text/csv')) {
return await response.text();
} else {
return await response.json();
}
};
export default handleResponse;

View File

@ -87,8 +87,8 @@ test.describe('User profile', async () => {
await modal.getByRole('button', {name: 'Change password'}).click();
await modal.getByLabel('New password').fill('newpassword');
await modal.getByLabel('Verify password').fill('newpassword');
await modal.getByLabel('New password').fill('newpasshere');
await modal.getByLabel('Verify password').fill('newpasshere');
await modal.getByRole('button', {name: 'Change password'}).click();
@ -96,8 +96,8 @@ test.describe('User profile', async () => {
expect(lastApiRequests.updatePassword?.body).toMatchObject({
password: [{
newPassword: 'newpassword',
ne2Password: 'newpassword',
newPassword: 'newpasshere',
ne2Password: 'newpasshere',
oldPassword: '',
user_id: responseFixtures.users.users.find(user => user.email === 'administrator@test.com')!.id
}]