Controllers use flash for paranoid_mode messages but do not redirect #1375

Closed
jimherz opened this Issue Oct 11, 2011 · 2 comments

Comments

Projects
None yet
2 participants
Contributor

jimherz commented Oct 11, 2011

Hi,

We would like to use paranoid mode, specifically for the passwords controller so that we don't leak existence/non-existence of account information when users are submitting the forgot password page. However, there is a subtle problem with how devise implements paranoid mode. For instance, here is the create action from the passwords controller:

def create
  self.resource = resource_class.send_reset_password_instructions(params[resource_name])

  if successful_and_sane?(resource)
    set_flash_message(:notice, :send_instructions) if is_navigational_format?
    respond_with({}, :location => after_sending_reset_password_instructions_path_for(resource_name))
  else
    respond_with_navigational(resource){ render_with_scope :new }
  end
end

successful_and_sane? adds the :send_paranoid_instructions to flash when in paranoid mode, but it returns false. Hence, the action falls through to the else case and merely re-renders the new template. Since there is no redirect, the flash sticks around for an additional request. And that can be a little messy if the user, for instance, clicks on the link in the forgot password email and it takes them to the session in the same browser...they see the "You will receive an email..." at the top of the password reset page, which is not ideal.

It seems like paranoid mode messages should either use flash.now, or the paranoid mode response should be a redirect. Personally, I think the redirect makes more sense because a paranoid mode response should appear in all ways to the user like a successful response.

Or, perhaps we are not understanding the correct usage of paranoid mode. If so, please explain.

Thanks!

Owner

josevalim commented Oct 12, 2011

Your analysis is correct: we should always redirect. Could you please provide a pull request or a failing test?

Contributor

jimherz commented Oct 12, 2011

Thanks, Jose. I am working on a pull request now.

josevalim closed this in 2a5ad46 Oct 15, 2011

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment