[Owncloud] Review of pull request#26(improved persistent cookies) - second pair of eyes required

[Andre Gemuend]

Hi Michael,

I like this approach better. Two nitpicks:

- Typo in comment on line 585: "multible"
- the unsetMagicInCookie on line 560 and 562 seem redundant. The one on 
560 can go imho.

There are two things that I still need to think about, maybe we can 
discuss this together:

1. Why do we hash the random number? Doesn't this just add the 
possibility of collisions? I'm unsure and don't have much time to think 
about it right now. Also, why do we use 10 bytes for the random part?

2. Do we maybe want to present something to the user in case the cookie 
is rejected? One possibility would be a page with a warning and the 
users last login time, so he can check that.
Another would be a page telling the user what happened, and "In case you 
did not change your password recently" asking the user for the password 
to invalidate all active tokens.

Admittedly, I might be a bit paranoid here, but I think this is important.

Greetings
André


On 10/11/2012 02:44 PM, Michael Göhler wrote:
> Hi together,
>
> don't be surprised, im currently sick and at home ;)
>
> I took Bart's approach and added everything I suggested in #26. Can you
> please review again before I create the merge request?
>
> https://github.com/visit1985/core/compare/persistentcookies
>
> - I've removed the backwards compatibility to the old token entries in
>    preferences table, because this would result in to many extra code
>    when checking/deleting them.
>
> - We have to add a password box to the email address field in personal
>    settings, because an attacker kann reset the password if we don't!
>    I'll have a look on that too, but in another merge request.
>
> Thanks,
> Michael