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

Model should set authentication token value before validation #316

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jeremywadsack
Copy link

I have a simple model:

class User < ApplicationRecord
  acts_as_token_authenticatable

  validates :service, presence: true, uniqueness: {case_sensitive: false}
  validates :authentication_token, presence: true, uniqueness: true
end

Testing this fails because acts_as_token_authenticatable sets the before_save validation which is not run until after validation:

2.3.1 :008 > User.create(service: "alpha")
   (2.1ms)  BEGIN
  User Exists (0.6ms)  SELECT  1 AS one FROM "users" WHERE LOWER("users"."service") = LOWER('alpha') LIMIT 1
  User Exists (0.6ms)  SELECT  1 AS one FROM "users" WHERE "users"."authentication_token" = '' LIMIT 1
   (0.2ms)  ROLLBACK
+----+---------+----------------------+
| id | service | authentication_token |
+----+---------+----------------------+
|    | alpha   |                      |
+----+---------+----------------------+

If the default behavior for simple_token_authentication is to set the authentication_token for any model that doesn't have one, then I think it makes sense to validate that as well.

This pull request resolves that by using before_validation callback which is supported by both ActiveRecord and Mongoid.

@gonzalo-bulnes
Copy link
Owner

Hi @jeremywadsack!

Thank you for your PR! I see no reason not to move to before_validation, I think your argument makes a lot of sense. I'll take some time to plan the release properly (I've a few more things pending) and let you know as soon as I do : )

@@ -41,7 +41,7 @@ def token_generator

module ClassMethods
def acts_as_token_authenticatable(options = {})
before_save :ensure_authentication_token
before_validation :ensure_authentication_token
end

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe you want add following line too

validates :authentication_token, presence: true, uniqueness: true

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

Successfully merging this pull request may close these issues.

3 participants