From b4f479a00f00fe540f742435d29b09148ae2e5ec Mon Sep 17 00:00:00 2001
From: Dominik Kwiatek <dom@dom.dog>
Date: Sun, 1 Nov 2020 17:55:31 +0100
Subject: [PATCH 01/15] Generate recovery keys on TOTP setup

---
 app/controllers/user_controller.rb            |  7 +++---
 app/models/totp_recovery_code.rb              |  3 +++
 .../settings/security/recovery_keys.haml      |  6 +++++
 ...201101155648_create_totp_recovery_codes.rb |  9 ++++++++
 db/schema.rb                                  |  8 ++++++-
 lib/core_ext/secure_random.rb                 | 23 +++++++++++++++++++
 spec/models/totp_recovery_code_spec.rb        |  5 ++++
 7 files changed, 57 insertions(+), 4 deletions(-)
 create mode 100644 app/models/totp_recovery_code.rb
 create mode 100644 app/views/settings/security/recovery_keys.haml
 create mode 100644 db/migrate/20201101155648_create_totp_recovery_codes.rb
 create mode 100644 lib/core_ext/secure_random.rb
 create mode 100644 spec/models/totp_recovery_code_spec.rb

diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb
index b2fc4b7c..00d3903e 100644
--- a/app/controllers/user_controller.rb
+++ b/app/controllers/user_controller.rb
@@ -190,13 +190,14 @@ class UserController < ApplicationController
     current_user.otp_module = :enabled
 
     if current_user.authenticate_otp(req_params[:otp_validation], drift: APP_CONFIG.fetch(:otp_drift_period, 30).to_i)
-      flash[:success] = t('views.auth.2fa.setup.success')
+      @recovery_keys = TotpRecoveryCode.create!(Array.new(10) { {user: current_user, code: SecureRandom.base58(8).downcase} })
       current_user.save!
+
+      render 'settings/security/recovery_keys'
     else
       flash[:error] = t('views.auth.2fa.errors.invalid_code')
+      redirect_to edit_user_security_path
     end
-
-    redirect_to edit_user_security_path
   end
 
   def destroy_2fa
diff --git a/app/models/totp_recovery_code.rb b/app/models/totp_recovery_code.rb
new file mode 100644
index 00000000..a83df0e4
--- /dev/null
+++ b/app/models/totp_recovery_code.rb
@@ -0,0 +1,3 @@
+class TotpRecoveryCode < ApplicationRecord
+  belongs_to :user
+end
diff --git a/app/views/settings/security/recovery_keys.haml b/app/views/settings/security/recovery_keys.haml
new file mode 100644
index 00000000..066facf0
--- /dev/null
+++ b/app/views/settings/security/recovery_keys.haml
@@ -0,0 +1,6 @@
+%p= t('views.auth.2fa.setup.success')
+
+%ul
+  - @recovery_keys.each do |key|
+    %li
+      %code= key.code
diff --git a/db/migrate/20201101155648_create_totp_recovery_codes.rb b/db/migrate/20201101155648_create_totp_recovery_codes.rb
new file mode 100644
index 00000000..3b506256
--- /dev/null
+++ b/db/migrate/20201101155648_create_totp_recovery_codes.rb
@@ -0,0 +1,9 @@
+class CreateTotpRecoveryCodes < ActiveRecord::Migration[5.2]
+  def change
+    create_table :totp_recovery_codes do |t|
+      t.bigint :user_id
+      t.string :code, limit: 8
+    end
+    add_index :totp_recovery_codes, [:user_id, :code]
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 4ad56134..a297997d 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: 2020_10_18_090453) do
+ActiveRecord::Schema.define(version: 2020_11_01_155648) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -219,6 +219,12 @@ ActiveRecord::Schema.define(version: 2020_10_18_090453) do
     t.index ["user_id", "created_at"], name: "index_themes_on_user_id_and_created_at"
   end
 
+  create_table "totp_recovery_codes", force: :cascade do |t|
+    t.bigint "user_id"
+    t.string "code", limit: 8
+    t.index ["user_id", "code"], name: "index_totp_recovery_codes_on_user_id_and_code"
+  end
+
   create_table "users", id: :bigint, default: -> { "gen_timestamp_id('users'::text)" }, force: :cascade do |t|
     t.string "email", default: "", null: false
     t.string "encrypted_password", default: "", null: false
