From ad2e7a5dfd7c8b1f302f8fa378a04cf817fefba8 Mon Sep 17 00:00:00 2001 From: Jake Naviasky Date: Mon, 26 Aug 2024 15:52:44 -0400 Subject: [PATCH 1/4] Eliminate Bans table. --- libs/model/src/models/address.ts | 5 ++ libs/model/src/models/associations.ts | 1 - libs/model/src/models/ban.ts | 34 --------- libs/model/src/models/factories.ts | 6 +- libs/model/src/models/index.ts | 1 - libs/schemas/src/entities/user.schemas.ts | 1 + .../delete_community.ts | 2 - .../update_community_id.ts | 2 - .../20240826192720-remove-bans-table.js | 70 +++++++++++++++++++ .../commonwealth/server/routes/banAddress.ts | 29 ++++---- .../server/routes/getBannedAddresses.ts | 43 ------------ .../commonwealth/server/routing/router.ts | 9 --- .../commonwealth/server/util/banCheckCache.ts | 4 +- 13 files changed, 95 insertions(+), 112 deletions(-) delete mode 100644 libs/model/src/models/ban.ts create mode 100644 packages/commonwealth/server/migrations/20240826192720-remove-bans-table.js delete mode 100644 packages/commonwealth/server/routes/getBannedAddresses.ts diff --git a/libs/model/src/models/address.ts b/libs/model/src/models/address.ts index 69c6bc18f5d..960349cd405 100644 --- a/libs/model/src/models/address.ts +++ b/libs/model/src/models/address.ts @@ -54,6 +54,11 @@ export default ( wallet_id: { type: Sequelize.STRING, allowNull: true }, wallet_sso_source: { type: Sequelize.STRING, allowNull: true }, block_info: { type: Sequelize.STRING, allowNull: true }, + is_banned: { + type: Sequelize.BOOLEAN, + allowNull: false, + defaultValue: false, + }, hex: { type: Sequelize.STRING(64), allowNull: true, diff --git a/libs/model/src/models/associations.ts b/libs/model/src/models/associations.ts index 6c3e49a2e14..7ab072a3eb8 100644 --- a/libs/model/src/models/associations.ts +++ b/libs/model/src/models/associations.ts @@ -60,7 +60,6 @@ export const buildAssociations = (db: DB) => { }) .withMany(db.Notification) .withMany(db.Webhook) - .withMany(db.Ban) .withMany(db.CommunityBanner) .withMany(db.CommunityTags, { onDelete: 'CASCADE', diff --git a/libs/model/src/models/ban.ts b/libs/model/src/models/ban.ts deleted file mode 100644 index d898ed013ea..00000000000 --- a/libs/model/src/models/ban.ts +++ /dev/null @@ -1,34 +0,0 @@ -import Sequelize from 'sequelize'; -import type { ModelInstance } from './types'; - -export type BanAttributes = { - id?: number; - address: string; - community_id: string; - created_at?: Date; - updated_at?: Date; -}; - -export type BanInstance = ModelInstance; - -export default ( - sequelize: Sequelize.Sequelize, -): Sequelize.ModelStatic => - sequelize.define( - 'Bans', - { - id: { type: Sequelize.INTEGER, primaryKey: true, autoIncrement: true }, - address: { type: Sequelize.STRING, allowNull: false }, - community_id: { type: Sequelize.STRING, allowNull: false }, - created_at: { type: Sequelize.DATE, allowNull: false }, - updated_at: { type: Sequelize.DATE, allowNull: false }, - }, - { - tableName: 'Bans', - underscored: true, - createdAt: 'created_at', - updatedAt: 'updated_at', - timestamps: true, - indexes: [{ fields: ['community_id'] }], - }, - ); diff --git a/libs/model/src/models/factories.ts b/libs/model/src/models/factories.ts index f2ecc3ebd45..7d3298c1217 100644 --- a/libs/model/src/models/factories.ts +++ b/libs/model/src/models/factories.ts @@ -2,7 +2,6 @@ import Sequelize from 'sequelize'; import type { Associable } from './types'; import Address from './address'; -import Ban from './ban'; import ChainNode from './chain_node'; import Collaboration from './collaboration'; import Comment from './comment'; @@ -50,7 +49,6 @@ import Webhook from './webhook'; export const Factories = { Address, - Ban, ChainNode, Collaboration, Comment, @@ -98,8 +96,8 @@ export const Factories = { }; export type DB = { - [K in keyof typeof Factories]: ReturnType & - Associable>; + [K in keyof typeof Factories]: ReturnType<(typeof Factories)[K]> & + Associable>; } & { sequelize: Sequelize.Sequelize; Sequelize: typeof Sequelize.Sequelize; diff --git a/libs/model/src/models/index.ts b/libs/model/src/models/index.ts index 2569a1dfbed..8929daf58d1 100644 --- a/libs/model/src/models/index.ts +++ b/libs/model/src/models/index.ts @@ -44,7 +44,6 @@ export const buildDb = (sequelize: Sequelize): DB => { // TODO: avoid legacy exports to /packages/commonwealth/server (keep db models encapsulated behind DB) export * from './address'; -export * from './ban'; export * from './chain_node'; export * from './collaboration'; export * from './comment'; diff --git a/libs/schemas/src/entities/user.schemas.ts b/libs/schemas/src/entities/user.schemas.ts index 5e050b75875..a03e9f389c7 100644 --- a/libs/schemas/src/entities/user.schemas.ts +++ b/libs/schemas/src/entities/user.schemas.ts @@ -62,6 +62,7 @@ export const Address = z.object({ is_user_default: z.boolean().default(false), role: z.enum(Roles).default('member'), wallet_sso_source: z.nativeEnum(WalletSsoSource).nullish(), + is_banned: z.boolean().default(false), hex: z.string().max(64).nullish(), User: User.optional(), diff --git a/packages/commonwealth/server/controllers/server_communities_methods/delete_community.ts b/packages/commonwealth/server/controllers/server_communities_methods/delete_community.ts index 389a6202e21..e90961c40d1 100644 --- a/packages/commonwealth/server/controllers/server_communities_methods/delete_community.ts +++ b/packages/commonwealth/server/controllers/server_communities_methods/delete_community.ts @@ -125,8 +125,6 @@ export async function __deleteCommunity( // @ts-expect-error StrictNullChecks this.models.DiscordBotConfig, // @ts-expect-error StrictNullChecks - this.models.Ban, - // @ts-expect-error StrictNullChecks this.models.Reaction, // @ts-expect-error StrictNullChecks this.models.Comment, diff --git a/packages/commonwealth/server/controllers/server_communities_methods/update_community_id.ts b/packages/commonwealth/server/controllers/server_communities_methods/update_community_id.ts index 32088ac0a00..6fef685c52e 100644 --- a/packages/commonwealth/server/controllers/server_communities_methods/update_community_id.ts +++ b/packages/commonwealth/server/controllers/server_communities_methods/update_community_id.ts @@ -78,8 +78,6 @@ export async function __updateCommunityId( // @ts-expect-error StrictNullChecks this.models.Address, // @ts-expect-error StrictNullChecks - this.models.Ban, - // @ts-expect-error StrictNullChecks this.models.Comment, // @ts-expect-error StrictNullChecks this.models.CommunityBanner, diff --git a/packages/commonwealth/server/migrations/20240826192720-remove-bans-table.js b/packages/commonwealth/server/migrations/20240826192720-remove-bans-table.js new file mode 100644 index 00000000000..be98a861803 --- /dev/null +++ b/packages/commonwealth/server/migrations/20240826192720-remove-bans-table.js @@ -0,0 +1,70 @@ +'use strict'; + +/** @type {import('sequelize-cli').Migration} */ +module.exports = { + async up(queryInterface, Sequelize) { + return queryInterface.sequelize.transaction(async (transaction) => { + await queryInterface.addColumn( + 'Addresses', + 'is_banned', + Sequelize.BOOLEAN, + { transaction, allowNull: false, defaultValue: false }, + ); + await queryInterface.sequelize.query( + ` + UPDATE "Addresses" AS a + SET is_banned = true + FROM "Bans" b + WHERE a.address = b.address AND a.community_id = b.community_id; + `, + { + transaction, + }, + ); + await queryInterface.dropTable('Bans', { transaction }); + }); + }, + + async down(queryInterface, Sequelize) { + return queryInterface.sequelize.transaction(async (transaction) => { + await queryInterface.createTable( + 'Bans', + { + id: { + type: Sequelize.INTEGER, + autoIncrement: true, + primaryKey: true, + }, + address: { type: Sequelize.STRING, allowNull: false }, + community_id: { + type: Sequelize.STRING, + allowNull: false, + references: { model: 'Communities', key: 'id' }, + }, + created_at: { type: Sequelize.DATE, allowNull: false }, + updated_at: { type: Sequelize.DATE, allowNull: false }, + }, + { + timestamps: true, + underscored: true, + indexes: [{ fields: ['community_id'] }], + transaction, + }, + ); + await queryInterface.sequelize.query( + ` + INSERT INTO "Bans" (address, community_id, created_at, updated_at) + SELECT address, community_id, NOW(), NOW() + FROM "Addresses" + WHERE is_banned = true; + `, + { + transaction, + }, + ); + await queryInterface.removeColumn('Addresses', 'is_banned', { + transaction, + }); + }); + }, +}; diff --git a/packages/commonwealth/server/routes/banAddress.ts b/packages/commonwealth/server/routes/banAddress.ts index 81592bbcf84..afd68f4e5a4 100644 --- a/packages/commonwealth/server/routes/banAddress.ts +++ b/packages/commonwealth/server/routes/banAddress.ts @@ -1,5 +1,5 @@ import { AppError } from '@hicommonwealth/core'; -import type { BanAttributes, BanInstance, DB } from '@hicommonwealth/model'; +import type { DB } from '@hicommonwealth/model'; import type { TypedRequestBody, TypedResponse } from '../types'; import { success } from '../types'; import { validateOwner } from '../util/validateOwner'; @@ -8,13 +8,15 @@ enum BanAddressErrors { NoAddress = 'Must supply an address', NoPermission = 'You do not have permission to ban an address', AlreadyExists = 'Ban for this address already exists', + NotFound = 'Address not found', } -type BanAddressReq = Omit & { +type BanAddressReq = { + community_id: string; address: string; }; -type BanAddressResp = BanAttributes; +type BanAddressResp = {}; const banAddress = async ( models: DB, @@ -40,25 +42,22 @@ const banAddress = async ( throw new AppError(BanAddressErrors.NoAddress); } - // find or create Ban - const [ban, created] = await models.Ban.findOrCreate({ + const addressInstance = await models.Address.findOne({ where: { - // @ts-expect-error StrictNullChecks - community_id: community.id, - address, - }, - defaults: { - // @ts-expect-error StrictNullChecks - community_id: community.id, + community_id: community!.id, address, }, }); - - if (!created) { + if (!addressInstance) { + throw new AppError(BanAddressErrors.NotFound); + } + if (addressInstance.is_banned) { throw new AppError(BanAddressErrors.AlreadyExists); } + addressInstance.is_banned = true; + await addressInstance.save(); - return success(res, ban.toJSON()); + return success(res, {}); }; export default banAddress; diff --git a/packages/commonwealth/server/routes/getBannedAddresses.ts b/packages/commonwealth/server/routes/getBannedAddresses.ts deleted file mode 100644 index 8fe1f5e20f2..00000000000 --- a/packages/commonwealth/server/routes/getBannedAddresses.ts +++ /dev/null @@ -1,43 +0,0 @@ -import { AppError } from '@hicommonwealth/core'; -import type { BanAttributes, DB } from '@hicommonwealth/model'; -import type { TypedRequest, TypedResponse } from '../types'; -import { success } from '../types'; -import { validateOwner } from '../util/validateOwner'; - -enum GetBannedAddressesErrors { - NoPermission = 'You do not have permission to get banned addresses', -} - -type GetBannedAddressesResp = BanAttributes[]; - -const getBannedAddresses = async ( - models: DB, - req: TypedRequest, - res: TypedResponse, -) => { - const { community } = req; - - const isAdmin = await validateOwner({ - models: models, - // @ts-expect-error StrictNullChecks - user: req.user, - // @ts-expect-error StrictNullChecks - communityId: community.id, - allowAdmin: true, - allowSuperAdmin: true, - }); - if (!isAdmin) { - throw new AppError(GetBannedAddressesErrors.NoPermission); - } - - const bans = await models.Ban.findAll({ - // @ts-expect-error StrictNullChecks - where: { community_id: community.id }, - }); - return success( - res, - bans.map((b) => b.toJSON()), - ); -}; - -export default getBannedAddresses; diff --git a/packages/commonwealth/server/routing/router.ts b/packages/commonwealth/server/routing/router.ts index 850c19dc1e5..f084149dbfc 100644 --- a/packages/commonwealth/server/routing/router.ts +++ b/packages/commonwealth/server/routing/router.ts @@ -75,7 +75,6 @@ import type ViewCountCache from '../util/viewCountCache'; import type { DB, GlobalActivityCache } from '@hicommonwealth/model'; import banAddress from '../routes/banAddress'; -import getBannedAddresses from '../routes/getBannedAddresses'; import setAddressWallet from '../routes/setAddressWallet'; import type BanCache from '../util/banCheckCache'; @@ -932,14 +931,6 @@ function setupRouter( databaseValidationService.validateCommunity, banAddress.bind(this, models), ); - registerRoute( - router, - 'get', - '/getBannedAddresses', - passport.authenticate('jwt', { session: false }), - databaseValidationService.validateCommunity, - getBannedAddresses.bind(this, models), - ); // Custom domain update route registerRoute( diff --git a/packages/commonwealth/server/util/banCheckCache.ts b/packages/commonwealth/server/util/banCheckCache.ts index 9f3d5995b27..61ed2bfd73a 100644 --- a/packages/commonwealth/server/util/banCheckCache.ts +++ b/packages/commonwealth/server/util/banCheckCache.ts @@ -46,10 +46,12 @@ export default class BanCache extends JobRunner { return [false, BanErrors.Banned]; } - const ban = await this._models.Ban.findOne({ + const ban = await this._models.Address.findOne({ + attributes: ['id'], // no data needed; only checking existence where: { community_id: communityId, address, + is_banned: true, }, }); From 279c9a14f04247d4886bec85e640c995b73c8eaa Mon Sep 17 00:00:00 2001 From: Jake Naviasky Date: Mon, 26 Aug 2024 15:57:26 -0400 Subject: [PATCH 2/4] Test fix. --- libs/model/src/tester/seedDb.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/libs/model/src/tester/seedDb.ts b/libs/model/src/tester/seedDb.ts index fbc5432ecfe..1e4369bdf0a 100644 --- a/libs/model/src/tester/seedDb.ts +++ b/libs/model/src/tester/seedDb.ts @@ -447,6 +447,7 @@ export const seedDb = async () => { role: 'admin' as Role, is_user_default: false, ghost_address: false, + is_banned: false, })), ); From e7b87b8a1ad6daebd23bca2a0d86b4f69f76d6bf Mon Sep 17 00:00:00 2001 From: Jake Naviasky Date: Mon, 26 Aug 2024 16:11:15 -0400 Subject: [PATCH 3/4] More build fixes. --- .../create_community.ts | 15 ++++++++------- packages/commonwealth/server/passport/magic.ts | 1 + .../commonwealth/server/routes/createAddress.ts | 1 + .../routes/linkExistingAddressToCommunity.ts | 1 + .../server/util/sanitizeDeletedComment.ts | 1 + 5 files changed, 12 insertions(+), 7 deletions(-) diff --git a/packages/commonwealth/server/controllers/server_communities_methods/create_community.ts b/packages/commonwealth/server/controllers/server_communities_methods/create_community.ts index 3489d16f4a8..c092af02507 100644 --- a/packages/commonwealth/server/controllers/server_communities_methods/create_community.ts +++ b/packages/commonwealth/server/controllers/server_communities_methods/create_community.ts @@ -327,13 +327,13 @@ export async function __createCommunity( base === ChainBase.CosmosSDK ? BalanceType.Cosmos : base === ChainBase.Substrate - ? BalanceType.Substrate - : base === ChainBase.Ethereum - ? BalanceType.Ethereum - : // beyond here should never really happen, but just to make sure... - base === ChainBase.Solana - ? BalanceType.Solana - : undefined, + ? BalanceType.Substrate + : base === ChainBase.Ethereum + ? BalanceType.Ethereum + : // beyond here should never really happen, but just to make sure... + base === ChainBase.Solana + ? BalanceType.Solana + : undefined, // use first chain name as node name name: community.name, }, @@ -486,6 +486,7 @@ export async function __createCommunity( role: 'admin', last_active: new Date(), ghost_address: false, + is_banned: false, }); await this.models.Subscription.findOrCreate({ diff --git a/packages/commonwealth/server/passport/magic.ts b/packages/commonwealth/server/passport/magic.ts index 68354e3f599..95e526d0b2e 100644 --- a/packages/commonwealth/server/passport/magic.ts +++ b/packages/commonwealth/server/passport/magic.ts @@ -74,6 +74,7 @@ async function createMagicAddressInstances( role: 'member', is_user_default: false, ghost_address: false, + is_banned: false, }, transaction: t, }); diff --git a/packages/commonwealth/server/routes/createAddress.ts b/packages/commonwealth/server/routes/createAddress.ts index 0d7e18004b8..b8411d0b065 100644 --- a/packages/commonwealth/server/routes/createAddress.ts +++ b/packages/commonwealth/server/routes/createAddress.ts @@ -215,6 +215,7 @@ const createAddress = async ( role: 'member', is_user_default: false, ghost_address: false, + is_banned: false, }, { transaction }, ); diff --git a/packages/commonwealth/server/routes/linkExistingAddressToCommunity.ts b/packages/commonwealth/server/routes/linkExistingAddressToCommunity.ts index 872f207631e..83e29946b37 100644 --- a/packages/commonwealth/server/routes/linkExistingAddressToCommunity.ts +++ b/packages/commonwealth/server/routes/linkExistingAddressToCommunity.ts @@ -170,6 +170,7 @@ const linkExistingAddressToCommunity = async ( role: 'member', is_user_default: false, ghost_address: false, + is_banned: false, }, { transaction }, ); diff --git a/packages/commonwealth/server/util/sanitizeDeletedComment.ts b/packages/commonwealth/server/util/sanitizeDeletedComment.ts index c9e83fb6f86..2a42d32187a 100644 --- a/packages/commonwealth/server/util/sanitizeDeletedComment.ts +++ b/packages/commonwealth/server/util/sanitizeDeletedComment.ts @@ -16,6 +16,7 @@ export function sanitizeDeletedComment( role: 'member', is_user_default: false, ghost_address: false, + is_banned: false, }, address_id: 0, canvas_hash: null, From 820c42dbbf9a32bec7f8c18a261a9abd8beed734 Mon Sep 17 00:00:00 2001 From: Jake Naviasky Date: Mon, 26 Aug 2024 16:17:58 -0400 Subject: [PATCH 4/4] Fix migration to properly set nullability and default value. --- .../server/migrations/20240826192720-remove-bans-table.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/commonwealth/server/migrations/20240826192720-remove-bans-table.js b/packages/commonwealth/server/migrations/20240826192720-remove-bans-table.js index be98a861803..ee8d33a76e5 100644 --- a/packages/commonwealth/server/migrations/20240826192720-remove-bans-table.js +++ b/packages/commonwealth/server/migrations/20240826192720-remove-bans-table.js @@ -7,8 +7,12 @@ module.exports = { await queryInterface.addColumn( 'Addresses', 'is_banned', - Sequelize.BOOLEAN, - { transaction, allowNull: false, defaultValue: false }, + { + type: Sequelize.BOOLEAN, + allowNull: false, + defaultValue: false, + }, + { transaction }, ); await queryInterface.sequelize.query( `