Add support for OTP secret key encryption

**Why**:
To provide an additional layer of security.
The TOTP spec (RFC 6238) recommends encrypting the keys.
http://tools.ietf.org/html/rfc6238

**How**:
Borrow the encryption code from the `attr_encrypted` gem and use it to
encrypt and decrypt the `otp_secret_key` attribute.

Allow users to add encryption by passing in `encrypted: true` to
`has_one_time_password`. This provides backwards-compatibility for
existing users of the gem.

See the README updates for more detailed instructions for both new
and existing users.

Gemfile

@@ -1,4 +1,4 @@
-source "http://rubygems.org"
+source 'https://rubygems.org'
 
 # Specify your gem's dependencies in devise_ip_filter.gemspec
 gemspec
@@ -20,6 +20,11 @@ if Gem::Version.new(RUBY_VERSION) >= Gem::Version.new('2.2.0')
   gem "test-unit", "~> 3.0"
 end
 
+group :test, :development do
+  gem 'sqlite3'
+end
+
 group :test do
-  gem "sqlite3"
+  gem 'rack_session_access'
+  gem 'ammeter'
 end

README.md

@@ -5,11 +5,12 @@
 
 ## Features
 
-* control sms code pattern
-* configure max login attempts
-* per user level control if he really need two factor authentication
-* your own sms logic
+* configurable OTP code digit length
+* configurable max login attempts
+* customizable logic to determine if a user needs two factor authentication
+* customizable logic for sending the OTP code to the user
 * configurable period where users won't be asked for 2FA again
+* option to encrypt the OTP secret key in the database, with iv and salt
 
 ## Configuration
 
@@ -23,62 +24,71 @@ Once that's done, run:
 
     bundle install
 
-### Automatic installation
+### Installation
 
-In order to add two factor authentication to a model, run the command:
+#### Automatic initial setup
+To set up the model and database migration file automatically, run the
+following command:
 
     bundle exec rails g two_factor_authentication MODEL
 
-Where MODEL is your model name (e.g. User or Admin). This generator will add `:two_factor_authenticatable` to your model
-and create a migration in `db/migrate/`, which will add `:otp_secret_key` and `:second_factor_attempts_count` to your table.
-Finally, run the migration with:
+Where MODEL is your model name (e.g. User or Admin). This generator will add
+`:two_factor_authenticatable` to your model's Devise options and create a
+migration in `db/migrate/`, which will add the following columns to your table:
 
-    bundle exec rake db:migrate
-
-Add the following line to your model to fully enable two-factor auth:
+- `:second_factor_attempts_count`
+- `:encrypted_otp_secret_key`
+- `:encrypted_otp_secret_key_iv`
+- `:encrypted_otp_secret_key_salt`
 
-    has_one_time_password
-
-Set config values, if desired:
+#### Manual initial setup
+If you prefer to set up the model and migration manually, add the
+`:two_factor_authentication` option to your existing devise options, such as:
 
 ```ruby
-config.max_login_attempts = 3  # Maximum second factor attempts count
-config.allowed_otp_drift_seconds = 30  # Allowed time drift
-config.otp_length = 6  # OTP code length
-config.remember_otp_session_for_seconds = 30.days  # Time before browser has to enter OTP code again
+devise :database_authenticatable, :registerable, :recoverable, :rememberable,
+       :trackable, :validatable, :two_factor_authenticatable
 ```
 
-Override the method to send one-time passwords in your model, this is automatically called when a user logs in:
+Then create your migration file using the Rails generator, such as:
 
-```ruby
-def send_two_factor_authentication_code
-  # use Model#otp_code and send via SMS, etc.
-end
+```
+rails g migration AddTwoFactorFieldsToUsers second_factor_attempts_count:integer encrypted_otp_secret_key:string:index encrypted_otp_secret_key_iv:string encrypted_otp_secret_key_salt:string
 ```
 
-### Manual installation
-
-To manually enable two factor authentication for the User model, you should add two_factor_authentication to your devise line, like:
+Open your migration file (it will be in the `db/migrate` directory and will be
+named something like `20151230163930_add_two_factor_fields_to_users.rb`), and
+add `unique: true` to the `add_index` line so that it looks like this:
 
 ```ruby
-devise :database_authenticatable, :registerable,
-       :recoverable, :rememberable, :trackable, :validatable, :two_factor_authenticatable
+add_index :users, :encrypted_otp_secret_key, unique: true
 ```
+Save the file.
+
+#### Complete the setup
+Run the migration with:
+
+    bundle exec rake db:migrate
 
 Add the following line to your model to fully enable two-factor auth:
 
-    has_one_time_password
+    has_one_time_password(encrypted: true)
 
-Set config values to devise.rb, if desired:
+Set config values in `config/initializers/devise.rb`:
 
 ```ruby
-config.max_login_attempts = 3  # Maximum second factor attempts count
-config.allowed_otp_drift_seconds = 30  # Allowed time drift
+config.max_login_attempts = 3  # Maximum second factor attempts count.
+config.allowed_otp_drift_seconds = 30  # Allowed time drift between client and server.
 config.otp_length = 6  # OTP code length
-config.remember_otp_session_for_seconds = 30.days  # Time before browser has to enter OTP code again
+config.remember_otp_session_for_seconds = 30.days  # Time before browser has to enter OTP code again. Default is 0.
+config.otp_secret_encryption_key = ENV['OTP_SECRET_ENCRYPTION_KEY']
 ```
+The `otp_secret_encryption_key` must be a random key that is not stored in the
+DB, and is not checked in to your repo. It is recommended to store it in an
+environment variable, and you can generate it with `bundle exec rake secret`.
 
-Override the method to send one-time passwords in your model, this is automatically called when a user logs in:
+Override the method to send one-time passwords in your model. This is
+automatically called when a user logs in:
 
 ```ruby
 def send_two_factor_authentication_code
@@ -86,10 +96,10 @@ def send_two_factor_authentication_code
 end
 ```
 
-
 ### Customisation and Usage
 
-By default second factor authentication enabled for each user, you can change it with this method in your User model:
+By default, second factor authentication is required for each user. You can
+change that by overriding the following method in your model:
 
 ```ruby
 def need_two_factor_authentication?(request)
