Merge pull request #11991 from meteor/fix/accounts-2fa

Adjusts in the package accounts-2fa

docs/history.md

@@ -4,14 +4,23 @@
 
 #### Breaking Changes
 
-N/A
+* `accounts-2fa@2.0.0`
+  - The method `has2faEnabled` no longer takes a selector as an argument, just the callback.
+  - `generate2faActivationQrCode` now throws an error if it's being called when the user already has 2FA enabled.
 
 #### Migration Steps
 
 #### Meteor Version Release
 
-* `accounts-2fa@1.0.1`
+* `accounts-2fa@2.0.0`
   - Reduce one DB call on 2FA login. [PR](https://github.com/meteor/meteor/pull/11985)
+  - Throw error when user is not found on `Accounts._is2faEnabledForUser`
+  - Remove vulnerability from the method `has2faEnabled`
+  - Now the package auto-publish the field `services.twoFactorAuthentication.type` for logged in users.
+* `accounts-password@2.3.1`
+  - Use method `Accounts._check2faEnabled` when validating 2FA
+* `accounts-passwordless@2.1.1`
+  - Use method `Accounts._check2faEnabled` when validating 2FA
 * `oauth@2.1.2`
   - Check effectively if popup was blocked by browser. [PR](https://github.com/meteor/meteor/pull/11984)
 * `standard-minifier-css@1.8.1`

docs/source/packages/accounts-2fa.md

@@ -5,7 +5,7 @@ description: Documentation of Meteor's `accounts-2fa` package.
 
 This package allows you to provide a way for your users to enable 2FA on their accounts, using an authenticator app such as Google Authenticator, or 1Password. When the user is logged in on your app, they will be able to generate a new QR code and read this code on the app they prefer. After that, they'll start receiving their codes. Then, they can finish enabling 2FA on your app, and every time they try to log in to your app, you can redirect them to a place where they can provide a code they received from the authenticator.
 
-This package uses [node-2fa](https://www.npmjs.com/package/node-2fa) which works on top of [notp](https://github.com/guyht/notp), **that** implements TOTP ([RFC 6238](https://www.ietf.org/rfc/rfc6238.txt)) (the Authenticator standard), which is based on HOTP ([RFC 4226](https://www.ietf.org/rfc/rfc4226.txt)) to provide codes that are exactly compatible with all other Authenticator apps and services that use them.
+To provide codes that are exactly compatible with all other Authenticator apps and services that implements TOTP, this package uses [node-2fa](https://www.npmjs.com/package/node-2fa) which works on top of [notp](https://github.com/guyht/notp), **that** implements TOTP ([RFC 6238](https://www.ietf.org/rfc/rfc6238.txt)) (the Authenticator standard), which is based on HOTP ([RFC 4226](https://www.ietf.org/rfc/rfc4226.txt)).
 
 > This package is meant to be used with [`accounts-password`](https://docs.meteor.com/api/passwords.html) or [`accounts-passwordless`](https://docs.meteor.com/packages/accounts-passwordless.html), so if you don't have either of those in your project, you'll need to add one of them. In the future, we want to enable the use of this package with other login methods, our oauth methods (Google, GitHub, etc...).
 
@@ -15,7 +15,7 @@ The first step, in order to enable 2FA, is to generate a QR code so that the use
 
 {% apibox "Accounts.generate2faActivationQrCode" "module":"accounts-base" %}
 
-Receives an `appName` which is the name of your app that will show up when the user scans the QR code. Also, a callback called with a QR code in SVG format and QR secret on success 
+Receives an `appName` which is the name of your app that will show up when the user scans the QR code. Also, a callback called, on success, with a QR code in SVG format, a QR secret, and the URI that can be used to activate the 2FA in an authenticator app, 
 or a single `Error` argument on failure.
 
 On success, this function will also add an object to the logged user's services object containing the QR secret:
@@ -43,8 +43,9 @@ const [qrCode, setQrCode] = useState(null);
   
 <button
   onClick={() => {
-    Accounts.generate2faActivationQrCode("My app name", (err, svg) => {
+    Accounts.generate2faActivationQrCode("My app name", (err, result) => {
       if (err) {console.error("...", err);return;}
+      const { svg, secret, uri } = result;
       /*
         the svg can be converted to base64, then be used like: 
          <img 
@@ -60,6 +61,9 @@ const [qrCode, setQrCode] = useState(null);
 </button>
 ```
 
+This method can fail throwing the following error:
+* "The 2FA is activated. You need to disable the 2FA first before trying to generate a new activation code [2fa-activated]" if trying to generate an activation when the user already have 2FA enabled.
+
 
 At this point, the 2FA won't be activated just yet. Now that the user has access to the codes generated by their authenticator app, you can call the function `Accounts.enableUser2fa`:
 
@@ -145,6 +149,10 @@ So the call of this function should look something like this:
 </button>
 ```
 
+This method can fail throwing one of the following errors:
+* "2FA code must be informed [no-2fa-code]" if a 2FA code was not provided.
+* "Invalid 2FA code [invalid-2fa-code]" if the provided 2FA code is invalid.
+
 <h3 id="working-with-accounts-passwordless">Working with accounts-passwordless</h3>
 
 Following the same logic from the previous package, if the 2FA is enabled, an error will be returned to the callback of the function [`Meteor.passwordlessLoginWithToken`](https://docs.meteor.com/packages/accounts-passwordless.html#Meteor-passwordlessLoginWithToken), then you can redirect the user to a place where they can provide a code.
@@ -180,13 +188,17 @@ Now you can call the function `Meteor.passwordlessLoginWithTokenAnd2faCode` that
 
 {% apibox "Meteor.passwordlessLoginWithTokenAnd2faCode" %}
 
+This method can fail throwing one of the following errors:
+* "2FA code must be informed [no-2fa-code]" if a 2FA code was not provided.
+* "Invalid 2FA code [invalid-2fa-code]" if the provided 2FA code is invalid.
+
 <h2 id="integrating-auth-package">How to integrate an Authentication Package with accounts-2fa</h2>
 
 To integrate this package with any other existing Login method, it's necessary following two steps:
 
 1 - For the client, create a new method from your current login method. So for example, from the method `Meteor.loginWithPassword` we created a new one called `Meteor.loginWithPasswordAnd2faCode`, and the only difference between them is that the latest one receives one additional parameter, the 2FA code, but we call the same function on the server side.
 
-2 - For the server, inside the function that will log the user in, you verify if the function `Accounts._is2faEnabledForUser` exists, and if yes, you call it providing the user you want to check if the 2FA is enabled, and if either of these statements are false, you proceed with the login flow. This function exists only when the package `accounts-2fa` is added to the project.
+2 - For the server, inside the function that will log the user in, you verify if the function `Accounts._check2faEnabled` exists, and if yes, you call it providing the user object you want to check if the 2FA is enabled, and if either of these statements are false, you proceed with the login flow. This function exists only when the package `accounts-2fa` is added to the project.
 
 If both statements are true, and the login validation succeeds, you verify if a code was provided: if not, throw an error; if it was provided, verify if the code is valid by calling the function `Accounts._isTokenValid`. if `Accounts._isTokenValid` returns false, throw an error.
 
@@ -196,8 +208,7 @@ Here it's an example:
   const result = validateLogin();
   if (
     !result.error &&
-    Accounts._is2faEnabledForUser &&
-    Accounts._is2faEnabledForUser(user)
+    Accounts._check2faEnabled?.(user)
   ) {
     if (!code) {
       Accounts._handleError('2FA code must be informed.');

packages/accounts-2fa/2fa-client.js

@@ -10,14 +10,13 @@ const reportError = (error, callback) => {
 };
 
 /**
- * @summary Verify if the user has 2FA enabled
+ * @summary Verify if the logged user has 2FA enabled
  * @locus Client
- * @param {Object|String} selector Username, email or custom selector to identify the user.
  * @param {Function} [callback] Called with a boolean on success that indicates whether the user has
  *    or not 2FA enabled, or with a single `Error` argument on failure.
  */
-Accounts.has2faEnabled = (selector, callback) => {
-  Accounts.connection.call('has2faEnabled', selector, callback);
+Accounts.has2faEnabled = callback => {
+  Accounts.connection.call('has2faEnabled', callback);
 };
 
 /**
@@ -25,8 +24,9 @@ Accounts.has2faEnabled = (selector, callback) => {
  * @locus Client
  * @param {String} appName It's the name of your app that will show up when the user scans the QR code.
  * @param {Function} callback
- *   Called with a QR code in SVG format on success, or with a single `Error` argument
- *   on failure.
+ *   Called with a single `Error` argument on failure.
+ *   Or, on success, called with an object containing the QR code in SVG format (svg),
+ *   the QR secret (secret), and the URI so the user can manually activate the 2FA without reading the QR code (uri).
  */
 Accounts.generate2faActivationQrCode = (appName, callback) => {
   if (!appName) {

packages/accounts-2fa/2fa-server.js

@@ -13,23 +13,11 @@ Accounts._check2faEnabled = user => {
   );
 };
 
-Accounts._is2faEnabledForUser = selector => {
-  if (!Meteor.isServer) {
-    throw new Meteor.Error(
-      400,
-      'The function _is2faEnabledForUser can only be called on the server'
-    );
+Accounts._is2faEnabledForUser = () => {
+  const user = Meteor.user();
+  if (!user) {
+    throw new Meteor.Error('no-logged-user', 'No user logged in.');
   }
-
-  if (typeof selector === 'string') {
-    if (!selector.includes('@')) {
-      selector = { $or: [{ _id: selector }, { username: selector }] };
-    } else {
-      selector = { email: selector };
-    }
-  }
-
-  const user = Meteor.users.findOne(selector) || {};
   return Accounts._check2faEnabled(user);
 };
 
@@ -43,7 +31,9 @@ Accounts._isTokenValid = (secret, code) => {
     );
   }
   const { delta } = twofactor.verifyToken(secret, code, 10) || {};
-  return delta != null && delta >= 0;
+  // we are using != instead of !==, which means "undefined != null" and "null != null" are both false,
+  // so we don't need to check delta !== undefined
+  return delta != null && delta === 0;
 };
 
 Meteor.methods({
@@ -58,9 +48,17 @@ Meteor.methods({
       );
     }
 
+    if (Accounts._check2faEnabled(user)) {
+      throw new Meteor.Error(
+        '2fa-activated',
+        'The 2FA is activated. You need to disable the 2FA first before trying to generate a new activation code.'
+      );
+    }
+
+    const emails = user.emails || [];
     const { secret, uri } = twofactor.generateSecret({
       name: appName.trim(),
-      account: user.username || user._id
+      account: user.username || emails[0]?.address || user._id,
     });
     const svg = new QRCode(uri).svg();
 
@@ -69,13 +67,13 @@ Meteor.methods({
       {
         $set: {
           'services.twoFactorAuthentication': {
-            secret
-          }
-        }
+            secret,
+          },
+        },
       }
     );
 
-    return { svg, secret };
+    return { svg, secret, uri };
   },
   enableUser2fa(code) {
     check(code, String);
@@ -86,7 +84,7 @@ Meteor.methods({
     }
 
     const {
-      services: { twoFactorAuthentication }
+      services: { twoFactorAuthentication },
     } = user;
 
     if (!twoFactorAuthentication || !twoFactorAuthentication.secret) {
@@ -96,7 +94,7 @@ Meteor.methods({
       );
     }
     if (!Accounts._isTokenValid(twoFactorAuthentication.secret, code)) {
-      throw new Meteor.Error(400, 'Invalid code.');
+      Accounts._handleError('Invalid 2FA code', true, 'invalid-2fa-code');
     }
 
     Meteor.users.update(
@@ -105,9 +103,9 @@ Meteor.methods({
         $set: {
           'services.twoFactorAuthentication': {
             ...twoFactorAuthentication,
-            type: 'otp'
-          }
-        }
+            type: 'otp',
+          },
+        },
       }
     );
   },
@@ -122,22 +120,16 @@ Meteor.methods({
       { _id: userId },
       {
         $unset: {
-          'services.twoFactorAuthentication': 1
-        }
+          'services.twoFactorAuthentication': 1,
+        },
       }
     );
   },
-  has2faEnabled(selector) {
-    check(selector, Match.Maybe(Match.OneOf(String, Object)));
-    const userId = Meteor.userId();
-    if (!userId) {
-      throw new Meteor.Error(400, 'No user logged in.');
-    }
-
-    if (!selector) {
-      selector = { _id: userId };
-    }
-
-    return Accounts._is2faEnabledForUser(selector);
-  }
+  has2faEnabled() {
+    return Accounts._is2faEnabledForUser();
+  },
+});
+
+Accounts.addAutopublishFields({
+  forLoggedInUser: ['services.twoFactorAuthentication.type'],
 });

packages/accounts-2fa/client_tests.js

@@ -1,13 +1,14 @@
 import { Accounts } from 'meteor/accounts-base';
 import { Random } from 'meteor/random';
 
-Tinytest.add('account - 2fa - has2faEnabled', test => {
-  const userId = Accounts.createUser({
+Tinytest.addAsync('account - 2fa - has2faEnabled - client', (test, done) => {
+  Accounts.createUser({
     username: Random.id(),
     password: Random.id(),
   });
 
-  Accounts.has2faEnabled(userId, (error, result) => {
-    test.equal(result, false);
+  Accounts.has2faEnabled((error, result) => {
+    test.isFalse(result);
+    done();
   });
 });

packages/accounts-2fa/package.js

@@ -1,5 +1,5 @@
 Package.describe({
-  version: '1.0.1',
+  version: '2.0.0',
   summary:
     'Package used to enable two factor authentication through OTP protocol',
 });

packages/accounts-2fa/server_tests.js

@@ -1,13 +1,26 @@
 import { Accounts } from 'meteor/accounts-base';
 import { Random } from 'meteor/random';
 
-Tinytest.add('account - 2fa - has2faEnabled', test => {
-  // Create users
-  const userWithout2FA = Accounts.insertUserDoc({}, { emails: [{address: `${Random.id()}@meteorapp.com`, verified: true}] });
-  const userWith2FA = Accounts.insertUserDoc({}, { emails: [{address: `${Random.id()}@meteorapp.com`, verified: true}], services: { twoFactorAuthentication: { type: 'otp', secret: 'superSecret' } } });
+const findUserById = id => Meteor.users.findOne(id);
 
-  test.equal(Accounts._is2faEnabledForUser(userWithout2FA), false);
-  test.equal(Accounts._is2faEnabledForUser(userWith2FA), true);
+Tinytest.add('account - 2fa - has2faEnabled - server', test => {
+  // Create users
+  const userWithout2FA = Accounts.insertUserDoc(
+    {},
+    { emails: [{ address: `${Random.id()}@meteorapp.com`, verified: true }] }
+  );
+  const userWith2FA = Accounts.insertUserDoc(
+    {},
+    {
+      emails: [{ address: `${Random.id()}@meteorapp.com`, verified: true }],
+      services: {
+        twoFactorAuthentication: { type: 'otp', secret: 'superSecret' },
+      },
+    }
+  );
+
+  test.equal(Accounts._check2faEnabled(findUserById(userWithout2FA)), false);
+  test.equal(Accounts._check2faEnabled(findUserById(userWith2FA)), true);
 
   // cleanup
   Accounts.users.remove(userWithout2FA);

packages/accounts-base/accounts_client_tests.js

@@ -240,12 +240,12 @@ Tinytest.addAsync(
           // enable 2fa
           Accounts.enableUser2fa(token, () => {
             // verifies if 2fa is enabled
-            Accounts.has2faEnabled(username, (err, isEnabled) => {
+            Accounts.has2faEnabled((err, isEnabled) => {
               test.isTrue(isEnabled);
               // disable 2fa
               Accounts.disableUser2fa(() => {
                 // verifies if 2fa is disabled
-                Accounts.has2faEnabled(username, (err, isEnabled) => {
+                Accounts.has2faEnabled((err, isEnabled) => {
                   test.isFalse(!!isEnabled);
                   removeTestUser(done);
                 });

packages/accounts-password/package.js

@@ -5,7 +5,7 @@ Package.describe({
   // 2.2.x in the future. The version was also bumped to 2.0.0 temporarily
   // during the Meteor 1.5.1 release process, so versions 2.0.0-beta.2
   // through -beta.5 and -rc.0 have already been published.
-  version: '2.3.0',
+  version: '2.3.1',
 });
 
 Npm.depends({

packages/accounts-passwordless/package.js

@@ -1,6 +1,6 @@
 Package.describe({
   summary: 'No-password login/sign-up support for accounts',
-  version: '2.1.0',
+  version: '2.1.1',
 });
 
 Package.onUse(api => {

packages/accounts-passwordless/passwordless_server.js

@@ -89,10 +89,7 @@ Accounts.registerLoginHandler('passwordless', options => {
 
   if (!error && verifiedEmail) {
     // This method is added by the package accounts-2fa
-    if (
-      Accounts._is2faEnabledForUser &&
-      Accounts._is2faEnabledForUser({ _id: user._id })
-    ) {
+    if (Accounts._check2faEnabled?.(user)) {
       if (!options.code) {
         Accounts._handleError('2FA code must be informed', true, 'no-2fa-code');
         return;