From f8f123db63f07f69ac36c1aacd431fbb04201a3f Mon Sep 17 00:00:00 2001 From: Xavier Lecours Date: Tue, 4 Jul 2023 15:52:54 -0400 Subject: [PATCH 01/11] [API] Using isAccessibleBy on candidate endpoint (#8824) Add accessibility check for candidate metadata, not just visits. --- modules/api/php/endpoints/candidates.class.inc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/modules/api/php/endpoints/candidates.class.inc b/modules/api/php/endpoints/candidates.class.inc index ed56e9a7242..0cf6904debb 100644 --- a/modules/api/php/endpoints/candidates.class.inc +++ b/modules/api/php/endpoints/candidates.class.inc @@ -150,6 +150,10 @@ class Candidates extends Endpoint implements \LORIS\Middleware\ETagCalculator $candidate = \NDB_Factory::singleton()->candidate($candID); + if (!$candidate->isAccessibleBy($user)) { + return new \LORIS\Http\Response\JSON\Forbidden(); + } + $endpoint = new Candidate\Candidate($candidate); $pathparts = array_slice($pathparts, 2); From 14418af478f6e568f7492078b160b54a505a8bb5 Mon Sep 17 00:00:00 2001 From: Rida Abou-Haidar Date: Thu, 10 Aug 2023 11:32:30 -0400 Subject: [PATCH 02/11] [survey_accounts] notices, warnings and filter options (#8859) This was made to remove overrides on COPN and CBIGR - removes notices - removes deprecation warning - add filter options to filter with limited set of options --- .../survey_accounts/js/survey_accounts_helper.js | 14 ++++++++------ modules/survey_accounts/jsx/surveyAccountsIndex.js | 6 +++++- modules/survey_accounts/php/addsurvey.class.inc | 9 +++++---- .../survey_accounts/php/survey_accounts.class.inc | 12 ++++++++++-- 4 files changed, 28 insertions(+), 13 deletions(-) diff --git a/modules/survey_accounts/js/survey_accounts_helper.js b/modules/survey_accounts/js/survey_accounts_helper.js index 917e91eef37..ce3acbbad9b 100644 --- a/modules/survey_accounts/js/survey_accounts_helper.js +++ b/modules/survey_accounts/js/survey_accounts_helper.js @@ -16,11 +16,13 @@ $(document).ready(function () { // Handles cases where there was an error on the page and we're resubmitting var email2 = $("input[name=Email2]").val(); var email = $("input[name=Email]").val(); - if (email.length > 0 && email2.length > 0 && email == email2) - { - $('#email_survey').removeAttr('disabled'); - } else { - $('#email_survey').attr('disabled','disabled'); + if (email && email2) { + if (email.length > 0 && email2.length > 0 && email == email2) + { + $('#email_survey').removeAttr('disabled'); + } else { + $('#email_survey').attr('disabled','disabled'); + } } // Reset Test_name so that the template can be loaded by ajax below $("select[name=Test_name]").val(""); @@ -93,7 +95,7 @@ $(document).ready(function () { $("#emailContent").val(content); } ); - + }); }); diff --git a/modules/survey_accounts/jsx/surveyAccountsIndex.js b/modules/survey_accounts/jsx/surveyAccountsIndex.js index 0fac30465de..7241e266873 100644 --- a/modules/survey_accounts/jsx/surveyAccountsIndex.js +++ b/modules/survey_accounts/jsx/surveyAccountsIndex.js @@ -112,7 +112,11 @@ class SurveyAccountsIndex extends Component { options: options.instruments, }}, {label: 'URL', show: true}, - {label: 'Status', show: true}, + {label: 'Status', show: true, filter: { + name: 'Status', + type: 'select', + options: options.statusOptions, + }}, ]; const addSurvey = () => { location.href='/survey_accounts/addSurvey/'; diff --git a/modules/survey_accounts/php/addsurvey.class.inc b/modules/survey_accounts/php/addsurvey.class.inc index 7bd74d60ae9..17e23330d55 100644 --- a/modules/survey_accounts/php/addsurvey.class.inc +++ b/modules/survey_accounts/php/addsurvey.class.inc @@ -162,8 +162,9 @@ class AddSurvey extends \NDB_Form ]; } } - - if ($_REQUEST['fire_away'] !== 'Create survey') { + if (!isset($_REQUEST['fire_away']) + || ($_REQUEST['fire_away'] !== 'Create survey') + ) { if (!filter_var( $values['Email'], FILTER_VALIDATE_EMAIL @@ -241,11 +242,11 @@ class AddSurvey extends \NDB_Form 'CommentID' => $commentID, ] ); + $this->tpl_data['success'] = true; } catch (\DatabaseException $e) { error_log($e->getMessage()); $this->tpl_data['success'] = false; } - $this->tpl_data['success'] = true; if ($email && ($values['send_email'] == 'true')) { $config = \NDB_Config::singleton(); @@ -291,7 +292,7 @@ class AddSurvey extends \NDB_Form "Instrument", array_merge( ['' => ''], - \Utility::getDirectInstruments() + \NDB_BVL_Instrument::getDirectEntryInstrumentNamesList($this->loris) ) ); $this->addBasicText("Email", "Email address"); diff --git a/modules/survey_accounts/php/survey_accounts.class.inc b/modules/survey_accounts/php/survey_accounts.class.inc index 9154921e4a1..7256ccc759e 100644 --- a/modules/survey_accounts/php/survey_accounts.class.inc +++ b/modules/survey_accounts/php/survey_accounts.class.inc @@ -74,14 +74,22 @@ class Survey_Accounts extends \DataFrameworkMenu */ public function getFieldOptions() : array { + $statusOptions = [ + 'Created' => 'Created', + 'Sent' => 'Sent', + 'In Progress' => 'In Progress', + 'Complete' => 'Complete', + ]; + $instruments = \NDB_BVL_Instrument::getDirectEntryInstrumentNamesList( $this->loris ); return [ - 'visits' => \Utility::getVisitList(), - 'instruments' => $instruments, + 'visits' => \Utility::getVisitList(), + 'instruments' => $instruments, + 'statusOptions' => $statusOptions, ]; } From d69d61eee3fe05606fa58a78110a48a5fb51a128 Mon Sep 17 00:00:00 2001 From: Rida Abou-Haidar Date: Thu, 10 Aug 2023 11:34:38 -0400 Subject: [PATCH 03/11] [LorisForm] add required to checkboxes (#8860) Checkbox elements can not be defined as required like the rest of the elements. This add support for required. A required checkbox must be checked for the form to be submitted. (ie. for questions like "Have you read the terms of service?") --- php/libraries/LorisForm.class.inc | 6 +++++- test/unittests/LorisForms_Test.php | 4 ++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/php/libraries/LorisForm.class.inc b/php/libraries/LorisForm.class.inc index bc29b94bc18..8a3211d37e8 100644 --- a/php/libraries/LorisForm.class.inc +++ b/php/libraries/LorisForm.class.inc @@ -1592,6 +1592,7 @@ class LorisForm $checked = ''; $value = ''; $disabled = ''; + $required = ''; if (!empty($val)) { $checked = 'checked="checked"'; } @@ -1601,6 +1602,9 @@ class LorisForm if (isset($el['disabled']) || $this->frozen) { $disabled = 'disabled'; } + if (isset($el['required'])) { + $required = 'required'; + } /// XXX: There seems to be a problem when using   to separate the // checkbox from the label. Both Firefox and Chrome will still put a // linebreak between the space and the checkbox. Instead, we wrap use @@ -1609,7 +1613,7 @@ class LorisForm // label it's still allowed to have linebreaks. return "" + . "$disabled $required/>" . " $el[label]"; } diff --git a/test/unittests/LorisForms_Test.php b/test/unittests/LorisForms_Test.php index 276616dbb56..61b269c357e 100644 --- a/test/unittests/LorisForms_Test.php +++ b/test/unittests/LorisForms_Test.php @@ -1336,7 +1336,7 @@ function testCheckboxHTMLWithNoAttributes() $this->form->addCheckbox("abc", "Hello", []); $this->assertEquals( " Hello", + "type=\"checkbox\" /> Hello", $this->form->checkboxHTML($this->form->form['abc']) ); } @@ -1358,7 +1358,7 @@ function testCheckboxHTMLWithAttributesSet() $this->assertEquals( " Hello", + " value=\"value1\" disabled /> Hello", $this->form->checkboxHTML($this->form->form['abc']) ); } From c6c05ae186a0512dd852312e9dae4e15e7eeeca1 Mon Sep 17 00:00:00 2001 From: charlottesce <75381352+charlottesce@users.noreply.github.com> Date: Wed, 16 Aug 2023 09:39:51 -0400 Subject: [PATCH 04/11] [Libraries] Feedback panel - fix fieldnames setting (#8848) In the feedback panel for an instrument, the only option in 'Field Name' was 'Across all fields'. This changes how the field names for an instrument are fetched so that all of them appear as an option too. --- php/libraries/BVL_Feedback_Panel.class.inc | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/php/libraries/BVL_Feedback_Panel.class.inc b/php/libraries/BVL_Feedback_Panel.class.inc index a93a5ea84f3..88988d6d0a7 100644 --- a/php/libraries/BVL_Feedback_Panel.class.inc +++ b/php/libraries/BVL_Feedback_Panel.class.inc @@ -101,7 +101,15 @@ class BVL_Feedback_Panel $summary = $this->feedbackThread->getSummaryOfThreads(); $this->tpl_data['thread_summary_headers'] = json_encode($summary); - $field_names = Utility::getSourcefields($_REQUEST['test_name'] ?? ''); + $test_name = ''; + if (array_key_exists('test_name', $_REQUEST)) { + $test_name = $_REQUEST['test_name']; + } else if (array_key_exists('lorispath', $_REQUEST)) { + $test_name = preg_split("#/#", $_REQUEST['lorispath'])[1] ?? ''; + } + + // Get field names + $field_names = Utility::getSourcefields($test_name); $fields = []; $fields['Across All Fields'] = 'Across All Fields'; foreach ($field_names as $field_name) { From 6705373508e679bb5052c52bc19ab6cf038f2a2b Mon Sep 17 00:00:00 2001 From: charlottesce <75381352+charlottesce@users.noreply.github.com> Date: Wed, 16 Aug 2023 09:47:53 -0400 Subject: [PATCH 05/11] [tools][CouchDB_MRI_Importer] Fix date in MRI data (#8851) - Fix date imported into DQT to be a real date rather than a unix timestamp. - Fix incorrect order of parameters to `join` in import script --- tools/CouchDB_MRI_Importer.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tools/CouchDB_MRI_Importer.php b/tools/CouchDB_MRI_Importer.php index c58c28f73c5..a8d3dde87ec 100755 --- a/tools/CouchDB_MRI_Importer.php +++ b/tools/CouchDB_MRI_Importer.php @@ -249,7 +249,10 @@ function _addMRIHeaderInfo($FileObj, $scan_type) $FileObj, 'acquisition_date' ); - $header['FileInsertDate_'.$type] = $FileObj->getParameter('InsertTime'); + $header['FileInsertDate_'.$type] = date( + 'Y-m-d', + $FileObj->getParameter('InsertTime') + ); $header['SeriesDescription_'.$type] = $FileObj->getParameter($ser_desc); $header['SeriesNumber_'.$type] = $FileObj->getParameter($ser_num); $header['EchoTime_'.$type] = number_format( @@ -438,7 +441,7 @@ function updateCandidateDocs($data, $ScanTypes) $row['PSCID'], $row['Visit_label'], ]; - $docid = 'MRI_Files:' . join($identifier, '_'); + $docid = 'MRI_Files:' . join('_', $identifier); unset($doc['PSCID']); unset($doc['Visit_label']); unset($doc['SessionID']); From 3baac58b332e0a14c48888650fc998f4dd6bce96 Mon Sep 17 00:00:00 2001 From: charlottesce <75381352+charlottesce@users.noreply.github.com> Date: Wed, 16 Aug 2023 10:18:57 -0400 Subject: [PATCH 06/11] [issue_tracker] Resolves special characters in titles (#8842) Do not escape data being inserted in the issue tracker, it gets escaped on rendering. --- modules/issue_tracker/php/edit.class.inc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/modules/issue_tracker/php/edit.class.inc b/modules/issue_tracker/php/edit.class.inc index 448dac4997b..0e3a0680d52 100644 --- a/modules/issue_tracker/php/edit.class.inc +++ b/modules/issue_tracker/php/edit.class.inc @@ -447,11 +447,11 @@ class Edit extends \NDB_Page implements ETagCalculator $historyValues = $this->getChangedValues($issueValues, $issueID, $user); if (!empty($issueID)) { - $db->update('issues', $issueValues, ['issueID' => $issueID]); + $db->unsafeUpdate('issues', $issueValues, ['issueID' => $issueID]); } else { $issueValues['reporter'] = $user->getUsername(); $issueValues['dateCreated'] = date('Y-m-d H:i:s'); - $db->insert('issues', $issueValues); + $db->unsafeInsert('issues', $issueValues); $issueID = intval($db->getLastInsertId()); } @@ -815,7 +815,7 @@ class Edit extends \NDB_Page implements ETagCalculator 'issueID' => $issueID, 'addedBy' => $user->getUsername(), ]; - $db->insert('issues_history', $changedValues); + $db->unsafeInsert('issues_history', $changedValues); } } } @@ -838,7 +838,7 @@ class Edit extends \NDB_Page implements ETagCalculator 'addedBy' => $user->getUsername(), 'issueID' => $issueID, ]; - $db->insert('issues_comments', $commentValues); + $db->unsafeInsert('issues_comments', $commentValues); } } From b38181660ab0c6adfbd032a60ccca44d01168d91 Mon Sep 17 00:00:00 2001 From: Dave MacFarlane Date: Wed, 16 Aug 2023 11:11:33 -0400 Subject: [PATCH 07/11] [LINST/Dictionary] Add Numeric elements to dictionary (#8869) Currently the numeric element type is only being added to the instrument data dictionary if it's on the top page. This fixes it so that the elements are always added to the dictionary regardless of the page. --- php/libraries/NDB_BVL_Instrument_LINST.class.inc | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/php/libraries/NDB_BVL_Instrument_LINST.class.inc b/php/libraries/NDB_BVL_Instrument_LINST.class.inc index 495843a4358..1c196e3753d 100644 --- a/php/libraries/NDB_BVL_Instrument_LINST.class.inc +++ b/php/libraries/NDB_BVL_Instrument_LINST.class.inc @@ -734,14 +734,14 @@ class NDB_BVL_Instrument_LINST extends \NDB_BVL_Instrument case 'numeric': if ($addElements) { $this->addNumericElement($pieces[1], $pieces[2]); - $this->dictionary[] = new DictionaryItem( - $this->testName."_".$pieces[1], - $pieces[2], - $scope, - new IntegerType(), - new Cardinality(Cardinality::SINGLE), - ); } + $this->dictionary[] = new DictionaryItem( + $this->testName."_".$pieces[1], + $pieces[2], + $scope, + new IntegerType(), + new Cardinality(Cardinality::SINGLE), + ); if ($firstFieldOfPage) { $this->_requiredElements[] = $fieldname; $firstFieldOfPage = false; From 80740e475f798016fc6b6971b2737d6dc0c6a50f Mon Sep 17 00:00:00 2001 From: Zaliqa Date: Tue, 12 Sep 2023 13:46:42 -0400 Subject: [PATCH 08/11] [imaging_uploader] Fix auto-populating VisitLabel (#8881) Properly handle parsing/auto-populating of visit label when there is a suffix after the visit label in the file name. Fixes #8803 --- modules/imaging_uploader/jsx/UploadForm.js | 36 ++++++++++++++++++---- 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/modules/imaging_uploader/jsx/UploadForm.js b/modules/imaging_uploader/jsx/UploadForm.js index 3b2fe0251a1..1561f009b20 100644 --- a/modules/imaging_uploader/jsx/UploadForm.js +++ b/modules/imaging_uploader/jsx/UploadForm.js @@ -66,10 +66,22 @@ class UploadForm extends Component { let ids = patientName.split('_'); formData.candID = ids[1]; formData.pSCID = ids[0]; - // visitLabel can contain underscores - // join the remaining elements of patientName and use as visitLabel + // visitLabel can contain underscores, filename can have suffix appended to PSCID_CandID_VisitLabel + // join the remaining elements of patientName and pattern match + // against each visit label. Use as visitLabel the best (longest) match ids.splice(0, 2); - formData.visitLabel = ids.join('_'); + const suffix = ids.join('_'); + const visitLabels = Object.keys(form.visitLabel.options); + let bestMatch = ''; + visitLabels.map((visitLabel) => { + if (suffix.match(visitLabel) !== null) { + // consider the first match only + if (suffix.match(visitLabel)[0].length > bestMatch.length) { + bestMatch = suffix.match(visitLabel)[0]; + } + } + }); + formData.visitLabel = bestMatch; } } @@ -81,10 +93,22 @@ class UploadForm extends Component { let ids = patientName.split('_'); formData.candID = ids[1]; formData.pSCID = ids[0]; - // visitLabel can contain underscores - // join the remaining elements of patientName and use as visitLabel + // visitLabel can contain underscores, filename can have suffix appended to PSCID_CandID_VisitLabel + // join the remaining elements of patientName and pattern match + // against each visit label. Use as visitLabel the best (longest) match ids.splice(0, 2); - formData.visitLabel = ids.join('_'); + const suffix = ids.join('_'); + const visitLabels = Object.keys(form.visitLabel.options); + let bestMatch = ''; + visitLabels.map((visitLabel) => { + if (suffix.match(visitLabel) !== null) { + // consider the first match only + if (suffix.match(visitLabel)[0].length > bestMatch.length) { + bestMatch = suffix.match(visitLabel)[0]; + } + } + }); + formData.visitLabel = bestMatch; } } From 6d6d0d9b4740ad907f929e40e904a4356af5cb09 Mon Sep 17 00:00:00 2001 From: Rida Abou-Haidar Date: Tue, 12 Sep 2023 15:03:14 -0400 Subject: [PATCH 09/11] [Dict] Skip hidden fields in data dictionary (#8882) Some instruments use hidden fields to pass data to the frontend. This skips over the fields in the dictionary building to prevent a 500 error. --- php/libraries/LorisFormDictionaryImpl.class.inc | 1 + 1 file changed, 1 insertion(+) diff --git a/php/libraries/LorisFormDictionaryImpl.class.inc b/php/libraries/LorisFormDictionaryImpl.class.inc index 9fae78137c8..f6290a60407 100644 --- a/php/libraries/LorisFormDictionaryImpl.class.inc +++ b/php/libraries/LorisFormDictionaryImpl.class.inc @@ -131,6 +131,7 @@ trait LorisFormDictionaryImpl $t = new \LORIS\Data\Types\StringType(255); break; case 'header': + case 'hidden': continue 2; default: throw new \LorisException( From 48d4d0e91b6bc1ba7aae5d12b98958ae9585fe78 Mon Sep 17 00:00:00 2001 From: Laetitia Fesselier Date: Thu, 21 Sep 2023 13:37:33 -0400 Subject: [PATCH 10/11] [Configuration] Fix values of email fields (#8891) Fixes #8890 --- modules/configuration/templates/form_configuration.tpl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/configuration/templates/form_configuration.tpl b/modules/configuration/templates/form_configuration.tpl index 75715b414d3..434ca978587 100644 --- a/modules/configuration/templates/form_configuration.tpl +++ b/modules/configuration/templates/form_configuration.tpl @@ -107,7 +107,7 @@ {elseif $node['DataType'] eq 'date_format'} {call createDateFormat k=$id v=$v d=$node['Disabled']} {elseif $node['DataType'] eq 'email'} - {call createEmail k=$id v=$id d=$node['Disabled']} + {call createEmail k=$id v=$v d=$node['Disabled']} {elseif $node['DataType'] eq 'textarea'} {call createTextArea k=$id v=$v d=$node['Disabled']} {elseif $node['DataType'] eq 'lookup_center'} From 9b08f461021ddec54ce1f58d6bc70cdbe6aff140 Mon Sep 17 00:00:00 2001 From: Dave MacFarlane Date: Tue, 3 Oct 2023 12:47:43 -0400 Subject: [PATCH 11/11] [media] Fix problematic SQL (#8908) This fixes 2 problems with the SQL in the media FileUpload?action=getData endpoint 1. There is an obvious SQL injection attack where user input from the request is directly concatenated into a string that's passed to the database. 2. There was an unnecessary sub-select that could have been a join This whole section of the code is a mess that should to be re-written, but this PR just tackles the urgent string concatenation. --- modules/media/ajax/FileUpload.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/modules/media/ajax/FileUpload.php b/modules/media/ajax/FileUpload.php index a91f64d00ca..5af60b691ae 100644 --- a/modules/media/ajax/FileUpload.php +++ b/modules/media/ajax/FileUpload.php @@ -349,7 +349,7 @@ function getUploadFields() $mediaData = $db->pselectRow( "SELECT " . "m.session_id as sessionID, " . - "(SELECT PSCID from candidate WHERE CandID=s.CandID) as pscid, " . + "c.PSCID as pscid, " . "Visit_label as visitLabel, " . "instrument, " . "CenterID as forSite, " . @@ -358,9 +358,10 @@ function getUploadFields() "file_name as fileName, " . "hide_file as hideFile, " . "language_id as language," . - "m.id FROM media m LEFT JOIN session s ON m.session_id = s.ID " . - "WHERE m.id = $idMediaFile", - [] + "m.id FROM media m LEFT JOIN session s ON m.session_id = s.ID + LEFT JOIN candidate c ON (c.CandID=s.CandID) " . + "WHERE m.id = :mediaId", + ['mediaId' => $idMediaFile] ); }