From 8bb88b8550d865cd3422702c8b5ac9baa88f2017 Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Sat, 9 Jul 2022 17:04:40 +0200 Subject: [PATCH] chore: Audit of all model column validations (#3757) * chore: Updating all model validations before the white-hatters get to it ;) * test * Remove isUrl validation, thinking about it need to account for minio and other weird urls here --- server/models/ApiKey.ts | 6 ++++++ server/models/Attachment.ts | 15 +++++++++++++++ server/models/AuthenticationProvider.ts | 9 +++++++++ server/models/Collection.ts | 14 ++++++++++---- server/models/Document.ts | 18 +++++++++++++++++- server/models/Event.ts | 5 +++++ server/models/Group.ts | 2 +- server/models/Integration.ts | 2 +- server/models/Revision.ts | 13 +++++++++++++ server/models/Team.ts | 14 ++++++++++++-- server/models/TeamDomain.ts | 2 +- server/models/User.ts | 17 +++++++++-------- server/test/factories.ts | 2 +- 13 files changed, 100 insertions(+), 19 deletions(-) diff --git a/server/models/ApiKey.ts b/server/models/ApiKey.ts index a2132770f..82708a3da 100644 --- a/server/models/ApiKey.ts +++ b/server/models/ApiKey.ts @@ -10,10 +10,16 @@ import { import User from "./User"; import ParanoidModel from "./base/ParanoidModel"; import Fix from "./decorators/Fix"; +import Length from "./validators/Length"; @Table({ tableName: "apiKeys", modelName: "apiKey" }) @Fix class ApiKey extends ParanoidModel { + @Length({ + min: 3, + max: 255, + msg: "Name must be between 3 and 255 characters", + }) @Column name: string; diff --git a/server/models/Attachment.ts b/server/models/Attachment.ts index f9cc96d32..65774881b 100644 --- a/server/models/Attachment.ts +++ b/server/models/Attachment.ts @@ -8,6 +8,7 @@ import { IsIn, Table, DataType, + IsNumeric, } from "sequelize-typescript"; import { publicS3Endpoint, deleteFromS3, getFileByKey } from "@server/utils/s3"; import Document from "./Document"; @@ -15,19 +16,33 @@ import Team from "./Team"; import User from "./User"; import IdModel from "./base/IdModel"; import Fix from "./decorators/Fix"; +import Length from "./validators/Length"; @Table({ tableName: "attachments", modelName: "attachment" }) @Fix class Attachment extends IdModel { + @Length({ + max: 4096, + msg: "key must be 4096 characters or less", + }) @Column key: string; + @Length({ + max: 4096, + msg: "url must be 4096 characters or less", + }) @Column url: string; + @Length({ + max: 255, + msg: "contentType must be 255 characters or less", + }) @Column contentType: string; + @IsNumeric @Column(DataType.BIGINT) size: number; diff --git a/server/models/AuthenticationProvider.ts b/server/models/AuthenticationProvider.ts index 60a7ecaf0..0d4c41740 100644 --- a/server/models/AuthenticationProvider.ts +++ b/server/models/AuthenticationProvider.ts @@ -20,6 +20,7 @@ import { ValidationError } from "../errors"; import Team from "./Team"; import UserAuthentication from "./UserAuthentication"; import Fix from "./decorators/Fix"; +import Length from "./validators/Length"; @Table({ tableName: "authentication_providers", @@ -34,6 +35,10 @@ class AuthenticationProvider extends Model { @Column(DataType.UUID) id: string; + @Length({ + max: 255, + msg: "name must be 255 characters or less", + }) @Column name: string; @@ -41,6 +46,10 @@ class AuthenticationProvider extends Model { @Column enabled: boolean; + @Length({ + max: 255, + msg: "providerId must be 255 characters or less", + }) @Column providerId: string; diff --git a/server/models/Collection.ts b/server/models/Collection.ts index a8339ef30..f72ab2e31 100644 --- a/server/models/Collection.ts +++ b/server/models/Collection.ts @@ -18,6 +18,7 @@ import { ForeignKey, Scopes, DataType, + Length as SimpleLength, } from "sequelize-typescript"; import isUUID from "validator/lib/isUUID"; import { MAX_TITLE_LENGTH } from "@shared/constants"; @@ -131,6 +132,11 @@ type Sort = CollectionSort; @Table({ tableName: "collections", modelName: "collection" }) @Fix class Collection extends ParanoidModel { + @SimpleLength({ + min: 10, + max: 10, + msg: `urlId must be 10 characters`, + }) @Unique @Column urlId: string; @@ -138,21 +144,21 @@ class Collection extends ParanoidModel { @NotContainsUrl @Length({ max: MAX_TITLE_LENGTH, - msg: `Collection name must be less than ${MAX_TITLE_LENGTH} characters`, + msg: `name must be ${MAX_TITLE_LENGTH} characters or less`, }) @Column name: string; @Length({ max: 1000, - msg: `Collection description must be less than 1000 characters`, + msg: `description must be 1000 characters or less`, }) @Column description: string; @Length({ max: 50, - msg: `Collection icon must be less than 50 characters`, + msg: `icon must be 50 characters or less`, }) @Column icon: string | null; @@ -163,7 +169,7 @@ class Collection extends ParanoidModel { @Length({ max: 50, - msg: `Collection index must be less than 50 characters`, + msg: `index must 50 characters or less`, }) @Column index: string | null; diff --git a/server/models/Document.ts b/server/models/Document.ts index fddfea5df..42846cae4 100644 --- a/server/models/Document.ts +++ b/server/models/Document.ts @@ -28,6 +28,9 @@ import { AfterCreate, Scopes, DataType, + Length as SimpleLength, + IsNumeric, + IsDate, } from "sequelize-typescript"; import MarkdownSerializer from "slate-md-serializer"; import isUUID from "validator/lib/isUUID"; @@ -183,13 +186,18 @@ export const DOCUMENT_VERSION = 2; @Table({ tableName: "documents", modelName: "document" }) @Fix class Document extends ParanoidModel { + @SimpleLength({ + min: 10, + max: 10, + msg: `urlId must be 10 characters`, + }) @PrimaryKey @Column urlId: string; @Length({ max: MAX_TITLE_LENGTH, - msg: `Document title must be less than ${MAX_TITLE_LENGTH} characters`, + msg: `Document title must be ${MAX_TITLE_LENGTH} characters or less`, }) @Column title: string; @@ -197,6 +205,7 @@ class Document extends ParanoidModel { @Column(DataType.ARRAY(DataType.STRING)) previousTitles: string[] = []; + @IsNumeric @Column(DataType.SMALLINT) version: number; @@ -206,6 +215,10 @@ class Document extends ParanoidModel { @Column fullWidth: boolean; + @SimpleLength({ + max: 255, + msg: `editorVersion must be 255 characters or less`, + }) @Column editorVersion: string; @@ -222,13 +235,16 @@ class Document extends ParanoidModel { @Column isWelcome: boolean; + @IsNumeric @Default(0) @Column(DataType.INTEGER) revisionCount: number; + @IsDate @Column archivedAt: Date | null; + @IsDate @Column publishedAt: Date | null; diff --git a/server/models/Event.ts b/server/models/Event.ts index 717efffb2..0e1c4727d 100644 --- a/server/models/Event.ts +++ b/server/models/Event.ts @@ -9,6 +9,7 @@ import { IsUUID, Table, DataType, + Length, } from "sequelize-typescript"; import { globalEventQueue } from "../queues"; import { Event as TEvent } from "../types"; @@ -26,6 +27,10 @@ class Event extends IdModel { @Column(DataType.UUID) modelId: string; + @Length({ + max: 255, + msg: "name must be 255 characters or less", + }) @Column name: string; diff --git a/server/models/Group.ts b/server/models/Group.ts index 511bf6c31..362b52718 100644 --- a/server/models/Group.ts +++ b/server/models/Group.ts @@ -52,7 +52,7 @@ import NotContainsUrl from "./validators/NotContainsUrl"; }) @Fix class Group extends ParanoidModel { - @Length({ min: 0, max: 255, msg: "Must be less than 255 characters" }) + @Length({ min: 0, max: 255, msg: "name must be be 255 characters or less" }) @NotContainsUrl @Column name: string; diff --git a/server/models/Integration.ts b/server/models/Integration.ts index a63ea391a..cd3ff95af 100644 --- a/server/models/Integration.ts +++ b/server/models/Integration.ts @@ -34,7 +34,7 @@ class Integration extends IdModel { service: string; @Column(DataType.JSONB) - settings: any; + settings: Record; @Column(DataType.ARRAY(DataType.STRING)) events: string[]; diff --git a/server/models/Revision.ts b/server/models/Revision.ts index 475b016e2..572a67d8c 100644 --- a/server/models/Revision.ts +++ b/server/models/Revision.ts @@ -6,12 +6,16 @@ import { DefaultScope, ForeignKey, Table, + IsNumeric, + Length as SimpleLength, } from "sequelize-typescript"; import MarkdownSerializer from "slate-md-serializer"; +import { MAX_TITLE_LENGTH } from "@shared/constants"; import Document from "./Document"; import User from "./User"; import IdModel from "./base/IdModel"; import Fix from "./decorators/Fix"; +import Length from "./validators/Length"; const serializer = new MarkdownSerializer(); @@ -27,12 +31,21 @@ const serializer = new MarkdownSerializer(); @Table({ tableName: "revisions", modelName: "revision" }) @Fix class Revision extends IdModel { + @IsNumeric @Column(DataType.SMALLINT) version: number; + @SimpleLength({ + max: 255, + msg: `editorVersion must be 255 characters or less`, + }) @Column editorVersion: string; + @Length({ + max: MAX_TITLE_LENGTH, + msg: `Revision title must be ${MAX_TITLE_LENGTH} characters or less`, + }) @Column title: string; diff --git a/server/models/Team.ts b/server/models/Team.ts index 568784337..f26248247 100644 --- a/server/models/Team.ts +++ b/server/models/Team.ts @@ -15,6 +15,7 @@ import { Scopes, Is, DataType, + IsUUID, } from "sequelize-typescript"; import { getBaseDomain, RESERVED_SUBDOMAINS } from "@shared/utils/domains"; import env from "@server/env"; @@ -26,6 +27,7 @@ import TeamDomain from "./TeamDomain"; import User from "./User"; import ParanoidModel from "./base/ParanoidModel"; import Fix from "./decorators/Fix"; +import IsFQDN from "./validators/IsFQDN"; import Length from "./validators/Length"; import NotContainsUrl from "./validators/NotContainsUrl"; @@ -48,12 +50,17 @@ const readFile = util.promisify(fs.readFile); @Fix class Team extends ParanoidModel { @NotContainsUrl + @Length({ max: 255, msg: "name must be 255 characters or less" }) @Column name: string; @IsLowercase @Unique - @Length({ min: 4, max: 32, msg: "Must be between 4 and 32 characters" }) + @Length({ + min: 4, + max: 32, + msg: "subdomain must be between 4 and 32 characters", + }) @Is({ args: [/^[a-z\d-]+$/, "i"], msg: "Must be only alphanumeric and dashes", @@ -66,13 +73,16 @@ class Team extends ParanoidModel { subdomain: string | null; @Unique + @Length({ max: 255, msg: "domain must be 255 characters or less" }) + @IsFQDN @Column domain: string | null; + @IsUUID(4) @Column(DataType.UUID) defaultCollectionId: string | null; - @Length({ min: 0, max: 255, msg: "Must be less than 255 characters" }) + @Length({ max: 255, msg: "avatarUrl must be 255 characters or less" }) @Column avatarUrl: string | null; diff --git a/server/models/TeamDomain.ts b/server/models/TeamDomain.ts index 1f8a026a6..6779f2607 100644 --- a/server/models/TeamDomain.ts +++ b/server/models/TeamDomain.ts @@ -26,7 +26,7 @@ class TeamDomain extends IdModel { msg: "You chose a restricted domain, please try another.", }) @NotEmpty - @Length({ min: 0, max: 255, msg: "Must be less than 255 characters" }) + @Length({ max: 255, msg: "name must be 255 characters or less" }) @IsFQDN @Column name: string; diff --git a/server/models/User.ts b/server/models/User.ts index 3fd7490ef..31f904e62 100644 --- a/server/models/User.ts +++ b/server/models/User.ts @@ -17,6 +17,7 @@ import { DataType, HasMany, Scopes, + IsDate, } from "sequelize-typescript"; import { languages } from "@shared/i18n"; import { stringToColor } from "@shared/utils/color"; @@ -84,17 +85,17 @@ export enum UserFlag { @Fix class User extends ParanoidModel { @IsEmail - @Length({ max: 255, msg: "User email must be less than 255 characters" }) + @Length({ max: 255, msg: "User email must be 255 characters or less" }) @Column email: string | null; @NotContainsUrl - @Length({ max: 255, msg: "User username must be less than 255 characters" }) + @Length({ max: 255, msg: "User username must be 255 characters or less" }) @Column username: string | null; @NotContainsUrl - @Length({ max: 255, msg: "User name must be less than 255 characters" }) + @Length({ max: 255, msg: "User name must be 255 characters or less" }) @Column name: string; @@ -116,6 +117,7 @@ class User extends ParanoidModel { setEncryptedColumn(this, "jwtSecret", value); } + @IsDate @Column lastActiveAt: Date | null; @@ -123,6 +125,7 @@ class User extends ParanoidModel { @Column lastActiveIp: string | null; + @IsDate @Column lastSignedInAt: Date | null; @@ -130,9 +133,11 @@ class User extends ParanoidModel { @Column lastSignedInIp: string | null; + @IsDate @Column lastSigninEmailSentAt: Date | null; + @IsDate @Column suspendedAt: Date | null; @@ -144,11 +149,7 @@ class User extends ParanoidModel { @Column language: string; - @Length({ - min: 0, - max: 1000, - msg: "avatarUrl must be less than 1000 characters", - }) + @Length({ max: 1000, msg: "avatarUrl must be less than 1000 characters" }) @Column(DataType.STRING) get avatarUrl() { const original = this.getDataValue("avatarUrl"); diff --git a/server/test/factories.ts b/server/test/factories.ts index 086ed4d74..211defb00 100644 --- a/server/test/factories.ts +++ b/server/test/factories.ts @@ -364,7 +364,7 @@ export async function buildAttachment(overrides: Partial = {}) { count++; return Attachment.create({ key: `uploads/key/to/file ${count}.png`, - url: `https://redirect.url.com/uploads/key/to/file ${count}.png`, + url: `https://redirect.url.com/uploads/key/to/file${count}.png`, contentType: "image/png", size: 100, acl: "public-read",