From 65240b2b1d30aa0cd3675b9787d154c02ea7818d Mon Sep 17 00:00:00 2001
From: Rasmus Kjellberg <2277443+kjellberg@users.noreply.github.com>
Date: Tue, 2 Apr 2024 12:12:05 +0200
Subject: [PATCH] feat: pages for disabling two factor authentication

---
 .../users/two_factor_controller.rb            | 16 +++++++++++++
 app/models/user.rb                            |  3 +--
 app/views/users/two_factor/disable.html.erb   | 23 ++++++++++++++++++
 app/views/users/two_factor/show.html.erb      |  2 +-
 config/locales/kiqr.en.yml                    |  9 +++++++
 config/routes/authentication.rb               |  2 ++
 .../users/two_factor_controller_test.rb       | 24 +++++++++++++++++++
 test/system/users/two_factor_test.rb          |  2 +-
 8 files changed, 77 insertions(+), 4 deletions(-)
 create mode 100644 app/views/users/two_factor/disable.html.erb

diff --git a/app/controllers/users/two_factor_controller.rb b/app/controllers/users/two_factor_controller.rb
index e7f8dc9..2d44ca5 100644
--- a/app/controllers/users/two_factor_controller.rb
+++ b/app/controllers/users/two_factor_controller.rb
@@ -32,6 +32,22 @@ def verify
     end
   end
 
+  def disable
+    redirect_to edit_two_factor_path unless enabled?
+  end
+
+  def destroy
+    return redirect_to edit_two_factor_path unless enabled?
+
+    if @user.validate_and_consume_otp!(params.dig(:user, :otp_attempt))
+      @user.update(otp_required_for_login: false, otp_backup_codes: [])
+      redirect_to edit_two_factor_path, notice: I18n.t("users.two_factor.disable.disabled")
+    else
+      @user.errors.add(:otp_attempt, :invalid)
+      render :disable, status: :unprocessable_entity
+    end
+  end
+
   private
 
   # Don't refresh the OTP secret if it's already enabled. This may lock the user
diff --git a/app/models/user.rb b/app/models/user.rb
index 6b47082..c425e40 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -19,8 +19,7 @@ def onboarded?
 
   def otp_uri
     issuer = Kiqr::Config.app_name
-    label = "#{issuer}:#{email}"
-    otp_provisioning_uri(label, issuer: issuer)
+    otp_provisioning_uri(email, issuer: issuer)
   end
 
   def reset_otp_secret!
diff --git a/app/views/users/two_factor/disable.html.erb b/app/views/users/two_factor/disable.html.erb
new file mode 100644
index 0000000..b3947d1
--- /dev/null
+++ b/app/views/users/two_factor/disable.html.erb
@@ -0,0 +1,23 @@
+<% title t(".title") %>
+
+<%= render(PageLayouts::Settings::Component.new(
+  title: t(".title"),
+  description: t(".description")
+)) do %>
+  <div class="box">
+    <header class="border-b pb-4 mb-4">
+      <h3 class="font-bold text-primary uppercase">
+        <%= t(".title") %>
+      </h3>
+    </header>
+    <article class="prose dark:prose-invert">
+      <p><%= t(".instructions") %></p>
+      <%= simple_form_for(@user, url: destroy_two_factor_path, method: :delete) do |f| %>
+        <%= f.input :otp_attempt, label: t(".label"), placeholder: t(".placeholder") %>
+        <div>
+          <%= f.submit t(".button"), class: "button danger" %>
+        </div>
+      <% end %>
+    </article>
+  </div>
+<% end %>
diff --git a/app/views/users/two_factor/show.html.erb b/app/views/users/two_factor/show.html.erb
index 1cd1afd..724465b 100644
--- a/app/views/users/two_factor/show.html.erb
+++ b/app/views/users/two_factor/show.html.erb
@@ -13,7 +13,7 @@
     <article class="prose dark:prose-invert">
       <p><%= t(".content") %></p>
       <%= link_to t(".enable.button"), new_two_factor_path, class:"button" unless enabled? %>
