From 0d7f8e1e401736d477689e44271805a238d291ef Mon Sep 17 00:00:00 2001 From: Martin Emde Date: Fri, 29 Nov 2024 12:10:44 -0800 Subject: [PATCH] Update PasswordsControllerTest to use modern Rails IntegrationTest --- app/controllers/passwords_controller.rb | 9 +- app/models/user.rb | 6 +- app/views/multifactor_auths/prompt.html.erb | 2 +- config/locales/de.yml | 1 + config/locales/en.yml | 3 +- config/locales/es.yml | 1 + config/locales/fr.yml | 1 + config/locales/ja.yml | 1 + config/locales/nl.yml | 1 + config/locales/pt-BR.yml | 1 + config/locales/zh-CN.yml | 1 + config/locales/zh-TW.yml | 1 + test/factories/user.rb | 4 + test/functional/passwords_controller_test.rb | 685 ++++++++----------- test/mailers/password_mailer_test.rb | 2 + test/system/avo/users_test.rb | 20 +- test/test_helper.rb | 23 + 17 files changed, 345 insertions(+), 417 deletions(-) diff --git a/app/controllers/passwords_controller.rb b/app/controllers/passwords_controller.rb index 9a20498dcfd..a9d1be8306b 100644 --- a/app/controllers/passwords_controller.rb +++ b/app/controllers/passwords_controller.rb @@ -38,10 +38,11 @@ def update @user.reset_api_key! if reset_params[:reset_api_key] == "true" # singular @user.api_keys.expire_all! if reset_params[:reset_api_keys] == "true" # plural delete_password_reset_session + flash[:notice] = t(".success") redirect_to signed_in? ? dashboard_path : sign_in_path else flash.now[:alert] = t(".failure") - render :edit + render :edit, status: :unprocessable_entity end end @@ -65,6 +66,7 @@ def ensure_email_present def validate_confirmation_token confirmation_token = params.permit(:token).fetch(:token, "").to_s + return login_failure(t("passwords.edit.token_failure")) if confirmation_token.blank? @user = User.find_by(confirmation_token:) return login_failure(t("passwords.edit.token_failure")) unless @user&.valid_confirmation_token? sign_out if signed_in? && @user != current_user @@ -82,7 +84,7 @@ def password_reset_session_verified def validate_password_reset_session return login_failure(t("passwords.edit.token_failure")) if session[:password_reset_verified].nil? - return login_failure(t("verification_expired")) if session[:password_reset_verified] < Time.current + return login_failure(t("verification_expired")) if Time.current.after?(session[:password_reset_verified]) @user = User.find_by(id: session[:password_reset_verified_user]) login_failure(t("verification_expired")) unless @user end @@ -98,8 +100,7 @@ def reset_params end def mfa_failure(message) - flash.now.alert = message - render template: "multifactor_auths/prompt", status: :unauthorized + prompt_mfa(alert: message, status: :unauthorized) end def login_failure(alert) diff --git a/app/models/user.rb b/app/models/user.rb index 3741b4c7003..b10a873d9c4 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -212,12 +212,12 @@ def total_rubygems_count def confirm_email! return false if unconfirmed_email && !update_email - update!(email_confirmed: true, confirmation_token: nil) + update!(email_confirmed: true, confirmation_token: nil, token_expires_at: Time.zone.now) end - # confirmation token expires after 15 minutes + # confirmation token expires after 3 hours def valid_confirmation_token? - token_expires_at > Time.zone.now + confirmation_token.present? && Time.zone.now.before?(token_expires_at) end def generate_confirmation_token(reset_unconfirmed_email: true) diff --git a/app/views/multifactor_auths/prompt.html.erb b/app/views/multifactor_auths/prompt.html.erb index 2c8c89707e2..7f7ef2a9393 100644 --- a/app/views/multifactor_auths/prompt.html.erb +++ b/app/views/multifactor_auths/prompt.html.erb @@ -19,7 +19,7 @@
<% if @user.totp_enabled? %> <%= label_tag :otp, t(".otp_or_recovery"), class: 'form__label' %> - <%= text_field_tag :otp, '', class: 'form__input', autofocus: true, autocomplete: :off %> + <%= text_field_tag :otp, '', class: 'form__input', autofocus: true, autocomplete: "one-time-code" %> <% elsif @user.webauthn_only_with_recovery? %> <%= text_field_tag :otp, '', diff --git a/config/locales/de.yml b/config/locales/de.yml index b4f178cef70..4e27d72653d 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -543,6 +543,7 @@ de: Ändern deines Passworts. failure_on_missing_email: Die E-Mail darf nicht leer sein. update: + success: failure: Dein Passwort konnte nicht geändert werden. Bitte versuche es erneut. multifactor_auths: session_expired: Deine Sitzung auf der Anmeldeseite ist abgelaufen. diff --git a/config/locales/en.yml b/config/locales/en.yml index 0a7860a919c..8aad71155ef 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -474,6 +474,7 @@ en: success: You will receive an email within the next few minutes. It contains instructions for changing your password. failure_on_missing_email: Email can't be blank. update: + success: Your password has been changed. failure: Your password could not be changed. Please try again. multifactor_auths: session_expired: Your login page session has expired. @@ -516,7 +517,7 @@ en: otp_code: OTP code otp_or_recovery: OTP or recovery code recovery_code: Recovery code - recovery_code_html: 'You can use a valid recovery code if you have lost access to your multi-factor authentication device or to your security device.' + recovery_code_html: 'You can use a valid recovery code if you have lost access to your multi-factor authentication device or to your security device.' security_device: Security Device verify_code: Verify code totps: diff --git a/config/locales/es.yml b/config/locales/es.yml index db31e6a96e9..13af010f64d 100644 --- a/config/locales/es.yml +++ b/config/locales/es.yml @@ -529,6 +529,7 @@ es: success: failure_on_missing_email: update: + success: failure: multifactor_auths: session_expired: Ha expirado tu sesión en la página de acceso. diff --git a/config/locales/fr.yml b/config/locales/fr.yml index b5a87ed326f..335f12a7271 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -493,6 +493,7 @@ fr: success: failure_on_missing_email: update: + success: failure: multifactor_auths: session_expired: diff --git a/config/locales/ja.yml b/config/locales/ja.yml index cc43432f515..da473912e4e 100644 --- a/config/locales/ja.yml +++ b/config/locales/ja.yml @@ -473,6 +473,7 @@ ja: success: 数分でEメールが届きます。メールにはパスワードを変更する手順が記載されています。 failure_on_missing_email: Eメールは空にできません。 update: + success: failure: パスワードを変更できませんでした。もう一度お試しください。 multifactor_auths: session_expired: ログインページのセッションが期限切れになりました。 diff --git a/config/locales/nl.yml b/config/locales/nl.yml index 4d12bdc8de0..8b80c356537 100644 --- a/config/locales/nl.yml +++ b/config/locales/nl.yml @@ -478,6 +478,7 @@ nl: success: failure_on_missing_email: update: + success: failure: multifactor_auths: session_expired: diff --git a/config/locales/pt-BR.yml b/config/locales/pt-BR.yml index 5aa7ccc65c5..17f9db3650f 100644 --- a/config/locales/pt-BR.yml +++ b/config/locales/pt-BR.yml @@ -490,6 +490,7 @@ pt-BR: success: failure_on_missing_email: update: + success: failure: multifactor_auths: session_expired: diff --git a/config/locales/zh-CN.yml b/config/locales/zh-CN.yml index b8de0c284b4..a4f888e0680 100644 --- a/config/locales/zh-CN.yml +++ b/config/locales/zh-CN.yml @@ -480,6 +480,7 @@ zh-CN: success: failure_on_missing_email: update: + success: failure: multifactor_auths: session_expired: 您的登录页面会话已过期。 diff --git a/config/locales/zh-TW.yml b/config/locales/zh-TW.yml index e5153572d9b..8d051bfeb67 100644 --- a/config/locales/zh-TW.yml +++ b/config/locales/zh-TW.yml @@ -474,6 +474,7 @@ zh-TW: success: failure_on_missing_email: update: + success: failure: multifactor_auths: session_expired: 您的登入頁面工作階段已過期。 diff --git a/test/factories/user.rb b/test/factories/user.rb index 29bd04cb235..4164fc96b13 100644 --- a/test/factories/user.rb +++ b/test/factories/user.rb @@ -13,6 +13,10 @@ end mfa_hashed_recovery_codes { mfa_recovery_codes.map { |code| BCrypt::Password.create(code) } } + after :create do |user, evaluator| + user.confirm_email! if evaluator.email_confirmed != false && evaluator.unconfirmed_email.blank? + end + trait :unconfirmed do email_confirmed { false } unconfirmed_email { "#{SecureRandom.hex(8)}#{email}" } diff --git a/test/functional/passwords_controller_test.rb b/test/functional/passwords_controller_test.rb index 9e7ae33ac0f..f03e166f2d3 100644 --- a/test/functional/passwords_controller_test.rb +++ b/test/functional/passwords_controller_test.rb @@ -1,22 +1,37 @@ require "test_helper" -class PasswordsControllerTest < ActionController::TestCase +class PasswordsControllerTest < ActionDispatch::IntegrationTest + context "on GET to new" do + should "display the password reset form" do + get new_password_path + + assert_response :success + assert_select "h1", "Change your password" + assert_select "form[action=?]", password_path do + assert_select "input[type=email][name=?]", "password[email]" + end + end + end + context "on POST to create" do context "when missing email" do should "alerts about missing email" do - post :create + post password_path assert_equal "Email can't be blank.", flash[:alert] end end context "with valid params" do - setup do + should "set a valid confirmation_token" do @user = create(:user) - get :create, params: { password: { email: @user.email } } - end - should "set a valid confirmation_token" do + assert_nil @user.confirmation_token + + post password_path, params: { password: { email: @user.email } } + + assert_select "p", "You will receive an email within the next few minutes. It contains instructions for changing your password." + assert_not_nil @user.reload.confirmation_token assert_predicate @user, :valid_confirmation_token? end end @@ -29,197 +44,127 @@ class PasswordsControllerTest < ActionController::TestCase end context "with incorrect token" do - setup do - get :edit, params: { token: "invalidtoken" } - end + should "redirect to the sign in page" do + get edit_password_path, params: { token: "invalidtoken" } - should redirect_to("the sign in page") { sign_in_path } - should set_flash[:alert].to "Please double check the URL or try submitting a new password reset." - - should "not sign in the user" do - refute_predicate @controller.request.env[:clearance], :signed_in? + assert_redirected_to sign_in_path + assert_equal "Please double check the URL or try submitting a new password reset.", flash[:alert] + refute_signed_in end end context "with valid confirmation_token" do context "when not signed in" do - setup do - get :edit, params: { token: @user.confirmation_token } - end - - should respond_with :success + should "presents the password edit form" do + get edit_password_path, params: { token: @user.confirmation_token } - should "not sign in the user" do - refute_predicate @controller.request.env[:clearance], :signed_in? - end + assert_response :success + assert_new_password_form - should "invalidate the confirmation_token" do assert_nil @user.reload.confirmation_token - end + refute_signed_in - should "display edit form" do - assert_text "Reset password" - assert_selector "input[type=password][autocomplete=new-password]" - end - - should "instruct the browser not to send referrer that contains the token" do + # instruct the browser not to send referrer that contains the token" do assert_equal "no-referrer", response.headers["Referrer-Policy"] end end context "when signed in as the user" do - setup do - sign_in_as @user + should "presents the password edit form" do + get edit_password_path(as: @user), params: { token: @user.confirmation_token } - get :edit, params: { token: @user.confirmation_token } - end + assert_response :success + assert_new_password_form - should respond_with :success - - should "leave the user signed in" do - assert_predicate @controller.request.env[:clearance], :signed_in? - end - - should "invalidate the confirmation_token" do assert_nil @user.reload.confirmation_token - end - - should "display edit form" do - assert_text "Reset password" - assert_selector "input[type=password][autocomplete=new-password]" - end - should "instruct the browser not to send referrer that contains the token" do + # instruct the browser not to send referrer that contains the token" do assert_equal "no-referrer", response.headers["Referrer-Policy"] end end context "when signed in as another user" do - setup do + should "presents the password edit form for the token identified user, signing the other user out" do @other_user = create(:user, api_key: "otheruserkey") - sign_in_as @other_user - get :edit, params: { token: @user.confirmation_token } - end + get edit_password_path(as: @other_user), params: { token: @user.confirmation_token } - should respond_with :success + assert_response :success + assert_new_password_form - should "sign the current user out" do - refute_predicate @controller.request.env[:clearance], :signed_in? - end - - should "invalidate the confirmation_token" do + refute_signed_in assert_nil @user.reload.confirmation_token - end - should "display edit form" do - assert_text "Reset password" - assert_selector "input[type=password][autocomplete=new-password]" - end - - should "instruct the browser not to send referrer that contains the token" do + # instruct the browser not to send referrer that contains the token" do assert_equal "no-referrer", response.headers["Referrer-Policy"] end end end context "with expired confirmation_token" do - setup do + should "redirect to the sign in page" do @user.update_attribute(:token_expires_at, 1.minute.ago) - get :edit, params: { token: @user.confirmation_token } - end - - should redirect_to("the sign in page") { sign_in_path } - should set_flash[:alert].to "Please double check the URL or try submitting a new password reset." + get edit_password_path, params: { token: @user.confirmation_token } - should "not sign in the user" do - refute_predicate @controller.request.env[:clearance], :signed_in? + assert_redirected_to sign_in_path + assert_equal I18n.t("passwords.edit.token_failure"), flash[:alert] + refute_signed_in end end context "with totp enabled" do - setup do + should "display otp form" do @user.enable_totp!(ROTP::Base32.random_base32, :ui_only) - get :edit, params: { token: @user.confirmation_token } - end + get edit_password_path, params: { token: @user.confirmation_token } - should respond_with :success - - should "not sign in the user" do - refute_predicate @controller.request.env[:clearance], :signed_in? - end - - should "display otp form" do - assert page.has_content?("Multi-factor authentication") - assert page.has_content?("OTP code") - assert page.has_button?("Authenticate") + assert_response :success + assert_otp_form + refute_signed_in end end context "when user has webauthn credentials but no recovery codes" do - setup do + should "display webauthn prompt only" do create(:webauthn_credential, user: @user) - @user.new_mfa_recovery_codes = nil - @user.mfa_hashed_recovery_codes = [] - @user.save! - get :edit, params: { token: @user.confirmation_token } - end + @user.update!(new_mfa_recovery_codes: nil, mfa_hashed_recovery_codes: []) - should respond_with :success + get edit_password_path, params: { token: @user.confirmation_token } - should "not sign in the user" do - refute_predicate @controller.request.env[:clearance], :signed_in? - end - - should "display webauthn prompt" do - assert page.has_button?("Authenticate with security device") - end - - should "not display recovery code prompt" do - refute page.has_content?("Recovery code") + assert_response :success + assert_webauthn_form + refute page.has_content?("Recovery code"), "Recovery code form should not be displayed" + refute_signed_in end end context "when user has webauthn credentials and recovery codes" do - setup do + should "display webauthn prompt and recovery code prompt" do create(:webauthn_credential, user: @user) - get :edit, params: { token: @user.confirmation_token } - end - - should respond_with :success + get edit_password_path, params: { token: @user.confirmation_token } - should "not sign in the user" do - refute_predicate @controller.request.env[:clearance], :signed_in? - end - - should "display webauthn prompt" do - assert page.has_button?("Authenticate with security device") - end - - should "display recovery code prompt" do - assert page.has_content?("Recovery code") + assert_response :success + assert_webauthn_form + assert_select "form[action=?]", otp_edit_password_url(token: @user.confirmation_token) do + assert_select "input[type=text][autocomplete=off]" # no autocomplete for recovery code only + assert_select "input[type=submit][value=?]", I18n.t("authenticate") + end + assert page.has_content?("Recovery code"), "Expect recovery code form" + refute_signed_in end end context "when user has webauthn and totp" do - setup do + should "display webauthn and otp prompt" do @user.enable_totp!(ROTP::Base32.random_base32, :ui_and_api) create(:webauthn_credential, user: @user) - get :edit, params: { token: @user.confirmation_token } - end - - should respond_with :success - - should "not sign in the user" do - refute_predicate @controller.request.env[:clearance], :signed_in? - end - should "display webauthn prompt" do - assert page.has_button?("Authenticate with security device") - end + get edit_password_path, params: { token: @user.confirmation_token } - should "display otp prompt" do - assert page.has_content?("OTP or recovery code") + assert_response :success + assert_webauthn_form + assert_otp_form + assert page.has_content?(I18n.t("multifactor_auths.prompt.otp_or_recovery")), "Expect OTP or recovery code form" + refute_signed_in end end end @@ -230,64 +175,59 @@ class PasswordsControllerTest < ActionController::TestCase @user.forgot_password! end + context "when providing incorrect token" do + should "redirect to the sign in page" do + post otp_edit_password_path, params: { token: "badtoken" } + + assert_redirected_to sign_in_path + assert_equal "Please double check the URL or try submitting a new password reset.", flash[:alert] + assert_nil session[:mfa_expires_at] + refute_signed_in + end + end + context "with mfa enabled" do setup { @user.enable_totp!(ROTP::Base32.random_base32, :ui_only) } context "when OTP is correct" do - setup do - get :edit, params: { token: @user.confirmation_token } - post :otp_edit, params: { token: @user.confirmation_token, otp: ROTP::TOTP.new(@user.totp_seed).now } - end - - should respond_with :success + should "display edit form" do + get edit_password_path, params: { token: @user.confirmation_token } + post otp_edit_password_path, params: { token: @user.confirmation_token, otp: ROTP::TOTP.new(@user.totp_seed).now } - should "not sign in the user" do - refute_predicate @controller.request.env[:clearance], :signed_in? - end + assert_response :success + assert_new_password_form - should "invalidate the confirmation_token" do + refute_signed_in assert_nil @user.reload.confirmation_token - end - - should "display edit form" do - assert_text "Reset password" - end - - should "clear mfa_expires_at" do - assert_nil @controller.session[:mfa_expires_at] + assert_nil session[:mfa_expires_at] end end context "when OTP is incorrect" do - setup do - get :edit, params: { token: @user.confirmation_token } - post :otp_edit, params: { token: @user.confirmation_token, otp: "eatthis" } - end + should "display error message and prompt for MFA again" do + get edit_password_path, params: { token: @user.confirmation_token } + post otp_edit_password_path, params: { token: @user.confirmation_token, otp: "wrong" } - should respond_with :unauthorized - should set_flash.now[:alert].to "Your OTP code is incorrect." + assert_response :unauthorized + assert_select "#flash_alert", "Your OTP code is incorrect." + assert_otp_form - should "not sign in the user" do - refute_predicate @controller.request.env[:clearance], :signed_in? + refute_signed_in end end context "when the OTP session is expired" do - setup do - get :edit, params: { token: @user.confirmation_token } + should "redirect to the sign in page" do + get edit_password_path, params: { token: @user.confirmation_token } travel 16.minutes do - post :otp_edit, params: { token: @user.confirmation_token, otp: ROTP::TOTP.new(@user.totp_seed).now } + post otp_edit_password_path, params: { token: @user.confirmation_token, otp: ROTP::TOTP.new(@user.totp_seed).now } end - end - should set_flash[:alert].to "Your login page session has expired." - should redirect_to("the sign in page") { sign_in_path } + assert_redirected_to sign_in_path + assert_equal "Your login page session has expired.", flash[:alert] - should "clear mfa_expires_at" do - assert_nil @controller.session[:mfa_expires_at] - end - should "not sign in the user" do - refute_predicate @controller.request.env[:clearance], :signed_in? + assert_nil session[:mfa_expires_at] + refute_signed_in end end end @@ -296,147 +236,87 @@ class PasswordsControllerTest < ActionController::TestCase context "on POST to webauthn_edit" do setup do @user = create(:user) + @user.forgot_password! @webauthn_credential = create(:webauthn_credential, user: @user) - get :edit, params: { token: @user.confirmation_token } + @origin = WebAuthn.configuration.origin @rp_id = URI.parse(@origin).host @client = WebAuthn::FakeClient.new(@origin, encoding: false) end - context "with webauthn enabled" do - setup do - @challenge = session[:webauthn_authentication]["challenge"] - WebauthnHelpers.create_credential( - webauthn_credential: @webauthn_credential, - client: @client - ) - post( - :webauthn_edit, - params: { - token: @user.confirmation_token, - credentials: - WebauthnHelpers.get_result( - client: @client, - challenge: @challenge - ) - } - ) - end - - should respond_with :success + context "with correct webauthn" do + should "display edit form" do + get edit_password_path, params: { token: @user.confirmation_token } + post webauthn_edit_password_path, params: { + token: @user.confirmation_token, credentials: webauthn_result + } - should "not sign in the user" do - refute_predicate @controller.request.env[:clearance], :signed_in? - end + assert_response :success + assert_new_password_form - should "invalidate the confirmation_token" do + refute_signed_in assert_nil @user.reload.confirmation_token - end - - should "display edit form" do - assert_text "Reset password" - end - - should "clear mfa_expires_at" do - assert_nil @controller.session[:mfa_expires_at] + assert_nil session[:mfa_expires_at] end end - context "when providing incorrect token" do - setup do - post(:webauthn_edit, params: { token: "badtoken" }) - end + context "when providing incorrect confirmation_token" do + should "redirect to the sign in page" do + get edit_password_path, params: { token: @user.confirmation_token } + post webauthn_edit_password_path, params: { + token: "wrongtoken", credentials: webauthn_result + } - should redirect_to("the sign in page") { sign_in_path } - should set_flash[:alert].to "Please double check the URL or try submitting a new password reset." + assert_redirected_to sign_in_path + assert_equal "Please double check the URL or try submitting a new password reset.", flash[:alert] - should "not sign in the user" do - refute_predicate @controller.request.env[:clearance], :signed_in? + assert_nil session[:mfa_expires_at] + refute_signed_in end end context "when not providing credentials" do - setup do - post :webauthn_edit, params: { token: @user.confirmation_token }, format: :html - end - - should respond_with :unauthorized + should "display error message and prompt for MFA again" do + get edit_password_path, params: { token: @user.confirmation_token } + post webauthn_edit_password_path, params: { token: @user.confirmation_token } - should "not sign in the user" do - refute_predicate @controller.request.env[:clearance], :signed_in? - end + assert_response :unauthorized + assert_select "#flash_alert", "Credentials required" + assert_webauthn_form - should "set flash notice" do - assert_equal "Credentials required", flash[:alert] + refute_signed_in end end context "when providing wrong credential" do - setup do - @wrong_challenge = SecureRandom.hex - WebauthnHelpers.create_credential( - webauthn_credential: @webauthn_credential, - client: @client - ) - post( - :webauthn_edit, - params: { - token: @user.confirmation_token, - credentials: - WebauthnHelpers.get_result( - client: @client, - challenge: @wrong_challenge - ) - } - ) - end - - should respond_with :unauthorized - - should "not sign in the user" do - refute_predicate @controller.request.env[:clearance], :signed_in? - end + should "display error message and prompt for MFA again" do + get edit_password_path, params: { token: @user.confirmation_token } + wrong_challenge = SecureRandom.hex + post webauthn_edit_password_path, params: { + token: @user.confirmation_token, credentials: webauthn_result(wrong_challenge) + } - should "set flash notice" do - assert_equal "WebAuthn::ChallengeVerificationError", flash[:alert] - end + assert_response :unauthorized + assert_select "#flash_alert", "WebAuthn::ChallengeVerificationError" + assert_webauthn_form - should "still have the webauthn form url" do - assert_not_nil page.find(".js-webauthn-session--form")[:action] + refute_signed_in end end context "when webauthn session is expired" do - setup do - @challenge = session[:webauthn_authentication]["challenge"] - WebauthnHelpers.create_credential( - webauthn_credential: @webauthn_credential, - client: @client - ) + should "redirect to the sign in page" do + get edit_password_path, params: { token: @user.confirmation_token } travel 16.minutes do - post( - :webauthn_edit, - params: { - token: @user.confirmation_token, - credentials: - WebauthnHelpers.get_result( - client: @client, - challenge: @challenge - ) - } - ) + post webauthn_edit_password_path, params: { + token: @user.confirmation_token, credentials: webauthn_result + } end - end - - should redirect_to("the sign in page") { sign_in_path } - should set_flash[:alert] - should "clear mfa_expires_at" do - assert_nil @controller.session[:mfa_expires_at] - end - - should "not sign in the user" do - refute_predicate @controller.request.env[:clearance], :signed_in? + assert_redirected_to sign_in_path + assert_equal "Your login page session has expired.", flash[:alert] + assert_nil session[:mfa_expires_at] + refute_signed_in end end end @@ -444,167 +324,176 @@ class PasswordsControllerTest < ActionController::TestCase context "on PUT to update" do setup do @user = create(:user) + @user.forgot_password! @api_key = @user.api_key @new_api_key = create(:api_key, owner: @user) @old_encrypted_password = @user.encrypted_password end context "when not verified for password reset" do - setup do - put :update, params: { + should "redirect to the sign in page" do + put password_path, params: { password_reset: { reset_api_key: "true", reset_api_keys: "true", password: PasswordHelpers::SECURE_TEST_PASSWORD } } - end - should redirect_to("the sign in page") { sign_in_path } + assert_redirected_to sign_in_path + assert_equal "Please double check the URL or try submitting a new password reset.", flash[:alert] + + @user.reload - should "not change api_key" do - assert_equal(@user.reload.api_key, @api_key) + assert_equal @user.api_key, @api_key + assert_equal @user.encrypted_password, @old_encrypted_password + refute_signed_in end - should "not change password" do - assert_equal(@user.reload.encrypted_password, @old_encrypted_password) + end + + context "when verification has expired" do + should "redirect to the sign in page" do + get edit_password_path, params: { token: @user.confirmation_token } + travel 16.minutes do + put password_path, params: { + password_reset: { password: PasswordHelpers::SECURE_TEST_PASSWORD } + } + end + + assert_redirected_to sign_in_path + assert_equal I18n.t("verification_expired"), flash[:alert] + + @user.reload + + assert_equal @user.api_key, @api_key + assert_equal @user.encrypted_password, @old_encrypted_password + refute_signed_in end - should "not sign in the user" do - refute_predicate @controller.request.env[:clearance], :signed_in? + end + + context "with invalid password" do + should "redisplay edit form and not change password" do + get edit_password_path, params: { token: @user.confirmation_token } + put password_path, params: { + password_reset: { reset_api_key: "true", password: "pass" } + } + + assert_response :unprocessable_entity + assert_select "#flash_alert", "Your password could not be changed. Please try again." + assert_select "h1", "Reset password" + assert_select "#errorExplanation", /Password is too short \(minimum is 10 characters\)/ + assert_select "form[action=?]", password_path do + assert_select "input[type=password][autocomplete=new-password]" + end + + @user.reload + + assert_equal @user.api_key, @api_key + assert_equal @user.encrypted_password, @old_encrypted_password end end - context "when not verified for password reset" do - setup do - put :update, params: { + context "with valid password without reset_api_key" do + should "change password but not change api_key" do + get edit_password_path, params: { token: @user.confirmation_token } + put password_path, params: { password_reset: { password: PasswordHelpers::SECURE_TEST_PASSWORD } } - end - should redirect_to("the sign in page") { sign_in_path } - should set_flash[:alert].to "Please double check the URL or try submitting a new password reset." + assert_redirected_to sign_in_path + assert_equal "Your password has been changed.", flash[:notice] - should "not change api_key" do - assert_equal(@user.reload.api_key, @api_key) - end - should "not change password" do - assert_equal(@user.reload.encrypted_password, @old_encrypted_password) + @user.reload + + assert_equal @user.api_key, @api_key + refute_equal @user.encrypted_password, @old_encrypted_password end end - context "when signed in" do - setup do - sign_in_as @user - get :edit, params: { token: @user.confirmation_token } - end + context "with valid password with reset_api_key false" do + should "change password but not change api_key" do + get edit_password_path, params: { token: @user.confirmation_token } + put password_path, params: { + password_reset: { reset_api_key: "false", password: PasswordHelpers::SECURE_TEST_PASSWORD } + } - context "with invalid password" do - setup do - put :update, params: { - password_reset: { reset_api_key: "true", password: "pass" } - } - end + assert_redirected_to sign_in_path + # assert_equal "Your password has been changed.", flash[:notice] - should respond_with :success - should set_flash.now[:alert].to "Your password could not be changed. Please try again." + @user.reload - should "not change api_key" do - assert_equal(@user.reload.api_key, @api_key) - end - should "not change password" do - assert_equal(@user.reload.encrypted_password, @old_encrypted_password) - end + assert_equal @user.api_key, @api_key + refute_equal @user.encrypted_password, @old_encrypted_password end + end - context "with a valid password" do - context "when verification has expired" do - setup do - travel 16.minutes do - put :update, params: { - password_reset: { password: PasswordHelpers::SECURE_TEST_PASSWORD } - } - end - end + context "with valid password with reset_api_key" do + should "change password and reset api_key" do + get edit_password_path, params: { token: @user.confirmation_token } + put password_path, params: { + password_reset: { reset_api_key: "true", password: PasswordHelpers::SECURE_TEST_PASSWORD } + } - should set_flash[:alert] - should redirect_to("the sign in page") { sign_in_path } + assert_redirected_to sign_in_path + assert_equal "Your password has been changed.", flash[:notice] - should "not sign the user out" do - assert_predicate @controller.request.env[:clearance], :signed_in? - end - end + @user.reload - context "without reset_api_key" do - setup do - put :update, params: { - password_reset: { password: PasswordHelpers::SECURE_TEST_PASSWORD } - } - end + refute_equal @user.api_key, @api_key + refute_equal @user.encrypted_password, @old_encrypted_password - should redirect_to("the dashboard") { dashboard_path } + refute_predicate @new_api_key.reload, :destroyed? + refute_empty @user.api_keys + end + end - should "not change api_key" do - assert_equal(@user.reload.api_key, @api_key) - end - should "change password" do - refute_equal(@user.reload.encrypted_password, @old_encrypted_password) - end - end + context "with valid password with reset_api_key and reset_api_keys" do + should "change password, reset legacy api_key, and expire all api_keys" do + get edit_password_path, params: { token: @user.confirmation_token } + put password_path, params: { + password_reset: { reset_api_key: "true", reset_api_keys: "true", password: PasswordHelpers::SECURE_TEST_PASSWORD } + } - context "with reset_api_key false" do - setup do - put :update, params: { - password_reset: { reset_api_key: "false", password: PasswordHelpers::SECURE_TEST_PASSWORD } - } - end + assert_redirected_to sign_in_path + # assert_equal "Your password has been changed.", flash[:notice] - should redirect_to("the dashboard") { dashboard_path } + @user.reload - should "not change api_key" do - assert_equal(@user.reload.api_key, @api_key) - end - should "change password" do - refute_equal(@user.reload.encrypted_password, @old_encrypted_password) - end - end - - context "with reset_api_key" do - setup do - put :update, params: { - password_reset: { reset_api_key: "true", password: PasswordHelpers::SECURE_TEST_PASSWORD } - } - end + refute_equal @user.api_key, @api_key + refute_equal @user.encrypted_password, @old_encrypted_password + assert_empty @user.api_keys.unexpired + refute_empty @user.api_keys.expired + end + end + end - should redirect_to("the dashboard") { dashboard_path } + private - should "change api_key" do - refute_equal(@user.reload.api_key, @api_key) - end - should "change password" do - refute_equal(@user.reload.encrypted_password, @old_encrypted_password) - end - should "not delete new api key" do - refute_predicate @new_api_key.reload, :destroyed? - refute_empty @user.reload.api_keys - end - end + def webauthn_result(challenge = nil) + challenge ||= session["webauthn_authentication"]["challenge"] + WebauthnHelpers.create_credential(webauthn_credential: @webauthn_credential, client: @client) + WebauthnHelpers.get_result(client: @client, challenge:) + end - context "with reset_api_key and reset_api_keys" do - setup do - put :update, params: { - password_reset: { reset_api_key: "true", reset_api_keys: "true", password: PasswordHelpers::SECURE_TEST_PASSWORD } - } - end + def assert_otp_form + assert_select "h1", "Multi-factor authentication" + assert_select "form[action=?]", otp_edit_password_url(token: @user.confirmation_token) do + assert_select "input[type=text][autocomplete=one-time-code]" + assert_select "input[type=submit][value=?]", I18n.t("authenticate") + end + end - should redirect_to("the dashboard") { dashboard_path } + def assert_webauthn_form + assert_select "h1", "Multi-factor authentication" + assert_select "p", "Authenticate with a security device such as Touch ID, YubiKey, etc." + assert_select "form.js-webauthn-session--form[action=?]", webauthn_edit_password_url(token: @user.confirmation_token) do + assert_select "input[type=submit][value=?]", I18n.t("multifactor_auths.prompt.sign_in_with_webauthn_credential") + end + end - should "change api_key" do - refute_equal(@user.reload.api_key, @api_key) - end - should "change password" do - refute_equal(@user.reload.encrypted_password, @old_encrypted_password) - end - should "expire new api key" do - assert_empty @user.reload.api_keys.unexpired - refute_empty @user.reload.api_keys.expired - end - end - end + def assert_new_password_form + assert_select "h1", I18n.t("passwords.edit.title") + assert_select "form[action=?]", password_path do + assert_select "input[type=password][autocomplete=new-password][name=?]", "password_reset[password]" + assert_select "input[type=checkbox][name=?]", "password_reset[reset_api_key]" + assert_select "input[type=checkbox][name=?]", "password_reset[reset_api_keys]" + assert_select "input[type=submit][value=?]", I18n.t("passwords.edit.submit") end end end diff --git a/test/mailers/password_mailer_test.rb b/test/mailers/password_mailer_test.rb index e923dd6c878..8ce1311b4e8 100644 --- a/test/mailers/password_mailer_test.rb +++ b/test/mailers/password_mailer_test.rb @@ -3,6 +3,7 @@ class PasswordMailerTest < ActionMailer::TestCase test "change password with handle" do user = create(:user) + user.forgot_password! email = PasswordMailer.change_password(user) assert_emails 1 do @@ -16,6 +17,7 @@ class PasswordMailerTest < ActionMailer::TestCase test "change password without handle should show email" do user = create(:user, handle: nil) + user.forgot_password! email = PasswordMailer.change_password(user) assert_emails 1 do diff --git a/test/system/avo/users_test.rb b/test/system/avo/users_test.rb index 97d4c1c4dab..be45809738c 100644 --- a/test/system/avo/users_test.rb +++ b/test/system/avo/users_test.rb @@ -130,7 +130,7 @@ class Avo::UsersSystemTest < ApplicationSystemTestCase "changes" => { "email" => [user_attributes[:email], user.email], "updated_at" => [user_attributes[:updated_at].as_json, user.updated_at.as_json], - "confirmation_token" => [user_attributes[:confirmation_token], nil], + "token_expires_at" => [user_attributes[:token_expires_at].as_json, user.token_expires_at.as_json], "mfa_level" => %w[ui_and_api disabled], "totp_seed" => [user_attributes[:totp_seed], nil], "mfa_hashed_recovery_codes" => [user_attributes[:mfa_hashed_recovery_codes], []], @@ -143,11 +143,11 @@ class Avo::UsersSystemTest < ApplicationSystemTestCase .except( "api_key", "blocked_email", - "confirmation_token", "email", "encrypted_password", "mfa_level", "mfa_hashed_recovery_codes", + "token_expires_at", "totp_seed", "remember_token", "updated_at" @@ -433,26 +433,26 @@ class Avo::UsersSystemTest < ApplicationSystemTestCase }, "gid://gemcutter/User/#{user.id}" => { "changes" => { + "api_key" => ["secret123", nil], + "blocked_email" => [nil, user_attributes[:email]], "email" => [user_attributes[:email], user.email], - "updated_at" => [user_attributes[:updated_at].as_json, user.updated_at.as_json], - "confirmation_token" => [user_attributes[:confirmation_token], nil], - "mfa_level" => %w[ui_and_api disabled], - "totp_seed" => [user_attributes[:totp_seed], nil], - "mfa_hashed_recovery_codes" => [user_attributes[:mfa_hashed_recovery_codes], []], "encrypted_password" => [user_attributes[:encrypted_password], user.encrypted_password], - "api_key" => ["secret123", nil], + "mfa_hashed_recovery_codes" => [user_attributes[:mfa_hashed_recovery_codes], []], + "mfa_level" => %w[ui_and_api disabled], "remember_token" => [user_attributes[:remember_token], nil], - "blocked_email" => [nil, user_attributes[:email]] + "token_expires_at" => [user_attributes[:token_expires_at].as_json, user.token_expires_at.as_json], + "totp_seed" => [user_attributes[:totp_seed], nil], + "updated_at" => [user_attributes[:updated_at].as_json, user.updated_at.as_json] }, "unchanged" => user.attributes .except( "api_key", "blocked_email", - "confirmation_token", "email", "encrypted_password", "mfa_level", "mfa_hashed_recovery_codes", + "token_expires_at", "totp_seed", "remember_token", "updated_at" diff --git a/test/test_helper.rb b/test/test_helper.rb index a1da1d4b86f..2a6b3ae5f9a 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -269,7 +269,30 @@ def refute_selector(selector) class ActionDispatch::IntegrationTest include OauthHelpers + setup { host! Gemcutter::HOST } + + def assert_signed_in_as(user) + flunk "Expected to be signed in as User #{user.handle.inspect}, but was not signed in." unless request.env[:clearance].signed_in? + if request.env[:clearance].current_user != user + current_user = request.env[:clearance].current_user + + flunk "Expected to be signed in as User: #{user.handle.inspect}\n" \ + "Actually signed in as User: #{current_user.handle.inspect}" + end + + assert_equal user, request.env[:clearance].current_user + end + + def refute_signed_in + if request.env[:clearance].signed_in? + current_user = request.env[:clearance].current_user + + flunk "Expected not to be signed in, but was signed in as User #{current_user.handle.inspect}" + end + + assert_nil request.env[:clearance].current_user + end end Gemcutter::Application.load_tasks