From 38266d4ac7a63d320e00358f8c099a4da6c23311 Mon Sep 17 00:00:00 2001 From: Saagar Arya <51128536+skarya22@users.noreply.github.com> Date: Thu, 31 Oct 2024 14:16:04 -0400 Subject: [PATCH] [data_release] Introduce Project Separation (#9385) Add project separation permissions for the data release module - This makes it so that users do not see data_release files from different projects. - If a data_release file does not have a projectID, then all users with access to that version/file can see it. - Improved the manage permission form because it was not very user friendly - Version and user options only show up as the ones that the current user has access to that project for --- SQL/0000-00-00-schema.sql | 4 +- ...-10-02-Data_Release_Project_separation.sql | 4 + modules/data_release/jsx/addPermissionForm.js | 6 +- modules/data_release/jsx/dataReleaseIndex.js | 6 + .../data_release/jsx/managePermissionsForm.js | 107 +++++++++++++----- modules/data_release/jsx/uploadFileForm.js | 17 +++ .../data_release/php/data_release.class.inc | 44 +++++-- .../php/datareleaseprovisioner.class.inc | 10 +- modules/data_release/php/files.class.inc | 32 ++++-- raisinbread/RB_files/RB_data_release.sql | 2 +- 10 files changed, 177 insertions(+), 55 deletions(-) create mode 100644 SQL/New_patches/2024-10-02-Data_Release_Project_separation.sql diff --git a/SQL/0000-00-00-schema.sql b/SQL/0000-00-00-schema.sql index 1759cd8669f..d6c8a1ddce0 100644 --- a/SQL/0000-00-00-schema.sql +++ b/SQL/0000-00-00-schema.sql @@ -2096,7 +2096,9 @@ CREATE TABLE `data_release` ( `file_name` varchar(255), `version` varchar(255), `upload_date` date, - PRIMARY KEY (`id`) + `ProjectID` INT(10) UNSIGNED NULL, + PRIMARY KEY (`id`), + FOREIGN KEY (ProjectID) REFERENCES Project (ProjectID) ) ENGINE=InnoDB DEFAULT CHARSET=utf8; CREATE TABLE `data_release_permissions` ( diff --git a/SQL/New_patches/2024-10-02-Data_Release_Project_separation.sql b/SQL/New_patches/2024-10-02-Data_Release_Project_separation.sql new file mode 100644 index 00000000000..d067ce80fc8 --- /dev/null +++ b/SQL/New_patches/2024-10-02-Data_Release_Project_separation.sql @@ -0,0 +1,4 @@ +ALTER TABLE data_release +ADD COLUMN ProjectID INT(10) UNSIGNED NULL DEFAULT NULL, +ADD CONSTRAINT FK_ProjectID +FOREIGN KEY (ProjectID) REFERENCES Project (ProjectID); \ No newline at end of file diff --git a/modules/data_release/jsx/addPermissionForm.js b/modules/data_release/jsx/addPermissionForm.js index d4a5aa370de..5dd8a471a96 100644 --- a/modules/data_release/jsx/addPermissionForm.js +++ b/modules/data_release/jsx/addPermissionForm.js @@ -59,8 +59,7 @@ class AddPermissionForm extends Component { * Called by React when the component has been rendered on the page. */ componentDidMount() { - this.fetchData() - .then(() => this.setState({isLoaded: true})); + this.fetchData().then(() => this.setState({isLoaded: true})); } /** @@ -112,6 +111,7 @@ class AddPermissionForm extends Component { errorMessage={this.state.errorMessage.Filename} required={false} value={this.state.formData.data_release_id} + autoSelect={false} />

OR


@@ -181,7 +182,6 @@ class AddPermissionForm extends Component { }).then(function() { window.location.assign('/data_release'); }); - this.props.fetchData(); } else { let msg = response.statusText ? response.statusText : diff --git a/modules/data_release/jsx/dataReleaseIndex.js b/modules/data_release/jsx/dataReleaseIndex.js index 22eb899fad0..efc1b7f8904 100644 --- a/modules/data_release/jsx/dataReleaseIndex.js +++ b/modules/data_release/jsx/dataReleaseIndex.js @@ -157,6 +157,11 @@ class DataReleaseIndex extends Component { name: 'uploadDate', type: 'text', }}, + {label: 'Project Name', show: true, filter: { + name: 'Project', + type: 'select', + options: this.state.data.fieldOptions.projects, + }}, {label: 'Data Release ID', show: false, }, ]; @@ -178,6 +183,7 @@ class DataReleaseIndex extends Component { + '/data_release/files' } action={loris.BaseURL + '/data_release/files'} + projects={this.state.data.fieldOptions.projects} /> ); diff --git a/modules/data_release/jsx/managePermissionsForm.js b/modules/data_release/jsx/managePermissionsForm.js index 966921e476d..17bc3416e5d 100644 --- a/modules/data_release/jsx/managePermissionsForm.js +++ b/modules/data_release/jsx/managePermissionsForm.js @@ -7,6 +7,7 @@ import { FormElement, CheckboxElement, StaticElement, + SearchableDropdown, } from 'jsx/Form'; /** @@ -24,11 +25,13 @@ class ManagePermissionsForm extends Component { this.state = { data: {}, + originalData: {}, hasError: {}, errorMessage: {}, isLoaded: false, }; + this.setFormData = this.setFormData.bind(this); this.fetchData = this.fetchData.bind(this); this.handleSubmit = this.handleSubmit.bind(this); } @@ -49,7 +52,7 @@ class ManagePermissionsForm extends Component { fetchData() { return fetch(this.props.DataURL, {credentials: 'same-origin'}) .then((resp) => resp.json()) - .then((data) => this.setState({data})) + .then((data) => this.setState({data, originalData: data})) .catch( (error) => { this.setState({error: 'An error occurred when loading the form!'}); console.error(error); @@ -88,26 +91,72 @@ class ManagePermissionsForm extends Component { onSubmit={this.handleSubmit} > - {Object.entries(data).map(([userId, user]) => { - const versions = Object.values(options.versions).map((version) => -
+ + {this.state.user && +
- this.setFormData(userId, version, permission) + name={'versionsByUser'} + label={version} + value={data[this.state.user].versions.includes(version)} + onUserInput={(formElement, checked) => + this.setFormData( + 'versionsByUser', { + userId: this.state.user, version, checked, + } + ) } />
- ); - - return {versions}
} - />; - })}; + )} + /> + } + + {this.state.version && + { + if (user.versions.includes(this.state.version)) { + return
+ + this.setFormData( + 'usersByVersion', + { + userId: user.id, + checked, + version: this.state.version, + } + ) + } + />
+
; + } + } + )} + /> + } ); @@ -116,19 +165,25 @@ class ManagePermissionsForm extends Component { /** * Store the value of the element in this.state.data * - * @param {string} userId - * @param {string} version - * @param {boolean} permission + * @param {string} formElement - name of the selected element + * @param {string} value - selected value for corresponding form element */ - setFormData(userId, version, permission) { + setFormData(formElement, value) { let {data} = JSON.parse(JSON.stringify(this.state)); - if (permission) { - data[userId].versions = [...data[userId].versions, version]; + if (formElement === 'versionsByUser' || formElement === 'usersByVersion') { + let {checked, version, userId} = value; + if (checked) { + data[userId].versions = [...data[userId].versions, version]; + } else { + data[userId].versions = data[userId].versions + .filter((e) => e !== version); + } + this.setState({data}); } else { - data[userId].versions = data[userId].versions - .filter((e) => e !== version); + this.setState({[formElement]: (value === '' ? null : value)}); + if (formElement != 'user') this.setState({user: null}); + if (formElement != 'version') this.setState({version: null}); } - this.setState({data}); } /** diff --git a/modules/data_release/jsx/uploadFileForm.js b/modules/data_release/jsx/uploadFileForm.js index 7563e468c8b..0be347b2f5e 100644 --- a/modules/data_release/jsx/uploadFileForm.js +++ b/modules/data_release/jsx/uploadFileForm.js @@ -8,6 +8,7 @@ import { FileElement, TextboxElement, ButtonElement, + SelectElement, } from 'jsx/Form'; /** @@ -79,6 +80,14 @@ class UploadFileForm extends Component { required={false} value={this.state.formData.version} /> +
@@ -130,6 +139,13 @@ class UploadFileForm extends Component { return; } + if (!formData.project) { + errorMessage.Project = 'You must select a project'; + hasError.Project = true; + this.setState({errorMessage, hasError}); + return; + } + // Check that the size of the file is not bigger than the allowed size let fileSize = formData.file ? Math.round((formData.file.size/1024)) : null; const maxSizeAllowed = this.state.data.maxUploadSize; @@ -230,6 +246,7 @@ class UploadFileForm extends Component { UploadFileForm.propTypes = { DataURL: PropTypes.string.isRequired, action: PropTypes.string.isRequired, + projects: PropTypes.array.isRequired, }; export default UploadFileForm; diff --git a/modules/data_release/php/data_release.class.inc b/modules/data_release/php/data_release.class.inc index 6f8a814c5c1..47ffded4583 100644 --- a/modules/data_release/php/data_release.class.inc +++ b/modules/data_release/php/data_release.class.inc @@ -59,8 +59,12 @@ class Data_Release extends \DataFrameworkMenu function getUsersList(\Database $DB) { return $DB->pselectColWithIndexKey( - "SELECT ID, UserID FROM users", - [], + "SELECT DISTINCT ID, LOWER(users.UserID) as UserID FROM users + JOIN user_project_rel upr ON upr.UserID = users.ID + WHERE upr.ProjectID IN + (SELECT ProjectID FROM user_project_rel WHERE UserID = :userID) + ORDER BY LOWER(users.UserID)", + [':userID' => \User::singleton()->getID()], "ID" ); } @@ -75,8 +79,10 @@ class Data_Release extends \DataFrameworkMenu function getFilesList(\Database $DB) { $result = $DB->pselectWithIndexKey( - "SELECT id, file_name, version FROM data_release", - [], + "SELECT id, file_name, version, ProjectID FROM data_release + WHERE ProjectID IS NULL OR ProjectID IN + (SELECT ProjectID FROM user_project_rel WHERE UserID = :userID)", + [':userID' => \User::singleton()->getID()], "id" ); @@ -99,9 +105,12 @@ class Data_Release extends \DataFrameworkMenu */ function getVersionsList(\Database $DB) { + $user =& \User::singleton(); $versions = $DB->pselectCol( - "SELECT DISTINCT version FROM data_release", - [], + "SELECT DISTINCT version FROM data_release + WHERE ProjectID IS NULL OR ProjectID IN + (SELECT ProjectID FROM user_project_rel WHERE UserID = :userID)", + ['userID' => $user->getID()], "version" ); @@ -125,8 +134,10 @@ class Data_Release extends \DataFrameworkMenu function getVersionFiles(\Database $db) { $result = $db->pselect( - "SELECT version, id FROM data_release", - [] + "SELECT version, id FROM data_release + WHERE ProjectID IS NULL OR ProjectID IN + (SELECT ProjectID FROM user_project_rel WHERE UserID = :userID)", + [':userID' => \User::singleton()->getID()] ); $versionFiles = []; @@ -151,16 +162,23 @@ class Data_Release extends \DataFrameworkMenu $result = $db->pselect( "SELECT u.ID as userId, u.UserID as userName, - drp.data_release_id fileId + drp.data_release_id fileId, + dr.ProjectID as ProjectID FROM users u LEFT JOIN data_release_permissions drp ON (u.ID=drp.userid) - LEFT JOIN data_release dr ON (drp.data_release_id=dr.id)", - [] + LEFT JOIN data_release dr ON (drp.data_release_id=dr.id) + JOIN user_project_rel upr ON upr.UserID = u.ID + WHERE upr.ProjectID IN + (SELECT ProjectID FROM user_project_rel WHERE UserID = :userID)", + [':userID' => \User::singleton()->getID()] ); + error_log(print_r($result, true)); + $userFiles = []; foreach ($result as $row) { $userFiles[$row['userId']]['name'] = $row['userName']; + $userFiles[$row['userId']]['id'] = $row['userId']; if (empty($userFiles[$row['userId']]['files'])) { $userFiles[$row['userId']]['files'] = []; } @@ -206,11 +224,13 @@ class Data_Release extends \DataFrameworkMenu */ protected function getFieldOptions() : array { - $db = $this->loris->getDatabaseConnection(); + $db = $this->loris->getDatabaseConnection(); + $projects = \Utility::getProjectList(); return [ 'users' => $this->getUsersList($db), 'versions' => $this->getVersionsList($db), 'filenames' => $this->getFilesList($db), + 'projects' => array_combine($projects, $projects) ]; } diff --git a/modules/data_release/php/datareleaseprovisioner.class.inc b/modules/data_release/php/datareleaseprovisioner.class.inc index 636f2b5c36f..f3eab13e526 100644 --- a/modules/data_release/php/datareleaseprovisioner.class.inc +++ b/modules/data_release/php/datareleaseprovisioner.class.inc @@ -44,8 +44,10 @@ class DataReleaseProvisioner extends \LORIS\Data\Provisioners\DBRowProvisioner file_name AS fileName, IF(version is null or version ='','Unversioned', version) AS version, upload_date AS uploadDate, + p.Name as ProjectName, dr.id as dataReleaseID - FROM data_release dr"; + FROM data_release dr + LEFT JOIN Project p on p.ProjectID = dr.ProjectID"; if (!$user->hasPermission("superuser")) { $query .= " @@ -54,7 +56,11 @@ class DataReleaseProvisioner extends \LORIS\Data\Provisioners\DBRowProvisioner ON (dr.id=drp.data_release_id) WHERE - drp.UserID=".$user->getID(); + drp.UserID=".$user->getID()." AND ( + dr.ProjectID IS NULL OR dr.ProjectID IN + (SELECT ProjectID FROM user_project_rel + WHERE UserID = ".$user->getID().") + )"; } $query .= " ORDER BY uploadDate"; diff --git a/modules/data_release/php/files.class.inc b/modules/data_release/php/files.class.inc index 4ac7cc31e36..b9067d51d37 100644 --- a/modules/data_release/php/files.class.inc +++ b/modules/data_release/php/files.class.inc @@ -116,6 +116,7 @@ class Files extends \NDB_Page $request, $posted['version'], $overwrite, + $posted['project'], ); } @@ -180,15 +181,17 @@ class Files extends \NDB_Page * Moves a to the appropriate place on the filesystem and inserts into * the database, returning an appropriate HTTP response. * - * @param \LORIS\FilesUploadHandler $files The FilesUploadHandler which moves - * the file and generates the - * response. - * @param \User $user The user uploading the file. - * @param string $fileName The file name being uploaded. - * @param ServerRequestInterface $request The incoming request. - * @param ?string $version The user submitted file version. - * @param bool $overwrite Flag to indicate if existing file - * should be overwritten. + * @param \LORIS\FilesUploadHandler $files The FilesUploadHandler which + * moves the file and generates + * the response. + * @param \User $user The user uploading the file. + * @param string $fileName The file name being uploaded. + * @param ServerRequestInterface $request The incoming request. + * @param ?string $version The user submitted file version. + * @param bool $overwrite Flag to indicate if existing + * file should be overwritten. + * @param ?string $projectName The name of the project the file + * belongs to * * @return ResponseInterface */ @@ -198,13 +201,20 @@ class Files extends \NDB_Page string $fileName, ServerRequestInterface $request, ?string $version, - bool $overwrite + bool $overwrite, + ?string $projectName ) : ResponseInterface { $DB = $this->loris->getDatabaseConnection(); if ($version !== null) { $version = strtolower($version); } + // Get ProjectID + $ProjectID = $DB->pselectOne( + "SELECT ProjectID FROM Project WHERE Name=:project", + ['project' => $projectName] + ); + $upload_date = date('Y-m-d'); if ($overwrite) { // update file in data_release table. @@ -213,6 +223,7 @@ class Files extends \NDB_Page [ 'version' => $version, 'upload_date' => $upload_date, + 'ProjectID' => $ProjectID, ], ['file_name' => $fileName] ); @@ -224,6 +235,7 @@ class Files extends \NDB_Page 'file_name' => $fileName, 'version' => $version, 'upload_date' => $upload_date, + 'ProjectID' => $ProjectID, ] ); diff --git a/raisinbread/RB_files/RB_data_release.sql b/raisinbread/RB_files/RB_data_release.sql index ee9b87faa9b..53a07f1e90a 100644 --- a/raisinbread/RB_files/RB_data_release.sql +++ b/raisinbread/RB_files/RB_data_release.sql @@ -1,6 +1,6 @@ SET FOREIGN_KEY_CHECKS=0; TRUNCATE TABLE `data_release`; LOCK TABLES `data_release` WRITE; -INSERT INTO `data_release` (`id`, `file_name`, `version`, `upload_date`) VALUES (1,'46-054.gif','','2018-05-07'); +INSERT INTO `data_release` (`id`, `file_name`, `version`, `upload_date`, `ProjectID`) VALUES (1,'46-054.gif','','2018-05-07', 1); UNLOCK TABLES; SET FOREIGN_KEY_CHECKS=1;