Skip to content

Commit

Permalink
[instruments/instrument_manager] Migrate instrument permissions from …
Browse files Browse the repository at this point in the history
…config.xml to database (#8302)

This migrates instrument permissions from the config.xml to the database in a new testnames_permissions_rel table. The table can be managed from the frontend in the instrument_manager module by any user who has the instrument_manager_write permission. The script tools/single_use/migrate_instrument_permissions.php can be used by existing projects to migrate their permissions from the config.xml to the new table.
  • Loading branch information
driusan committed Mar 15, 2023
1 parent 1a6c006 commit 7cb33cf
Show file tree
Hide file tree
Showing 14 changed files with 7,920 additions and 129 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ changes in the following format: PR #1234***
#### Features
- Added new interface intended to be used for querying module data from PHP (PR #8215)
- Added the NOT NULL constraint on Project Name (PR #8295)
- Migrated instrument permissions from config.xml to database and added the ability
to manage instrument permissions in the frontend from the `instrument_manager`
module. (PR #8302)

#### Updates and Improvements
- Rename subproject to Cohort (PR #7817)
Expand Down
4 changes: 4 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,12 @@ check: checkstatic unittests
testdata:
php tools/raisinbread_refresh.php

instrument_manager:
target=instrument_manager npm run compile

login:
target=login npm run compile

mri_violations:
target=mri_violations npm run compile

8 changes: 8 additions & 0 deletions SQL/0000-00-02-Permission.sql
Original file line number Diff line number Diff line change
Expand Up @@ -152,3 +152,11 @@ CREATE TABLE `notification_modules_perm_rel` (
INSERT INTO notification_modules_perm_rel SELECT nm.id, p.permID FROM notification_modules nm JOIN permissions p WHERE nm.module_name='media' AND (p.code='media_write' OR p.code='media_read');
INSERT INTO notification_modules_perm_rel SELECT nm.id, p.permID FROM notification_modules nm JOIN permissions p WHERE nm.module_name='document_repository' AND (p.code='document_repository_view' OR p.code='document_repository_delete');
INSERT INTO notification_modules_perm_rel SELECT nm.id, p.permID FROM notification_modules nm JOIN permissions p WHERE nm.module_name='publication' AND (p.code='publication_view' OR p.code='publication_propose' OR p.code='publication_approve');

CREATE TABLE `testnames_permissions_rel` (
`TestID` int(10) unsigned NOT NULL,
`permID` int(10) unsigned NOT NULL,
PRIMARY KEY (`TestID`,`permID`),
CONSTRAINT `FK_testnames_permissions_rel_test` FOREIGN KEY (`TestID`) REFERENCES `test_names` (`ID`),
CONSTRAINT `FK_testnames_permissions_rel_perm` FOREIGN KEY (`permID`) REFERENCES `permissions` (`permID`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8;
7 changes: 7 additions & 0 deletions SQL/New_patches/2022-12-20-instrumentpermissions.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
CREATE TABLE `testnames_permissions_rel` (
`TestID` int(10) unsigned NOT NULL,
`permID` int(10) unsigned NOT NULL,
PRIMARY KEY (`TestID`,`permID`),
CONSTRAINT `FK_testnames_permissions_rel_test` FOREIGN KEY (`TestID`) REFERENCES `test_names` (`ID`),
CONSTRAINT `FK_testnames_permissions_rel_perm` FOREIGN KEY (`permID`) REFERENCES `permissions` (`permID`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8;
27 changes: 0 additions & 27 deletions docs/config/config.xml
Original file line number Diff line number Diff line change
Expand Up @@ -70,33 +70,6 @@
<test value="InstrumentName">Instrument Display Name</test>
</CertificationInstruments>
</Certification>

</study>
<!-- end of study definition -->

<!-- used by _hasAccess in NDB_BVL_Instrument to determine
what permissions should be required for each instrument
on a configurable basis. -->
<instrumentPermissions>
<!-- By default anyone has permission -->
<useInstrumentPermissions>false</useInstrumentPermissions>
<!-- Add one instrument tag for each instrument that is
having it's permissions configured. If an instrument
is missing, the default is for users to have access
-->
<instrument>
<!-- Instrument name -->
<Test_name>sampleInstrument</Test_name>
<!-- Permission required for accessing instrument.
If multiple permissions are added, *any* of
them individually will allow access to the
instrument -->
<permission>sampleInstrumentPermissionName</permission>
</instrument>
<instrument>
<Test_name>sampleInstrument2</Test_name>
<permission>sampleInstrument2PermissionName</permission>
</instrument>
</instrumentPermissions>

</config>
141 changes: 141 additions & 0 deletions modules/instrument_manager/jsx/instrumentManagerIndex.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@ import FilterableDataTable from 'FilterableDataTable';

import InstrumentUploadForm from './uploadForm';

import Modal from 'jsx/Modal';
import InfoPanel from 'jsx/InfoPanel';

import Select from 'react-select';
import swal from 'sweetalert2';

/**
* Instrument Manager Index component
*/
Expand All @@ -23,6 +29,7 @@ class InstrumentManagerIndex extends Component {
data: {},
error: false,
isLoaded: false,
modifyPermissions: false,
};

this.fetchData = this.fetchData.bind(this);
Expand Down Expand Up @@ -62,6 +69,40 @@ class InstrumentManagerIndex extends Component {
* @return {*} a formated table cell for a given column
*/
formatColumn(column, cell, row) {
if (column == 'Permission Required') {
const clickHandler = (row) => {
return () => {
this.setState({
'modifyPermissions': {
'instrument': row.Instrument,
'permissions': row['Permission Required'],
},
});
};
};
if (cell == null) {
if (this.props.hasPermission('instrument_manager_write')) {
return (<td>No Permissions enforced.
<ButtonElement
label='Add Permissions'
onUserInput={clickHandler(row)}
/>
</td>);
} else {
return <td>No Permissions enforced.</td>;
}
}
if (this.props.hasPermission('instrument_manager_write')) {
return (<td>{cell.join(',')}
<ButtonElement
label='Modify Permissions'
onUserInput={clickHandler(row)}
/>
</td>);
} else {
return <td>{cell.join(',')}</td>;
}
}
return (
<td>{cell}</td>
);
Expand Down Expand Up @@ -114,12 +155,82 @@ class InstrumentManagerIndex extends Component {
name: 'pagesValid',
type: 'text',
}},
{label: 'Permission Required', show: true, filter: {
name: 'permissionsRequired',
type: 'text',
}},
];

const tabs = [
{id: 'browse', label: 'Browse'},
];

let permsModal = null;
if (this.state.modifyPermissions !== false) {
const submitPromise = () => {
return new Promise(
(resolve, reject) => {
fetch(
this.props.BaseURL + '/instrument_manager/permissions',
{
method: 'POST',
body: JSON.stringify(this.state.modifyPermissions),
}).then((response) => {
if (!response.ok) {
console.error(response.status);
throw new Error('Could not modify permissions');
}
return response.json();
}).then( (data) => {
resolve();
this.fetchData();
}).catch((message) => {
swal.fire({
title: 'Oops..',
text: 'Something went wrong!',
type: 'error',
});
reject();
});
});
};

permsModal = (<Modal
title={'Edit Permissions for '
+ this.state.modifyPermissions.instrument}
show={true}
onSubmit={submitPromise}
onClose={
() => {
this.setState({'modifyPermissions': false});
}
}>
<p>Select the permissions required for accessing&nbsp;
{this.state.modifyPermissions.instrument} in the dropdown below.
</p>
<p>Any user accessing the instrument (either for viewing the data
or data entry) must have one of the access permissions selected.
</p>
<InfoPanel>
A user with <em>any</em> of the selected permissions will
be able to access&nbsp;
{this.state.modifyPermissions.instrument}.
If no permissions are selected, the default LORIS permissions
will be enforced for this instrument.
</InfoPanel>

<PermissionSelect
codes={this.state.data.fieldOptions.allPermissionCodes}
selected={this.state.modifyPermissions.permissions}
instrument={this.state.modifyPermissions.instrument}
modifySelected={(newselected) => {
let modifyPermissions = {...this.state.modifyPermissions};
modifyPermissions.permissions = newselected;
this.setState({modifyPermissions});
}}
/>
</Modal>);
}
if (this.props.hasPermission('instrument_manager_write')) {
tabs.push({id: 'upload', label: 'Upload'});
}
Expand Down Expand Up @@ -165,6 +276,7 @@ class InstrumentManagerIndex extends Component {
return (
<Tabs tabs={tabs} defaultTab="browse" updateURL={true}>
<TabPane TabId={tabs[0].id}>
{permsModal}
<FilterableDataTable
name="instrument_manager"
data={this.state.data.Data}
Expand All @@ -181,6 +293,34 @@ class InstrumentManagerIndex extends Component {
}
}

/**
* Create a componenet to select permissions from a list of available
* permissions.
*
* @param {object} props - react props
* @return {JSX}
*/
function PermissionSelect(props) {
const options = props.codes.map((val) => {
return {value: val, label: val};
});
const values = options.filter((row) => {
if (props.selected == null) {
// nothing selected, filter everything
return false;
}
return props.selected.includes(row.value);
});
return <Select
isMulti={true}
options={options}
value={values}
onChange={(newValue) => {
props.modifySelected(newValue.map((row) => row.value));
}}
/>;
}

InstrumentManagerIndex.propTypes = {
dataURL: PropTypes.string.isRequired,
hasPermission: PropTypes.func.isRequired,
Expand All @@ -190,6 +330,7 @@ window.addEventListener('load', () => {
const root = createRoot(document.getElementById('lorisworkspace'));
root.render(
<InstrumentManagerIndex
BaseURL={loris.BaseURL}
dataURL={`${loris.BaseURL}/instrument_manager/?format=json`}
hasPermission={loris.userHasPermission}
/>
Expand Down
31 changes: 27 additions & 4 deletions modules/instrument_manager/php/instrument_manager.class.inc
Original file line number Diff line number Diff line change
Expand Up @@ -247,17 +247,31 @@ class Instrument_Manager extends \NDB_Menu_Filter
*/
function _setupVariables()
{
$this->headers = [
$this->headers = [
'Instrument',
'Instrument_Type',
'Table_Installed',
'Table_Valid',
'Pages_Valid',
'Permissions_Required',
];
$this->columns = ['Test_name as Instrument'];
$this->query = " FROM test_names";

// Most of these are overwritten by toArray
$this->columns = [
'Test_name as Instrument',
"'type' as Instrument_Type",
"'table' as Table_Installed",
"'tablevalid' as Table_Valid",
"'pagevalid' as Pages_Valid",
'GROUP_CONCAT(p.code) as Permissions_Required'
];
$this->query = " FROM test_names tn
LEFT JOIN testnames_permissions_rel rel ON (rel.TestID=tn.ID)
LEFT JOIN permissions p ON (rel.permID=p.permID)";

$this->validFilters = [];
$this->formToFilter = [];
$this->group_by = 'Instrument';
}

/**
Expand All @@ -279,6 +293,12 @@ class Instrument_Manager extends \NDB_Menu_Filter
$row['Table_Installed'] = $this->checkTableInstalled($instrument);
$row['Table_Valid'] = $this->checkTableValid($instrument);
$row['Pages_Valid'] = $this->checkPagesValid($instrument);
if ($row['Permissions_Required'] !== null) {
$row['Permissions_Required'] = explode(
',',
$row['Permissions_Required'],
);
}
return $row;
},
$this->_getFullList()
Expand All @@ -296,10 +316,13 @@ class Instrument_Manager extends \NDB_Menu_Filter
$MappedData[] = array_values($row);
}

$DB = $this->loris->getDatabaseConnection();
$perms = $DB->pselectCol("SELECT code FROM permissions", []);
$fieldOptions = ['allPermissionCodes' => $perms];
return [
'Headers' => $headers,
'Data' => $MappedData,
'fieldOptions' => $this->fieldOptions,
'fieldOptions' => $fieldOptions,
'writable' => $this->canWriteFiles(),
'caninstall' => $this->isAdminUserConfigured(),
];
Expand Down
Loading

0 comments on commit 7cb33cf

Please # to comment.