Add ability to bypass state validation, for Platform Storage use-case

[junglebarry]

When we implement LTI Platform Storage API, downstream single-page-apps must take responsibility for validating the state against the Platform Storage API, rather than using cookies. In this scenario, we want to be able to bypass the cookie-based state validation step entirely.

In this PR, we add an optional parameter - which defaults to false - to insecurelyBypassStateValidation.

[rsinger]

So, codewise, this all seems fine to me.

I'll be honest that I do not really understand the risks introduced by this. I mean, I understand what it's doing and can think of theoretical problems, but I can't fully wrap my head around how the launch flow could be compromised in practice.

I think the bigger issue, honestly, is social engineering, and how we manage customer expectations around this, especially the VLE administrators, who have no reason to trust us whatsoever.

[junglebarry]

@rsinger the changes introduced here should have no consequence for the launch flow - it should not be compromised in practice or in theory, because the CSRF protection will still be implemented by another means (in the SPA, using Platform Storage API). This is just providing users this library with the ability to take responsibility for the CSRF validation themselves.

However, by even allowing this there is an inherent risk, in that someone might accidentally introduce mistakes in their implementation of the CSRF protection. However, it can't be the responsibility of this library (as a server-side only component) to enforce this. I think the remit of this library's involvement in CSRF protection stops with "making clear to the developer that they have taken responsibility for CSRF". If you feel this code achieves that, then this PR can be merged.

I think the LTI working group did have a good discussion of how the CSRF attack works, see around 11:45 in this video. I don't disagree that there are other attack vectors that are probably more practically problematic, but they're probably for a different discussion.

[rsinger]

@junglebarry I feel pretty satisfied with this explanation. Like I said, I didn't feel there were really very many practical vulnerabilities, but I wanted to understand the risks a little better. And the default is for it to be on, anyway.

I agree that it's not the responsibility of the library to enforce that users don't misuse it. I do think it's reasonable to relay the risks of what disabling this would leave the developer on the hook for, although I don't think that needs to be programmatic.