From e70f6f5c7fdc6d78970f4272c097e0d5eff9b3d0 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Fri, 22 Mar 2019 17:09:22 +0000 Subject: [PATCH] [CORL-144, CORL-143] Nudging (#2236) * feat: added nudging beheviour to server * fix: review fixes * fix: fixed missed change * fix: fixed sitewide permission error * feat: implement client side nudging * test: add feature tests --- src/core/client/framework/lib/errors/index.ts | 1 + .../lib/errors/invalidRequestError.ts | 2 +- .../lib/errors/moderationNudgeError.ts | 39 ++++++++++ .../framework/lib/network/extractError.ts | 9 ++- src/core/client/framework/lib/rest.ts | 2 +- .../testHelpers/createRelayEnvironment.ts | 10 ++- .../stream/mutations/CreateCommentMutation.ts | 1 + .../mutations/CreateCommentReplyMutation.ts | 1 + .../PostCommentFormContainer.spec.tsx | 1 + .../containers/PostCommentFormContainer.tsx | 27 ++++++- .../ReplyCommentFormContainer.spec.tsx | 1 + .../containers/ReplyCommentFormContainer.tsx | 27 ++++++- .../stream/test/comments/postComment.spec.tsx | 73 +++++++++++++++++-- .../stream/test/comments/postReply.spec.tsx | 72 +++++++++++++++++- src/core/common/errors.ts | 18 +++++ .../server/app/middleware/passport/index.ts | 16 +--- .../middleware/passport/strategies/local.ts | 6 +- src/core/server/errors/index.ts | 31 ++++++++ src/core/server/errors/translations.ts | 3 + .../server/graph/tenant/mutators/Comments.ts | 2 + .../server/graph/tenant/schema/schema.graphql | 16 +++- src/core/server/locales/en-US/errors.ftl | 5 +- src/core/server/models/comment.ts | 1 + src/core/server/services/comments/index.ts | 38 +++++++--- .../services/comments/pipeline/index.ts | 1 + .../services/comments/pipeline/phases/spam.ts | 13 ++++ .../comments/pipeline/phases/toxic.ts | 46 +++++++----- 27 files changed, 401 insertions(+), 61 deletions(-) create mode 100644 src/core/client/framework/lib/errors/moderationNudgeError.ts diff --git a/src/core/client/framework/lib/errors/index.ts b/src/core/client/framework/lib/errors/index.ts index fdbc76efd..24292d061 100644 --- a/src/core/client/framework/lib/errors/index.ts +++ b/src/core/client/framework/lib/errors/index.ts @@ -1,2 +1,3 @@ export { default as UnknownServerError } from "./unknownServerError"; export { default as InvalidRequestError } from "./invalidRequestError"; +export { default as ModerationNudgeError } from "./moderationNudgeError"; diff --git a/src/core/client/framework/lib/errors/invalidRequestError.ts b/src/core/client/framework/lib/errors/invalidRequestError.ts index fe0655af7..04a5b1e33 100644 --- a/src/core/client/framework/lib/errors/invalidRequestError.ts +++ b/src/core/client/framework/lib/errors/invalidRequestError.ts @@ -14,7 +14,7 @@ interface InvalidRequestExtension { } /** - * InvalidRequestError wraps the `BAD_USER_INPUT` error returned from the + * InvalidRequestError wraps the `INVALID_REQUEST_ERROR` error returned from the * server. */ export default class InvalidRequestError extends Error diff --git a/src/core/client/framework/lib/errors/moderationNudgeError.ts b/src/core/client/framework/lib/errors/moderationNudgeError.ts new file mode 100644 index 000000000..a7d19457f --- /dev/null +++ b/src/core/client/framework/lib/errors/moderationNudgeError.ts @@ -0,0 +1,39 @@ +import { ERROR_CODES } from "talk-common/errors"; + +/** + * Shape of the `ModerationNudge` extension as + * the client requires. Note: the only crucial + * field is the `code` field. + */ +interface ModerationNudgeExtension { + code: ERROR_CODES; + message?: string; + id?: string; +} + +/** + * ModeratioNudgeError wraps the `MODERATION_NUDGE_ERROR` error returned from the + * server. + */ +export default class ModeratioNudgeError extends Error + implements ModerationNudgeExtension { + // Keep extension of original server response. + public readonly extension: ModerationNudgeExtension; + public readonly code: ERROR_CODES; + public readonly id?: string; + public readonly message: string; + public readonly extensions: string; + + constructor(extension: ModerationNudgeExtension) { + super("ModeratioNudgeError"); + + // Maintains proper stack trace for where our error was thrown. + if (Error.captureStackTrace) { + Error.captureStackTrace(this, ModeratioNudgeError); + } + this.extension = extension; + this.code = extension.code; + this.id = extension.id; + this.message = extension.message || extension.code; + } +} diff --git a/src/core/client/framework/lib/network/extractError.ts b/src/core/client/framework/lib/network/extractError.ts index 5430a5107..91bcea0aa 100644 --- a/src/core/client/framework/lib/network/extractError.ts +++ b/src/core/client/framework/lib/network/extractError.ts @@ -1,6 +1,10 @@ import { ERROR_TYPES } from "talk-common/errors"; -import { InvalidRequestError, UnknownServerError } from "../errors"; +import { + InvalidRequestError, + ModerationNudgeError, + UnknownServerError, +} from "../errors"; export default function extractError(errors: Error[]): Error | null { if (errors.length > 1 || !(errors[0] as any).extensions) { @@ -15,5 +19,8 @@ export default function extractError(errors: Error[]): Error | null { if ((err as any).extensions.type === ERROR_TYPES.INVALID_REQUEST_ERROR) { return new InvalidRequestError((err as any).extensions); } + if ((err as any).extensions.type === ERROR_TYPES.MODERATION_NUDGE_ERROR) { + return new ModerationNudgeError((err as any).extensions); + } return new UnknownServerError(err.message, (err as any).extensions); } diff --git a/src/core/client/framework/lib/rest.ts b/src/core/client/framework/lib/rest.ts index 84986b9bc..7d7cb2698 100644 --- a/src/core/client/framework/lib/rest.ts +++ b/src/core/client/framework/lib/rest.ts @@ -27,7 +27,7 @@ const handleResp = async (res: Response) => { if (!res.ok) { const response = await res.json(); - throw new Error(response.error); + throw new Error(response.error.message); } if (res.status === 204) { diff --git a/src/core/client/framework/testHelpers/createRelayEnvironment.ts b/src/core/client/framework/testHelpers/createRelayEnvironment.ts index a0edc2b11..e6ee25874 100644 --- a/src/core/client/framework/testHelpers/createRelayEnvironment.ts +++ b/src/core/client/framework/testHelpers/createRelayEnvironment.ts @@ -19,7 +19,10 @@ import { } from "talk-framework/lib/relay"; import { loadSchema } from "talk-common/graphql"; -import { InvalidRequestError } from "talk-framework/lib/errors"; +import { + InvalidRequestError, + ModerationNudgeError, +} from "talk-framework/lib/errors"; export interface CreateRelayEnvironmentNetworkParams { /** project name of graphql-config */ @@ -72,7 +75,10 @@ function createFetch({ if (payload.errors) { payload.errors.forEach(e => { // Throw our custom errors directly. - if (e.originalError instanceof InvalidRequestError) { + if ( + e.originalError instanceof InvalidRequestError || + e.originalError instanceof ModerationNudgeError + ) { throw e.originalError; } }); diff --git a/src/core/client/stream/mutations/CreateCommentMutation.ts b/src/core/client/stream/mutations/CreateCommentMutation.ts index 4fb8eea59..100d7577a 100644 --- a/src/core/client/stream/mutations/CreateCommentMutation.ts +++ b/src/core/client/stream/mutations/CreateCommentMutation.ts @@ -125,6 +125,7 @@ function commit( input: { storyID: input.storyID, body: input.body, + nudge: input.nudge, clientMutationId: clientMutationId.toString(), }, }, diff --git a/src/core/client/stream/mutations/CreateCommentReplyMutation.ts b/src/core/client/stream/mutations/CreateCommentReplyMutation.ts index b99050192..dd26cd7f4 100644 --- a/src/core/client/stream/mutations/CreateCommentReplyMutation.ts +++ b/src/core/client/stream/mutations/CreateCommentReplyMutation.ts @@ -154,6 +154,7 @@ function commit( parentID: input.parentID, parentRevisionID: input.parentRevisionID, body: input.body, + nudge: input.nudge, clientMutationId: clientMutationId.toString(), }, }, diff --git a/src/core/client/stream/tabs/comments/containers/PostCommentFormContainer.spec.tsx b/src/core/client/stream/tabs/comments/containers/PostCommentFormContainer.spec.tsx index 6206b5f7b..746b93243 100644 --- a/src/core/client/stream/tabs/comments/containers/PostCommentFormContainer.spec.tsx +++ b/src/core/client/stream/tabs/comments/containers/PostCommentFormContainer.spec.tsx @@ -118,6 +118,7 @@ it("creates a comment", async () => { expect( createCommentStub.calledWith({ storyID, + nudge: true, ...input, }) ).toBeTruthy(); diff --git a/src/core/client/stream/tabs/comments/containers/PostCommentFormContainer.tsx b/src/core/client/stream/tabs/comments/containers/PostCommentFormContainer.tsx index 2d1b7fd12..3accef551 100644 --- a/src/core/client/stream/tabs/comments/containers/PostCommentFormContainer.tsx +++ b/src/core/client/stream/tabs/comments/containers/PostCommentFormContainer.tsx @@ -1,7 +1,11 @@ +import { FORM_ERROR } from "final-form"; import React, { Component } from "react"; import { withContext } from "talk-framework/lib/bootstrap"; -import { InvalidRequestError } from "talk-framework/lib/errors"; +import { + InvalidRequestError, + ModerationNudgeError, +} from "talk-framework/lib/errors"; import { graphql, withFragmentContainer, @@ -41,6 +45,8 @@ interface Props { } interface State { + /** nudge will turn on the nudging behavior on the server */ + nudge: boolean; initialValues?: PropTypesOf["initialValues"]; initialized: boolean; keepFormWhenClosed: boolean; @@ -52,6 +58,7 @@ const contextKey = "postCommentFormBody"; export class PostCommentFormContainer extends Component { public state: State = { initialized: false, + nudge: true, keepFormWhenClosed: this.props.local.loggedIn && !this.props.story.isClosed && @@ -78,6 +85,12 @@ export class PostCommentFormContainer extends Component { }); } + private disableNudge = () => { + if (this.state.nudge) { + this.setState({ nudge: false }); + } + }; + private handleOnSubmit: PropTypesOf< typeof PostCommentForm >["onSubmit"] = async (input, form) => { @@ -85,13 +98,14 @@ export class PostCommentFormContainer extends Component { const submitStatus = getSubmitStatus( await this.props.createComment({ storyID: this.props.story.id, + nudge: this.state.nudge, ...input, }) ); if (submitStatus !== "RETRY") { form.reset({}); } - this.setState({ submitStatus }); + this.setState({ submitStatus, nudge: true }); } catch (error) { if (error instanceof InvalidRequestError) { if (shouldTriggerSettingsRefresh(error.code)) { @@ -99,6 +113,15 @@ export class PostCommentFormContainer extends Component { } return error.invalidArgs; } + /** + * Comment was caught in one of the moderation filters on the server. + * We give the user another change to submit the comment, and we + * turn off the nudging behavior on the next try. + */ + if (error instanceof ModerationNudgeError) { + this.disableNudge(); + return { [FORM_ERROR]: error.message }; + } // tslint:disable-next-line:no-console console.error(error); } diff --git a/src/core/client/stream/tabs/comments/containers/ReplyCommentFormContainer.spec.tsx b/src/core/client/stream/tabs/comments/containers/ReplyCommentFormContainer.spec.tsx index c2a91c6b3..abb2d69b4 100644 --- a/src/core/client/stream/tabs/comments/containers/ReplyCommentFormContainer.spec.tsx +++ b/src/core/client/stream/tabs/comments/containers/ReplyCommentFormContainer.spec.tsx @@ -133,6 +133,7 @@ it("creates a comment", async () => { storyID, parentID: props.comment.id, parentRevisionID: "revision-id", + nudge: true, ...input, }) ).toBeTruthy(); diff --git a/src/core/client/stream/tabs/comments/containers/ReplyCommentFormContainer.tsx b/src/core/client/stream/tabs/comments/containers/ReplyCommentFormContainer.tsx index 72d39c9f5..9f4ea8bb1 100644 --- a/src/core/client/stream/tabs/comments/containers/ReplyCommentFormContainer.tsx +++ b/src/core/client/stream/tabs/comments/containers/ReplyCommentFormContainer.tsx @@ -1,9 +1,13 @@ import { CoralRTE } from "@coralproject/rte"; +import { FORM_ERROR } from "final-form"; import React, { Component } from "react"; import { graphql } from "react-relay"; import { withContext } from "talk-framework/lib/bootstrap"; -import { InvalidRequestError } from "talk-framework/lib/errors"; +import { + InvalidRequestError, + ModerationNudgeError, +} from "talk-framework/lib/errors"; import { withFragmentContainer } from "talk-framework/lib/relay"; import { PromisifiedStorage } from "talk-framework/lib/storage"; import { PropTypesOf } from "talk-framework/types"; @@ -42,6 +46,8 @@ interface Props { } interface State { + /** nudge will turn on the nudging behavior on the server */ + nudge: boolean; initialValues?: ReplyCommentFormProps["initialValues"]; initialized: boolean; submitStatus: SubmitStatus | null; @@ -49,6 +55,7 @@ interface State { export class ReplyCommentFormContainer extends Component { public state: State = { + nudge: true, initialized: false, submitStatus: null, }; @@ -86,6 +93,12 @@ export class ReplyCommentFormContainer extends Component { } }; + private disableNudge = () => { + if (this.state.nudge) { + this.setState({ nudge: false }); + } + }; + private handleOnSubmit: ReplyCommentFormProps["onSubmit"] = async input => { try { const submitStatus = getSubmitStatus( @@ -94,6 +107,7 @@ export class ReplyCommentFormContainer extends Component { parentID: this.props.comment.id, parentRevisionID: this.props.comment.revision.id, local: this.props.localReply, + nudge: this.state.nudge, ...input, }) ); @@ -104,7 +118,7 @@ export class ReplyCommentFormContainer extends Component { return; } } - this.setState({ submitStatus }); + this.setState({ submitStatus, nudge: true }); } catch (error) { if (error instanceof InvalidRequestError) { if (shouldTriggerSettingsRefresh(error.code)) { @@ -112,6 +126,15 @@ export class ReplyCommentFormContainer extends Component { } return error.invalidArgs; } + /** + * Comment was caught in one of the moderation filters on the server. + * We give the user another change to submit the comment, and we + * turn off the nudging behavior on the next try. + */ + if (error instanceof ModerationNudgeError) { + this.disableNudge(); + return { [FORM_ERROR]: error.message }; + } // tslint:disable-next-line:no-console console.error(error); } diff --git a/src/core/client/stream/test/comments/postComment.spec.tsx b/src/core/client/stream/test/comments/postComment.spec.tsx index b2f2d89c6..34752bfc2 100644 --- a/src/core/client/stream/test/comments/postComment.spec.tsx +++ b/src/core/client/stream/test/comments/postComment.spec.tsx @@ -2,7 +2,10 @@ import sinon from "sinon"; import timekeeper from "timekeeper"; import { ERROR_CODES } from "talk-common/errors"; -import { InvalidRequestError } from "talk-framework/lib/errors"; +import { + InvalidRequestError, + ModerationNudgeError, +} from "talk-framework/lib/errors"; import { createSinonStub, findParentWithType, @@ -168,7 +171,7 @@ it("post a comment and handle non-visible comment state (dismiss by typing)", as })); it("post a comment and handle server error", async () => { - const { form, rte, tabPane } = await createTestRenderer( + const { form, rte } = await createTestRenderer( { Mutation: { createComment: sinon.stub().callsFake(() => { @@ -180,12 +183,72 @@ it("post a comment and handle server error", async () => { ); rte.props.onChange({ html: "Hello world!" }); - timekeeper.freeze(new Date(baseComment.createdAt)); form.props.onSubmit(); - timekeeper.reset(); // Look for internal error being displayed. - await waitForElement(() => within(tabPane).getByText("INTERNAL_ERROR")); + await waitForElement(() => within(form).getByText("INTERNAL_ERROR")); +}); + +it("handle moderation nudge error", async () => { + const { form, rte } = await createTestRenderer( + { + Mutation: { + createComment: createSinonStub( + s => + s.onFirstCall().callsFake((_, data) => { + expectAndFail(data).toMatchObject({ + input: { + storyID: stories[0].id, + body: "Hello world!", + nudge: true, + }, + }); + throw new ModerationNudgeError({ + code: ERROR_CODES.TOXIC_COMMENT, + }); + }), + s => + s.onSecondCall().callsFake((_, data) => { + expectAndFail(data).toMatchObject({ + input: { + storyID: stories[0].id, + body: "Hello world!", + nudge: false, + }, + }); + return { + edge: { + cursor: "", + node: { + ...baseComment, + id: "comment-x", + status: "SYSTEM_WITHHELD", + author: users[0], + body: "Hello world!", + }, + }, + clientMutationId: data.input.clientMutationId, + }; + }) + ), + }, + }, + { muteNetworkErrors: true } + ); + + rte.props.onChange({ html: "Hello world!" }); + form.props.onSubmit(); + + // Look for internal error being displayed. + await waitForElement(() => within(form).getByText("TOXIC_COMMENT")); + + // Try again, now nudging should be disabled. + form.props.onSubmit(); + + // Comment should now go to moderation. + await waitForElement(() => + within(form).getByText("will be reviewed", { exact: false }) + ); }); it("handle disabled commenting error", async () => { diff --git a/src/core/client/stream/test/comments/postReply.spec.tsx b/src/core/client/stream/test/comments/postReply.spec.tsx index 933d25853..401629d96 100644 --- a/src/core/client/stream/test/comments/postReply.spec.tsx +++ b/src/core/client/stream/test/comments/postReply.spec.tsx @@ -2,8 +2,12 @@ import sinon from "sinon"; import timekeeper from "timekeeper"; import { ERROR_CODES } from "talk-common/errors"; -import { InvalidRequestError } from "talk-framework/lib/errors"; import { + InvalidRequestError, + ModerationNudgeError, +} from "talk-framework/lib/errors"; +import { + createSinonStub, findParentWithType, waitForElement, within, @@ -182,13 +186,77 @@ it("post a reply and handle server error", async () => { // Write reply . rte.props.onChange({ html: "Hello world!" }); - timekeeper.freeze(new Date(baseComment.createdAt)); form.props.onSubmit(); // Look for internal error being displayed. await waitForElement(() => within(comment).getByText("INTERNAL_ERROR")); }); +it("handle moderation nudge error", async () => { + const { form, rte, comment } = await createTestRenderer( + { + Mutation: { + createCommentReply: createSinonStub( + s => + s.onFirstCall().callsFake((_, data) => { + expectAndFail(data).toMatchObject({ + input: { + storyID: stories[0].id, + parentID: stories[0].comments.edges[0].node.id, + parentRevisionID: + stories[0].comments.edges[0].node.revision.id, + body: "Hello world!", + nudge: true, + }, + }); + throw new ModerationNudgeError({ + code: ERROR_CODES.TOXIC_COMMENT, + }); + }), + s => + s.onSecondCall().callsFake((_, data) => { + expectAndFail(data).toMatchObject({ + input: { + storyID: stories[0].id, + body: "Hello world!", + nudge: false, + }, + }); + return { + edge: { + cursor: "", + node: { + ...baseComment, + id: "comment-x", + status: "SYSTEM_WITHHELD", + author: users[0], + body: "Hello world!", + }, + }, + clientMutationId: data.input.clientMutationId, + }; + }) + ), + }, + }, + { muteNetworkErrors: true } + ); + + rte.props.onChange({ html: "Hello world!" }); + form.props.onSubmit(); + + // Look for internal error being displayed. + await waitForElement(() => within(form).getByText("TOXIC_COMMENT")); + + // Try again, now nudging should be disabled. + form.props.onSubmit(); + + // Comment should now go to moderation. + await waitForElement(() => + within(comment).getByText("will be reviewed", { exact: false }) + ); +}); + it("handle disabled commenting error", async () => { let returnSettings = settings; const { rte, form } = await createTestRenderer( diff --git a/src/core/common/errors.ts b/src/core/common/errors.ts index f2003d643..6631194d2 100644 --- a/src/core/common/errors.ts +++ b/src/core/common/errors.ts @@ -1,5 +1,6 @@ export enum ERROR_TYPES { INVALID_REQUEST_ERROR = "INVALID_REQUEST_ERROR", + MODERATION_NUDGE_ERROR = "MODERATION_NUDGE_ERROR", } export enum ERROR_CODES { @@ -180,4 +181,21 @@ export enum ERROR_CODES { * occurred and the request can not be processed. */ AUTHENTICATION_ERROR = "AUTHENTICATION_ERROR", + + /** + * INVALID_CREDENTIALS is returned when the passed credentials are invalid. + */ + INVALID_CREDENTIALS = "INVALID_CREDENTIALS", + + /** + * TOXIC_COMMENT is returned when a comment is detected as Toxic and nudging + * is enabled. + */ + TOXIC_COMMENT = "TOXIC_COMMENT", + + /** + * SPAM_COMMENT is returned when a comment is detected as spam and nudging is + * enabled. + */ + SPAM_COMMENT = "SPAM_COMMENT", } diff --git a/src/core/server/app/middleware/passport/index.ts b/src/core/server/app/middleware/passport/index.ts index 7ab75c11e..5d863d347 100644 --- a/src/core/server/app/middleware/passport/index.ts +++ b/src/core/server/app/middleware/passport/index.ts @@ -3,7 +3,6 @@ import { Redis } from "ioredis"; import Joi from "joi"; import jwt from "jsonwebtoken"; import passport, { Authenticator } from "passport"; -import now from "performance-now"; import { AppOptions } from "talk-server/app"; import FacebookStrategy from "talk-server/app/middleware/passport/strategies/facebook"; @@ -12,7 +11,7 @@ import { JWTStrategy } from "talk-server/app/middleware/passport/strategies/jwt" import { createLocalStrategy } from "talk-server/app/middleware/passport/strategies/local"; import OIDCStrategy from "talk-server/app/middleware/passport/strategies/oidc"; import { validate } from "talk-server/app/request/body"; -import logger from "talk-server/logger"; +import { AuthenticationError } from "talk-server/errors"; import { User } from "talk-server/models/user"; import { extractJWTFromRequest, @@ -207,31 +206,22 @@ export const wrapAuthn = ( signingConfig: JWTSigningConfig, name: string, options?: any -): RequestHandler => (req: Request, res, next) => { - const startTime = now(); - +): RequestHandler => (req: Request, res, next) => authenticator.authenticate( name, { ...options, session: false }, (err: Error | null, user: User | null) => { - // Compute the end time. - const responseTime = Math.round(now() - startTime); - - logger.debug({ responseTime }, "user token generated"); - if (err) { return next(err); } if (!user) { - // TODO: (wyattjoh) replace with better error. - return next(new Error("no user on request")); + return next(new AuthenticationError("user not on request")); } // Pass the login off to be signed. handleSuccessfulLogin(user, signingConfig, req, res, next); } )(req, res, next); -}; /** * authenticate will wrap the authenticator to forward any error to the error diff --git a/src/core/server/app/middleware/passport/strategies/local.ts b/src/core/server/app/middleware/passport/strategies/local.ts index 3b1cc88c1..2a0b90944 100644 --- a/src/core/server/app/middleware/passport/strategies/local.ts +++ b/src/core/server/app/middleware/passport/strategies/local.ts @@ -2,6 +2,7 @@ import { Db } from "mongodb"; import { Strategy as LocalStrategy } from "passport-local"; import { VerifyCallback } from "talk-server/app/middleware/passport"; +import { InvalidCredentialsError } from "talk-server/errors"; import { retrieveUserWithProfile, verifyUserPassword, @@ -27,14 +28,13 @@ const verifyFactory = (mongo: Db) => async ( }); if (!user) { // The user didn't exist. - return done(null, null); + return done(new InvalidCredentialsError("user not found")); } // Verify the password. const passwordVerified = await verifyUserPassword(user, password); if (!passwordVerified) { - // TODO: return better error - return done(new Error("invalid password")); + return done(new InvalidCredentialsError("invalid password")); } return done(null, user); diff --git a/src/core/server/errors/index.ts b/src/core/server/errors/index.ts index 1c58eb8a9..e6d295ffb 100644 --- a/src/core/server/errors/index.ts +++ b/src/core/server/errors/index.ts @@ -411,6 +411,16 @@ export class TenantInstalledAlreadyError extends TalkError { } } +export class InvalidCredentialsError extends TalkError { + constructor(reason: string) { + super({ + code: ERROR_CODES.INVALID_CREDENTIALS, + status: 401, + context: { pvt: { reason } }, + }); + } +} + export class AuthenticationError extends TalkError { constructor(reason: string) { super({ @@ -420,3 +430,24 @@ export class AuthenticationError extends TalkError { }); } } + +export class ToxicCommentError extends TalkError { + constructor(model: string, score: number, threshold: number) { + super({ + code: ERROR_CODES.TOXIC_COMMENT, + type: ERROR_TYPES.MODERATION_NUDGE_ERROR, + status: 400, + context: { pvt: { model, score, threshold } }, + }); + } +} + +export class SpamCommentError extends TalkError { + constructor() { + super({ + code: ERROR_CODES.SPAM_COMMENT, + type: ERROR_TYPES.MODERATION_NUDGE_ERROR, + status: 400, + }); + } +} diff --git a/src/core/server/errors/translations.ts b/src/core/server/errors/translations.ts index 29c4b7443..956d32699 100644 --- a/src/core/server/errors/translations.ts +++ b/src/core/server/errors/translations.ts @@ -32,4 +32,7 @@ export const ERROR_TRANSLATIONS: Record = { USERNAME_EXCEEDS_MAX_LENGTH: "error-usernameExceedsMaxLength", USERNAME_TOO_SHORT: "error-usernameTooShort", AUTHENTICATION_ERROR: "error-authenticationError", + INVALID_CREDENTIALS: "error-invalidCredentials", + TOXIC_COMMENT: "error-toxicCommentError", + SPAM_COMMENT: "error-spamCommentError", }; diff --git a/src/core/server/graph/tenant/mutators/Comments.ts b/src/core/server/graph/tenant/mutators/Comments.ts index be2bb0b21..b8fe9beb7 100644 --- a/src/core/server/graph/tenant/mutators/Comments.ts +++ b/src/core/server/graph/tenant/mutators/Comments.ts @@ -26,6 +26,7 @@ import { validateMaximumLength } from "./util"; export const Comments = (ctx: TenantContext) => ({ create: ({ clientMutationId, + nudge = false, ...comment }: GQLCreateCommentInput | GQLCreateCommentReplyInput) => mapFieldsetToErrorCodes( @@ -35,6 +36,7 @@ export const Comments = (ctx: TenantContext) => ({ ctx.tenant, ctx.user!, { authorID: ctx.user!.id, ...comment }, + nudge, ctx.req ), { diff --git a/src/core/server/graph/tenant/schema/schema.graphql b/src/core/server/graph/tenant/schema/schema.graphql index 52160b824..92e5496ac 100644 --- a/src/core/server/graph/tenant/schema/schema.graphql +++ b/src/core/server/graph/tenant/schema/schema.graphql @@ -1834,6 +1834,13 @@ type Query { CreateCommentInput provides the input for the createComment Mutation. """ input CreateCommentInput { + """ + nudge when true will instead return an error related to recoverable moderation + faults such as a toxic comment or spam comment to provide user feedback to + nudge the user to correct the comment. + """ + nudge: Boolean = false + """ storyID is the ID of the Story where we are creating a comment on. """ @@ -1874,6 +1881,13 @@ type CreateCommentPayload { CreateCommentReplyInput provides the input for the createCommentReply Mutation. """ input CreateCommentReplyInput { + """ + nudge when true will instead return an error related to recoverable moderation + faults such as a toxic comment or spam comment to provide user feedback to + nudge the user to correct the comment. + """ + nudge: Boolean = false + """ storyID is the ID of the Story where we are creating a comment on. """ @@ -3465,7 +3479,7 @@ type Mutation { updateSettings will update the Settings for the given Tenant. """ updateSettings(input: UpdateSettingsInput!): UpdateSettingsPayload! - @auth(roles: [ADMIN, MODERATOR]) + @auth(roles: [ADMIN]) """ regenerateSSOKey will regenerate the SSO key used to sign secrets. This will diff --git a/src/core/server/locales/en-US/errors.ftl b/src/core/server/locales/en-US/errors.ftl index 955f12114..7dfd0f16a 100644 --- a/src/core/server/locales/en-US/errors.ftl +++ b/src/core/server/locales/en-US/errors.ftl @@ -38,4 +38,7 @@ error-internalError = Internal Error error-tenantInstalledAlready = Tenant has already been installed already. error-userNotEntitled = You are not authorized to access that resource. error-storyNotFound = Story ({$storyID}) not found. -error-commentNotFound = Comment ({$commentID}) not found. \ No newline at end of file +error-commentNotFound = Comment ({$commentID}) not found. +error-invalidCredentials = Email and/or password combination incorrect. +error-toxicCommentError = Are you sure? The language in this comment might violate our community guidelines. You can edit the comment or submit it for moderator review. +error-spamCommentError = The language in this comment looks like spam. You can edit the comment or submit it anyway for moderator review. diff --git a/src/core/server/models/comment.ts b/src/core/server/models/comment.ts index 18b00a16d..2a43dae3d 100644 --- a/src/core/server/models/comment.ts +++ b/src/core/server/models/comment.ts @@ -198,6 +198,7 @@ export type CreateCommentInput = Omit< | "replyCount" | "actionCounts" | "revisions" + | "deletedAt" > & Required> & Partial>; diff --git a/src/core/server/services/comments/index.ts b/src/core/server/services/comments/index.ts index 317e6a865..e863857ab 100644 --- a/src/core/server/services/comments/index.ts +++ b/src/core/server/services/comments/index.ts @@ -26,10 +26,12 @@ import { Tenant } from "talk-server/models/tenant"; import { User } from "talk-server/models/user"; import { Request } from "talk-server/types/express"; +import { ERROR_TYPES } from "talk-common/errors"; +import { TalkError } from "talk-server/errors"; import { AugmentedRedis } from "../redis"; import { addCommentActions, CreateAction } from "./actions"; import { calculateCounts, calculateCountsDiff } from "./moderation/counts"; -import { processForModeration } from "./pipeline"; +import { PhaseResult, processForModeration } from "./pipeline"; export type CreateComment = Omit< CreateCommentInput, @@ -42,6 +44,7 @@ export async function create( tenant: Tenant, author: User, input: CreateComment, + nudge: boolean, req?: Request ) { let log = logger.child({ @@ -49,6 +52,7 @@ export async function create( tenantID: tenant.id, storyID: input.storyID, parentID: input.parentID, + nudge, }); // TODO: (wyattjoh) perform rate limiting based on the user? @@ -86,14 +90,30 @@ export async function create( ); } - // Run the comment through the moderation phases. - const { actions, body, status, metadata } = await processForModeration({ - story, - tenant, - comment: input, - author, - req, - }); + let result: PhaseResult; + + try { + // Run the comment through the moderation phases. + result = await processForModeration({ + nudge, + story, + tenant, + comment: input, + author, + req, + }); + } catch (err) { + if ( + err instanceof TalkError && + err.type === ERROR_TYPES.MODERATION_NUDGE_ERROR + ) { + log.info({ err }, "detected pipeline nudge"); + } + + throw err; + } + + const { actions, body, status, metadata } = result; // This is the first time this comment is being published.. So we need to // ensure we don't run into any race conditions when we create the comment. diff --git a/src/core/server/services/comments/pipeline/index.ts b/src/core/server/services/comments/pipeline/index.ts index 4857cb1bb..0176cb3d0 100644 --- a/src/core/server/services/comments/pipeline/index.ts +++ b/src/core/server/services/comments/pipeline/index.ts @@ -26,6 +26,7 @@ export interface ModerationPhaseContext { tenant: Tenant; comment: RequireProperty, "body">; author: User; + nudge?: boolean; req?: Request; } diff --git a/src/core/server/services/comments/pipeline/phases/spam.ts b/src/core/server/services/comments/pipeline/phases/spam.ts index a27835b41..ed503bba4 100644 --- a/src/core/server/services/comments/pipeline/phases/spam.ts +++ b/src/core/server/services/comments/pipeline/phases/spam.ts @@ -1,5 +1,6 @@ import { Client } from "akismet-api"; +import { SpamCommentError } from "talk-server/errors"; import { GQLCOMMENT_FLAG_REASON, GQLCOMMENT_STATUS, @@ -17,6 +18,7 @@ export const spam: IntermediateModerationPhase = async ({ comment, author, req, + nudge, }): Promise => { const integration = tenant.integrations.akismet; @@ -96,6 +98,12 @@ export const spam: IntermediateModerationPhase = async ({ }); if (isSpam) { log.trace({ isSpam }, "comment contained spam"); + + // Throw an error if we're nudging instead of recording. + if (nudge) { + throw new SpamCommentError(); + } + return { status: GQLCOMMENT_STATUS.SYSTEM_WITHHELD, actions: [ @@ -114,6 +122,11 @@ export const spam: IntermediateModerationPhase = async ({ log.trace({ isSpam }, "comment did not contain spam"); } catch (err) { + // Rethrow any SpamCommentError. + if (err instanceof SpamCommentError) { + throw err; + } + log.error({ err }, "could not determine if comment contained spam"); } }; diff --git a/src/core/server/services/comments/pipeline/phases/toxic.ts b/src/core/server/services/comments/pipeline/phases/toxic.ts index 0261ed699..487a90252 100644 --- a/src/core/server/services/comments/pipeline/phases/toxic.ts +++ b/src/core/server/services/comments/pipeline/phases/toxic.ts @@ -3,6 +3,7 @@ import ms from "ms"; import fetch from "node-fetch"; import { Omit } from "talk-common/types"; +import { ToxicCommentError } from "talk-server/errors"; import { GQLCOMMENT_FLAG_REASON, GQLCOMMENT_STATUS, @@ -18,6 +19,7 @@ import { export const toxic: IntermediateModerationPhase = async ({ tenant, comment, + nudge, }): Promise => { if (!comment.body) { return; @@ -74,14 +76,18 @@ export const toxic: IntermediateModerationPhase = async ({ } // TODO: (wyattjoh) replace hardcoded default with config. - const timeout = ms("300ms"); + const timeout = ms("500ms"); try { logger.trace("checking comment toxicity"); + // TODO: (wyattjoh) support custom toxicity model. + const model = "TOXICITY"; + // Call into the Toxic comment API. - const scores = await getScores( + const score = await getScore( comment.body, + model, { endpoint, key: integration.key, @@ -90,10 +96,15 @@ export const toxic: IntermediateModerationPhase = async ({ timeout ); - const score = scores.SEVERE_TOXICITY.summaryScore; const isToxic = score > threshold; if (isToxic) { - log.trace({ score, isToxic, threshold }, "comment was toxic"); + log.trace({ score, isToxic, threshold, model }, "comment was toxic"); + + // Throw an error if we're nudging instead of recording. + if (nudge) { + throw new ToxicCommentError(model, score, threshold); + } + return { status: GQLCOMMENT_STATUS.SYSTEM_WITHHELD, actions: [ @@ -105,33 +116,40 @@ export const toxic: IntermediateModerationPhase = async ({ ], metadata: { // Store the scores from perspective in the Comment metadata. - perspective: scores, + perspective: { model, score }, }, }; } log.trace({ score, isToxic, threshold }, "comment was not toxic"); } catch (err) { + // Rethrow any ToxicCommentError. + if (err instanceof ToxicCommentError) { + throw err; + } + log.error({ err }, "could not determine comment toxicity"); } }; /** - * getScores will return the toxicity scores for the comment text. + * getScore will return the toxicity score for the comment text. * * @param text comment text to check for toxicity + * @param model the specific model to use when storing the toxicity * @param settings integration settings used to communicate with the perspective api * @param timeout timeout for communicating with the perspective api */ -async function getScores( +async function getScore( text: string, + model: string, { key, endpoint, doNotStore, }: Required>, timeout: number -) { +): Promise { try { const response = await fetch(`${endpoint}/comments:analyze?key=${key}`, { method: "POST", @@ -147,8 +165,7 @@ async function getScores( languages: ["en"], doNotStore, requestedAttributes: { - TOXICITY: {}, - SEVERE_TOXICITY: {}, + [model]: {}, }, }), }); @@ -157,14 +174,7 @@ async function getScores( const data = await response.json(); // Reformat the scores. - return { - TOXICITY: { - summaryScore: data.attributeScores.TOXICITY.summaryScore.value, - }, - SEVERE_TOXICITY: { - summaryScore: data.attributeScores.SEVERE_TOXICITY.summaryScore.value, - }, - }; + return data.attributeScores[model].summaryScore.value as number; } catch (err) { // Ensure that the API key doesn't get leaked to the logs by accident. if (err.message) {