lucyparsons · abandoned-prototype · Oct 10, 2021 · Sep 30, 2021 · Sep 30, 2021 · Sep 30, 2021
diff --git a/OpenOversight/app/__init__.py b/OpenOversight/app/__init__.py
@@ -3,6 +3,8 @@
 from logging.handlers import RotatingFileHandler
 import os
 
+import bleach
+from bleach_allowlist import markdown_tags, markdown_attrs
 from flask import Flask, render_template
 from flask_bootstrap import Bootstrap
 from flask_limiter import Limiter
@@ -12,6 +14,8 @@
 from flask_migrate import Migrate
 from flask_sitemap import Sitemap
 from flask_wtf.csrf import CSRFProtect
+import markdown as _markdown
+from markupsafe import Markup
 
 from .config import config
 
@@ -95,6 +99,11 @@ def get_age_from_birth_year(birth_year):
         if birth_year:
             return int(datetime.datetime.now().year - birth_year)
 
+    @app.template_filter('markdown')
+    def markdown(text):
+        html = bleach.clean(_markdown.markdown(text), markdown_tags, markdown_attrs)
+        return Markup(html)
+
     # Add commands
     Migrate(app, db, os.path.join(os.path.dirname(__file__), '..', 'migrations'))  # Adds 'db' command
     from .commands import (make_admin_user, link_images_to_department,

diff --git a/OpenOversight/app/templates/incident_detail.html b/OpenOversight/app/templates/incident_detail.html
@@ -46,8 +46,7 @@ <h1>Incident {% if incident.report_number %}{{incident.report_number}}{% endif %
 				{% endwith %}
 			</div>
 			<div class="col-sm-12 col-md-6">
-				<h5>Description</h5>
-				<p>{{ incident.description }}</p>
+				{{ incident.description | markdown}}
 			</div>
 		</div>
 		{% include 'partials/links_and_videos_row.html' %}

diff --git a/OpenOversight/app/templates/officer.html b/OpenOversight/app/templates/officer.html
@@ -394,7 +394,7 @@ <h3>Descriptions</h3>
           {% for description in officer.descriptions %}
           <li class="list-group-item">
                 <em>{{ description.date_updated.strftime('%b %d, %Y')}}</em> <br/>
-                {{ description.text_contents }} <br/>
+                {{ description.text_contents | markdown }} 
                 <em>{{ description.creator.username }} </em>
              {% if description.creator_id == current_user.get_id() or current_user.is_administrator %}
                 <a href="{{ url_for('main.description_api', officer_id=officer.id,
@@ -426,7 +426,7 @@ <h3>Notes</h3>
         {% for note in officer.notes %}
           <li class="list-group-item">
               <em>{{ note.date_updated.strftime('%b %d, %Y')}}</em> <br/>
-              {{ note.text_contents }} <br/>
+              {{ note.text_contents | markdown }}
               <em>{{ note.creator.username }} </em>
             {% if note.creator_id == current_user.get_id() or current_user.is_administrator %}
               <a href="{{ url_for('main.note_api', officer_id=officer.id,

diff --git a/OpenOversight/app/templates/partials/incident_fields.html b/OpenOversight/app/templates/partials/incident_fields.html
@@ -42,7 +42,7 @@
 			<tr>
 				<td><strong>Description</strong></td>
 				<td class="incident-description" id="incident-description_{{incident.id}}" data-incident='{{incident.id | tojson}}'>
-					{{ incident.description }}
+					{{ incident.description | markdown}}
 				</td>
 			</tr>
 			<tr id="description-overflow-row_{{ incident.id }}" >

diff --git a/OpenOversight/tests/conftest.py b/OpenOversight/tests/conftest.py
@@ -121,20 +121,24 @@ def build_assignment(officer: Officer, units: List[Unit], jobs: Job):
                              resign_date=pick_date(officer.full_name().encode('utf-8')))
 
 
-def build_note(officer, user):
+def build_note(officer, user, content=None):
     date = factory.date_time_this_year()
+    if content is None:
+        content = factory.text()
     return models.Note(
-        text_contents=factory.text(),
+        text_contents=content,
         officer_id=officer.id,
         creator_id=user.id,
         date_created=date,
         date_updated=date)
 
 
-def build_description(officer, user):
+def build_description(officer, user, content=None):
     date = factory.date_time_this_year()
+    if content is None:
+        content = factory.text()
     return models.Description(
-        text_contents=factory.text(),
+        text_contents=content,
         officer_id=officer.id,
         creator_id=user.id,
         date_created=date,
@@ -403,7 +407,7 @@ def add_mockdata(session):
             date=datetime.date(2016, 3, 16),
             time=datetime.time(4, 20),
             report_number='42',
-            description='A thing happened',
+            description='### A thing happened\n **Markup** description',
             department_id=1,
             address=test_addresses[0],
             license_plates=test_license_plates,
@@ -445,7 +449,7 @@ def add_mockdata(session):
 
     # for testing routes
     first_officer = models.Officer.query.get(1)
-    note = build_note(first_officer, test_admin)
+    note = build_note(first_officer, test_admin, "### A markdown note\nA **test** note!")
     session.add(note)
     for officer in models.Officer.query.limit(20):
         user = random.choice(users_that_can_create_notes)
@@ -458,7 +462,7 @@ def add_mockdata(session):
 
     # for testing routes
     first_officer = models.Officer.query.get(1)
-    description = build_description(first_officer, test_admin)
+    description = build_description(first_officer, test_admin, "### A markdown description\nA **test** description!")
     session.add(description)
     for officer in models.Officer.query.limit(20):
         user = random.choice(users_that_can_create_descriptions)

diff --git a/OpenOversight/tests/routes/test_descriptions.py b/OpenOversight/tests/routes/test_descriptions.py
@@ -31,6 +31,41 @@ def test_route_admin_or_required(route, client, mockdata):
         assert rv.status_code == 403
 
 
+def test_officer_descriptions_markdown(mockdata, client, session):
+    with current_app.test_request_context():
+        login_user(client)
+        rv = client.get(url_for('main.officer_profile', officer_id=1))
+        assert rv.status_code == 200
+        html = rv.data.decode()
+        print(html)
+        assert "<h3>A markdown description</h3>" in html
+        assert "<p>A <strong>test</strong> description!</p>" in html
+
+
+def test_admins_cannot_inject_unsafe_html(mockdata, client, session):
+    with current_app.test_request_context():
+        login_admin(client)
+        officer = Officer.query.first()
+        text_contents = 'New description\n<script>alert();</script>'
+        admin = User.query.filter_by(email='jen@example.org').first()
+        form = TextForm(
+            text_contents=text_contents,
+            officer_id=officer.id,
+            creator_id=admin.id
+        )
+
+        rv = client.post(
+            url_for('main.description_api', officer_id=officer.id),
+            data=form.data,
+            follow_redirects=True
+        )
+
+        assert rv.status_code == 200
+        assert 'created' in rv.data.decode('utf-8')
+        assert "<script>" not in rv.data.decode()
+        assert "&lt;script&gt;" in rv.data.decode()
+
+
 def test_admins_can_create_descriptions(mockdata, client, session):
     with current_app.test_request_context():
         login_admin(client)

diff --git a/OpenOversight/tests/routes/test_incidents.py b/OpenOversight/tests/routes/test_incidents.py
@@ -601,3 +601,57 @@ def test_form_with_officer_id_prepopulates(mockdata, client, session):
         officer_id = '1234'
         rv = client.get(url_for('main.incident_api') + 'new?officer_id={}'.format(officer_id))
         assert officer_id in rv.data.decode('utf-8')
+
+
+def test_incident_markdown(mockdata, client, session):
+    with current_app.test_request_context():
+        rv = client.get(url_for('main.incident_api'))
+        html = rv.data.decode()
+        assert "<h3>A thing happened</h3>" in html
+        assert "<p><strong>Markup</strong> description</p>" in html
+
+
+def test_admins_cannot_inject_unsafe_html(mockdata, client, session):
+    with current_app.test_request_context():
+        login_admin(client)
+        inc = Incident.query.options(
+            joinedload(Incident.links),
+            joinedload(Incident.license_plates),
+            joinedload(Incident.officers)).first()
+
+        address_form = LocationForm(
+            street_name=inc.address.street_name,
+            cross_street1=inc.address.cross_street1,
+            cross_street2=inc.address.cross_street2,
+            city=inc.address.city,
+            state=inc.address.state,
+            zip_code=inc.address.zip_code
+        )
+        links_forms = [LinkForm(url=link.url, link_type=link.link_type).data for link in inc.links]
+        license_plates_forms = [LicensePlateForm(number=lp.number, state=lp.state).data for lp in inc.license_plates]
+
+        old_officer_ids = [officer.id for officer in inc.officers]
+        ooid_forms = [OOIdForm(oo_id=the_id) for the_id in old_officer_ids]
+
+        form = IncidentForm(
+            date_field=str(inc.date),
+            time_field=str(inc.time),
+            report_number=inc.report_number,
+            description="<script>alert();</script>",
+            department='1',
+            address=address_form.data,
+            links=links_forms,
+            license_plates=license_plates_forms,
+            officers=ooid_forms
+        )
+        data = process_form_data(form.data)
+
+        rv = client.post(
+            url_for('main.incident_api', obj_id=inc.id) + '/edit',
+            data=data,
+            follow_redirects=True
+        )
+        assert rv.status_code == 200
+        assert 'successfully updated' in rv.data.decode('utf-8')
+        assert "<script>" not in rv.data.decode()
+        assert "&lt;script&gt;" in rv.data.decode()
diff --git a/OpenOversight/tests/routes/test_notes.py b/OpenOversight/tests/routes/test_notes.py
@@ -31,6 +31,40 @@ def test_route_admin_or_required(route, client, mockdata):
         assert rv.status_code == 403
 
 
+def test_officer_notes_markdown(mockdata, client, session):
+    with current_app.test_request_context():
+        login_admin(client)  # need to be admin or AC to see notes
+        rv = client.get(url_for('main.officer_profile', officer_id=1))
+        assert rv.status_code == 200
+        html = rv.data.decode()
+        assert "<h3>A markdown note</h3>" in html
+        assert "<p>A <strong>test</strong> note!</p>" in html
+
+
+def test_admins_cannot_inject_unsafe_html(mockdata, client, session):
+    with current_app.test_request_context():
+        login_admin(client)
+        officer = Officer.query.first()
+        text_contents = 'New note\n<script>alert();</script>'
+        admin = User.query.filter_by(email='jen@example.org').first()
+        form = TextForm(
+            text_contents=text_contents,
+            officer_id=officer.id,
+            creator_id=admin.id
+        )
+
+        rv = client.post(
+            url_for('main.note_api', officer_id=officer.id),
+            data=form.data,
+            follow_redirects=True
+        )
+
+        assert rv.status_code == 200
+        assert 'created' in rv.data.decode('utf-8')
+        assert "<script>" not in rv.data.decode()
+        assert "&lt;script&gt;" in rv.data.decode()
+
+
 def test_admins_can_create_notes(mockdata, client, session):
     with current_app.test_request_context():
         login_admin(client)

diff --git a/mypy.ini b/mypy.ini
@@ -55,3 +55,9 @@ ignore_missing_imports = True
 
 [mypy-us.*]
 ignore_missing_imports = True
+
+[mypy-bleach_allowlist.*]
+ignore_missing_imports = True
+
+[mypy-markdown.*]
+ignore_missing_imports = True
diff --git a/requirements.txt b/requirements.txt
@@ -20,3 +20,6 @@ itsdangerous==0.24
 us==1.0
 future==0.18.2
 email-validator==1.0.5
+bleach==3.3.1
+bleach-allowlist==1.0.3
+Markdown==3.2.2