Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

runfix: Re-enable showing enrollment flow when e2ei is activated #16543

Merged
merged 3 commits into from
Jan 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions src/script/E2EIdentity/DelayTimer/DelayTimer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
*/

import {FIFTEEN_MINUTES, FOUR_HOURS, ONE_HOUR, ONE_MINUTE} from './delay';
import {DelayTimerService} from './DelayTimer'; // Update this with your module's actual path
import {DelayTimerService} from './DelayTimer';

describe('createGracePeriodTimer', () => {
let timer: DelayTimerService | undefined;
Expand All @@ -27,16 +27,14 @@ describe('createGracePeriodTimer', () => {
jest.clearAllMocks();
jest.useFakeTimers();
global.localStorage.clear();
timer = DelayTimerService?.getInstance({
timer = new DelayTimerService({
gracePeriodInMS: 0,
gracePeriodExpiredCallback: jest.fn(),
delayPeriodExpiredCallback: jest.fn(),
});
});

afterEach(() => {
timer?.resetInstance();
timer = undefined;
jest.useRealTimers();
});

Expand Down
37 changes: 2 additions & 35 deletions src/script/E2EIdentity/DelayTimer/DelayTimer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,42 +32,21 @@ interface CreateGracePeriodTimerParams {
delayPeriodExpiredCallback: () => void;
}

class DelayTimerService {
private static instance: DelayTimerService | null = null;
export class DelayTimerService {
private gracePeriodInMS: number;
private gracePeriodExpiredCallback: () => void;
private delayPeriodExpiredCallback: () => void;
private readonly logger = logdown('@wireapp/core/DelayTimer');
private delayPeriodTimerKey: string = 'E2EIdentity_DelayTimer';
private gracePeriodTimerKey: string = 'E2EIdentity_GracePeriodTimer';

private constructor({
gracePeriodInMS,
gracePeriodExpiredCallback,
delayPeriodExpiredCallback,
}: CreateGracePeriodTimerParams) {
constructor({gracePeriodInMS, gracePeriodExpiredCallback, delayPeriodExpiredCallback}: CreateGracePeriodTimerParams) {
this.gracePeriodInMS = gracePeriodInMS;
this.gracePeriodExpiredCallback = gracePeriodExpiredCallback;
this.delayPeriodExpiredCallback = delayPeriodExpiredCallback;
this.initialize();
}

/**
* Get the singleton instance of GracePeriodTimer or create a new one
* For the first time, params are required to create the instance
* @param params The params to create the grace period timer
* @returns The singleton instance of GracePeriodTimer
*/
public static getInstance(params?: CreateGracePeriodTimerParams) {
if (!DelayTimerService.instance) {
if (!params) {
throw new Error('DelayTimerService is not initialized. Please call getInstance with params.');
}
DelayTimerService.instance = new DelayTimerService(params);
}
return DelayTimerService.instance;
}
Comment on lines -61 to -69
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was never needed that this class is a singleton.
It simplifies the API to make it a regular instantiable class


/**
* @param CreateGracePeriodTimerParams The params to create the grace period timer
*/
Expand Down Expand Up @@ -263,16 +242,6 @@ class DelayTimerService {
: 0;
}

/**
* Reset the instance
*/
public resetInstance() {
DelayTimerStore.clear.all();
this.clearGracePeriodTimer();
this.clearDelayPeriodTimer();
DelayTimerService.instance = null;
}

public isDelayTimerActive() {
return TaskScheduler.hasActiveTask(this.delayPeriodTimerKey);
}
Expand All @@ -283,5 +252,3 @@ class DelayTimerService {
return remainingTime - delayTime > 0;
}
}

export {DelayTimerService};
13 changes: 6 additions & 7 deletions src/script/E2EIdentity/E2EIdentityEnrollment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ export enum E2EIHandlerStep {
interface E2EIHandlerParams {
discoveryUrl: string;
gracePeriodInSeconds: number;
isFreshMLSSelfClient?: boolean;
}

type Events = {
Expand Down Expand Up @@ -121,13 +120,13 @@ export class E2EIHandler extends TypedEventEmitter<Events> {
E2EIHandler.instance = null;
}

public async initialize({discoveryUrl, gracePeriodInSeconds}: E2EIHandlerParams) {
public initialize({discoveryUrl, gracePeriodInSeconds}: E2EIHandlerParams) {
if (isE2EIEnabled()) {
const gracePeriodInMs = gracePeriodInSeconds * TIME_IN_MILLIS.SECOND;
this.config = {
discoveryUrl,
gracePeriodInMs,
timer: DelayTimerService.getInstance({
timer: new DelayTimerService({
gracePeriodInMS: gracePeriodInMs,
gracePeriodExpiredCallback: () => null,
delayPeriodExpiredCallback: () => null,
Expand Down Expand Up @@ -211,10 +210,10 @@ export class E2EIHandler extends TypedEventEmitter<Events> {
/**
* Calculates the date when the E2EI certificate renewal should be prompted.
*
* @param {number} timeRemainingMS - Certificate validity period in days (VP).
* @param {number} historyTime - Maximum time messages are stored in days (HT).
* @param {number} gracePeriod - Time to activate certificate in days (GP).
* @returns {Date} - The date to start prompting for certificate renewal.
* @param timeRemainingMS - Certificate validity period in days (VP).
* @param historyTime - Maximum time messages are stored in days (HT).
* @param gracePeriod - Time to activate certificate in days (GP).
* @returns The date to start prompting for certificate renewal.
*/
private calculateRenewalTime(timeRemainingMS: number, historyTimeMS: number, gracePeriodMS: number) {
// Calculate a random time between 0 and 1 days
Expand Down
2 changes: 1 addition & 1 deletion src/script/main/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@ export class App {
telemetry.timeStep(AppInitTimingsStep.INITIALIZED_CRYPTOGRAPHY);

const {members: teamMembers, features: teamFeatures} = await teamRepository.initTeam(selfUser.teamId);
const e2eiHandler = await configureE2EI(this.logger, teamFeatures);
const e2eiHandler = configureE2EI(this.logger, teamFeatures);
if (e2eiHandler) {
/* We first try to do the initial enrollment (if the user has not yet enrolled)
* We need to enroll before anything else (in particular joining MLS conversations)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export function FeatureConfigChangeHandler({teamState}: Props): null {
useEffect(() => {
if (config) {
// initialize feature handlers
configureE2EI(logger, config);
configureE2EI(logger, config)?.attemptEnrollment();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the actual fix. When we detect the feature change, we trigger the attemptEnrollment right away

}
}, [config]);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,39 +19,37 @@

import {FeatureStatus, FEATURE_KEY, FeatureList} from '@wireapp/api-client/lib/team';

import {E2EIHandler, isFreshMLSSelfClient} from 'src/script/E2EIdentity';
import {E2EIHandler} from 'src/script/E2EIdentity';
import {Logger} from 'Util/Logger';

import {hasE2EIVerificationExpiration, hasMLSDefaultProtocol} from '../../../../../guards/Protocol';

export const configureE2EI = async (logger: Logger, config: FeatureList): Promise<void | E2EIHandler> => {
export const configureE2EI = (logger: Logger, config: FeatureList): undefined | E2EIHandler => {
const e2eiConfig = config[FEATURE_KEY.MLSE2EID];
const mlsConfig = config[FEATURE_KEY.MLS];
// Check if MLS or MLS E2EIdentity feature is existent
if (!hasE2EIVerificationExpiration(e2eiConfig) || !hasMLSDefaultProtocol(mlsConfig)) {
return;
return undefined;
}

// Check if E2EIdentity feature is enabled
if (e2eiConfig?.status === FeatureStatus.ENABLED) {
// Check if MLS feature is enabled
if (mlsConfig?.status !== FeatureStatus.ENABLED) {
logger.info('Warning: E2EIdentity feature enabled but MLS feature is not active');
return;
return undefined;
}
// Check if E2EIdentity feature has a server discoveryUrl
if (!e2eiConfig.config || !e2eiConfig.config.acmeDiscoveryUrl || e2eiConfig.config.acmeDiscoveryUrl.length <= 0) {
logger.info('Warning: E2EIdentity feature enabled but no discoveryUrl provided');
return;
return undefined;
}

const freshMLSSelfClient = await isFreshMLSSelfClient();

// Either get the current E2EIdentity handler instance or create a new one
return E2EIHandler.getInstance().initialize({
discoveryUrl: e2eiConfig.config.acmeDiscoveryUrl!,
gracePeriodInSeconds: e2eiConfig.config.verificationExpiration,
isFreshMLSSelfClient: freshMLSSelfClient,
});
}
return undefined;
};
2 changes: 1 addition & 1 deletion src/script/util/TimeUtil.ts
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,6 @@ export const formatDelayTime = (delayTimeInMS: number): string => {
const hours = Math.floor(delayTimeInMS / TIME_IN_MILLIS.HOUR);
return `${hours} hour${hours > 1 ? 's' : ''}`;
}
const minutes = Math.floor(delayTimeInMS / (TIME_IN_MILLIS.SECOND * TIME_IN_MILLIS.MINUTE));
const minutes = Math.floor(delayTimeInMS / TIME_IN_MILLIS.MINUTE);
return `${minutes} minute${minutes > 1 ? 's' : ''}`;
};
Loading