Skip to content

Commit

Permalink
Add post props validation (#8323) (#8383)
Browse files Browse the repository at this point in the history
* Validate props

* Add calls changes and fix attachments

* Address feedback

(cherry picked from commit 732b17a)

Co-authored-by: Daniel Espino García <[email protected]>
  • Loading branch information
mattermost-build and larkox authored Nov 28, 2024
1 parent 68aa016 commit 8468d81
Show file tree
Hide file tree
Showing 41 changed files with 801 additions and 138 deletions.
2 changes: 1 addition & 1 deletion app/actions/local/post.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ export async function markPostAsDeleted(serverUrl: string, post: Post, prepareRe
p.message = '';
p.messageSource = '';
p.metadata = null;
p.props = undefined;
p.props = null;
});

if (!prepareRecordsOnly) {
Expand Down
4 changes: 2 additions & 2 deletions app/actions/remote/post.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ export async function createPost(serverUrl: string, post: Partial<Post>, files:
const pendingPostId = post.pending_post_id || `${currentUserId}:${timestamp}`;

const existing = await getPostById(database, pendingPostId);
if (existing && !existing.props.failed) {
if (existing && !existing.props?.failed) {
return {data: false};
}

Expand Down Expand Up @@ -240,7 +240,7 @@ export const retryFailedPost = async (serverUrl: string, post: PostModel) => {
// timestamps will remain the same as the initial attempt for createAt
// but updateAt will be use for the optimistic post UI
post.prepareUpdate((p) => {
p.props = newPost.props;
p.props = newPost.props || null;
p.updateAt = timestamp;
});
await operator.batchRecords([post], 'retryFailedPost - first update');
Expand Down
2 changes: 1 addition & 1 deletion app/actions/remote/search.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ export const searchFiles = async (serverUrl: string, teamId: string, params: Fil
}, {});
files.forEach((f) => {
if (f.post_id) {
f.postProps = idToPost[f.post_id]?.props;
f.postProps = idToPost[f.post_id]?.props || {};
}
});
return {files, channels};
Expand Down
2 changes: 1 addition & 1 deletion app/components/files/files.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ type FilesProps = {
location: string;
isReplyPost: boolean;
postId: string;
postProps: Record<string, any>;
postProps: Record<string, unknown>;
publicLinkEnabled: boolean;
}

Expand Down
30 changes: 28 additions & 2 deletions app/components/markdown/channel_mention/channel_mention.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,38 @@ import {useServerUrl} from '@context/server';
import {t} from '@i18n';
import {alertErrorWithFallback} from '@utils/draft';
import {preventDoubleTap} from '@utils/tap';
import {secureGetFromRecord} from '@utils/types';
import {secureGetFromRecord, isRecordOf} from '@utils/types';

import type ChannelModel from '@typings/database/models/servers/channel';
import type TeamModel from '@typings/database/models/servers/team';

export type ChannelMentions = Record<string, {id?: string; display_name: string; name?: string; team_name: string}>;
export type ChannelMentions = Record<string, {id?: string; display_name: string; name?: string; team_name?: string}>;

export function isChannelMentions(v: unknown): v is ChannelMentions {
return isRecordOf(v, (e) => {
if (typeof e !== 'object' || !e) {
return false;
}

if (!('display_name' in e) || typeof e.display_name !== 'string') {
return false;
}

if ('team_name' in e && typeof e.team_name !== 'string') {
return false;
}

if ('id' in e && typeof e.id !== 'string') {
return false;
}

if ('name' in e && typeof e.name !== 'string') {
return false;
}

return true;
});
}

type ChannelMentionProps = {
channelMentions?: ChannelMentions;
Expand Down
2 changes: 0 additions & 2 deletions app/components/markdown/channel_mention/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ import ChannelMention from './channel_mention';

import type {WithDatabaseArgs} from '@typings/database/database';

export type ChannelMentions = Record<string, {id?: string; display_name: string; name?: string; team_name: string}>;

const enhance = withObservables([], ({database}: WithDatabaseArgs) => {
const currentTeamId = observeCurrentTeamId(database);
const channels = currentTeamId.pipe(
Expand Down
3 changes: 2 additions & 1 deletion app/components/markdown/markdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {typography} from '@utils/typography';
import {getScheme} from '@utils/url';

import AtMention from './at_mention';
import ChannelMention, {type ChannelMentions} from './channel_mention';
import ChannelMention from './channel_mention';
import Hashtag from './hashtag';
import MarkdownBlockQuote from './markdown_block_quote';
import MarkdownCodeBlock from './markdown_code_block';
Expand All @@ -33,6 +33,7 @@ import MarkdownTableImage from './markdown_table_image';
import MarkdownTableRow, {type MarkdownTableRowProps} from './markdown_table_row';
import {addListItemIndices, combineTextNodes, highlightMentions, highlightWithoutNotification, highlightSearchPatterns, parseTaskLists, pullOutImages} from './transform';

import type {ChannelMentions} from './channel_mention/channel_mention';
import type {
MarkdownAtMentionRenderer, MarkdownBaseRenderer, MarkdownBlockStyles, MarkdownChannelMentionRenderer,
MarkdownEmojiRenderer, MarkdownImageRenderer, MarkdownLatexRenderer, MarkdownTextStyles, SearchPattern, UserMentionKey, HighlightWithoutNotificationKey,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.

import React, {useCallback, useEffect} from 'react';
import React, {useCallback, useEffect, useMemo} from 'react';
import {useIntl} from 'react-intl';
import {Keyboard, type StyleProp, TouchableHighlight, View, type ViewStyle} from 'react-native';

Expand All @@ -15,6 +15,7 @@ import {useIsTablet} from '@hooks/device';
import {bottomSheetModalOptions, showModal, showModalOverCurrentContext} from '@screens/navigation';
import {emptyFunction} from '@utils/general';
import {getMarkdownTextStyles} from '@utils/markdown';
import {isUserActivityProp} from '@utils/post_list';
import {changeOpacity, makeStyleSheetFromTheme} from '@utils/theme';
import {secureGetFromRecord} from '@utils/types';

Expand Down Expand Up @@ -71,6 +72,13 @@ const CombinedUserActivity = ({
const content = [];
const removedUserIds: string[] = [];

const userActivity = useMemo(() => {
if (isUserActivityProp(post?.props?.user_activity)) {
return post?.props?.user_activity;
}
return undefined;
}, [post?.props?.user_activity]);

const getUsernames = (userIds: string[]) => {
const someone = intl.formatMessage({id: 'channel_loader.someone', defaultMessage: 'Someone'});
const you = intl.formatMessage({id: 'combined_system_message.you', defaultMessage: 'You'});
Expand Down Expand Up @@ -170,26 +178,27 @@ const CombinedUserActivity = ({
};

useEffect(() => {
if (!post) {
if (!userActivity) {
return;
}

const {allUserIds, allUsernames} = post.props.user_activity;
const allUserIds = userActivity.allUserIds;
const allUsernames = userActivity.allUsernames;
if (allUserIds.length) {
fetchMissingProfilesByIds(serverUrl, allUserIds);
}

if (allUsernames.length) {
fetchMissingProfilesByUsernames(serverUrl, allUsernames);
}
}, [post?.props.user_activity.allUserIds, post?.props.user_activity.allUsernames]);
}, [userActivity?.allUserIds, userActivity?.allUsernames]);

if (!post) {
return null;
}

const itemTestID = `${testID}.${post.id}`;
const {messageData} = post.props.user_activity;
const messageData = userActivity?.messageData || [];
for (const message of messageData) {
const {postType, actorId} = message;
const userIds = new Set<string>(message.userIds);
Expand Down
7 changes: 4 additions & 3 deletions app/components/post_list/combined_user_activity/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {queryPostsById} from '@queries/servers/post';
import {observePermissionForPost} from '@queries/servers/role';
import {observeCurrentUserId} from '@queries/servers/system';
import {observeUser, queryUsersByIdsOrUsernames} from '@queries/servers/user';
import {generateCombinedPost, getPostIdsForCombinedUserActivityPost} from '@utils/post_list';
import {generateCombinedPost, getPostIdsForCombinedUserActivityPost, isUserActivityProp} from '@utils/post_list';

import CombinedUserActivity from './combined_user_activity';

Expand All @@ -35,10 +35,11 @@ const withCombinedPosts = withObservables(['postId'], ({database, postId}: WithD
const usernamesById = post.pipe(
switchMap(
(p) => {
if (!p) {
const userActivity = isUserActivityProp(p?.props?.user_activity) ? p.props.user_activity : undefined;
if (!userActivity) {
return of$<Record<string, string>>({});
}
return queryUsersByIdsOrUsernames(database, p.props.user_activity.allUserIds, p.props.user_activity.allUsernames).observeWithColumns(['username']).
return queryUsersByIdsOrUsernames(database, userActivity.allUserIds, userActivity.allUsernames).observeWithColumns(['username']).
pipe(
// eslint-disable-next-line max-nested-callbacks
switchMap((users) => {
Expand Down
10 changes: 7 additions & 3 deletions app/components/post_list/post/avatar/avatar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {useServerUrl} from '@context/server';
import {useTheme} from '@context/theme';
import {openAsBottomSheet} from '@screens/navigation';
import {preventDoubleTap} from '@utils/tap';
import {ensureString} from '@utils/types';

import type PostModel from '@typings/database/models/servers/post';
import type UserModel from '@typings/database/models/servers/user';
Expand All @@ -39,12 +40,15 @@ const Avatar = ({author, enablePostIconOverride, isAutoReponse, location, post}:

const fromWebHook = post.props?.from_webhook === 'true';
const iconOverride = enablePostIconOverride && post.props?.use_user_icon !== 'true';
const propsIconUrl = ensureString(post.props?.override_icon_url);
const propsUsername = ensureString(post.props?.override_username);

if (fromWebHook && iconOverride) {
const isEmoji = Boolean(post.props?.override_icon_emoji);
const frameSize = ViewConstant.PROFILE_PICTURE_SIZE;
const pictureSize = isEmoji ? ViewConstant.PROFILE_PICTURE_EMOJI_SIZE : ViewConstant.PROFILE_PICTURE_SIZE;
const borderRadius = isEmoji ? 0 : ViewConstant.PROFILE_PICTURE_SIZE / 2;
const overrideIconUrl = buildAbsoluteUrl(serverUrl, post.props?.override_icon_url);
const overrideIconUrl = buildAbsoluteUrl(serverUrl, propsIconUrl);

let iconComponent: ReactNode;
if (overrideIconUrl) {
Expand Down Expand Up @@ -95,8 +99,8 @@ const Avatar = ({author, enablePostIconOverride, isAutoReponse, location, post}:
userId: author.id,
channelId: post.channelId,
location,
userIconOverride: post.props?.override_username,
usernameOverride: post.props?.override_icon_url,
userIconOverride: propsIconUrl,
usernameOverride: propsUsername,
};

Keyboard.dismiss();
Expand Down
53 changes: 42 additions & 11 deletions app/components/post_list/post/body/add_members/add_members.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {useServerUrl} from '@context/server';
import {t} from '@i18n';
import {getMarkdownTextStyles} from '@utils/markdown';
import {changeOpacity, makeStyleSheetFromTheme} from '@utils/theme';
import {isStringArray} from '@utils/types';

import type PostModel from '@typings/database/models/servers/post';
import type UserModel from '@typings/database/models/servers/user';
Expand All @@ -26,6 +27,39 @@ type AddMembersProps = {
theme: Theme;
}

export type AddMemberPostProps = {
post_id: string;
not_in_channel_user_ids?: string[];
not_in_groups_usernames?: string[];
not_in_channel_usernames?: string[];
user_ids?: string[];
usernames?: string[];
}

export function isAddMemberProps(v: unknown): v is AddMemberPostProps {
if (typeof v !== 'object' || !v) {
return false;
}

if (!('post_id' in v) || typeof v.post_id !== 'string') {
return false;
}

if (('not_in_channel_user_ids' in v) && !isStringArray(v.not_in_channel_user_ids)) {
return false;
}

if (('not_in_groups_usernames' in v) && !isStringArray(v.not_in_groups_usernames)) {
return false;
}

if (('not_in_channel_usernames' in v) && !isStringArray(v.not_in_channel_usernames)) {
return false;
}

return true;
}

const getStyleSheet = makeStyleSheetFromTheme((theme: Theme) => {
return {
message: {
Expand All @@ -41,20 +75,17 @@ const AddMembers = ({channelType, currentUser, location, post, theme}: AddMember
const styles = getStyleSheet(theme);
const textStyles = getMarkdownTextStyles(theme);
const serverUrl = useServerUrl();
const postId: string = post.props.add_channel_member?.post_id;
const noGroupsUsernames = post.props.add_channel_member?.not_in_groups_usernames;
let userIds: string[] = post.props.add_channel_member?.not_in_channel_user_ids;
let usernames: string[] = post.props.add_channel_member?.not_in_channel_usernames;

if (!postId || !channelType) {
if (!isAddMemberProps(post.props?.add_channel_member)) {
return null;
}

if (!userIds) {
userIds = post.props.add_channel_member?.user_ids;
}
if (!usernames) {
usernames = post.props.add_channel_member?.usernames;
const postId = post.props.add_channel_member.post_id;
const noGroupsUsernames = post.props.add_channel_member.not_in_groups_usernames || [];
const userIds = post.props.add_channel_member.not_in_channel_user_ids || post.props.add_channel_member.user_ids || [];
const usernames = post.props.add_channel_member.not_in_channel_usernames || post.props.add_channel_member?.usernames || [];

if (!postId || !channelType) {
return null;
}

const handleAddChannelMember = () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {observeChannel} from '@queries/servers/channel';
import {observeCurrentTeamId} from '@queries/servers/system';
import {showAppForm} from '@screens/navigation';
import {createCallContext} from '@utils/apps';
import {getStatusColors} from '@utils/message_attachment_colors';
import {getStatusColors} from '@utils/message_attachment';
import {preventDoubleTap} from '@utils/tap';
import {makeStyleSheetFromTheme, changeOpacity} from '@utils/theme';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
import React from 'react';
import {View} from 'react-native';

import {isAppBinding, validateBindings} from '@utils/apps';
import {isArrayOf} from '@utils/types';

import EmbeddedBinding from './embedded_binding';

import type PostModel from '@typings/database/models/servers/post';
Expand All @@ -16,7 +19,7 @@ type Props = {

const EmbeddedBindings = ({location, post, theme}: Props) => {
const content: React.ReactNode[] = [];
const embeds: AppBinding[] = post.props.app_bindings;
const embeds: AppBinding[] = isArrayOf<AppBinding>(post.props?.app_bindings, isAppBinding) ? validateBindings(post.props.app_bindings) : [];

embeds.forEach((embed, i) => {
content.push(
Expand Down
13 changes: 9 additions & 4 deletions app/components/post_list/post/body/content/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

import React from 'react';

import {isMessageAttachmentArray} from '@utils/message_attachment';
import {isYoutubeLink} from '@utils/url';

import EmbeddedBindings from './embedded_bindings';
Expand Down Expand Up @@ -31,14 +32,18 @@ const contentType: Record<string, string> = {

const Content = ({isReplyPost, layoutWidth, location, post, theme}: ContentProps) => {
let type: string | undefined = post.metadata?.embeds?.[0].type;
if (!type && post.props?.app_bindings?.length) {

const nAppBindings = Array.isArray(post.props?.app_bindings) ? post.props.app_bindings.length : 0;
if (!type && nAppBindings) {
type = contentType.app_bindings;
}

if (!type) {
return null;
}

const attachments = isMessageAttachmentArray(post.props?.attachments) ? post.props.attachments : [];

switch (contentType[type]) {
case contentType.image:
return (
Expand Down Expand Up @@ -74,10 +79,10 @@ const Content = ({isReplyPost, layoutWidth, location, post, theme}: ContentProps
/>
);
case contentType.message_attachment:
if (post.props.attachments?.length) {
if (attachments.length) {
return (
<MessageAttachments
attachments={post.props.attachments}
attachments={attachments}
channelId={post.channelId}
layoutWidth={layoutWidth}
location={location}
Expand All @@ -89,7 +94,7 @@ const Content = ({isReplyPost, layoutWidth, location, post, theme}: ContentProps
}
break;
case contentType.app_bindings:
if (post.props.app_bindings?.length) {
if (nAppBindings) {
return (
<EmbeddedBindings
location={location}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import React, {useCallback, useRef} from 'react';

import {postActionWithCookie} from '@actions/remote/integrations';
import {useServerUrl} from '@context/server';
import {getStatusColors} from '@utils/message_attachment_colors';
import {getStatusColors} from '@utils/message_attachment';
import {preventDoubleTap} from '@utils/tap';
import {makeStyleSheetFromTheme, changeOpacity} from '@utils/theme';
import {secureGetFromRecord} from '@utils/types';
Expand Down
Loading

0 comments on commit 8468d81

Please sign in to comment.