Skip to content

Commit

Permalink
fix: Gracefully handling timed out MFA code
Browse files Browse the repository at this point in the history
  • Loading branch information
Frank Steiler committed Aug 29, 2023
1 parent 32a7e96 commit 7e7d156
Show file tree
Hide file tree
Showing 14 changed files with 186 additions and 52 deletions.
3 changes: 1 addition & 2 deletions .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@
"--experimental-vm-modules",
"node_modules/.bin/jest",
"--config", "jest.config.json",
//"test/unit/.*.test.ts"
],
"console": "integratedTerminal",
"internalConsoleOptions": "neverOpen",
Expand All @@ -66,7 +65,7 @@
"--runInBand",
"--config", "jest.config.json",
//"--detectOpenHandles",
"test/unit/resources.resource-manager.test.ts"
"test/unit/icloud.test.ts"
],
"env": {
"NODE_NO_WARNINGS": "1"
Expand Down
11 changes: 1 addition & 10 deletions app/jest.config.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"preset": "ts-jest/presets/default-esm-legacy",
"preset": "ts-jest/presets/default-esm",
"testEnvironment": "node",
"slowTestThreshold": 30,
"reporters": [
Expand All @@ -20,15 +20,6 @@
"html",
"json-summary"
],
"extensionsToTreatAsEsm": [".ts"],
"transform": {
"\\.ts$": [
"ts-jest",
{
"useESM": true
}
]
},
"moduleNameMapper": {
"^(\\.{1,2}/.*)\\.js$": "$1"
}
Expand Down
4 changes: 4 additions & 0 deletions app/src/app/error/codes/archive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ import {buildErrorStruct, ErrorStruct} from "../error-codes.js";
const name = `ArchiveError`;
const prefix = `ARCHIVE`;

export const NO_ASSETS: ErrorStruct = buildErrorStruct(
name, prefix, `NO_ASSETS`, `No remote assets available`,
);

export const UUID_PATH: ErrorStruct = buildErrorStruct(
name, prefix, `UUID_PATH`, `UUID path selected, use named path only`,
);
Expand Down
5 changes: 3 additions & 2 deletions app/src/app/error/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export class iCPSError extends Error {
/**
* If this error was reported, it will receive a UUID for future reference
*/
btUUID: string = undefined;
btUUID?: string = undefined;

/**
* Creates an application specific error using the provided
Expand Down Expand Up @@ -121,7 +121,8 @@ export class iCPSError extends Error {
* @returns the error code for the first thrown error
*/
getRootErrorCode(onlyICPSError: boolean = false): string {
return this.getErrorCodeStack(onlyICPSError).pop();
// getErrorCodeStack returns at least one item
return this.getErrorCodeStack(onlyICPSError).pop()!;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion app/src/app/event/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ export class CLIInterface {
this.print(chalk.white(`Resending MFA code via ${method.toString()}...`));
})
.on(iCPSEventMFA.MFA_RECEIVED, (method: MFAMethod, code: string) => {
this.print(chalk.white(`MFA code received via ${method.toString()} (${code})`));
this.print(chalk.white(`MFA code received from ${method.toString()} (${code})`));
})
.on(iCPSEventMFA.MFA_NOT_PROVIDED, () => {
this.print(chalk.yellowBright(`MFA code not provided in time, aborting...`));
Expand Down
6 changes: 6 additions & 0 deletions app/src/app/event/error-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,9 @@ export class ErrorHandler {
}

async handleFiletype(_ext: string, _descriptor?: string) {
if(this.btClient === undefined){
return
}
const report = new bt.BacktraceReport(new iCPSError(FILETYPE_REPORT),
{
'icps.filetype.extension': _ext,
Expand All @@ -140,6 +143,9 @@ export class ErrorHandler {
* Registers event listeners to provide breadcrumbs
*/
registerBreadcrumbs() {
if (this.btClient === undefined || this.btClient.breadcrumbs === undefined ){
return;
}
Resources.events(this)
.on(iCPSEventRuntimeWarning.MFA_ERROR, (err: Error) => {
this.btClient.breadcrumbs.warn(`MFA_ERROR`, {error: iCPSError.toiCPSError(err).getDescription()});
Expand Down
22 changes: 13 additions & 9 deletions app/src/app/icloud-app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,11 @@ export class DaemonApp extends iCPSApp {
async performScheduledSync(syncApp: SyncApp = new SyncApp()) {
try {
Resources.emit(iCPSEventApp.SCHEDULED_START);
await syncApp.run();
Resources.emit(iCPSEventApp.SCHEDULED_DONE, this.job?.nextRun());
const [remoteAssets] = await syncApp.run() as [Asset[], Album[]];

if (remoteAssets.length > 0) {
Resources.emit(iCPSEventApp.SCHEDULED_DONE, this.job?.nextRun());
}
} catch (err) {
Resources.emit(iCPSEventRuntimeError.SCHEDULED_ERROR, new iCPSError(APP_ERR.DAEMON).addCause(err));
Resources.emit(iCPSEventApp.SCHEDULED_RETRY, this.job?.nextRun());
Expand Down Expand Up @@ -89,7 +92,7 @@ abstract class iCloudApp extends iCPSApp {
/**
* This function acquires the library lock and establishes the iCloud connection.
* @param eventHandlers - A list of EventHandlers that will be registering relevant objects
* @returns A promise that resolves once the iCloud service is fully available
* @returns A promise that resolves to true once the iCloud service is fully available. If it resolves to false, the MFA code was not provided in time and the object is not ready.
* @throws A iCPSError in case an error occurs
*/
async run(): Promise<unknown> {
Expand Down Expand Up @@ -177,7 +180,7 @@ export class TokenApp extends iCloudApp {
// Emitting event for CLI
Resources.emit(iCPSEventApp.TOKEN, token);
});
await super.run();
return await super.run();
} catch (err) {
throw new iCPSError(APP_ERR.TOKEN)
.addCause(err);
Expand All @@ -187,9 +190,6 @@ export class TokenApp extends iCloudApp {
await this.clean();
}
}

// Has to return something (TS2355)
return true;
}

/**
Expand Down Expand Up @@ -227,12 +227,16 @@ export class SyncApp extends iCloudApp {
/**
* Runs the synchronization of the local Photo Library
* @param eventHandlers - A list of EventHandlers that will be registering relevant objects
* @returns A Promise that resolves to a tuple containing a list of assets as fetched from the remote state. It can be assumed that this reflects the local state (given a warning free execution of the sync).
* @returns A Promise that resolves to a tuple containing a list of assets and albums as fetched from the remote state. The returned arrays might be empty, if the iCloud connection was not established successfully.
* @throws A SyncError in case an error occurs
*/
async run(): Promise<unknown> {
try {
await super.run();
const ready = await super.run() as boolean;
if (!ready) {
return [[], []];
}

return await this.syncEngine.sync();
} catch (err) {
throw new iCPSError(APP_ERR.SYNC)
Expand Down
4 changes: 4 additions & 0 deletions app/src/lib/archive-engine/archive-engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ export class ArchiveEngine {
Resources.logger(this).debug(`Archiving path ${archivePath}`);
Resources.emit(iCPSEventArchiveEngine.ARCHIVE_START, archivePath);

if (assetList.length === 0) {
throw new iCPSError(ARCHIVE_ERR.NO_ASSETS);
}

const albumName = path.basename(archivePath);
if (albumName.startsWith(`.`)) {
throw new iCPSError(ARCHIVE_ERR.UUID_PATH);
Expand Down
22 changes: 12 additions & 10 deletions app/src/lib/icloud/icloud.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {AxiosRequestConfig} from 'axios';
import {AxiosError, AxiosRequestConfig} from 'axios';
import {MFAServer} from './mfa/mfa-server.js';
import {iCloudPhotos} from './icloud-photos/icloud-photos.js';
import {MFAMethod} from './mfa/mfa-method.js';
Expand All @@ -24,9 +24,9 @@ export class iCloud {
photos: iCloudPhotos;

/**
* A promise that will resolve, once the object is ready or reject, in case there is an error
* 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<void>;
ready: Promise<boolean>;

/**
* Creates a new iCloud Object
Expand Down Expand Up @@ -66,14 +66,14 @@ export class iCloud {

/**
*
* @returns - A promise, that will resolve once this objects emits 'READY' or reject if it emits 'ERROR' or the MFA server times out
* @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<void> {
return new Promise<void>((resolve, reject) => {
getReady(): Promise<boolean> {
return new Promise<boolean>((resolve, reject) => {
Resources.events(this)
.once(iCPSEventPhotos.READY, () => resolve())
.once(iCPSEventPhotos.READY, () => resolve(true))
.once(iCPSEventMFA.MFA_NOT_PROVIDED, err => resolve(false))
.once(iCPSEventCloud.ERROR, err => reject(err))
.once(iCPSEventMFA.MFA_NOT_PROVIDED, err => reject(err))
.once(iCPSEventMFA.ERROR, err => reject(err));
});
}
Expand All @@ -82,7 +82,7 @@ export class iCloud {
* Initiates authentication flow
* Tries to directly login using trustToken, otherwise starts MFA flow
*/
async authenticate(): Promise<void> {
async authenticate(): Promise<boolean> {
Resources.logger(this).info(`Authenticating user`);
Resources.emit(iCPSEventCloud.AUTHENTICATION_STARTED);

Expand Down Expand Up @@ -131,7 +131,9 @@ export class iCloud {
return;
}

if (err?.response?.status) {
// Does not seem to work
//if (err instanceof AxiosError) {
if ((err as AxiosError).isAxiosError) {
switch (err.response.status) {
case 401:
Resources.emit(iCPSEventCloud.ERROR, new iCPSError(AUTH_ERR.UNAUTHORIZED).addCause(err));
Expand Down
6 changes: 4 additions & 2 deletions app/src/lib/sync-engine/sync-engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export class SyncEngine {

/**
* Performs the sync and handles all connections
* @returns A list of assets as fetched from the remote state. It can be assumed that this reflects the local state (given a warning free execution of the sync)
* @returns A list 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)
*/
async sync(): Promise<[Asset[], Album[]]> {
Resources.logger(this).info(`Starting sync`);
Expand Down Expand Up @@ -70,7 +70,9 @@ export class SyncEngine {
Resources.logger(this).debug(`Refreshing iCloud cookies...`);
const iCloudReady = this.icloud.getReady();
await this.icloud.setupAccount();
await iCloudReady;
if (!await iCloudReady) {
return [[], []];
}
}
}

Expand Down
Loading

0 comments on commit 7e7d156

Please sign in to comment.