Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[media] fixes permissions. Allows media_read to download, renames "Delete" to "Hide". #9222

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

[media] fixes permissions. Allows media_read to download, renames "Delete" to "Hide". #9222

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
81c6e3f
[media] fixes permissions. Allows media_read to download, renames "De…
Apr 25, 2024
1b5ff78
Adds "Edit/Upload/Hide" permission in place of renaming "Edit/Upload/…
May 23, 2024
2ff7fd1
fixing typo/bug permission should be write not read for uploading.
May 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions SQL/0000-00-02-Permission.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ CREATE TABLE `permissions` (
'View/Create/Edit',
'Create/Edit',
'Edit/Upload',
'Edit/Upload/Delete'),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we want to change this? This is the generic category used for all module permissions.

@ridz1208

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@racostas @driusan I'm okai with changing it if it's the only module using it.

Or, to be safe, just create a new one

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@driusan , @ridz1208 I'm fine with both solutions. Here the considerations:

  1. The only module that is using this permission it's in facts publication.
  2. We can create a new permission and leave the old one, just the already existing permission will be there but not used in any other part.
  3. We should consider also that some projects might be using this permission (projects that we might not aware)

I just want to reconfirm with you. After this note do we want to leave the old permission there and create a new one or should we modify it since it's "the only module" that its using it ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@racostas Create a new one please

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great !

'Edit/Upload/Hide'),
`categoryID` int(10) NOT NULL DEFAULT '2',
PRIMARY KEY (`permID`),
UNIQUE KEY `code` (`code`),
Expand Down Expand Up @@ -102,7 +102,7 @@ INSERT INTO `permissions` VALUES
(31,'acknowledgements_edit','Acknowledgee List',(SELECT ID FROM modules WHERE Name='acknowledgements'),'Edit','2'),
(32,'dataquery_view','Cross-Modality Data',(SELECT ID FROM modules WHERE Name='dataquery'),'View/Download','2'),
(33,'genomic_data_manager','Genomic Files',(SELECT ID FROM modules WHERE Name='genomic_browser'),'Upload','2'),
(34,'media_write','Candidate Media Files',(SELECT ID FROM modules WHERE Name='media'),'Edit/Upload/Delete','2'),
(34,'media_write','Candidate Media Files',(SELECT ID FROM modules WHERE Name='media'),'Edit/Upload/Hide','2'),
(35,'media_read','Candidate Media Files',(SELECT ID FROM modules WHERE Name='media'),'View/Download','2'),
(36,'issue_tracker_reporter', 'Create/Edit Own Issues and Comment on All Issues',(SELECT ID FROM modules WHERE Name='issue_tracker'),NULL, 2),
(37,'issue_tracker_developer', 'Close/Edit/Re-assign/Comment on All Issues',(SELECT ID FROM modules WHERE Name='issue_tracker'),NULL, 2),
Expand Down
2 changes: 1 addition & 1 deletion modules/media/ajax/FileUpload.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ function uploadFile()
$db = \NDB_Factory::singleton()->database();
$config = NDB_Config::singleton();
$user =& User::singleton();
if (!$user->hasPermission('media_write')) {
if (!$user->hasPermission('media_read')) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@racostas why is uploading being changed to read ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's looks as typo/bug from my side :) but will retest before

showMediaError("Permission Denied", 403);
exit;
}
Expand Down
2 changes: 1 addition & 1 deletion modules/media/jsx/mediaIndex.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ class MediaIndex extends Component {
let result = <td className={style}>{cell}</td>;
switch (column) {
case 'File Name':
if (this.props.hasPermission('media_write')) {
if (this.props.hasPermission('media_read')) {
const downloadURL = loris.BaseURL
+ '/media/files/'
+ encodeURIComponent(row['File Name']);
Expand Down
5 changes: 1 addition & 4 deletions modules/media/php/files.class.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,7 @@ class Files extends \LORIS\Http\FilesPassthroughEndpoint
*/
function _hasAccess(\User $user): bool
{
//XXX: Should this be 'media_read' instead? It seems that downloading
// files should be a read permission, not write.. but this is the
// permission that the old ajax script was checking.
return $user->hasPermission('media_write');
return $user->hasPermission('media_read');
}

/**
Expand Down