Add ability to fail application / addon tests when a CSP violation is detected.

[jelhan]

This addon helps to ensure that an application or addon is not violating a strict CSP while developing but doesn't provide any testing solution. Providing an easy to use solution for ensuring that an addon is not violating a strict CSP would help the ecosystem a lot IMO. I'm maintaining an application that requires a strict CSP and noticed that from time to time CSP violations are added in addons without being aware of it.

This is only a PoC and not a fully functional solution. There are some open questions:

• I would like to add some tests. Actually doing so shouldn't be that difficult. A combination of htmlbars-inline-precompile and assert.rejects should just work fine. But doing it with old testing API feels like a burden. Would be great if Drop node 4 and Ember CLI < 2.13 (update to Ember 3.7 blueprint) #87 lands first.

• CSP headers are only present if tests are executed through opening http://localhost:4200/tests in browser. They are not present for ember test --server nor ember test. Failing on CSP violations in that scenarios also would require injecting CSP as meta tag (contentSecurityPolicyMeta = true).

• Providing an option to disable testing feels strange. I know we have to provide a way to not throw for a transition period but allowing to disable the addon per environment would feel more familiar, e.g. contentSecurityPolicy: { enabled: environment !== 'test' }. This is current only supported by an undocumented hack (Added development configuration example #77).

• ~~The last point already demonstrated that current configuration interface isn't inline with the rest of the ecosystem. It's using three different keys on the root configuration object: contentSecurityPolicy, contentSecurityPolicyHeader and contentSecurityPolicyMeta. Maybe we should refactor that before adding another option.~~~

Update

This adds a new configuration option failTests. If it's true (and addon is enabled) it will cause tests to fail on every CSP violation. This is done by adding an event listener for CSP violations, which throws an error. Uncaught global errors cause the test to fail.

It enforces a delivery through meta tag for test environment cause that one may be executed without an express server (and therefore without CSP headers). Otherwise ember test would not fail on CSP violations if delivery option does not include 'meta'.

This might be considered a breaking change. Tests that passed previously may fail after upgrade cause failTests defaults to true.

[sandstrom]

• CSP headers are only present if tests are executed through opening http://localhost:4200/tests in browser. They are not present for ember test --server nor ember test. Failing on CSP violations in that scenarios also would require injecting CSP as meta tag (contentSecurityPolicyMeta = true).

What do you think about only using the meta for this? That way we'd have the same mechanism both for tests running through the browser and for ember test.

• The last point already demonstrated that current configuration interface isn't inline with the rest of the ecosystem. It's using three different keys on the root configuration object: contentSecurityPolicy, contentSecurityPolicyHeader and contentSecurityPolicyMeta. Maybe we should refactor that before adding another option.

I agree, and I'm culpable here 😄 I don't know enough about Ember testing to say anything about this PR in general (rwjblue is an expert at that though). But I'm happy to assist a rework of the options for this addon (single key with nested opts inside).

[jelhan]

What do you think about only using the meta for this? That way we'd have the same mechanism both for tests running through the browser and for ember test.

That might be the best idea. Especially cause the tests are run with CSP enforced. So even if tests don't throw cause of CSP violation they might fail due to code been blocked by CSP.

I agree, and I'm culpable here I don't know enough about Ember testing to say anything about this PR in general (rwjblue is an expert at that though). But I'm happy to assist a rework of the options for this addon (single key with nested opts inside).

Please have a look at #92.

[jelhan]

@rwjblue @sandstrom This is ready to review from my side. Updated the initial post with the implementation details.

[rwjblue]

😩, I'm sorry looks like merging #103 caused a merge conflict

[rwjblue]

this is great btw!

[rwjblue]

I think we can use template strings here, which would make this a bit easier to author and read...

[jelhan]

Need to escape the backticks used by htmlbars-inline-precompile than. So doesn't make a big difference. But refactored to use backticks cause I don't care to be honest. 😄

[rwjblue]

Can we implement this using ember-try? I don't think this needs to block landing the change though...

[jelhan]

I'm not sure how we could use ember-try in the context of an ember-cli-addon-tests apps. The issue here isn't with ember-cli-addon-tests but with ember-cli-version-checker. Haven't had the time yet to open a issue but it seems to make some wrong assumptions leading to wrong version reported.

The test is setting up a correct environment. npm list returns the specified version of ember-qunit but ember-cli-version-checker seems to pick up the version used by the host project. It seems to be confused by some internals used by ember-cli-addon-tests. At least that's what I got from a quick investigation.

Will try to find the time for opening a bug report including a reproduction.

[rwjblue]

👍, sounds good I had forgotten RE: this being an ember-cli-addon-tests type test

[jelhan]

@rwjblue Thanks for your review feedback. I addressed all of it.

I expected the merge conflict. It needed a rebase cause it has included same commits as #103, which got merged earlier. So no changes where needed to fix the conflicts.

Rebased and squashed. Ready to be merged from my side.