Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Password reset allows user to bypass confirmable #590

Closed
dchersey opened this issue Mar 29, 2016 · 5 comments
Closed

Password reset allows user to bypass confirmable #590

dchersey opened this issue Mar 29, 2016 · 5 comments

Comments

@dchersey
Copy link

In passwords_controller.rb, the following code:

        # ensure that user is confirmed
        @resource.skip_confirmation! if @resource.devise_modules.include?(:confirmable) && !@resource.confirmed_at

actually sets the confirmed_at property of a resource if the resource has not been confirmed.

This effectively allows the user to bypass email confirmation by resetting their password.

I'd like to remove this line, so that the user IS able to change their password with the reset link but will not be able to log in until they confirm the email.

Thoughts?

@cars10
Copy link

cars10 commented Mar 31, 2016

Why would you? There is no security issure there. If the user can change the password then he can access his email account and that means that he is able to confirm the account.

@dchersey
Copy link
Author

dchersey commented Apr 1, 2016

But the email is passed as a parameter for reset; it could be different
than the one used for userid.

On Thu, Mar 31, 2016 at 4:07 PM Carsten [email protected] wrote:

Why would you? There is no security issure there. If the user can change
the password then he can access his email account and that means that he is
able to confirm the account.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#590 (comment)

@dchersey
Copy link
Author

dchersey commented Apr 4, 2016

Also, the comment seems to communicate the correct intention but the code actually does the opposite.

@midnight-wonderer
Copy link

But in which case the email for password resetting is different than the user id?

@zachfeldman
Copy link
Contributor

Seems like a nonissue from the discussion so far, so I'm closing for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants