diff --git a/ci.json b/ci.json index c18b0d5..b0d1b49 100644 --- a/ci.json +++ b/ci.json @@ -56,7 +56,6 @@ ] } ], - "hoursAfterWarningExpires": 24, "rateLimit": { "vote": { "windowMs": 900000, diff --git a/config.json.example b/config.json.example index 7adefc2..473a02e 100644 --- a/config.json.example +++ b/config.json.example @@ -25,8 +25,6 @@ "webhooks": [], "categoryList": ["sponsor", "intro", "outro", "interaction", "selfpromo", "preview", "music_offtopic", "poi_highlight"], // List of supported categories any other category will be rejected "getTopUsersCacheTimeMinutes": 5, // cacheTime for getTopUsers result in minutes - "maxNumberOfActiveWarnings": 3, // Users with this number of warnings will be blocked until warnings expire - "hoursAfterWarningExpire": 24, "rateLimit": { "vote": { "windowMs": 900000, // 15 minutes diff --git a/src/config.ts b/src/config.ts index 6a35c07..9ff08fe 100644 --- a/src/config.ts +++ b/src/config.ts @@ -37,8 +37,6 @@ addDefaults(config, { }, deArrowTypes: ["title", "thumbnail"], maxTitleLength: 110, - maxNumberOfActiveWarnings: 1, - hoursAfterWarningExpires: 16300000, adminUserID: "", discordCompletelyIncorrectReportWebhookURL: null, discordFirstTimeSubmissionsWebhookURL: null, diff --git a/src/routes/postSkipSegments.ts b/src/routes/postSkipSegments.ts index 8759b1e..32e5e81 100644 --- a/src/routes/postSkipSegments.ts +++ b/src/routes/postSkipSegments.ts @@ -164,20 +164,15 @@ async function autoModerateSubmission(apiVideoDetails: videoDetails, } async function checkUserActiveWarning(userID: HashedUserID): Promise { - const MILLISECONDS_IN_HOUR = 3600000; - const now = Date.now(); - const warnings = (await db.prepare("all", + const warning = await db.prepare("get", `SELECT "reason" FROM warnings - WHERE "userID" = ? AND "issueTime" > ? AND enabled = 1 AND type = 0 + WHERE "userID" = ? AND enabled = 1 AND type = 0 ORDER BY "issueTime" DESC`, - [ - userID, - Math.floor(now - (config.hoursAfterWarningExpires * MILLISECONDS_IN_HOUR)) - ], - ) as {reason: string}[]).sort((a, b) => (b?.reason?.length ?? 0) - (a?.reason?.length ?? 0)); + [userID], + ) as {reason: string}; - if (warnings?.length >= config.maxNumberOfActiveWarnings) { + if (warning != null) { const defaultMessage = "Submission rejected due to a tip from a moderator. This means that we noticed you were making some common mistakes" + " that are not malicious, and we just want to clarify the rules. " + "Could you please send a message in discord.gg/SponsorBlock or matrix.to/#/#sponsor:ajay.app so we can further help you? " @@ -185,7 +180,7 @@ async function checkUserActiveWarning(userID: HashedUserID): Promise 0 ? `\n\nTip message: '${warnings[0].reason}'` : ""), + errorMessage: defaultMessage + (warning.reason?.length > 0 ? `\n\nTip message: '${warning.reason}'` : ""), errorCode: 403 }; } diff --git a/src/routes/postWarning.ts b/src/routes/postWarning.ts index 159f199..d8169af 100644 --- a/src/routes/postWarning.ts +++ b/src/routes/postWarning.ts @@ -4,7 +4,6 @@ import { db } from "../databases/databases"; import { isUserVIP } from "../utils/isUserVIP"; import { getHashCache } from "../utils/getHashCache"; import { HashedUserID, UserID } from "../types/user.model"; -import { config } from "../config"; import { generateWarningDiscord, warningData, dispatchEvent } from "../utils/webhookUtils"; import { WarningType } from "../types/warning.model"; @@ -16,12 +15,7 @@ type warningEntry = { reason: string } -function checkExpiredWarning(warning: warningEntry): boolean { - const MILLISECONDS_IN_HOUR = 3600000; - const now = Date.now(); - const expiry = Math.floor(now - (config.hoursAfterWarningExpires * MILLISECONDS_IN_HOUR)); - return warning.issueTime > expiry && !warning.enabled; -} +const MAX_EDIT_DELAY = 900000; // 15 mins const getUsername = (userID: HashedUserID) => db.prepare("get", `SELECT "userName" FROM "userNames" WHERE "userID" = ?`, [userID], { useReplica: true }); @@ -44,25 +38,25 @@ export async function postWarning(req: Request, res: Response): Promise ? AND enabled = 1 AND type = 0`, - [nonAnonUserID, Math.floor(now - (config.hoursAfterWarningExpires * MILLISECONDS_IN_HOUR))], + const warning = (await db.prepare("get", `SELECT "reason" FROM warnings WHERE "userID" = ? AND enabled = 1 AND type = 0`, + [nonAnonUserID], )); - if (warnings.length >= config.maxNumberOfActiveWarnings) { - const warningReason = warnings[0]?.reason; + if (warning != null) { + const warningReason = warning.reason; lock.unlock(); return { status: 403, message: "Vote rejected due to a tip from a moderator. This means that we noticed you were making some common mistakes that are not malicious, and we just want to clarify the rules. " + "Could you please send a message in Discord or Matrix so we can further help you?" + diff --git a/src/types/config.model.ts b/src/types/config.model.ts index 5a6eb21..2fe8963 100644 --- a/src/types/config.model.ts +++ b/src/types/config.model.ts @@ -109,8 +109,6 @@ export interface SBSConfig { categorySupport: Record; maxTitleLength: number; getTopUsersCacheTimeMinutes: number; - maxNumberOfActiveWarnings: number; - hoursAfterWarningExpires: number; rateLimit: { vote: RateLimitConfig; view: RateLimitConfig; diff --git a/test.json b/test.json index 8a4a045..4e34fed 100644 --- a/test.json +++ b/test.json @@ -50,7 +50,6 @@ ] } ], - "hoursAfterWarningExpires": 24, "rateLimit": { "vote": { "windowMs": 900000, diff --git a/test/cases/postSkipSegmentsWarnings.ts b/test/cases/postSkipSegmentsWarnings.ts index 2af7a56..2d6227d 100644 --- a/test/cases/postSkipSegmentsWarnings.ts +++ b/test/cases/postSkipSegmentsWarnings.ts @@ -1,4 +1,3 @@ -import { config } from "../../src/config"; import { getHash } from "../../src/utils/getHash"; import { db } from "../../src/databases/databases"; import assert from "assert"; @@ -33,16 +32,14 @@ describe("postSkipSegments Warnings", () => { const reason02 = ""; const reason03 = "Reason03"; - const MILLISECONDS_IN_HOUR = 3600000; - const WARNING_EXPIRATION_TIME = config.hoursAfterWarningExpires * MILLISECONDS_IN_HOUR; - const insertWarningQuery = 'INSERT INTO warnings ("userID", "issuerUserID", "enabled", "reason", "issueTime") VALUES(?, ?, ?, ?, ?)'; // User 1 | 1 active | custom reason db.prepare("run", insertWarningQuery, [warnUser01Hash, warnVip01Hash, 1, reason01, now]); // User 2 | 1 inactive | default reason db.prepare("run", insertWarningQuery, [warnUser02Hash, warnVip01Hash, 0, reason02, now]); - // User 3 | 1 expired, active | custom reason - db.prepare("run", insertWarningQuery, [warnUser03Hash, warnVip01Hash, 1, reason03, (now - WARNING_EXPIRATION_TIME - 1000)]); + // User 3 | 1 inactive, 1 active | different reasons + db.prepare("run", insertWarningQuery, [warnUser03Hash, warnVip01Hash, 0, reason01, now]); + db.prepare("run", insertWarningQuery, [warnUser03Hash, warnVip01Hash, 1, reason03, now]); // User 4 | 1 active | default reason db.prepare("run", insertWarningQuery, [warnUser04Hash, warnVip01Hash, 1, reason02, now]); }); @@ -87,17 +84,25 @@ describe("postSkipSegments Warnings", () => { .catch(err => done(err)); }); - it("Should be accepted if user has expired warning", (done) => { + it("Should be rejected with custom message if user has active warnings, even if has one inactive warning, should use current message", (done) => { postSkipSegmentJSON({ userID: warnUser03, videoID: warnVideoID, segments: [{ - segment: [53, 60], + segment: [10, 20], category: "sponsor", }], }) .then(res => { - assert.ok(res.status === 200, `Status code was ${res.status} ${res.data}`); + assert.strictEqual(res.status, 403); + const errorMessage = res.data; + const reason = "Reason03"; + const expected = "Submission rejected due to a tip from a moderator. This means that we noticed you were making some common mistakes" + + " that are not malicious, and we just want to clarify the rules. " + + "Could you please send a message in discord.gg/SponsorBlock or matrix.to/#/#sponsor:ajay.app so we can further help you? " + + `Your userID is ${warnUser03Hash}.\n\nTip message: '${reason}'`; + + assert.strictEqual(errorMessage, expected); done(); }) .catch(err => done(err)); diff --git a/test/cases/postWarning.ts b/test/cases/postWarning.ts index 1f7e6a1..f28b585 100644 --- a/test/cases/postWarning.ts +++ b/test/cases/postWarning.ts @@ -7,25 +7,65 @@ import { client } from "../utils/httpClient"; describe("postWarning", () => { // constants const endpoint = "/api/warnUser"; - const getWarning = (userID: string, type = 0) => db.prepare("get", `SELECT "userID", "issueTime", "issuerUserID", enabled, "reason" FROM warnings WHERE "userID" = ? AND "type" = ?`, [userID, type]); + const getWarning = (userID: string, type = 0) => db.prepare("all", `SELECT "userID", "issueTime", "issuerUserID", enabled, "reason" FROM warnings WHERE "userID" = ? AND "type" = ? ORDER BY "issueTime" ASC`, [userID, type]); - const warneduserOneID = "warning-0"; - const warnedUserTwoID = "warning-1"; - const warnedUserOnePublicID = getHash(warneduserOneID); - const warnedUserTwoPublicID = getHash(warnedUserTwoID); - const warningVipOne = "warning-vip-1"; - const warningVipTwo = "warning-vip-2"; + const userID0 = "warning-0"; + const userID1 = "warning-1"; + const userID2 = "warning-2"; + const userID3 = "warning-3"; + const userID4 = "warning-4"; + const userID5 = "warning-5"; + const userID6 = "warning-6"; + const userID7 = "warning-7"; + const userID8 = "warning-8"; + const userID9 = "warning-9"; + const userID10 = "warning-10"; + const userID11 = "warning-11"; + const userID12 = "warning-12"; + const userID13 = "warning-13"; + const publicUserID0 = getHash(userID0); + const publicUserID1 = getHash(userID1); + const publicUserID2 = getHash(userID2); + const publicUserID3 = getHash(userID3); + const publicUserID4 = getHash(userID4); + const publicUserID5 = getHash(userID5); + const publicUserID6 = getHash(userID6); + const publicUserID7 = getHash(userID7); + const publicUserID8 = getHash(userID8); + const publicUserID9 = getHash(userID9); + const publicUserID10 = getHash(userID10); + const publicUserID11 = getHash(userID11); + const publicUserID12 = getHash(userID12); + const publicUserID13 = getHash(userID13); + const vipID1 = "warning-vip-1"; + const vipID2 = "warning-vip-2"; + const publicVipID1 = getHash(vipID1); + const publicVipID2 = getHash(vipID2); const nonVipUser = "warning-non-vip"; before(async () => { - await db.prepare("run", `INSERT INTO "vipUsers" ("userID") VALUES (?)`, [getHash(warningVipOne)]); - await db.prepare("run", `INSERT INTO "vipUsers" ("userID") VALUES (?)`, [getHash(warningVipTwo)]); + const insertWarningQuery = 'INSERT INTO warnings ("userID", "issuerUserID", "enabled", "reason", "issueTime") VALUES(?, ?, ?, ?, ?)'; + const HOUR = 60 * 60 * 1000; + + await db.prepare("run", `INSERT INTO "vipUsers" ("userID") VALUES (?)`, [publicVipID1]); + await db.prepare("run", `INSERT INTO "vipUsers" ("userID") VALUES (?)`, [publicVipID2]); + + await db.prepare("run", insertWarningQuery, [publicUserID1, publicVipID1, 1, "warn reason 1", (Date.now() - 24 * HOUR)]); // 24 hours is much past the edit deadline + await db.prepare("run", insertWarningQuery, [publicUserID2, publicVipID1, 1, "warn reason 2", (Date.now() - 24 * HOUR)]); + await db.prepare("run", insertWarningQuery, [publicUserID3, publicVipID1, 1, "warn reason 3", Date.now()]); + await db.prepare("run", insertWarningQuery, [publicUserID4, publicVipID1, 1, "warn reason 4", Date.now()]); + await db.prepare("run", insertWarningQuery, [publicUserID6, publicVipID1, 1, "warn reason 6", Date.now()]); + await db.prepare("run", insertWarningQuery, [publicUserID9, publicVipID1, 0, "warn reason 9", Date.now()]); + await db.prepare("run", insertWarningQuery, [publicUserID10, publicVipID1, 1, "warn reason 10", Date.now()]); + await db.prepare("run", insertWarningQuery, [publicUserID11, publicVipID1, 1, "warn reason 11", Date.now()]); + await db.prepare("run", insertWarningQuery, [publicUserID12, publicVipID1, 0, "warn reason 12", Date.now()]); + await db.prepare("run", insertWarningQuery, [publicUserID13, publicVipID1, 0, "warn reason 13", Date.now()]); }); it("Should be able to create warning if vip (exp 200)", (done) => { const json = { - issuerUserID: warningVipOne, - userID: warnedUserOnePublicID, + issuerUserID: vipID1, + userID: publicUserID0, reason: "warning-reason-0" }; client.post(endpoint, json) @@ -37,16 +77,18 @@ describe("postWarning", () => { issuerUserID: getHash(json.issuerUserID), reason: json.reason, }; - assert.ok(partialDeepEquals(row, expected)); + assert.equal(row.length, 1); + assert.ok(partialDeepEquals(row[0], expected)); done(); }) .catch(err => done(err)); }); - it("Should be not be able to create a duplicate warning if vip", (done) => { + it("Should be not be able to edit a warning if past deadline and same vip", (done) => { const json = { - issuerUserID: warningVipOne, - userID: warnedUserOnePublicID, + issuerUserID: vipID1, + userID: publicUserID1, + reason: "edited reason 1", }; client.post(endpoint, json) @@ -55,18 +97,41 @@ describe("postWarning", () => { const row = await getWarning(json.userID); const expected = { enabled: 1, - issuerUserID: getHash(json.issuerUserID), + issuerUserID: publicVipID1, }; - assert.ok(partialDeepEquals(row, expected)); + assert.equal(row.length, 1); + assert.ok(partialDeepEquals(row[0], expected)); done(); }) .catch(err => done(err)); }); - it("Should be able to remove warning if vip", (done) => { + it("Should be not be able to edit a warning if past deadline and different vip", (done) => { const json = { - issuerUserID: warningVipOne, - userID: warnedUserOnePublicID, + issuerUserID: vipID2, + userID: publicUserID2, + reason: "edited reason 2", + }; + + client.post(endpoint, json) + .then(async res => { + assert.strictEqual(res.status, 409); + const row = await getWarning(json.userID); + const expected = { + enabled: 1, + issuerUserID: publicVipID1, + }; + assert.equal(row.length, 1); + assert.ok(partialDeepEquals(row[0], expected)); + done(); + }) + .catch(err => done(err)); + }); + + it("Should be able to remove warning if same vip as issuer", (done) => { + const json = { + issuerUserID: vipID1, + userID: publicUserID3, enabled: false }; @@ -77,7 +142,29 @@ describe("postWarning", () => { const expected = { enabled: 0 }; - assert.ok(partialDeepEquals(row, expected)); + assert.equal(row.length, 1); + assert.ok(partialDeepEquals(row[0], expected)); + done(); + }) + .catch(err => done(err)); + }); + + it("Should be able to remove warning if not the same vip as issuer", (done) => { + const json = { + issuerUserID: vipID2, + userID: publicUserID4, + enabled: false + }; + + client.post(endpoint, json) + .then(async res => { + assert.strictEqual(res.status, 200); + const row = await getWarning(json.userID); + const expected = { + enabled: 0 + }; + assert.equal(row.length, 1); + assert.ok(partialDeepEquals(row[0], expected)); done(); }) .catch(err => done(err)); @@ -86,7 +173,8 @@ describe("postWarning", () => { it("Should not be able to create warning if not vip (exp 403)", (done) => { const json = { issuerUserID: nonVipUser, - userID: warnedUserOnePublicID, + userID: publicUserID5, + reason: "warn reason 5", }; client.post(endpoint, json) @@ -106,40 +194,21 @@ describe("postWarning", () => { .catch(err => done(err)); }); - it("Should re-enable disabled warning", (done) => { - const json = { - issuerUserID: warningVipOne, - userID: warnedUserOnePublicID, - enabled: true - }; - - client.post(endpoint, json) - .then(async res => { - assert.strictEqual(res.status, 200); - const data = await getWarning(json.userID); - const expected = { - enabled: 1 - }; - assert.ok(partialDeepEquals(data, expected)); - done(); - }) - .catch(err => done(err)); - }); - it("Should be able to remove your own warning", (done) => { const json = { - userID: warneduserOneID, + userID: userID6, enabled: false }; client.post(endpoint, json) .then(async res => { assert.strictEqual(res.status, 200); - const data = await getWarning(warnedUserOnePublicID); + const row = await getWarning(publicUserID6); const expected = { enabled: 0 }; - assert.ok(partialDeepEquals(data, expected)); + assert.equal(row.length, 1); + assert.ok(partialDeepEquals(row[0], expected)); done(); }) .catch(err => done(err)); @@ -147,18 +216,16 @@ describe("postWarning", () => { it("Should not be able to add your own warning", (done) => { const json = { - userID: warneduserOneID, - enabled: true + userID: userID7, + enabled: true, + reason: "warn reason 7", }; client.post(endpoint, json) .then(async res => { assert.strictEqual(res.status, 403); - const data = await getWarning(warnedUserOnePublicID); - const expected = { - enabled: 0 - }; - assert.ok(partialDeepEquals(data, expected)); + const data = await getWarning(publicUserID7); + assert.equal(data.length, 0); done(); }) .catch(err => done(err)); @@ -166,8 +233,8 @@ describe("postWarning", () => { it("Should not be able to warn a user without reason", (done) => { const json = { - issuerUserID: warningVipOne, - userID: warnedUserTwoPublicID, + issuerUserID: vipID1, + userID: publicUserID8, enabled: true }; @@ -179,21 +246,128 @@ describe("postWarning", () => { .catch(err => done(err)); }); - it("Should be able to re-warn a user without reason", (done) => { + it("Should not be able to re-warn a user without reason", (done) => { const json = { - issuerUserID: warningVipOne, - userID: warnedUserOnePublicID, + issuerUserID: vipID1, + userID: publicUserID9, enabled: true }; + client.post(endpoint, json) + .then(res => { + assert.strictEqual(res.status, 400); + done(); + }) + .catch(err => done(err)); + }); + + it("Should be able to edit a warning if within deadline and same vip", (done) => { + const json = { + issuerUserID: vipID1, + userID: publicUserID10, + enabled: true, + reason: "edited reason 10", + }; + client.post(endpoint, json) .then(async res => { assert.strictEqual(res.status, 200); - const data = await getWarning(warnedUserOnePublicID); + const row = await getWarning(json.userID); const expected = { - enabled: 1 + enabled: 1, + issuerUserID: publicVipID1, + reason: json.reason, }; - assert.ok(partialDeepEquals(data, expected)); + assert.equal(row.length, 1); + assert.ok(partialDeepEquals(row[0], expected)); + done(); + }) + .catch(err => done(err)); + }); + + it("Should not be able to edit a warning if within deadline and different vip", (done) => { + const json = { + issuerUserID: vipID2, + userID: publicUserID11, + enabled: true, + reason: "edited reason 11", + }; + + client.post(endpoint, json) + .then(async res => { + assert.strictEqual(res.status, 409); + const row = await getWarning(json.userID); + const expected = { + enabled: 1, + issuerUserID: publicVipID1, + reason: "warn reason 11", + }; + assert.equal(row.length, 1); + assert.ok(partialDeepEquals(row[0], expected)); + done(); + }) + .catch(err => done(err)); + }); + + it("Should be able to warn a previously warned user again (same vip)", (done) => { + const json = { + issuerUserID: vipID1, + userID: publicUserID12, + enabled: true, + reason: "new reason 12", + }; + + client.post(endpoint, json) + .then(async res => { + assert.strictEqual(res.status, 200); + const row = await getWarning(json.userID); + const expected = [ + { + enabled: 0, + issuerUserID: publicVipID1, + reason: "warn reason 12", + }, + { + enabled: 1, + issuerUserID: publicVipID1, + reason: "new reason 12", + } + ]; + assert.equal(row.length, 2); + assert.ok(partialDeepEquals(row[0], expected[0])); + assert.ok(partialDeepEquals(row[1], expected[1])); + done(); + }) + .catch(err => done(err)); + }); + + it("Should be able to warn a previously warned user again (different vip)", (done) => { + const json = { + issuerUserID: vipID2, + userID: publicUserID13, + enabled: true, + reason: "new reason 13", + }; + + client.post(endpoint, json) + .then(async res => { + assert.strictEqual(res.status, 200); + const row = await getWarning(json.userID); + const expected = [ + { + enabled: 0, + issuerUserID: publicVipID1, + reason: "warn reason 13", + }, + { + enabled: 1, + issuerUserID: publicVipID2, + reason: "new reason 13", + } + ]; + assert.equal(row.length, 2); + assert.ok(partialDeepEquals(row[0], expected[0])); + assert.ok(partialDeepEquals(row[1], expected[1])); done(); }) .catch(err => done(err)); diff --git a/test/cases/voteOnSponsorTime.ts b/test/cases/voteOnSponsorTime.ts index abdfc7c..75375e6 100644 --- a/test/cases/voteOnSponsorTime.ts +++ b/test/cases/voteOnSponsorTime.ts @@ -1,4 +1,3 @@ -import { config } from "../../src/config"; import { db, privateDB } from "../../src/databases/databases"; import { getHash } from "../../src/utils/getHash"; import { ImportMock } from "ts-mock-imports"; @@ -26,8 +25,6 @@ describe("voteOnSponsorTime", () => { const warnUser01Hash = getHash("warn-voteuser01"); const warnUser02Hash = getHash("warn-voteuser02"); const categoryChangeUserHash = getHash(categoryChangeUser); - const MILLISECONDS_IN_HOUR = 3600000; - const warningExpireTime = MILLISECONDS_IN_HOUR * config.hoursAfterWarningExpires; const insertSponsorTimeQuery = 'INSERT INTO "sponsorTimes" ("videoID", "startTime", "endTime", "votes", "locked", "UUID", "userID", "timeSubmitted", "views", "category", "actionType", "shadowHidden", "hidden") VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)'; await db.prepare("run", insertSponsorTimeQuery, ["vote-testtesttest", 1, 11, 2, 0, "vote-uuid-0", "testman", 0, 50, "sponsor", "skip", 0, 0]); @@ -96,8 +93,6 @@ describe("voteOnSponsorTime", () => { await db.prepare("run", insertWarningQuery, [warnUser01Hash, (now - 2000), warnVip01Hash, 1]); await db.prepare("run", insertWarningQuery, [warnUser01Hash, (now - 3601000), warnVip01Hash, 1]); await db.prepare("run", insertWarningQuery, [warnUser02Hash, now, warnVip01Hash, 1]); - await db.prepare("run", insertWarningQuery, [warnUser02Hash, (now - (warningExpireTime + 1000)), warnVip01Hash, 1]); - await db.prepare("run", insertWarningQuery, [warnUser02Hash, (now - (warningExpireTime + 2000)), warnVip01Hash, 1]); await db.prepare("run", 'INSERT INTO "vipUsers" ("userID") VALUES (?)', [getHash(vipUser)]);