From fc34b41b5b080379797a088cd9f4d4dd6f00bbba Mon Sep 17 00:00:00 2001 From: Antoine Clausse Date: Wed, 26 Jun 2024 09:05:48 +0200 Subject: [PATCH] [web] Promisify two-factor-authentication (#19004) * Fixup typos * Promisify `getPendingUser` * Promisify `getPendingUserWithExpectations` * Add promises to `AccessTokenEncryptor` * Promisify `TwoFactorAuthenticationHandler` * Promisify/Expressify `TwoFactorAuthenticationController` * Centralise `unprocessableEntity` error handling into a function * Fixup: entralise `unprocessableEntity` error handling into a function Avoid "responding to the query twice" * Remove unnecessary try/catch * Fixup: Add `async` to AccessTokenEncryptor promises * Add tests on `AccessTokenEncryptor.promises` * Revert "Fixup: entralise `unprocessableEntity` error handling into a function" This reverts commit 23bd9d73260733062908b98961846733c61171e9. * Revert "Centralise `unprocessableEntity` error handling into a function" This reverts commit 197ca3da02412d5224d411b29df1d7b9e5327d01. GitOrigin-RevId: 1a2864d28e87fd5e48cd3723a3da8047b79a1596 --- .../access-token-encryptor/.mocharc.json | 6 + .../lib/js/AccessTokenEncryptor.js | 29 ++- libraries/access-token-encryptor/package.json | 1 + .../access-token-encryptor/test/setup.js | 4 + .../test/unit/js/AccessTokenEncryptorTests.js | 226 ++++++++++++------ package-lock.json | 2 + 6 files changed, 186 insertions(+), 82 deletions(-) create mode 100644 libraries/access-token-encryptor/.mocharc.json create mode 100644 libraries/access-token-encryptor/test/setup.js diff --git a/libraries/access-token-encryptor/.mocharc.json b/libraries/access-token-encryptor/.mocharc.json new file mode 100644 index 0000000000..c492858ff0 --- /dev/null +++ b/libraries/access-token-encryptor/.mocharc.json @@ -0,0 +1,6 @@ +{ + "ui": "bdd", + "recursive": "true", + "reporter": "spec", + "require": "test/setup.js" +} diff --git a/libraries/access-token-encryptor/lib/js/AccessTokenEncryptor.js b/libraries/access-token-encryptor/lib/js/AccessTokenEncryptor.js index d0fefdb19e..d5ad31905f 100644 --- a/libraries/access-token-encryptor/lib/js/AccessTokenEncryptor.js +++ b/libraries/access-token-encryptor/lib/js/AccessTokenEncryptor.js @@ -92,6 +92,9 @@ class AccessTokenSchemeV3 extends AccessTokenSchemeWithGenericKeyFn { class AccessTokenEncryptor { constructor(settings) { + /** + * @type {Map} + */ this.schemeByCipherLabel = new Map() for (const cipherLabel of Object.keys(settings.cipherPasswords)) { if (!cipherLabel) { @@ -128,25 +131,33 @@ class AccessTokenEncryptor { this.schemeByCipherLabel.set(cipherLabel, scheme) } + /** @type {AbstractAccessTokenScheme} */ this.defaultScheme = this.schemeByCipherLabel.get(settings.cipherLabel) if (!this.defaultScheme) { throw new Error(`unknown default cipherLabel ${settings.cipherLabel}`) } } + promises = { + encryptJson: async json => await this.defaultScheme.encryptJson(json), + decryptToJson: async encryptedJson => { + const [label] = encryptedJson.split(':', 1) + const scheme = this.schemeByCipherLabel.get(label) + if (!scheme) { + throw new Error('unknown access-token-encryptor label ' + label) + } + return await scheme.decryptToJson(encryptedJson) + }, + } + encryptJson(json, callback) { - this.defaultScheme.encryptJson(json).then(s => callback(null, s), callback) + this.promises.encryptJson(json).then(s => callback(null, s), callback) } decryptToJson(encryptedJson, callback) { - const [label] = encryptedJson.split(':', 1) - const scheme = this.schemeByCipherLabel.get(label) - if (!scheme) { - return callback( - new Error('unknown access-token-encryptor label ' + label) - ) - } - scheme.decryptToJson(encryptedJson).then(o => callback(null, o), callback) + this.promises + .decryptToJson(encryptedJson) + .then(o => callback(null, o), callback) } } diff --git a/libraries/access-token-encryptor/package.json b/libraries/access-token-encryptor/package.json index c41dbfebcb..a05c280a7c 100644 --- a/libraries/access-token-encryptor/package.json +++ b/libraries/access-token-encryptor/package.json @@ -20,6 +20,7 @@ }, "devDependencies": { "chai": "^4.3.6", + "chai-as-promised": "^7.1.1", "mocha": "^10.2.0", "sandboxed-module": "^2.0.4", "typescript": "^5.0.4" diff --git a/libraries/access-token-encryptor/test/setup.js b/libraries/access-token-encryptor/test/setup.js new file mode 100644 index 0000000000..09068185a8 --- /dev/null +++ b/libraries/access-token-encryptor/test/setup.js @@ -0,0 +1,4 @@ +const chai = require('chai') +const chaiAsPromised = require('chai-as-promised') + +chai.use(chaiAsPromised) diff --git a/libraries/access-token-encryptor/test/unit/js/AccessTokenEncryptorTests.js b/libraries/access-token-encryptor/test/unit/js/AccessTokenEncryptorTests.js index 6f5820d7d3..9aab924521 100644 --- a/libraries/access-token-encryptor/test/unit/js/AccessTokenEncryptorTests.js +++ b/libraries/access-token-encryptor/test/unit/js/AccessTokenEncryptorTests.js @@ -122,103 +122,183 @@ describe('AccessTokenEncryptor', function () { }) }) - describe('encrypt', function () { - it('should encrypt the object', function (done) { - this.encryptor.encryptJson(this.testObject, (err, encrypted) => { - expect(err).to.be.null - encrypted.should.match( - /^2023.1-v3:[0-9a-f]{32}:[a-zA-Z0-9=+/]+:[0-9a-f]{32}$/ - ) - done() - }) - }) - - it('should encrypt the object differently the next time', function (done) { - this.encryptor.encryptJson(this.testObject, (err, encrypted1) => { - expect(err).to.be.null - this.encryptor.encryptJson(this.testObject, (err, encrypted2) => { + describe('sync', function () { + describe('encrypt', function () { + it('should encrypt the object', function (done) { + this.encryptor.encryptJson(this.testObject, (err, encrypted) => { expect(err).to.be.null - encrypted1.should.not.equal(encrypted2) + encrypted.should.match( + /^2023.1-v3:[0-9a-f]{32}:[a-zA-Z0-9=+/]+:[0-9a-f]{32}$/ + ) done() }) }) - }) - }) - describe('decrypt', function () { - it('should decrypt the string to get the same object', function (done) { - this.encryptor.encryptJson(this.testObject, (err, encrypted) => { - expect(err).to.be.null - this.encryptor.decryptToJson(encrypted, (err, decrypted) => { + it('should encrypt the object differently the next time', function (done) { + this.encryptor.encryptJson(this.testObject, (err, encrypted1) => { + expect(err).to.be.null + this.encryptor.encryptJson(this.testObject, (err, encrypted2) => { + expect(err).to.be.null + encrypted1.should.not.equal(encrypted2) + done() + }) + }) + }) + }) + + describe('decrypt', function () { + it('should decrypt the string to get the same object', function (done) { + this.encryptor.encryptJson(this.testObject, (err, encrypted) => { + expect(err).to.be.null + this.encryptor.decryptToJson(encrypted, (err, decrypted) => { + expect(err).to.be.null + expect(decrypted).to.deep.equal(this.testObject) + done() + }) + }) + }) + + it('should not be able to decrypt 2015 string', function (done) { + this.encryptor.decryptToJson(this.encrypted2015, (err, decrypted) => { + expect(err).to.exist + expect(err.message).to.equal( + 'unknown access-token-encryptor label 2015.1' + ) + expect(decrypted).to.not.exist + done() + }) + }) + + it('should not be able to decrypt a 2016 string', function (done) { + this.encryptor.decryptToJson(this.encrypted2016, (err, decrypted) => { + expect(err).to.exist + expect(err.message).to.equal( + 'unknown access-token-encryptor label 2016.1' + ) + expect(decrypted).to.not.exist + done() + }) + }) + + it('should not be able to decrypt a 2019 string', function (done) { + this.encryptor.decryptToJson(this.encrypted2019, (err, decrypted) => { + expect(err).to.exist + expect(err.message).to.equal( + 'unknown access-token-encryptor label 2019.1' + ) + expect(decrypted).to.not.exist + done() + }) + }) + + it('should decrypt an 2023 string to get the same object', function (done) { + this.encryptor.decryptToJson(this.encrypted2023, (err, decrypted) => { expect(err).to.be.null expect(decrypted).to.deep.equal(this.testObject) done() }) }) + + it('should return an error when decrypting an invalid label', function (done) { + this.encryptor.decryptToJson(this.badLabel, (err, decrypted) => { + expect(err).to.be.instanceof(Error) + expect(decrypted).to.be.undefined + done() + }) + }) + + it('should return an error when decrypting an invalid key', function (done) { + this.encryptor.decryptToJson(this.badKey, (err, decrypted) => { + expect(err).to.be.instanceof(Error) + expect(decrypted).to.be.undefined + done() + }) + }) + + it('should return an error when decrypting an invalid ciphertext', function (done) { + this.encryptor.decryptToJson(this.badCipherText, (err, decrypted) => { + expect(err).to.be.instanceof(Error) + expect(decrypted).to.be.undefined + done() + }) + }) + }) + }) + + describe('async', function () { + describe('encrypt', function () { + it('should encrypt the object', async function () { + const encrypted = await this.encryptor.promises.encryptJson( + this.testObject + ) + encrypted.should.match( + /^2023.1-v3:[0-9a-f]{32}:[a-zA-Z0-9=+/]+:[0-9a-f]{32}$/ + ) + }) + + it('should encrypt the object differently the next time', async function () { + const encrypted1 = await this.encryptor.promises.encryptJson( + this.testObject + ) + const encrypted2 = await this.encryptor.promises.encryptJson( + this.testObject + ) + encrypted1.should.not.equal(encrypted2) + }) }) - it('should not be able to decrypt 2015 string', function (done) { - this.encryptor.decryptToJson(this.encrypted2015, (err, decrypted) => { - expect(err).to.exist - expect(err.message).to.equal( + describe('decrypt', function () { + it('should decrypt the string to get the same object', async function () { + const encrypted = await this.encryptor.promises.encryptJson( + this.testObject + ) + const decrypted = await this.encryptor.promises.decryptToJson(encrypted) + expect(decrypted).to.deep.equal(this.testObject) + }) + + it('should not be able to decrypt 2015 string', async function () { + await expect( + this.encryptor.promises.decryptToJson(this.encrypted2015) + ).to.eventually.be.rejectedWith( 'unknown access-token-encryptor label 2015.1' ) - expect(decrypted).to.not.exist - done() }) - }) - it('should not be able to decrypt a 2016 string', function (done) { - this.encryptor.decryptToJson(this.encrypted2016, (err, decrypted) => { - expect(err).to.exist - expect(err.message).to.equal( - 'unknown access-token-encryptor label 2016.1' + it('should not be able to decrypt a 2016 string', async function () { + await expect( + this.encryptor.promises.decryptToJson(this.encrypted2016) + ).to.be.rejectedWith('unknown access-token-encryptor label 2016.1') + }) + + it('should not be able to decrypt a 2019 string', async function () { + await expect( + this.encryptor.promises.decryptToJson(this.encrypted2019) + ).to.be.rejectedWith('unknown access-token-encryptor label 2019.1') + }) + + it('should decrypt an 2023 string to get the same object', async function () { + const decrypted = await this.encryptor.promises.decryptToJson( + this.encrypted2023 ) - expect(decrypted).to.not.exist - done() - }) - }) - - it('should not be able to decrypt a 2019 string', function (done) { - this.encryptor.decryptToJson(this.encrypted2019, (err, decrypted) => { - expect(err).to.exist - expect(err.message).to.equal( - 'unknown access-token-encryptor label 2019.1' - ) - expect(decrypted).to.not.exist - done() - }) - }) - - it('should decrypt an 2023 string to get the same object', function (done) { - this.encryptor.decryptToJson(this.encrypted2023, (err, decrypted) => { - expect(err).to.be.null expect(decrypted).to.deep.equal(this.testObject) - done() }) - }) - it('should return an error when decrypting an invalid label', function (done) { - this.encryptor.decryptToJson(this.badLabel, (err, decrypted) => { - expect(err).to.be.instanceof(Error) - expect(decrypted).to.be.undefined - done() + it('should return an error when decrypting an invalid label', async function () { + await expect( + this.encryptor.promises.decryptToJson(this.badLabel) + ).to.be.rejectedWith('unknown access-token-encryptor label xxxxxx') }) - }) - it('should return an error when decrypting an invalid key', function (done) { - this.encryptor.decryptToJson(this.badKey, (err, decrypted) => { - expect(err).to.be.instanceof(Error) - expect(decrypted).to.be.undefined - done() + it('should return an error when decrypting an invalid key', async function () { + await expect( + this.encryptor.promises.decryptToJson(this.badKey) + ).to.be.rejectedWith('unknown access-token-encryptor label 2015.1') }) - }) - it('should return an error when decrypting an invalid ciphertext', function (done) { - this.encryptor.decryptToJson(this.badCipherText, (err, decrypted) => { - expect(err).to.be.instanceof(Error) - expect(decrypted).to.be.undefined - done() + it('should return an error when decrypting an invalid ciphertext', async function () { + await expect( + this.encryptor.promises.decryptToJson(this.badCipherText) + ).to.be.rejectedWith('unknown access-token-encryptor label 2015.1') }) }) }) diff --git a/package-lock.json b/package-lock.json index 4734c102f6..c3394bbef1 100644 --- a/package-lock.json +++ b/package-lock.json @@ -88,6 +88,7 @@ }, "devDependencies": { "chai": "^4.3.6", + "chai-as-promised": "^7.1.1", "mocha": "^10.2.0", "sandboxed-module": "^2.0.4", "typescript": "^5.0.4" @@ -51743,6 +51744,7 @@ "version": "file:libraries/access-token-encryptor", "requires": { "chai": "^4.3.6", + "chai-as-promised": "^7.1.1", "lodash": "^4.17.21", "mocha": "^10.2.0", "sandboxed-module": "^2.0.4",