Navigation Menu

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

when password is reset from UI, all tokens must be removed if remove_tokens_after_password_reset is true #634

Closed
rcanand opened this issue May 13, 2016 · 1 comment

Comments

@rcanand
Copy link

rcanand commented May 13, 2016

I have an app where remove_tokens_after_password_reset is true. I discover that my password has been compromised, so I go from the web ui, and change the password for a user. The last password token of the user still remains. This means the hacker who hacked my password can still access my app using the stale token.

The problem seems to be in the below code from https://github.com/lynndylanhurley/devise_token_auth/blob/15bf7857eca2d33602c7a9cb9d08db8a160f8ab8/app/models/devise_token_auth/concerns/user.rb

def remove_tokens_after_password_reset
    there_is_more_than_one_token = self.tokens && self.tokens.keys.length > 1
    should_remove_old_tokens = DeviseTokenAuth.remove_tokens_after_password_reset &&
                               encrypted_password_changed? && there_is_more_than_one_token

    if should_remove_old_tokens
      latest_token = self.tokens.max_by { |cid, v| v[:expiry] || v["expiry"] }
      self.tokens = { latest_token.first => latest_token.last }
    end
  end

I believe the tokens should be cleared including the last token, in this scenario. i.e, something like:

def remove_tokens_after_password_reset
    should_remove_old_tokens = DeviseTokenAuth.remove_tokens_after_password_reset &&
                               encrypted_password_changed? && self.tokens.any?

    if should_remove_old_tokens
      self.tokens = '{}'
      self.save!
    end
  end

Please let me know if I am misunderstanding this setting, and what is the best way to ensure a mobile app cannot use tokens based on old password after password reset.

@booleanbetrayal
Copy link
Collaborator

that last token is required for the user to re-authenticate upon completion of password-reset cycle. (see also: https://github.com/lynndylanhurley/devise_token_auth/blob/master/app/views/devise/mailer/reset_password_instructions.html.erb)

If there are any other tokens that are being generated or aren't being deleted, that's legitimate, and I can re-open.

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

2 participants