Adhoc/security patch (#873)

* csrf fixes

* fix tests and formatting

OpenOversight/app/__init__.py

@@ -11,6 +11,7 @@
 from flask_mail import Mail
 from flask_migrate import Migrate
 from flask_sitemap import Sitemap
+from flask_wtf.csrf import CSRFProtect
 
 from .config import config
 
@@ -26,6 +27,7 @@
                   default_limits=["100 per minute", "5 per second"])
 
 sitemap = Sitemap()
+csrf = CSRFProtect()
 
 
 def create_app(config_name='default'):
@@ -40,6 +42,7 @@ def create_app(config_name='default'):
     login_manager.init_app(app)
     limiter.init_app(app)
     sitemap.init_app(app)
+    csrf.init_app(app)
 
     from .main import main as main_blueprint  # noqa
     app.register_blueprint(main_blueprint)

OpenOversight/app/auth/forms.py

@@ -88,7 +88,11 @@ class EditUserForm(Form):
     ac_department = QuerySelectField('Department', validators=[Optional()],
                                      query_factory=dept_choices, get_label='name', allow_blank=True)
     is_administrator = BooleanField('Is administrator?', false_values={'False', 'false', ''})
-    submit = SubmitField(label='Update')
+    is_disabled = BooleanField('Disabled?', false_values={'False', 'false', ''})
+    approved = BooleanField('Approved?', false_values={'False', 'false', ''})
+    submit = SubmitField(label='Update', false_values={'False', 'false', ''})
+    resend = SubmitField(label='Resend', false_values={'False', 'false', ''})
+    delete = SubmitField(label='Delete', false_values={'False', 'false', ''})
 
     def validate(self):
         success = super(EditUserForm, self).validate()

OpenOversight/app/auth/views.py

@@ -1,7 +1,4 @@
-from future.utils import iteritems
-
 from flask import render_template, redirect, request, url_for, flash, current_app
-from flask.views import MethodView
 from flask_login import login_user, logout_user, login_required, \
     current_user
 from . import auth
@@ -101,7 +98,7 @@ def register():
     return render_template('auth/register.html', form=form, jsloads=jsloads)
 
 
-@auth.route('/confirm/<token>')
+@auth.route('/confirm/<token>', methods=['GET'])
 @login_required
 def confirm(token):
     if current_user.confirmed:
@@ -237,144 +234,76 @@ def change_dept():
     return render_template('auth/change_dept_pref.html', form=form)
 
 
-class UserAPI(MethodView):
-    decorators = [admin_required]
-
-    def get(self, user_id):
-        # isolate the last part of the url
-        end_of_url = request.url.split('/')[-1].split('?')[0]
+@auth.route('/users/', methods=['GET'])
+@admin_required
+def get_users():
+    if request.args.get('page'):
+        page = int(request.args.get('page'))
+    else:
+        page = 1
+    USERS_PER_PAGE = int(current_app.config['USERS_PER_PAGE'])
+    users = User.query.order_by(User.username) \
+        .paginate(page, USERS_PER_PAGE, False)
 
-        if user_id is None:
-            if request.args.get('page'):
-                page = int(request.args.get('page'))
-            else:
-                page = 1
-            USERS_PER_PAGE = int(current_app.config['USERS_PER_PAGE'])
-            users = User.query.order_by(User.email) \
-                .paginate(page, USERS_PER_PAGE, False)
+    return render_template('auth/users.html', objects=users)
 
-            return render_template('auth/users.html', objects=users)
-        else:
-            user = User.query.get(user_id)
-            if not user:
-                return render_template('403.html'), 403
-
-            actions = ['delete', 'enable', 'disable', 'resend', 'approve']
-            if end_of_url in actions:
-                action = getattr(self, end_of_url, None)
-                return action(user)
-            else:
-                form = EditUserForm(
-                    email=user.email,
-                    is_area_coordinator=user.is_area_coordinator,
-                    ac_department=user.ac_department,
-                    is_administrator=user.is_administrator)
-                return render_template('auth/user.html', user=user, form=form)
 
-    def post(self, user_id):
-        # isolate the last part of the url
-        end_of_url = request.url.split('/')[-1].split('?')[0]
+@auth.route('/users/<int:user_id>', methods=['GET', 'POST'])
+@admin_required
+def edit_user(user_id):
+    user = User.query.get(user_id)
+    if not user:
+        return render_template('404.html'), 404
 
-        user = User.query.get(user_id)
+    if request.method == 'GET':
+        form = EditUserForm(obj=user)
+        return render_template('auth/user.html', user=user, form=form)
+    elif request.method == 'POST':
         form = EditUserForm()
+        if form.delete.data:
+            # forward to confirm delete
+            return redirect(url_for('auth.delete_user', user_id=user.id))
+        elif form.resend.data:
+            return admin_resend_confirmation(user)
+        elif form.submit.data:
+            if form.validate_on_submit():
+                # prevent user from removing own admin rights (or disabling account)
+                if user.id == current_user.id:
+                    flash('You cannot edit your own account!')
+                    form = EditUserForm(obj=user)
+                    return render_template('auth/user.html', user=user, form=form)
+                form.populate_obj(user)
+                db.session.add(user)
+                db.session.commit()
+                flash('{} has been updated!'.format(user.username))
+                return redirect(url_for('auth.edit_user', user_id=user.id))
+            else:
+                flash('Invalid entry')
+                return render_template('auth/user.html', user=user, form=form)
 
-        if user and end_of_url and end_of_url == 'delete':
-            return self.delete(user)
-        elif user and form.validate_on_submit():
-            for field, data in iteritems(form.data):
-                setattr(user, field, data)
-
-            db.session.add(user)
-            db.session.commit()
-            flash('{} has been updated!'.format(user.username))
-            return redirect(url_for('auth.user_api'))
-        elif not form.validate_on_submit():
-            flash('Invalid entry')
-            return render_template('auth/user.html', user=user, form=form)
-        else:
-            return render_template('403.html'), 403
-
-    def delete(self, user):
-        if request.method == 'POST':
-            username = user.username
-            db.session.delete(user)
-            db.session.commit()
-            flash('User {} has been deleted!'.format(username))
-            return redirect(url_for('auth.user_api'))
-
-        return render_template('auth/user_delete.html', user=user)
 
-    def enable(self, user):
-        if not user.is_disabled:
-            flash('User {} is already enabled.'.format(user.username))
-        else:
-            user.is_disabled = False
-            db.session.add(user)
-            db.session.commit()
-            flash('User {} has been enabled!'.format(user.username))
-        return redirect(url_for('auth.user_api'))
+@auth.route('/users/<int:user_id>/delete', methods=['GET', 'POST'])
+@admin_required
+def delete_user(user_id):
+    user = User.query.get(user_id)
+    if not user or user.is_administrator:
+        return render_template('403.html'), 403
+    if request.method == 'POST':
+        username = user.username
+        db.session.delete(user)
+        db.session.commit()
+        flash('User {} has been deleted!'.format(username))
+        return redirect(url_for('auth.get_users'))
 
-    def disable(self, user):
-        if user.is_disabled:
-            flash('User {} is already disabled.'.format(user.username))
-        else:
-            user.is_disabled = True
-            db.session.add(user)
-            db.session.commit()
-            flash('User {} has been disabled!'.format(user.username))
-        return redirect(url_for('auth.user_api'))
+    return render_template('auth/user_delete.html', user=user)
 
-    def resend(self, user):
-        if user.confirmed:
-            flash('User {} is already confirmed.'.format(user.username))
-        else:
-            token = user.generate_confirmation_token()
-            send_email(user.email, 'Confirm Your Account',
-                       'auth/email/confirm', user=user, token=token)
-            flash('A new confirmation email has been sent to {}.'.format(user.email))
-        return redirect(url_for('auth.user_api'))
 
