#226 Add WYSIWYG editor and solve urllib3/requests dependency error
Merged a month ago by jflory7. Opened 2 months ago by shraddhaag.
fedora-commops/ shraddhaag/fedora-happiness-packets WYSIWYG  into  master

@@ -12,6 +12,7 @@ 

  from django.db.models import Q

  from django.utils import timezone

  from email_normalize import normalize

+ import bleach

  

  from .models import Message, strip_email

  

@@ -75,7 +76,37 @@ 

              return True

          else:

              return False

-             

+     

+     def clean_message(self): 

+         """ Cleans given HTML with bleach.clean() """

+ 

+         allowed_tags = set(bleach.ALLOWED_TAGS + [ 

+             'a', 'blockquote', 'code', 'del', 'dd', 'dl', 'dt', 

+             'h1', 'h2', 'h3', 'h3', 'h4', 'h5', 'i', 'img', 'kbd', 

+             'li', 'ol', 'ul', 'p', 'pre', 's', 'sup', 'sub', 'em', 

+             'strong', 'strike', 'ul', 'br', 'hr', ]) 

+ 

+         allowed_styles = set(bleach.ALLOWED_STYLES + [ 

+             'color', 'background-color', 'font', 'font-weight', 

+             'height', 'max-height', 'min-height', 

+             'width', 'max-width', 'min-width', ]) 

+ 

+         allowed_attributes = {}   

+         allowed_attributes.update(bleach.ALLOWED_ATTRIBUTES) 

+         allowed_attributes.update({ 

+             '*': ['class', 'title'], 

+             'a': ['href', 'rel'], 

+             'img': ['alt', 'src', 'width', 'height', 'align', 'style', 'max-width'], 

+         }) 

+         html = self.cleaned_data['message']

+         return bleach.clean(

+             html, 

+             strip=True,

+             tags=allowed_tags, 

+             attributes=allowed_attributes, 

+             styles=allowed_styles

+         ) 

+     

      def clean(self):

          super(MessageSendForm, self).clean()

          isREEqualsSE = self.is_recipient_email_equals_sender_email()

@@ -0,0 +1,19 @@ 

+ # Generated by Django 2.0 on 2019-05-08 11:53

+ 

+ import ckeditor.fields

+ from django.db import migrations

+ 

+ 

+ class Migration(migrations.Migration):

+ 

+     dependencies = [

+         ('messaging', '0006_message_recipient_username'),

+     ]

+ 

+     operations = [

+         migrations.AlterField(

+             model_name='message',

+             name='message',

+             field=ckeditor.fields.RichTextField(),

+         ),

+     ] 

\ No newline at end of file

@@ -10,6 +10,7 @@ 

  from django.utils.crypto import salted_hmac

  from model_utils import Choices

  from model_utils.models import TimeStampedModel

+ from ckeditor.fields import RichTextField

  

  from happinesspackets.utils.misc import readable_random_token

  from happinesspackets.tasks import send_html_mail

@@ -40,7 +41,7 @@ 

      recipient_email_stripped = models.CharField(max_length=255)

      recipient_email_token = models.CharField(max_length=255, db_index=True)

  

-     message = models.TextField()

+     message = RichTextField()

  

      sender_named = models.BooleanField(default=False)

      sender_approved_public = models.BooleanField(default=False)

@@ -11,6 +11,18 @@ 

  

  PROJECT_DIR = Path(__file__).ancestor(3)

  

+ # CKEditor configurations

+ CKEDITOR_ALLOW_NONIMAGE_FILES = False

+ 

+ CKEDITOR_CONFIGS = {

+     'default': {

+         'removePlugins':'smiley',

+         'extraPlugins': 'stylesheetparser',

+         'width': 'auto',

+         'contentsCss': 'html, iframe, body, img {max-width:100%;}',

+     },

+ }

+ 

  # For clean_pyc to work without complaining

  BASE_DIR = PROJECT_DIR

  

