Skip to content

Commit

Permalink
[Instrument] Remove exception from NDB_BVL_Instrument::factory
Browse files Browse the repository at this point in the history
This removes the exception from NDB_BVL_Instrument::factory(), so that
it's possible to call hasAccess on an instrument. Currently the hasAccess
or Factory call mix concerns, making it difficult for a module to check
if a user has access to an instrument.
  • Loading branch information
driusan committed Feb 15, 2023
1 parent 9286da1 commit 473bfea
Show file tree
Hide file tree
Showing 8 changed files with 45 additions and 18 deletions.
8 changes: 7 additions & 1 deletion htdocs/survey.php
Expand Up @@ -103,10 +103,16 @@ function initialize()
throw new Exception("Data has already been submitted.", 403);
}

$instrumentObj = \NDB_BVL_Instrument::factory(
$instrumentObj = \NDB_BVL_Instrument::factory(
$this->loris,
$this->TestName,
);

$user = \User::singleton();
if ($instrumentObj->_hasAccess($user) !== true) {
throw new \Exception("Permission denied", 403);
}

$subtests = $instrumentObj->getSubtestList();
$this->NumPages = count($subtests) + 1;

Expand Down
Expand Up @@ -131,6 +131,11 @@ class Instruments extends Endpoint implements \LORIS\Middleware\ETagCalculator
'',
true
);

$user = $request->getAttribute('user');
if ($instrument->_hasAccess($user) == false) {
return new \LORIS\Http\Response\JSON\Forbidden();
};
} catch (\Exception $e) {
return new \LORIS\Http\Response\JSON\NotFound();
}
Expand Down
5 changes: 5 additions & 0 deletions modules/api/php/endpoints/project/instruments.class.inc
Expand Up @@ -108,6 +108,11 @@ class Instruments extends Endpoint implements \LORIS\Middleware\ETagCalculator
'',
true
);

$user = $request->getAttribute("user");
if ($instrument->_hasAccess($user) == false) {
return new \LORIS\Http\Response\JSON\Forbidden();
}
} catch (\Exception $e) {
return new \LORIS\Http\Response\JSON\NotFound();
}
Expand Down
5 changes: 5 additions & 0 deletions modules/conflict_resolver/php/endpoints/unresolved.class.inc
Expand Up @@ -252,6 +252,11 @@ class Unresolved extends Endpoint implements ETagCalculator
'',
true
);
if ($instrument->_hasAccess($user) == false) {
return new \LORIS\Http\Response\JSON\NotFound(
'Permission denied for ' . $conflict['TableName']
);
}
$instrument->_saveValues(
[$conflict['FieldName'] => $conflict['CorrectAnswer']]
);
Expand Down
8 changes: 8 additions & 0 deletions modules/datadict/php/datadict.class.inc
Expand Up @@ -60,6 +60,7 @@ class Datadict extends \DataFrameworkMenu
{
$instruments = \Utility::getAllInstruments();
$dictInstruments = [];

foreach ($instruments as $instrument => $name) {
// Since the testname does not always match the table name in the
// database we need to instantiate the object to get the table name
Expand All @@ -70,6 +71,13 @@ class Datadict extends \DataFrameworkMenu
'',
''
);
if ($iObj->_hasAccess($user)) {
$this->logger->debug(
"Skipping $instrument in field options"
. " because user does not have permission"
);
continue;
}
} catch (\Exception $e) {
error_log(
"There was a problem instantiating the instrument ".
Expand Down
8 changes: 7 additions & 1 deletion modules/instruments/php/module.class.inc
Expand Up @@ -84,6 +84,13 @@ class Module extends \Module
$page
);

$user = $request->getAttribute("user");
if ($instrument->_hasAccess($user) == false) {
return (new \Laminas\Diactoros\Response())
->withStatus(403)
->withBody(new \LORIS\Http\StringStream("Permission denied"));
}

$request = $request->withAttribute('pageclass', $instrument);

return $instrument->process($request, $instrument);
Expand Down Expand Up @@ -151,4 +158,3 @@ class Module extends \Module
return [];
}
}

9 changes: 7 additions & 2 deletions modules/instruments/php/visitsummary.class.inc
Expand Up @@ -52,6 +52,7 @@ class VisitSummary extends \NDB_Page
return $this->_handleGET(
new CandID($params['CandID']),
$params['VisitLabel'],
$user,
);
default:
return new \LORIS\Http\Response\JSON\MethodNotAllowed(
Expand All @@ -61,15 +62,16 @@ class VisitSummary extends \NDB_Page
}

/**
* Helper to specifically handle PATCH HTTP methods to the endpoint.
* Helper to specifically handle GET HTTP methods to the endpoint.
*
* @param CandID $candid The CandID for the visit.
* @param string $visitLabel The visit label string.
* @param \User $user The user accessing the endpoint
*
* @return ResponseInterface
*/
private function _handleGET(
CandID $candid, string $visitLabel
CandID $candid, string $visitLabel, \User $user
) : ResponseInterface {
$DB = \NDB_Factory::singleton()->database();

Expand Down Expand Up @@ -111,6 +113,9 @@ class VisitSummary extends \NDB_Page
'',
false
);
if ($instrument->_hasAccess($user) == false) {
continue;
}
if ($instrument === null) {
$bvl_result[$key]['Completion'] = 0;
} else {
Expand Down
15 changes: 1 addition & 14 deletions php/libraries/NDB_BVL_Instrument.class.inc
Expand Up @@ -308,17 +308,6 @@ abstract class NDB_BVL_Instrument extends NDB_Page
$base . "project/instruments/$instrument.rules"
);

$user = \User::singleton();
$access = $obj->_hasAccess($user);

// check that user has access
if ($access == false) {
throw new Exception(
"You do not have access to this page.",
403
);
}

return $obj;
}

Expand Down Expand Up @@ -380,9 +369,7 @@ abstract class NDB_BVL_Instrument extends NDB_Page
return true;
}
}

//no user permissions match required instrument permissions
throw new Exception("You do not have access to this page.", 403);
return false;
}
}

Expand Down

0 comments on commit 473bfea

Please sign in to comment.