@@ -97,19 +107,29 @@ def need_two_factor_authentication?(request)
 end
 ```
 
-this will disable two factor authentication for local users
+In the example above, two factor authentication will not be required for local
+users.
 
-This gem is compatible with Google Authenticator (https://support.google.com/accounts/answer/1066447?hl=en).  You can generate provisioning uris by invoking the following method on your model:
+This gem is compatible with [Google Authenticator](https://support.google.com/accounts/answer/1066447?hl=en).
+You can generate provisioning uris by invoking the following method on your model:
 
-    user.provisioning_uri #This assumes a user model with an email attributes
+```ruby
+user.provisioning_uri # This assumes a user model with an email attribute
+```
 
-This provisioning uri can then be turned in to a QR code if desired so that users may add the app to Google Authenticator easily.  Once this is done they may retrieve a one-time password directly from the Google Authenticator app as well as through whatever method you define in `send_two_factor_authentication_code`
+This provisioning uri can then be turned in to a QR code if desired so that
+users may add the app to Google Authenticator easily.  Once this is done, they
+may retrieve a one-time password directly from the Google Authenticator app as
+well as through whatever method you define in
+`send_two_factor_authentication_code`.
 
 #### Overriding the view
 
-The default view that shows the form can be overridden by first adding a folder named: "two_factor_authentication" inside "app/views/devise", in here you want to create a "show.html.erb" view.
+The default view that shows the form can be overridden by adding a
+file named `show.html.erb` (or `show.html.haml` if you prefer HAML)
+inside `app/views/devise/two_factor_authentication/` and customizing it.
+Below is an example using ERB:
 
-The full path should be "app/views/devise/two_factor_authentication/show.html.erb"
 
 ```html
 <h2>Hi, you received a code by email, please enter it below, thanks!</h2>
@@ -125,21 +145,94 @@ The full path should be "app/views/devise/two_factor_authentication/show.html.er
 
 #### Updating existing users with OTP secret key
 
-If you have existing users that needs to be provided with a OTP secret key, so they can take benefit of the two factor authentication, create a rake. It could look like this one below:
+If you have existing users that need to be provided with a OTP secret key, so
+they can use two factor authentication, create a rake task. It could look like this one below:
 
 ```ruby
-desc "rake task to update users with otp secret key"
+desc 'rake task to update users with otp secret key'
 task :update_users_with_otp_secret_key  => :environment do
-	users = User.all
-
-	users.each do |user|
-		key = ROTP::Base32.random_base32
-		user.update_attributes(:otp_secret_key => key)
-		user.save
-		puts "Rake[:update_users_with_otp_secret_key] => User '#{user.email}' OTP secret key set to '#{key}'"
-	end
+  User.find_each do |user|
+    user.otp_secret_key = ROTP::Base32.random_base32
+    user.save!
+    puts "Rake[:update_users_with_otp_secret_key] => OTP secret key set to '#{key}' for User '#{user.email}'"
+  end
 end
 ```
+Then run the task with `bundle exec rake update_users_with_otp_secret_key`
+
+#### Adding the OTP encryption option to an existing app
+
+If you've already been using this gem, and want to start encrypting the OTP
+secret key in the database (recommended), you'll need to perform the following
+steps:
+
+1. Generate a migration to add the necessary columns to your model's table:
+
+   ```
+   rails g migration AddEncryptionFieldsToUsers encrypted_otp_secret_key:string:index encrypted_otp_secret_key_iv:string encrypted_otp_secret_key_salt:string
+   ```
+
+   Open your migration file (it will be in the `db/migrate` directory and will be
+   named something like `20151230163930_add_encryption_fields_to_users.rb`), and
+   add `unique: true` to the `add_index` line so that it looks like this:
+
+   ```ruby
+   add_index :users, :encrypted_otp_secret_key, unique: true
+   ```
+   Save the file.
+
+2. Run the migration: `bundle exec rake db:migrate`
+
+2. Update the gem: `bundle update two_factor_authentication`
+
+3. Add `encrypted: true` to `has_one_time_password` in your model.
+   For example: `has_one_time_password(encrypted: true)`
+
+4. Generate a migration to populate the new encryption fields:
+   ```
+   rails g migration PopulateEncryptedOtpFields
+   ```
+
+   Open the generated file, and replace its contents with the following:
+   ```ruby
+   class PopulateEncryptedOtpFields < ActiveRecord::Migration
+      def up
+        User.reset_column_information
+
+        User.find_each do |user|
+          user.otp_secret_key = user.read_attribute('otp_secret_key')
+          user.save!
+        end
+      end
+
+      def down
+        User.reset_column_information
+
+        User.find_each do |user|
+          user.otp_secret_key = ROTP::Base32.random_base32
+          user.save!
+        end
+      end
+    end
+  ```
+
+5. Generate a migration to remove the `:otp_secret_key` column:
+   ```
+   rails g migration RemoveOtpSecretKeyFromUsers otp_secret_key:string
+   ```
+
+6. Run the migrations: `bundle exec rake db:migrate`
+
+If, for some reason, you want to switch back to the old non-encrypted version,
+use these steps:
+
+1. Remove `(encrypted: true)` from `has_one_time_password`
+
+2. Roll back the last 3 migrations (assuming you haven't added any new ones
+after them):
+   ```
+   bundle exec rake db:rollback STEP=3
+   ```
 
 #### Executing some code after the user signs in and before they sign out
 
@@ -174,6 +267,6 @@ and you need different logic for each type of user, create a second class for
 your admin user, such as `AdminOtpSender`, with its own logic for
 `#reset_otp_state`.
 
