From 5e9856c72544f29c8ce040291b61dbb1edd09e9a Mon Sep 17 00:00:00 2001 From: Thomas Belin Date: Wed, 28 Feb 2024 09:18:37 +0100 Subject: [PATCH] runfix: Improve renewal timer computation (#16935) --- package.json | 2 +- .../E2EIdentity/E2EIdentityEnrollment.test.ts | 6 +- .../E2EIdentity/E2EIdentityEnrollment.ts | 40 ++++++----- src/script/E2EIdentity/Enrollment.store.ts | 4 ++ .../EnrollmentTimer/EnrollmentTimer.test.ts | 42 ++++++++---- .../EnrollmentTimer/EnrollmentTimer.ts | 68 +++++++++++++------ yarn.lock | 10 +-- 7 files changed, 111 insertions(+), 61 deletions(-) diff --git a/package.json b/package.json index 94f77679764..c798ecfcaae 100644 --- a/package.json +++ b/package.json @@ -7,7 +7,7 @@ "@lexical/react": "0.12.5", "@wireapp/avs": "9.6.12", "@wireapp/commons": "5.2.5", - "@wireapp/core": "45.0.4", + "@wireapp/core": "45.0.5", "@wireapp/react-ui-kit": "9.16.0", "@wireapp/store-engine-dexie": "2.1.8", "@wireapp/webapp-events": "0.20.1", diff --git a/src/script/E2EIdentity/E2EIdentityEnrollment.test.ts b/src/script/E2EIdentity/E2EIdentityEnrollment.test.ts index 541891d9d0a..0dcd1261610 100644 --- a/src/script/E2EIdentity/E2EIdentityEnrollment.test.ts +++ b/src/script/E2EIdentity/E2EIdentityEnrollment.test.ts @@ -18,7 +18,7 @@ */ import {waitFor} from '@testing-library/react'; -import {TaskScheduler} from '@wireapp/core/lib/util'; +import {LowPrecisionTaskScheduler} from '@wireapp/core/lib/util/LowPrecisionTaskScheduler'; import {container} from 'tsyringe'; import {PrimaryModal} from 'Components/Modals/PrimaryModal'; @@ -163,12 +163,12 @@ describe('E2EIHandler', () => { jest.spyOn(coreMock.service!.e2eIdentity!, 'isEnrollmentInProgress').mockResolvedValue(false); jest.spyOn(coreMock.service!.e2eIdentity!, 'isFreshMLSSelfClient').mockResolvedValue(false); - const taskMock = jest.spyOn(TaskScheduler, 'addTask'); + const taskMock = jest.spyOn(LowPrecisionTaskScheduler, 'addTask'); const instance = await E2EIHandler.getInstance().initialize(params); await instance.startTimers(); - expect(taskMock).toHaveBeenCalledWith(expect.objectContaining({key: 'enrollmentTimer', persist: true})); + expect(taskMock).toHaveBeenCalledWith(expect.objectContaining({key: 'enrollmentTimer'})); }); }); diff --git a/src/script/E2EIdentity/E2EIdentityEnrollment.ts b/src/script/E2EIdentity/E2EIdentityEnrollment.ts index 95ec33943ff..556ce3f7430 100644 --- a/src/script/E2EIdentity/E2EIdentityEnrollment.ts +++ b/src/script/E2EIdentity/E2EIdentityEnrollment.ts @@ -17,11 +17,11 @@ * */ +import {LowPrecisionTaskScheduler} from '@wireapp/core/lib/util/LowPrecisionTaskScheduler'; import {amplify} from 'amplify'; import {container} from 'tsyringe'; import {TypedEventEmitter} from '@wireapp/commons'; -import {util} from '@wireapp/core'; import {WebAppEvents} from '@wireapp/webapp-events'; import {PrimaryModal, removeCurrentModal} from 'Components/Modals/PrimaryModal'; @@ -38,7 +38,6 @@ import {getModalOptions, ModalType} from './Modals'; import {OIDCService} from './OIDCService'; import {OIDCServiceStore} from './OIDCService/OIDCServiceStorage'; -const {TaskScheduler} = util; interface E2EIHandlerParams { discoveryUrl: string; gracePeriodInSeconds: number; @@ -147,21 +146,28 @@ export class E2EIHandler extends TypedEventEmitter { const timerKey = 'enrollmentTimer'; const identity = await getActiveWireIdentity(); - const {firingDate, isSnoozable} = getEnrollmentTimer(identity, e2eActivatedAt, this.config.gracePeriodInMs); + const {firingDate: computedFiringDate, isSnoozable} = getEnrollmentTimer( + identity, + e2eActivatedAt, + this.config.gracePeriodInMs, + ); + + const task = async () => { + EnrollmentStore.clear.timer(); + await this.processEnrollmentUponExpiry(isSnoozable); + }; - const task = async () => this.processEnrollmentUponExpiry(isSnoozable); + const firingDate = EnrollmentStore.get.timer() || computedFiringDate; + EnrollmentStore.store.timer(firingDate); - if (TaskScheduler.hasActiveTask(timerKey)) { - TaskScheduler.continueTask({ - key: timerKey, - task, - }); + if (firingDate <= Date.now()) { + void task(); } else { - TaskScheduler.addTask({ + LowPrecisionTaskScheduler.addTask({ key: timerKey, task, firingDate: firingDate, - persist: true, + intervalDelay: TIME_IN_MILLIS.SECOND * 10, }); } return firingDate - Date.now(); @@ -250,6 +256,10 @@ export class E2EIHandler extends TypedEventEmitter { await this.cleanUp(false); this.emit('deviceStatusUpdated', {status: 'valid'}); + if (isCertificateRenewal) { + await this.startTimers(); + } + await this.showSuccessMessage(isCertificateRenewal); } catch (error) { this.logger.error('E2EI enrollment failed', error); @@ -278,13 +288,7 @@ export class E2EIHandler extends TypedEventEmitter { extraParams: { isRenewal: isCertificateRenewal, }, - primaryActionFn: async () => { - if (isCertificateRenewal) { - // restart the timers for device certificate renewal - await this.startTimers(); - } - resolve(); - }, + primaryActionFn: resolve, secondaryActionFn: () => { amplify.publish(WebAppEvents.PREFERENCES.MANAGE_DEVICES); resolve(); diff --git a/src/script/E2EIdentity/Enrollment.store.ts b/src/script/E2EIdentity/Enrollment.store.ts index ca1a1de8eb7..a51b3e2a22b 100644 --- a/src/script/E2EIdentity/Enrollment.store.ts +++ b/src/script/E2EIdentity/Enrollment.store.ts @@ -18,15 +18,19 @@ */ const e2eActivatedAtKey = 'e2eActivatedAt'; +const e2eTimer = 'e2eTimer'; export const EnrollmentStore = { store: { e2eiActivatedAt: (time: number) => localStorage.setItem(e2eActivatedAtKey, String(time)), + timer: (time: number) => localStorage.setItem(e2eTimer, String(time)), }, get: { e2eiActivatedAt: () => Number(localStorage.getItem(e2eActivatedAtKey)), + timer: () => Number(localStorage.getItem(e2eTimer)), }, clear: { deviceCreatedAt: () => localStorage.removeItem(e2eActivatedAtKey), + timer: () => localStorage.removeItem(e2eTimer), }, }; diff --git a/src/script/E2EIdentity/EnrollmentTimer/EnrollmentTimer.test.ts b/src/script/E2EIdentity/EnrollmentTimer/EnrollmentTimer.test.ts index b2ab5d669c4..abd91707cfb 100644 --- a/src/script/E2EIdentity/EnrollmentTimer/EnrollmentTimer.test.ts +++ b/src/script/E2EIdentity/EnrollmentTimer/EnrollmentTimer.test.ts @@ -17,15 +17,17 @@ * */ +import {TimeInMillis} from '@wireapp/commons/lib/util/TimeUtil'; + import {getEnrollmentTimer, messageRetentionTime} from './EnrollmentTimer'; import {MLSStatuses} from '../E2EIdentityVerification'; describe('e2ei delays', () => { - const gracePeriod = 3600; + const gracePeriod = 7 * TimeInMillis.DAY; beforeEach(() => { jest.useFakeTimers(); - jest.setSystemTime(0); + jest.setSystemTime(1709050878009); }); it('should return an immediate delay if the identity is expired', () => { @@ -34,32 +36,48 @@ describe('e2ei delays', () => { expect(delay).toEqual({firingDate: Date.now(), isSnoozable: false}); }); - it('should return a snoozable timer if device is new and still in the grace period', () => { - const {firingDate, isSnoozable} = getEnrollmentTimer(undefined, Date.now(), gracePeriod); + it.each([ + [TimeInMillis.DAY * 2, TimeInMillis.DAY * 30, TimeInMillis.DAY], + [TimeInMillis.DAY, TimeInMillis.DAY * 30, TimeInMillis.HOUR * 4], + [TimeInMillis.HOUR, TimeInMillis.DAY * 30, TimeInMillis.MINUTE * 15], + [TimeInMillis.HOUR * 3, TimeInMillis.DAY * 30, TimeInMillis.HOUR], + [TimeInMillis.MINUTE * 10, TimeInMillis.DAY * 30, TimeInMillis.MINUTE * 5], + [TimeInMillis.MINUTE * 30, TimeInMillis.DAY * 30, TimeInMillis.MINUTE * 15], + ])('should return a snoozable timer if device is still valid', (validityPeriod, grace, expectedTimer) => { + const {firingDate, isSnoozable} = getEnrollmentTimer( + {certificate: ' ', notAfter: (Date.now() + validityPeriod) / 1000} as any, + Date.now(), + grace, + ); expect(isSnoozable).toBeTruthy(); - expect(firingDate).toBeLessThanOrEqual(gracePeriod); + expect(firingDate).toBe(Date.now() + expectedTimer); }); - it('should return a snoozable timer if device is certified and still in the grace period', () => { + it('should return a snoozable timer in the long future if device is certified before the grace period', () => { + const deadline = Date.now() + messageRetentionTime + gracePeriod + 1000; + const gracePeriodStartingPoint = deadline - gracePeriod; + const {firingDate, isSnoozable} = getEnrollmentTimer( - {certificate: ' ', notAfter: Date.now() + messageRetentionTime + gracePeriod + 1000} as any, + {certificate: ' ', notAfter: deadline / 1000} as any, Date.now(), gracePeriod, ); expect(isSnoozable).toBeTruthy(); - expect(firingDate).toBeLessThanOrEqual(gracePeriod); + expect(firingDate).toBe(gracePeriodStartingPoint); }); - it('should return a non snoozable timer if device is certified about to expired', () => { + it('should return a non snoozable timer if device is out of the grace period', () => { + const deadline = Date.now() + gracePeriod + 1000; + const gracePeriodStartingPoint = deadline - gracePeriod; const {firingDate, isSnoozable} = getEnrollmentTimer( - {certificate: ' ', notAfter: Date.now() + gracePeriod + 1000} as any, + {certificate: ' ', notAfter: deadline / 1000} as any, Date.now(), gracePeriod, ); - expect(isSnoozable).toBeFalsy(); - expect(firingDate).toBeLessThanOrEqual(gracePeriod); + expect(isSnoozable).toBeTruthy(); + expect(firingDate).toBe(gracePeriodStartingPoint); }); }); diff --git a/src/script/E2EIdentity/EnrollmentTimer/EnrollmentTimer.ts b/src/script/E2EIdentity/EnrollmentTimer/EnrollmentTimer.ts index 36df081e942..6feb1c73bf9 100644 --- a/src/script/E2EIdentity/EnrollmentTimer/EnrollmentTimer.ts +++ b/src/script/E2EIdentity/EnrollmentTimer/EnrollmentTimer.ts @@ -32,31 +32,58 @@ export const ONE_DAY = TimeInMillis.DAY; // message retention time on backend (hardcoded to 28 days) export const messageRetentionTime = 28 * TimeInMillis.DAY; +type GracePeriod = { + /** start date of the grace period (unix timestamp) */ + start: number; + /** end date of the grace period (unix timestamp) */ + end: number; +}; /** * Will return a suitable snooze time based on the grace period - * @param expiryDate - the full grace period length in milliseconds + * @param deadline - the full grace period length in milliseconds */ -function getNextTick(expiryDate: number, gracePeriodDuration: number, isFirstEnrollment: boolean): number { - const leftoverTimer = expiryDate - Date.now(); +function getNextTick({end, start}: GracePeriod): number { + if (Date.now() >= end) { + // If the grace period is over, we should force the user to enroll + return 0; + } - // First a first enrollment we only consider the grace period. For enrolled devices we also consider the backend message retention time - const extraDelay = isFirstEnrollment ? 0 : randomInt(TimeInMillis.DAY) + messageRetentionTime; + if (Date.now() < start) { + // If we are not in the grace period yet, we start the timer when the grace period starts + return start - Date.now(); + } + const validityPeriod = end - Date.now(); - const gracePeriod = Math.max(0, Math.min(gracePeriodDuration, leftoverTimer - extraDelay)); - if (gracePeriod <= 0) { - return 0; + if (validityPeriod <= FIFTEEN_MINUTES) { + return Math.min(FIVE_MINUTES, validityPeriod); + } else if (validityPeriod <= ONE_HOUR) { + return Math.min(FIFTEEN_MINUTES, validityPeriod); + } else if (validityPeriod <= FOUR_HOURS) { + return Math.min(ONE_HOUR, validityPeriod); + } else if (validityPeriod <= ONE_DAY) { + return Math.min(FOUR_HOURS, validityPeriod); } + return Math.min(ONE_DAY, validityPeriod); +} - if (gracePeriod <= FIFTEEN_MINUTES) { - return Math.min(FIVE_MINUTES, gracePeriod); - } else if (gracePeriod <= ONE_HOUR) { - return Math.min(FIFTEEN_MINUTES, gracePeriod); - } else if (gracePeriod <= FOUR_HOURS) { - return Math.min(ONE_HOUR, gracePeriod); - } else if (gracePeriod <= ONE_DAY) { - return Math.min(FOUR_HOURS, gracePeriod); +function getGracePeriod( + identity: WireIdentity | undefined, + e2eActivatedAt: number, + teamGracePeriodDuration: number, +): GracePeriod { + const isFirstEnrollment = !identity?.certificate; + if (isFirstEnrollment) { + // For a new device, the deadline is the e2ei activate date + the grace period + return {end: e2eActivatedAt + teamGracePeriodDuration, start: Date.now()}; } - return Math.min(ONE_DAY, gracePeriod); + + // To be sure the device does not expire, we want to keep a safe delay + const safeDelay = randomInt(TimeInMillis.DAY) + messageRetentionTime; + + const end = Number(identity.notAfter) * TimeInMillis.SECOND; + const start = Math.max(end - safeDelay, end - teamGracePeriodDuration); + + return {end, start}; } export function getEnrollmentTimer( @@ -68,12 +95,9 @@ export function getEnrollmentTimer( return {isSnoozable: false, firingDate: Date.now()}; } - const isFirstEnrollment = !identity?.certificate; - const expiryDate = isFirstEnrollment - ? e2eiActivatedAt + teamGracePeriodDuration - : Number(identity.notAfter) * TimeInMillis.SECOND; + const deadline = getGracePeriod(identity, e2eiActivatedAt, teamGracePeriodDuration); + const nextTick = getNextTick(deadline); - const nextTick = getNextTick(expiryDate, teamGracePeriodDuration, isFirstEnrollment); // When logging in to a old device that doesn't have an identity yet, we trigger an enrollment timer return {isSnoozable: nextTick > 0, firingDate: Date.now() + nextTick}; } diff --git a/yarn.lock b/yarn.lock index 6b56fbf366a..8355cc1b502 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4754,9 +4754,9 @@ __metadata: languageName: node linkType: hard -"@wireapp/core@npm:45.0.4": - version: 45.0.4 - resolution: "@wireapp/core@npm:45.0.4" +"@wireapp/core@npm:45.0.5": + version: 45.0.5 + resolution: "@wireapp/core@npm:45.0.5" dependencies: "@wireapp/api-client": ^26.10.12 "@wireapp/commons": ^5.2.5 @@ -4775,7 +4775,7 @@ __metadata: long: ^5.2.0 uuidjs: 4.2.13 zod: 3.22.4 - checksum: e9b3af41c2092c589f20eda71a79336489008ebcddeb42076c298a2b694bd9318ea83d8141e998b3d5ea5729535f95c01b305b8a7750b8e90e9f63bac5f6e708 + checksum: 6efa9c7351b6ac6aeea6288437e3ddb41e1928f9876d4ae3d853c7c4787f2864fce5052d484e49f07e181fe85937e4fa61ea6df5a9ad67781e2340cc80e3f1d7 languageName: node linkType: hard @@ -17444,7 +17444,7 @@ __metadata: "@wireapp/avs": 9.6.12 "@wireapp/commons": 5.2.5 "@wireapp/copy-config": 2.1.14 - "@wireapp/core": 45.0.4 + "@wireapp/core": 45.0.5 "@wireapp/eslint-config": 3.0.5 "@wireapp/prettier-config": 0.6.3 "@wireapp/react-ui-kit": 9.16.0