diff --git a/src/core/client/admin/components/BanModal.tsx b/src/core/client/admin/components/BanModal.tsx index b603a81c17..0af8569092 100644 --- a/src/core/client/admin/components/BanModal.tsx +++ b/src/core/client/admin/components/BanModal.tsx @@ -1,4 +1,5 @@ import { Localized } from "@fluent/react/compat"; +import { FORM_ERROR } from "final-form"; import React, { FunctionComponent, useCallback, @@ -221,27 +222,39 @@ const BanModal: FunctionComponent = ({ const onFormSubmit = useCallback(async () => { switch (updateType) { case UpdateType.ALL_SITES: - await banUser({ - userID, // Should be defined because the modal shouldn't open if author is null - message: customizeMessage ? emailMessage : getDefaultMessage, - rejectExistingComments, - siteIDs: viewerIsScoped - ? viewer?.moderationScopes?.sites?.map(({ id }) => id) - : [], - }); + try { + await banUser({ + userID, // Should be defined because the modal shouldn't open if author is null + message: customizeMessage ? emailMessage : getDefaultMessage, + rejectExistingComments, + siteIDs: viewerIsScoped + ? viewer?.moderationScopes?.sites?.map(({ id }) => id) + : [], + }); + } catch (err) { + return { [FORM_ERROR]: err.message }; + } break; case UpdateType.SPECIFIC_SITES: - await updateUserBan({ - userID, - message: customizeMessage ? emailMessage : getDefaultMessage, - banSiteIDs, - unbanSiteIDs, - }); + try { + await updateUserBan({ + userID, + message: customizeMessage ? emailMessage : getDefaultMessage, + banSiteIDs, + unbanSiteIDs, + }); + } catch (err) { + return { [FORM_ERROR]: err.message }; + } break; case UpdateType.NO_SITES: - await removeUserBan({ - userID, - }); + try { + await removeUserBan({ + userID, + }); + } catch (err) { + return { [FORM_ERROR]: err.message }; + } } if (banDomain) { void createDomainBan({ diff --git a/src/core/client/stream/tabs/Comments/Comment/UserBanPopover/UserBanPopoverContainer.tsx b/src/core/client/stream/tabs/Comments/Comment/UserBanPopover/UserBanPopoverContainer.tsx index b2e3f960e9..7e1c5f36a3 100644 --- a/src/core/client/stream/tabs/Comments/Comment/UserBanPopover/UserBanPopoverContainer.tsx +++ b/src/core/client/stream/tabs/Comments/Comment/UserBanPopover/UserBanPopoverContainer.tsx @@ -124,13 +124,8 @@ const UserBanPopoverContainer: FunctionComponent = ({ }); } } catch (e) { - if (e.message === "CANNOT_BAN_ACCOUNT_WITH_MOD_PRIVILEGES") { - const errorMessage = getMessage( - localeBundles, - "comments-userBanPopover-moderator-ban-error", - "Cannot ban accounts with moderator privileges" - ); - setBanError(errorMessage); + if (e.message) { + setBanError(e.message); } } if (siteBan) { diff --git a/src/core/client/stream/test/comments/stream/moderation.spec.tsx b/src/core/client/stream/test/comments/stream/moderation.spec.tsx index 2d1bd7f6d7..1a2617c671 100644 --- a/src/core/client/stream/test/comments/stream/moderation.spec.tsx +++ b/src/core/client/stream/test/comments/stream/moderation.spec.tsx @@ -579,7 +579,7 @@ it("site moderator cannot ban another moderator with site privileges", async () siteIDs: ["site-id"], }); throw new InvalidRequestError({ - code: ERROR_CODES.CANNOT_BAN_ACCOUNT_WITH_MOD_PRIVILEGES, + code: ERROR_CODES.MODERATOR_CANNOT_BE_BANNED_ON_SITE, param: "input.body", traceID: "traceID", }); diff --git a/src/core/common/errors.ts b/src/core/common/errors.ts index abc69f8de9..e30ad009f8 100644 --- a/src/core/common/errors.ts +++ b/src/core/common/errors.ts @@ -267,12 +267,6 @@ export enum ERROR_CODES { */ USER_ALREADY_BANNED = "USER_ALREADY_BANNED", - /** - * CANNOT_BAN_ACCOUNT_WITH_MOD_PRIVILEGES is returned when a moderator attempts - * to ban another moderator with site privileges. - */ - CANNOT_BAN_ACCOUNT_WITH_MOD_PRIVILEGES = "CANNOT_BAN_ACCOUNT_WITH_MOD_PRIVILEGES", - /** * USER_SUSPENDED is returned when the user attempts to perform an action that * is not permitted if they are suspended. diff --git a/src/core/server/errors/index.ts b/src/core/server/errors/index.ts index d2236c4aab..d00aea0563 100644 --- a/src/core/server/errors/index.ts +++ b/src/core/server/errors/index.ts @@ -783,14 +783,6 @@ export class UserAlreadyBannedError extends CoralError { } } -export class CannotBanAccountWithModPrivilegesError extends CoralError { - constructor() { - super({ - code: ERROR_CODES.CANNOT_BAN_ACCOUNT_WITH_MOD_PRIVILEGES, - }); - } -} - export class UserBanned extends CoralError { constructor(userID: string, resource?: string, operation?: string) { super({ diff --git a/src/core/server/errors/translations.ts b/src/core/server/errors/translations.ts index 73a2711f89..0285afacad 100644 --- a/src/core/server/errors/translations.ts +++ b/src/core/server/errors/translations.ts @@ -47,9 +47,7 @@ export const ERROR_TRANSLATIONS: Record = { USER_ALREADY_BANNED: "error-userAlreadyBanned", USER_BANNED: "error-userBanned", USER_SITE_BANNED: "error-userSiteBanned", - MODERATOR_CANNOT_BE_BANNED_ON_SITE: "error-moderatorCannotBeBannedOnSite", - CANNOT_BAN_ACCOUNT_WITH_MOD_PRIVILEGES: - "error-cannotBanAccountWithModPrivileges", + MODERATOR_CANNOT_BE_BANNED_ON_SITE: "error-cannotBanAccountWithModPrivileges", USER_SUSPENDED: "error-userSuspended", USER_WARNED: "error-userWarned", INTEGRATION_DISABLED: "error-integrationDisabled", diff --git a/src/core/server/locales/en-US/errors.ftl b/src/core/server/locales/en-US/errors.ftl index 6dfaa3e45a..5ec0fff6dc 100644 --- a/src/core/server/locales/en-US/errors.ftl +++ b/src/core/server/locales/en-US/errors.ftl @@ -16,6 +16,7 @@ error-tokenNotFound = Specified token does not exist. error-emailAlreadySet = Email address has already been set. error-emailNotSet = Email address has not been set yet. error-emailDomainProtected = Email domain cannot be moderated. +error-cannotBanAccountWithModPrivileges = Cannot ban accounts with moderator privileges error-duplicateUser = Specified user already exists with a different login method. error-duplicateEmail = Specified email address is already in use. diff --git a/src/core/server/models/user/user.ts b/src/core/server/models/user/user.ts index 4cceca1f05..daaed68079 100644 --- a/src/core/server/models/user/user.ts +++ b/src/core/server/models/user/user.ts @@ -16,7 +16,6 @@ import { PasswordResetTokenExpired, SSOProfileNotSetError, TokenNotFoundError, - UserAlreadyBannedError, UserAlreadyPremoderated, UserAlreadySuspendedError, UsernameAlreadySetError, @@ -1992,12 +1991,6 @@ export async function siteBanUser( throw new UserNotFoundError(id); } - // Check to see if the user is already banned. - const ban = consolidateUserBanStatus(user.status.ban); - if (ban.active) { - throw new UserAlreadyBannedError(); - } - throw new Error("an unexpected error occurred"); } @@ -2037,9 +2030,6 @@ export async function banUser( { id, tenantID, - "status.ban.active": { - $ne: true, - }, }, { $set: { @@ -2063,12 +2053,6 @@ export async function banUser( throw new UserNotFoundError(id); } - // Check to see if the user is already banned. - const ban = consolidateUserBanStatus(user.status.ban); - if (ban.active) { - throw new UserAlreadyBannedError(); - } - throw new Error("an unexpected error occurred"); } diff --git a/src/core/server/services/users/users.ts b/src/core/server/services/users/users.ts index 2de82b2413..98566db6e7 100644 --- a/src/core/server/services/users/users.ts +++ b/src/core/server/services/users/users.ts @@ -18,7 +18,6 @@ import { Config } from "coral-server/config"; import { DataCache } from "coral-server/data/cache/dataCache"; import { MongoContext } from "coral-server/data/context"; import { - CannotBanAccountWithModPrivilegesError, DuplicateEmailError, DuplicateUserError, EmailAlreadySetError, @@ -29,7 +28,6 @@ import { ModeratorCannotBeBannedOnSiteError, PasswordIncorrect, TokenNotFoundError, - // UserAlreadyBannedError, UserAlreadyPremoderated, UserAlreadySuspendedError, UserBioTooLongError, @@ -1405,6 +1403,30 @@ export async function ban( banner.moderationScopes.siteIDs && banner.moderationScopes.siteIDs.length !== 0; + // used to determine whether to send another ban email notification or not + let alreadyBanned = false; + + // Check to see if the User is currently banned. + const banStatus = consolidateUserBanStatus(targetUser.status.ban); + if (banStatus.active) { + alreadyBanned = true; + } + + // check if user is already banned on all of the siteIDs + if ( + targetUser.status.ban.siteIDs && + targetUser.status.ban.siteIDs.length > 0 + ) { + const siteIDsAlreadyBanned = targetUser.status.ban.siteIDs?.filter( + (siteID) => { + return siteIDs?.includes(siteID); + } + ); + if (siteIDsAlreadyBanned.length === siteIDs?.length) { + alreadyBanned = true; + } + } + if (bannerIsSiteMod) { // ensure they've provided at least one site ID if (!siteIDs || siteIDs.length === 0) { @@ -1412,21 +1434,29 @@ export async function ban( "site moderators must provide at least one site ID to ban" ); } + // make sure org moderators aren't site banned + if ( + targetUser.role === GQLUSER_ROLE.MODERATOR && + !targetUser.moderationScopes?.scoped + ) { + throw new ModeratorCannotBeBannedOnSiteError(); + } + + // a site moderator cannot ban another site moderator with privileges for one of + // the site ids they are trying to ban against. if ( targetUser.role === GQLUSER_ROLE.MODERATOR && targetUser.moderationScopes && targetUser.moderationScopes.siteIDs && targetUser.moderationScopes.siteIDs.length !== 0 ) { - // a site moderator cannot ban another site moderator with privilegs for one of - // the site ids they are trying to ban against. const siteIDInModScopes = targetUser.moderationScopes.siteIDs.some( (site) => { return siteIDs.includes(site); } ); if (siteIDInModScopes) { - throw new CannotBanAccountWithModPrivilegesError(); + throw new ModeratorCannotBeBannedOnSiteError(); } } } @@ -1459,20 +1489,10 @@ export async function ban( } // Otherwise, perform a regular ban else { - // Check to see if the User is currently banned. - const banStatus = consolidateUserBanStatus(targetUser.status.ban); - if (banStatus.active) { - if (rejectExistingComments) { - await rejector.add({ - tenantID: tenant.id, - authorID: userID, - moderatorID: banner.id, - }); - } - return targetUser; - // throw new UserAlreadyBannedError(); + // moderators can't be generally banned + if (targetUser.role === GQLUSER_ROLE.MODERATOR) { + throw new ModeratorCannotBeBannedOnSiteError(); } - // Ban the user. user = await banUser(mongo, tenant.id, userID, banner.id, message, now); @@ -1521,27 +1541,29 @@ export async function ban( } } - // If the user has an email address associated with their account, send them - // a ban notification email. - if (user?.email) { - // Send the ban user email. - await mailer.add({ - tenantID: tenant.id, - message: { - to: user.email, - }, - template: { - name: "account-notification/ban", - context: { - // TODO: (wyattjoh) possibly reevaluate the use of a required username. - username: user.username!, - organizationName: tenant.organization.name, - organizationURL: tenant.organization.url, - organizationContactEmail: tenant.organization.contactEmail, - customMessage: (message || "").replace(/\n/g, "
"), + if (!alreadyBanned) { + // If the user has an email address associated with their account, send them + // a ban notification email. + if (user?.email) { + // Send the ban user email. + await mailer.add({ + tenantID: tenant.id, + message: { + to: user.email, }, - }, - }); + template: { + name: "account-notification/ban", + context: { + // TODO: (wyattjoh) possibly reevaluate the use of a required username. + username: user.username!, + organizationName: tenant.organization.name, + organizationURL: tenant.organization.url, + organizationContactEmail: tenant.organization.contactEmail, + customMessage: (message || "").replace(/\n/g, "
"), + }, + }, + }); + } } return user;