Skip to content
This repository was 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
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 18 additions & 5 deletions src/Angular.js
Original file line number Diff line number Diff line change
Expand Up @@ -921,12 +921,25 @@ function equals(o1, o2) {
return false;
}

var csp = function() {
if (isDefined(csp.isActive_)) return csp.isActive_;

var active = !!(document.querySelector('[ng-csp]') ||
document.querySelector('[data-ng-csp]'));

if (!active) {
try {
/* jshint -W031, -W054 */
new Function('');
/* jshint +W031, +W054 */
} catch (e) {
active = true;
}
}

return (csp.isActive_ = active);
};

function csp() {
return (document.securityPolicy && document.securityPolicy.isActive) ||
(document.querySelector &&
!!(document.querySelector('[ng-csp]') || document.querySelector('[data-ng-csp]')));
}


function concat(array1, array2, index) {
Expand Down
25 changes: 19 additions & 6 deletions src/ng/directive/ngCsp.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.

* For us to be compatible, we just need to implement the "getterFn" in $parse without violating
* any of these restrictions.
* For Angular to be CSP compatible there are only two things that we need to do differently:
*
* - don't use `Function` constructor to generate optimized value getters
* - don't inject custom stylesheet into the document
*
* AngularJS uses `Function(string)` generated functions as a speed optimization. Applying the `ngCsp`
* directive will cause Angular to use CSP compatibility mode. When this mode is on AngularJS will
Expand All @@ -23,7 +25,18 @@
* includes some CSS rules (e.g. {@link ng.directive:ngCloak ngCloak}).
* To make those directives work in CSP mode, include the `angular-csp.css` manually.
*
* In order to use this feature put the `ngCsp` directive on the root element of the application.
* Angular tries to autodetect if CSP is active and automatically turn on the CSP-safe mode. This
* autodetection however triggers a CSP error to be logged in the console:
*
* ```
* Refused to evaluate a string as JavaScript because 'unsafe-eval' is not an allowed source of
* script in the following Content Security Policy directive: "default-src 'self'". Note that
* 'script-src' was not explicitly set, so 'default-src' is used as a fallback.
* ```
*
* This error is harmless but annoying. To prevent the error from showing up, put the `ngCsp`
* directive on the root element of the application or on the `angular.js` script tag, whichever
* appears first in the html document.
*
* *Note: This directive is only available in the `ng-csp` and `data-ng-csp` attribute form.*
*
Expand All @@ -38,6 +51,6 @@
```
*/

// ngCsp is not implemented as a proper directive any more, because we need it be processed while we bootstrap
// the system (before $parse is instantiated), for this reason we just have a csp() fn that looks for ng-csp attribute
// anywhere in the current doc
// ngCsp is not implemented as a proper directive any more, because we need it be processed while we
// bootstrap the system (before $parse is instantiated), for this reason we just have
// the csp.isActive() fn that looks for ng-csp attribute anywhere in the current doc
10 changes: 6 additions & 4 deletions test/AngularSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -413,14 +413,15 @@ describe('angular', function() {


describe('csp', function() {
var originalSecurityPolicy;
var originalFunction;

beforeEach(function() {
originalSecurityPolicy = document.securityPolicy;
originalFunction = window.Function;
});

afterEach(function() {
document.securityPolicy = originalSecurityPolicy;
window.Function = originalFunction;
delete csp.isActive_;
});


Expand All @@ -430,10 +431,11 @@ describe('angular', function() {


it('should return true if CSP is autodetected via CSP v1.1 securityPolicy.isActive property', function() {
document.securityPolicy = {isActive: true};
window.Function = function() { throw new Error('CSP test'); };
expect(csp()).toBe(true);
});


it('should return the true when CSP is enabled manually via [ng-csp]', function() {
spyOn(document, 'querySelector').andCallFake(function(selector) {
if (selector == '[ng-csp]') return {};
Expand Down