diff --git a/lib/core_ext/secure_random.rb b/lib/core_ext/secure_random.rb
new file mode 100644
index 00000000..5cc3c363
--- /dev/null
+++ b/lib/core_ext/secure_random.rb
@@ -0,0 +1,23 @@
+# From Rails 6
+# Remove once upgraded
+module SecureRandom
+  BASE36_ALPHABET = ("0".."9").to_a + ("a".."z").to_a
+  # SecureRandom.base36 generates a random base36 string in lowercase.
+  #
+  # The argument _n_ specifies the length of the random string to be generated.
+  #
+  # If _n_ is not specified or is +nil+, 16 is assumed. It may be larger in the future.
+  # This method can be used over +base58+ if a deterministic case key is necessary.
+  #
+  # The result will contain alphanumeric characters in lowercase.
+  #
+  #   p SecureRandom.base36 # => "4kugl2pdqmscqtje"
+  #   p SecureRandom.base36(24) # => "77tmhrhjfvfdwodq8w7ev2m7"
+  def self.base36(n = 16)
+    SecureRandom.random_bytes(n).unpack("C*").map do |byte|
+      idx = byte % 64
+      idx = SecureRandom.random_number(36) if idx >= 36
+      BASE36_ALPHABET[idx]
+    end.join
+  end
+end
\ No newline at end of file
diff --git a/spec/models/totp_recovery_code_spec.rb b/spec/models/totp_recovery_code_spec.rb
new file mode 100644
index 00000000..66f2040a
--- /dev/null
+++ b/spec/models/totp_recovery_code_spec.rb
@@ -0,0 +1,5 @@
+require 'rails_helper'
+
+RSpec.describe TotpRecoveryCode, type: :model do
+  pending "add some examples to (or delete) #{__FILE__}"
+end

From 5dd920eba29d9e22db363ee93265597aed0dcee1 Mon Sep 17 00:00:00 2001
From: Dominik Kwiatek <dom@dom.dog>
Date: Sun, 1 Nov 2020 18:29:11 +0100
Subject: [PATCH 02/15] Allow recovery codes to be used to sign in in place of
 a OTP

---
 app/controllers/user/sessions_controller.rb | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/app/controllers/user/sessions_controller.rb b/app/controllers/user/sessions_controller.rb
index cd8d96bf..0820de04 100644
--- a/app/controllers/user/sessions_controller.rb
+++ b/app/controllers/user/sessions_controller.rb
@@ -18,7 +18,15 @@ class User::SessionsController < Devise::SessionsController
         warden.lock!
         render 'auth/two_factor_authentication'
       else
-        if resource.authenticate_otp(params[:user][:otp_attempt], drift: APP_CONFIG.fetch(:otp_drift_period, 30).to_i)
+        if params[:user][:otp_attempt].length == 8
+          found = TotpRecoveryCode.where(user_id: resource.id, code: params[:user][:otp_attempt].downcase).delete_all
+          if found == 1
+            continue_sign_in(resource, resource_name)
+          else
+            flash[:error] = t('views.auth.2fa.errors.invalid_code')
+            redirect_to new_user_session_url
+          end
+        elsif resource.authenticate_otp(params[:user][:otp_attempt], drift: APP_CONFIG.fetch(:otp_drift_period, 30).to_i)
           continue_sign_in(resource, resource_name)
         else
           sign_out(resource)

From 5eb4f326608ba284d3359b3bcdcfa77c4b118060 Mon Sep 17 00:00:00 2001
From: Dominik Kwiatek <dom@dom.dog>
Date: Sun, 1 Nov 2020 18:31:20 +0100
Subject: [PATCH 03/15] Clean up after TOTP is disabled.

---
 app/controllers/user_controller.rb | 1 +
 1 file changed, 1 insertion(+)

diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb
index 00d3903e..1d744cd2 100644
--- a/app/controllers/user_controller.rb
+++ b/app/controllers/user_controller.rb
@@ -203,6 +203,7 @@ class UserController < ApplicationController
   def destroy_2fa
     current_user.otp_module = :disabled
     current_user.save!
+    TotpRecoveryCode.where(user_id: resource.id).delete_all
     flash[:success] = 'Two factor authentication has been disabled for your account.'
     redirect_to edit_user_security_path
   end

From 61d82bdbecd00e65d2d0c56e24f24c5f6d560410 Mon Sep 17 00:00:00 2001
From: Dominik Kwiatek <dom@dom.dog>
Date: Sun, 1 Nov 2020 18:41:37 +0100
Subject: [PATCH 04/15] Display count of remaining recovery codes

---
 app/controllers/user/sessions_controller.rb    | 1 +
 app/controllers/user_controller.rb             | 2 ++
 app/views/settings/security/_totp_enabled.haml | 4 +++-
 config/locales/en.yml                          | 1 +
 4 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/app/controllers/user/sessions_controller.rb b/app/controllers/user/sessions_controller.rb
index 0820de04..2cab208b 100644
--- a/app/controllers/user/sessions_controller.rb
+++ b/app/controllers/user/sessions_controller.rb
@@ -21,6 +21,7 @@ class User::SessionsController < Devise::SessionsController
         if params[:user][:otp_attempt].length == 8
           found = TotpRecoveryCode.where(user_id: resource.id, code: params[:user][:otp_attempt].downcase).delete_all
           if found == 1
+            flash[:info] = "You have #{TotpRecoveryCode.where(user_id: resource.id).count} recovery codes remaining."
             continue_sign_in(resource, resource_name)
           else
             flash[:error] = t('views.auth.2fa.errors.invalid_code')
diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb
index 1d744cd2..2cd0a6f5 100644
--- a/app/controllers/user_controller.rb
+++ b/app/controllers/user_controller.rb
@@ -182,6 +182,8 @@ class UserController < ApplicationController
       qr_code = RQRCode::QRCode.new(current_user.provisioning_uri("Retrospring:#{current_user.screen_name}", issuer: "Retrospring"))
 
       @qr_svg = qr_code.as_svg({offset: 4, module_size: 4, color: '000;fill:var(--primary)'}).html_safe
+    else
+      @recovery_code_count = TotpRecoveryCode.where(user_id: current_user.id).count
     end
   end
 
diff --git a/app/views/settings/security/_totp_enabled.haml b/app/views/settings/security/_totp_enabled.haml
index 4c383227..0ed03a30 100644
--- a/app/views/settings/security/_totp_enabled.haml
+++ b/app/views/settings/security/_totp_enabled.haml
@@ -1,3 +1,5 @@
-%p Your account is set up to require the use of a one-time password in order to log in
+%p Your account is set up to require the use of a one-time password in order to log in.
+%p You currently have #{@recovery_code_count} unused recovery codes.
 = link_to t('views.actions.remove'), destroy_user_2fa_path, class: 'btn btn-primary', method: 'delete',
           data: { confirm: t('views.settings.security.2fa.detach_confirm') }
+D
\ No newline at end of file
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 0c2e5325..1bb41e5e 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -265,6 +265,7 @@ en:
       done: "Done"
       y: "Yes"
       n: "No"
+      remove: "Remove"
     sessions:
       destroy: "Logout"
       create: "Sign in"

From e16896fac109a95a2964cd8724f68cee078bdcb7 Mon Sep 17 00:00:00 2001
From: Dominik Kwiatek <dom@dom.dog>
Date: Sun, 1 Nov 2020 18:52:42 +0100
Subject: [PATCH 05/15] Provide the user a way to generate new codes.

---
 app/controllers/user_controller.rb             | 8 +++++++-
 app/views/settings/security/_totp_enabled.haml | 5 +++--
 config/routes.rb                               | 1 +
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb
index 2cd0a6f5..8ca8588a 100644
--- a/app/controllers/user_controller.rb
+++ b/app/controllers/user_controller.rb
@@ -205,8 +205,14 @@ class UserController < ApplicationController
   def destroy_2fa
     current_user.otp_module = :disabled
     current_user.save!
-    TotpRecoveryCode.where(user_id: resource.id).delete_all
+    TotpRecoveryCode.where(user_id: current_user.id).delete_all
     flash[:success] = 'Two factor authentication has been disabled for your account.'
     redirect_to edit_user_security_path
   end
+
+  def reset_user_recovery_codes
+    TotpRecoveryCode.where(user_id: current_user.id).delete_all
+    @recovery_keys = TotpRecoveryCode.create!(Array.new(10) { {user: current_user, code: SecureRandom.base58(8).downcase} })
+    render 'settings/security/recovery_keys'
+  end
 end
diff --git a/app/views/settings/security/_totp_enabled.haml b/app/views/settings/security/_totp_enabled.haml
index 0ed03a30..ac45ab59 100644
--- a/app/views/settings/security/_totp_enabled.haml
+++ b/app/views/settings/security/_totp_enabled.haml
@@ -1,5 +1,6 @@
 %p Your account is set up to require the use of a one-time password in order to log in.
 %p You currently have #{@recovery_code_count} unused recovery codes.
-= link_to t('views.actions.remove'), destroy_user_2fa_path, class: 'btn btn-primary', method: 'delete',
+= link_to t('views.actions.remove'), destroy_user_2fa_path, class: 'btn btn-danger', method: 'delete',
           data: { confirm: t('views.settings.security.2fa.detach_confirm') }
-D
\ No newline at end of file
+= link_to "Re-generate recovery codes", reset_user_recovery_codes_path, class: 'btn btn-primary', method: 'delete',
+          data: { confirm: "Are you sure? This will disable your previous set of recovery codes." }
\ No newline at end of file
diff --git a/config/routes.rb b/config/routes.rb
index 757b520c..dc67cee9 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -70,6 +70,7 @@ Rails.application.routes.draw do
   match '/settings/security', to: 'user#edit_security', via: :get, as: :edit_user_security
   match '/settings/security/2fa', to: 'user#update_2fa', via: :patch, as: :update_user_2fa
   match '/settings/security/2fa', to: 'user#destroy_2fa', via: :delete, as: :destroy_user_2fa
+  match '/settings/security/recovery', to: 'user#reset_user_recovery_codes', via: :delete, as: :reset_user_recovery_codes
 
   # resources :services, only: [:index, :destroy]
   match '/settings/services', to: 'services#index', via: 'get', as: :services

From 9b69ae8fc20d5ed3c79184dce87c34be0c2742a9 Mon Sep 17 00:00:00 2001
From: Dominik Kwiatek <dom@dom.dog>
Date: Sun, 1 Nov 2020 19:10:41 +0100
Subject: [PATCH 06/15] Remove SecureRandom#base36 extension method

---
 lib/core_ext/secure_random.rb | 23 -----------------------
 1 file changed, 23 deletions(-)
 delete mode 100644 lib/core_ext/secure_random.rb

diff --git a/lib/core_ext/secure_random.rb b/lib/core_ext/secure_random.rb
deleted file mode 100644
index 5cc3c363..00000000
--- a/lib/core_ext/secure_random.rb
+++ /dev/null
@@ -1,23 +0,0 @@
-# From Rails 6
-# Remove once upgraded
-module SecureRandom
-  BASE36_ALPHABET = ("0".."9").to_a + ("a".."z").to_a
-  # SecureRandom.base36 generates a random base36 string in lowercase.
-  #
-  # The argument _n_ specifies the length of the random string to be generated.
-  #
-  # If _n_ is not specified or is +nil+, 16 is assumed. It may be larger in the future.
-  # This method can be used over +base58+ if a deterministic case key is necessary.
-  #
-  # The result will contain alphanumeric characters in lowercase.
-  #
-  #   p SecureRandom.base36 # => "4kugl2pdqmscqtje"
-  #   p SecureRandom.base36(24) # => "77tmhrhjfvfdwodq8w7ev2m7"
-  def self.base36(n = 16)
-    SecureRandom.random_bytes(n).unpack("C*").map do |byte|
-      idx = byte % 64
-      idx = SecureRandom.random_number(36) if idx >= 36
-      BASE36_ALPHABET[idx]
-    end.join
-  end
-end
\ No newline at end of file

From c7463df4d4b4bb2fb19efeef5ecf517d5f01d9b3 Mon Sep 17 00:00:00 2001
From: Dominik Kwiatek <dom@dom.dog>
Date: Sun, 1 Nov 2020 19:10:59 +0100
Subject: [PATCH 07/15] Fix lint errors

---
 app/views/settings/_security.haml              | 2 +-
 app/views/settings/security/_totp_enabled.haml | 6 +++---
 app/views/user/edit_security.haml              | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/app/views/settings/_security.haml b/app/views/settings/_security.haml
index bc673ba7..8d7f1f67 100644
--- a/app/views/settings/_security.haml
+++ b/app/views/settings/_security.haml
@@ -4,4 +4,4 @@
     - if current_user.otp_module_disabled?
       = render partial: 'settings/security/totp_setup', locals: { qr_svg: qr_svg }
     - else
-      = render partial: 'settings/security/totp_enabled'
+      = render partial: 'settings/security/totp_enabled', locals: { recovery_code_count: recovery_code_count }
diff --git a/app/views/settings/security/_totp_enabled.haml b/app/views/settings/security/_totp_enabled.haml
index ac45ab59..5e15f872 100644
--- a/app/views/settings/security/_totp_enabled.haml
+++ b/app/views/settings/security/_totp_enabled.haml
@@ -1,6 +1,6 @@
 %p Your account is set up to require the use of a one-time password in order to log in.
-%p You currently have #{@recovery_code_count} unused recovery codes.
+%p You currently have #{recovery_code_count} unused recovery codes.
 = link_to t('views.actions.remove'), destroy_user_2fa_path, class: 'btn btn-danger', method: 'delete',
           data: { confirm: t('views.settings.security.2fa.detach_confirm') }
-= link_to "Re-generate recovery codes", reset_user_recovery_codes_path, class: 'btn btn-primary', method: 'delete',
-          data: { confirm: "Are you sure? This will disable your previous set of recovery codes." }
\ No newline at end of file
+= link_to 'Re-generate recovery codes', reset_user_recovery_codes_path, class: 'btn btn-primary', method: 'delete',
+          data: { confirm: 'Are you sure? This will disable your previous set of recovery codes.' }
diff --git a/app/views/user/edit_security.haml b/app/views/user/edit_security.haml
index bcdebbea..e68d2636 100644
--- a/app/views/user/edit_security.haml
+++ b/app/views/user/edit_security.haml
@@ -1,4 +1,4 @@
-= render 'settings/security', qr_svg: @qr_svg
+= render 'settings/security', qr_svg: @qr_svg, recovery_code_count: @recovery_code_count
 
 - provide(:title, generate_title('Security Settings'))
 - parent_layout 'user/settings'

From f12d56ff7d459dc770ca41d5702d16868c4c2c70 Mon Sep 17 00:00:00 2001
From: Dominik Kwiatek <dom@dom.dog>
Date: Sun, 1 Nov 2020 19:25:49 +0100
Subject: [PATCH 08/15] Remove unused spec file for TotpRecoveryCode model

---
 spec/models/totp_recovery_code_spec.rb | 5 -----
 1 file changed, 5 deletions(-)
 delete mode 100644 spec/models/totp_recovery_code_spec.rb

diff --git a/spec/models/totp_recovery_code_spec.rb b/spec/models/totp_recovery_code_spec.rb
deleted file mode 100644
index 66f2040a..00000000
--- a/spec/models/totp_recovery_code_spec.rb
+++ /dev/null
@@ -1,5 +0,0 @@
-require 'rails_helper'
-
-RSpec.describe TotpRecoveryCode, type: :model do
-  pending "add some examples to (or delete) #{__FILE__}"
-end

From d7cac67c22fd6b58fb15666dcb010c19811b67db Mon Sep 17 00:00:00 2001
From: Dominik Kwiatek <dom@dom.dog>
Date: Sun, 15 Nov 2020 10:20:55 +0100
Subject: [PATCH 09/15] Create print view for recovery codes

---
 .../stylesheets/components/_totp-setup.scss   | 28 ++++++++++++++++++-
 app/views/auth/two_factor_authentication.haml |  2 +-
 .../settings/security/recovery_keys.haml      | 27 ++++++++++++++----
 app/views/shared/_locales.haml                |  2 +-
 4 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/app/assets/stylesheets/components/_totp-setup.scss b/app/assets/stylesheets/components/_totp-setup.scss
index 67d0c932..2bcf341b 100644
--- a/app/assets/stylesheets/components/_totp-setup.scss
+++ b/app/assets/stylesheets/components/_totp-setup.scss
@@ -1,6 +1,6 @@
 %totp-input {
   font-family: "Monaco", "Inconsolata", "Cascadia Code", "Consolas", monospace;
-  width: 86px;
+  width: 100px;
 }
 
 .totp-setup {
@@ -43,6 +43,32 @@
   &__code-field {
     @extend %totp-input;
   }
+
+  &__recovery {
+    &-container {
+      max-width: 455px;
+    }
+
+    &-icon {
+      font-size: .75in;
+    }
+
+    &-title {
+      text-align: left;
+    }
+
+    &-codes {
+      display: grid;
+      grid-template-columns: 1fr 1fr;
+      padding: 0;
+
+      li {
+        list-style-type: none;
+        font-size: 16px;
+        text-align: center;
+      }
+    }
+  }
 }
 
 #user_otp_attempt {
diff --git a/app/views/auth/two_factor_authentication.haml b/app/views/auth/two_factor_authentication.haml
index 75583213..01547311 100644
--- a/app/views/auth/two_factor_authentication.haml
+++ b/app/views/auth/two_factor_authentication.haml
@@ -7,7 +7,7 @@
           %h1.mb-3.mt-0= t('views.auth.2fa.title')
           = bootstrap_form_for(resource, as: resource_name, url: session_path(resource_name), method: :post) do |f|
 
-            = f.text_field :otp_attempt, autofocus: true, label: t('views.auth.2fa.otp_field')
+            = f.text_field :otp_attempt, autofocus: true, label: 'Please enter the code from your authenticator app'
 
             = f.submit t('views.sessions.create'), class: 'btn btn-primary mt-3 mb-3'
 
diff --git a/app/views/settings/security/recovery_keys.haml b/app/views/settings/security/recovery_keys.haml
index 066facf0..ef58cec4 100644
--- a/app/views/settings/security/recovery_keys.haml
+++ b/app/views/settings/security/recovery_keys.haml
@@ -1,6 +1,23 @@
-%p= t('views.auth.2fa.setup.success')
+.container.container--main
+  .row.justify-content-center
+    .col-md-5.totp-setup__recovery-container
+      .card
+        .card-body
+          .d-none.d-print-block.totp-setup__recovery-icon
+            %i.fa.fa-comments
+          %h1.totp-setup__recovery-title Your Retrospring recovery codes
 
