about summary refs log tree commit diff
diff options
context:
space:
mode:
authorPatrick Figel <patrick@figel.email>2017-04-15 13:26:03 +0200
committerEugen <eugen@zeonfederated.com>2017-04-15 13:26:03 +0200
commitdf4ff9a8e13d776e1670c232655db0275a353a0f (patch)
treea7bdb4c0240e169bac01bf67b76f685e9a9b4a67
parent67ad84b7ebf080d6a6cbcb7d299e02c2a51d955e (diff)
Add recovery code support for two-factor auth (#1773)
* Add recovery code support for two-factor auth

When users enable two-factor auth, the app now generates ten
single-use recovery codes. Users are encouraged to print the codes
and store them in a safe place.

The two-factor prompt during login now accepts both OTP codes and
recovery codes.

The two-factor settings UI allows users to regenerated lost
recovery codes. Users who have set up two-factor auth prior to
this feature being added can use it to generate recovery codes
for the first time.

Fixes #563 and fixes #987

* Set OTP_SECRET in test enviroment

* add missing .html to view file names
-rw-r--r--.env.test1
-rw-r--r--app/assets/stylesheets/lists.scss9
-rw-r--r--app/controllers/auth/sessions_controller.rb3
-rw-r--r--app/controllers/settings/two_factor_auths_controller.rb10
-rw-r--r--app/models/user.rb4
-rw-r--r--app/views/auth/sessions/two_factor.html.haml4
-rw-r--r--app/views/settings/two_factor_auths/_recovery_codes.html.haml7
-rw-r--r--app/views/settings/two_factor_auths/create.html.haml4
-rw-r--r--app/views/settings/two_factor_auths/recovery_codes.html.haml4
-rw-r--r--app/views/settings/two_factor_auths/show.html.haml5
-rw-r--r--config/initializers/devise.rb1
-rw-r--r--config/locales/en.yml5
-rw-r--r--config/locales/simple_form.en.yml2
-rw-r--r--config/routes.rb1
-rw-r--r--db/migrate/20170414080609_add_devise_two_factor_backupable_to_users.rb5
-rw-r--r--db/schema.rb3
-rw-r--r--spec/controllers/auth/sessions_controller_spec.rb93
-rw-r--r--spec/models/user_spec.rb3
18 files changed, 149 insertions, 15 deletions
diff --git a/.env.test b/.env.test
index b57f52e30..e25c040ac 100644
--- a/.env.test
+++ b/.env.test
@@ -1,3 +1,4 @@
 # Federation
 LOCAL_DOMAIN=cb6e6126.ngrok.io
 LOCAL_HTTPS=true
+OTP_SECRET=100c7faeef00caa29242f6b04156742bf76065771fd4117990c4282b8748ff3d99f8fdae97c982ab5bd2e6756a159121377cce4421f4a8ecd2d67bd7749a3fb4
diff --git a/app/assets/stylesheets/lists.scss b/app/assets/stylesheets/lists.scss
index eac9f5a2c..13243aae5 100644
--- a/app/assets/stylesheets/lists.scss
+++ b/app/assets/stylesheets/lists.scss
@@ -6,3 +6,12 @@
     margin: 0 5px;
   }
 }
+
+.recovery-codes {
+  column-count: 2;
+  height: 100px;
+  li {
+    list-style: decimal;
+    margin-left: 20px;
+  }
+}
diff --git a/app/controllers/auth/sessions_controller.rb b/app/controllers/auth/sessions_controller.rb
index 4184750f3..a187ae6da 100644
--- a/app/controllers/auth/sessions_controller.rb
+++ b/app/controllers/auth/sessions_controller.rb
@@ -49,7 +49,8 @@ class Auth::SessionsController < Devise::SessionsController
   end
 
   def valid_otp_attempt?(user)
-    user.validate_and_consume_otp!(user_params[:otp_attempt])
+    user.validate_and_consume_otp!(user_params[:otp_attempt]) ||
+      user.invalidate_otp_backup_code!(user_params[:otp_attempt])
   end
 
   def authenticate_with_two_factor
