On 19 mars 09, at 17:43, Sergiu Dumitriu wrote:

The use case is:

- I log in
- I spend a lot of time writing a document
- I hit save
- Unfortunately, my authentication expired
- I am redirected to the login page, as I don't have the right to save
the document as a guest
- I login
- I just lost my hard work

SavedRequestRestorer saves the posted data in the session object, so
that after authentication it can be safely retrieved and used.

Is it causing problems on your side?

No really. I have made further testing, and I had seen only one case where the request is not properly restored:

- when your server session expire (JSESSIONID lost or invalid) and you also loose your username and password session cookies (or permanent cookies if you check j_remerberme).

This seems too me really unusual, and that is why I wondered about the addition of this filter. What I would have done, probably more in the spirit of the current (ugly?) code, is to use the already saved request, the one saved by the Authenticator derived either from BasicAuthenticator or FormAuthenticator, and put it back during request wrapping.

I join a simple patch (not finalized, neither review, that revert yours and do so, don't forget to remove the filter in web.xml as well), to be clearer. I feel that the best would be something in the middle, but currently I dislike the added srid parameter, and at least the need of that for the purposed currently announced. I completely agree that the current coupling with the org.securityfilter is really dependant on its implementation which is not good, but it was how the current code has been built on. I am thinking about an improvement, half way between what is the ultimate road (writing components) and the current implementation.

I want something enough flexible ad robust to easily add both authenticator and authentication provider independently and chained. From what I have review, I am more and more convinced that I have to start over apart to avoid even more complexity in the current implementation, which undoubtedly lead to more bugs. I am just not sure I will had the time required :(

Denis

Attachment: requestrestorer.patch
Description: Binary data


_______________________________________________
devs mailing list
[email protected]
http://lists.xwiki.org/mailman/listinfo/devs

Reply via email to