jdtornow / challah

User authorization and session management in Rails.
MIT License
18 stars 7 forks source link

Not requiring a password for inactive users. #33

Open philtr opened 7 years ago

philtr commented 7 years ago

I had a requirement to be able to create and manage inactive users, but not set a password until they activated their accounts. This is what I ended up doing. I don't know if you have any interest in something like this, but here is where my implementation shook out.

I don't really have the time right now to dig into this to add it as a pull request, so feel free to take it or leave it. Just food for thought 😊

config/initializers/challah.rb ```ruby Challah.options[:skip_user_validations] = true ```
app/models/user.rb ```ruby class User < ActiveRecord::Base include Challah::Userable # I have here a custom email validation but could just as easily use Challah's validates :email, presence: true, uniqueness: true, format: { with: /@/ } with_options if: :active? do |active| active.validates :first_name, presence: true active.validates :last_name, presence: true # This is my forked version of Challah::PasswordValidator active.validates_with ::PasswordValidator end end ```
app/validators/password_validator.rb ```ruby class PasswordValidator < ActiveModel::Validator def validate(record) if record.password_changed? if record.password.to_s.size < 4 record.errors.add :password, :invalid_password elsif record.password.to_s != record.password_confirmation.to_s record.errors.add :password, :no_match_password end elsif ((record.status_changed? && record.active?) || record.new_record?) # This line right here is kind of icky, since we're hitting the authorizations table. if record.password.to_s.blank? && Authorization.where(user_id: record.id, provider: "password").none? record.errors.add :password, :blank end end end end ```

The main thing I don't like about what I went with is the Authorization lookup in the validator. It could probably be tracked by a separate activated_at column or something.

Curious what your thoughts are on this? Do you think it might be better to use SecureRandom.urlsafe_base64 for a password instead?

jdtornow commented 7 years ago

Cool idea @philtr! Thanks for the suggestion.

I'm still thinking on this to figure out a great approach. Did you consider adding a new status enum option on User? Maybe pending or something? Then we could have the different validations for pending users only or something.

I don't necessarily object to looking up the Authorization in the validator, but I agree it doesn't feel totally correct.

Still thinking, just letting you know I saw this and think it's worth doing :)

philtr commented 7 years ago

Yeah I think that would work nicely!