Merge pull request #1337 from balena-io/fix-2fa-isEnabled

Fix auth.twoFactor.isEnabled() regression returning always true
balena-io · May 30, 2023 · 5b428c1 · 5b428c1
2 parents 8f0688b + 9695592
commit 5b428c1
Show file tree

Hide file tree

Showing 6 changed files with 72 additions and 3 deletions.
diff --git a/.github/actions/test/action.yml b/.github/actions/test/action.yml
@@ -52,6 +52,9 @@ runs:
         export TEST_MEMBER_USERNAME=${{ fromJSON(inputs.secrets)[ format('TEST_MEMBER_USERNAME{0}', fromJSON('{"windows-2019":"","ubuntu-20.04":"_1","macos-12":"_2"}')[env.os_value || 'windows-2019' ]) ] }}
         export TEST_MEMBER_PASSWORD=${{ fromJSON(inputs.secrets).TEST_MEMBER_PASSWORD }}
         export TEST_ONLY_ON_ENVIRONMENT=${{ fromJSON('{"windows-2019":"node","ubuntu-20.04":"node","macos-12":"browser"}')[env.os_value || 'windows-2019' ]  }}
+        export TEST_2FA_EMAIL=${{ fromJSON(inputs.secrets).TEST_2FA_EMAIL }}
+        export TEST_2FA_PASSWORD=${{ fromJSON(inputs.secrets).TEST_2FA_PASSWORD }}
+        export TEST_2FA_SECRET=${{ fromJSON(inputs.secrets).TEST_2FA_SECRET }}
 
         echo "TEST_EMAIL_KEY: [ ${TEST_EMAIL_KEY} ]"
         echo "TEST_EMAIL: [ ${TEST_EMAIL} ]"

diff --git a/karma.conf.js b/karma.conf.js
@@ -14,6 +14,9 @@ module.exports = function (config) {
 		'TEST_MEMBER_EMAIL',
 		'TEST_MEMBER_PASSWORD',
 		'TEST_MEMBER_USERNAME',
+		'TEST_2FA_EMAIL',
+		'TEST_2FA_PASSWORD',
+		'TEST_2FA_SECRET',
 		'TEST_REGISTER_EMAIL',
 		'TEST_REGISTER_PASSWORD',
 		'TEST_REGISTER_USERNAME',

diff --git a/package.json b/package.json
@@ -93,6 +93,7 @@
     "mocha.parallel": "^0.15.6",
     "mockttp": "^3.0.0",
     "os-browserify": "^0.3.0",
+    "otplib": "^12.0.1",
     "process": "^0.11.10",
     "rimraf": "^5.0.1",
     "rindle": "^1.3.6",
@@ -115,7 +116,7 @@
     "@types/json-schema": "^7.0.9",
     "@types/node": "^14.0.0",
     "abortcontroller-polyfill": "^1.7.1",
-    "balena-auth": "^4.1.0",
+    "balena-auth": "^4.2.0",
     "balena-errors": "^4.7.3",
     "balena-hup-action-utils": "~5.0.0",
     "balena-register-device": "^8.0.0",

diff --git a/src/2fa.ts b/src/2fa.ts
@@ -41,8 +41,8 @@ const get2fa = function (
 	 * });
 	 */
 	async function isEnabled(): Promise<boolean> {
-		const twoFactorRequired = await auth.needs2FA();
-		return twoFactorRequired != null;
+		const twoFactorStatus = await auth.get2FAStatus();
+		return twoFactorStatus !== 'not_required';
 	}
 
 	/**

diff --git a/tests/integration/auth.spec.js b/tests/integration/auth.spec.js
@@ -1,12 +1,14 @@
 import { expect } from 'chai';
 import { timeSuite } from '../util';
+import { authenticator } from 'otplib';
 
 import {
 	balena,
 	sdkOpts,
 	credentials,
 	givenLoggedInUser,
 	givenLoggedInUserWithApiKey,
+	loginUserWith2FA,
 } from './setup';
 
 describe('SDK authentication', function () {
@@ -324,4 +326,52 @@ describe('SDK authentication', function () {
 			});
 		});
 	});
+
+	describe('given a user without 2FA', function () {
+		givenLoggedInUser(before);
+
+		describe('balena.auth.twoFactor.isEnabled()', function () {
+			it('should not be enabled', async () => {
+				expect(await balena.auth.twoFactor.isEnabled()).to.be.false;
+			});
+		});
+
+		describe('balena.auth.twoFactor.isPassed()', function () {
+			it('should be true', async () => {
+				expect(await balena.auth.twoFactor.isPassed()).to.be.true;
+			});
+		});
+	});
+
+	describe('given a user with 2FA', function () {
+		const has2FAAccount = credentials.twoFactor.email != null;
+
+		const given2FAUserIt = (description, testFn) => {
+			const $it = has2FAAccount ? it : it.skip;
+			$it(description, testFn);
+		};
+
+		before(async () => {
+			if (!has2FAAccount) {
+				return;
+			}
+			await loginUserWith2FA();
+		});
+
+		describe('balena.auth.twoFactor.isEnabled()', function () {
+			given2FAUserIt('should be true', async () => {
+				expect(await balena.auth.twoFactor.isEnabled()).to.be.true;
+			});
+		});
+		describe('balena.auth.twoFactor.isPassed()', function () {
+			given2FAUserIt('should false when 2FA is not passed', async () => {
+				expect(await balena.auth.twoFactor.isPassed()).to.be.false;
+			});
+			given2FAUserIt('should be true when 2FA is passed', async () => {
+				const code = authenticator.generate(credentials.twoFactor.secret);
+				await balena.auth.twoFactor.challenge(code);
+				expect(await balena.auth.twoFactor.isPassed()).to.be.true;
+			});
+		});
+	});
 });
diff --git a/tests/integration/setup.ts b/tests/integration/setup.ts
@@ -74,6 +74,11 @@ const buildCredentials = function () {
 			password: env.TEST_REGISTER_PASSWORD,
 			username: env.TEST_REGISTER_USERNAME,
 		},
+		twoFactor: {
+			email: env.TEST_2FA_EMAIL,
+			password: env.TEST_2FA_PASSWORD,
+			secret: env.TEST_2FA_SECRET,
+		},
 	};
 	if (
 		// TODO: this should include the paid account eventually as well
@@ -187,6 +192,13 @@ export function givenLoggedInUser(beforeFn: Mocha.HookFunction) {
 	afterFn(() => resetUser());
 }
 
+export function loginUserWith2FA() {
+	return balena.auth.login({
+		email: credentials.twoFactor.email,
+		password: credentials.twoFactor.password,
+	});
+}
+
 export function loginPaidUser() {
 	return balena.auth.login({
 		email: credentials.paid.email,