-    def approve(self, user):
-        if user.approved:
-            flash('User {} is already approved.'.format(user.username))
-        else:
-            user.approved = True
-            db.session.add(user)
-            db.session.commit()
-            token = user.generate_confirmation_token()
-            send_email(user.email, 'Confirm Your Account',
-                       'auth/email/confirm', user=user, token=token)
-            flash('User {} has been approved!'.format(user.username))
-        return redirect(url_for('auth.user_api'))
-
-
-user_view = UserAPI.as_view('user_api')
-auth.add_url_rule(
-    '/users/',
-    defaults={'user_id': None},
-    view_func=user_view,
-    methods=['GET'])
-auth.add_url_rule(
-    '/users/<int:user_id>',
-    view_func=user_view,
-    methods=['GET', 'POST'])
-auth.add_url_rule(
-    '/users/<int:user_id>/delete',
-    view_func=user_view,
-    methods=['GET', 'POST'])
-auth.add_url_rule(
-    '/users/<int:user_id>/enable',
-    view_func=user_view,
-    methods=['GET'])
-auth.add_url_rule(
-    '/users/<int:user_id>/disable',
-    view_func=user_view,
-    methods=['GET'])
-auth.add_url_rule(
-    '/users/<int:user_id>/resend',
-    view_func=user_view,
-    methods=['GET'])
-auth.add_url_rule(
-    '/users/<int:user_id>/approve',
-    view_func=user_view,
-    methods=['GET'])
+def admin_resend_confirmation(user):
+    if user.confirmed:
+        flash('User {} is already confirmed.'.format(user.username))
+    else:
+        token = user.generate_confirmation_token()
+        send_email(user.email, 'Confirm Your Account',
+                   'auth/email/confirm', user=user, token=token)
+        flash('A new confirmation email has been sent to {}.'.format(user.email))
+    return redirect(url_for('auth.get_users'))

OpenOversight/app/config.py

@@ -79,4 +79,4 @@ def init_app(cls, app):  # pragma: no cover
     'testing': TestingConfig,
     'production': ProductionConfig,
 }
-config['default'] = config.get(os.environ.get('FLASK_ENV'), DevelopmentConfig)
+config['default'] = config.get(os.environ.get('FLASK_ENV', ""), DevelopmentConfig)

OpenOversight/app/main/views.py

@@ -10,7 +10,7 @@
 from traceback import format_exc
 
 from flask import (abort, render_template, request, redirect, url_for,
-                   flash, current_app, jsonify, Response, Markup)
+                   flash, current_app, jsonify, Response)
 from flask_login import current_user, login_required, login_user
 
 from . import main
@@ -331,23 +331,6 @@ def edit_salary(officer_id, salary_id):
     return render_template('add_edit_salary.html', form=form, update=True)
 
 
-@main.route('/user/toggle/<int:uid>', methods=['POST'])
-@login_required
-@admin_required
-def toggle_user(uid):
-    try:
-        user = User.query.filter_by(id=uid).one()
-        if user.is_disabled:
-            user.is_disabled = False
-        elif not user.is_disabled:
-            user.is_disabled = True
-        db.session.commit()
-        flash('Updated user status')
-    except NoResultFound:
-        flash('Unknown error occurred')
-    return redirect(url_for('main.profile', username=user.username))
-
-
 @main.route('/image/<int:image_id>')
 @login_required
 def display_submission(image_id):
@@ -376,6 +359,9 @@ def display_tag(tag_id):
 def classify_submission(image_id, contains_cops):
     try:
         image = Image.query.filter_by(id=image_id).one()
+        if image.contains_cops is not None and not current_user.is_administrator:
+            flash('Only administrator can re-classify image')
+            return redirect(redirect_url())
         image.user_id = current_user.get_id()
         if contains_cops == 1:
             image.contains_cops = True
@@ -473,9 +459,7 @@ def edit_department(department_id):
                         if Assignment.query.filter(Assignment.job_id.in_([rank.id])).count() != 0:
                             failed_deletions.append(rank)
                     for rank in failed_deletions:
-                        formatted_rank = rank.job_title.replace(" ", "+")
-                        link = '/department/{}?name=&badge=&unique_internal_identifier=&rank={}&min_age=16&max_age=100&submit=Submit'.format(department_id, formatted_rank)
-                        flash(Markup('You attempted to delete a rank, {}, that is in use by <a href={}>the linked officers</a>.'.format(rank, link)))
+                        flash('You attempted to delete a rank, {}, that is still in use'.format(rank))
                     return redirect(url_for('main.edit_department', department_id=department_id))
 
             for (new_rank, order) in new_ranks:
