nbudin / devise_openid_authenticatable

OpenID authentication for Devise
MIT License
99 stars 32 forks source link

Rejuvenation #28

Closed ecoologic closed 8 years ago

ecoologic commented 8 years ago

I closed it because I'm not sure it works, seems like it resets the token even when it's not needed.

I think the original problem is that in devise 3.5 remember_me is not set according to the params passed from the controller, the rest works fine.

See https://github.com/ecoologic/devise_openid_authenticatable/pull/3

nbudin commented 8 years ago

Hi @ecoologic, sorry for not getting around to reviewing this until now. I think you're right; the solution in this PR is overzealous since it's treating it as if the user has always checked "remember me".

I'm not sure how this is supposed to work in Devise 3.5, but your explanation makes sense to me.

ecoologic commented 8 years ago

@nbudin I think I found a solution.

https://github.com/ecoologic/devise_openid_authenticatable/pull/3/files#diff-7a1dfc26f4671847a5f40fd9c23c0228R68

I will rebase, doc my commit better and send a new PR.

I've also added an issue in Devise asking their opinion, I hope they don't mind: https://github.com/plataformatec/devise/issues/4266

What do you think of this solution?

ecoologic commented 8 years ago

@nbudin rails 5 is also on the way: https://github.com/ecoologic/devise_openid_authenticatable/pull/4/files#diff-7a1dfc26f4671847a5f40fd9c23c0228R69

(will also need rebase)