-%ul
-  - @recovery_keys.each do |key|
-    %li
-      %code= key.code
+          %ul.totp-setup__recovery-codes
+            - @recovery_keys.each do |key|
+              %li
+                %code= key.code
+          .d-none.d-print-block= "These codes were generated #{Time.now.strftime("%F at %T %Z")}"
+        .card-footer.d-print-none
+          We recommend storing these in a password manager or printing them out on paper.
+          %br
+          %a.btn{onclick: 'print()'}
+            %i.fa.fa-print
+            Print
+        .d-none.d-print-block.text-success
+          %i.fa.fa-tree
+          Please consider the environment before printing this page.
\ No newline at end of file
diff --git a/app/views/shared/_locales.haml b/app/views/shared/_locales.haml
index 52e687d7..f1d8f1ff 100644
--- a/app/views/shared/_locales.haml
+++ b/app/views/shared/_locales.haml
@@ -1,4 +1,4 @@
-.container
+.container.d-print-none
   .locales
     %span
       %a{ href: '#', id: 'locale-switch' }

From aeb13964222aeda4e3578b75f4d4fd4aeca8a93b Mon Sep 17 00:00:00 2001
From: Dominik Kwiatek <dom@dom.dog>
Date: Sun, 15 Nov 2020 10:21:06 +0100
Subject: [PATCH 10/15] Add tests for recovery codes

---
 app/controllers/user_controller.rb       | 11 ++++---
 app/models/totp_recovery_code.rb         | 14 +++++++++
 app/models/user.rb                       |  1 +
 spec/controllers/user_controller_spec.rb | 38 +++++++++++++++++++-----
 4 files changed, 50 insertions(+), 14 deletions(-)

diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb
index 8ca8588a..eca7ae09 100644
--- a/app/controllers/user_controller.rb
+++ b/app/controllers/user_controller.rb
@@ -175,10 +175,9 @@ class UserController < ApplicationController
 
   def edit_security
     if current_user.otp_module_disabled?
-      current_user.otp_secret_key = User.otp_random_secret(26)
+      current_user.otp_secret_key = User.otp_random_secret(25)
       current_user.save
 
-      @provisioning_uri = current_user.provisioning_uri(nil, issuer: APP_CONFIG[:hostname])
       qr_code = RQRCode::QRCode.new(current_user.provisioning_uri("Retrospring:#{current_user.screen_name}", issuer: "Retrospring"))
 
       @qr_svg = qr_code.as_svg({offset: 4, module_size: 4, color: '000;fill:var(--primary)'}).html_safe
@@ -192,7 +191,7 @@ class UserController < ApplicationController
     current_user.otp_module = :enabled
 
     if current_user.authenticate_otp(req_params[:otp_validation], drift: APP_CONFIG.fetch(:otp_drift_period, 30).to_i)
-      @recovery_keys = TotpRecoveryCode.create!(Array.new(10) { {user: current_user, code: SecureRandom.base58(8).downcase} })
+      @recovery_keys = TotpRecoveryCode.generate_for(current_user)
       current_user.save!
 
       render 'settings/security/recovery_keys'
@@ -205,14 +204,14 @@ class UserController < ApplicationController
   def destroy_2fa
     current_user.otp_module = :disabled
     current_user.save!
-    TotpRecoveryCode.where(user_id: current_user.id).delete_all
+    TotpRecoveryCode.remove_all_for(current_user)
     flash[:success] = 'Two factor authentication has been disabled for your account.'
     redirect_to edit_user_security_path
   end
 
   def reset_user_recovery_codes
-    TotpRecoveryCode.where(user_id: current_user.id).delete_all
-    @recovery_keys = TotpRecoveryCode.create!(Array.new(10) { {user: current_user, code: SecureRandom.base58(8).downcase} })
+    TotpRecoveryCode.remove_all_for(current_user)
+    @recovery_keys = TotpRecoveryCode.generate_for(current_user)
     render 'settings/security/recovery_keys'
   end
 end
diff --git a/app/models/totp_recovery_code.rb b/app/models/totp_recovery_code.rb
index a83df0e4..b8a3b479 100644
--- a/app/models/totp_recovery_code.rb
+++ b/app/models/totp_recovery_code.rb
@@ -1,3 +1,17 @@
 class TotpRecoveryCode < ApplicationRecord
+  NUMBER_OF_CODES_TO_GENERATE = 16
+
   belongs_to :user
+
+  # @param user [User]
+  # @return [Array<TotpRecoveryCode>]
+  def self.generate_for(user)
+    TotpRecoveryCode.create!(Array.new(16) { {user: user, code: SecureRandom.base58(8).downcase} })
+  end
+
+  # @param user [User]
+  # @return [Integer]
+  def self.remove_all_for(user)
+    TotpRecoveryCode.where(user: user).delete_all
+  end
 end
diff --git a/app/models/user.rb b/app/models/user.rb
index b1e636d7..c0606081 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -43,6 +43,7 @@ class User < ApplicationRecord
   has_many :list_memberships, class_name: "ListMember", foreign_key: 'user_id', dependent: :destroy
 
   has_many :subscriptions, dependent: :destroy
+  has_many :totp_recovery_codes, dependent: :destroy
 
   has_one :theme, dependent: :destroy
 
