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

fix(ngCsp): allow CSP to be configurable #12346

Merged
merged 2 commits into from Jul 16, 2015

Conversation

petebacondarwin
Copy link
Member

There are two different features in Angular that can break CSP rules:
use of eval to execute a string as JavaScript and dynamic injection of
CSS style rules into the DOM.

This change allows us to configure which of these features should be turned
off to allow a more fine grained set of CSP rules to be supported.

Closes #11933
Closes #8459

@petebacondarwin
Copy link
Member Author

@jdalton, @realityking, @lgalfaso and @btford - can you take a look?

csp.rules = {
noUnsafeEval: !ngCspAttribute || (ngCspAttribute.indexOf('no-unsafe-eval') !== -1),
noInlineStyle: !ngCspAttribute || (ngCspAttribute.indexOf('no-inline-style') !== -1)
};
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 the ngCspAttribute.indexOf checks be looking for > -1 as if they're declared then they're true?

Copy link
Contributor

Choose a reason for hiding this comment

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

Checking for !== -1 has the same effect, so this is fine.

Copy link
Contributor

Choose a reason for hiding this comment

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

Checking for !== -1 has the same effect, so this is fine.

Doh! I blame it on all the double negatives 😸

@realityking
Copy link
Contributor

@petebacondarwin I like it (a lot)!

expect(csp()).toBe(true);
it('should return true when CSP is enabled manually via [data-ng-csp]', function() {
var spy = mockCspElement('data-ng-csp');
expect(csp()).toEqual({ noUnsafeEval: true, noInlineStyle: true });
expect(document.querySelector).toHaveBeenCalledWith('[data-ng-csp]');
Copy link
Member

Choose a reason for hiding this comment

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

expect(spy) for consistency ? 😄

Copy link
Member Author

Choose a reason for hiding this comment

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

Right!

There are two different features in Angular that can break CSP rules:
use of `eval` to execute a string as JavaScript and dynamic injection of
CSS style rules into the DOM.

This change allows us to configure which of these features should be turned
off to allow a more fine grained set of CSP rules to be supported.

Closes angular#11933
Closes angular#8459
There are two different features in Angular that can break CSP rules:
use of `eval` to execute a string as JavaScript and dynamic injection of
CSS style rules into the DOM.

This change allows us to configure which of these features should be turned
off to allow a more fine grained set of CSP rules to be supported.

Closes angular#11933
Closes angular#8459
@petebacondarwin
Copy link
Member Author

I have added a commit with changes thanks to the feedback.

@gkalpak
Copy link
Member

gkalpak commented Jul 15, 2015

LGTM 👍

@petebacondarwin petebacondarwin modified the milestones: 1.4.3, 1.4.4 Jul 16, 2015
@petebacondarwin petebacondarwin merged commit e1fd333 into angular:master Jul 16, 2015
petebacondarwin added a commit that referenced this pull request Jul 16, 2015
There are two different features in Angular that can break CSP rules:
use of `eval` to execute a string as JavaScript and dynamic injection of
CSS style rules into the DOM.

This change allows us to configure which of these features should be turned
off to allow a more fine grained set of CSP rules to be supported.

Closes #11933
Closes #8459
Closes #12346
@btford
Copy link
Contributor

btford commented Jul 16, 2015

👍

netman92 pushed a commit to netman92/angular.js that referenced this pull request Aug 8, 2015
There are two different features in Angular that can break CSP rules:
use of `eval` to execute a string as JavaScript and dynamic injection of
CSS style rules into the DOM.

This change allows us to configure which of these features should be turned
off to allow a more fine grained set of CSP rules to be supported.

Closes angular#11933
Closes angular#8459
Closes angular#12346
ggershoni pushed a commit to ggershoni/angular.js that referenced this pull request Sep 29, 2015
There are two different features in Angular that can break CSP rules:
use of `eval` to execute a string as JavaScript and dynamic injection of
CSS style rules into the DOM.

This change allows us to configure which of these features should be turned
off to allow a more fine grained set of CSP rules to be supported.

Closes angular#11933
Closes angular#8459
Closes angular#12346
@petebacondarwin petebacondarwin deleted the csp-config branch November 24, 2016 09:04
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.

None yet

5 participants