Several fixes (#2157)

* Ensure uploaded pictures are images and delete the previous ones

* Add CSRF protection to admin routes

* Only allow https urls in the client envs

* Close connection to try to get a new one

* Missing parameter

* start_time can be non existant. Set a default value

app/admin_model.py

@@ -2,6 +2,7 @@ from typing import Optional
 
 import arrow
 import sqlalchemy
+from flask_admin.form import SecureForm
 from flask_admin.model.template import EndpointLinkRowAction
 from markupsafe import Markup
 
@@ -100,6 +101,7 @@ def _user_upgrade_channel_formatter(view, context, model, name):
 
 
 class UserAdmin(SLModelView):
+    form_base_class = SecureForm
     column_searchable_list = ["email", "id"]
     column_exclude_list = [
         "salt",
@@ -344,6 +346,7 @@ def manual_upgrade(way: str, ids: [int], is_giveaway: bool):
 
 
 class EmailLogAdmin(SLModelView):
+    form_base_class = SecureForm
     column_searchable_list = ["id"]
     column_filters = ["id", "user.email", "mailbox.email", "contact.website_email"]
 
@@ -352,6 +355,7 @@ class EmailLogAdmin(SLModelView):
 
 
 class AliasAdmin(SLModelView):
+    form_base_class = SecureForm
     column_searchable_list = ["id", "user.email", "email", "mailbox.email"]
     column_filters = ["id", "user.email", "email", "mailbox.email"]
 
@@ -377,6 +381,7 @@ class AliasAdmin(SLModelView):
 
 
 class MailboxAdmin(SLModelView):
+    form_base_class = SecureForm
     column_searchable_list = ["id", "user.email", "email"]
     column_filters = ["id", "user.email", "email"]
 
@@ -387,11 +392,13 @@ class MailboxAdmin(SLModelView):
 
 
 class CouponAdmin(SLModelView):
+    form_base_class = SecureForm
     can_edit = False
     can_create = True
 
 
 class ManualSubscriptionAdmin(SLModelView):
+    form_base_class = SecureForm
     can_edit = True
     column_searchable_list = ["id", "user.email"]
 
@@ -433,12 +440,14 @@ class ManualSubscriptionAdmin(SLModelView):
 
 
 class CustomDomainAdmin(SLModelView):
+    form_base_class = SecureForm
     column_searchable_list = ["domain", "user.email", "user.id"]
     column_exclude_list = ["ownership_txt_token"]
     can_edit = False
 
 
 class ReferralAdmin(SLModelView):
+    form_base_class = SecureForm
     column_searchable_list = ["id", "user.email", "code", "name"]
     column_filters = ["id", "user.email", "code", "name"]
 
@@ -467,6 +476,7 @@ def _admin_created_at_formatter(view, context, model, name):
 
 
 class AdminAuditLogAdmin(SLModelView):
+    form_base_class = SecureForm
     column_searchable_list = ["admin.id", "admin.email", "model_id", "created_at"]
     column_filters = ["admin.id", "admin.email", "model_id", "created_at"]
     column_exclude_list = ["id"]
@@ -497,6 +507,7 @@ def _transactionalcomplaint_refused_email_id_formatter(view, context, model, nam
 
 
 class ProviderComplaintAdmin(SLModelView):
+    form_base_class = SecureForm
     column_searchable_list = ["id", "user.id", "created_at"]
     column_filters = ["user.id", "state"]
     column_hide_backrefs = False
@@ -567,6 +578,7 @@ def _newsletter_html_formatter(view, context, model: Newsletter, name):
 
 
 class NewsletterAdmin(SLModelView):
+    form_base_class = SecureForm
     list_template = "admin/model/newsletter-list.html"
     edit_template = "admin/model/newsletter-edit.html"
     edit_modal = False
@@ -648,6 +660,7 @@ class NewsletterAdmin(SLModelView):
 
 
 class NewsletterUserAdmin(SLModelView):
+    form_base_class = SecureForm
     column_searchable_list = ["id"]
     column_filters = ["id", "user.email", "newsletter.subject"]
     column_exclude_list = ["created_at", "updated_at", "id"]
@@ -657,17 +670,20 @@ class NewsletterUserAdmin(SLModelView):
 
 
 class DailyMetricAdmin(SLModelView):
+    form_base_class = SecureForm
     column_exclude_list = ["created_at", "updated_at", "id"]
 
     can_export = True
 
 
 class MetricAdmin(SLModelView):
+    form_base_class = SecureForm
     column_exclude_list = ["created_at", "updated_at", "id"]
 
     can_export = True
 
 
 class InvalidMailboxDomainAdmin(SLModelView):
+    form_base_class = SecureForm
     can_create = True
     can_delete = True

app/api/views/user_info.py

@@ -10,6 +10,7 @@ from app.api.base import api_bp, require_api_auth
 from app.config import SESSION_COOKIE_NAME
 from app.dashboard.views.index import get_stats
 from app.db import Session
+from app.image_validation import detect_image_format, ImageFormat
 from app.models import ApiKey, File, PartnerUser, User
 from app.proton.utils import get_proton_partner
 from app.session import logout_session
@@ -78,7 +79,6 @@ def update_user_info():
     data = request.get_json() or {}
 
     if "profile_picture" in data:
-        if data["profile_picture"] is None:
         if user.profile_picture_id:
             file = user.profile_picture
             user.profile_picture_id = None
@@ -89,6 +89,8 @@ def update_user_info():
                 Session.flush()
         else:
             raw_data = base64.decodebytes(data["profile_picture"].encode())
+            if detect_image_format(raw_data) == ImageFormat.Unknown:
+                return jsonify(error="Unsupported image format"), 400
             file_path = random_string(30)
             file = File.create(user_id=user.id, path=file_path)
             Session.flush()

app/developer/views/client_detail.py

@@ -1,4 +1,5 @@
 from io import BytesIO
+from urllib.parse import urlparse
 
 from flask import request, render_template, redirect, url_for, flash
 from flask_login import current_user, login_required
@@ -11,6 +12,7 @@ from app.config import ADMIN_EMAIL
 from app.db import Session
 from app.developer.base import developer_bp
 from app.email_utils import send_email
+from app.image_validation import detect_image_format, ImageFormat
 from app.log import LOG
 from app.models import Client, RedirectUri, File, Referral
 from app.utils import random_string
@@ -46,16 +48,25 @@ def client_detail(client_id):
         approval_form.description.data = client.description
 
     if action == "edit" and form.validate_on_submit():
+        parsed_url = urlparse(form.url.data)
+        if parsed_url.scheme != "https":
+            flash("Only https urls are allowed", "error")
+            return redirect(url_for("developer.index"))
         client.name = form.name.data
         client.home_url = form.url.data
 
         if form.icon.data:
-            # todo: remove current icon if any
+            icon_data = form.icon.data.read(10240)
-            # todo: handle remove icon
+            if detect_image_format(icon_data) == ImageFormat.Unknown:
+                flash("Unknown file format", "warning")
+                return redirect(url_for("developer.index"))
+            if client.icon:
+                s3.delete(client.icon_id)
+                File.delete(client.icon)
             file_path = random_string(30)
             file = File.create(path=file_path, user_id=client.user_id)
 
-            s3.upload_from_bytesio(file_path, BytesIO(form.icon.data.read()))
+            s3.upload_from_bytesio(file_path, BytesIO(icon_data))
 
             Session.flush()
             LOG.d("upload file %s to s3", file)

app/developer/views/new_client.py

@@ -1,3 +1,5 @@
+from urllib.parse import urlparse
+
 from flask import render_template, redirect, url_for, flash
 from flask_login import current_user, login_required
 from flask_wtf import FlaskForm
@@ -20,6 +22,10 @@ def new_client():
 
     if form.validate_on_submit():
         client = Client.create_new(form.name.data, current_user.id)
+        parsed_url = urlparse(form.url.data)
+        if parsed_url.scheme != "https":
+            flash("Only https urls are allowed", "error")
+            return redirect(url_for("developer.new_client"))
         client.home_url = form.url.data
         Session.commit()
 

app/models.py

@@ -3484,6 +3484,7 @@ class AdminAuditLog(Base):
             action=AuditLogActionEnum.stop_trial.value,
             model="User",
             model_id=user_id,
+            data={},
         )
 
     @classmethod

email_handler.py

@@ -262,7 +262,8 @@ def get_or_create_contact(from_header: str, mail_from: str, alias: Alias) -> Con
 
             Session.commit()
         except IntegrityError:
-            # No need to manually rollback, as IntegrityError already rolls back
+            # If the tx has been rolled back, the connection is borked. Force close to try to get a new one and start fresh
+            Session.close()
             LOG.info(
                 f"Contact with email {contact_email} for alias_id {alias_id} already existed, fetching from DB"
             )

server.py

@@ -283,6 +283,7 @@ def set_index_page(app):
             and not request.path.startswith("/git")
             and not request.path.startswith("/favicon.ico")
         ):
+            start_time = g.start_time or time.time()
             LOG.d(
                 "%s %s %s %s %s, takes %s",
                 request.remote_addr,
@@ -290,7 +291,7 @@ def set_index_page(app):
                 request.path,
                 request.args,
                 res.status_code,
-                time.time() - g.start_time,
+                time.time() - start_time,
             )
 
         return res