plone.session raises "form authenticator is invalid" exception on secret management

[pjnaylor]

With a fresh install of Plone 3.1.1, going to /SiteName/acl_users/session/manage_secret in the ZMI and clicking on the "New secret" button, results in an exception of type "Forbidden", and value "Form Authenticator is invalid". The traceback is:

Module ZPublisher.Publish, line 119, in publish
Module ZPublisher.mapply, line 88, in mapply
Module ZPublisher.Publish, line 42, in call_object
Module <string>, line 4, in _facade
Module plone.protect.utils, line 32, in _curried
Module plone.protect.authenticator, line 60, in check

Clicking on the "Clear secrets" button results in the same problem, with an almost identical trace back (line 6 in _facade, rather than line 4).

[limi]

Seems like we missed this form. Andi, can you take a look?

[hannosch]

I suspect what happens on these forms is a bit more special. My guess is that they are protected, but both of them change the underlying key, which the authenticator is based upon. If the key is changed in the request before the authenticator is checked, you get an invalid authenticator, as it is not based on the new key.

Not sure about the order in which updating and validating happens here, maybe that can be changed, otherwise they need to except an authenticator based on an old key if that is possible or the authenticator be removed.

[hannosch]

s/except/accept/

[witsch]

fix description (ReSt) and properties first...

[witsch]

(In [21086]) add  csrf protection to PAS session plugin form used for managing server secrets (fixes #8176)

[witsch]

Replying to hannosch:

I suspect what happens on these forms is a bit more special. My guess is that they are protected, but both of them change the underlying key, which the authenticator is based upon. If the key is changed in the request before the authenticator is checked, you get an invalid authenticator, as it is not based on the new key.

just fyi, it's not changed before it's checked. the decorator (well, still a monkey patch atm, but now that the decorator is actually working that can be fixed as well) checks the token before the function is invoked, of course. the other way round doesn't really make sense, since that would allow an attacker to actually do something "useful" before the protection kicks in, wouldn't it? :)