diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5bf0f74..a688d57 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,24 @@
## UNRELEASED
+Summary: Add reset token action, and hide/repurpose disable token action
+
+Details:
+- Add reset token action to disable OTP, reset token secret, and redirect to otp_tokens#edit to re-enable with new token secret;
+- Update disable action to preserve the existing token secret (since the reset action now accomplishes this functionality);
+- Hide disable button when mandatory OTP;
+- Move disable button to bottom of page;
+
+Breaking Changes (config/locales/en.yml):
+- Add:
+ - reset\_link
+ - successfully\_reset\_otp
+- Move/Update
+ - disable\_explain > reset\_explain
+ - disable\_explain\_warn > reset\_explain\_warn
+
+## UNRELEASED
+
Fix regression due to warden session scope usage
Details:
diff --git a/app/controllers/devise_otp/devise/otp_tokens_controller.rb b/app/controllers/devise_otp/devise/otp_tokens_controller.rb
index b6ae3c6..c80aa7a 100644
--- a/app/controllers/devise_otp/devise/otp_tokens_controller.rb
+++ b/app/controllers/devise_otp/devise/otp_tokens_controller.rb
@@ -45,7 +45,6 @@ def update
#
def destroy
if resource.disable_otp!
- resource.clear_otp_fields!
otp_set_flash_message :success, :successfully_disabled_otp
end
@@ -95,6 +94,15 @@ def recovery
end
end
+ def reset
+ if resource.disable_otp!
+ resource.clear_otp_fields!
+ otp_set_flash_message :success, :successfully_reset_otp
+ end
+
+ redirect_to action: :edit
+ end
+
private
def ensure_credentials_refresh
diff --git a/app/views/devise/otp_tokens/_token_secret.html.erb b/app/views/devise/otp_tokens/_token_secret.html.erb
index 7ae09b5..a8c6a3c 100644
--- a/app/views/devise/otp_tokens/_token_secret.html.erb
+++ b/app/views/devise/otp_tokens/_token_secret.html.erb
@@ -7,11 +7,11 @@
<%= resource.otp_auth_secret %>
<%= button_to I18n.t('disable_link', :scope => 'devise.otp.otp_tokens'), @resource, :method => :delete, :data => { "turbo-method": "DELETE" } %>
+<%= button_to I18n.t('reset_link', :scope => 'devise.otp.otp_tokens'), reset_otp_token_path_for(resource), :method => :post , :data => { "turbo-method": "POST" } %>
- <%= I18n.t('disable_explain', :scope => 'devise.otp.otp_tokens') %> - <%= I18n.t('disable_explain_warn', :scope => 'devise.otp.otp_tokens') %> + <%= I18n.t('reset_explain', :scope => 'devise.otp.otp_tokens') %> + <%= I18n.t('reset_explain_warn', :scope => 'devise.otp.otp_tokens') %>
<%- if recovery_enabled? %> diff --git a/app/views/devise/otp_tokens/show.html.erb b/app/views/devise/otp_tokens/show.html.erb index 8ca6d0a..5a663c2 100644 --- a/app/views/devise/otp_tokens/show.html.erb +++ b/app/views/devise/otp_tokens/show.html.erb @@ -5,6 +5,10 @@ <%- if resource.otp_enabled? %> <%= render :partial => 'token_secret' if resource.otp_enabled? %> <%= render :partial => 'trusted_devices' if trusted_devices_enabled? %> + + <% unless otp_mandatory_on?(resource) %> + <%= button_to I18n.t('disable_link', :scope => 'devise.otp.otp_tokens'), @resource, :method => :delete, :data => { "turbo-method": "DELETE" } %> + <% end %> <% else %> <%= link_to I18n.t('enable_link', :scope => 'devise.otp.otp_tokens'), edit_otp_token_path_for(resource) %> <% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 133a162..40e17cb 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -30,11 +30,13 @@ en: title: 'Two-factors Authentication:' enable_link: 'Enable Two-Factor Authentication' disable_link: 'Disable Two-Factor Authentication' - disable_explain: 'This will disable Two-Factor authentication and clear the OTP secret.' - disable_explain_warn: 'To re-enable Two-Factor authentication, you will need to enroll your mobile device again.' + reset_link: 'Reset Token Secret' + reset_explain: 'Resetting your token secret will temporarilly disable Two-Factor authentication.' + reset_explain_warn: 'To re-enable Two-Factor authentication, you will need to re-enroll your mobile device with the new token secret.' successfully_updated: 'Your two-factors authentication settings have been updated.' could_not_confirm: 'The Confirmation Code you entered did not match the QR code shown below.' successfully_disabled_otp: 'Two-Factor authentication has been disabled.' + successfully_reset_otp: 'Your token secret has been reset. Please confirm your new token secret below.' successfully_set_persistence: 'Your device is now trusted.' successfully_cleared_persistence: 'Your device has been removed from the list of trusted devices.' successfully_reset_persistence: 'Your list of trusted devices has been cleared.' diff --git a/lib/devise_otp_authenticatable/controllers/public_helpers.rb b/lib/devise_otp_authenticatable/controllers/public_helpers.rb index 3ce7293..30ef1ba 100644 --- a/lib/devise_otp_authenticatable/controllers/public_helpers.rb +++ b/lib/devise_otp_authenticatable/controllers/public_helpers.rb @@ -16,7 +16,7 @@ def self.define_helpers(mapping) #:nodoc: def ensure_mandatory_#{mapping}_otp! resource = current_#{mapping} if !devise_controller? - if otp_mandatory_on?(resource) + if mandatory_otp_missing_on?(resource) redirect_to edit_#{mapping}_otp_token_path end end @@ -25,10 +25,13 @@ def ensure_mandatory_#{mapping}_otp! end def otp_mandatory_on?(resource) - return true if resource.class.otp_mandatory && !resource.otp_enabled return false unless resource.respond_to?(:otp_mandatory) - resource.otp_mandatory && !resource.otp_enabled + resource.class.otp_mandatory or resource.otp_mandatory + end + + def mandatory_otp_missing_on?(resource) + otp_mandatory_on?(resource) && !resource.otp_enabled end end diff --git a/lib/devise_otp_authenticatable/controllers/url_helpers.rb b/lib/devise_otp_authenticatable/controllers/url_helpers.rb index 4b8415e..14754fb 100644 --- a/lib/devise_otp_authenticatable/controllers/url_helpers.rb +++ b/lib/devise_otp_authenticatable/controllers/url_helpers.rb @@ -26,6 +26,11 @@ def edit_otp_token_path_for(resource_or_scope, opts = {}) send("edit_#{scope}_otp_token_path", opts) end + def reset_otp_token_path_for(resource_or_scope, opts = {}) + scope = ::Devise::Mapping.find_scope!(resource_or_scope) + send("reset_#{scope}_otp_token_path", opts) + end + def otp_credential_path_for(resource_or_scope, opts = {}) scope = ::Devise::Mapping.find_scope!(resource_or_scope) send("#{scope}_otp_credential_path", opts) diff --git a/lib/devise_otp_authenticatable/engine.rb b/lib/devise_otp_authenticatable/engine.rb index 035c83a..6b9f168 100644 --- a/lib/devise_otp_authenticatable/engine.rb +++ b/lib/devise_otp_authenticatable/engine.rb @@ -12,11 +12,13 @@ class Engine < ::Rails::Engine ActiveSupport.on_load(:devise_controller) do include DeviseOtpAuthenticatable::Controllers::UrlHelpers include DeviseOtpAuthenticatable::Controllers::Helpers + include DeviseOtpAuthenticatable::Controllers::PublicHelpers end ActiveSupport.on_load(:action_view) do include DeviseOtpAuthenticatable::Controllers::UrlHelpers include DeviseOtpAuthenticatable::Controllers::Helpers + include DeviseOtpAuthenticatable::Controllers::PublicHelpers end # See: https://guides.rubyonrails.org/engines.html#separate-assets-and-precompiling diff --git a/lib/devise_otp_authenticatable/routes.rb b/lib/devise_otp_authenticatable/routes.rb index a751306..45e7296 100644 --- a/lib/devise_otp_authenticatable/routes.rb +++ b/lib/devise_otp_authenticatable/routes.rb @@ -13,6 +13,7 @@ def devise_otp(mapping, controllers) end get :recovery + post :reset end resource :credential, only: [:show, :update], diff --git a/test/integration/disable_token_test.rb b/test/integration/disable_token_test.rb new file mode 100644 index 0000000..ff3efbe --- /dev/null +++ b/test/integration/disable_token_test.rb @@ -0,0 +1,53 @@ +require "test_helper" +require "integration_tests_helper" + +class DisableTokenTest < ActionDispatch::IntegrationTest + + def setup + # log in 1fa + @user = enable_otp_and_sign_in + assert_equal user_otp_credential_path, current_path + + # otp 2fa + fill_in "user_token", with: ROTP::TOTP.new(@user.otp_auth_secret).at(Time.now) + click_button "Submit Token" + assert_equal root_path, current_path + end + + def teardown + Capybara.reset_sessions! + end + + test "disabling OTP after successfully enabling" do + # disable OTP + disable_otp + + assert page.has_content? "Disabled" + + # logout + sign_out + + # log back in 1fa + sign_user_in(@user) + + assert_equal root_path, current_path + end + + test "disabling OTP does not reset token secrets" do + # get otp secrets + @user.reload + auth_secret = @user.otp_auth_secret + recovery_secret = @user.otp_recovery_secret + + # disable OTP + disable_otp + + # compare otp secrets + assert_not_nil @user.otp_auth_secret + assert_equal @user.otp_auth_secret, auth_secret + + assert_not_nil @user.otp_recovery_secret + assert_equal @user.otp_recovery_secret, recovery_secret + end + +end diff --git a/test/integration/reset_token_test.rb b/test/integration/reset_token_test.rb new file mode 100644 index 0000000..a6eba69 --- /dev/null +++ b/test/integration/reset_token_test.rb @@ -0,0 +1,45 @@ +require "test_helper" +require "integration_tests_helper" + +class ResetTokenTest < ActionDispatch::IntegrationTest + + def setup + # log in 1fa + @user = enable_otp_and_sign_in + assert_equal user_otp_credential_path, current_path + + # otp 2fa + fill_in "user_token", with: ROTP::TOTP.new(@user.otp_auth_secret).at(Time.now) + click_button "Submit Token" + assert_equal root_path, current_path + end + + + def teardown + Capybara.reset_sessions! + end + + test "redirects to otp_tokens#edit page" do + reset_otp + + assert_equal "/users/otp/token/edit", current_path + end + + test "generates new token secrets" do + # get auth secrets + auth_secret = @user.otp_auth_secret + recovery_secret = @user.otp_recovery_secret + + # reset otp + reset_otp + + # compare auth secrets + @user.reload + assert_not_nil @user.otp_auth_secret + assert_not_equal @user.otp_auth_secret, auth_secret + + assert_not_nil @user.otp_recovery_secret + assert_not_equal @user.otp_recovery_secret, recovery_secret + end + +end diff --git a/test/integration/token_test.rb b/test/integration/token_test.rb deleted file mode 100644 index ed2f63d..0000000 --- a/test/integration/token_test.rb +++ /dev/null @@ -1,32 +0,0 @@ -require "test_helper" -require "integration_tests_helper" - -class TokenTest < ActionDispatch::IntegrationTest - def teardown - Capybara.reset_sessions! - end - - test "disabling OTP after successfully enabling" do - # log in 1fa - user = enable_otp_and_sign_in - assert_equal user_otp_credential_path, current_path - - # otp 2fa - fill_in "user_token", with: ROTP::TOTP.new(user.otp_auth_secret).at(Time.now) - click_button "Submit Token" - assert_equal root_path, current_path - - # disable OTP - disable_otp - - assert page.has_content? "Disabled" - - # logout - sign_out - - # log back in 1fa - sign_user_in(user) - - assert_equal root_path, current_path - end -end diff --git a/test/integration_tests_helper.rb b/test/integration_tests_helper.rb index 36ca0a0..b11f47b 100644 --- a/test/integration_tests_helper.rb +++ b/test/integration_tests_helper.rb @@ -59,6 +59,11 @@ def disable_otp click_button "Disable Two-Factor Authentication" end + def reset_otp + visit user_otp_token_path + click_button "Reset Token Secret" + end + def sign_out logout :user end