Hobo bug 638

[Bryan Larsen]

One of Barquin's apps is currently getting bitten by bug 638:
https://hobo.lighthouseapp.com/projects/8324/tickets/638-password-validations-are-awkward-to-override

It appears that Tom's intention was that you just redefine
password_validations in your user model, but by then it's too late:
password_validations is called in self.included, before the user's new
definition has been encountered.

I can't think of a great solution for the problem. Possibilities:

1) require password_validations to be defined before the call to
hobo_user_model.

2) allow hobo_user_model to take a block

3) add a function that the user should call at the end of their model:
hobo_user_model_complete, for example.

3's a non-starter because it breaks existing apps.

Of course, most people would probably just vendor Hobo and modify the
file directly, but I'm trying to avoid that. (the dogfood principle).

Any better ideas?

thanks,
Bryan

[kevinpfromnm]

can't you just add the validations normally afterward without having
to redefine password_validations? or is password_validations used
elsewhere?

unless I'm missing something (which is quite possible, my ruby-fu
isn't the best), I'd say 1 would be alright. Assuming you can define
standard rails validations that check the password field, I'd think
this would be fine as the first approach from a new user will likely
be the standard rails method and this hobo approach can be done after
spotting documentation (which can mandate being before
hobo_user_model).

On Nov 30, 2:27 pm, Bryan Larsen <bryan.lar...@gmail.com> wrote:
> One of Barquin's apps is currently getting bitten by bug 638:https://hobo.lighthouseapp.com/projects/8324/tickets/638-password-val...

[Matt Jones]

On Nov 30, 2010, at 5:36 PM, kevinpfromnm wrote:

> can't you just add the validations normally afterward without having
> to redefine password_validations? or is password_validations used
> elsewhere?
>
> unless I'm missing something (which is quite possible, my ruby-fu
> isn't the best), I'd say 1 would be alright. Assuming you can define
> standard rails validations that check the password field, I'd think
> this would be fine as the first approach from a new user will likely
> be the standard rails method and this hobo approach can be done after
> spotting documentation (which can mandate being before
> hobo_user_model).

The biggest issue I could see with that is twofold:

- there's a strong note of "DO NOT put stuff above hobo_*" in the docs, for good reasons

- some validations stuff won't work (passing lifecycle states to :on, for instance) until after the hobo_model call brings them in

The issue with redefining validations later is that it's not possible (without some pretty spirited hackery) to skip/remove an existing validation. Thus, if the plugin says:

validates_length_of :password, :within => 8..40, :if => :new_password_required?

then after adding in the model:

validates_length_of :password, :within => 4..80, :if => :new_password_required?

you'll still get validation errors on passwords that fail the first validation.

On Bryan's other options:

(2) seems very messy, and somewhat confusing since you'd be setting up a block to define stuff in the model - in the model...

And as noted, (3) would cause all sorts of mayhem.

What about just passing a flag to hobo_user_model? For instance:

hobo_user_model :skip_password_validation => true

That seems a little more Rails-flavored, and would allow users to handle password validation themselves.

--Matt Jones

[Bryan Larsen]

Thank you. I knew the group would have a sane option for me.

Bryan

[Domizio Demichelis]

- there's a strong note of "DO NOT put stuff above hobo_*" in the docs, for good reasons

good reasons that apply when you have not to redefine anything, but in this case it is quite different.

My vote to the option 1, which is the standard procedure when you are calling a class method which is not yet defined. Just move it before the call!

ciao
dd

[Domizio Demichelis]

I forgot that we have another better alternative!

Instead of using a validation helper we could use the the
validate :validation_method and that is called only at validation
time, so it is overridable also after the hobo_user_model declaration!

In our case we can change the password_validations call for
validate :password_validation
and define a password_validation instance method instead of a
password_validations class method.

If the user wants to override it it would probably need to finetune
its validation anyway, so an instance method seems even more
appropriate.

ciao
dd

[Bryan Larsen]

Even better. More embarassingly, this method is used for
validate_current_password_when_changing_password on the next line of
user.rb, right after password_validations is called. So having to ask
the list is embarassing, but oh well.

I'm also tempted to change the validation to: "must be at least 6
characters long and must not consist solely of lowercase letters."

I feel this is a more sane default, It's also the type of validation
that has to use a function rather than one of the standard validations.

thoughts? Obviously this change wouldn't be pushed onto 1.0.

Bryan

[Domizio Demichelis]

I'm also tempted to change the validation to:  "must be at least 6 characters long and must not consist solely of lowercase letters."

I feel this is a more sane default,    It's also the type of validation that has to use a function rather than one of the standard validations.

Just 4 letter long is a weak password, so your proposed default seems better to me.

ciao
dd