django / django-contrib-comments

BSD 3-Clause "New" or "Revised" License
614 stars 196 forks source link

CSP compliance #182

Closed martin-lueders closed 2 years ago

martin-lueders commented 2 years ago

While trying to tighten the security measures of a web page, I am working on, using the django CSP middleware, I noticed issues with django_comments:

In order to 'hide' the honeypot fields, the templates use inline styles, e.g. form.html:

{% load comments i18n %}
<form action="{% comment_form_target %}" method="post">{% csrf_token %}
  {% if next %}
    <div><input type="hidden" name="next" value="{{ next }}"/></div>{% endif %}
  {% for field in form %}
    {% if field.is_hidden %}
      <div>{{ field }}</div>
    {% else %}
      {% if field.errors %}{{ field.errors }}{% endif %}
      <p
              {% if field.errors %} class="error"{% endif %}
              {% if field.name == "honeypot" %} style="display:none;"{% endif %}>
        {{ field.label_tag }} {{ field }}
      </p>
    {% endif %}
  {% endfor %}
  <p class="submit">
    <input type="submit" name="post" class="submit-post" value="{% trans "Post" %}"/>
    <input type="submit" name="preview" class="submit-preview" value="{% trans "Preview" %}"/>
  </p>
</form>

These inline styles cause the messages:

Refused to apply inline style because it violates the following Content Security Policy directive: "style-src 'self'". Either the 'unsafe-inline' keyword, a hash ('sha256-0EZqoz+oBhx7gF4nvY2bSqoGyy4zLjNF+SDQXGp/ZrY='), or a nonce ('nonce-...') is required to enable inline execution.

As the suggested measures (unsafe-inline, hash or nonce) are discouraged, the best solution would be to use a css style-sheet, which defines a class (e.g. comments-no-display), which then can be set in the template. It only would require to load that css sheet in the base template, but would allow for tighter CSP settings.

Would it be possible to include this in the next release?

claudep commented 2 years ago

What about replacing the style="display:none;" by the hidden attribute? It seems to be widely implemented now (https://caniuse.com/?search=hidden).

martin-lueders commented 2 years ago

@claudep Yes, I guess that also would possible and does not required the extra style-sheet. Indeed it seems to be widely implemented. I just tried this in my project, and it works for me, but I have not tested on a wide variety of browsers.

claudep commented 2 years ago

Would you like to suggest a patch with this change?

martin-lueders commented 2 years ago

Good idea. Attached are the two patch files patches.tar.gz .