Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

fix(csp): fix autodetection of CSP + better docs #8191

Closed
wants to merge 1 commit into from

Conversation

IgorMinar
Copy link
Contributor

CSP spec got changed and it is no longer possible to autodetect if a policy is
active without triggering a CSP error:

w3c/webappsec@1888295

Now we use new Function('') to detect if CSP is on. To prevent error from this
detection to show up in console developers have to use the ngCsp directive.

(This problem became more severe after our recent removal of simpleGetterFn
which made us depend on function constructor for all expressions.)

Closes #8162

@mary-poppins
Copy link

Thanks for the PR! Please check the items below to help us merge this faster. See the contributing docs for more information.

  • Uses the issue template (#8191)

If you need to make changes to your pull request, you can update the commit with git commit --amend.
Then, update the pull request with git push -f.

Thanks again for your help!

@IgorMinar
Copy link
Contributor Author

@caitp please review

@@ -921,12 +921,26 @@ function equals(o1, o2) {
return false;
}

var csp = {
isActive_: undefined,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: whitespace is a bit awkward here

@caitp
Copy link
Contributor

caitp commented Jul 14, 2014

looks generally good, just double checking the docs.

@@ -11,8 +11,10 @@
* This is necessary when developing things like Google Chrome Extensions.
*
* CSP forbids apps to use `eval` or `Function(string)` generated functions (among other things).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the trouble with this is that we're making an assumption that eval is disabled --- which it isn't necessarily (unsafe-eval). I expect this will be fine for most users, but it's not entirely accurate. I guess we need to wait until there is a better way to get this information (before changing that)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if unsafe-eval is enabled then we will be able to use our generated getters which means that we'll possibly display an error due to the stylesheet, but that's not a big deal.

@caitp
Copy link
Contributor

caitp commented Jul 14, 2014

Okay, looks good with nits --- fixing the nits will keep the api strictly compatible with the previous implementation, but it's a private API so I don't care a whole lot. It's up to you.

@IgorMinar
Copy link
Contributor Author

unit tests are broken. fixing that now.

IgorMinar added a commit to IgorMinar/angular.js that referenced this pull request Jul 14, 2014
CSP spec got changed and it is no longer possible to autodetect if a policy is
active without triggering a CSP error:

w3c/webappsec@1888295

Now we use `new Function('')` to detect if CSP is on. To prevent error from this
detection to show up in console developers have to use the ngCsp directive.

(This problem became more severe after our recent removal of `simpleGetterFn`
 which made us depend on function constructor for all expressions.)

Closes angular#8162
Closes angular#8191
IgorMinar added a commit to IgorMinar/angular.js that referenced this pull request Jul 14, 2014
CSP spec got changed and it is no longer possible to autodetect if a policy is
active without triggering a CSP error:

w3c/webappsec@1888295

Now we use `new Function('')` to detect if CSP is on. To prevent error from this
detection to show up in console developers have to use the ngCsp directive.

(This problem became more severe after our recent removal of `simpleGetterFn`
 which made us depend on function constructor for all expressions.)

Closes angular#8162
Closes angular#8191
CSP spec got changed and it is no longer possible to autodetect if a policy is
active without triggering a CSP error:

w3c/webappsec@1888295

Now we use `new Function('')` to detect if CSP is on. To prevent error from this
detection to show up in console developers have to use the ngCsp directive.

(This problem became more severe after our recent removal of `simpleGetterFn`
 which made us depend on function constructor for all expressions.)

Closes angular#8162
Closes angular#8191
IgorMinar added a commit that referenced this pull request Jul 15, 2014
CSP spec got changed and it is no longer possible to autodetect if a policy is
active without triggering a CSP error:

w3c/webappsec@1888295

Now we use `new Function('')` to detect if CSP is on. To prevent error from this
detection to show up in console developers have to use the ngCsp directive.

(This problem became more severe after our recent removal of `simpleGetterFn`
 which made us depend on function constructor for all expressions.)

Closes #8162
Closes #8191
@IgorMinar IgorMinar closed this in 0113f22 Jul 15, 2014
ckknight pushed a commit to ckknight/angular.js that referenced this pull request Jul 16, 2014
CSP spec got changed and it is no longer possible to autodetect if a policy is
active without triggering a CSP error:

w3c/webappsec@1888295

Now we use `new Function('')` to detect if CSP is on. To prevent error from this
detection to show up in console developers have to use the ngCsp directive.

(This problem became more severe after our recent removal of `simpleGetterFn`
 which made us depend on function constructor for all expressions.)

Closes angular#8162
Closes angular#8191
tlvince added a commit to tlvince/LMIS-Chrome that referenced this pull request Jul 25, 2014
Manually enable Angular CSP mode as auto-detection triggers a harmless, though
nonetheless annoying CSP error.

CSP detection has also proved to be brittle.

Override Angular's Bower main block to include `angular-csp.css` so that Wiredep
can inject it for us.

See: angular/angular.js#8191
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CSP auto detection not working in 1.3.0-beta 15 and Packaged Chrome App?
3 participants