owncloud/core — pull request #19 by arkascha (2012-09-28 14:52:21+00:00)
Title: Reimplementation of CSRF protection strategy

As discussed on the mailinglist I reimplemented the CSRF protection code ( "[Owncloud] CSRF behaviour is annoying" )

Motivation:
the current (static) strategy has an annoying side effect: since the token expires the user gets an error and is asked to "reload" when using an OC view older than 1 hour. This is fine for most things. This happens when leaving the client for a while (having lunch...) or using apps that do _not_ require a full page reload for every click (like the 'Shorty' app for example).
In the Shorty app I implemented a refresh strategy to pull fresh tokens to prevent that annoying problem That strategy works and appears safe to me. I was asked to implement that strategy in OC core which I did.

Details:
I tried to stick with the previous naming scheme where possible. I added two files and modified a few details. The new strategy:
- pull a fresh token right before the current one expires (96%)
- register that fresh token where required (currently ajax requests and EventSource)

Bugfix:
This commit also fixes Bug oc-1593 - Race condition with CSRF protection token initialization

Implementation:
The (first) token is still generated whilst evaluating the layout.user template. Alongside its lifespan is stored, that way that value is controlled from a single place in the code instead of being hard coded at various places. The new file core/js/requesttoken.js handles the token and its refreshing. It stores the token in the OC namespace and binds it to ajax requests. EventSource has been modified to use that token storage too.
A fresh token is pulled via an ajax call shortyl before the old one expire (96%). For this a new ajax backend has been implemented: core/ajax/requesttoken.php

Security:
The new strategy allows autorefreshment of the token. In my eyes this is not a security thread, since a fresh token can be generated anyway at all times simply by loading and OC view. So complex implementations can work around that protection anyway.
In the current state pulling a fresh token itself is _not_ protected by a token. Also, in my eyes this is _not_ a security problem, same reason. Though this can be argued about. Pulling a fresh token obviously still requires being authenticated and having a valid session.

Conclusion:
the commit implements an advanced strategy offering two advantages
1.) get rid of annoying error thrown into the users face
2.) fix race condition (bug 1593)

For questions and discussions: I ofen hang around at IRC #owncloud & #owncloud-dev, nick nae 'arkascha'.