Skip to content

Commit

Permalink
fix: Adding iCloud connection timeout to ensure setup does not get stuck
Browse files Browse the repository at this point in the history
  • Loading branch information
@steilerDev
steilerDev committed Sep 7, 2023
1 parent 9e9ce2e commit afed26a
Show file tree
Hide file tree
Showing 7 changed files with 75 additions and 36 deletions.
25 changes: 24 additions & 1 deletion app/package-lock.json
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions app/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@
"croner": "^6.0.3",
"p-event": "^5.0.1",
"p-queue": "^7.2.0",
"p-timeout": "^6.1.2",
"tough-cookie": "^4.0.0"
}
}
4 changes: 4 additions & 0 deletions app/src/app/error/codes/icloud-auth.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,8 @@ export const ACQUIRE_ACCOUNT_TOKENS: ErrorStruct = buildErrorStruct(

export const ACCOUNT_SETUP: ErrorStruct = buildErrorStruct(
name, prefix, `ACCOUNT_SETUP`, `Unable to setup iCloud Account`,
);

export const SETUP_TIMEOUT: ErrorStruct = buildErrorStruct(
name, prefix, `SETUP_TIMEOUT`, `iCloud setup did not complete successfully within expected amount of time`,
);
30 changes: 15 additions & 15 deletions app/src/lib/icloud/icloud.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {ICLOUD_PHOTOS_ERR, MFA_ERR, AUTH_ERR} from '../../app/error/error-codes.
import {Resources} from '../resources/main.js';
import {ENDPOINTS} from '../resources/network-types.js';
import {iCPSEventCloud, iCPSEventMFA, iCPSEventPhotos, iCPSEventRuntimeWarning} from '../resources/events-types.js';
import pTimeout from 'p-timeout';

/**
* This class holds the iCloud connection
Expand All @@ -22,11 +23,6 @@ export class iCloud {
*/
photos: iCloudPhotos;

/**
* A promise that will resolve to true, if the connection was established successfully, false in case the MFA code was not provided in time or reject, in case there is an error
*/
ready: Promise<boolean>;

/**
* Creates a new iCloud Object
* @emits iCPSEventCloud.ERROR - If the MFA code is required and the failOnMfa flag is set - the iCPSError is provided as argument
Expand Down Expand Up @@ -62,22 +58,25 @@ export class iCloud {
.on(iCPSEventCloud.SESSION_EXPIRED, async () => {
await this.authenticate();
});

this.ready = this.getReady();
}

/**
*
* @returns A promise that will resolve to true, if the connection was established successfully, false in case the MFA code was not provided in time or reject, in case there is an error
*/
getReady(): Promise<boolean> {
return new Promise<boolean>((resolve, reject) => {
Resources.events(this)
.once(iCPSEventPhotos.READY, () => resolve(true))
.once(iCPSEventMFA.MFA_NOT_PROVIDED, () => resolve(false))
.once(iCPSEventCloud.ERROR, err => reject(err))
.once(iCPSEventMFA.ERROR, err => reject(err));
});
return pTimeout(
new Promise<boolean>((resolve, reject) => {
Resources.events(this)
.once(iCPSEventPhotos.READY, () => resolve(true))
.once(iCPSEventMFA.MFA_NOT_PROVIDED, () => resolve(false))
.once(iCPSEventCloud.ERROR, err => reject(err))
.once(iCPSEventMFA.ERROR, err => reject(err));
}), {
milliseconds: 1000 * 60 * 5, // 5 minutes should be sufficient
message: new iCPSError(AUTH_ERR.SETUP_TIMEOUT),
},
);
}

/**
Expand All @@ -88,6 +87,7 @@ export class iCloud {
* @emits iCPSEventCloud.ERROR - When an error occurs - provides iCPSError as argument
*/
async authenticate(): Promise<boolean> {
const ready = this.getReady();
Resources.logger(this).info(`Authenticating user`);
Resources.emit(iCPSEventCloud.AUTHENTICATION_STARTED);

Expand Down Expand Up @@ -159,7 +159,7 @@ export class iCloud {
Resources.emit(iCPSEventCloud.ERROR, new iCPSError(AUTH_ERR.UNKNOWN).addCause(err));
return;
} finally {
return this.ready;
return ready;
}
}

Expand Down
7 changes: 3 additions & 4 deletions app/src/lib/sync-engine/sync-engine.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export class SyncEngine {
}

/**
* Performs the sync and handles all connections
* Performs the sync and handles all connections and retries
* @returns A tuple consisting of assets and albums as fetched from the remote state. It can be assumed that this reflects the local state (given a warning free execution of the sync)
* @throws An iCPSError, in case the sync could not be completed within the amount of allowed retries
* @emits iCPSEventSyncEngine.START - When the sync starts
Expand All @@ -53,7 +53,6 @@ export class SyncEngine {

while (Resources.manager().maxRetries >= retryCount) {
Resources.logger(this).info(`Performing sync, try #${retryCount}`);

try {
const [remoteAssets, remoteAlbums, localAssets, localAlbums] = await this.fetchAndLoadState();
const [assetQueue, albumQueue] = await this.diffState(remoteAssets, remoteAlbums, localAssets, localAlbums);
Expand All @@ -71,9 +70,9 @@ export class SyncEngine {

await Resources.network().settleCCYLimiter();

Resources.logger(this).debug(`Refreshing iCloud cookies...`);
Resources.logger(this).debug(`Refreshing iCloud connection...`);
const iCloudReady = this.icloud.getReady();
await this.icloud.setupAccount();
this.icloud.setupAccount();
if (!await iCloudReady) {
return [[], []];
}
Expand Down
3 changes: 2 additions & 1 deletion app/test/unit/app.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,9 @@ describe(`App control flow`, () => {
tokenApp.acquireLibraryLock = jest.fn<typeof tokenApp.acquireLibraryLock>()
.mockResolvedValue();
tokenApp.icloud.authenticate = jest.fn<typeof tokenApp.icloud.authenticate>(() => {
const ready = tokenApp.icloud.getReady();
Resources.emit(iCPSEventCloud.TRUSTED);
return tokenApp.icloud.ready;
return ready;
});
tokenApp.releaseLibraryLock = jest.fn<typeof tokenApp.releaseLibraryLock>()
.mockResolvedValue();
Expand Down
41 changes: 26 additions & 15 deletions app/test/unit/icloud.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,27 +72,30 @@ describe(`Control structure`, () => {
});

test(`MFA Server Startup error`, async () => {
const iCloudReady = icloud.getReady()
mockedEventManager.emit(iCPSEventMFA.ERROR, new iCPSError(MFA_ERR.STARTUP_FAILED));

await expect(icloud.ready).rejects.toThrow(/^Unable to start MFA server$/);
await expect(iCloudReady).rejects.toThrow(/^Unable to start MFA server$/);
});

test(`Fail on MFA`, async () => {
mockedResourceManager._resources.failOnMfa = true;
const iCloudReady = icloud.getReady()

icloud.mfaServer.startServer = jest.fn<typeof icloud.mfaServer.startServer>();

mockedEventManager.emit(iCPSEventCloud.MFA_REQUIRED);

await expect(icloud.ready).rejects.toThrow(/^MFA code required, failing due to failOnMfa flag$/);
await expect(iCloudReady).rejects.toThrow(/^MFA code required, failing due to failOnMfa flag$/);

expect(icloud.mfaServer.startServer).not.toHaveBeenCalled();
});
});

test(`MFA_NOT_PROVIDED event triggered`, async () => {
const iCloudReady = icloud.getReady()
mockedEventManager.emit(iCPSEventMFA.MFA_NOT_PROVIDED, new iCPSError(MFA_ERR.SERVER_TIMEOUT));
await expect(icloud.ready).resolves.toBeFalsy();
await expect(iCloudReady).resolves.toBeFalsy();
});

test.each([
Expand All @@ -105,10 +108,10 @@ describe(`Control structure`, () => {
},
])(`$desc error event triggered`, async ({event}) => {
mockedEventManager._eventBus.removeAllListeners(event); // Not sure why this is necessary
icloud.ready = icloud.getReady();
const iCloudReady = icloud.getReady();

mockedEventManager.emit(event, new iCPSError());
await expect(icloud.ready).rejects.toThrow(/^Unknown error occurred$/);
await expect(iCloudReady).rejects.toThrow(/^Unknown error occurred$/);
});
});

Expand Down Expand Up @@ -136,7 +139,7 @@ describe.each([
describe(`Authenticate`, () => {
test(`Valid Trust Token`, async () => {
// ICloud.authenticate returns ready promise. Need to modify in order to resolve at the end of the test
icloud.ready = new Promise<boolean>((resolve, _reject) => resolve(true));
icloud.getReady = jest.fn<typeof icloud.getReady>().mockResolvedValue(true);

const authenticationEvent = mockedEventManager.spyOnEvent(iCPSEventCloud.AUTHENTICATION_STARTED);
const trustedEvent = mockedEventManager.spyOnEvent(iCPSEventCloud.TRUSTED);
Expand Down Expand Up @@ -173,7 +176,7 @@ describe.each([
});

// ICloud.authenticate returns ready promise. Need to modify in order to resolve at the end of the test
icloud.ready = new Promise<boolean>((resolve, _reject) => resolve(true));
icloud.getReady = jest.fn<typeof icloud.getReady>().mockResolvedValue(true);

const authenticationEvent = mockedEventManager.spyOnEvent(iCPSEventCloud.AUTHENTICATION_STARTED);
const mfaEvent = mockedEventManager.spyOnEvent(iCPSEventCloud.MFA_REQUIRED);
Expand Down Expand Up @@ -528,6 +531,7 @@ describe.each([
});

test(`Failure`, async () => {
const iCloudReady = icloud.getReady()
mockedNetworkManager._headerJar.setCookie(Config.aaspCookieString);
mockedNetworkManager.scnt = Config.iCloudAuthSecrets.scnt;
mockedNetworkManager.sessionId = Config.iCloudAuthSecrets.sessionSecret;
Expand All @@ -546,7 +550,7 @@ describe.each([

await icloud.submitMFA(new MFAMethod(method as any), `123456`);

await expect(icloud.ready).rejects.toThrow(/^Unable to submit MFA code$/);
await expect(iCloudReady).rejects.toThrow(/^Unable to submit MFA code$/);
});
});
});
Expand Down Expand Up @@ -580,6 +584,7 @@ describe.each([
});

test(`Error - Invalid Response`, async () => {
const iCloudReady = icloud.getReady()
mockedNetworkManager._headerJar.setCookie(Config.aaspCookieString);
mockedNetworkManager.scnt = Config.iCloudAuthSecrets.scnt;
mockedNetworkManager.sessionId = Config.iCloudAuthSecrets.sessionSecret;
Expand All @@ -598,12 +603,13 @@ describe.each([
.reply(204);

await icloud.getTokens();
await expect(icloud.ready).rejects.toThrow(/^Unable to acquire account tokens$/);
await expect(iCloudReady).rejects.toThrow(/^Unable to acquire account tokens$/);

expect(mockedValidator.validateTrustResponse).toHaveBeenCalled();
});

test(`Error - Invalid Status Code`, async () => {
const iCloudReady = icloud.getReady()
mockedNetworkManager._headerJar.setCookie(Config.aaspCookieString);
mockedNetworkManager.scnt = Config.iCloudAuthSecrets.scnt;
mockedNetworkManager.sessionId = Config.iCloudAuthSecrets.sessionSecret;
Expand All @@ -618,7 +624,7 @@ describe.each([
.reply(500);

await icloud.getTokens();
await expect(icloud.ready).rejects.toThrow(/^Unable to acquire account tokens$/);
await expect(iCloudReady).rejects.toThrow(/^Unable to acquire account tokens$/);
});
});

Expand Down Expand Up @@ -668,6 +674,7 @@ describe.each([
});

test(`Error - Invalid Response`, async () => {
const iCloudReady = icloud.getReady()
mockedNetworkManager.sessionToken = Config.iCloudAuthSecrets.sessionSecret;

mockedValidator.validateSetupResponse = jest.fn<typeof mockedValidator.validateSetupResponse>(() => {
Expand All @@ -679,20 +686,21 @@ describe.each([
.reply(200);

await icloud.setupAccount();
await expect(icloud.ready).rejects.toThrow(/^Unable to setup iCloud Account$/);
await expect(iCloudReady).rejects.toThrow(/^Unable to setup iCloud Account$/);

expect(mockedValidator.validateSetupResponse).toHaveBeenCalled();
});

test(`Error - Invalid Status Code`, async () => {
const iCloudReady = icloud.getReady()
mockedNetworkManager.sessionToken = Config.iCloudAuthSecrets.sessionSecret;

mockedNetworkManager.mock
.onAny()
.reply(500);

await icloud.setupAccount();
await expect(icloud.ready).rejects.toThrow(/^Unable to setup iCloud Account$/);
await expect(iCloudReady).rejects.toThrow(/^Unable to setup iCloud Account$/);
});

describe(`Get iCloud Photos Ready`, () => {
Expand All @@ -701,33 +709,36 @@ describe.each([
});

test(`Setup resolves`, async () => {
const iCloudReady = icloud.getReady()
icloud.photos.setup = jest.fn<typeof icloud.photos.setup>(() => {
Resources.emit(iCPSEventPhotos.READY);
return Promise.resolve();
});

await icloud.getPhotosReady();

await expect(icloud.ready).resolves.not.toThrow();
await expect(iCloudReady).resolves.not.toThrow();

expect(icloud.photos.setup).toHaveBeenCalled();
});

test(`Setup rejects`, async () => {
const iCloudReady = icloud.getReady()
icloud.photos.setup = jest.fn<typeof icloud.photos.setup>()
.mockRejectedValue(new Error());

await icloud.getPhotosReady();
await expect(icloud.ready).rejects.toThrow(/^Unable to get iCloud Photos service ready$/);
await expect(iCloudReady).rejects.toThrow(/^Unable to get iCloud Photos service ready$/);

expect(icloud.photos.setup).toHaveBeenCalled();
});

test(`Photos Object invalid`, async () => {
const iCloudReady = icloud.getReady()
icloud.photos = undefined as any;
await icloud.getPhotosReady();

await expect(icloud.ready).rejects.toThrow(/^Unable to get iCloud Photos service ready$/);
await expect(iCloudReady).rejects.toThrow(/^Unable to get iCloud Photos service ready$/);
});
});
});
Expand Down

0 comments on commit afed26a

Please sign in to comment.