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

fix(csp): fix csp auto-detection and stylesheet injection #4444

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
2 changes: 1 addition & 1 deletion lib/grunt/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ module.exports = {
.replace(/\\/g, '\\\\')
.replace(/'/g, "\\'")
.replace(/\r?\n/g, '\\n');
return "angular.element(document).find('head').prepend('<style type=\"text/css\">" + css + "</style>');";
return "!angular.$$csp() && angular.element(document).find('head').prepend('<style type=\"text/css\">" + css + "</style>');";
}
},

Expand Down
7 changes: 7 additions & 0 deletions src/Angular.js
Original file line number Diff line number Diff line change
Expand Up @@ -760,6 +760,13 @@ function equals(o1, o2) {
}


function csp() {
return (document.securityPolicy && document.securityPolicy.isActive) ||
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we cache this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ignore that, not a big deal.

(document.querySelector &&
!!(document.querySelector('[ng-csp]') || document.querySelector('[data-ng-csp]')));
}


function concat(array1, array2, index) {
return array1.concat(slice.call(array2, index));
}
Expand Down
6 changes: 3 additions & 3 deletions src/AngularPublic.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,13 @@ function publishExternalAPI(angular){
'isNumber': isNumber,
'isElement': isElement,
'isArray': isArray,
'$$minErr': minErr,
'version': version,
'isDate': isDate,
'lowercase': lowercase,
'uppercase': uppercase,
'callbacks': {counter: 0}
'callbacks': {counter: 0},
'$$minErr': minErr,
'$$csp': csp
});

angularModule = setupModuleLoader(window);
Expand Down Expand Up @@ -77,7 +78,6 @@ function publishExternalAPI(angular){
ngClass: ngClassDirective,
ngClassEven: ngClassEvenDirective,
ngClassOdd: ngClassOddDirective,
ngCsp: ngCspDirective,
ngCloak: ngCloakDirective,
ngController: ngControllerDirective,
ngForm: ngFormDirective,
Expand Down
24 changes: 10 additions & 14 deletions src/ng/directive/ngCsp.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,26 @@
/**
* @ngdoc directive
* @name ng.directive:ngCsp
* @priority 1000
*
* @element html
* @description
* Enables [CSP (Content Security Policy)](https://developer.mozilla.org/en/Security/CSP) support.
*
*
* This is necessary when developing things like Google Chrome Extensions.
*
*
* CSP forbids apps to use `eval` or `Function(string)` generated functions (among other things).
* For us to be compatible, we just need to implement the "getterFn" in $parse without violating
* any of these restrictions.
*
*
* 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
* evaluate all expressions up to 30% slower than in non-CSP mode, but no security violations will
* be raised.
*
*
* In order to use this feature put the `ngCsp` directive on the root element of the application.
*
*
* *Note: This directive is only available in the ng-csp and data-ng-csp attribute form.*
*
* @example
* This example shows how to apply the `ngCsp` directive to the `html` tag.
<pre>
Expand All @@ -33,11 +34,6 @@
</pre>
*/

var ngCspDirective = ['$sniffer', function($sniffer) {
return {
priority: 1000,
compile: function() {
$sniffer.csp = true;
}
};
}];
// 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
2 changes: 1 addition & 1 deletion src/ng/sniffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ function $SnifferProvider() {

return eventSupport[event];
},
csp: document.securityPolicy ? document.securityPolicy.isActive : false,
csp: csp(),
Copy link
Contributor

Choose a reason for hiding this comment

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

@tbosch and I looked at this yesterday.  document.securityPolicy appears to be undefined even when the Content-Security-Policy header is present and csp is enabled.  This is on Chrome 29.  So not sure that this fixes it.  We couldn't find a good way to detect csp.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually the spec says that this is the correct way of doing it: https://dvcs.w3.org/hg/content-security-policy/raw-file/tip/csp-specification.dev.html

However, Chrome and others don't seem to support this yet. I like the change of @IgorMinar as it allows to manually override the autodetection and it is quite simple.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@chirayuk securityPolicy is a CSPv1.1 feature that is not in non-canary Chrome yet.

Copy link
Contributor

Choose a reason for hiding this comment

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

Aha.  Good to know.  Thanks!

vendorPrefix: vendorPrefix,
transitions : transitions,
animations : animations
Expand Down
40 changes: 40 additions & 0 deletions test/AngularSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,46 @@ describe('angular', function() {
});


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

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

afterEach(function() {
document.securityPolicy = originalSecurityPolicy;
});


it('should return the false when CSP is not enabled (the default)', function() {
expect(csp()).toBe(false);
});


it('should return true if CSP is autodetected via CSP v1.1 securityPolicy.isActive property', function() {
document.securityPolicy = {isActive: true};
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 {};
});
expect(csp()).toBe(true);
});


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


describe('parseKeyValue', function() {
it('should parse a string into key-value pairs', function() {
expect(parseKeyValue('')).toEqual({});
Expand Down
10 changes: 0 additions & 10 deletions test/ng/directive/ngCspSpec.js

This file was deleted.

12 changes: 1 addition & 11 deletions test/ng/snifferSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,19 +85,9 @@ describe('$sniffer', function() {


describe('csp', function() {
it('should be false if document.securityPolicy.isActive not available', function() {
it('should be false by default', function() {
expect(sniffer({}).csp).toBe(false);
});


it('should use document.securityPolicy.isActive if available', function() {
var createDocumentWithCSP = function(csp) {
return {securityPolicy: {isActive: csp}};
};

expect(sniffer({}, createDocumentWithCSP(false)).csp).toBe(false);
expect(sniffer({}, createDocumentWithCSP(true)).csp).toBe(true);
});
});

describe('vendorPrefix', function() {
Expand Down