Skip to content

[dicom archive] add project permission check based on tarchiveID #9359

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

Merged
driusan merged 8 commits into from
Feb 17, 2025
Merged

[dicom archive] add project permission check based on tarchiveID #9359

Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5f07844
dicom archive - add project permission check based on tarchiveID
regisoc Sep 24, 2024
dad4227
remove unnecessary line
regisoc Sep 24, 2024
aee5e8a
lint
regisoc Sep 24, 2024
0b8519c
projectID singleton
regisoc Sep 24, 2024
2b59569
use loadResources to preload GET tarchiveID
regisoc Sep 24, 2024
f696d58
lint
regisoc Sep 24, 2024
2fe0229
phan
regisoc Sep 24, 2024
23f1647
Update modules/dicom_archive/php/viewdetails.class.inc
regisoc Feb 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 59 additions & 1 deletion modules/dicom_archive/php/viewdetails.class.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
*/
namespace LORIS\dicom_archive;

use \Psr\Http\Message\ServerRequestInterface;

/**
* Implements the ViewDetails subpage of the dicom_archive module.
*
Expand Down Expand Up @@ -51,7 +53,63 @@ class ViewDetails extends \NDB_Form
*/
function _hasAccess(\User $user) : bool
{
return $user->hasPermission('dicom_archive_view_allsites');
// remove the possibility to have no tarchive ID in this page
if (is_null($this->tarchiveID)) {
// defaults to permission denied
return false;
}

// get project ID from Tarchive ID.
$projectID = $this->_getProjectFromTarchiveID();
if (is_null($projectID)) {
return false;
Copy link
Collaborator

@driusan driusan Sep 24, 2024

Choose a reason for hiding this comment

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

Should this be return true? Otherwise no one will ever be able to see it?

(Maybe a discussion for an imaging meeting?)

Copy link
Contributor Author

@regisoc regisoc Sep 24, 2024

Choose a reason for hiding this comment

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

That means the TarchiveID does not exist in db or is not linked to a project.. ?
I was not sure about this. It is even possible?

Copy link
Contributor Author

@regisoc regisoc Sep 24, 2024

Choose a reason for hiding this comment

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

Maybe we can involve @cmadjar

Copy link
Contributor Author

@regisoc regisoc Sep 24, 2024

Choose a reason for hiding this comment

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

Point added to next imaging meeting.

Copy link
Collaborator

@driusan driusan Sep 25, 2024

Choose a reason for hiding this comment

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

I'm assuming it would mean the TarchiveID is not linked to a project

Copy link
Contributor Author

@regisoc regisoc Oct 4, 2024

Choose a reason for hiding this comment

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

Imaging meeting: no one should see it by default.
There should be a specific permission to see the list of "dangling TarchiveIDs" (Tarchive not assigned to any Project). Also might be good to have a front-end page for that.
It will be linked to a new issue.

Copy link
Contributor Author

@regisoc regisoc Oct 4, 2024

Choose a reason for hiding this comment

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

Created here: #9389

}

// check permissions
return $user->hasPermission('dicom_archive_view_allsites')
&& $user->hasProject($projectID);
}

/**
* {@inheritDoc}
*
* @param \User $user The user this request is for
* @param ServerRequestInterface $request The PSR7 request
*
* @return void
*/
public function loadResources(
\User $user, ServerRequestInterface $request
) : void {
$gets = $request->getQueryParams();
if (is_null($gets['tarchiveID'])) {
$this->tarchiveID = null;
} else {
$this->tarchiveID = intval($gets['tarchiveID']);
}
}

/**
* Get the ProjectID attached to a given tarchive ID.
*
* @return \ProjectID|null a ProjectID if found, else null
*/
private function _getProjectFromTarchiveID(): ?\ProjectID
{
$db = $this->loris->getDatabaseConnection();
$pid = $db->pselectOne(
"SELECT p.ProjectID
FROM tarchive t
JOIN session s ON (t.SessionID = s.ID)
JOIN Project p ON (p.ProjectID = s.ProjectID)
WHERE t.TarchiveID = :tar",
['tar' => $this->tarchiveID]
);
//
if (is_null($pid)) {
return null;
}
return \ProjectID::singleton(intval($pid));
}

/**
Expand Down
Loading