feat(checkboxes): Added checkbox support for Markdown.

[michaelkleinhenz]

Adds checkbox support for Markdown.

[nimishamukherjee]

@michaelkleinhenz what should checked="false" do ?
Checked Item.

[michaelkleinhenz]

Leftover. I'll remove that. :-)

[nimishamukherjee]

LGTM

[dlabrecq]

Could be wrong, but this object appears to disable/bypass Angular's built-in sanitation? If so, bypassing JavaScript sanitation isn't a good idea.

I'm concerned we're converting user input into a trusted value and introducing a security vulnerability by trusting values that could be malicious?

See: https://angular.io/guide/security#bypass-security-apis
"But be careful. If you trust a value that might be malicious, you are introducing a security vulnerability into your application. If in doubt, find a professional security reviewer."

[michaelkleinhenz]

See comment below. Note: JS sanitizing is still enabled.

[joshuawilson]

@michaelkleinhenz does your new PR address the issue that @dlabrecq raised?

[michaelkleinhenz]

@joshuawilson @dlabrecq yes and no. The wrapping in SafeValue is now happening outside of ngx-widgets in the enclosing component (namely the Planner detail view). ngx-widgets now only just supports incoming SafeValue content. All sanitizer handling has been removed from ngx-widgets.

Although the problem still persists, because we still need to use SafeValue in Planner to wrap the rendered markdown. The Angular sanitizer can not be configured on a fine granular (element) basis (like the golang sanitizer can). So we either need to disable HTML sanitizing entirely for that value or we can not support any feature that needs input elements in the HTML. Note that the HTML sanitizing can be individually turned off, JavaScript and URLs are still stripped from the HTML.

Although this is still not ideal, the impact there is low. We're only turning off HTML sanitizing, other active components are still stripped. The HTML comes from a trusted source and is not directly user-editable but is always generated from Markdown. Also, the generated HTML is sanitized on the server side as well in a much more restricted way (only allowing input elements in addition to the standard set of allowed elements).

[dlabrecq]

I confirmed the github-link-area component is still bypassing HTML, while the markdown example is bypassing HTML, styles, JavaScript, URLs, etc.

Is the intention to simply move the bypass code (in the example) to Planner (i.e., sanitized server-side)?

./app/github-link-area/github-link-area.component.ts:      return this.sanitizer.bypassSecurityTrustHtml(input);
./app/markdown/examples/markdown-example.component.ts:      case 'html': return this.sanitizer.bypassSecurityTrustHtml(value);
./app/markdown/examples/markdown-example.component.ts:      case 'style': return this.sanitizer.bypassSecurityTrustStyle(value);
./app/markdown/examples/markdown-example.component.ts:      case 'script': return this.sanitizer.bypassSecurityTrustScript(value);
./app/markdown/examples/markdown-example.component.ts:      case 'url': return this.sanitizer.bypassSecurityTrustUrl(value);
./app/markdown/examples/markdown-example.component.ts:      case 'resourceUrl': return this.sanitizer.bypassSecurityTrustResourceUrl(value);
./app/markdown/examples/markdown-example.component.ts:      callBack(rawText, this.sanitizer.bypassSecurityTrustHtml(text));
./app/markdown/markdown.component.spec.ts:          comp.inpRenderedText = domSanitizer.bypassSecurityTrustHtml(originalHTML);

[joshuawilson]

@dlabrecq are you ok with these updates?

[dlabrecq]

Yes. As long as the JavaScript and URLs are still stripped from the HTML, I'm fine with the markup being sanitized server-side.