-      <%= link_to t(".disable.button"), "#", class:"button danger" if enabled? %>
+      <%= link_to t(".disable.button"), disable_two_factor_path, class:"button danger" if enabled? %>
     </article>
   </div>
 <% end %>
diff --git a/config/locales/kiqr.en.yml b/config/locales/kiqr.en.yml
index ade66a0..c03c77a 100644
--- a/config/locales/kiqr.en.yml
+++ b/config/locales/kiqr.en.yml
@@ -123,6 +123,15 @@ en:
           - "Enter the 6-digit code generated by your authenticator app."
         success: "Two-factor authentication has been enabled."
         invalid_code: "Invalid code. Please try again."
+      disable:
+        title: "Disable two-factor authentication"
+        description: "Remove the extra layer of security from your user account."
+        instructions: "To disable two-factor authentication, enter the 6-digit code from your authenticator app."
+        disabled: "Two-factor authentication has been disabled."
+        invalid_code: "Invalid code. Please try again."
+        label: "One-time password"
+        placeholder: "Enter the 6-digit code"
+        button: "Disable two-factor authentication"
       form:
         instructions: "Enter the code provided by your app to finish the two factor setup:"
         label: "One-time password"
diff --git a/config/routes/authentication.rb b/config/routes/authentication.rb
index 8b58796..9a5d3b7 100644
--- a/config/routes/authentication.rb
+++ b/config/routes/authentication.rb
@@ -12,7 +12,9 @@
   get "two-factor" => "two_factor#show", :as => :edit_two_factor
   get "two-factor/new" => "two_factor#new", :as => :new_two_factor
   get "two-factor/setup" => "two_factor#setup", :as => :setup_two_factor
+  get "two-factor/disable" => "two_factor#disable", :as => :disable_two_factor
   post "two-factor/verify" => "two_factor#verify", :as => :verify_two_factor
+  delete "two-factor/destroy" => "two_factor#destroy", :as => :destroy_two_factor
 
   get "delete" => "cancellations#show", :as => :delete_user_registration
 end
diff --git a/test/controllers/users/two_factor_controller_test.rb b/test/controllers/users/two_factor_controller_test.rb
index 9c5b0de..61fe2a5 100644
--- a/test/controllers/users/two_factor_controller_test.rb
+++ b/test/controllers/users/two_factor_controller_test.rb
@@ -18,4 +18,28 @@ class Users::TwoFactorControllerTest < ActionDispatch::IntegrationTest
     get new_two_factor_path
     assert_not_equal user.otp_secret, user.reload.otp_secret
   end
+
+  test "redirects to show page if two factor is alredy disabled" do
+    user = create(:user)
+    sign_in user
+    get disable_two_factor_path
+    assert_redirected_to edit_two_factor_path
+
+    delete destroy_two_factor_path
+    assert_redirected_to edit_two_factor_path
+  end
+
+  test "requires valid otp code to disable two factor authentication" do
+    user = create(:user, :otp_enabled)
+    sign_in user
+
+    delete destroy_two_factor_path, params: {user: {otp_attempt: "123456"}}
+    assert_response :unprocessable_entity
+    assert_template "two_factor/disable"
+    assert user.reload.otp_required_for_login?
+
+    delete destroy_two_factor_path, params: {user: {otp_attempt: user.current_otp}}
+    assert_redirected_to edit_two_factor_path
+    assert_not user.reload.otp_required_for_login?
+  end
 end
diff --git a/test/system/users/two_factor_test.rb b/test/system/users/two_factor_test.rb
index 2c32168..b698d05 100644
--- a/test/system/users/two_factor_test.rb
+++ b/test/system/users/two_factor_test.rb
@@ -52,7 +52,7 @@ def prepare_otp_setup(user)
     assert_not_equal first_image, second_image
   end
 
-  test "Shows instructions on how to disable two factor authentication" do
+  test "shows instructions on how to disable two factor authentication" do
     user = create(:user, :otp_enabled)
     sign_in user
     visit edit_two_factor_path