From 454c6ad8d36107c339de98b59a0651cc0268ce5e Mon Sep 17 00:00:00 2001 From: Laney Stroup Date: Thu, 19 Sep 2024 20:43:25 +0000 Subject: [PATCH 1/4] add support for :now option to "otp_set_flash_message" method (follows devise "set_flash_message"); --- lib/devise_otp_authenticatable/controllers/helpers.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/devise_otp_authenticatable/controllers/helpers.rb b/lib/devise_otp_authenticatable/controllers/helpers.rb index 03b8e38..332679e 100644 --- a/lib/devise_otp_authenticatable/controllers/helpers.rb +++ b/lib/devise_otp_authenticatable/controllers/helpers.rb @@ -16,7 +16,14 @@ def otp_set_flash_message(key, kind, options = {}) options[:resource_name] = resource_name options = devise_i18n_options(options) if respond_to?(:devise_i18n_options, true) message = I18n.t("#{options[:resource_name]}.#{kind}", **options) - flash[key] = message if message.present? + + if message.present? + if options[:now] + flash.now[key] = message + else + flash[key] = message + end + end end def otp_t From 193978c4dfe86168c7930f8ec778d7d19fa88b32 Mon Sep 17 00:00:00 2001 From: Laney Stroup Date: Thu, 19 Sep 2024 20:45:59 +0000 Subject: [PATCH 2/4] update flash message for invalid token to render immediately within "update" action; --- app/controllers/devise_otp/devise/otp_credentials_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/devise_otp/devise/otp_credentials_controller.rb b/app/controllers/devise_otp/devise/otp_credentials_controller.rb index 510fe85..7093ff1 100644 --- a/app/controllers/devise_otp/devise/otp_credentials_controller.rb +++ b/app/controllers/devise_otp/devise/otp_credentials_controller.rb @@ -36,7 +36,7 @@ def update otp_refresh_credentials_for(resource) respond_with resource, location: after_sign_in_path_for(resource) else - otp_set_flash_message :alert, :token_invalid + otp_set_flash_message :alert, :token_invalid, :now => true render :show end end From a883aaaeac6ec3d04bec5683737ef5274102d8e7 Mon Sep 17 00:00:00 2001 From: Laney Stroup Date: Thu, 19 Sep 2024 21:12:48 +0000 Subject: [PATCH 3/4] update test specs to ensure correct display of blank/invalid token flash messages, and to ensure that they do not display after a successful authentication; --- .../app/views/layouts/application.html.erb | 8 ++++- test/integration/sign_in_test.rb | 30 +++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/test/dummy/app/views/layouts/application.html.erb b/test/dummy/app/views/layouts/application.html.erb index 4cab268..b04369d 100644 --- a/test/dummy/app/views/layouts/application.html.erb +++ b/test/dummy/app/views/layouts/application.html.erb @@ -8,7 +8,13 @@ -<%= yield %> + <% if flash[:alert].present? %> +
+ <%= flash[:alert] %> +
+ <% end %> + + <%= yield %> diff --git a/test/integration/sign_in_test.rb b/test/integration/sign_in_test.rb index 774e6e3..88429d3 100644 --- a/test/integration/sign_in_test.rb +++ b/test/integration/sign_in_test.rb @@ -43,6 +43,7 @@ def teardown click_button "Submit Token" assert_equal user_otp_credential_path, current_path + assert page.has_content? "The token you provided was invalid." end test "fail blank token authentication" do @@ -53,6 +54,7 @@ def teardown click_button "Submit Token" assert_equal user_otp_credential_path, current_path + assert page.has_content? "You need to type in the token you generated with your device." end test "successful token authentication" do @@ -78,4 +80,32 @@ def teardown User.otp_authentication_timeout = old_timeout assert_equal new_user_session_path, current_path end + + test "blank token flash message does not persist to successful authentication redirect." do + user = enable_otp_and_sign_in + + fill_in "token", with: "123456" + click_button "Submit Token" + + assert page.has_content?("The token you provided was invalid.") + + fill_in "token", with: ROTP::TOTP.new(user.otp_auth_secret).at(Time.now) + click_button "Submit Token" + + assert !page.has_content?("The token you provided was invalid.") + end + + test "invalid token flash message does not persist to successful authentication redirect." do + user = enable_otp_and_sign_in + + fill_in "token", with: "" + click_button "Submit Token" + + assert page.has_content?("You need to type in the token you generated with your device.") + + fill_in "token", with: ROTP::TOTP.new(user.otp_auth_secret).at(Time.now) + click_button "Submit Token" + + assert !page.has_content?("You need to type in the token you generated with your device.") + end end From 61bc8bc537600e01defc569a166e9ac3296e34c2 Mon Sep 17 00:00:00 2001 From: Laney Stroup Date: Thu, 19 Sep 2024 21:17:56 +0000 Subject: [PATCH 4/4] update both blank and invalid OTP authentication attempts to render :show action for consistency; simplify condition for setting flash message; --- .../devise_otp/devise/otp_credentials_controller.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/app/controllers/devise_otp/devise/otp_credentials_controller.rb b/app/controllers/devise_otp/devise/otp_credentials_controller.rb index 7093ff1..ae6c6f1 100644 --- a/app/controllers/devise_otp/devise/otp_credentials_controller.rb +++ b/app/controllers/devise_otp/devise/otp_credentials_controller.rb @@ -26,17 +26,15 @@ def show # signs the resource in, if the OTP token is valid and the user has a valid challenge # def update - if @token.blank? - otp_set_flash_message(:alert, :token_blank) - redirect_to otp_credential_path_for(resource_name, challenge: @challenge, recovery: @recovery) - elsif resource.otp_challenge_valid? && resource.validate_otp_token(@token, @recovery) + if resource.otp_challenge_valid? && resource.validate_otp_token(@token, @recovery) sign_in(resource_name, resource) otp_set_trusted_device_for(resource) if params[:enable_persistence] == "true" otp_refresh_credentials_for(resource) respond_with resource, location: after_sign_in_path_for(resource) else - otp_set_flash_message :alert, :token_invalid, :now => true + kind = (@token.blank? ? :token_blank : :token_invalid) + otp_set_flash_message :alert, kind, :now => true render :show end end