Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(history): handle sanitization when fetching versions #20212

Merged
merged 19 commits into from Apr 30, 2024
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -59,16 +59,48 @@ const LinkEllipsis = styled(Link)`

const CustomRelationInput = (props: RelationsFieldProps) => {
const { formatMessage } = useIntl();
const field = useField<{ results: RelationResult[]; meta: { missingCount: number } }>(props.name);
const { results, meta } = field.value!;
const field = useField<
{ results: RelationResult[]; meta: { missingCount: number } } | RelationResult[]
>(props.name);

/**
* TODO:
markkaylor marked this conversation as resolved.
Show resolved Hide resolved
*
* Ideally the server would return the correct shape, however, for admin user relations
* it sanitizes everyything out when it finds an object for the relation value.
markkaylor marked this conversation as resolved.
Show resolved Hide resolved
*/
const formattedFieldValue = Array.isArray(field.value)
? {
results: field.value,
meta: { missingCount: 0 },
}
: field.value;

if (!formattedFieldValue || formattedFieldValue.results.length === 0) {
return (
<>
<FieldLabel>{props.label}</FieldLabel>
<Box marginTop={1}>
<StyledAlert variant="default">
{formatMessage({
id: 'content-manager.history.content.no-relations',
defaultMessage: 'No relations.',
})}
</StyledAlert>
</Box>
</>
);
}

const { results, meta } = formattedFieldValue;

return (
<Box>
<FieldLabel>{props.label}</FieldLabel>
{results.length > 0 && (
<Flex direction="column" gap={2} marginTop={1} alignItems="stretch">
{results.map((relationData) => {
// @ts-expect-error targetModel does exist on the attribute. But it's not typed.
// @ts-expect-error - targetModel does exist on the attribute. But it's not typed.
const href = `../${COLLECTION_TYPES}/${props.attribute.targetModel}/${relationData.documentId}`;
const label = getRelationLabel(relationData, props.mainField);

Expand Down
Expand Up @@ -55,6 +55,8 @@ describe('VersionHeader', () => {
componentsSchemas: {},
locale: null,
data: {
documentId: '1234',
id: 1,
title: 'Test Title',
},
meta: {
Expand Down Expand Up @@ -117,7 +119,7 @@ describe('VersionHeader', () => {

it('should display the correct subtitle without an entry title (mainField)', async () => {
render(
{ selectedVersion, mainField: 'id' },
{ selectedVersion, mainField: 'plop' },
'/collection-types/api::kitchensink.kitchensink/pcwmq3rlmp5w0be3cuplhnpr/history'
);

Expand All @@ -138,6 +140,8 @@ describe('VersionHeader', () => {
componentsSchemas: {},
locale: null,
data: {
documentId: '1234',
id: 1,
title: 'Test Title',
},
meta: {
Expand Down
@@ -1,5 +1,6 @@
import { errors } from '@strapi/utils';
import { async, errors } from '@strapi/utils';
import type { Core, UID } from '@strapi/types';
import { pick } from 'lodash/fp';
import { getService as getContentManagerService } from '../../utils';
import { getService } from '../utils';
import { HistoryVersions } from '../../../../shared/contracts';
Expand Down Expand Up @@ -60,21 +61,33 @@ const createHistoryVersionController = ({ strapi }: { strapi: Core.Strapi }) =>
return ctx.forbidden();
}

const params: HistoryVersions.GetHistoryVersions.Request['query'] =
const query: HistoryVersions.GetHistoryVersions.Request['query'] =
await permissionChecker.sanitizeQuery(ctx.query);

const { results, pagination } = await getService(strapi, 'history').findVersionsPage({
...params,
...getValidPagination({ page: params.page, pageSize: params.pageSize }),
query,
state: { userAbility: ctx.state.userAbility },
...getValidPagination({ page: query.page, pageSize: query.pageSize }),
});

const sanitizedResults = await async.map(
results,
async (version: HistoryVersions.HistoryVersionDataResponse & { locale: string }) => {
return {
...version,
data: await permissionChecker.sanitizeOutput({
...version.data,
locale: version.locale.code,
}),
markkaylor marked this conversation as resolved.
Show resolved Hide resolved
createdBy: version.createdBy
? pick(['id', 'firstname', 'lastname', 'username', 'email'], version.createdBy)
: undefined,
};
Comment on lines +81 to +83
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

arent the creator fields already inside the version data?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are omitting them as well 🤔 :

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to omit it if we are readding it again here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@remidej do you remember why we omit this in data and then return it here in the response?

}
);

return {
data: await Promise.all(
results.map(async (result) => ({
...result,
data: await permissionChecker.sanitizeOutput(result.data),
}))
),
data: sanitizedResults,
meta: { pagination },
};
},
Expand Down
Expand Up @@ -246,6 +246,8 @@ describe('history-version service', () => {
const historyVersionData = {
contentType: 'api::article.article' as UID.ContentType,
data: {
documentId: '1234',
id: 1,
title: 'My article',
},
locale: 'en',
Expand Down Expand Up @@ -275,6 +277,8 @@ describe('history-version service', () => {
const historyVersionData = {
contentType: 'api::article.article' as UID.ContentType,
data: {
documentId: '1234',
id: 1,
title: 'My article',
},
locale: 'en',
Expand Down