gistfile1.txt

In reply to: https://twitter.com/plataformatec/status/525670283899072513

Hi,

This would probably be a long story, and I am not going to go
through all the details here. You're doing well in what
could be done, but in general, I oppose to build a user system
on top of some generic framework. The reason is that a user
system could be very specific to business logic, and it
could be very hard to customize devise in a way that works
well for that.

Honestly this could simply because I am not familiar with how
devise works, but it's really frustrating to track devise code.
For example, now I am getting this message when trying to do:
`sign_in_and_redirect(user)`

    You have to confirm your account before continuing.

[updated: This is now fixed. See comments below. It's totally my own fault]

And I have no idea where this was triggered. The user passed
definitely has `confirmed_at` set, and the user could
successfully login with the form when tried again. It's simply
that calling `sign_in_and_redirect(user)` didn't work.

We're also stuck at 3.0.4 and cannot upgrade right now due to
the flow change from later devise, and I don't yet find a way to
make it work the same way for now. I believe this could be done,
but it's just costing me too much time and effort than
trying to do on my own.

In short, I don't think such user system would work if we're
building a business which would require a bunch of customization,
unless I am so familiar with devise to begin with. (for example,
if I developed devise, then of course I could find a way to do it)
and I think surely devise could have better implemented somehow.
I guess this is simply too ambitious and could hardly do right.
It's improving over time, definitely, but we're already stuck there...

To show a quick implementation improvement:
https://github.com/hassox/warden/blob/v1.2.3/lib/warden/proxy.rb#L212

I believe it's not a good idea to name a local variable the same as
the defining method. Without looking though line-by-line, it could look
like doing some crazy recursion. Which is not.

I saw things like that tracking through warden and devise.
Really a bad experience for me.

My two cents.
Thanks for listening to my complaining.

Also a quick suggestion for:
http://blog.plataformatec.com.br/2013/08/devise-3-1-now-with-more-secure-defaults/
Regarding "Store digested tokens in the database":
Since we're already using bcrypt, I think we could just use bcrypt for
this purpose. Just treat confirmation tokens as password, and of course
remove the tokens after it's used.

Stupid me. Just realized that I was reading devise 3.3.0 instead of 3.0.4.

Finally tracked it down after looking at the right code.
It's because there's no update_columns in rails 3,
and so I was using self.class.where(:id => id).update_all(args),
and then the instance is not reflecting the value from database.
Silly me. It's totally my fault. After putting a quick reload now it works.
Should probably just change the instance without reloading though.

Replaced reload to: args.each{ |k, v| send("#{k}=", v) }