RAILS - Dont understand password reset code

Question

Im having problems understanding the app logic to this password reset code i found below on the web.

1. A user receives a an email with a link with some reset code.
2. After clicking on this they go to the action below called reset
3. The user is found in the db by referencing the reset code.
4. The form to change password is shown and the user enters the new password in.

Heres where i get confused.

1. When the form is submitted the action is called again.
2. This time there will be no reset code in the params so no user will be found @user = nil
3. This time its a post request so we enter that part of the logic.

My question is - How can this code ever be valid if @user = nil if @user.update_attributes(:password => params[:user][:password], :password_confirmation => params[:user][:password_confirmation])

# app/controllers/users_controller.rb

def reset
  @user = User.find_by_reset_code(params[:reset_code]) unless params[:reset_code].nil?
  if request.post?
    if @user.update_attributes(:password => params[:user][:password], :password_confirmation => params[:user][:password_confirmation])
      self.current_user = @user
      @user.delete_reset_code
      flash[:notice] = "Password reset successfully for #{@user.email}"
      redirect_to root_url
    else
      render :action => :reset
    end
  end
end

---

# app/views/users/reset.html.erb

<%= error_messages_for :user %>

<% form_for :user do |f| -%>

Pick a new password for <%= @user.email %>

Password
<%= f.password_field :password %>

Confirm Password
<%= f.password_field :password_confirmation %>

<%= submit_tag 'Reset' %>

<% end -%>

Answer 1

How can this code ever be valid if @user = nil

The reset code, which isn't the view you pasted by the way, looks for the user with the same reset_code... if it doesn't find one, then user is nil. if the reset code exists, it won't be nil, but true and return the record instead.

Answer 2

I think you misunderstood the flow of logic. The view you provided is for requesting a password reset email. The email will likely contain a url for the reset, which will include the reset_code param. Check the reset.html view to make sure the reset_code param is handled there.

Answer 3

When the form is submitted the action is called again.

The request.post? if statement checks to see if they are submitting a form. The stuff inside it are only executed if the form submits a POST request, if not, it will render the page like you see.

This time there will be no reset code in the params so no user will be found @user = nil

The page isn't rendered so not setting the user to anything will be ok, it is redirecting back to the main url which will have the user based on the session or cookie, not the reset code.

This time its a post request so we enter that part of the logic.

Although I can't figure out how @user.update_attributes is working because like you said there is that unless statement at the end.

Answer 4

My answer would be that it is buggy code.

Firstly, the reset_code should be passed back to the controller via a hidden field on the form, or a parameter in the URL.

Secondly, if no reset_code is passed in (as shown), @user will be nil and you will get an AV. You'll need to add a guard clause around the code:

if !@user
  flash.now[:notice] = "User not found."
elsif request.post?
  ...

Also, if it's not included in the code already you need some way of expiring reset codes. On my system they expire after five days. You would need to add an extra field to your users table. Store the date the code is sent in there and include another guard clause that this is within the given time frame.

(Remember, just because you found it on the 'net doesn't mean it's correct and bug free).