fix(core): OTP please method had non-strict code

- Lerna original code had non-strict code on null/undefined while it was strict in this lib, this commit fixes this

packages/core/package.json

@@ -91,6 +91,7 @@
     "@types/fs-extra": "^9.0.13",
     "@types/git-url-parse": "^9.0.1",
     "@types/glob-parent": "^5.1.1",
+    "@types/inquirer": "^8.2.0",
     "@types/load-json-file": "^5.1.0",
     "@types/node": "^17.0.8",
     "@types/npm-package-arg": "^6.1.1",

packages/core/src/__mocks__/prompt.js

@@ -0,0 +1,37 @@
+"use strict";
+
+let choiceIndices = [];
+afterEach(() => {
+  choiceIndices = [];
+});
+
+const mockConfirm = jest.fn(() => Promise.resolve(true));
+const mockSelect = jest.fn((_, { choices }) => {
+  // default selection is "patch"
+  const idx = choiceIndices.shift() || 0;
+
+  // each choice => { value: '<semver>', name: '<desc>' }
+  return Promise.resolve(choices[idx].value);
+});
+const mockInput = jest.fn(() => Promise.resolve());
+
+exports.promptConfirmation = mockConfirm;
+exports.promptSelectOne = mockSelect;
+exports.promptTextInput = mockInput;
+
+const semverIndex = new Map(
+  [
+    "patch",
+    "minor",
+    "major",
+    "prepatch",
+    "preminor",
+    "premajor",
+    "PRERELEASE",
+    "CUSTOM",
+  ].map((keyword, idx) => [keyword, idx])
+);
+
+mockSelect.chooseBump = (keyword) => {
+  choiceIndices.push(semverIndex.get(keyword));
+};

packages/core/src/__tests__/otplease.spec.ts

@@ -0,0 +1,213 @@
+
+// mocked modules
+const promptModule = require('../prompt');
+jest.mock('../prompt', () => jest.requireActual('../__mocks__/prompt'));
+
+// file under test
+const { otplease, getOneTimePassword } = require('../otplease');
+
+// global mock setup
+promptModule.promptTextInput.mockResolvedValue('123456');
+
+describe('@lerna/otplease', () => {
+  const stdinIsTTY = process.stdin.isTTY;
+  const stdoutIsTTY = process.stdout.isTTY;
+
+  beforeEach(() => {
+    process.stdin.isTTY = true;
+    process.stdout.isTTY = true;
+  });
+
+  afterEach(() => {
+    process.stdin.isTTY = stdinIsTTY;
+    process.stdout.isTTY = stdoutIsTTY;
+  });
+
+  it('no error', async () => {
+    const obj = {};
+    const fn = jest.fn(() => obj);
+    const result = await otplease(fn, {});
+
+    expect(fn).toHaveBeenCalled();
+    expect(promptModule.promptTextInput).not.toHaveBeenCalled();
+    expect(result).toBe(obj);
+  });
+
+  it('request otp', async () => {
+    const obj = {};
+    const fn = jest.fn(makeTestCallback('123456', obj));
+    const result = await otplease(fn, {});
+
+    expect(fn).toHaveBeenCalledTimes(2);
+    expect(promptModule.promptTextInput).toHaveBeenCalled();
+    expect(result).toBe(obj);
+  });
+
+  it('request otp updates cache', async () => {
+    const otpCache = { otp: undefined };
+    const obj = {};
+    const fn = jest.fn(makeTestCallback('123456', obj));
+
+    const result = await otplease(fn, {}, otpCache);
+    expect(fn).toHaveBeenCalledTimes(2);
+    expect(promptModule.promptTextInput).toHaveBeenCalled();
+    expect(result).toBe(obj);
+    expect(otpCache.otp).toBe('123456');
+  });
+
+  it('uses cache if opts does not have own otp', async () => {
+    const otpCache = { otp: '654321' };
+    const obj = {};
+    const fn = jest.fn(makeTestCallback('654321', obj));
+    const result = await otplease(fn, {}, otpCache);
+
+    expect(fn).toHaveBeenCalledTimes(1);
+    expect(promptModule.promptTextInput).not.toHaveBeenCalled();
+    expect(result).toBe(obj);
+    expect(otpCache.otp).toBe('654321');
+  });
+
+  it('uses explicit otp regardless of cache value', async () => {
+    const otpCache = { otp: '654321' };
+    const obj = {};
+    const fn = jest.fn(makeTestCallback('987654', obj));
+    const result = await otplease(fn, { otp: '987654' }, otpCache);
+
+    expect(fn).toHaveBeenCalledTimes(1);
+    expect(promptModule.promptTextInput).not.toHaveBeenCalled();
+    expect(result).toBe(obj);
+    // do not replace cache
+    expect(otpCache.otp).toBe('654321');
+  });
+
+  it('using cache updated in a different task', async () => {
+    const otpCache = { otp: undefined };
+    const obj = {};
+    const fn = jest.fn(makeTestCallback('654321', obj));
+
+    // enqueue a promise resolution to update the otp at the start of the next turn.
+    Promise.resolve().then(() => {
+      otpCache.otp = '654321';
+    });
+
+    // start intial otplease call, 'catch' will happen in next turn *after* the cache is set.
+    const result = await otplease(fn, {}, otpCache);
+    expect(fn).toHaveBeenCalledTimes(2);
+    expect(promptModule.promptTextInput).not.toHaveBeenCalled();
+    expect(result).toBe(obj);
+  });
+
+  it('semaphore prevents overlapping requests for OTP', async () => {
+    const otpCache = { otp: undefined };
+
+    // overlapped calls to otplease that share an otpCache should
+    // result in the user only being prompted *once* for an OTP.
+    const obj1 = {};
+    const fn1 = jest.fn(makeTestCallback('123456', obj1));
+    const p1 = otplease(fn1, {}, otpCache);
+
+    const obj2 = {};
+    const fn2 = jest.fn(makeTestCallback('123456', obj2));
+    const p2 = otplease(fn2, {}, otpCache);
+
+    const [res1, res2] = await Promise.all([p1, p2]);
+
+    expect(fn1).toHaveBeenCalledTimes(2);
+    expect(fn2).toHaveBeenCalledTimes(2);
+    // only prompt once for the two concurrent requests
+    expect(promptModule.promptTextInput).toHaveBeenCalledTimes(1);
+    expect(res1).toBe(obj1);
+    expect(res2).toBe(obj2);
+  });
+
+  it('strips whitespace from OTP prompt value', async () => {
+    promptModule.promptTextInput.mockImplementationOnce((msg, opts) => Promise.resolve(opts.filter(' 121212 ')));
+
+    const obj = {};
+    const fn = jest.fn(makeTestCallback('121212', obj));
+    const result = await otplease(fn, {});
+
+    expect(result).toBe(obj);
+  });
+
+  it('validates OTP prompt response', async () => {
+    promptModule.promptTextInput.mockImplementationOnce((msg, opts) =>
+      Promise.resolve(opts.validate('i am the very model of a modern major general'))
+    );
+
+    const obj = {};
+    const fn = jest.fn(makeTestCallback('343434', obj));
+
+    await expect(otplease(fn, {})).rejects.toThrow('Must be a valid one-time-password');
+  });
+
+  it('rejects prompt errors', async () => {
+    promptModule.promptTextInput.mockImplementationOnce(() => Promise.reject(new Error('poopypants')));
+
+    const obj = {};
+    const fn = jest.fn(makeTestCallback('343434', obj));
+
+    await expect(otplease(fn, {})).rejects.toThrow('poopypants');
+  });
+
+  it('re-throws non-EOTP errors', async () => {
+    const fn = jest.fn(() => {
+      const err: any = new Error('not found');
+      err.code = 'E404';
+      throw err;
+    });
+
+    await expect(otplease(fn, {})).rejects.toThrow('not found');
+  });
+
+  it('re-throws E401 errors that do not contain "one-time pass" in the body', async () => {
+    const fn = jest.fn(() => {
+      const err: any = new Error('auth required');
+      err.body = 'random arbitrary noise';
+      err.code = 'E401';
+      throw err;
+    });
+
+    await expect(otplease(fn, {})).rejects.toThrow('auth required');
+  });
+
+  it.each([['stdin'], ['stdout']])('re-throws EOTP error when %s is not a TTY', async (pipe) => {
+    const fn = jest.fn(() => {
+      const err: any = new Error(`non-interactive ${pipe}`);
+      err.code = 'EOTP';
+      throw err;
+    });
+
+    process[pipe].isTTY = false;
+
+    await expect(otplease(fn)).rejects.toThrow(`non-interactive ${pipe}`);
+  });
+
+  describe('getOneTimePassword()', () => {
+    it('defaults message argument', async () => {
+      await getOneTimePassword();
+
+      expect(promptModule.promptTextInput).toHaveBeenCalledWith(
+        'This operation requires a one-time password:',
+        expect.any(Object)
+      );
+    });
+
+    it('accepts custom message', async () => {
+      await getOneTimePassword('foo bar');
+
+      expect(promptModule.promptTextInput).toHaveBeenCalledWith('foo bar', expect.any(Object));
+    });
+  });
+});
+
+function makeTestCallback(otp, result) {
+  return (opts) => {
+    if (opts.otp !== otp) {
+      const err: any = new Error(`oops, received otp ${opts.otp}`);
+      err.code = 'EOTP';
+      throw err;
+    }
+    return result;
+  };
+}

packages/core/src/otplease.ts

@@ -1,4 +1,4 @@
-import { PromptUtilities } from './prompt';
+import { promptTextInput } from './prompt';
 
 /**
  * @typedef {object} OneTimePasswordCache - Passed between concurrent executions
@@ -58,14 +58,14 @@ function attempt(fn, opts, otpCache) {
       throw err;
     } else {
       // check the cache in case a concurrent caller has already updated the otp.
-      if (otpCache !== null && otpCache.otp !== null && otpCache.otp !== opts.otp) {
+      if (!isNullOrUndefined(otpCache) && !isNullOrUndefined(otpCache.otp) && otpCache.otp !== opts.otp) {
         return attempt(fn, { ...opts, ...otpCache }, otpCache);
       }
       // only allow one getOneTimePassword attempt at a time to reuse the value
       // from the preceeding prompt
       return semaphore.wait().then(() => {
         // check the cache again in case a previous waiter already updated it.
-        if (otpCache !== null && otpCache.otp !== null && otpCache.otp !== opts.otp) {
+        if (!isNullOrUndefined(otpCache) && !isNullOrUndefined(otpCache.otp) && otpCache.otp !== opts.otp) {
           semaphore.release();
           return attempt(fn, { ...opts, ...otpCache }, otpCache);
         }
@@ -74,7 +74,7 @@ function attempt(fn, opts, otpCache) {
             (otp) => {
               // update the otp and release the lock so that waiting
               // callers can see the updated otp.
-              if (otpCache !== null) {
+              if (!isNullOrUndefined(otpCache)) {
                 // eslint-disable-next-line no-param-reassign
                 otpCache.otp = otp;
               }
@@ -101,11 +101,15 @@ function attempt(fn, opts, otpCache) {
  */
 export function getOneTimePassword(message = 'This operation requires a one-time password:') {
   // Logic taken from npm internals: https://git.io/fNoMe
-  return PromptUtilities.input(message, {
+  return promptTextInput(message, {
     filter: (otp) => otp.replace(/\s+/g, ''),
     validate: (otp) =>
       (otp && /^[\d ]+$|^[A-Fa-f0-9]{64,64}$/.test(otp)) ||
       'Must be a valid one-time-password. ' +
       'See https://docs.npmjs.com/getting-started/using-two-factor-authentication',
   });
 }
+
+function isNullOrUndefined(val: any) {
+  return val === null || val === undefined;
+}