From f60429cab554da0ef455c01e1ed660e300efaa73 Mon Sep 17 00:00:00 2001 From: Pascal Hartig Date: Fri, 20 Aug 2021 03:51:33 -0700 Subject: [PATCH] Small refactors in CertificateProvider Summary: - Remove `fs` dependency in favour of `fs-extra`. - Replaced `Sync` variants with async wherever possible. - Removed some unnecessary Promise constructions. Reviewed By: timur-valiev Differential Revision: D30411434 fbshipit-source-id: 9faebbc1f9fb2283fec895ce3397918bc85a6c51 --- .../src/server/utils/CertificateProvider.tsx | 101 +++++++++--------- 1 file changed, 48 insertions(+), 53 deletions(-) diff --git a/desktop/app/src/server/utils/CertificateProvider.tsx b/desktop/app/src/server/utils/CertificateProvider.tsx index 01999804d..1be86be70 100644 --- a/desktop/app/src/server/utils/CertificateProvider.tsx +++ b/desktop/app/src/server/utils/CertificateProvider.tsx @@ -187,7 +187,7 @@ export default class CertificateProvider { .then(async (deviceId) => { if (medium === 'WWW') { const zipPromise = new Promise((resolve, reject) => { - const output = fs.createWriteStream(certsZipPath); + const output = fsExtra.createWriteStream(certsZipPath); const archive = archiver('zip', { zlib: {level: 9}, // Sets the compression level. }); @@ -292,13 +292,11 @@ export default class CertificateProvider { if (!certPathExists) { await fsExtra.mkdir(certFolder); } - return promisify(fs.writeFile)(certFolder + filename, contents).catch( - (e) => { - throw new Error( - `Failed to write ${filename} to temporary folder. Error: ${e}`, - ); - }, - ); + return fsExtra.writeFile(certFolder + filename, contents).catch((e) => { + throw new Error( + `Failed to write ${filename} to temporary folder. Error: ${e}`, + ); + }); } if (os === 'Android') { @@ -317,8 +315,9 @@ export default class CertificateProvider { ); } if (os === 'iOS' || os === 'windows' || os == 'MacOS') { - return promisify(fs.writeFile)(destination + filename, contents).catch( - async (err) => { + return fsExtra + .writeFile(destination + filename, contents) + .catch(async (err) => { if (os === 'iOS') { // Writing directly to FS failed. It's probably a physical device. const relativePathInsideApp = @@ -341,31 +340,28 @@ export default class CertificateProvider { `Invalid appDirectory recieved from ${os} device: ${destination}: ` + err.toString(), ); - }, - ); + }); } return Promise.reject(new Error(`Unsupported device os: ${os}`)); } - private pushFileToiOSDevice( + private async pushFileToiOSDevice( udid: string, bundleId: string, destination: string, filename: string, contents: string, ): Promise { - return tmpDir({unsafeCleanup: true}).then((dir) => { - const filePath = path.resolve(dir, filename); - promisify(fs.writeFile)(filePath, contents).then(() => - iosUtil.push( - udid, - filePath, - bundleId, - destination, - this.config.idbPath, - ), - ); - }); + const dir = await tmpDir({unsafeCleanup: true}); + const filePath = path.resolve(dir, filename); + await fsExtra.writeFile(filePath, contents); + return await iosUtil.push( + udid, + filePath, + bundleId, + destination, + this.config.idbPath, + ); } private getTargetAndroidDeviceId( @@ -512,7 +508,8 @@ export default class CertificateProvider { .then(() => dir); }) .then((dir) => { - return promisify(fs.readdir)(dir) + return fsExtra + .readdir(dir) .then((items) => { if (items.length > 1) { throw new Error('Conflict in temp dir'); @@ -524,9 +521,9 @@ export default class CertificateProvider { }) .then((fileName) => { const copiedFile = path.resolve(dir, fileName); - return promisify(fs.readFile)(copiedFile).then((data) => - this.santitizeString(data.toString()), - ); + return fsExtra + .readFile(copiedFile) + .then((data) => this.santitizeString(data.toString())); }); }) .then((csrFromDevice) => csrFromDevice === this.santitizeString(csr)); @@ -577,20 +574,19 @@ export default class CertificateProvider { }); } - loadSecureServerConfig(): Promise { - return this.certificateSetup.then(() => { - return { - key: fs.readFileSync(serverKey), - cert: fs.readFileSync(serverCert), - ca: fs.readFileSync(caCert), - requestCert: true, - rejectUnauthorized: true, // can be false if necessary as we don't strictly need to verify the client - }; - }); + async loadSecureServerConfig(): Promise { + await this.certificateSetup; + return { + key: await fsExtra.readFile(serverKey), + cert: await fsExtra.readFile(serverCert), + ca: await fsExtra.readFile(caCert), + requestCert: true, + rejectUnauthorized: true, // can be false if necessary as we don't strictly need to verify the client + }; } async ensureCertificateAuthorityExists(): Promise { - if (!fs.existsSync(caKey)) { + if (!(await fsExtra.pathExists(caKey))) { return this.generateCertificateAuthority(); } return this.checkCertIsValid(caCert).catch(() => @@ -598,8 +594,8 @@ export default class CertificateProvider { ); } - private checkCertIsValid(filename: string): Promise { - if (!fs.existsSync(filename)) { + private async checkCertIsValid(filename: string): Promise { + if (!(await fsExtra.pathExists(filename))) { return Promise.reject(new Error(`${filename} does not exist`)); } // openssl checkend is a nice feature but it only checks for certificates @@ -655,9 +651,9 @@ export default class CertificateProvider { }); } - private generateCertificateAuthority(): Promise { - if (!fs.existsSync(getFilePath(''))) { - fs.mkdirSync(getFilePath('')); + private async generateCertificateAuthority(): Promise { + if (!(await fsExtra.pathExists(getFilePath('')))) { + await fsExtra.mkdir(getFilePath('')); } console.log('Generating new CA', logTag); return openssl('genrsa', {out: caKey, '2048': false}) @@ -674,13 +670,12 @@ export default class CertificateProvider { } private async ensureServerCertExists(): Promise { - if ( - !( - fs.existsSync(serverKey) && - fs.existsSync(serverCert) && - fs.existsSync(caCert) - ) - ) { + const allExist = Promise.all([ + fsExtra.existsSync(serverKey), + fsExtra.existsSync(serverCert), + fsExtra.existsSync(caCert), + ]).then((exist) => exist.every(Boolean)); + if (!allExist) { return this.generateServerCertificate(); } @@ -719,7 +714,7 @@ export default class CertificateProvider { private writeToTempFile(content: string): Promise { return tmpFile().then((path) => - promisify(fs.writeFile)(path, content).then((_) => path), + fsExtra.writeFile(path, content).then((_) => path), ); } }