#70 ✓resolved
deleted

Blank password/confirmation doesn`t raise an error

Reported by deleted | March 31st, 2009 @ 06:25 AM

password_resets_controller.rb:

def update @user.password = params[:user][:password] @user.password_confirmation = params[:user][:password_confirmation]

if @user.save

flash[:notice] = "Password successfully updated"

redirect_to(root_path)

else

 render :action => :edit

end end

Updating password with empty password and confirmation doesnt rise an error.

Comments and changes to this ticket

  • Ben Johnson

    Ben Johnson March 31st, 2009 @ 01:37 PM

    • Tag changed from errors, password, reset to errors, password, reset
    • State changed from “new” to “resolved”

    Right, think about updating profiles where that field is optional. So this is correct behavior.

  • Si

    Si April 6th, 2009 @ 01:26 PM

    • Title changed from “Blank password/confirmation doesn`t rise an error” to “Blank password/confirmation doesn`t raise an error”

    Whilst I definitely see the use of optional password updates, I think this goes against the principle of least surprise.

    As of right now, Authlogic refuses to accept blank passwords, and so unfortunately the work of the model validations has to be repeated outside of Authlogic. e.g.

    @user.errors.add_on_blank(:password) if @user.errors.empty? && @user.update_attributes(params[:user]) ...

    This might work, but it's "double validation", requiring duplication of validation rules, and possibly two submits - one for blank password, then a new bunch of errors that were previously screened.

    I guess one could indulge in some ugly "def @user.blank?; false; end" kind of hacking, to fool Authlogic, but whatever happens you leak implementation knowledge up into the controller.

    I see from the forum that there used to be an option, similar sort of idea to Rails' password_will_change!, which allowed unrestricted password setting, but it appears to have gone now.

    However, if the default was to process blank passwords too, to get the opposite we could do the following:

    params[:user].delete(:password) if params[:user][:password].blank?

    Imo this signals an application intent to disregard blank passwords for that update action, rather than duplication of validation rules - explicitness that clarifies the code.

  • Ben Johnson

    Ben Johnson April 6th, 2009 @ 01:55 PM

    I think an option to toggle this would be sufficient. What do you think? I dont want to change the default behavior because people's applications would break. But I can provide an option to change it.

  • Si

    Si April 6th, 2009 @ 02:54 PM

    Yes please! For me it would be great if I could change my own app default behaviour for blank passwords, via the acts_as_authentic class config method.

    Changing behaviour prior to an individual update would be gravy, but I can't see myself using it if the default was configurable. I guess most apps would consistently go one route or the other.

  • Ben Johnson

    Ben Johnson April 6th, 2009 @ 04:19 PM

    That has been added, update from the repo.

  • Si

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile ยป

Object based authentication solution that handles all of the non sense for you. It's as easy as ActiveRecord is with a database.

People watching this ticket

Pages