OnlySteveH
OnlySteveH

Reputation: 152

Password validation fails in two opposed scenarios

I am working through the Ruby on Rails Tutorial by Michael Hartl and have generated an interesting dilemma. I will have done something wrong, so I need your help finding the issue.

The issue surrounds the validation of a password property within a User model. The initial validation of this property was:

validates :password,  presence: true, 
                        confirmation: true, 
                        length: { minimum: 6 }

This requires a minimum length of the password and is designed to satisfy the situation where a new user creates their instance.

I have created the following tests (and I wish I had used Rspec!) guided by the book. These tests check that the validations work:

test "password must not be blank or made up of spaces" do
  @user.password = @user.password_confirmation = " "
  assert_not @user.valid?
end

test "password must not be empty/nil" do
  @user.password = @user.password_confirmation = ""
  assert_not @user.valid?
end

So, we’re checking that the password field cannot contain either a space, or a nil entry. With the current validations, these tests pass. All is well.

I have progressed to allowing a user to edit their profile. This enables the user to change their name, email address and password/confirmation if they choose. In order to allow a user not to change their password if they don’t want to, additional validation is added to the password property of the model, adding allow_blank: true such as:

validates :password,  presence: true, 
                      confirmation: true, 
                      length: { minimum: 6 }, 
                      allow_blank: true # added this!

So, the user can now leave the two password fields blank when they edit their profile if they don’t want to change their profile. This satisfies the test:

test "successful edit" do
  log_in_as @user
  get edit_user_path(@user)
  assert_template 'users/edit'
  name = "Foo Bar"
  email = "[email protected]"
  patch user_path(@user), params: { user: { name: name,
                                            email: email,
                                            password: "",
                                            password_confirmation: "" } }
  assert_not flash.empty?
  assert_redirected_to @user
  @user.reload
  assert_equal @user.name, name
  assert_equal @user.email, email
end

This enables a user to edit just their name & email and, by leaving their two password fields blank, there’s no need to change, or re-enter, their password. This throws a FAIL on a long passing test, as above, such as:

test "password must not be blank or made up of spaces" do
  @user.password = @user.password_confirmation = " "
  assert_not @user.valid?
end

The test fails because the user is validated. The slightly different test, which tests for nil, not blank, passes:

test "password must not be empty/nil" do
  @user.password = @user.password_confirmation = ""
  assert_not @user.valid?
end

So a password of “” is caught but a password of “ “ works fine for creating a new user or editing an existing user.

Adding allow_blank: true to the user model validation of password seems to have caused this. So, I am stuck between two tests failing. If I omit allow_blank: true, this test fails (full test pasted above):

test "successful edit" do
.
.
  patch user_path(@user), params: { user: 
                                    { name: name,
                                      email: email,
                                      password: "",
                                      password_confirmation: "" } }
.
  assert_equal @user.name, name
  assert_equal @user.email, email
end

Sending the blank password and password_confirmation fails the test as it isn’t allowed to be blank.

Adding allow_blank: true within the validation fails this test:

test "password must not be blank or made up of spaces" do
  @user.password = @user.password_confirmation = " "
  assert_not @user.valid?
end

This fail allows a user to be created with a password consisting of spaces. A nil password, i.e. no characters at all, is not allowed. That test works.

This leaves me in the position where I must decide between a user having to change/repeat their two password fields if they edit their profile, OR, allowing a scenario where a user can sign up with a password consisting of one space, or many spaces, as this test doesn’t throw the expected failure message:

test "password must not be blank or made up of spaces" do
   @user.password = @user.password_confirmation = " "
   assert_not @user.valid?
end  

The addition of allow_blank: true bypasses this test or the validation generally. A password of any number of spaces is accepted which is against the validation in the model. How is that possible?

Any thoughts how to test better (apart from using Rspec!). I bow to your greater knowledge.

TIA.

[EDIT]

The suggested changes in the comments below made my test suite green. This was due to the suite being inadequate. To test the unsuccessful integration, the suggested code tested multiple scenarios in one go, such as:

test "unsuccessful edit with multiple errors" do
  log_in_as @user
  get edit_user_path(@user)
  assert_template 'users/edit'
  patch user_path(@user), params: { user: 
                                    { name: "",
                                      email: "foo@invalid",
                                      password: "foo",
                                      password_confirmation: "bar" } }
  assert_template 'users/edit'
  assert_select 'div.alert', "The form contains 3 errors."
end

The key part here is getting the number of expected errors correct so that assert_select gives the right result. I didn't. The errors should be blank name, invalid email format, password too short, pwd & confirmation don't match. The error for short password isn't showing.

I decided to pull out two more tests to demonstrate the failure of the validations of password length and presence. The point of allow_blank is to allow the password & confirmation fields to have nothing in them when editing the user profile so it isn't compulsory to enter the password every time the user profile is edited. These tests are:

test "unsuccessful edit with short password" do
  log_in_as @user
  get edit_user_path(@user)
  assert_template 'users/edit'
  patch user_path(@user), params: { user: 
                                    { name: @user.name,
                                      email: "[email protected]",
                                      password: "foo",
                                      password_confirmation: "foo" } }
  assert_select 'div.alert', "The form contains 1 error."
end

test "unsuccessful edit with blank (spaces) password" do
  log_in_as @user
  get edit_user_path(@user)
  assert_template 'users/edit'
  patch user_path(@user), params: { user: 
                                    { name: @user.name,
                                      email: "[email protected]",
                                      password: " ",
                                      password_confirmation: " " } }
  assert_select 'div.alert', "The form contains 1 error."
end

If the password is changed, then the validation rules should apply, i.e. the password should not be blank and must have a minimum length. That's not what's happening here either in the code the Tutorial book suggests or the amended code using on: :create and on: :edit.

Upvotes: 1

Views: 374

Answers (1)

OnlySteveH
OnlySteveH

Reputation: 152

I figured this out so am posting here in case others come across a similar problem.

I amended the validations to include the :update action on the User, rather than just :edit. This covered the action of saving to the database and caught the short password update validations but still allowed passwords made of spaces.

A bit of checking the documentation showed me that using allow_blank: true allows nil and strings made up of spaces. The scenario here wants a nil password to be acceptable, but not a blank one. The alternative validation of allow_nil: true is more appropriate to the scenario here.

The updated code from above looks like, in User.rb:

validates :password,  presence: true,
                      length: { minimum: 6 }, 
                      allow_nil: true, 
                      on: [:edit, :update]

validates :password,  presence: true, 
                      confirmation: true, 
                      length: { minimum: 6 }, 
                      on: :create

The extended test suite is now all green.

Upvotes: 1

Related Questions