Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid inline-styling to meet tighter content-security-policy #2606

Closed
KKoukiou opened this issue Jul 29, 2019 · 6 comments · Fixed by #2653
Closed

Avoid inline-styling to meet tighter content-security-policy #2606

KKoukiou opened this issue Jul 29, 2019 · 6 comments · Fixed by #2653
Assignees
Milestone

Comments

@KKoukiou
Copy link
Collaborator

To make use of the patternfly-react in projects with higher security demands, like the Cockpit (www.cockpit-project.org), modifications/use of style HTML element attribute must be avoided.

I tried to import Table component, and I got the following warning in the browser:


  | insert | @ | index.esm.js:198
-- | -- | -- | --
  | insertRule | @ | index.esm.js:242
  | toSheet | @ | index.js:16
  | ruleSheet | @ | index.js:44
  | H | @ | stylis.esm.js:526
  | B | @ | stylis.esm.js:588
  | stylis | @ | index.esm.js:407
  | insert | @ | index.esm.js:415
  | injectGlobal | @ | index.esm.js:441
  | inject | @ | StyleSheet.js:45
  | ./node_modules/@patternfly/react-core/dist/esm/layouts/Toolbar/Toolbar.js | @ | Toolbar.js:38
  | __webpack_require__ | @ | bootstrap:19
  | ./node_modules/@patternfly/react-core/dist/esm/layouts/Toolbar/index.js | @ | index.js:1
  | __webpack_require__ | @ | bootstrap:19
  | ./node_modules/@patternfly/react-core/dist/esm/layouts/index.js | @ | index.js:1
  | __webpack_require__ | @ | bootstrap:19
  | ./node_modules/@patternfly/react-core/dist/esm/index.js | @ | index.js:1
  | __webpack_require__ | @ | bootstrap:19
  | ./node_modules/@patternfly/react-table/dist/esm/components/Table/ActionsColumn.js | @ | ActionsColumn.js:1
  | __webpack_require__ | @ | bootstrap:19
  | ./node_modules/@patternfly/react-table/dist/esm/components/Table/index.js | @ | index.js:1
  | __webpack_require__ | @ | bootstrap:19
  | ./node_modules/@patternfly/react-table/dist/esm/components/index.js | @ | index.js:1
  | __webpack_require__ | @ | bootstrap:19
  | ./node_modules/@patternfly/react-table/dist/esm/index.js | @ | index.js:1
  | __webpack_require__ | @ | bootstrap:19
  | ./pkg/machines/components/vmDisksTab.jsx | @ | vmDisksTab.jsx:1
  | __webpack_require__ | @ | bootstrap:19
  | ./pkg/machines/components/vmDisksTabLibvirt.jsx | @ | vmDisksTabLibvirt.jsx:1
  | __webpack_require__ | @ | bootstrap:19
  | ./pkg/machines/components/vm/vm.jsx | @ | vm.jsx:1
  | __webpack_require__ | @ | bootstrap:19
  | ./pkg/machines/hostvmslist.jsx | @ | hostvmslist.jsx:1
  | __webpack_require__ | @ | bootstrap:19
  | ./pkg/machines/app.jsx | @ | machines.js:294799
  | __webpack_require__ | @ | bootstrap:19
  | ./pkg/machines/index.js | @ | machines.js:305312
  | __webpack_require__ | @ | bootstrap:19
  | 5 | @ | machines.js:309979
  | __webpack_require__ | @ | bootstrap:19
  | (anonymous) | @ | bootstrap:83
  | (anonymous) | @ | bootstrap:83

Looks like that some of your component are injecting styles into the html.

[kkoukiou@sequoia patternfly-react]$ git grep 'inject()'
packages/patternfly-4/react-core/src/layouts/Toolbar/Toolbar.tsx:toolbarCss.inject();
packages/patternfly-4/react-core/src/layouts/Toolbar/ToolbarGroup.tsx:toolbarCss.inject();
packages/patternfly-4/react-core/src/layouts/Toolbar/ToolbarItem.tsx:toolbarCss.inject();
packages/patternfly-4/react-core/src/layouts/Toolbar/ToolbarSection.tsx:toolbarCss.inject();
packages/patternfly-4/react-inline-edit-extension/src/components/InlineEdit/ConfirmButtons.js:inlineEditCss.inject();
packages/patternfly-4/react-inline-edit-extension/src/components/InlineEdit/editableRowWrapper.js:inlineEditCss.inject();
packages/patternfly-4/react-styles/README.md:styles.inject();
packages/patternfly-4/react-styles/src/StyleSheet.d.ts:  inject(): void;
packages/patternfly-4/react-styles/src/StyleSheet.js:      // style.__inject();
packages/patternfly-4/react-styles/src/utils.d.ts:  __inject(): void;
packages/patternfly-4/react-styles/src/utils.js:    __inject() {
packages/patternfly-4/react-topology/src/components/TopologyControlBar/TopologyControlBar.tsx:topologyControlbarCss.inject();
packages/patternfly-4/react-topology/src/components/TopologySideBar/TopologySideBar.tsx:topologySideBarCss.inject();
packages/patternfly-4/react-topology/src/components/TopologyView/TopologyView.tsx:topologyViewCss.inject();

Can we use css for that?

@priley86
Copy link
Member

Interesting... thanks for this!

Since you are referencing the table, I took a quick glance at this (although I realize this is a global thing related to current Css-In-JS libs). It looks like it may be easiest to add support for this by using Emotion's nonce prop. We'd have to do that in @patternfly/react-styles. What do you think?

Related issues:
threepointone/glamor#323
emotion-js/emotion#403
emotion-js/emotion#464

Emotion nonce:
https://github.com/emotion-js/emotion/blob/e549891a4171b5fdef1d9cbe5426f69ae8048788/packages/sheet/src/index.js#L68

@rachael-phillips
Copy link
Contributor

Hi @KKoukiou is this a blocking issue for you?

@dgutride
Copy link
Member

@priley86 - we should be avoiding use of the inject function within react styles wherever possible. Toolbar is being replaced by work happening now (it hasn't made it to react yet), but as for topology and the table packages, those should be importing from a css file so we don't use a style tag. cc: @jeff-phillips-18 since topology is mentioned, too

@priley86
Copy link
Member

@dgutride - ok, good to know. Yes i seem to recall this now (that we can avoid inject here). Thanks.

@priley86
Copy link
Member

priley86 commented Jul 29, 2019

@dgutride do you have a suggestion as far as inline css goes? I believe the InlineEdit CSS is currently only impacting those consuming the @patternfly/react-inline-edit-extension w.r.t. to tables... We can surely change this usage in a future PR...

@KKoukiou
Copy link
Collaborator Author

Hi @KKoukiou is this a blocking issue for you?

Oh yes, Cockpit has strict CDP, we don't allow inline-styling.

In addition, this issue does not appear only when consuming @patternfly/react-inline-edit-extension, in fact I don't even have this installed.

As I mentioned in the issue description, I see this when importing the Table component from patternfly-react-table. But I it's happening with other components from @patternfly/react-core.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants