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

Token generation fails when user has incomplete data that is mandatory #938

Closed
heridev opened this issue Aug 9, 2017 · 5 comments
Closed

Comments

@heridev
Copy link

heridev commented Aug 9, 2017

As far as I know all the versions for devise_token_auth use self.save! when a new token is generated within create_new_auth_token, the problem with that occurs when you have custom validation rules, for instance:

User schema:

User(id: integer, email: string, first_name:string, last_name:string)

Validation rule:

validates_presence_of :first_name

So any user created previously(before adding the validation rule) and that did not have first_name will have authentication problems because in every place that you invoke current_user or those sort of helpers that internally calls create_new_auth_token will not create a valid token.

Right now the way we solve this authentication issue for those kinds of users is skipping validation rules with the usage of an attr accessor, as an example:

#user.rb
validates_presence_of :first_name, unless: :skip_validation_name

Within the controller(eg: sessions_controller.rb)

@resource.tokens[@client_id] = {
  token:  BCrypt::Password.create(@token),
  expiry: (Time.now + DeviseTokenAuth.token_lifespan).to_i
}

@resource.skip_validation_name = true
@resource.save!

and the other way is deactivating any validation rules overriding the original method create_new_auth_token(client_id=nil) and using validate: false:

With something like this:

    self.tokens[client_id] = {
      token:      token_hash,
      expiry:     expiry,
      last_token: last_token,
      updated_at: Time.now
    }

    self.save!(validate: false)

    return build_auth_header(token, client_id)

Do you guys know any other approach to solve these sort of issues?

@lynndylanhurley
Copy link
Owner

Can you use the :on option to only apply the validation in the context of your app?

For example, on your model:

validates :first_name, presence: true, on: :update_account

And then in the part of your app that handles the user updates:

user.save(context: :update_account)

This way the validation won't be triggered by auth token updates or anything else unrelated to the user's actions.

@heridev
Copy link
Author

heridev commented Aug 10, 2017

@lynndylanhurley I didn't know about context good to know, thanks for pointing this topic out, the problem I see using a custom context is that if in the future we add a new validation rule and we forgot to specify this special context we will have authentication issues again :(.

@lynndylanhurley
Copy link
Owner

So then are you suggesting that we save the user model using validate: false for token updates?

@heridev
Copy link
Author

heridev commented Aug 10, 2017

@lynndylanhurley yeah why not, do you know what are the implications/disadvantages of doing that?

@zachfeldman
Copy link
Contributor

@heridev well, I think it's kind of bad practice to just not run validations at all on a save action by default as part of a library...

I'm going to close this but reopen if you disagree and have a compelling case or examples to the contrary from other libs.

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

3 participants