diff --git a/spec/controllers/user_controller_spec.rb b/spec/controllers/user_controller_spec.rb
index 9a18b75d..4966f936 100644
--- a/spec/controllers/user_controller_spec.rb
+++ b/spec/controllers/user_controller_spec.rb
@@ -3,7 +3,9 @@
 require "rails_helper"
 
 describe UserController, type: :controller do
-  let(:user) { FactoryBot.create :user, otp_module: :disabled }
+  let(:user) { FactoryBot.create :user,
+                                 otp_module: :disabled,
+                                 otp_secret_key: 'EJFNIJPYXXTCQSRTQY6AG7XQLAT2IDG5H7NGLJE3'}
 
   describe "#edit" do
     subject { get :edit }
@@ -97,8 +99,7 @@ describe UserController, type: :controller do
       context "user enters the incorrect code" do
         let(:update_params) do
           {
-              user: { otp_secret_key: 'EJFNIJPYXXTCQSRTQY6AG7XQLAT2IDG5H7NGLJE3',
-                      otp_validation: 123456 }
+              user: { otp_validation: 123456 }
           }
         end
 
@@ -106,6 +107,7 @@ describe UserController, type: :controller do
           Timecop.freeze(Time.at(1603290888)) do
             subject
             expect(response).to redirect_to :edit_user_security
+            expect(flash[:error]).to eq('The code you entered was invalid.')
           end
         end
       end
@@ -113,21 +115,23 @@ describe UserController, type: :controller do
       context "user enters the correct code" do
         let(:update_params) do
           {
-              user: { otp_secret_key: 'EJFNIJPYXXTCQSRTQY6AG7XQLAT2IDG5H7NGLJE3',
-                      otp_validation: 187894 }
+              user: { otp_validation: 187894 }
           }
         end
 
-        it "enables 2FA for the logged in user" do
+        it "enables 2FA for the logged in user and generates recovery keys" do
           Timecop.freeze(Time.at(1603290888)) do
             subject
-            expect(response).to redirect_to :edit_user_security
+            expect(response).to have_rendered(:recovery_keys)
+
+            expect(user.totp_recovery_codes.count).to be(TotpRecoveryCode::NUMBER_OF_CODES_TO_GENERATE)
           end
         end
 
         it "shows an error if the user attempts to use the code once it has expired" do
           Timecop.freeze(Time.at(1603290910)) do
             subject
+            expect(response).to redirect_to :edit_user_security
             expect(flash[:error]).to eq('The code you entered was invalid.')
           end
         end
@@ -142,13 +146,31 @@ describe UserController, type: :controller do
       before(:each) do
         user.otp_module = :enabled
         user.save
-        sign_in user
+        sign_in(user)
       end
 
       it "disables 2FA for the logged in user" do
         subject
         user.reload
         expect(user.otp_module_enabled?).to be_falsey
+        expect(user.totp_recovery_codes.count).to be(0)
+      end
+    end
+  end
+
+  describe "#reset_user_recovery_codes" do
+    subject { delete :reset_user_recovery_codes }
+
+    context "user signed in" do
+      before(:each) do
+        sign_in(user)
+      end
+
+      it "regenerates codes on request" do
+        old_codes = user.totp_recovery_codes.pluck(:code)
+        subject
+        new_codes = user.totp_recovery_codes.pluck(:code)
+        expect(new_codes).not_to match_array(old_codes)
       end
     end
   end

From 4dc00a011419edcbd23c62587075d209e721a2d9 Mon Sep 17 00:00:00 2001
From: Dominik Kwiatek <dom@dom.dog>
Date: Sun, 15 Nov 2020 10:29:09 +0100
Subject: [PATCH 11/15] Fix test for activating with expired code

---
 spec/controllers/user_controller_spec.rb | 1 -
 1 file changed, 1 deletion(-)

diff --git a/spec/controllers/user_controller_spec.rb b/spec/controllers/user_controller_spec.rb
index 4966f936..a5b92a44 100644
--- a/spec/controllers/user_controller_spec.rb
+++ b/spec/controllers/user_controller_spec.rb
@@ -131,7 +131,6 @@ describe UserController, type: :controller do
         it "shows an error if the user attempts to use the code once it has expired" do
           Timecop.freeze(Time.at(1603290910)) do
             subject
-            expect(response).to redirect_to :edit_user_security
             expect(flash[:error]).to eq('The code you entered was invalid.')
           end
         end

From b4358772e1dafbdfd65145534cf54e1b3911ce96 Mon Sep 17 00:00:00 2001
From: Dominik Kwiatek <dom@dom.dog>
Date: Sun, 15 Nov 2020 21:27:38 +0100
Subject: [PATCH 12/15] Fix test for expired OTP

