Log if password matches current for password reset event (#11423)

* Log password changed

* Document analytics property

* Add spec coverage for password_different

* changelog: Internal, Analytics, Add logging property for unchanged password reset of verified account

* Typo: Profile deactivated

* Invert password_different to password_matches_existing

Co-authored-by: Zach Margolis <[email protected]>

* Refactor form specs for updated expectations

* Add form test case for password_matches_existing

* A/B test subset of password resets for logging

* Remove inaccurate route verb

Co-authored-by: Zach Margolis <[email protected]>

---------

Co-authored-by: Zach Margolis <[email protected]>

app/controllers/users/reset_passwords_controller.rb

@@ -3,6 +3,8 @@
 module Users
   class ResetPasswordsController < Devise::PasswordsController
     include AuthorizationCountConcern
+    include AbTestingConcern
+
     before_action :store_sp_metadata_in_session, only: [:edit]
     before_action :store_token_in_session, only: [:edit]
 
@@ -40,10 +42,15 @@ def edit
       end
     end
 
-    # PUT /resource/password
     def update
       self.resource = user_matching_token(user_params[:reset_password_token])
-      @reset_password_form = ResetPasswordForm.new(user: resource)
+      @reset_password_form = ResetPasswordForm.new(
+        user: resource,
+        log_password_matches_existing: ab_test_bucket(
+          :LOG_PASSWORD_RESET_MATCHES_EXISTING,
+          user: resource,
+        ) == :log,
+      )
 
       result = @reset_password_form.submit(user_params)
 

app/forms/reset_password_form.rb

@@ -5,11 +5,14 @@ class ResetPasswordForm
   include FormPasswordValidator
 
   attr_accessor :reset_password_token
+  private attr_reader :log_password_matches_existing
+  alias_method :log_password_matches_existing?, :log_password_matches_existing
 
   validate :valid_token
 
-  def initialize(user:)
+  def initialize(user:, log_password_matches_existing: false)
     @user = user
+    @log_password_matches_existing = log_password_matches_existing
     @reset_password_token = @user.reset_password_token
     @validate_confirmation = true
     @active_profile = user.active_profile
@@ -47,6 +50,8 @@ def handle_valid_password
   end
 
   def update_user
+    @password_matches_existing = user.valid_password?(password) if log_password_matches_existing?
+
     attributes = { password: password }
 
     ActiveRecord::Base.transaction do
@@ -87,6 +92,7 @@ def extra_analytics_attributes
     {
       user_id: user.uuid,
       profile_deactivated: active_profile.present?,
+      password_matches_existing: @password_matches_existing,
       pending_profile_invalidated: pending_profile.present?,
       pending_profile_pending_reasons: (pending_profile&.pending_reasons || [])&.join(','),
     }

app/services/analytics_events.rb

@@ -5713,13 +5713,15 @@ def password_reset_email(
   # @param [String] pending_profile_pending_reasons Comma-separated list of the pending states
   # associated with the associated profile.
   # @param [Hash] error_details Details for errors that occurred in unsuccessful submission
+  # @param [Boolean] password_matches_existing Whether the password is same as the user's current
   # The user changed the password for their account via the password reset flow
   def password_reset_password(
     success:,
     errors:,
     profile_deactivated:,
     pending_profile_invalidated:,
     pending_profile_pending_reasons:,
+    password_matches_existing:,
     error_details: {},
     **extra
   )
@@ -5731,6 +5733,7 @@ def password_reset_password(
       profile_deactivated:,
       pending_profile_invalidated:,
       pending_profile_pending_reasons:,
+      password_matches_existing:,
       **extra,
     )
   end

config/application.yml.default

@@ -226,6 +226,7 @@ lexisnexis_trueid_username: test_username
 lexisnexis_username: test_username
 ###################################################################
 lockout_period_in_minutes: 10
+log_password_reset_matches_existing_ab_test_percent: 0
 log_to_stdout: false
 login_otp_confirmation_max_attempts: 10
 logins_per_email_and_ip_bantime: 60

config/initializers/ab_tests.rb

@@ -81,4 +81,10 @@ def self.all
       SecureRandom.alphanumeric(8)
     end
   end.freeze
+
+  LOG_PASSWORD_RESET_MATCHES_EXISTING = AbTest.new(
+    experiment_name: 'Log password_matches_existing event property on password reset',
+    should_log: ['Password Reset: Password Submitted'].to_set,
+    buckets: { log: IdentityConfig.store.log_password_reset_matches_existing_ab_test_percent },
+  ).freeze
 end

lib/identity_config.rb

@@ -235,6 +235,7 @@ def self.store
     config.add(:lexisnexis_trueid_username, type: :string)
     config.add(:lexisnexis_username, type: :string)
     config.add(:lockout_period_in_minutes, type: :integer)
+    config.add(:log_password_reset_matches_existing_ab_test_percent, type: :integer)
     config.add(:log_to_stdout, type: :boolean)
     config.add(:login_otp_confirmation_max_attempts, type: :integer)
     config.add(:logins_per_email_and_ip_bantime, type: :integer)

spec/controllers/users/reset_passwords_controller_spec.rb

@@ -1,6 +1,8 @@
 require 'rails_helper'
 
 RSpec.describe Users::ResetPasswordsController, devise: true do
+  include AbTestsHelper
+
   let(:password_error_message) do
     t('errors.attributes.password.too_short.other', count: Devise.password_length.first)
   end
@@ -402,6 +404,58 @@
         expect(user.active_profile.present?).to eq false
         expect(response).to redirect_to new_user_session_path
       end
+
+      context 'proofed user submits same password as current' do
+        let(:user) { create(:user, :proofed) }
+        let(:password) { user.password }
+
+        it 'logs event indicating profile deactivated while password the same' do
+          stub_analytics
+
+          reset_password_token = user.set_reset_password_token
+
+          put :update, params: {
+            reset_password_form: {
+              password:,
+              password_confirmation: password,
+              reset_password_token:,
+            },
+          }
+
+          expect(@analytics).to have_logged_event(
+            'Password Reset: Password Submitted',
+            hash_not_including(password_matches_existing: be_in([true, false])),
+          )
+        end
+
+        context 'when in ab test for logging password matches existing' do
+          before do
+            allow(controller).to receive(:ab_test_bucket).with(
+              :LOG_PASSWORD_RESET_MATCHES_EXISTING,
+              user:,
+            ).and_return(:log)
+          end
+
+          it 'logs event indicating profile deactivated while password the same' do
+            stub_analytics
+
+            reset_password_token = user.set_reset_password_token
+
+            put :update, params: {
+              reset_password_form: {
+                password:,
+                password_confirmation: password,
+                reset_password_token:,
+              },
+            }
+
+            expect(@analytics).to have_logged_event(
+              'Password Reset: Password Submitted',
+              hash_including(profile_deactivated: true, password_matches_existing: true),
+            )
+          end
+        end
+      end
     end
 
     context 'unconfirmed user submits valid new password' do