From 3237a76e28700fd4221f6a74f0a404787b838703 Mon Sep 17 00:00:00 2001 From: Kian Cross Date: Fri, 26 Feb 2021 00:39:27 +0000 Subject: [PATCH] Added improved validation and tests to backend models (#269) --- packages/backend-core/package.json | 3 +- .../src/models/__tests__/base.test.ts | 26 +++- .../src/models/__tests__/community.test.ts | 62 +++++++- .../src/models/__tests__/post.test.ts | 135 ++++++++++++------ .../src/models/__tests__/public-user-test.ts | 41 ++++++ .../models/__tests__/remote-reference.test.ts | 19 ++- .../src/models/__tests__/user.test.ts | 2 +- packages/backend-core/src/models/base.ts | 6 +- packages/backend-core/src/models/community.ts | 25 +++- packages/backend-core/src/models/index.ts | 1 + packages/backend-core/src/models/post.ts | 22 ++- .../backend-core/src/models/public-user.ts | 42 ++++++ .../src/models/remote-reference.ts | 19 +-- packages/backend-core/src/models/user.ts | 23 +-- .../src/resolvers/users.ts | 6 +- .../src/services/communities.ts | 13 +- .../src/services/users.ts | 12 +- 17 files changed, 337 insertions(+), 120 deletions(-) create mode 100644 packages/backend-core/src/models/__tests__/public-user-test.ts create mode 100644 packages/backend-core/src/models/public-user.ts diff --git a/packages/backend-core/package.json b/packages/backend-core/package.json index 4dff1b7f..446689c2 100644 --- a/packages/backend-core/package.json +++ b/packages/backend-core/package.json @@ -53,7 +53,8 @@ "ts-node/register" ], "environmentVariables": { - "UNIFED_INTERNAL_REFERENCE": "this" + "UNIFED_INTERNAL_REFERENCE": "this", + "UNIFED_SITE_HOST": "localhost:8080" } } } diff --git a/packages/backend-core/src/models/__tests__/base.test.ts b/packages/backend-core/src/models/__tests__/base.test.ts index f88e846b..7ec5ba50 100644 --- a/packages/backend-core/src/models/__tests__/base.test.ts +++ b/packages/backend-core/src/models/__tests__/base.test.ts @@ -3,23 +3,35 @@ */ import test from "ava"; +import { validate } from "class-validator"; import { Base } from "../base"; class MockBase extends Base {} -test("id getter", (t) => { +test("Invalid ID", async (t) => { const base = new MockBase(); - t.is(base.id, undefined); + base.host = "foo.edu"; + + const result = await validate(base); + + t.is(result.length, 1); }); -test("id setter", (t) => { +test("Valid", async (t) => { const base = new MockBase(); - base.id = "someid"; - t.is(base.id, "someid"); + base.id = "foo"; + base.host = "foo.edu"; + + const result = await validate(base); + + t.is(result.length, 0); + t.is(base.id, "foo"); + t.is(base.host, "foo.edu"); }); test("toJSON", (t) => { const base = new MockBase(); - base.id = "someid"; - t.deepEqual(base.toJSON(), { id: "someid" }); + base.id = "foo"; + + t.deepEqual(base.toJSON(), { id: "foo" }); }); diff --git a/packages/backend-core/src/models/__tests__/community.test.ts b/packages/backend-core/src/models/__tests__/community.test.ts index 5a05768b..06c1b8ee 100644 --- a/packages/backend-core/src/models/__tests__/community.test.ts +++ b/packages/backend-core/src/models/__tests__/community.test.ts @@ -3,17 +3,67 @@ */ import test from "ava"; +import { validate } from "class-validator"; import { Community } from "../community"; +test("Title too short", async (t) => { + const community = new Community(); + community.id = "foo"; + community.title = ""; + community.description = "baz"; + community.admins = []; + + const result = await validate(community); + + t.is(result.length, 1); +}); + +test("Title too long", async (t) => { + const community = new Community(); + community.id = "foo"; + community.title = "a".repeat(64 + 1); + community.description = "baz"; + community.admins = []; + + const result = await validate(community); + + t.is(result.length, 1); +}); + +test("Description too short", async (t) => { + const community = new Community(); + community.id = "foo"; + community.title = "bar"; + community.description = ""; + community.admins = []; + + const result = await validate(community); + + t.is(result.length, 1); +}); + +test("Description too long", async (t) => { + const community = new Community(); + community.id = "foo"; + community.title = "bar"; + community.description = "a".repeat(10 * 1024 + 1); + community.admins = []; + + const result = await validate(community); + + t.is(result.length, 1); +}); + test("toJSON", (t) => { const community = new Community(); - community.id = "someid"; - community.title = "Some title"; - community.description = "Some description"; + community.id = "foo"; + community.title = "bar"; + community.description = "baz"; + t.deepEqual(community.toJSON(), { - id: "someid", - title: "Some title", - description: "Some description", + id: "foo", + title: "bar", + description: "baz", admins: [], }); }); diff --git a/packages/backend-core/src/models/__tests__/post.test.ts b/packages/backend-core/src/models/__tests__/post.test.ts index 6f706261..c13f39c5 100644 --- a/packages/backend-core/src/models/__tests__/post.test.ts +++ b/packages/backend-core/src/models/__tests__/post.test.ts @@ -3,66 +3,115 @@ */ import test from "ava"; +import { validate } from "class-validator"; import { Post } from "../post"; import { RemoteReference } from "../remote-reference"; -test("toJSON - community", (t) => { +const getPost = () => { const post = new Post(); - post.id = "someid"; - post.community = "somecomm"; - post.title = "A title"; + post.id = "foo"; + post.community = "bar"; + post.parentPost = "baz"; + post.title = "bat"; post.contentType = "markdown"; - post.body = "This is a post body"; + post.body = "bax"; + post.author = new RemoteReference(); - post.author.id = "someusername"; - post.author.host = "somehost:420"; + post.author.id = "mat"; + post.author.host = "foo.edu"; + post.approved = true; post.createdAt = new Date("2020-11-13T09:25:10+00:00"); post.updatedAt = new Date("2020-11-13T09:26:07+00:00"); - t.like(post.toJSON(), { - id: "someid", - parentPost: null, - community: "somecomm", - title: "A title", - contentType: "markdown", - body: "This is a post body", - children: [], - author: { - id: "someusername", - host: "somehost:420", - }, - created: 1605259510, - modified: 1605259567, - }); + return post; +}; + +test("Valid", async (t) => { + const post = getPost(); + + const result = await validate(post); + + t.is(result.length, 0); }); -test("toJSON - post", (t) => { - const post = new Post(); - post.id = "someid"; - post.community = "somecomm"; - post.parentPost = "parentpost"; - post.title = "A title"; - post.contentType = "markdown"; - post.body = "This is a post body"; - post.author = new RemoteReference(); - post.author.id = "someusername"; - post.author.host = "somehost:420"; - post.approved = true; - post.createdAt = new Date("2020-11-13T09:25:10+00:00"); - post.updatedAt = new Date("2020-11-13T09:26:07+00:00"); +test("Invalid community", async (t) => { + const post = getPost(); + post.community = undefined; + + const result = await validate(post); + + t.is(result.length, 1); +}); + +test("Valid parentPost", async (t) => { + const post = getPost(); + post.parentPost = undefined; + + const result = await validate(post); + + t.is(result.length, 0); +}); + +test("Title too short", async (t) => { + const post = getPost(); + post.title = ""; + + const result = await validate(post); + + t.is(result.length, 1); +}); + +test("Title too long", async (t) => { + const post = getPost(); + post.title = "a".repeat(128 + 1); + + const result = await validate(post); + + t.is(result.length, 1); +}); + +test("Invalid contentType", async (t) => { + const post = getPost(); + post.contentType = "bar"; + + const result = await validate(post); + + t.is(result.length, 1); +}); + +test("Body too short", async (t) => { + const post = getPost(); + post.body = ""; + + const result = await validate(post); + + t.is(result.length, 1); +}); + +test("Body too long", async (t) => { + const post = getPost(); + post.body = "a".repeat(1024 * 1024 * 500 + 1); + + const result = await validate(post); + + t.is(result.length, 1); +}); + +test("toJSON", (t) => { + const post = getPost(); t.like(post.toJSON(), { - id: "someid", - parentPost: "parentpost", - community: "somecomm", - title: "A title", + id: "foo", + community: "bar", + parentPost: "baz", + title: "bat", contentType: "markdown", - body: "This is a post body", + body: "bax", children: [], author: { - id: "someusername", - host: "somehost:420", + id: "mat", + host: "foo.edu", }, created: 1605259510, modified: 1605259567, diff --git a/packages/backend-core/src/models/__tests__/public-user-test.ts b/packages/backend-core/src/models/__tests__/public-user-test.ts new file mode 100644 index 00000000..6d7857ec --- /dev/null +++ b/packages/backend-core/src/models/__tests__/public-user-test.ts @@ -0,0 +1,41 @@ +/* + * CS3099 Group A3 + */ + +import test from "ava"; +import { validate } from "class-validator"; +import { PublicUser, UserProfile } from "../public-user"; + +test("Invalid username", async (t) => { + const user = new PublicUser(); + user.profile = new UserProfile(); + user.profile.name = "foo"; + + const result = await validate(user); + + t.is(result.length, 1); +}); + +test("Invalid profile", async (t) => { + const user = new PublicUser(); + user.username = "foo"; + + const result = await validate(user); + + t.is(result.length, 1); +}); + +test("toJSON", (t) => { + const user = new PublicUser(); + user.username = "foo"; + + user.profile = new UserProfile(); + user.profile.name = "bar"; + + t.deepEqual(user.toJSON(), { + username: "foo", + profile: { + name: "bar", + }, + }); +}); diff --git a/packages/backend-core/src/models/__tests__/remote-reference.test.ts b/packages/backend-core/src/models/__tests__/remote-reference.test.ts index 99c72681..e3772c9d 100644 --- a/packages/backend-core/src/models/__tests__/remote-reference.test.ts +++ b/packages/backend-core/src/models/__tests__/remote-reference.test.ts @@ -5,13 +5,24 @@ import test from "ava"; import { RemoteReference } from "../remote-reference"; -test("toJSON", (t) => { +test("Internal reference", async (t) => { const reference = new RemoteReference(); - reference.id = "someid"; - reference.host = "localhost:8080"; + reference.id = "foo"; + reference.host = "this"; t.deepEqual(reference.toJSON(), { - id: "someid", + id: "foo", host: "localhost:8080", }); }); + +test("toJSON", (t) => { + const reference = new RemoteReference(); + reference.id = "foo"; + reference.host = "bar"; + + t.deepEqual(reference.toJSON(), { + id: "foo", + host: "bar", + }); +}); diff --git a/packages/backend-core/src/models/__tests__/user.test.ts b/packages/backend-core/src/models/__tests__/user.test.ts index 72f36531..540c4daf 100644 --- a/packages/backend-core/src/models/__tests__/user.test.ts +++ b/packages/backend-core/src/models/__tests__/user.test.ts @@ -5,7 +5,7 @@ import test from "ava"; import { User } from "../user"; -test("New user", (t) => { +test("Load", (t) => { new User(); t.pass(); }); diff --git a/packages/backend-core/src/models/base.ts b/packages/backend-core/src/models/base.ts index 36753e30..59b3cbab 100644 --- a/packages/backend-core/src/models/base.ts +++ b/packages/backend-core/src/models/base.ts @@ -14,16 +14,12 @@ export type EntityID = string; export abstract class Base extends defaultClasses.TimeStamps { @IsNotEmpty() @IsString() - @Property({ default: uuidv4 }) + @Property({ default: uuidv4, required: true }) _id!: EntityID; - @IsNotEmpty() - @IsString() @Field() host?: string; - @IsNotEmpty() - @IsString() @Field(() => ID) get id(): EntityID { return this._id; diff --git a/packages/backend-core/src/models/community.ts b/packages/backend-core/src/models/community.ts index a487fd36..13b161eb 100644 --- a/packages/backend-core/src/models/community.ts +++ b/packages/backend-core/src/models/community.ts @@ -2,24 +2,34 @@ * CS3099 Group A3 */ -import { IsArray, IsString, IsNotEmpty, ValidateNested } from "class-validator"; +import { IsArray, IsString, IsNotEmpty, ValidateNested, MaxLength } from "class-validator"; import { Type } from "class-transformer"; import { prop as Property, getModelForClass, Ref } from "@typegoose/typegoose"; import { ObjectType, Field } from "type-graphql"; -import { RemoteReference } from "./remote-reference"; import { JSONMap } from "../types"; import { Base } from "./base"; import { Post } from "./post"; +import { RemoteReference } from "./remote-reference"; @ObjectType() export class Community extends Base { - @IsNotEmpty() + @MaxLength(64, { + message: "Title is too long", + }) + @IsNotEmpty({ + message: "Title must not be empty", + }) @IsString() @Field() @Property({ required: true }) title!: string; - @IsNotEmpty() + @MaxLength(10 * 1024, { + message: "Description is too long", + }) + @IsNotEmpty({ + message: "Description must not be empty", + }) @IsString() @Field() @Property({ required: true }) @@ -29,7 +39,6 @@ export class Community extends Base { @Field(() => [Post]) @Property({ ref: "Post", - type: String, foreignField: "community", localField: "_id", }) @@ -37,8 +46,12 @@ export class Community extends Base { @IsArray() @ValidateNested() + @Field(() => [RemoteReference]) @Type(() => RemoteReference) - @Property({ _id: false, type: RemoteReference, required: true }) + @Property({ + _id: false, + ref: RemoteReference, + }) admins?: RemoteReference[]; toJSON(): JSONMap { diff --git a/packages/backend-core/src/models/index.ts b/packages/backend-core/src/models/index.ts index 38b57c89..232878cc 100644 --- a/packages/backend-core/src/models/index.ts +++ b/packages/backend-core/src/models/index.ts @@ -4,6 +4,7 @@ import "reflect-metadata"; +export * from "./public-user"; export * from "./user"; export * from "./post"; export * from "./community"; diff --git a/packages/backend-core/src/models/post.ts b/packages/backend-core/src/models/post.ts index 416a4947..f908a0b4 100644 --- a/packages/backend-core/src/models/post.ts +++ b/packages/backend-core/src/models/post.ts @@ -2,6 +2,7 @@ * CS3099 Group A3 */ +import { ValidateNested, Matches, MaxLength, IsString, IsNotEmpty } from "class-validator"; import { prop as Property, getModelForClass, Ref } from "@typegoose/typegoose"; import { ObjectType, Field } from "type-graphql"; import { dateToUnixTimestamp } from "./helpers"; @@ -13,6 +14,7 @@ import { JSONMap } from "../types"; @ObjectType() export class Post extends Base { + @IsNotEmpty() @Field(() => Community) @Property({ ref: "Community", type: String }) community!: Ref; @@ -21,18 +23,36 @@ export class Post extends Base { @Property({ ref: "Post", type: String }) parentPost?: Ref; + @MaxLength(128, { + message: "Title is too long", + }) + @IsNotEmpty({ + message: "Title must not be empty", + }) + @IsString() @Field({ nullable: true }) @Property() title!: string; - @Field() + @Matches(/^(text)|(markdown)$/, { + message: "Only `text` and `markdown` content types are supported", + }) @Property({ required: true }) contentType!: string; + @MaxLength(1024 * 1024 * 500, { + message: "Body is too long", + }) + @IsNotEmpty({ + message: "Body must be a non-empty string", + }) + @IsString() @Field() @Property({ required: true }) body!: string; + @IsNotEmpty() + @ValidateNested() @Field() @Property({ _id: false, required: true }) author!: RemoteReference; diff --git a/packages/backend-core/src/models/public-user.ts b/packages/backend-core/src/models/public-user.ts new file mode 100644 index 00000000..858ddb52 --- /dev/null +++ b/packages/backend-core/src/models/public-user.ts @@ -0,0 +1,42 @@ +/* + * CS3099 Group A3 + */ + +import { ValidateNested, IsNotEmpty } from "class-validator"; +import { prop as Property } from "@typegoose/typegoose"; +import { ObjectType, Field } from "type-graphql"; +import { JSONMap } from "../types"; +import { Base } from "./base"; + +@ObjectType() +export class UserProfile { + @Field() + @Property({ required: true }) + name!: string; + + toJSON(): JSONMap { + return { + name: this.name, + }; + } +} + +export class PublicUser extends Base { + @IsNotEmpty() + @Field() + @Property({ required: true }) + username!: string; + + @ValidateNested() + @Field() + @Property({ _id: false, ref: UserProfile, required: true }) + profile!: UserProfile; + + toJSON(): JSONMap { + return { + ...super.toJSON(), + ...this.profile.toJSON(), + username: this.username, + }; + } +} diff --git a/packages/backend-core/src/models/remote-reference.ts b/packages/backend-core/src/models/remote-reference.ts index 5712cbce..6de21d2b 100644 --- a/packages/backend-core/src/models/remote-reference.ts +++ b/packages/backend-core/src/models/remote-reference.ts @@ -2,30 +2,19 @@ * CS3099 Group A3 */ -import { IsString, IsNotEmpty } from "class-validator"; -import { ObjectType, Field } from "type-graphql"; -import { prop as Property } from "@typegoose/typegoose"; +import { ObjectType } from "type-graphql"; import { JSONMap } from "../types"; import { config } from "../config"; +import { Base } from "./base"; @ObjectType() -export class RemoteReference { - @IsNotEmpty() - @IsString() - @Field() - @Property({ required: true }) - id!: string; - - @IsNotEmpty() - @IsString() - @Field() - @Property({ required: true }) +export class RemoteReference extends Base { host!: string; toJSON(): JSONMap { return { id: this.id, - host: this.host == config.internalReference ? config.siteHost : this.host, + host: this.host === config.internalReference ? config.siteHost : this.host || null, }; } } diff --git a/packages/backend-core/src/models/user.ts b/packages/backend-core/src/models/user.ts index 7c220d0c..db74526e 100644 --- a/packages/backend-core/src/models/user.ts +++ b/packages/backend-core/src/models/user.ts @@ -4,16 +4,9 @@ import { prop as Property, getModelForClass } from "@typegoose/typegoose"; import { ObjectType, Field } from "type-graphql"; -import { Base } from "./base"; +import { PublicUser } from "./public-user"; import { RemoteReference } from "./remote-reference"; -@ObjectType() -export class UserProfile { - @Field() - @Property({ required: true }) - name!: string; -} - @ObjectType() class EmailRecord { @Field({ nullable: true }) @@ -26,21 +19,13 @@ class EmailRecord { } @ObjectType() -export class User extends Base { - @Field() - @Property({ required: true }) - username!: string; - +export class User extends PublicUser { @Field(() => [EmailRecord]) - @Property({ type: EmailRecord, id: false, required: true }) + @Property({ id: false, ref: EmailRecord, type: EmailRecord, required: true }) emails!: EmailRecord[]; - @Field() - @Property({ _id: false, required: true }) - profile!: UserProfile; - @Field(() => [RemoteReference]) - @Property({ id: false, required: true }) + @Property({ id: false, Ref: RemoteReference, type: RemoteReference, required: true }) subscriptions!: RemoteReference[]; } diff --git a/packages/backend-internal-server/src/resolvers/users.ts b/packages/backend-internal-server/src/resolvers/users.ts index 6181858a..b6dff794 100644 --- a/packages/backend-internal-server/src/resolvers/users.ts +++ b/packages/backend-internal-server/src/resolvers/users.ts @@ -14,7 +14,7 @@ import { } from "type-graphql"; import { CurrentUser, translateHost } from "./helpers"; import { AuthoriseUser } from "../auth-checkers"; -import { RemoteReference, User } from "@unifed/backend-core"; +import { RemoteReference, User, UserProfile } from "@unifed/backend-core"; import { RemoteReferenceInput, UserProfileInput } from "./inputs"; import { UsersService } from "../services"; @@ -60,8 +60,8 @@ export class UsersResolver implements ResolverInterface { return this.usersService.getSubscriptions(user.id); } - @FieldResolver() - profile(@Root() user: User) { + @FieldResolver(() => UserProfile) + profile(@Root() user: User): UserProfile { return user.profile; } } diff --git a/packages/backend-internal-server/src/services/communities.ts b/packages/backend-internal-server/src/services/communities.ts index 1da851a9..62eda176 100644 --- a/packages/backend-internal-server/src/services/communities.ts +++ b/packages/backend-internal-server/src/services/communities.ts @@ -3,22 +3,21 @@ */ import { Service } from "typedi"; -import { CommunityModel } from "@unifed/backend-core"; +import { CommunityModel, RemoteReference } from "@unifed/backend-core"; import { CommunitiesFederationService } from "@unifed/backend-federation-client"; @Service() export class CommunitiesService extends CommunitiesFederationService { async create(username: string, id: string, title: string, description: string): Promise { + const admin = new RemoteReference(); + admin.id = username; + admin.host = "this"; + await CommunityModel.create({ _id: id, title, description, - admins: [ - { - id: username, - host: "this", - }, - ], + admins: [admin], }); return true; } diff --git a/packages/backend-internal-server/src/services/users.ts b/packages/backend-internal-server/src/services/users.ts index 88cd35f8..0b5fce8f 100644 --- a/packages/backend-internal-server/src/services/users.ts +++ b/packages/backend-internal-server/src/services/users.ts @@ -8,7 +8,7 @@ import { RemoteReference, UserModel } from "@unifed/backend-core"; @Service() export class UsersService { async updateProfile(id: string, profile: { name: string }): Promise { - await UserModel.updateOne({ _id: id }, { $set: { profile } }); + await UserModel.updateOne({ _id: id }, { $set: { profile: profile as any } }); return true; } @@ -16,7 +16,9 @@ export class UsersService { const community: RemoteReference = new RemoteReference(); community.id = communityId; community.host = host; + if (await UserModel.exists({ _id: userId, subscriptions: community })) return true; + return ( (await UserModel.findByIdAndUpdate( { _id: userId }, @@ -29,6 +31,7 @@ export class UsersService { const community: RemoteReference = new RemoteReference(); community.id = communityId; community.host = host; + if (await UserModel.exists({ _id: userId, subscriptions: community })) { return ( (await UserModel.findByIdAndUpdate( @@ -37,12 +40,17 @@ export class UsersService { )) != null ); } + return true; } async getSubscriptions(id: string): Promise { const user = await UserModel.findOne({ _id: id }, "subscriptions").exec(); + if (!user) return []; - return user?.subscriptions; + + return user.subscriptions.filter( + (subscription): subscription is RemoteReference => typeof subscription !== "string", + ); } }