Improve validation and error reporting during CSV import (#1979)

* Improve validation during CSV import process by using existing Marshmallow Schemas
* Return CSV import errors from import functions to render out to the user
* Ignore invalid fields that we can't use in Challenge CSV import

CTFd/admin/__init__.py

@@ -6,6 +6,7 @@ from io import StringIO
 from flask import Blueprint, abort
 from flask import current_app as app
 from flask import (
+    jsonify,
     redirect,
     render_template,
     render_template_string,
@@ -139,8 +140,11 @@ def import_csv():
 
     loader = loaders[csv_type]
     reader = csv.DictReader(csvfile)
-    loader(reader)
-    return redirect(url_for("admin.config"))
+    success = loader(reader)
+    if success is True:
+        return redirect(url_for("admin.config"))
+    else:
+        return jsonify(success), 500
 
 
 @admin.route("/admin/export/csv")

CTFd/themes/admin/assets/js/pages/configs.js

@@ -246,6 +246,77 @@ function removeSmallIcon() {
   });
 }
 
+function importCSV(event) {
+  event.preventDefault();
+  let csv_file = document.getElementById("import-csv-file").files[0];
+  let csv_type = document.getElementById("import-csv-type").value;
+
+  let form_data = new FormData();
+  form_data.append("csv_file", csv_file);
+  form_data.append("csv_type", csv_type);
+  form_data.append("nonce", CTFd.config.csrfNonce);
+
+  let pg = ezProgressBar({
+    width: 0,
+    title: "Upload Progress"
+  });
+
+  $.ajax({
+    url: CTFd.config.urlRoot + "/admin/import/csv",
+    type: "POST",
+    data: form_data,
+    processData: false,
+    contentType: false,
+    statusCode: {
+      500: function(resp) {
+        // Normalize errors
+        let errors = JSON.parse(resp.responseText);
+        let errorText = "";
+        errors.forEach(element => {
+          errorText += `Line ${element[0]}: ${JSON.stringify(element[1])}\n`;
+        });
+
+        // Show errors
+        alert(errorText);
+
+        // Hide progress modal if its there
+        pg = ezProgressBar({
+          target: pg,
+          width: 100
+        });
+        setTimeout(function() {
+          pg.modal("hide");
+        }, 500);
+      }
+    },
+    xhr: function() {
+      let xhr = $.ajaxSettings.xhr();
+      xhr.upload.onprogress = function(e) {
+        if (e.lengthComputable) {
+          let width = (e.loaded / e.total) * 100;
+          pg = ezProgressBar({
+            target: pg,
+            width: width
+          });
+        }
+      };
+      return xhr;
+    },
+    success: function(_data) {
+      pg = ezProgressBar({
+        target: pg,
+        width: 100
+      });
+      setTimeout(function() {
+        pg.modal("hide");
+      }, 500);
+      setTimeout(function() {
+        window.location.reload();
+      }, 700);
+    }
+  });
+}
+
 function importConfig(event) {
   event.preventDefault();
   let import_file = document.getElementById("import-file").files[0];
@@ -413,6 +484,7 @@ $(() => {
   $("#remove-small-icon").click(removeSmallIcon);
   $("#export-button").click(exportConfig);
   $("#import-button").click(importConfig);
+  $("#import-csv-form").submit(importCSV);
   $("#config-color-update").click(function() {
     const hex_code = $("#config-color-picker").val();
     const user_css = theme_header_editor.getValue();

CTFd/themes/admin/templates/configs/backup.html

@@ -32,7 +32,10 @@
 					You should only import data that you trust!</p>
 
 				<div class="alert alert-warning" role="alert">
-					IMPORTING A CTF WILL COMPLETELY WIPE YOUR EXISTING DATA
+					<small class="text-muted text-right">
+						<i class="fas fa-exclamation pr-2"></i>
+						Importing a CTFd export will completely wipe your existing data
+					</small>
 				</div>
 			</div>
 			<div class="form-group">
@@ -42,6 +45,12 @@
 			<input id="import-button" type="submit" class="btn btn-warning" value="Import">
 		</div>
 		<div role="tabpanel" class="tab-pane" id="save-csv">
+			<div class="alert alert-warning" role="alert">
+				<small class="text-muted text-right">
+					<i class="fas fa-exclamation pr-2"></i>
+					CSVs exported from CTFd are not guaranteed to import back in via the Import CSV functionality. See <a href="https://docs.ctfd.io/docs/imports/csv/" target="_blank">CSV Importing instructions</a> for details.
+				</small>
+			</div>
 			{% with form = Forms.config.ExportCSVForm() %}
 			<form method="GET" action="{{ url_for('admin.export_csv') }}">
 				<div class="form-group">
@@ -54,26 +63,26 @@
 		</div>
 		<div role="tabpanel" class="tab-pane" id="import-csv">
 			{% with form = Forms.config.ImportCSVForm() %}
-			<form method="POST" action="{{ url_for('admin.import_csv') }}" enctype="multipart/form-data">
+			<form method="POST" action="{{ url_for('admin.import_csv') }}" enctype="multipart/form-data" id="import-csv-form">
 				<div class="form-group">
 					<div class="alert alert-info" role="alert">
 						<small class="text-muted text-right">
 							<a href="https://docs.ctfd.io/docs/imports/csv/" target="_blank">
-								<i class="far fa-question-circle"></i> Instructions and CSV templates
+								<i class="far fa-question-circle pr-2"></i> Instructions and CSV templates
 							</a>
 						</small>
 					</div>
 				</div>
 				<div class="form-group">
 					<b>{{ form.csv_type.label }}</b>
-					{{ form.csv_type(class="form-control custom-select") }}
+					{{ form.csv_type(class="form-control custom-select", id="import-csv-type") }}
 					<small class="form-text text-muted">
 						{{ form.csv_type.description }}
 					</small>
 				</div>
 				<div class="form-group">
 					<b>{{ form.csv_file.label }}</b>
-					{{ form.csv_file(class="form-control-file") }}
+					{{ form.csv_file(class="form-control-file", id="import-csv-file", accept=".csv") }}
 					<small class="form-text text-muted">
 						{{ form.csv_file.description }}
 					</small>

CTFd/utils/csv/__init__.py

@@ -14,6 +14,9 @@ from CTFd.models import (
     get_class_by_tablename,
 )
 from CTFd.plugins.challenges import get_chal_class
+from CTFd.schemas.challenges import ChallengeSchema
+from CTFd.schemas.teams import TeamSchema
+from CTFd.schemas.users import UserSchema
 from CTFd.utils.config import is_teams_mode, is_users_mode
 from CTFd.utils.scores import get_standings
 
@@ -230,23 +233,44 @@ def dump_database_table(tablename):
 
 
 def load_users_csv(dict_reader):
-    for line in dict_reader:
-        result = Users(**line)
-        db.session.add(result)
-        db.session.commit()
+    schema = UserSchema()
+    errors = []
+    for i, line in enumerate(dict_reader):
+        response = schema.load(line)
+        if response.errors:
+            errors.append((i, response.errors))
+        else:
+            db.session.add(response.data)
+            db.session.commit()
+    if errors:
+        return errors
     return True
 
 
 def load_teams_csv(dict_reader):
-    for line in dict_reader:
-        result = Teams(**line)
-        db.session.add(result)
-        db.session.commit()
+    schema = TeamSchema()
+    errors = []
+    for i, line in enumerate(dict_reader):
+        response = schema.load(line)
+        if response.errors:
+            errors.append((i, response.errors))
+        else:
+            db.session.add(response.data)
+            db.session.commit()
+    if errors:
+        return errors
     return True
 
 
 def load_challenges_csv(dict_reader):
-    for line in dict_reader:
+    schema = ChallengeSchema()
+    errors = []
+
+    for i, line in enumerate(dict_reader):
+        # Throw away fields that we can't trust if provided
+        _ = line.pop("id", None)
+        _ = line.pop("requirements", None)
+
         flags = line.pop("flags", None)
         tags = line.pop("tags", None)
         hints = line.pop("hints", None)
@@ -256,6 +280,11 @@ def load_challenges_csv(dict_reader):
         type_data = json.loads(line.pop("type_data", "{}") or "{}")
         line.update(type_data)
 
+        response = schema.load(line)
+        if response.errors:
+            errors.append((i + 1, response.errors))
+            continue
+
         ChallengeClass = get_chal_class(challenge_type)
         challenge = ChallengeClass.challenge_model(**line)
         db.session.add(challenge)
@@ -281,6 +310,8 @@ def load_challenges_csv(dict_reader):
                 h = Hints(challenge_id=challenge.id, content=hint,)
                 db.session.add(h)
                 db.session.commit()
+    if errors:
+        return errors
     return True