diff --git a/app/controllers/settings/two_factor_auths_controller.rb b/app/controllers/settings/two_factor_auths_controller.rb
index 203d1fc46..bfe3868f3 100644
--- a/app/controllers/settings/two_factor_auths_controller.rb
+++ b/app/controllers/settings/two_factor_auths_controller.rb
@@ -19,9 +19,9 @@ class Settings::TwoFactorAuthsController < ApplicationController
   def create
     if current_user.validate_and_consume_otp!(confirmation_params[:code])
       current_user.otp_required_for_login = true
+      @codes = current_user.generate_otp_backup_codes!
       current_user.save!
-
-      redirect_to settings_two_factor_auth_path, notice: I18n.t('two_factor_auth.enabled_success')
+      flash[:notice] = I18n.t('two_factor_auth.enabled_success')
     else
       @confirmation = Form::TwoFactorConfirmation.new
       set_qr_code
@@ -30,6 +30,12 @@ class Settings::TwoFactorAuthsController < ApplicationController
     end
   end
 
+  def recovery_codes
+    @codes = current_user.generate_otp_backup_codes!
+    current_user.save!
+    flash[:notice] = I18n.t('two_factor_auth.recovery_codes_regenerated')
+  end
+
   def disable
     current_user.otp_required_for_login = false
     current_user.save!
diff --git a/app/models/user.rb b/app/models/user.rb
index d2aa5d809..27a38674e 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -5,7 +5,9 @@ class User < ApplicationRecord
 
   devise :registerable, :recoverable,
          :rememberable, :trackable, :validatable, :confirmable,
-         :two_factor_authenticatable, otp_secret_encryption_key: ENV['OTP_SECRET']
+         :two_factor_authenticatable, :two_factor_backupable,
+         otp_secret_encryption_key: ENV['OTP_SECRET'],
+         otp_number_of_backup_codes: 10
 
   belongs_to :account, inverse_of: :user
   accepts_nested_attributes_for :account
diff --git a/app/views/auth/sessions/two_factor.html.haml b/app/views/auth/sessions/two_factor.html.haml
index 1deff82b2..3dec40c44 100644
--- a/app/views/auth/sessions/two_factor.html.haml
+++ b/app/views/auth/sessions/two_factor.html.haml
@@ -2,7 +2,9 @@
   = t('auth.login')
 
 = simple_form_for(resource, as: resource_name, url: session_path(resource_name), method: :post) do |f|
-  = f.input :otp_attempt, type: :number, placeholder: t('simple_form.labels.defaults.otp_attempt'), input_html: { 'aria-label' => t('simple_form.labels.defaults.otp_attempt') }, required: true, autofocus: true, autocomplete: 'off'
+  = f.input :otp_attempt, type: :number, placeholder: t('simple_form.labels.defaults.otp_attempt'),
+      input_html: { 'aria-label' => t('simple_form.labels.defaults.otp_attempt') }, required: true, autofocus: true, autocomplete: 'off',
+      hint: t('simple_form.hints.sessions.otp')
 
   .actions
     = f.button :button, t('auth.login'), type: :submit
diff --git a/app/views/settings/two_factor_auths/_recovery_codes.html.haml b/app/views/settings/two_factor_auths/_recovery_codes.html.haml
new file mode 100644
index 000000000..c23311e2a
--- /dev/null
+++ b/app/views/settings/two_factor_auths/_recovery_codes.html.haml
@@ -0,0 +1,7 @@
+%p.hint= t('two_factor_auth.recovery_instructions')
+
+%h3= t('two_factor_auth.recovery_codes')
+%ol.recovery-codes
+  - @codes.each do |code|
+    %li
+      %samp= code
diff --git a/app/views/settings/two_factor_auths/create.html.haml b/app/views/settings/two_factor_auths/create.html.haml
new file mode 100644
index 000000000..8710b6e02
--- /dev/null
+++ b/app/views/settings/two_factor_auths/create.html.haml
@@ -0,0 +1,4 @@
+- content_for :page_title do
+  = t('settings.two_factor_auth')
+
+= render 'recovery_codes'
diff --git a/app/views/settings/two_factor_auths/recovery_codes.html.haml b/app/views/settings/two_factor_auths/recovery_codes.html.haml
new file mode 100644
index 000000000..8710b6e02
--- /dev/null
+++ b/app/views/settings/two_factor_auths/recovery_codes.html.haml
@@ -0,0 +1,4 @@
+- content_for :page_title do
+  = t('settings.two_factor_auth')
+
+= render 'recovery_codes'
diff --git a/app/views/settings/two_factor_auths/show.html.haml b/app/views/settings/two_factor_auths/show.html.haml
index 047fe0c54..bf19d24f1 100644
--- a/app/views/settings/two_factor_auths/show.html.haml
+++ b/app/views/settings/two_factor_auths/show.html.haml
@@ -8,3 +8,8 @@
     = link_to t('two_factor_auth.disable'), disable_settings_two_factor_auth_path, data: { method: 'POST' }, class: 'block-button'
   - else
     = link_to t('two_factor_auth.setup'), new_settings_two_factor_auth_path, class: 'block-button'
+
+- if current_user.otp_required_for_login
+  .simple_form
+    %p.hint= t('two_factor_auth.lost_recovery_codes')
+    = link_to t('two_factor_auth.generate_recovery_codes'), recovery_codes_settings_two_factor_auth_path, data: { method: 'POST' }, class: 'block-button'
diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb
index 3c23e7b2e..4754c2c8c 100644
--- a/config/initializers/devise.rb
+++ b/config/initializers/devise.rb
@@ -1,6 +1,7 @@
 Devise.setup do |config|
   config.warden do |manager|
     manager.default_strategies(scope: :user).unshift :two_factor_authenticatable
+    manager.default_strategies(scope: :user).unshift :two_factor_backupable
   end
 
   # The secret key used by Devise. Devise uses this key to generate
diff --git a/config/locales/en.yml b/config/locales/en.yml
index e2f187399..474de3985 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -290,8 +290,13 @@ en:
     disable: Disable
     enable: Enable
     enabled_success: Two-factor authentication successfully enabled
+    generate_recovery_codes: Generate Recovery Codes
     instructions_html: "<strong>Scan this QR code into Google Authenticator or a similiar TOTP app on your phone</strong>. From now on, that app will generate tokens that you will have to enter when logging in."
+    lost_recovery_codes: Recovery codes allow you to regain access to your account if you lose your phone. If you've lost your recovery codes, you can regenerate them here. Your old recovery codes will be invalidated.
     manual_instructions: 'If you can''t scan the QR code and need to enter it manually, here is the plain-text secret:'
+    recovery_codes: Recovery Codes
+    recovery_codes_regenerated: Recovery codes successfully regenerated
+    recovery_instructions: If you ever lose access to your phone, you can use one of the recovery codes below to regain access to your account. Keep the recovery codes safe, for example by printing them and storing them with other important documents.
     setup: Set up
     warning: If you cannot configure an authenticator app right now, you should click "disable" or you won't be able to login.
     wrong_code: The entered code was invalid! Are server time and device time correct?
diff --git a/config/locales/simple_form.en.yml b/config/locales/simple_form.en.yml
index 74649da51..c25407f2b 100644
--- a/config/locales/simple_form.en.yml
+++ b/config/locales/simple_form.en.yml
@@ -10,6 +10,8 @@ en:
         note: At most 160 characters
       imports:
         data: CSV file exported from another Mastodon instance
+      sessions:
+        otp: Enter the Two-factor code from your phone or use one of your recovery codes.
     labels:
       defaults:
         avatar: Avatar
diff --git a/config/routes.rb b/config/routes.rb
index 045be940e..8dcd4b330 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -64,6 +64,7 @@ Rails.application.routes.draw do
     resource :two_factor_auth, only: [:show, :new, :create] do
       member do
         post :disable
+        post :recovery_codes
       end
     end
   end
diff --git a/db/migrate/20170414080609_add_devise_two_factor_backupable_to_users.rb b/db/migrate/20170414080609_add_devise_two_factor_backupable_to_users.rb
new file mode 100644
index 000000000..65517d9f4
--- /dev/null
+++ b/db/migrate/20170414080609_add_devise_two_factor_backupable_to_users.rb
@@ -0,0 +1,5 @@
+class AddDeviseTwoFactorBackupableToUsers < ActiveRecord::Migration[5.0]
+  def change
+    add_column :users, :otp_backup_codes, :string, array: true
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 2decf5471..5f995ebda 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20170406215816) do
+ActiveRecord::Schema.define(version: 20170414080609) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -313,6 +313,7 @@ ActiveRecord::Schema.define(version: 20170406215816) do
     t.integer  "consumed_timestep"
     t.boolean  "otp_required_for_login"
     t.datetime "last_emailed_at"
+    t.string   "otp_backup_codes",                                       array: true
     t.index ["account_id"], name: "index_users_on_account_id", using: :btree
     t.index ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true, using: :btree
     t.index ["email"], name: "index_users_on_email", unique: true, using: :btree
diff --git a/spec/controllers/auth/sessions_controller_spec.rb b/spec/controllers/auth/sessions_controller_spec.rb
index c2a1fbe91..0ec0b8f2f 100644
--- a/spec/controllers/auth/sessions_controller_spec.rb
+++ b/spec/controllers/auth/sessions_controller_spec.rb
@@ -5,7 +5,7 @@ RSpec.describe Auth::SessionsController, type: :controller do
 
   describe 'GET #new' do
     before do
-      request.env["devise.mapping"] = Devise.mappings[:user]
+      request.env['devise.mapping'] = Devise.mappings[:user]
     end
 
     it 'returns http success' do
@@ -15,19 +15,94 @@ RSpec.describe Auth::SessionsController, type: :controller do
   end
 
   describe 'POST #create' do
-    let(:user) { Fabricate(:user, email: 'foo@bar.com', password: 'abcdefgh') }
-
     before do
-      request.env["devise.mapping"] = Devise.mappings[:user]
-      post :create, params: { user: { email: user.email, password: user.password } }
+      request.env['devise.mapping'] = Devise.mappings[:user]
     end
 
-    it 'redirects to home' do
-      expect(response).to redirect_to(root_path)
+    context 'using password authentication' do
+      let(:user) { Fabricate(:user, email: 'foo@bar.com', password: 'abcdefgh') }
+
+      context 'using a valid password' do
+        before do
+          post :create, params: { user: { email: user.email, password: user.password } }
+        end
+
+        it 'redirects to home' do
+          expect(response).to redirect_to(root_path)
+        end
+
+        it 'logs the user in' do
+          expect(controller.current_user).to eq user
+        end
+      end
+
+      context 'using an invalid password' do
+        before do
+          post :create, params: { user: { email: user.email, password: 'wrongpw' } }
+        end
+
+        it 'shows a login error' do
+          expect(flash[:alert]).to match I18n.t('devise.failure.invalid', authentication_keys: 'Email')
+        end
+
+        it "doesn't log the user in" do
+          expect(controller.current_user).to be_nil
+        end
+      end
     end
 
-    it 'logs the user in' do
-      expect(controller.current_user).to eq user
+    context 'using two-factor authentication' do
+      let(:user) do
+        Fabricate(:user, email: 'x@y.com', password: 'abcdefgh',
+                         otp_required_for_login: true, otp_secret: User.generate_otp_secret(32))
+      end
+      let(:recovery_codes) do
+        codes = user.generate_otp_backup_codes!
+        user.save
+        return codes
+      end
+
+      context 'using a valid OTP' do
+        before do
+          post :create, params: { user: { otp_attempt: user.current_otp } }, session: { otp_user_id: user.id }
+        end
+
+        it 'redirects to home' do
+          expect(response).to redirect_to(root_path)
+        end
+
+        it 'logs the user in' do
+          expect(controller.current_user).to eq user
+        end
+      end
+
+      context 'using a valid recovery code' do
+        before do
+          post :create, params: { user: { otp_attempt: recovery_codes.first } }, session: { otp_user_id: user.id }
+        end
+
+        it 'redirects to home' do
+          expect(response).to redirect_to(root_path)
+        end
+
+        it 'logs the user in' do
+          expect(controller.current_user).to eq user
+        end
+      end
+
+      context 'using an invalid OTP' do
+        before do
+          post :create, params: { user: { otp_attempt: 'wrongotp' } }, session: { otp_user_id: user.id }
+        end
+
+        it 'shows a login error' do
+          expect(flash[:alert]).to match I18n.t('users.invalid_otp_token')
+        end
+
+        it "doesn't log the user in" do
+          expect(controller.current_user).to be_nil
+        end
+      end
     end
   end
 end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index eb2a4aaea..4ed1d5a0a 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -1,6 +1,9 @@
 require 'rails_helper'
+require 'devise_two_factor/spec_helpers'
 
 RSpec.describe User, type: :model do
+  it_behaves_like 'two_factor_backupable'
+
   describe 'validations' do
     it 'is invalid without an account' do
       user = Fabricate.build(:user, account: nil)