silverstripe / silverstripe-framework

Silverstripe Framework, the MVC framework that powers Silverstripe CMS
https://www.silverstripe.org
BSD 3-Clause "New" or "Revised" License
721 stars 821 forks source link

[FORMS] Protection against CSRF attacks should be AJAX friendly #5187

Open elvinas-liut opened 8 years ago

elvinas-liut commented 8 years ago

Protection against CSRF attacks should be AJAX friendly, just like getValidationErrorResponse(). Now it breaks ajax requests if php session is expired.

dhensby commented 8 years ago

What way do you want it to be nicer?

tractorcow commented 8 years ago

If the PHP Session is expired, then the in-cms authentication should popup, and request the user to login again.

Once that's done, it's meant to re-populate all CSRF hidden inputs in the current page.

If your session is expiring, are you getting the CMS popup requesting you to re-enter your password?

dhensby commented 8 years ago

I had thought this was more aimed at front end, not cms, ajax

tractorcow commented 8 years ago

Hm, maybe the error condition isn't well catered for... you could be posting to an un-authenticated form, although the CSRF itself could fail. In which case, maybe the CMS re-authentication won't kick in properly. :)

I think in this case I'll need a more concise error reproduction process, if you are able to help, @uniun .

Which form did you post to, what url, what was the expected response, and what was the actual response? Were you expecting json and getting html, right?

elvinas-liut commented 8 years ago

Sorry for the late response.

@dhensby is right, I was talking about frontend.

Sometimes session lifetime is very short and you cannot increase because you are not allowed to change it. Lets say the session lifetime is just about 10mins and you leave the page with a form for 15mins. Then do AJAX form submission.

Form checks for CSRF attacks and redirects you back instead of rendering a form: Form.php#L380.

But I think that it could check for an AJAX request and render a form, just like on Form.php#L501.

tractorcow commented 8 years ago

Ok, my ideal fix for this is posted on another issue... I've responded to thousands and can't remember where it is though... take hard coded logic out of Form.php and put it into a CSRFFormField, and implement validate() logic there.

That way invalid forms will trigger normal formfield validation, rather than having custom handling that breaks cms randomly.

zanderwar commented 8 years ago

@tractorcow is this still planned?

In other CMS (Invision Power Suite) for example. They would have a simple function: $this->getCSRFtoken() which would output the hidden field, then on the recipient side of things you could have a one-liner $this->validateCSRFtoken()

maxime-rainville commented 4 years ago

Can someone confirmed if this is still a problem in SS4?