identity-idp icon indicating copy to clipboard operation
identity-idp copied to clipboard

[LG-7191] add configuration flipper to suppress content security policy (CSP)

Open gangelo opened this issue 3 years ago • 5 comments

https://cm-jira.usa.gov/browse/LG-7191

Suppress content security policy (CSP) if IdentityConfig.store.suppress_content_security_policy is true.

ONLY to be set to true on an AWS sandbox or during local development!

508 tools that rely on javascript execution (ANDI for example) are unable to execute in the development environment due to Content Security Policy (CSP) policies.

Creating/setting IdentityConfig.store.suppress_content_security_policy in an AWS sandbox will allow developers to turn CSP off, allowing these valuable tools to run.

gangelo avatar Aug 10 '22 14:08 gangelo

Copying from https://github.com/18F/identity-idp/pull/6705#issuecomment-1208570432

Would the existing disable_csp_unsafe_inline configuration help with the need? Could we consider other tools? It's a bit concerning to me that a tool wouldn't be functional without the ability to download third-party scripts or call out to a third-party API. Historically I've been able to use the Deque Axe DevTools and Accessibility Insights for Web extensions without any issues, and those have been pretty popular in the program.

If we did add this, would[/could] the idea be to only allow for it in local development, and not in any deployed environments?

aduth avatar Aug 11 '22 21:08 aduth

Copying from #6705 (comment)

Would the existing disable_csp_unsafe_inline configuration help with the need? Could we consider other tools? It's a bit concerning to me that a tool wouldn't be functional without the ability to download third-party scripts or call out to a third-party API. Historically I've been able to use the Deque Axe DevTools and Accessibility Insights for Web extensions without any issues, and those have been pretty popular in the program.

Apologies for not addressing this, but unfortunately, disable_csp_unsafe_inline didn't do the trick.

If we did add this, would[/could] the idea be to only allow for it in local development, and not in any deployed environments?

The idea is to use it with an AWS sandbox that everyone can use and share; although, we could also use it locally of course.

gangelo avatar Aug 11 '22 22:08 gangelo

I put a lot of value in the CSP, so I'd rather see it enforced in more places than in fewer (e.g. local development). One reason is that local development can currently give a false sense of confidence that a feature relying on a CSP bypass will work, only for it to break once it reaches a deployed environment (previously #5852, #4813, #4429, and likely soon again in #6694). As proposed, a fear I'd have is that we'd not know of these issues 'til they start affecting real users in production.

Personally, I'd rather see one of:

  • Open the CSP only as much as necessary to support what we need (i.e. add the directives and hosts necessary to support this tool as configuration)
  • Or, limit its usage to personal sandbox environments, not common ones like dev or int
  • Or (my preference), switch to using an accessibility tool that's compatible with the current CSP, like the ones mentioned in my previous comment

aduth avatar Aug 12 '22 12:08 aduth

put a lot of value in the CSP, so I'd rather see it enforced in more places than in fewer (e.g. local development). One reason is that local development can currently give a false sense of confidence that a feature relying on a CSP bypass will work, only for it to break once it reaches a deployed environment (previously #5852, #4813, #4429, and likely soon again in #6694). As proposed, a fear I'd have is that we'd not know of these issues 'til they start affecting real users in production.

Personally, I'd rather see one of:

  • Open the CSP only as much as necessary to support what we need (i.e. add the directives and hosts necessary to support this tool as configuration)
  • Or, limit its usage to personal sandbox environments, not common ones like dev or int
  • Or (my preference), switch to using an accessibility tool that's compatible with the current CSP, like the ones mentioned in my previous comment

The very idea behind this flipper is to explicitly turn CSP off to open up the possibility to use tools that are helpful, yet otherwise would be unavailable for use; tools that have nothing to do with CSP. 508 tools for example. No one should have any sense of security, false or otherwise, when utilizing this flipper. No one should be using a specific sandbox, unless they know what the sandbox is used for and anyone who would want to test anything to do with CSP on a box that has CSP turned off, probably shouldn't be there to begin with :).

The other tools you listed, are (from what I saw) mostly automated tools that require a lot of set up, and much more than a developer may need, as it is in our situation.

I'm okay with option 2 (Or, limit its usage to personal sandbox environments, not common ones like dev or int); however, it was Zach's suggestion to place this on a sandbox, and I think it's a good one. If we have to limit this to option 2, I'm okay with that.

gangelo avatar Aug 12 '22 13:08 gangelo

The very idea behind this flipper is to explicitly turn CSP off to open up the possibility to use tools that are helpful, yet otherwise would be unavailable for use; tools that have nothing to do with CSP. 508 tools for example. No one should have any sense of security, false or otherwise, when utilizing this flipper. No one should be using a specific sandbox, unless they know what the sandbox is used for and anyone who would want to test anything to do with CSP on a box that has CSP turned off, probably shouldn't be there to begin with :).

The other tools you listed, are (from what I saw) mostly automated tools that require a lot of set up, and much more than a developer may need, as it is in our situation.

I'm okay with option 2 (Or, limit its usage to personal sandbox environments, not common ones like dev or int); however, it was Zach's suggestion to place this on a sandbox, and I think it's a good one. If we have to limit this to option 2, I'm okay with that.

I dug in a bit, and I think the approach is a bit riskier than is necessary. The ANDI bookmarklet in question only tries to inject the https://www.ssa.gov/accessibility/andi/andi.js script into the page rather than as a browser extension, which requires the CSP hole-punching.

WAVE, Accessibility Insights, or Deque offer alternatives that should only take a few clicks to install and inspect elements or potential accessibility issues. With the complexity/risk present here, I think one of the aforementioned alternatives should be pursued.

mitchellhenke avatar Aug 12 '22 15:08 mitchellhenke

It looks like the ticket for this got cancelled? Is this PR still open for review?

jmhooper avatar Aug 15 '22 18:08 jmhooper

i agree there are times when i need it locally and do something similar in the code.

stevegsa avatar Aug 15 '22 18:08 stevegsa

Looks like this PR hasn't been updated in a bit, I'm going to close it out to keep the list of open PRs small

zachmargolis avatar Aug 22 '22 19:08 zachmargolis

It looks like the ticket for this got cancelled? Is this PR still open for review?

It was closed due to the aforementioned concerns. I personally didn't share the same concerns as this would be sandboxed, either locally or on a literal AWS sandbox. While this flipper was not meant to be tool-specific, other tools were offered in lieu of ANDI (which was what prompted this PR). A cursory of the tools offered weren't as intuitive or user-friendly IMHO; but, in all fairness, I didn't have time to dig too deeply into them, which may speak to the intuitiveness/user-friendliness of the tools offered, but like I mentioned, this may be an unfair assessment on my part.

gangelo avatar Aug 24 '22 11:08 gangelo

i agree there are times when i need it locally and do something similar in the code.

I see no problem with having that flipper available, even if (albeit) simply for local dev use. We'll probably resort to passing a patch around since some members on our team are familiar with and like the ANDI 508 tool :(

gangelo avatar Aug 24 '22 11:08 gangelo