twilio / authy-devise

Authy Devise plugin to add Two-Factor Authentication
MIT License
201 stars 84 forks source link

Feature Flag/Opt in Two Factor Auth #112

Closed dlinch closed 2 years ago

dlinch commented 5 years ago

Loving the integration so far, but ran into a small hiccup. We do not have a model that allows all users to simultaneously be enabled for two factor authentication.

First, I have the need to feature flag two factor authentication. We have several environments and we're going to slowly roll out two factor auth between environments. We usually set some kind of ENV variable, here probably ENV['TWOFACTOR_ENABLED'], which would then activate the two_factor flow. I realize I can not reveal the links in the UI, but the routes are still available and in case someone starts URL snooping I want to ensure they can't hit any of those routes at all. It seems vaguely possibly through monkey patching, but I feel there's a better way we could approach the problem as a feature in the library.

I tried overwriting the DeviseAuthy::PasswordsController, as that seems like where the library is intercepting the sign_in method to determine whether to send them down the authy flow or directly to normal devise sign in flow. I attempted to overwrite it similarly to how the documentation recommended to customize redirect after auth:

devise_for :user,
                  controllers: {
                    devise_authy: 'custom_devise_authy',
                    devise_authy_passwords: 'devise_authy/passwords',
                  }

I need to add some logic that a "group" a user belongs to has enabled two factor authentication for the users before they can access it. Is this something devise authy supports?

I can imagine a config setting in the initializer for an overall feature flag:

Authy.api_key = ENV['AUTHY_API_KEY']
Authy.api_uri = 'https://api.authy.com/'
Authy.enabled = ENV['AUTH_ENABLED'] # deafults to true

And perhaps a method on the devise model to run in the passwords sign_in controller as well:

class User < ActiveRecord::Base
  devise :authy_authenticatable
  def two_factor_enabled?
    # custom logic to determine if the two factor auth flow should be overridden for this 
        particular user
  end
end

so perhaps the controller looks like:

class DeviseAuthy::PasswordsController < Devise::PasswordsController
  def sign_in(resource_or_scope, *args)

    resource = args.last || resource_or_scope
    return super unless Authy.enabled?
    if resource.respond_to?(:with_authy_authentication?) && resource.with_authy_authentication?(request) && resource.two_factor_enabled?
      # Do nothing. Because we need verify the 2FA
      true
    else
      super
    end
  end
end
philnash commented 5 years ago

This is an interesting point. I would consider implementing two_factor_enabled? on your user, and then over-riding with_authy_authentication? too.

Something like this (using the method wrapping technique from this SO answer:

class User < ActiveRecord::Base
  devise :authy_authenticatable
  def two_factor_enabled?
    # custom logic to determine if the two factor auth flow should be overridden for this 
        particular user
  end

  old_with_authy_authentication? = instance_method(:with_authy_authentication?)

  define_method(:with_authy_authentication?) do |request|
    two_factor_enabled? && old_with_authy_authentication?.bind(self).(request)
  end
end

This doesn't help you if users go snooping for URLs though. They'll still be able to find the routes and sign up for 2FA, even if they aren't then asked for it when they log in. To avoid that you might want to look into opening up Devise::DeviseAuthyController and adding a before action that checks whether @resource.two_factor_enabled? and throws an error if not.

Does that help at all?

dlinch commented 5 years ago

@philnash Yes, this is actually very helpful, I've done a variation of this already to get me moving on the feature, but I opened the issue to potentially add features to the roadmap to support these kinds of overrides as configuration options rather than monkey patching.

Are you guys accepting PR's? If I have time maybe I can work on an implementation for some of these features.

philnash commented 5 years ago

Definitely happy to take PRs, and if this can be less hacky then I’m all for it. I am away for a little bit though, so if I don’t respond quickly, don’t worry, I’ll get to it eventually.

Thanks!

philnash commented 2 years ago

This library is no longer actively maintained. The Authy API has been replaced with the Twilio Verify API. Twilio will support the Authy API through November 1, 2022 for SMS/Voice. After this date, we’ll start to deprecate the service for SMS/Voice. Any requests sent to the API after May 1, 2023, will automatically receive an error. Push and TOTP will continue to be supported through July 2023.

Learn more about migrating from Authy to Verify.

Please visit the Twilio Docs for:

Please direct any questions to Twilio Support. Thank you!