devise
devise copied to clipboard
Validation crashes for password-less authentication
Environment
- Ruby 2.7.1p83 (2020-03-31 revision a0c7c23c9c) [x86_64-darwin19]
- Rails 6.1.1
- Devise 4.7.3
Current behavior
Context: On my project we implemented passwordless authentication - magic link gets sent to user email address to log in. We wanted to do away with passwords entirely, so I got to removing any mention of passwords from our user model.
A User model defined by the following:
class User < ApplicationRecord
devise : validatable, :registerable, :trackable, :confirmable, :passwordless_authenticatable
def password_required?
false
end
end
With the following table from schema:
create_table "users", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t|
t.string "full_name", null: false
t.string "email", default: "", null: false
t.string "login_token"
t.datetime "login_token_valid_until"
t.datetime "remember_created_at"
t.datetime "last_sign_in_at"
t.datetime "current_sign_in_at"
t.inet "current_sign_in_ip"
t.inet "last_sign_in_ip"
t.integer "sign_in_count", default: 0, null: false
t.string "confirmation_token"
t.datetime "confirmed_at"
t.datetime "confirmation_sent_at"
t.datetime "created_at", precision: 6, null: false
t.datetime "updated_at", precision: 6, null: false
t.index ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true
t.index ["email"], name: "index_users_on_email", unique: true
end
will cause a crash whenever it attempts validation. As mentioned in https://github.com/heartcombo/devise/issues/4913
That's because there is no password method defined on that User.
Expected behavior
Password validation does not cause crashes when the User model does not require password.
Suggested solution
I believe the crash is caused by the line
validates_length_of :password, within: password_length, allow_blank: true
inside validatable. And I think it could be fixed by changing it to be
validates_length_of :password, within: password_length, allow_blank: true, if: :password_required?
I checked that it fixed my issue locally, but I don't know if it goes against the philosophy of what validatable should do.
FWIW I encountered this same issue with a gem I wrote to add a passwordless auth strategy to Devise: https://github.com/abevoelker/devise-passwordless/blob/bad1cdc4a45be0a85c7f4410e8254b0ef2b90c10/lib/devise/models/magic_link_authenticatable.rb#L12
I actually just remembered to open a ticket to report it, and lo and behold this issue is already open. So I'm +1'ing a fix for this.
Funny enough, this was changed on purpose almost 10 years ago: https://github.com/heartcombo/devise/commit/a59410a254ef3d5ca6fecf2b2f8f3b93b448a179, but it wasn't clearly documented why. 🤷♂️
Perhaps instead of checking :password_required? just checking if the model responds to password instead would be acceptable?
validates_length_of :password, within: password_length, allow_blank: true, if: -> (x) { x.respond_to?(:password) }
:password_required? may have some semantics related to when a :password field exists on the model, but presumably it doesn't make sense to check the length if the field doesn't exist?
@kicferk1 It looks like you approve of abevoelker's solution. Can this ticket be closed?
@timlawrenz I am happy for this ticket to be closed.