Content security policy violation

[musaffa]

The presence of attributeBindings: ['style'] in bs-collapse and bs-progress-bar violates content security policy of style-src for being unsafe-inline.

[simonihmig]

Yes, this is a known limitation. Should probably be documented better. Currently I have no better solution than to allow inline styles.

[musaffa]

Bootstrap javascript modules don't use inline styling then why should ember-bootstrap components? It makes the whole code base unsafe.

[simonihmig]

Bootstrap uses inline styling at the same places ember-bootstrap does, but does so programmatically using jQuery's .css(). This does not violate CSP as the change is coming from JavaScript and not from inserting some static HTML with a style attribute, which is what happens when you Ember renders a template/component with style={{style}} or using attributeBindings: ['style'].

This could be fixed by using jQuery for all inline styles, but that does not feel to be the right way of doing things in ember, and would not work in Fastboot. See also this thread: http://discuss.emberjs.com/t/binding-style-attribute-and-content-security-policy-best-practice/10921

This is a general problem in ember, not something specific to e-b. Would be glad if there would be an easy to use and elegant solution at sometime!

[musaffa]

If bs-collapse sets style to null by default, then the style attribute won't be rendered in the template. If a value is passed for the style attribute, it will use that value instead of null. To achieve that, we need to return null from this computed property. This solves the problem while keeping the functionality of style attribute intact.

[simonihmig]

Yes, sure, but we need to set the style to set the height of the element. Always returning null will remove the style but also never set the height!?

[musaffa]

Here's the computed property right now:

style: computed('collapseSize', function() {
  let size = this.get('collapseSize');
  let dimension = this.get('collapseDimension');
  if (Ember.isEmpty(size)) {
    return Ember.String.htmlSafe('');
  }
  return Ember.String.htmlSafe(`${dimension}: ${size}px`);
})

If collapseSize isn't present then we return an empty string. We can change this line to look like below:

style: computed('collapseSize', function() {
  let size = this.get('collapseSize');
  let dimension = this.get('collapseDimension');
  if (Ember.isEmpty(size)) {
    return null;
  }
  return Ember.String.htmlSafe(`${dimension}: ${size}px`);
})

There will be three scenarios in this case:

1. It will return null by default. So style attribute wont be rendered during the template compilation.
2. If a value for collapseSize is passed, it will return the height and style will be rendered.
3. If a value for style is passed, it will take precedence over the computed property and style will be rendered.

Scenario 1 means, the style-src is safe by default. Scenario 2 and 3 means unsafe-inline styling is also possible but at the developer's own risk.

[simonihmig]

Scenario 1: collapseSize will be set when showing the content here: https://github.com/kaliber5/ember-bootstrap/blob/master/addon/components/bs-collapse.js#L151. This is needed for the show transition. There is no way to have a working collapse component without setting the height, so we do need to set the style attribute. (unless there is a completely different implementation based on jQuery, see my second comment)

Scenario 2+3 do not really apply, as collapseSize and style are not public properties that can be set from the consumer of the component.

[musaffa]

Scenario 1 is more complicated than I thought.

[jelhan]

@simonihmig: Are you sure we encounter an issue in ember-fastboot if changing CSSOM directly? I'm asking cause this is how adjustFeedbackIcons of FormElement works. https://github.com/kaliber5/ember-bootstrap/blob/master/addon/components/base/bs-form/element.js#L842-L882 #98

[simonihmig]

@jelhan depends on how/when this code is executed. When this is only triggered by lifecycle hooks like didInsertElement that are disabled when running in FastBoot, this should be ok! See https://ember-fastboot.com/docs/user-guide#lifecycle-hooks-in-fastboot

[simonihmig]

This has been fixed by #746 meanwhile.