---
 spec/controllers/user_controller_spec.rb | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/spec/controllers/user_controller_spec.rb b/spec/controllers/user_controller_spec.rb
index a5b92a44..d3789066 100644
--- a/spec/controllers/user_controller_spec.rb
+++ b/spec/controllers/user_controller_spec.rb
@@ -129,9 +129,10 @@ describe UserController, type: :controller do
         end
 
         it "shows an error if the user attempts to use the code once it has expired" do
-          Timecop.freeze(Time.at(1603290910)) do
+          Timecop.freeze(Time.at(1603290950)) do
             subject
-            expect(flash[:error]).to eq('The code you entered was invalid.')
+            expect(response).to redirect_to :edit_user_security
+            expect(flash[:error]).to eq(I18n.t('views.auth.2fa.errors.invalid_code'))
           end
         end
       end

From f031143b45d2b2e7eb67320ef76733de3a052536 Mon Sep 17 00:00:00 2001
From: Dominik Kwiatek <dom@dom.dog>
Date: Sun, 15 Nov 2020 21:35:50 +0100
Subject: [PATCH 13/15] Fix linter errors

---
 app/views/settings/security/recovery_keys.haml | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/app/views/settings/security/recovery_keys.haml b/app/views/settings/security/recovery_keys.haml
index ef58cec4..c09e9164 100644
--- a/app/views/settings/security/recovery_keys.haml
+++ b/app/views/settings/security/recovery_keys.haml
@@ -11,13 +11,13 @@
             - @recovery_keys.each do |key|
               %li
                 %code= key.code
-          .d-none.d-print-block= "These codes were generated #{Time.now.strftime("%F at %T %Z")}"
+          .d-none.d-print-block These codes were generated #{Time.now.strftime('%F at %T %Z')}
         .card-footer.d-print-none
           We recommend storing these in a password manager or printing them out on paper.
           %br
-          %a.btn{onclick: 'print()'}
+          %a.btn{ onclick: 'print()' }
             %i.fa.fa-print
             Print
         .d-none.d-print-block.text-success
           %i.fa.fa-tree
-          Please consider the environment before printing this page.
\ No newline at end of file
+          Please consider the environment before printing this page.

From 2e6f49819af345d05395d33ca2f9d8be77bd19c1 Mon Sep 17 00:00:00 2001
From: Dominik Kwiatek <dom@dom.dog>
Date: Sun, 15 Nov 2020 22:08:18 +0100
Subject: [PATCH 14/15] Address @nilsding's review comments

---
 app/controllers/user_controller.rb | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb
index eca7ae09..0cb802cb 100644
--- a/app/controllers/user_controller.rb
+++ b/app/controllers/user_controller.rb
@@ -182,7 +182,7 @@ class UserController < ApplicationController
 
       @qr_svg = qr_code.as_svg({offset: 4, module_size: 4, color: '000;fill:var(--primary)'}).html_safe
     else
-      @recovery_code_count = TotpRecoveryCode.where(user_id: current_user.id).count
+      @recovery_code_count = current_user.totp_recovery_codes.count
     end
   end
 
@@ -204,13 +204,13 @@ class UserController < ApplicationController
   def destroy_2fa
     current_user.otp_module = :disabled
     current_user.save!
-    TotpRecoveryCode.remove_all_for(current_user)
+    current_user.totp_recovery_codes.delete_all
     flash[:success] = 'Two factor authentication has been disabled for your account.'
     redirect_to edit_user_security_path
   end
 
   def reset_user_recovery_codes
-    TotpRecoveryCode.remove_all_for(current_user)
+    current_user.totp_recovery_codes.delete_all
     @recovery_keys = TotpRecoveryCode.generate_for(current_user)
     render 'settings/security/recovery_keys'
   end

From 277799ff4b669b27bde7356f7489a7a0fd1a46f9 Mon Sep 17 00:00:00 2001
From: Dominik Kwiatek <dom@dom.dog>
Date: Sun, 15 Nov 2020 22:09:27 +0100
Subject: [PATCH 15/15] Remove `TotpRecoveryCode.remove_all_for(user)` method

---
 app/models/totp_recovery_code.rb | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/app/models/totp_recovery_code.rb b/app/models/totp_recovery_code.rb
index b8a3b479..846f52d8 100644
--- a/app/models/totp_recovery_code.rb
+++ b/app/models/totp_recovery_code.rb
@@ -8,10 +8,4 @@ class TotpRecoveryCode < ApplicationRecord
   def self.generate_for(user)
     TotpRecoveryCode.create!(Array.new(16) { {user: user, code: SecureRandom.base58(8).downcase} })
   end
-
-  # @param user [User]
-  # @return [Integer]
-  def self.remove_all_for(user)
-    TotpRecoveryCode.where(user: user).delete_all
-  end
 end