Add XSRF mitigation to form processing actions

Basics

Technical

Logistics

Basics

Technical

Logistics

Description

We don't currently have any XSRF mitigations.

It would be possible to explore doing this by overriding the flow execution keys with something random, but it's difficult to prevent those from showing up on the URLs.

An alternative would be to hit all of our view states and have them generate and insert a token into the views, store it in the PRC, and then rebase some of the action beans on a base class that checks for the token.

That would require modifying views but we could add this as a property defaulting off in 3.4 and then default it on in 4.0.

Environment

None

Activity

Show:

Philip Smart December 3, 2019 at 10:38 AM
Edited

That's better, thanks. I will make sure to add this to the 'Inserting anti-CSRF tokens into HTML forms' deployer doc.

I probably should have looked at some sort of auto-csrf approach to dynamically embed the token into forms. I could still look into that, or maybe as an enhancement later on - or maybe it is overkill given the IdP will only have a limited number of views and forms and just parsing the csrf.vm template is similar to adding a JSP tag etc.

Scott Cantor December 2, 2019 at 5:50 PM

I moved the input element boilerplate into system/views/csrf/csrf.vm so it's system-controllable HTML, and removed the id attribute (there was one view where it was included twice, so technically the HTML IDs would collide.

Philip Smart November 15, 2019 at 2:56 PM

Agreed w.r.t the upgrade and new install defaults.

Scott Cantor November 15, 2019 at 2:31 PM

This all looks pretty good to me, without having dug into the code itself much.

My guess given which files are going to get skipped on an upgrade is that a new install is safe to enable it using the include "*" and exclude list approach. Upgrades will default to having it off, and people can choose how to apply it then if they have custom views that they don't want to mess with.

But this is probably one of those cases where the internal property defaults may not match what we want to include in the file. We probably want to default the include and exclude lists to nothing as a code/wiring default.