-
Notifications
You must be signed in to change notification settings - Fork 27.4k
fix(ngCsp): allow CSP to be configurable #12346
Conversation
@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) | ||
}; |
There was a problem hiding this comment.
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
?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 😸
@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]'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
expect(spy)
for consistency ? 😄
There was a problem hiding this comment.
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
ae22415
to
e1fd333
Compare
I have added a commit with changes thanks to the feedback. |
LGTM 👍 |
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
👍 |
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
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
There are two different features in Angular that can break CSP rules:
use of
eval
to execute a string as JavaScript and dynamic injection ofCSS 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