feathersjs / feathers

The API and real-time application framework
https://feathersjs.com
MIT License
14.97k stars 744 forks source link

Two Factor Authentication #1601

Open somyaranjank opened 4 years ago

somyaranjank commented 4 years ago

I want to add two factor authentication service to my feathers app. Please give a suggestion how to proceed in new version of feathers to achieve this. Thanks in advance

MarcGodard commented 4 years ago

@somyaranjank I am having the same issue. Trying to figure out how to add a custom verifier to handle this and other special auth cases. I was looking at the feathers-authentication-management stuff but seems to have everything but 2nd factor stuff... Let me know what you find.

jnardone commented 4 years ago

The technical approach to this would vary a bit based on 2FA implementation. A 2FA system that required a code for all users could send a user+pass+code to a custom authentication strategy, which would likely just need to extend the LocalStrategy and also verify the code (using either a saved value which has previously been communicated to the user; this value should be hashed like a password!) or via an algorithm like TOTP (using a seed and time+digits calculation - this is what all the QR-based apps use based on RFC 6238).

Alternately, if 2FA enforcement depends on information about the user, you would need two round-trips, one to submit the user (or user + password) and then indicate in the response that a 2nd factor is also needed (this initial response should NOT return an accessToken!) Then, you capture the 2nd factor token and then verification is like the above.

These are not the only two ways, but these are two possible ways of solving this.

Fundamentally, though, any solution should abide by the fact that:

MarcGodard commented 4 years ago

@jnardone Do you have any code samples of this?

jnardone commented 4 years ago

I don't have anything I can publicly share - this is mostly an amalgamation of things we've built pre-v4 and how we'd want to (re)build it. If I find some time I'll try to code up some examples.

MarcGodard commented 4 years ago

@jnardone So far I have rebuilt enough in v4 that today I will give this a shot. Will post code once completed.

MarcGodard commented 4 years ago

My 2fa implementation (NOT TESTED YET AS I HAVE TO GO):

class MyLocalStrategy extends LocalStrategy {
  async authenticate (authRequest, params) { // eslint-disable-line no-unused-vars
    const { email, password, twoFa } = authRequest

    const result = await this.findEntity(email, omit(params, 'provider'))
    await this.comparePassword(result, password)

    if (result.secondFactor === 'email' && !twoFa) {
      const verifyToken = randomBytes(5).toString('hex').toUpperCase()
      const verifyExpiry = moment().subtract(10, 'minutes').toISOString()
      await this.app.service('api/users').patch(result._id, { verifyToken, verifyExpiry })
      const useMail = this.app.get('email').provider
      if (useMail !== 'postmark' && useMail !== 'mailer') throw new Error('email provider invalid.')
      const emailMessages = this.app.service('api/' + useMail)
      await emailMessages.create({ locale: result.locale || 'en', email: result.email, type: 'secondFactorAuth', emailCode: verifyToken })
      throw new NotAuthenticated('Sent 2FA')
    } else if (result.secondFactor === 'sms' && !twoFa && result.mobileNumber) {
      const verifyToken = parseInt(randomBytes(6).toString('hex'), 16).toString().substr(0, 6)
      const verifyExpiry = moment().subtract(5, 'minutes').toISOString()
      await this.app.service('api/users').patch(result._id, { verifyToken, verifyExpiry })
      await this.app.service('api/twilio-sms').create({ locale: result.locale || 'en', phone: result.mobileNumber, type: 'secondFactorAuth', smsCode: verifyToken })
      throw new NotAuthenticated('Sent 2FA')
    } if (twoFa && result.verifyToken === twoFa && moment(result.verifyExpiry).isBefore(Date.now())) {
      await this.app.service('api/users').patch(result._id, { verifyToken: null, verifyExpiry: null })
    } else if (twoFa && result.verifyToken !== twoFa) {
      throw new NotAuthenticated('Invalid 2FA!')
    } else {
      return {
        authentication: { strategy: this.name },
        user: await this.getEntity(result, params)
      }
    }
  }

  async comparePassword (entity, password) {
    const hash = entity.password
    const tempHash = entity.tempPassword

    if (!hash && !tempHash) {
      throw new NotAuthenticated('User record in the database is missing both "password" and "tempPassword"')
    }
    if (tempHash && !entity.tempPasswordExpiry) {
      throw new NotAuthenticated('Temp password expiry missing!')
    }
    if (tempHash && moment(entity.tempPasswordExpiry).isBefore(Date.now())) {
      throw new NotAuthenticated('Temp password has expired!')
    }

    let result = await bcrypt.compare(password, hash)
    if (!result && tempHash) {
      result = await bcrypt.compare(password, tempHash)
    }
    if (result) {
      return entity
    }

    throw new NotAuthenticated('Invalid login')
  }

  getEntityQuery (query, params) { // eslint-disable-line no-unused-vars
    return Object.assign(query, {
      blocked: { $nin: [true] },
      $limit: 1
    })
  }
}

Please all comments and criticism are welcomed.

somyaranjank commented 4 years ago

@MarcGodard is it working? did you test it?

MarcGodard commented 4 years ago

@somyaranjank Had to make a few small edits, but this code does work and works perfectly. The front end has to handle the error "Sent 2FA" to ask for the code.

jandix commented 4 years ago

@MarcGodard thanks for providing a code sample. The sample helps a lot. Might it be possible to provide the whole code to replace the code in authentication.js?

MarcGodard commented 4 years ago

@jandix The only other difference compared to the generated code is authentication.register('local', new MyLocalStrategy()) instead of authentication.register('local', new LocalStrategy()). Not sure what you are looking for.

jandix commented 4 years ago

@MarcGodard thanks for your fast reply. Already figured it out using the doucmentation, but your example helps a lot. I came across a question: Shouldn't the following line const verifyExpiry = moment().subtract(10, 'minutes').toISOString() rather use moment().add() instead? Otherwise the test moment(result.verifyExpire).isBefore(Date.now()) always yields true, isn't it?

MarcGodard commented 4 years ago

@jandix Yes I did make that change when I fully tested and made all the little fixes.

somyaranjank commented 4 years ago

@jandix @MarcGodard Can you please share the code in authentication.js ?

MarcGodard commented 4 years ago

@somyaranjank What are you missing? Everything is above. Sorry everyone but I am unsubscribing from this issue. If anyone wants my help implementing this, please find me on the feathers slack. Thanks.

jandix commented 4 years ago

@somyaranjank attached you find the current version of my authentication.js. However, please be aware that a few features are still missing (e.g.: moving twilio to it's own service).

const { AuthenticationService, JWTStrategy } = require('@feathersjs/authentication');
const { NotAuthenticated } = require('@feathersjs/errors');
const { LocalStrategy } = require('@feathersjs/authentication-local');
const { expressOauth } = require('@feathersjs/authentication-oauth');
const randomBytes = require('randombytes');
const moment = require('moment');
const credentials = require('../config/twilio.json');
const twilio = require('twilio')(credentials.accountSid, credentials.authToken);

class MyLocalStrategy extends LocalStrategy {
  async authenticate(authRequest, params) {
    const { email, password, otp } = authRequest;

    const result = await this.findEntity(email, {});

    await this.comparePassword(result, password);

    if (!otp) {
      const verifyToken = parseInt(randomBytes(6).toString('hex'), 16)
        .toString()
        .substr(0, 6);
      const verifyExpire = moment()
        .add(5, 'minutes')
        .toISOString();
      await this.app.service('users').patch(result.id, { verifyToken, verifyExpire });
      twilio.messages
        .create({
          body: verifyToken,
          from: credentials.phoneNumber,
          to: result.phone,
        })
        .then((message) => {
          console.log(message.sid);
        });
      throw new NotAuthenticated('OTP missing.');
    }

    if (
      otp &&
      parseInt(result.verifyToken) === parseInt(otp) &&
      moment(result.verifyExpire).isBefore(Date.now())
    ) {
      throw new NotAuthenticated('OTP expired.');
    } else if (otp && parseInt(result.verifyToken) !== parseInt(otp)) {
      throw new NotAuthenticated('OTP wrong.');
    } else {
      return {
        authentication: { strategy: this.name },
        user: await this.getEntity(result, params),
      };
    }
  }

  getEntityQuery(query, params) {
    // Query for user but only include users marked as `active`
    return {
      ...query,
      active: true,
      $limit: 1,
    };
  }
}

module.exports = (app) => {
  const authentication = new AuthenticationService(app);

  authentication.register('jwt', new JWTStrategy());
  authentication.register('local', new MyLocalStrategy());

  app.use('/authentication', authentication);
  app.configure(expressOauth());
};
somyaranjank commented 4 years ago

@jandix Thank You.

jd1378 commented 4 years ago

also providing TOTP support out of the box would be nice to have

bartello1982 commented 3 years ago

How can I customise the response code for the authentication service ? I did extend the LocalStrategy as @jandix did it but I would like to return 201 status code without the token after first authentications step in my two factor authentication process instead of throwing NotAuthenticated error. If I return just an empty object, accesstoken is always present in a response. I would appreciate any help.

wethinkagile commented 2 years ago

Thanks a lot for the example @MarcGodard Can we have a working example that takes all the fixes from this thread into account please?

@jandix Also there is little to no sense for many of us now in using some US-cloud based SDK like Twillio Incorporated in case you are located in the European Union. Caspar Bowden's prophecy came true and anybody doing any digital business in the EU has to leave the US Cloud. It is a legislatory fact and everyone will need to adapt.

MarcGodard commented 2 years ago

@stevek-pro I am sorry, my code is not a repo, I have made many changes since that post and added yubikey support and many other changes. Meaning I would need to fix that code as my exiting code is very different. I am sure you are a capable programmer capable of taking that code and fixing and modifying it as needed, if not, consider it an exercise to improve your skills. I also do not use twilio anymore and use "messagebird".

itoonx commented 1 year ago

You can create 2FA hook and check it in before hook on authentication if user has been registered 2FA then you can do it on verify code params and verify by you self