From 13811233b6147913cd534472974d593c957297ae Mon Sep 17 00:00:00 2001 From: Mikunj Date: Fri, 26 Jun 2020 14:31:40 +1000 Subject: [PATCH] Add session request expiry checks --- js/background.js | 9 +++ .../outgoing/content/SessionRequestMessage.ts | 3 +- ts/session/protocols/SessionProtocol.ts | 25 +++++- .../session/protocols/SessionProtocol_test.ts | 78 ++++++++++++++++++- 4 files changed, 111 insertions(+), 4 deletions(-) diff --git a/js/background.js b/js/background.js index 1b6de3a46..d1b054bd1 100644 --- a/js/background.js +++ b/js/background.js @@ -1640,6 +1640,15 @@ } } + libsession.Protocols.SessionProtocol.checkSessionRequestExpiry().catch( + e => { + window.log.error( + 'Error occured which checking for session request expiry', + e + ); + } + ); + storage.onready(async () => { idleDetector.start(); }); diff --git a/ts/session/messages/outgoing/content/SessionRequestMessage.ts b/ts/session/messages/outgoing/content/SessionRequestMessage.ts index 89563d9e0..b7c0be756 100644 --- a/ts/session/messages/outgoing/content/SessionRequestMessage.ts +++ b/ts/session/messages/outgoing/content/SessionRequestMessage.ts @@ -17,6 +17,7 @@ interface SessionRequestParams extends MessageParams { } export class SessionRequestMessage extends ContentMessage { + public static readonly ttl = 4 * 24 * 60 * 60 * 1000; // 4 days private readonly preKeyBundle: PreKeyBundleType; constructor(params: SessionRequestParams) { @@ -25,7 +26,7 @@ export class SessionRequestMessage extends ContentMessage { } public ttl(): number { - return 4 * 24 * 60 * 60 * 1000; // 4 days + return SessionRequestMessage.ttl; } protected getPreKeyBundleMessage(): SignalService.PreKeyBundleMessage { diff --git a/ts/session/protocols/SessionProtocol.ts b/ts/session/protocols/SessionProtocol.ts index cd62d4b84..45b3673f6 100644 --- a/ts/session/protocols/SessionProtocol.ts +++ b/ts/session/protocols/SessionProtocol.ts @@ -12,7 +12,7 @@ interface StringToNumberMap { export class SessionProtocol { private static dbLoaded: Boolean = false; /** - * This map olds the sent session timestamps, i.e. session requests message effectively sent to the recipient. + * This map holds the sent session timestamps, i.e. session requests message effectively sent to the recipient. * It is backed by a database entry so it's loaded from db on startup. * This map should not be used directly, but instead through * `updateSendSessionTimestamp()`, or `hasSendSessionRequest()` @@ -73,6 +73,29 @@ export class SessionProtocol { return pendingSend || hasSent; } + /** + * Checks to see if any outgoing session requests have expired and re-sends them again if they have. + */ + public static async checkSessionRequestExpiry(): Promise { + await this.fetchFromDBIfNeeded(); + + const now = Date.now(); + const sentTimestamps = Object.entries(this.sentSessionsTimestamp); + const promises = sentTimestamps.map(async ([device, sent]) => { + const expireTime = sent + SessionRequestMessage.ttl; + // Check if we need to send a session request + if (now < expireTime) { + return; + } + + // Unset the timestamp, so that if it fails to send in this function, it will be guaranteed to send later on. + await this.updateSentSessionTimestamp(device, undefined); + await this.sendSessionRequestIfNeeded(new PubKey(device)); + }); + + return Promise.all(promises) as Promise; + } + /** * Triggers a SessionRequestMessage to be sent if: * - we do not already have a session and diff --git a/ts/test/session/protocols/SessionProtocol_test.ts b/ts/test/session/protocols/SessionProtocol_test.ts index 5f87a99b9..2ce8ccaf4 100644 --- a/ts/test/session/protocols/SessionProtocol_test.ts +++ b/ts/test/session/protocols/SessionProtocol_test.ts @@ -11,7 +11,7 @@ import { PubKey } from '../../../session/types'; // tslint:disable-next-line: max-func-body-length describe('SessionProtocol', () => { const sandbox = sinon.createSandbox(); - const ourNumber = 'ourNumber'; + const ourNumber = TestUtils.generateFakePubKey(); const pubkey = TestUtils.generateFakePubKey(); let getItemById: sinon.SinonStub; let send: sinon.SinonStub; @@ -51,7 +51,7 @@ describe('SessionProtocol', () => { getItemById = TestUtils.stubData('getItemById').resolves({ value: {} }); - sandbox.stub(UserUtil, 'getCurrentDevicePubKey').resolves(ourNumber); + sandbox.stub(UserUtil, 'getCurrentDevicePubKey').resolves(ourNumber.key); send = sandbox.stub(MessageSender, 'send' as any); SessionProtocol.reset(); }); @@ -99,6 +99,80 @@ describe('SessionProtocol', () => { }); }); + describe('checkSessionRequestExpiry', () => { + let clock: sinon.SinonFakeTimers; + let now: number; + let sendSessionRequestStub: sinon.SinonStub< + [SessionRequestMessage, PubKey], + Promise + >; + beforeEach(() => { + now = Date.now(); + clock = sandbox.useFakeTimers(now); + + sendSessionRequestStub = sandbox + .stub(SessionProtocol, 'sendSessionRequest') + .resolves(); + }); + + it('should not send a session request if none have expired', async () => { + getItemById.withArgs('sentSessionsTimestamp').resolves({ + id: 'sentSessionsTimestamp', + value: { + [pubkey.key]: now, + }, + }); + + // Set the time just before expiry + clock.tick(SessionRequestMessage.ttl - 100); + + await SessionProtocol.checkSessionRequestExpiry(); + expect(getItemById.calledWith('sentSessionsTimestamp')); + expect(sendSessionRequestStub.callCount).to.equal(0); + }); + + it('should send a session request if expired', async () => { + getItemById.withArgs('sentSessionsTimestamp').resolves({ + id: 'sentSessionsTimestamp', + value: { + [pubkey.key]: now, + }, + }); + + // Expire the request + clock.tick(SessionRequestMessage.ttl + 100); + + await SessionProtocol.checkSessionRequestExpiry(); + expect(getItemById.calledWith('sentSessionsTimestamp')); + expect(sendSessionRequestStub.callCount).to.equal(1); + }); + + it('should remove the old sent timestamp when expired', async () => { + getItemById.withArgs('sentSessionsTimestamp').resolves({ + id: 'sentSessionsTimestamp', + value: { + [pubkey.key]: now, + }, + }); + + // Remove this call from the equation + sandbox.stub(SessionProtocol, 'sendSessionRequestIfNeeded').resolves(); + + // Expire the request + clock.tick(SessionRequestMessage.ttl + 100); + + await SessionProtocol.checkSessionRequestExpiry(); + expect(getItemById.calledWith('sentSessionsTimestamp')); + expect(await SessionProtocol.hasSentSessionRequest(pubkey)).to.equal( + false, + 'hasSentSessionRequest should return false.' + ); + expect(SessionProtocol.getSentSessionsTimestamp()).to.not.have.property( + pubkey.key + ); + }); + }); + describe('onSessionEstablished', () => { beforeEach(async () => { // add an existing entry in the sentMap