@@ -782,6 +766,9 @@ def label_data(department_id=None, image_id=None):
             image = get_random_image(image_query)
 
     if image:
+        if image.is_tagged and not current_user.is_administrator:
+            flash('This image cannot be tagged anymore')
+            return redirect(url_for('main.label_data'))
         proper_path = serve_image(image.filepath)
     else:
         proper_path = None

OpenOversight/app/templates/auth/email/new_confirmation.html

@@ -4,9 +4,9 @@
   <li>Username: {{ user.username }}</li>
   <li>Email: {{ user.email }}</li>
 </ul>
-<p>To view or delete this user, please <a href="{{ url_for('auth.user_api', _external=True) }}">click here</a>.</p>
+<p>To view or delete this user, please <a href="{{ url_for('auth.edit_user', user_id=user.id, _external=True) }}">click here</a>.</p>
 <p>Alternatively, you can paste the following link in your browser's address bar:</p>
-<p>{{ url_for('auth.user_api', _external=True) }}</p>
+<p>{{ url_for('auth.edit_user', user_id=user.id, _external=True) }}</p>
 <p>Sincerely,</p>
 <p>The OpenOversight Team</p>
 <p><small>Note: replies to this email address are not monitored.</small></p>

OpenOversight/app/templates/auth/email/new_confirmation.txt

@@ -7,7 +7,7 @@ Email: {{ user.email }}
 
 To view or delete this user, click on the following link:
 
-{{ url_for('auth.user_api', _external=True) }}
+{{ url_for('auth.edit_user', user_id=user.id, _external=True) }}
 
 Sincerely,
 

OpenOversight/app/templates/auth/email/new_registration.html

@@ -4,9 +4,9 @@
   <li>Username: {{ user.username }}</li>
   <li>Email: {{ user.email }}</li>
 </ul>
-<p>To approve or delete this user, please <a href="{{ url_for('auth.user_api', _external=True) }}">click here</a>.</p>
+<p>To approve or delete this user, please <a href="{{ url_for('auth.edit_user', user_id=user.id, _external=True) }}">click here</a>.</p>
 <p>Alternatively, you can paste the following link in your browser's address bar:</p>
-<p>{{ url_for('auth.user_api', _external=True) }}</p>
+<p>{{ url_for('auth.edit_user', user_id=user.id, _external=True) }}</p>
 <p>Sincerely,</p>
 <p>The OpenOversight Team</p>
 <p><small>Note: replies to this email address are not monitored.</small></p>

OpenOversight/app/templates/auth/email/new_registration.txt

@@ -7,7 +7,7 @@ Email: {{ user.email }}
 
 To approve or delete this user, click on the following link:
 
-{{ url_for('auth.user_api', _external=True) }}
+{{ url_for('auth.edit_user', user_id=user.id, _external=True) }}
 
 Sincerely,
 

OpenOversight/app/templates/auth/user.html

@@ -13,7 +13,7 @@ <h1>
 	</div>
 	<div class="col-md-6 col-xs-12">
 	    <form class="form" method="post" role="form">
-				{{ wtf.quick_form(form, button_map={'submit':'primary'}) }}
+				{{ wtf.quick_form(form, button_map={"submit":"primary", "delete": "danger"}) }}
 	    </form>
 	    <br>
 	</div>

OpenOversight/app/templates/auth/user_delete.html

@@ -11,7 +11,8 @@ <h1>
 		<p class="lead">
 			Are you sure you want to delete this user?
 			This cannot be undone.
-			<form action="{{ '{}/delete'.format(url_for('auth.user_api', user_id=user.id)) }}" method="post">
+			<form action="{{ url_for('auth.delete_user', user_id=user.id) }}" method="post">
+				<input type="hidden" name="csrf_token" value="{{ csrf_token() }}"/>
 				<button class='btn btn-danger' type="submit">Delete</button>
 			</form>
 		</p>