@@ -64,7 +76,6 @@ 

  MIDDLEWARE = [

      'django.contrib.sessions.middleware.SessionMiddleware',

      'happinesspackets.utils.middleware.SetRemoteAddrFromForwardedFor',

-     'opbeat.contrib.django.middleware.OpbeatAPMMiddleware',

      'dogslow.WatchdogMiddleware',

      'django.middleware.common.CommonMiddleware',

      'django.middleware.csrf.CsrfViewMiddleware',

@@ -118,6 +129,7 @@ 

      'haystack',

      'happinesspackets.messaging',

      'djcelery_email',

+     'ckeditor',

  ]

  

  

@@ -20,8 +20,6 @@ 

      }

  }

  

- INSTALLED_APPS.append('opbeat.contrib.django')

- 

  TEMPLATES[0]['OPTIONS']['loaders'] = (

      ('django.template.loaders.cached.Loader', (

          'django.template.loaders.filesystem.Loader',

@@ -88,3 +88,4 @@ 

      secrets = json.load(f)

      ADMIN_USERNAME = secrets["ADMIN_USERNAME"]

      ADMIN_PASSWORD =  secrets["ADMIN_PASSWORD"]

+ 

file modified
+7 -2

@@ -28,15 +28,16 @@ 

  pep8==1.7.0

  

  # Misc

+ urllib3==1.24.2       # Dependency for requests 

  python-dateutil==2.5.0

  factory-boy==2.9.2

- opbeat==3.6.1

- mozilla-django-oidc==1.2.1

+ mozilla-django-oidc==1.2.2

  fedora-messaging>=1.4.0

  happinesspacket-schema>=0.1.2

  celery[redis]==4.2.1

  django-celery-email==2.0.1

  psycopg2==2.7.5 # PostgreSQL driver

+ bleach>=3.1.0

  

  # Search engine

  Whoosh==2.7.4

@@ -50,3 +51,7 @@ 

  

  # Dependency for YAML file

  pyyaml==5.1

+ 

+ # WYSIWYG addition

+ django-ckeditor==5.7.0

+ 

t
file modified
-1

@@ -1,3 +1,2 @@ 

  export DJANGO_SETTINGS_MODULE=happinesspackets.settings.tsting &&

- OPBEAT_DISABLE_SEND=true coverage run ./manage.py test $@ &&

  coverage report --fail-under=100

@@ -7,7 +7,7 @@ 

          {% endif %}

      </h4>

  <blockquote>

-   {{ message.message|linebreaksbr }}

+   {{ message.message|safe }}

  </blockquote>

  {% endfor %} {% block pagination %} {% if is_paginated %}

  {% include 'messaging/_pagination.html' %}

@@ -16,7 +16,7 @@ 

          Your Happiness Packet contains:

          </p>

  

-     <blockquote>{{ message.message|linebreaksbr }}</blockquote>

+     <blockquote>{{ message.message|safe }}</blockquote>

  

      {% crispy form %}

  {% endblock content %}

@@ -3,6 +3,9 @@ 

  {% block extra_head %}<title>Send a Happiness Packet</title>{% endblock %}

  

  {% load crispy_forms_tags %}

+ {% load static %}

+ <script type="text/javascript" src="{% static "ckeditor/ckeditor-init.js" %}"></script>

+ <script type="text/javascript" src="{% static "ckeditor/ckeditor/ckeditor.js" %}"></script>

  

  {% block content %}

  

@@ -13,4 +16,11 @@ 

  {% crispy form %}

  

  <button id="search_fasid">Search</button>

+ 

+ <script> 

+ $("#submit-id-submit").click(function() {

+                 $(".cke_wysiwyg_frame").contents().find("img")

+                     .css({"max-width": "100%", "height": "auto"});

+ });

+ </script>

  {% endblock content %} 

\ No newline at end of file

@@ -20,7 +20,7 @@ 

          {% endif %}

      </h4>

      <blockquote>

-       {{ message.message|linebreaksbr }}

+       {{ message.message|safe }}

Just curious, what does this change?

      </blockquote>

      {% endfor %}

  

@@ -19,7 +19,7 @@ 

  

  <p>Your Happiness Packet contains:</p>

  

- <p><em>{{ message.message|linebreaksbr }}</em></p>

+ <p><em>{{ message.message|safe }}</em></p>

  

  <p>

      If you and the sender of the Happiness Packet both agree, we'd love to publish the message to our

@@ -12,7 +12,7 @@ 

      Your message reads:

  </p>

  

- <p><em>{{ message.message|linebreaksbr }}</em></p>

+ <p><em>{{ message.message|safe }}</em></p>

  

  <p>

      To confirm and send your message, click or copy this link to a web browser:

@@ -5,7 +5,7 @@ 

  Your message reads: 

  

  ---------------------

- {{ message.message }}

+ {{ message.message|safe }}

  ---------------------

  

  To confirm and send your message, click or copy this link to a web browser:

@@ -17,7 +17,7 @@ 

              Sent to {{ message.recipient_name }}

      </h4>

      <blockquote>

-       {{ message.message|linebreaksbr }}

+       {{ message.message|safe }}

      </blockquote>

      {% endfor %}

  

This commit includes the following:

  1. Adds a WYSIWYG text editor using django-ckeditor.
  2. Resolve urllib3 dependency error requests 2.21.0 has requirement urllib3<1.25,>=1.21.1, but you'll have urllib3 1.25.2 which is incompatible.

The usage of all the formatting options is tested on email. The output looks like this on the website and same on email as well.

Metadata Update from @jflory7:
- Pull-request tagged with: PASSED, needs testing, new change, type - frontend, type - summer coding

2 months ago

Metadata Update from @jflory7:
- Request assigned

a month ago

A few weeks ago, CVE-2019-11324 was opened on urllib versions 1.24.1 and earlier. Please bump this dependency to 1.24.2 to patch the CVE.

I don't know if this is standard or not, but I prefer to put variables outside of a method after import statements at the top of the module. Since these are used for configuration, it would help to group all related configuration values at the top of the file for easier reading. You could group it with the PROJECT_DIR variable on line 12.

Metadata Update from @jflory7:
- Pull-request untagged with: needs testing
- Pull-request tagged with: needs changes

a month ago

Hi @shraddhaag, nice work! :thumbsup: It's super exciting to see the WYSIWYG editor!

I left some feedback in-line, but I have a few other comments:

  1. Happiness Archive does not display rich text formatting
  2. Editor window should fit user's screen size (not sure about mobile devices?)
  3. Embedded images should always adjust for the user's screen size in email and Happiness Archive / confirmation screen

Screenshot examples of each are below:

Comment 1: Happiness Archive does not render rich-text formatting:

Comment 1: Happiness Archive does not render rich-text formatting

Comment 2: Editor window goes off-screen when sending a new Happiness Packet:

Comment 2: Editor window goes off-screen when sending a new Happiness Packet

Do you think you can figure these out? If we can make these improvements, the PR is ready to go. :grinning:

rebased onto 114b245

a month ago

@jflory7 I integrated all the changes. Please let me know if I need to change anything else :)

Just curious, what does this change?

By default HTML is not escaped and so is displayed as text. We need to mark the field as safe so that Django renders it as HTML. It's detailed here in the documentation.

@shraddhaag It's generally not advisable to mark text as safe in Django if it is coming from a user input.

YOu can

From a security standpoint, a user can inject <script> elements into the input and this can lead to Cross Side Scripting

AFAIK, Django does not have any method by which you can render HTML and protect XSS.

You might be interested in Mozilla Bleach for this

rebased onto 816e744

a month ago

Hi @skamath! I've updated the PR with your suggestions. Please let me know if I need to make anymore changes :)

rebased onto c826e84

a month ago

rebased onto a1ed1b2

a month ago

Metadata Update from @jflory7:
- Pull-request untagged with: needs changes
- Pull-request tagged with: needs review
- Request assigned

a month ago

I re-tested and everything looks good. :thumbsup: I think this is ready to go, but I'll let @skamath give a final review + merge.

@shraddhaag Excellent work!! :100: :100:

This PR LGTM, however I am unable to merge this. Maybe a permission issue.

1 new commit added

  • Remove opbeat as a requirement
a month ago

@skamath Oops, your ACL's weren't updated. They are now. :smile:

I'm going to go ahead and merge this. Thanks @shraddhaag! :clapper:

Pull-Request has been merged by jflory7

a month ago

Metadata Update from @skamath:
- Request assignee reset

a month ago