Skip to content

Model should set authentication token value before validation - #316

Open
jeremywadsack wants to merge 1 commit into
gonzalo-bulnes:masterfrom
keylimetoolbox:ensure_token_before_validation
Open

Model should set authentication token value before validation#316
jeremywadsack wants to merge 1 commit into
gonzalo-bulnes:masterfrom
keylimetoolbox:ensure_token_before_validation

Conversation

@jeremywadsack

Copy link
Copy Markdown

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
Copy Markdown
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 : )

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

Copy link
Copy Markdown

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