Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Two Factor Authentication #1601

Open
somyaranjank opened this issue Oct 4, 2019 · 21 comments
Open

Two Factor Authentication #1601

somyaranjank opened this issue Oct 4, 2019 · 21 comments

Comments

@somyaranjank
Copy link

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
Copy link
Contributor

MarcGodard commented Oct 6, 2019

@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
Copy link
Contributor

jnardone commented Oct 6, 2019

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:

  • only a successful validation of user+pass+factor returns an accessToken. Any flow returning an accessToken that does not meet this is essentially bypassing the factor
  • don't store any one-time codes in the clear on the back-end; these are essentially 2nd passwords and should be encrypted as such. You can't do the same thing with TOTP seeds however.

@MarcGodard
Copy link
Contributor

@jnardone Do you have any code samples of this?

@jnardone
Copy link
Contributor

jnardone commented Oct 6, 2019

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
Copy link
Contributor

MarcGodard commented Oct 6, 2019

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

@MarcGodard
Copy link
Contributor

MarcGodard commented Oct 6, 2019

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
Copy link
Author

@MarcGodard is it working? did you test it?

@MarcGodard
Copy link
Contributor

@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
Copy link

jandix commented Oct 14, 2019

@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
Copy link
Contributor

@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
Copy link

jandix commented Oct 15, 2019

@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
Copy link
Contributor

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

@somyaranjank
Copy link
Author

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

@MarcGodard
Copy link
Contributor

@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
Copy link

jandix commented Oct 17, 2019

@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
Copy link
Author

@jandix Thank You.

@jd1378
Copy link
Contributor

jd1378 commented May 20, 2020

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

@bartello1982
Copy link

bartello1982 commented Nov 24, 2020

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
Copy link

wethinkagile commented Dec 17, 2021

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
Copy link
Contributor

@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
Copy link

itoonx commented Mar 3, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants