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

Latest 1.2 does not work with ng-csp mode. #4394

Closed
mbelshe opened this issue Oct 13, 2013 · 9 comments
Closed

Latest 1.2 does not work with ng-csp mode. #4394

mbelshe opened this issue Oct 13, 2013 · 9 comments

Comments

@mbelshe
Copy link

mbelshe commented Oct 13, 2013

At least two major problems:

  • injects inline script on startup
  • ng-show/ng-hide are broken

And I'm not 100% certain, but I think ng-csp itself is broken in top-of-tree (it gets added to sniffer too late? - not sure, this may be my debugging failure)

To confirm:

  • in Gruntfile.js, uncomment the "util.csp()" line, and try to run the tests.

Note: if you can point me at the person that knows about the state of CSP - I'd be happy to contribute to fixing.

@petebacondarwin
Copy link
Contributor

I uncommented that line then ran

grunt test

Runs successfully for me.

@mbelshe - Can you confirm that this is a problem on HEAD of master?

@IgorMinar
Copy link
Contributor

1/ Angular doesn't inject scripts into the document. The error you are referring to comes from the index.html of our docs app.

2/ Angular does inject css stylesheet into the document. This is not csp friendly and causes ngShow/ngHide/ngCloak to fail because they depend on ng-cloak and ng-hide css classes defined in the injected stylesheet.


We should not do anything about 1/, but we should try to make 2/ better. That can be done either by detecting csp and not injecting the stylesheet or at least just document this behavior in ngShow/ngHide/ngCloak documentation and explain how to recreate the stylesheet by hand and include in from an external file. We could even create angular.css file at build time and distribute it with angular for the use with CSP.

@ghost ghost assigned tbosch Oct 14, 2013
@IgorMinar
Copy link
Contributor

@tigbro and @vojtajina are going to look into this.

@tbosch
Copy link
Contributor

tbosch commented Oct 14, 2013

The util.csp() is used in the devserver target of the Gruntfile, that is not used when executing grunt test.

@IgorMinar
Copy link
Contributor

end to end tests started via grunt test don't fail because even if you turn on csp in the web server, the webserver is proxied by karma, which doesn't preserve the csp headers.

@tbosch
Copy link
Contributor

tbosch commented Oct 14, 2013

Talked to @IgorMinar: only add inline css when not in csp mode, and add angular-csp.css to the resulting build files.

@mbelshe
Copy link
Author

mbelshe commented Oct 14, 2013

Thanks everyone for helping me fumble through this bug report. I believe Igor sorted it out correctly; its the inline style which is messing up, not inline scripts.

@tbosch
Copy link
Contributor

tbosch commented Oct 14, 2013

PR: #4411

IgorMinar added a commit to IgorMinar/angular.js that referenced this issue Oct 19, 2013
When we refactored , we broke the csp mode because the previous implementation
relied on the fact that it was ok to lazy initialize the .csp property, this
is not the case any more.

Besides, we need to know about csp mode during bootstrap and avoid injecting the
stylesheet when csp is active, so I refactored the code to fix both issues.

PR angular#4411 will follow up on this commit and add more improvements.

Closes angular#917
Closes angular#2963
Closes angular#4394
Closes angular#4444

BREAKING CHANGE: triggering ngCsp directive via `ng:csp` attribute is not
supported any more. Please use data-ng-csp instead.
@robinboehm
Copy link
Contributor

Got CSP problem with 1.2-rc.3.
Fix worked for me.
thx @IgorMinar

jamesdaily pushed a commit to jamesdaily/angular.js that referenced this issue Jan 27, 2014
When we refactored , we broke the csp mode because the previous implementation
relied on the fact that it was ok to lazy initialize the .csp property, this
is not the case any more.

Besides, we need to know about csp mode during bootstrap and avoid injecting the
stylesheet when csp is active, so I refactored the code to fix both issues.

PR angular#4411 will follow up on this commit and add more improvements.

Closes angular#917
Closes angular#2963
Closes angular#4394
Closes angular#4444

BREAKING CHANGE: triggering ngCsp directive via `ng:csp` attribute is not
supported any more. Please use data-ng-csp instead.
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this issue Jan 27, 2014
When we refactored , we broke the csp mode because the previous implementation
relied on the fact that it was ok to lazy initialize the .csp property, this
is not the case any more.

Besides, we need to know about csp mode during bootstrap and avoid injecting the
stylesheet when csp is active, so I refactored the code to fix both issues.

PR angular#4411 will follow up on this commit and add more improvements.

Closes angular#917
Closes angular#2963
Closes angular#4394
Closes angular#4444

BREAKING CHANGE: triggering ngCsp directive via `ng:csp` attribute is not
supported any more. Please use data-ng-csp instead.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.