From 2f5e4f1b9841330f654d8c43bdbe4c7130269da5 Mon Sep 17 00:00:00 2001 From: Chris Eager <79161849+eager-signal@users.noreply.github.com> Date: Tue, 12 Apr 2022 12:54:04 -0700 Subject: [PATCH] Update message reporting to use sender ACI instead of E164 --- ts/jobs/helpers/addReportSpamJob.ts | 8 +-- ts/jobs/reportSpamJobQueue.ts | 6 +- ts/sql/migrations/55-report-message-aci.ts | 68 +++++++++++++++++++ ts/sql/migrations/index.ts | 2 + .../jobs/helpers/addReportSpamJob_test.ts | 6 +- ts/test-node/sql_migrations_test.ts | 46 +++++++++++++ ts/textsecure/WebAPI.ts | 6 +- 7 files changed, 129 insertions(+), 13 deletions(-) create mode 100644 ts/sql/migrations/55-report-message-aci.ts diff --git a/ts/jobs/helpers/addReportSpamJob.ts b/ts/jobs/helpers/addReportSpamJob.ts index b6d0d8e108..847f6379c6 100644 --- a/ts/jobs/helpers/addReportSpamJob.ts +++ b/ts/jobs/helpers/addReportSpamJob.ts @@ -22,10 +22,10 @@ export async function addReportSpamJob({ 'addReportSpamJob: cannot report spam for non-direct conversations' ); - const { e164 } = conversation; - if (!e164) { + const { uuid } = conversation; + if (!uuid) { log.info( - 'addReportSpamJob got a conversation with no E164, which the server does not support. Doing nothing' + 'addReportSpamJob got a conversation with no UUID, which the server does not support. Doing nothing' ); return; } @@ -41,5 +41,5 @@ export async function addReportSpamJob({ return; } - await jobQueue.add({ e164, serverGuids }); + await jobQueue.add({ uuid, serverGuids }); } diff --git a/ts/jobs/reportSpamJobQueue.ts b/ts/jobs/reportSpamJobQueue.ts index 85c52f2c02..cd4e1428ab 100644 --- a/ts/jobs/reportSpamJobQueue.ts +++ b/ts/jobs/reportSpamJobQueue.ts @@ -27,7 +27,7 @@ const isRetriable4xxStatus = (code: number): boolean => RETRYABLE_4XX_FAILURE_STATUSES.has(code); const reportSpamJobDataSchema = z.object({ - e164: z.string().min(1), + uuid: z.string().min(1), serverGuids: z.string().array().min(1).max(1000), }); @@ -48,7 +48,7 @@ export class ReportSpamJobQueue extends JobQueue { { data }: Readonly<{ data: ReportSpamJobData }>, { log }: Readonly<{ log: LoggerType }> ): Promise { - const { e164, serverGuids } = data; + const { uuid, serverGuids } = data; await new Promise(resolve => { window.storage.onready(resolve); @@ -66,7 +66,7 @@ export class ReportSpamJobQueue extends JobQueue { try { await Promise.all( - map(serverGuids, serverGuid => server.reportMessage(e164, serverGuid)) + map(serverGuids, serverGuid => server.reportMessage(uuid, serverGuid)) ); } catch (err: unknown) { if (!(err instanceof HTTPError)) { diff --git a/ts/sql/migrations/55-report-message-aci.ts b/ts/sql/migrations/55-report-message-aci.ts new file mode 100644 index 0000000000..95b31dd972 --- /dev/null +++ b/ts/sql/migrations/55-report-message-aci.ts @@ -0,0 +1,68 @@ +// Copyright 2022 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +import type { Database } from 'better-sqlite3'; +import type { LoggerType } from '../../types/Logging'; +import { getJobsInQueueSync, insertJobSync } from '../Server'; +import { isRecord } from '../../util/isRecord'; +import { isIterable } from '../../util/iterables'; + +export default function updateToSchemaVersion55( + currentVersion: number, + db: Database, + logger: LoggerType +): void { + if (currentVersion >= 55) { + return; + } + + db.transaction(() => { + const deleteJobsInQueue = db.prepare( + 'DELETE FROM jobs WHERE queueType = $queueType' + ); + + // First, make sure that report spam job data has e164 and serverGuids + const reportSpamJobs = getJobsInQueueSync(db, 'report spam'); + deleteJobsInQueue.run({ queueType: 'report spam' }); + + reportSpamJobs.forEach(job => { + const { data, id } = job; + + if (!isRecord(data)) { + logger.warn( + `updateToSchemaVersion55: report spam queue job ${id} was missing valid data` + ); + return; + } + + const { e164, serverGuids } = data; + if (typeof e164 !== 'string') { + logger.warn( + `updateToSchemaVersion55: report spam queue job ${id} had a non-string e164` + ); + return; + } + + if (!isIterable(serverGuids)) { + logger.warn( + `updateToSchemaVersion55: report spam queue job ${id} had a non-iterable serverGuids` + ); + return; + } + + const newJob = { + ...job, + queueType: 'report spam', + data: { + uuid: e164, // this looks odd, but they are both strings and interchangeable in the server API + serverGuids, + }, + }; + + insertJobSync(db, newJob); + }); + + db.pragma('user_version = 55'); + })(); + logger.info('updateToSchemaVersion55: success!'); +} diff --git a/ts/sql/migrations/index.ts b/ts/sql/migrations/index.ts index cbe07c0081..862e3eee1b 100644 --- a/ts/sql/migrations/index.ts +++ b/ts/sql/migrations/index.ts @@ -30,6 +30,7 @@ import updateToSchemaVersion51 from './51-centralize-conversation-jobs'; import updateToSchemaVersion52 from './52-optimize-stories'; import updateToSchemaVersion53 from './53-gv2-banned-members'; import updateToSchemaVersion54 from './54-unprocessed-received-at-counter'; +import updateToSchemaVersion55 from './55-report-message-aci'; function updateToSchemaVersion1( currentVersion: number, @@ -1923,6 +1924,7 @@ export const SCHEMA_VERSIONS = [ updateToSchemaVersion52, updateToSchemaVersion53, updateToSchemaVersion54, + updateToSchemaVersion55, ]; export function updateSchema(db: Database, logger: LoggerType): void { diff --git a/ts/test-node/jobs/helpers/addReportSpamJob_test.ts b/ts/test-node/jobs/helpers/addReportSpamJob_test.ts index 04d7bbbe89..342e76f312 100644 --- a/ts/test-node/jobs/helpers/addReportSpamJob_test.ts +++ b/ts/test-node/jobs/helpers/addReportSpamJob_test.ts @@ -29,9 +29,9 @@ describe('addReportSpamJob', () => { }; }); - it('does nothing if the conversation lacks an E164', async () => { + it('does nothing if the conversation lacks a UUID', async () => { await addReportSpamJob({ - conversation: getDefaultConversation({ e164: undefined }), + conversation: getDefaultConversation({ uuid: undefined }), getMessageServerGuidsForSpam, jobQueue, }); @@ -66,7 +66,7 @@ describe('addReportSpamJob', () => { sinon.assert.calledOnce(jobQueue.add); sinon.assert.calledWith(jobQueue.add, { - e164: conversation.e164, + uuid: conversation.uuid, serverGuids: ['abc', 'xyz'], }); }); diff --git a/ts/test-node/sql_migrations_test.ts b/ts/test-node/sql_migrations_test.ts index f02a917bb5..29ec49ae3b 100644 --- a/ts/test-node/sql_migrations_test.ts +++ b/ts/test-node/sql_migrations_test.ts @@ -1726,4 +1726,50 @@ describe('SQL migrations test', () => { ); }); }); + + describe('updateToSchemaVersion55', () => { + it('moves existing report spam jobs to new schema', () => { + updateToVersion(54); + + const E164_1 = '+12125550155'; + const MESSAGE_ID_1 = generateGuid(); + + db.exec( + ` + INSERT INTO jobs + (id, timestamp, queueType, data) + VALUES + ('id-1', 1, 'random job', '{}'), + ('id-2', 2, 'report spam', '{"serverGuids": ["${MESSAGE_ID_1}"], "e164": "${E164_1}"}'); + ` + ); + + const totalJobs = db.prepare('SELECT COUNT(*) FROM jobs;').pluck(); + const reportSpamJobs = db + .prepare("SELECT COUNT(*) FROM jobs WHERE queueType = 'report spam';") + .pluck(); + + assert.strictEqual(totalJobs.get(), 2, 'before total'); + assert.strictEqual(reportSpamJobs.get(), 1, 'before report spam'); + + updateToVersion(55); + + assert.strictEqual(totalJobs.get(), 2, 'after total'); + assert.strictEqual(reportSpamJobs.get(), 1, 'after report spam'); + + const jobs = getJobsInQueueSync(db, 'report spam'); + + assert.deepEqual(jobs, [ + { + id: 'id-2', + queueType: 'report spam', + timestamp: 2, + data: { + serverGuids: [`${MESSAGE_ID_1}`], + uuid: `${E164_1}`, + }, + }, + ]); + }); + }); }); diff --git a/ts/textsecure/WebAPI.ts b/ts/textsecure/WebAPI.ts index f25b0c91af..b5724cea77 100644 --- a/ts/textsecure/WebAPI.ts +++ b/ts/textsecure/WebAPI.ts @@ -918,7 +918,7 @@ export type WebAPIType = { registerCapabilities: (capabilities: CapabilitiesUploadType) => Promise; registerKeys: (genKeys: KeysType, uuidKind: UUIDKind) => Promise; registerSupportForUnauthenticatedDelivery: () => Promise; - reportMessage: (senderE164: string, serverGuid: string) => Promise; + reportMessage: (senderUuid: string, serverGuid: string) => Promise; requestVerificationSMS: (number: string, token: string) => Promise; requestVerificationVoice: (number: string, token: string) => Promise; checkAccountExistence: (uuid: UUID) => Promise; @@ -1663,13 +1663,13 @@ export function initialize({ } async function reportMessage( - senderE164: string, + senderUuid: string, serverGuid: string ): Promise { await _ajax({ call: 'reportMessage', httpType: 'POST', - urlParameters: `/${senderE164}/${serverGuid}`, + urlParameters: `/${senderUuid}/${serverGuid}`, responseType: 'bytes', }); }