-### Example
+### Example App
 
 [TwoFactorAuthenticationExample](https://github.com/Houdini/TwoFactorAuthenticationExample)

lib/generators/active_record/templates/migration.rb

@@ -1,15 +1,10 @@
 class TwoFactorAuthenticationAddTo<%= table_name.camelize %> < ActiveRecord::Migration
-  def up
-    change_table :<%= table_name %> do |t|
-      t.string   :otp_secret_key
-      t.integer  :second_factor_attempts_count, :default => 0
-    end
+  def change
+    add_column :<%= table_name %>, :second_factor_attempts_count, :integer, default: 0
+    add_column :<%= table_name %>, :encrypted_otp_secret_key, :string
+    add_column :<%= table_name %>, :encrypted_otp_secret_key_iv, :string
+    add_column :<%= table_name %>, :encrypted_otp_secret_key_salt, :string
 
-    add_index :<%= table_name %>, :otp_secret_key, :unique => true
-  end
-
-  def down
-    remove_column :<%= table_name %>, :otp_secret_key
-    remove_column :<%= table_name %>, :second_factor_attempts_count
+    add_index :<%= table_name %>, :encrypted_otp_secret_key, unique: true
   end
 end

lib/two_factor_authentication.rb

@@ -18,6 +18,9 @@ module Devise
 
   mattr_accessor :remember_otp_session_for_seconds
   @@remember_otp_session_for_seconds = 0
+
+  mattr_accessor :otp_secret_encryption_key
+  @@otp_secret_encryption_key = ''
 end
 
 module TwoFactorAuthentication

lib/two_factor_authentication/models/two_factor_authenticatable.rb

@@ -1,5 +1,6 @@
 require 'two_factor_authentication/hooks/two_factor_authenticatable'
 require 'rotp'
+require 'encryptor'
 
 module Devise
   module Models
@@ -8,46 +9,37 @@ module Devise
 
       module ClassMethods
         def has_one_time_password(options = {})
-
-          cattr_accessor :otp_column_name
-          self.otp_column_name = "otp_secret_key"
-
           include InstanceMethodsOnActivation
+          include EncryptionInstanceMethods if options[:encrypted] == true
 
           before_create { populate_otp_column }
-
-          if respond_to?(:attributes_protected_by_default)
-            def self.attributes_protected_by_default #:nodoc:
-              super + [self.otp_column_name]
-            end
-          end
         end
-        ::Devise::Models.config(self, :max_login_attempts, :allowed_otp_drift_seconds, :otp_length, :remember_otp_session_for_seconds)
+
+        ::Devise::Models.config(
+          self, :max_login_attempts, :allowed_otp_drift_seconds, :otp_length,
+          :remember_otp_session_for_seconds, :otp_secret_encryption_key)
       end
 
       module InstanceMethodsOnActivation
         def authenticate_otp(code, options = {})
-          totp = ROTP::TOTP.new(self.otp_column, { digits: options[:otp_length] || self.class.otp_length })
+          totp = ROTP::TOTP.new(
+            otp_secret_key, digits: options[:otp_length] || self.class.otp_length
+          )
           drift = options[:drift] || self.class.allowed_otp_drift_seconds
 
           totp.verify_with_drift(code, drift)
         end
 
         def otp_code(time = Time.now, options = {})
-          ROTP::TOTP.new(self.otp_column, { digits: options[:otp_length] || self.class.otp_length }).at(time, true)
+          ROTP::TOTP.new(
+            otp_secret_key,
+            digits: options[:otp_length] || self.class.otp_length
+          ).at(time, true)
         end
 
         def provisioning_uri(account = nil, options = {})
           account ||= self.email if self.respond_to?(:email)
-          ROTP::TOTP.new(self.otp_column, options).provisioning_uri(account)
-        end
-
-        def otp_column
-          self.send(self.class.otp_column_name)
-        end
-
-        def otp_column=(attr)
-          self.send("#{self.class.otp_column_name}=", attr)
+          ROTP::TOTP.new(otp_secret_key, options).provisioning_uri(account)
         end
 
         def need_two_factor_authentication?(request)
@@ -67,9 +59,83 @@ module Devise
         end
 
         def populate_otp_column
-          self.otp_column = ROTP::Base32.random_base32
+          self.otp_secret_key = ROTP::Base32.random_base32
+        end
+      end
+
+      module EncryptionInstanceMethods
+        def otp_secret_key
+          decrypt(encrypted_otp_secret_key)
+        end
+
+        def otp_secret_key=(value)
+          self.encrypted_otp_secret_key = encrypt(value)
+        end
+
+        private
+
+        def decrypt(encrypted_value)
+          return encrypted_value if encrypted_value.blank?
+
+          encrypted_value = encrypted_value.unpack('m').first
+
+          value = ::Encryptor.decrypt(encryption_options_for(encrypted_value))
+
+          if defined?(Encoding)
+            encoding = Encoding.default_internal || Encoding.default_external
+            value = value.force_encoding(encoding.name)
+          end
+
+          value
+        end
+
+        def encrypt(value)
+          return value if value.blank?
+
+          value = value.to_s
+          encrypted_value = ::Encryptor.encrypt(encryption_options_for(value))
+
+          encrypted_value = [encrypted_value].pack('m')
+
+          encrypted_value
+        end
+
+        def encryption_options_for(value)
+          {
+            value: value,
+            key: Devise.otp_secret_encryption_key,
+            iv: iv_for_attribute,
+            salt: salt_for_attribute
+          }
+        end
+
+        def iv_for_attribute(algorithm = 'aes-256-cbc')
+          iv = encrypted_otp_secret_key_iv
+
+          if iv.nil?
+            algo = OpenSSL::Cipher::Cipher.new(algorithm)
+            iv = [algo.random_iv].pack('m')
+            self.encrypted_otp_secret_key_iv = iv
+          end
+
+          iv.unpack('m').first if iv.present?
+        end
+
+        def salt_for_attribute
+          salt = encrypted_otp_secret_key_salt ||
+                 self.encrypted_otp_secret_key_salt = generate_random_base64_encoded_salt
+
+          decode_salt_if_encoded(salt)
         end
 
+        def generate_random_base64_encoded_salt
+          prefix = '_'
+          prefix + [SecureRandom.random_bytes].pack('m')
+        end
+
+        def decode_salt_if_encoded(salt)
+          salt.slice(0).eql?('_') ? salt.slice(1..-1).unpack('m').first : salt
+        end
       end
     end
   end

lib/two_factor_authentication/schema.rb

@@ -1,11 +1,19 @@
 module TwoFactorAuthentication
   module Schema
-    def otp_secret_key
-      apply_devise_schema :otp_secret_key, String
-    end
-
     def second_factor_attempts_count
       apply_devise_schema :second_factor_attempts_count, Integer, :default => 0
     end
+
+    def encrypted_otp_secret_key
+      apply_devise_schema :encrypted_otp_secret_key, String
+    end
+
+    def encrypted_otp_secret_key_iv
+      apply_devise_schema :encrypted_otp_secret_key_iv, String
+    end
+
+    def encrypted_otp_secret_key_salt
+      apply_devise_schema :encrypted_otp_secret_key_salt, String
+    end
   end
 end

spec/controllers/two_factor_auth_spec.rb

@@ -1,18 +0,0 @@
-require 'spec_helper'
-
-include Warden::Test::Helpers
-
-describe HomeController, :type => :controller do
-  context "passed only 1st factor auth" do
-    let(:user) { create_user }
-
-    describe "is_fully_authenticated helper" do
-      it "should be true" do
-        login_as user, scope: :user
-        visit user_two_factor_authentication_path
-
-        expect(controller.is_fully_authenticated?).to be_truthy
-      end
-    end
-  end
-end

spec/controllers/two_factor_authentication_controller_spec.rb

@@ -0,0 +1,33 @@
+require 'spec_helper'
+
+describe Devise::TwoFactorAuthenticationController, type: :controller do
+  describe 'is_fully_authenticated? helper' do
+    before do
+      sign_in
+    end
+
+    context 'after user enters valid OTP code' do
+      it 'returns true' do
+        post :update, code: controller.current_user.otp_code
+
+        expect(subject.is_fully_authenticated?).to eq true
+      end
+    end
+
+    context 'when user has not entered any OTP yet' do
+      it 'returns false' do
+        get :show
+
+        expect(subject.is_fully_authenticated?).to eq false
+      end
+    end
+
+    context 'when user enters an invalid OTP' do
+      it 'returns false' do
+        post :update, code: '12345'
+
+        expect(subject.is_fully_authenticated?).to eq false
+      end
+    end
+  end
+end

spec/features/two_factor_authenticatable_spec.rb

@@ -1,47 +1,64 @@
 require 'spec_helper'
+include AuthenticatedModelHelper
 
 feature "User of two factor authentication" do
-  let(:user) { create_user }
+  context 'sending two factor authentication code via SMS' do
+    shared_examples 'sends and authenticates code' do |user, type|
+      before do
+        if type == 'encrypted'
+          allow(User).to receive(:has_one_time_password).with(encrypted: true)
+        end
+      end
 
-  scenario "must be logged in" do
-    visit user_two_factor_authentication_path
+      it 'does not send an SMS before the user has signed in' do
+        expect(SMSProvider.messages).to be_empty
+      end
 
-    expect(page).to have_content("Welcome Home")
-    expect(page).to have_content("You are signed out")
-  end
+      it 'sends code via SMS after sign in' do
+        visit new_user_session_path
+        complete_sign_in_form_for(user)
 
-  scenario "sends two factor authentication code after sign in" do
-    expect(SMSProvider.messages).to be_empty
+        expect(page).to have_content 'Enter your personal code'
 
-    visit new_user_session_path
-    complete_sign_in_form_for(user)
+        expect(SMSProvider.messages.size).to eq(1)
+        message = SMSProvider.last_message
+        expect(message.to).to eq(user.phone_number)
+        expect(message.body).to eq(user.otp_code)
+      end
 
-    expect(page).to have_content "Enter your personal code"
+      it 'authenticates a valid OTP code' do
+        visit new_user_session_path
+        complete_sign_in_form_for(user)
 
-    expect(SMSProvider.messages.size).to eq(1)
-    message = SMSProvider.last_message
-    expect(message.to).to eq(user.phone_number)
-    expect(message.body).to eq(user.otp_code)
-  end
+        expect(page).to have_content('You are signed in as Marissa')
 
-  context "when logged in" do
+        fill_in 'code', with: user.otp_code
+        click_button 'Submit'
 
-    background do
-      login_as user
+        within('.flash.notice') do
+          expect(page).to have_content('Two factor authentication successful.')
+        end
+
+        expect(current_path).to eq root_path
+      end
     end
 
-    scenario "can fill in TFA code" do
-      visit user_two_factor_authentication_path
+    it_behaves_like 'sends and authenticates code', create_user('not_encrypted')
+    it_behaves_like 'sends and authenticates code', create_user, 'encrypted'
+  end
 
-      expect(page).to have_content("You are signed in as Marissa")
-      expect(page).to have_content("Enter your personal code")
+  scenario "must be logged in" do
+    visit user_two_factor_authentication_path
 
-      fill_in "code", with: user.otp_code
-      click_button "Submit"
+    expect(page).to have_content("Welcome Home")
+    expect(page).to have_content("You are signed out")
+  end
 
-      within(".flash.notice") do
-        expect(page).to have_content("Two factor authentication successful.")
-      end
+  context "when logged in" do
+    let(:user) { create_user }
+
+    background do
+      login_as user
     end
 
     scenario "is redirected to TFA when path requires authentication" do
@@ -122,6 +139,12 @@ feature "User of two factor authentication" do
         expect(page).to have_content("Enter your personal code")
       end
     end
+
+    it 'sets the warden session need_two_factor_authentication key to true' do
+      session_hash = { 'need_two_factor_authentication' => true }
+
+      expect(page.get_rack_session_key('warden.user.user.session')).to eq session_hash
+    end
   end
 
   describe 'signing in' do

spec/generators/active_record/two_factor_authentication_generator_spec.rb

@@ -0,0 +1,36 @@
+require 'spec_helper'
+
+require 'generators/active_record/two_factor_authentication_generator'
+
+describe ActiveRecord::Generators::TwoFactorAuthenticationGenerator, type: :generator do
+  destination File.expand_path('../../../../../tmp', __FILE__)
+
+  before do
+    prepare_destination
+  end
+
+  it 'runs all methods in the generator' do
+    gen = generator %w(users)
+    expect(gen).to receive(:copy_two_factor_authentication_migration)
+    gen.invoke_all
+  end
+
+  describe 'the generated files' do
+    before do
+      run_generator %w(users)
+    end
+
+    describe 'the migration' do
+      subject { migration_file('db/migrate/two_factor_authentication_add_to_users.rb') }
+
+      it { is_expected.to exist }
+      it { is_expected.to be_a_migration }
+      it { is_expected.to contain /def change/ }
+      it { is_expected.to contain /add_column :users, :second_factor_attempts_count, :integer, default: 0/ }
+      it { is_expected.to contain /add_column :users, :encrypted_otp_secret_key, :string/ }
+      it { is_expected.to contain /add_column :users, :encrypted_otp_secret_key_iv, :string/ }
+      it { is_expected.to contain /add_column :users, :encrypted_otp_secret_key_salt, :string/ }
+      it { is_expected.to contain /add_index :users, :encrypted_otp_secret_key, unique: true/ }
+    end
+  end
+end

spec/lib/two_factor_authentication/models/two_factor_authenticatable_spec.rb

@@ -1,168 +1,264 @@
 require 'spec_helper'
 include AuthenticatedModelHelper
 
-describe Devise::Models::TwoFactorAuthenticatable, '#otp_code' do
-  let(:instance) { build_guest_user }
-  subject { instance.otp_code(time) }
-  let(:time) { 1392852456 }
-
-  it "should return an error if no secret is set" do
-    expect {
-      subject
-    }.to raise_error Exception
+describe Devise::Models::TwoFactorAuthenticatable do
+  describe '#otp_code' do
+    shared_examples 'otp_code' do |instance|
+      subject { instance.otp_code(time) }
+      let(:time) { 1_392_852_456 }
+
+      it 'returns an error if no secret is set' do
+        expect { subject }.to raise_error Exception
+      end
+
+      context 'secret is set' do
+        before :each do
+          instance.otp_secret_key = '2z6hxkdwi3uvrnpn'
+        end
+
+        it 'does not return an error' do
+          subject
+        end
+
+        it 'matches Devise configured length' do
+          expect(subject.length).to eq(Devise.otp_length)
+        end
+
+        context 'with a known time' do
+          let(:time) { 1_392_852_756 }
+
+          it 'returns a known result' do
+            expect(subject).
+              to eq('0000000524562202'.split(//).last(Devise.otp_length).join)
+          end
+        end
+
+        context 'with a known time yielding a result with less than 6 digits' do
+          let(:time) { 1_393_065_856 }
+
+          it 'returns a known result padded with zeroes' do
+            expect(subject).
+              to eq('0000001608007672'.split(//).last(Devise.otp_length).join)
+          end
+        end
+      end
+    end
+
+    it_behaves_like 'otp_code', GuestUser.new
+    it_behaves_like 'otp_code', EncryptedUser.new
   end
 
-  context "secret is set" do
-    before :each do
-      instance.otp_secret_key = "2z6hxkdwi3uvrnpn"
+  describe '#authenticate_otp' do
+    shared_examples 'authenticate_otp' do |instance|
+      before :each do
+        instance.otp_secret_key = '2z6hxkdwi3uvrnpn'
+      end
+
+      def do_invoke(code, user)
+        user.authenticate_otp(code)
+      end
+
+      it 'authenticates a recently created code' do
+        code = instance.otp_code
+        expect(do_invoke(code, instance)).to eq(true)
+      end
+
+      it 'does not authenticate an old code' do
+        code = instance.otp_code(1.minutes.ago.to_i)
+        expect(do_invoke(code, instance)).to eq(false)
+      end
     end
 
-    it "should not return an error" do
-      subject
+    it_behaves_like 'authenticate_otp', GuestUser.new
+    it_behaves_like 'authenticate_otp', EncryptedUser.new
+  end
+
+  describe '#send_two_factor_authentication_code' do
+    let(:instance) { build_guest_user }
+
+    it 'raises an error by default' do
+      expect { instance.send_two_factor_authentication_code }.
+        to raise_error(NotImplementedError)
     end
 
-    it "should be configured length" do
-      expect(subject.length).to eq(Devise.otp_length)
+    it 'is overrideable' do
+      def instance.send_two_factor_authentication_code
+        'Code sent'
+      end
+      expect(instance.send_two_factor_authentication_code).to eq('Code sent')
     end
+  end
 
-    context "with a known time" do
-      let(:time) { 1392852756 }
+  describe '#provisioning_uri' do
+    shared_examples 'provisioning_uri' do |instance|
+      before do
+        instance.email = 'houdini@example.com'
+        instance.run_callbacks :create
+      end
 
-      it "should return a known result" do
-        expect(subject).to eq("0000000524562202".split(//).last(Devise.otp_length).join)
+      it "returns uri with user's email" do
+        expect(instance.provisioning_uri).
+          to match(%r{otpauth://totp/houdini@example.com\?secret=\w{16}})
+      end
+
+      it 'returns uri with issuer option' do
+        expect(instance.provisioning_uri('houdini')).
+          to match(%r{otpauth://totp/houdini\?secret=\w{16}$})
       end
-    end
 
-    context "with a known time yielding a result with less than 6 digits" do
-      let(:time) { 1393065856 }
+      it 'returns uri with issuer option' do
+        require 'cgi'
 
-      it "should return a known result padded with zeroes" do
-        expect(subject).to eq("0000001608007672".split(//).last(Devise.otp_length).join)
+        uri = URI.parse(instance.provisioning_uri('houdini', issuer: 'Magic'))
+        params = CGI.parse(uri.query)
+
+        expect(uri.scheme).to eq('otpauth')
+        expect(uri.host).to eq('totp')
+        expect(uri.path).to eq('/houdini')
+        expect(params['issuer'].shift).to eq('Magic')
+        expect(params['secret'].shift).to match(/\w{16}/)
       end
     end
+
+    it_behaves_like 'provisioning_uri', GuestUser.new
+    it_behaves_like 'provisioning_uri', EncryptedUser.new
   end
-end
 
-describe Devise::Models::TwoFactorAuthenticatable, '#authenticate_otp' do
-  let(:instance) { build_guest_user }
+  describe '#populate_otp_column' do
+    shared_examples 'populate_otp_column' do |klass|
+      let(:instance) { klass.new }
 
-  before :each do
-    instance.otp_secret_key = "2z6hxkdwi3uvrnpn"
-  end
+      it 'populates otp_column on create' do
+        expect(instance.otp_secret_key).to be_nil
 
-  def do_invoke code, options = {}
-    instance.authenticate_otp(code, options)
-  end
+        # populate_otp_column called via before_create
+        instance.run_callbacks :create
 
-  it "should be able to authenticate a recently created code" do
-    code = instance.otp_code
-    expect(do_invoke(code)).to eq(true)
-  end
+        expect(instance.otp_secret_key).to match(/\w{16}/)
+      end
 
-  it "should not authenticate an old code" do
-    code = instance.otp_code(1.minutes.ago.to_i)
-    expect(do_invoke(code)).to eq(false)
-  end
-end
+      it 'repopulates otp_column' do
+        instance.run_callbacks :create
+        original_key = instance.otp_secret_key
 
-describe Devise::Models::TwoFactorAuthenticatable, '#send_two_factor_authentication_code' do
-  let(:instance) { build_guest_user }
+        instance.populate_otp_column
 
-  it "should raise an error by default" do
-    expect {
-      instance.send_two_factor_authentication_code
-    }.to raise_error(NotImplementedError)
+        expect(instance.otp_secret_key).to match(/\w{16}/)
+        expect(instance.otp_secret_key).to_not eq(original_key)
+      end
+    end
+
+    it_behaves_like 'populate_otp_column', GuestUser
+    it_behaves_like 'populate_otp_column', EncryptedUser
   end
 
-  it "should be overrideable" do
-    def instance.send_two_factor_authentication_code
-      "Code sent"
+  describe '#max_login_attempts' do
+    let(:instance) { build_guest_user }
+
+    before do
+      @original_max_login_attempts = GuestUser.max_login_attempts
+      GuestUser.max_login_attempts = 3
     end
-    expect(instance.send_two_factor_authentication_code).to eq("Code sent")
-  end
-end
 
-describe Devise::Models::TwoFactorAuthenticatable, '#provisioning_uri' do
-  let(:instance) { build_guest_user }
+    after { GuestUser.max_login_attempts = @original_max_login_attempts }
 
-  before do
-    instance.email = "houdini@example.com"
-    instance.run_callbacks :create
-  end
+    it 'returns class setting' do
+      expect(instance.max_login_attempts).to eq(3)
+    end
 
-  it "should return uri with user's email" do
-    expect(instance.provisioning_uri).to match(%r{otpauth://totp/houdini@example.com\?secret=\w{16}})
-  end
+    it 'returns false as boolean' do
+      instance.second_factor_attempts_count = nil
+      expect(instance.max_login_attempts?).to be_falsey
+      instance.second_factor_attempts_count = 0
+      expect(instance.max_login_attempts?).to be_falsey
+      instance.second_factor_attempts_count = 1
+      expect(instance.max_login_attempts?).to be_falsey
+      instance.second_factor_attempts_count = 2
+      expect(instance.max_login_attempts?).to be_falsey
+    end
 
-  it "should return uri with issuer option" do
-    expect(instance.provisioning_uri("houdini")).to match(%r{otpauth://totp/houdini\?secret=\w{16}$})
+    it 'returns true as boolean after too many attempts' do
+      instance.second_factor_attempts_count = 3
+      expect(instance.max_login_attempts?).to be_truthy
+      instance.second_factor_attempts_count = 4
+      expect(instance.max_login_attempts?).to be_truthy
+    end
   end
 
-  it "should return uri with issuer option" do
-    require 'cgi'
+  describe '.has_one_time_password' do
+    context 'when encrypted: true option is passed' do
+      let(:instance) { EncryptedUser.new }
 
-    uri = URI.parse(instance.provisioning_uri("houdini", issuer: 'Magic'))
-    params = CGI::parse(uri.query)
+      it 'encrypts otp_secret_key with iv, salt, and encoding' do
+        instance.otp_secret_key = '2z6hxkdwi3uvrnpn'
 
-    expect(uri.scheme).to eq("otpauth")
-    expect(uri.host).to eq("totp")
-    expect(uri.path).to eq("/houdini")
-    expect(params['issuer'].shift).to eq('Magic')
-    expect(params['secret'].shift).to match(%r{\w{16}})
-  end
-end
+        expect(instance.encrypted_otp_secret_key).to match(/.{44}/)
+
+        expect(instance.encrypted_otp_secret_key_iv).to match(/.{24}/)
 
-describe Devise::Models::TwoFactorAuthenticatable, '#populate_otp_column' do
-  let(:instance) { build_guest_user }
+        expect(instance.encrypted_otp_secret_key_salt).to match(/.{25}/)
+      end
 
-  it "populates otp_column on create" do
-    expect(instance.otp_secret_key).to be_nil
+      it 'does not encrypt a nil otp_secret_key' do
+        instance.otp_secret_key = nil
 
-    instance.run_callbacks :create # populate_otp_column called via before_create
+        expect(instance.encrypted_otp_secret_key).to be_nil
 
-    expect(instance.otp_secret_key).to match(%r{\w{16}})
-  end
+        expect(instance.encrypted_otp_secret_key_iv).to be_nil
 
-  it "repopulates otp_column" do
-    instance.run_callbacks :create
-    original_key = instance.otp_secret_key
+        expect(instance.encrypted_otp_secret_key_salt).to be_nil
+      end
 
-    instance.populate_otp_column
+      it 'does not encrypt an empty otp_secret_key' do
+        instance.otp_secret_key = ''
 
-    expect(instance.otp_secret_key).to match(%r{\w{16}})
-    expect(instance.otp_secret_key).to_not eq(original_key)
-  end
-end
+        expect(instance.encrypted_otp_secret_key).to eq ''
 
-describe Devise::Models::TwoFactorAuthenticatable, '#max_login_attempts' do
-  let(:instance) { build_guest_user }
+        expect(instance.encrypted_otp_secret_key_iv).to be_nil
 
-  before do
-    @original_max_login_attempts = GuestUser.max_login_attempts
-    GuestUser.max_login_attempts = 3
-  end
+        expect(instance.encrypted_otp_secret_key_salt).to be_nil
+      end
 
-  after { GuestUser.max_login_attempts = @original_max_login_attempts }
+      it 'raises an error when Devise.otp_secret_encryption_key is not set' do
+        allow(Devise).to receive(:otp_secret_encryption_key).and_return nil
 
-  it "returns class setting" do
-    expect(instance.max_login_attempts).to eq(3)
-  end
+        # This error is raised by the encryptor gem
+        expect { instance.otp_secret_key = '2z6hxkdwi3uvrnpn' }.
+          to raise_error ArgumentError, 'must specify a :key'
+      end
 
-  it "returns false as boolean" do
-    instance.second_factor_attempts_count = nil
-    expect(instance.max_login_attempts?).to be_falsey
-    instance.second_factor_attempts_count = 0
-    expect(instance.max_login_attempts?).to be_falsey
-    instance.second_factor_attempts_count = 1
-    expect(instance.max_login_attempts?).to be_falsey
-    instance.second_factor_attempts_count = 2
-    expect(instance.max_login_attempts?).to be_falsey
-  end
+      it 'passes in the correct options to Encryptor' do
+        instance.otp_secret_key = 'testing'
+        iv = instance.encrypted_otp_secret_key_iv
+        salt = instance.encrypted_otp_secret_key_salt
+
+        encrypted = Encryptor.encrypt(
+          value: 'testing',
+          key: Devise.otp_secret_encryption_key,
+          iv: iv.unpack('m').first,
+          salt: salt.unpack('m').first
+        )
 
-  it "returns true as boolean after too many attempts" do
-    instance.second_factor_attempts_count = 3
-    expect(instance.max_login_attempts?).to be_truthy
-    instance.second_factor_attempts_count = 4
-    expect(instance.max_login_attempts?).to be_truthy
+        expect(instance.encrypted_otp_secret_key).to eq [encrypted].pack('m')
+      end
+
+      it 'varies the iv per instance' do
+        instance.otp_secret_key = 'testing'
+        user2 = EncryptedUser.new
+        user2.otp_secret_key = 'testing'
+
+        expect(user2.encrypted_otp_secret_key_iv).
+          to_not eq instance.encrypted_otp_secret_key_iv
+      end
+
+      it 'varies the salt per instance' do
+        instance.otp_secret_key = 'testing'
+        user2 = EncryptedUser.new
+        user2.otp_secret_key = 'testing'
+
+        expect(user2.encrypted_otp_secret_key_salt).
+          to_not eq instance.encrypted_otp_secret_key_salt
+      end
+    end
   end
 end

spec/rails_app/app/models/encrypted_user.rb

@@ -0,0 +1,14 @@
+class EncryptedUser
+  extend ActiveModel::Callbacks
+  include ActiveModel::Validations
+  include Devise::Models::TwoFactorAuthenticatable
+
+  define_model_callbacks :create
+  attr_accessor :encrypted_otp_secret_key,
+                :encrypted_otp_secret_key_iv,
+                :encrypted_otp_secret_key_salt,
+                :email,
+                :second_factor_attempts_count
+
+  has_one_time_password(encrypted: true)
+end

spec/rails_app/app/models/user.rb

@@ -1,7 +1,6 @@
 class User < ActiveRecord::Base
   devise :two_factor_authenticatable, :database_authenticatable, :registerable,
-         :recoverable, :rememberable, :trackable, :validatable,
-         :two_factor_authenticatable
+         :recoverable, :rememberable, :trackable, :validatable
 
   has_one_time_password
 

spec/rails_app/config/environments/test.rb

@@ -35,4 +35,7 @@ Dummy::Application.configure do
 
   # Print deprecation notices to the stderr
   config.active_support.deprecation = :stderr
+
+  # For testing session variables in Capybara specs
+  config.middleware.use RackSessionAccess::Middleware
 end

spec/rails_app/config/initializers/devise.rb

@@ -253,4 +253,6 @@ Devise.setup do |config|
   # When using omniauth, Devise cannot automatically set Omniauth path,
   # so you need to do it manually. For the users scope, it would be:
   # config.omniauth_path_prefix = '/my_engine/users/auth'
+
+  config.otp_secret_encryption_key = '0a8283fba984da1de24e4df1e93046cb53c5787944ef037b2dbf3e61d20fe11f25e25a855cec605fdf65b162329890d7230afdf64f681b4c32020281054e73ec'
 end

spec/rails_app/db/migrate/20151224171231_add_encrypted_columns_to_user.rb

@@ -0,0 +1,9 @@
+class AddEncryptedColumnsToUser < ActiveRecord::Migration
+  def change
+    add_column :users, :encrypted_otp_secret_key, :string
+    add_column :users, :encrypted_otp_secret_key_iv, :string
+    add_column :users, :encrypted_otp_secret_key_salt, :string
+
+    add_index :users, :encrypted_otp_secret_key, unique: true
+  end
+end

spec/rails_app/db/migrate/20151224180310_populate_otp_column.rb

@@ -0,0 +1,19 @@
+class PopulateOtpColumn < ActiveRecord::Migration
+  def up
+    User.reset_column_information
+
+    User.find_each do |user|
+      user.otp_secret_key = user.read_attribute('otp_secret_key')
+      user.save!
+    end
+  end
+
+  def down
+    User.reset_column_information
+
+    User.find_each do |user|
+      user.otp_secret_key = ROTP::Base32.random_base32
+      user.save!
+    end
+  end
+end

spec/rails_app/db/migrate/20151228230340_remove_otp_secret_key_from_user.rb

@@ -0,0 +1,5 @@
+class RemoveOtpSecretKeyFromUser < ActiveRecord::Migration
+  def change
+    remove_column :users, :otp_secret_key, :string
+  end
+end

spec/rails_app/db/schema.rb

@@ -9,30 +9,32 @@
 # from scratch. The latter is a flawed and unsustainable approach (the more migrations
 # you'll amass, the slower it'll run and the greater likelihood for issues).
 #
-# It's strongly recommended to check this file into your version control system.
+# It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(:version => 20140407215513) do
+ActiveRecord::Schema.define(version: 20151228230340) do
 
-  create_table "users", :force => true do |t|
-    t.string   "email",                                      :default => "", :null => false
-    t.string   "encrypted_password",                         :default => "", :null => false
+  create_table "users", force: :cascade do |t|
+    t.string   "email",                                    default: "", null: false
+    t.string   "encrypted_password",                       default: "", null: false
     t.string   "reset_password_token"
     t.datetime "reset_password_sent_at"
     t.datetime "remember_created_at"
-    t.integer  "sign_in_count",                              :default => 0,  :null => false
+    t.integer  "sign_in_count",                            default: 0,  null: false
     t.datetime "current_sign_in_at"
     t.datetime "last_sign_in_at"
     t.string   "current_sign_in_ip"
     t.string   "last_sign_in_ip"
-    t.datetime "created_at",                                                 :null => false
-    t.datetime "updated_at",                                                 :null => false
-    t.string   "otp_secret_key"
-    t.integer  "second_factor_attempts_count",               :default => 0
-    t.string   "nickname",                     :limit => 64
+    t.datetime "created_at",                                            null: false
+    t.datetime "updated_at",                                            null: false
+    t.integer  "second_factor_attempts_count",             default: 0
+    t.string   "nickname",                      limit: 64
+    t.string   "encrypted_otp_secret_key"
+    t.string   "encrypted_otp_secret_key_iv"
+    t.string   "encrypted_otp_secret_key_salt"
   end
 
-  add_index "users", ["email"], :name => "index_users_on_email", :unique => true
-  add_index "users", ["otp_secret_key"], :name => "index_users_on_otp_secret_key", :unique => true
-  add_index "users", ["reset_password_token"], :name => "index_users_on_reset_password_token", :unique => true
+  add_index "users", ["email"], name: "index_users_on_email", unique: true
+  add_index "users", ["encrypted_otp_secret_key"], name: "index_users_on_encrypted_otp_secret_key", unique: true
+  add_index "users", ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true
 
 end

spec/spec_helper.rb

@@ -3,6 +3,7 @@ require File.expand_path("../rails_app/config/environment.rb",  __FILE__)
 
 require 'rspec/rails'
 require 'timecop'
+require 'rack_session_access/capybara'
 
 # See http://rubydoc.info/gems/rspec-core/RSpec/Core/Configuration
 RSpec.configure do |config|

spec/support/authenticated_model_helper.rb

@@ -1,10 +1,11 @@
 module AuthenticatedModelHelper
-
   def build_guest_user
     GuestUser.new
   end
 
-  def create_user(attributes={})
+  def create_user(type = 'encrypted', attributes = {})
+    create_table_for_nonencrypted_user if type == 'not_encrypted'
+
     User.create!(valid_attributes(attributes))
   end
 
@@ -23,6 +24,29 @@ module AuthenticatedModelHelper
     "user#{@@email_count}@example.com"
   end
 
+  def create_table_for_nonencrypted_user
+    silence_stream(STDOUT) do
+      ActiveRecord::Schema.define(version: 1) do
+        create_table 'users', force: :cascade do |t|
+          t.string   'email', default: '', null: false
+          t.string   'encrypted_password', default: '', null: false
+          t.string   'reset_password_token'
+          t.datetime 'reset_password_sent_at'
+          t.datetime 'remember_created_at'
+          t.integer  'sign_in_count', default: 0,  null: false
+          t.datetime 'current_sign_in_at'
+          t.datetime 'last_sign_in_at'
+          t.string   'current_sign_in_ip'
+          t.string   'last_sign_in_ip'
+          t.datetime 'created_at', null: false
+          t.datetime 'updated_at', null: false
+          t.integer  'second_factor_attempts_count', default: 0
+          t.string   'nickname', limit: 64
+          t.string   'otp_secret_key'
+        end
+      end
+    end
+  end
 end
 
 RSpec.configuration.send(:include, AuthenticatedModelHelper)

spec/support/controller_helper.rb

@@ -0,0 +1,16 @@
+module ControllerHelper
+  def sign_in(user = create_user('not_encrypted'))
+    allow(warden).to receive(:authenticated?).with(:user).and_return(true)
+    allow(controller).to receive(:current_user).and_return(user)
+    warden.session(:user)[TwoFactorAuthentication::NEED_AUTHENTICATION] = true
+  end
+end
+
+RSpec.configure do |config|
+  config.include Devise::TestHelpers, type: :controller
+  config.include ControllerHelper, type: :controller
+
+  config.before(:example, type: :controller) do
+    @request.env['devise.mapping'] = Devise.mappings[:user]
+  end
+end

spec/support/features_spec_helper.rb

@@ -15,5 +15,13 @@ end
 RSpec.configure do |config|
   config.include Warden::Test::Helpers, type: :feature
   config.include FeaturesSpecHelper, type: :feature
+
+  config.before(:each) do
+    Warden.test_mode!
+  end
+
+  config.after(:each) do
+    Warden.test_reset!
+  end
 end
 

two_factor_authentication.gemspec

@@ -28,6 +28,7 @@ Gem::Specification.new do |s|
   s.add_runtime_dependency 'devise'
   s.add_runtime_dependency 'randexp'
   s.add_runtime_dependency 'rotp'
+  s.add_runtime_dependency 'encryptor'
 
   s.add_development_dependency 'bundler'
   s.add_development_dependency 'rake'