From 662f5ce314420deebdce26d58705dde49cbf905d Mon Sep 17 00:00:00 2001 From: Nick Funk Date: Thu, 13 Jun 2019 11:24:50 -0600 Subject: [PATCH] [CORL-409] Prevent users from ignoring staff members (#2355) * Throw error if user tries to ignore a staff member Throws a UserCannotBeIgnoredError if a user tries to ignore a user who is a staff member. A staff member in this case is considered anyone who has a role of staff, moderator, or admin. CORL-409 * Prevent users from ignoring staff in the user info popover Creates the staff roles in a constant next to the user model. Uses this to add a computed property to the user resolver. CORL-409 * Remove unnecessary async declaration from userIsStaff helper function CORL-409 * Specify ignoreable on users in client test fixtures Allows the tests to pass for the required computed property of ignoreable that is computed by whether a user is a staff member or not. CORL-409 * Update more fixtures with ignoreable property on mocked users/commenters CORL-409 * Consolidate ignore-able calculation into re-usable helper methods Re-use the logic for whether a role is a staff member to clearly define when a user is ignore-able or not across the business logic. CORL-409 * Set the ignoreable optimisticResponse on comment mutations We have set the ignoreable value in the graphQL schema, so now the optimisticResponses are looking for a default value to use until the data result arrives. Put to false since the ignoreable value is set on our author, we likely don't want to ignore ourselves. CORL-409 --- src/core/client/admin/test/fixtures.ts | 7 +++++++ .../CreateCommentReplyMutation.ts | 1 + .../UserPopover/UserPopoverOverviewContainer.tsx | 4 +++- .../PostCommentForm/CreateCommentMutation.ts | 1 + src/core/client/stream/test/fixtures.ts | 5 +++++ src/core/common/errors.ts | 7 +++++++ src/core/server/errors/index.ts | 9 +++++++++ src/core/server/errors/translations.ts | 1 + src/core/server/graph/tenant/resolvers/User.ts | 2 ++ .../server/graph/tenant/schema/schema.graphql | 7 +++++++ src/core/server/models/user/constants.ts | 7 +++++++ src/core/server/models/user/helpers.ts | 16 ++++++++++++++++ .../server/models/{user.ts => user/index.ts} | 10 +++++----- src/core/server/services/users/index.ts | 7 +++++++ 14 files changed, 78 insertions(+), 6 deletions(-) create mode 100644 src/core/server/models/user/constants.ts create mode 100644 src/core/server/models/user/helpers.ts rename src/core/server/models/{user.ts => user/index.ts} (99%) diff --git a/src/core/client/admin/test/fixtures.ts b/src/core/client/admin/test/fixtures.ts index 1094aaa80..8ca62719d 100644 --- a/src/core/client/admin/test/fixtures.ts +++ b/src/core/client/admin/test/fixtures.ts @@ -289,6 +289,7 @@ export const users = { username: "Markus", email: "markus@test.com", role: GQLUSER_ROLE.ADMIN, + ignoreable: false, }, ], baseUser @@ -300,6 +301,7 @@ export const users = { username: "Lukas", email: "lukas@test.com", role: GQLUSER_ROLE.MODERATOR, + ignoreable: false, }, ], baseUser @@ -311,6 +313,7 @@ export const users = { username: "Huy", email: "huy@test.com", role: GQLUSER_ROLE.STAFF, + ignoreable: false, }, ], baseUser @@ -322,18 +325,21 @@ export const users = { username: "Isabelle", email: "isabelle@test.com", role: GQLUSER_ROLE.COMMENTER, + ignoreable: true, }, { id: "user-commenter-1", username: "Ngoc", email: "ngoc@test.com", role: GQLUSER_ROLE.COMMENTER, + ignoreable: true, }, { id: "user-commenter-2", username: "Max", email: "max@test.com", role: GQLUSER_ROLE.COMMENTER, + ignoreable: true, }, ], baseUser @@ -344,6 +350,7 @@ export const users = { username: "Ingrid", email: "ingrid@test.com", role: GQLUSER_ROLE.COMMENTER, + ignoreable: true, status: { current: [GQLUSER_STATUS.BANNED], ban: { active: true }, diff --git a/src/core/client/stream/tabs/Comments/Comment/ReplyCommentForm/CreateCommentReplyMutation.ts b/src/core/client/stream/tabs/Comments/Comment/ReplyCommentForm/CreateCommentReplyMutation.ts index 094bd340e..58c667895 100644 --- a/src/core/client/stream/tabs/Comments/Comment/ReplyCommentForm/CreateCommentReplyMutation.ts +++ b/src/core/client/stream/tabs/Comments/Comment/ReplyCommentForm/CreateCommentReplyMutation.ts @@ -178,6 +178,7 @@ function commit( id: viewer.id, username: viewer.username, createdAt: viewer.createdAt, + ignoreable: false, }, body: input.body, revision: { diff --git a/src/core/client/stream/tabs/Comments/Comment/UserPopover/UserPopoverOverviewContainer.tsx b/src/core/client/stream/tabs/Comments/Comment/UserPopover/UserPopoverOverviewContainer.tsx index b0099ac12..9b063e0b0 100644 --- a/src/core/client/stream/tabs/Comments/Comment/UserPopover/UserPopoverOverviewContainer.tsx +++ b/src/core/client/stream/tabs/Comments/Comment/UserPopover/UserPopoverOverviewContainer.tsx @@ -30,7 +30,8 @@ export const UserPopoverOverviewContainer: FunctionComponent = ({ const canIgnore = viewer && viewer.id !== user.id && - viewer.ignoredUsers.every(u => u.id !== user.id); + viewer.ignoredUsers.every(u => u.id !== user.id) && + user.ignoreable; return ( @@ -73,6 +74,7 @@ const enhanced = withFragmentContainer({ id username createdAt + ignoreable } `, })(UserPopoverOverviewContainer); diff --git a/src/core/client/stream/tabs/Comments/Stream/PostCommentForm/CreateCommentMutation.ts b/src/core/client/stream/tabs/Comments/Stream/PostCommentForm/CreateCommentMutation.ts index 64f808bf1..a6372d103 100644 --- a/src/core/client/stream/tabs/Comments/Stream/PostCommentForm/CreateCommentMutation.ts +++ b/src/core/client/stream/tabs/Comments/Stream/PostCommentForm/CreateCommentMutation.ts @@ -148,6 +148,7 @@ function commit( id: viewer.id, username: viewer.username, createdAt: viewer.createdAt, + ignoreable: false, }, revision: { id: uuidGenerator(), diff --git a/src/core/client/stream/test/fixtures.ts b/src/core/client/stream/test/fixtures.ts index 487694d87..06f6783b1 100644 --- a/src/core/client/stream/test/fixtures.ts +++ b/src/core/client/stream/test/fixtures.ts @@ -104,21 +104,25 @@ export const commenters = createFixtures( id: "user-0", username: "Markus", role: GQLUSER_ROLE.COMMENTER, + ignoreable: true, }, { id: "user-1", username: "Lukas", role: GQLUSER_ROLE.COMMENTER, + ignoreable: true, }, { id: "user-2", username: "Isabelle", role: GQLUSER_ROLE.COMMENTER, + ignoreable: true, }, { id: "user-3", username: "Markus", role: GQLUSER_ROLE.COMMENTER, + ignoreable: true, }, ], baseUser @@ -354,6 +358,7 @@ export const moderators = createFixtures( id: "me-as-moderator", username: "Moderator", role: GQLUSER_ROLE.MODERATOR, + ignoreable: false, }, ], baseUser diff --git a/src/core/common/errors.ts b/src/core/common/errors.ts index 7b7fc4618..795916152 100644 --- a/src/core/common/errors.ts +++ b/src/core/common/errors.ts @@ -231,6 +231,13 @@ export enum ERROR_CODES { */ USER_BANNED = "USER_BANNED", + /** + * USER_CANNOT_BE_IGNORED is returned when the user attempts to ignore + * a user that is not allowed to be ignored. This is usually because the + * user is staff member. + */ + USER_CANNOT_BE_IGNORED = "USER_CANNOT_BE_IGNORED", + /** * INTEGRATION_DISABLED is returned when an operation is attempted against an * integration that has been disabled. diff --git a/src/core/server/errors/index.ts b/src/core/server/errors/index.ts index 3cd5f9271..d8d684a0a 100644 --- a/src/core/server/errors/index.ts +++ b/src/core/server/errors/index.ts @@ -573,6 +573,15 @@ export class UserSuspended extends CoralError { } } +export class UserCannotBeIgnoredError extends CoralError { + constructor(userID: string) { + super({ + code: ERROR_CODES.USER_CANNOT_BE_IGNORED, + context: { pub: { userID } }, + }); + } +} + export class PasswordResetTokenExpired extends CoralError { constructor(reason: string, cause?: Error) { super({ diff --git a/src/core/server/errors/translations.ts b/src/core/server/errors/translations.ts index 5e033211d..a29730f0b 100644 --- a/src/core/server/errors/translations.ts +++ b/src/core/server/errors/translations.ts @@ -27,6 +27,7 @@ export const ERROR_TRANSLATIONS: Record = { TOKEN_NOT_FOUND: "error-tokenNotFound", USER_NOT_ENTITLED: "error-userNotEntitled", USER_NOT_FOUND: "error-userNotFound", + USER_CANNOT_BE_IGNORED: "error-userCannotBeIgnored", USERNAME_ALREADY_SET: "error-usernameAlreadySet", USERNAME_CONTAINS_INVALID_CHARACTERS: "error-usernameContainsInvalidCharacters", diff --git a/src/core/server/graph/tenant/resolvers/User.ts b/src/core/server/graph/tenant/resolvers/User.ts index a035b6136..7db2cbba2 100644 --- a/src/core/server/graph/tenant/resolvers/User.ts +++ b/src/core/server/graph/tenant/resolvers/User.ts @@ -6,6 +6,7 @@ import { GQLUserTypeResolver, } from "coral-server/graph/tenant/schema/__generated__/types"; import * as user from "coral-server/models/user"; +import { roleIsStaff } from "coral-server/models/user/helpers"; import { UserStatusInput } from "./UserStatus"; import { getRequestedFields } from "./util"; @@ -41,4 +42,5 @@ export const User: GQLUserTypeResolver = { }), ignoredUsers: ({ ignoredUsers }, input, ctx, info) => maybeLoadOnlyIgnoredUserID(ctx, info, ignoredUsers), + ignoreable: ({ role }) => !roleIsStaff(role), }; diff --git a/src/core/server/graph/tenant/schema/schema.graphql b/src/core/server/graph/tenant/schema/schema.graphql index 248a30b53..cd78a1e7e 100644 --- a/src/core/server/graph/tenant/schema/schema.graphql +++ b/src/core/server/graph/tenant/schema/schema.graphql @@ -1456,6 +1456,13 @@ type User { permit: [SUSPENDED, BANNED] ) + """ + ignoreable is a computed property based on the + user's role. Typically, users with elevated privileges + aren't allowed to be ignored. + """ + ignoreable: Boolean! + """ comments are the comments written by the User. """ diff --git a/src/core/server/models/user/constants.ts b/src/core/server/models/user/constants.ts new file mode 100644 index 000000000..2202403cf --- /dev/null +++ b/src/core/server/models/user/constants.ts @@ -0,0 +1,7 @@ +import { GQLUSER_ROLE } from "coral-server/graph/tenant/schema/__generated__/types"; + +export const STAFF_ROLES = [ + GQLUSER_ROLE.ADMIN, + GQLUSER_ROLE.MODERATOR, + GQLUSER_ROLE.STAFF, +]; diff --git a/src/core/server/models/user/helpers.ts b/src/core/server/models/user/helpers.ts new file mode 100644 index 000000000..a3f3df44d --- /dev/null +++ b/src/core/server/models/user/helpers.ts @@ -0,0 +1,16 @@ +import { GQLUSER_ROLE } from "coral-server/graph/tenant/schema/__generated__/types"; +import { STAFF_ROLES } from "coral-server/models/user/constants"; + +import { User } from "."; + +export function roleIsStaff(role: GQLUSER_ROLE) { + if (STAFF_ROLES.includes(role)) { + return true; + } + + return false; +} + +export function userIsStaff(user: User) { + return roleIsStaff(user.role); +} diff --git a/src/core/server/models/user.ts b/src/core/server/models/user/index.ts similarity index 99% rename from src/core/server/models/user.ts rename to src/core/server/models/user/index.ts index e77a35162..d79eec768 100644 --- a/src/core/server/models/user.ts +++ b/src/core/server/models/user/index.ts @@ -25,17 +25,17 @@ import { } from "coral-server/graph/tenant/schema/__generated__/types"; import { getLocalProfile, hasLocalProfile } from "coral-server/helpers/users"; import logger from "coral-server/logger"; +import { + Connection, + ConnectionInput, + resolveConnection, +} from "coral-server/models/helpers/connection"; import { createConnectionOrderVariants, createIndexFactory, } from "coral-server/models/helpers/indexing"; import Query from "coral-server/models/helpers/query"; import { TenantResource } from "coral-server/models/tenant"; -import { - Connection, - ConnectionInput, - resolveConnection, -} from "./helpers/connection"; function collection(mongo: Db) { return mongo.collection>("users"); diff --git a/src/core/server/services/users/index.ts b/src/core/server/services/users/index.ts index ca8a2f72e..bb43ae47f 100644 --- a/src/core/server/services/users/index.ts +++ b/src/core/server/services/users/index.ts @@ -9,6 +9,7 @@ import { TokenNotFoundError, UserAlreadyBannedError, UserAlreadySuspendedError, + UserCannotBeIgnoredError, UsernameAlreadySetError, UserNotFoundError, } from "coral-server/errors"; @@ -39,6 +40,7 @@ import { updateUserUsername, User, } from "coral-server/models/user"; +import { userIsStaff } from "coral-server/models/user/helpers"; import { MailerQueue } from "coral-server/queue/tasks/mailer"; import { JWTSigningConfig, signPATString } from "coral-server/services/jwt"; @@ -585,6 +587,11 @@ export async function ignore( throw new UserNotFoundError(userID); } + const userToBeIgnoredIsStaff = userIsStaff(targetUser); + if (userToBeIgnoredIsStaff) { + throw new UserCannotBeIgnoredError(userID); + } + // TODO: extract function if (user.ignoredUsers && user.ignoredUsers.some(u => u.id === userID)) { // TODO: improve error