Skip to content

Commit

Permalink
feat: pages for disabling two factor authentication
Browse files Browse the repository at this point in the history
  • Loading branch information
kjellberg committed Apr 2, 2024
1 parent 96b8888 commit 65240b2
Show file tree
Hide file tree
Showing 8 changed files with 77 additions and 4 deletions.
16 changes: 16 additions & 0 deletions app/controllers/users/two_factor_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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!
Expand Down
23 changes: 23 additions & 0 deletions app/views/users/two_factor/disable.html.erb
Original file line number Diff line number Diff line change
@@ -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 %>
2 changes: 1 addition & 1 deletion app/views/users/two_factor/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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 %>
9 changes: 9 additions & 0 deletions config/locales/kiqr.en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 2 additions & 0 deletions config/routes/authentication.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
24 changes: 24 additions & 0 deletions test/controllers/users/two_factor_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion test/system/users/two_factor_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 65240b2

Please sign in to comment.