fix: Improve assessment attachment downloads (#5613)

pages/partials/attachFilePanel.ejs

@@ -11,7 +11,7 @@
       <% file_list.forEach(function(file, iFile) { %>
       <tr>
         <td style="word-break:break-all;">
-          <a class="attached-file" target="_blank" href="<%= urlPrefix %>/assessment_instance/<%= assessment_instance.id %>/file/<%= file.id %>/<%= file.display_filename %>">
+          <a class="attached-file" href="<%= urlPrefix %>/assessment_instance/<%= assessment_instance.id %>/file/<%= file.id %>/<%= file.display_filename %>">
             <%= file.display_filename %>
           </a>
         </td>

pages/studentAssessmentInstanceFile/studentAssessmentInstanceFile.js

@@ -2,18 +2,34 @@ const express = require('express');
 const router = express.Router();
 const asyncHandler = require('express-async-handler');
 
+const error = require('../../prairielib/error');
+const sqlLoader = require('../../prairielib/lib/sql-loader');
+const sqldb = require('../../prairielib/lib/sql-db');
 const fileStore = require('../../lib/file-store');
 
+const sql = sqlLoader.loadSqlEquiv(__filename);
+
 router.get(
-  '/:file_id/:display_filename',
+  '/:unsafe_file_id/:unsafe_display_filename',
   asyncHandler(async (req, res, next) => {
-    const options = {
+    const params = {
       assessment_instance_id: res.locals.assessment_instance.id,
-      file_id: req.params.file_id,
-      display_filename: req.params.display_filename,
+      unsafe_file_id: req.params.unsafe_file_id,
+      unsafe_display_filename: req.params.unsafe_display_filename,
     };
 
-    const stream = await fileStore.getStream(options.file_id);
+    // Assert that the file belongs to this assessment, that the display
+    // filename matches, and that the file is not deleted.
+    const result = await sqldb.queryZeroOrOneRowAsync(sql.select_file, params);
+    if (result.rows.length === 0) {
+      return next(error.make(404, 'File not found'));
+    }
+
+    const { id: fileId, display_filename: displayFilename } = result.rows[0];
+    const stream = await fileStore.getStream(fileId);
+    // Ensure the response is interpreted as an "attachment" (file to be downloaded)
+    // and not as a webpage.
+    res.attachment(displayFilename);
     stream.on('error', next).pipe(res);
   })
 );

pages/studentAssessmentInstanceFile/studentAssessmentInstanceFile.sql

@@ -1,9 +1,8 @@
 -- BLOCK select_file
-SELECT f.storage_filename
-FROM
-    files AS f
+SELECT id, display_filename
+FROM files
 WHERE
-    f.id = $file_id
-    AND f.assessment_instance_id = $assessment_instance_id
-    AND f.display_filename = $display_filename
-    AND f.deleted_at IS NULL;
+    id = $unsafe_file_id
+    AND assessment_instance_id = $assessment_instance_id
+    AND display_filename = $unsafe_display_filename
+    AND deleted_at IS NULL;