Skip to content

[has_secure_password] Password confirmation validation skipped if password is whitespace-only #55225

@callmesangio

Description

@callmesangio

Hi all,

this is actually a follow-up to #35444, which was opened many years ago but unfortunately got stale and then closed.

The password confirmation validation added by has_secure_password is skipped if the password string is a sequence of whitespace characters, regardless of the string provided as confirmation, which is ignored with no error messages. This happens both in the latest Rails (currently 8.0.2) and in main.

As far as I can tell Rails is committed to support whitespace-only passwords (#25373, #16412), and since they trigger actual changes to the password digest I think it would make sense to align the validation logic to this use case (IMHO it would just make a better default).

I've already taken a quick look at the code; I wasn't able to find any tests documenting the current behavior is intended. But I did find a possible fix, so I volunteer to open a PR should the core team approve the change.

Steps to reproduce

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"
  gem "rails", github: "rails/rails", branch: "main"
  gem "sqlite3"
  gem "bcrypt", "~> 3.1.7"
end

require "active_record/railtie"
require "minitest/autorun"

ENV["DATABASE_URL"] = "sqlite3::memory:"

class TestApp < Rails::Application
  config.load_defaults Rails::VERSION::STRING.to_f
  config.eager_load = false
  config.logger = Logger.new($stdout)
  config.secret_key_base = "secret_key_base"

  config.active_record.encryption.primary_key = "primary_key"
  config.active_record.encryption.deterministic_key = "deterministic_key"
  config.active_record.encryption.key_derivation_salt = "key_derivation_salt"
end
Rails.application.initialize!

ActiveRecord::Schema.define do
  create_table :users, force: true do |t|
    t.string :password_digest, null: false
  end
end

class User < ActiveRecord::Base
  has_secure_password
end

class BugTest < ActiveSupport::TestCase
  test "create new user with whitespace password and wrong password confirmation" do
    user = User.new(password: " ", password_confirmation: "anything")

    assert_not user.valid?(:create), "user should not be valid"
  end

  test "update existing user with whitespace password and wrong password confirmation" do
    user = User.create!(password: "password", password_confirmation: "password")

    user.password = " "
    user.password_confirmation = "anything"

    assert_not user.valid?(:update), "user should not be valid"
  end
end

Expected behavior

The validation should fail if the password and the password confirmation strings are different, no matter if the password is a sequence of whitespace-only characters.

Actual behavior

If the password is a sequence of whitespace-only characters, the validation succeeds regardless of the contents of the confirmation string.

System configuration

Rails version: 8.0.2, main@1ac4003547

Ruby version: 3.4.4

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions