Skip to content

Commit be27b1f

Browse files
committed
[Instrument] Remove exception from NDB_BVL_Instrument::factory
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.
1 parent a286050 commit be27b1f

File tree

8 files changed

+46
-16
lines changed

8 files changed

+46
-16
lines changed

htdocs/survey.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,11 @@ function initialize()
107107
$this->loris,
108108
$this->TestName,
109109
);
110+
$user = \User::singleton();
111+
if ($instrumentObj->_hasAccess($user) !== true) {
112+
throw new \Exception("Permission denied", 403);
113+
}
114+
110115
$subtests = $instrumentObj->getSubtestList();
111116
$this->NumPages = count($subtests) + 1;
112117

modules/api/php/endpoints/candidate/visit/instruments.class.inc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,10 @@ class Instruments extends Endpoint implements \LORIS\Middleware\ETagCalculator
131131
'',
132132
true
133133
);
134+
$user = $request->getAttribute('user');
135+
if($instrument->_hasAccess($user) == false) {
136+
return new \LORIS\Http\Response\JSON\Forbidden();
137+
};
134138
} catch (\Exception $e) {
135139
return new \LORIS\Http\Response\JSON\NotFound();
136140
}

modules/api/php/endpoints/project/instruments.class.inc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,10 @@ class Instruments extends Endpoint implements \LORIS\Middleware\ETagCalculator
108108
'',
109109
true
110110
);
111+
$user = $request->getAttribute("user");
112+
if ($instrument->_hasAccess($user) == false) {
113+
return new \LORIS\Http\Response\JSON\Forbidden();
114+
}
111115
} catch (\Exception $e) {
112116
return new \LORIS\Http\Response\JSON\NotFound();
113117
}

modules/conflict_resolver/php/endpoints/unresolved.class.inc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,11 @@ class Unresolved extends Endpoint implements ETagCalculator
252252
'',
253253
true
254254
);
255+
if ($instrument->_hasAccess($user) == false) {
256+
return new \LORIS\Http\Response\JSON\NotFound(
257+
'Permission denied for ' . $conflict['TableName']
258+
);
259+
}
255260
$instrument->_saveValues(
256261
[$conflict['FieldName'] => $conflict['CorrectAnswer']]
257262
);

modules/datadict/php/datadict.class.inc

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ class Datadict extends \DataFrameworkMenu
6161
$instruments = \Utility::getAllInstruments();
6262
$dictInstruments = [];
6363
$lorisinstance = $this->lorisinstance;
64+
$user = \User::singleton();
6465
foreach ($instruments as $instrument => $name) {
6566
// Since the testname does not always match the table name in the
6667
// database we need to instantiate the object to get the table name
@@ -71,6 +72,13 @@ class Datadict extends \DataFrameworkMenu
7172
'',
7273
''
7374
);
75+
if ($iObj->_hasAccess($user)) {
76+
$this->logger->debug(
77+
"Skipping $instrument in field options"
78+
. " because user does not have permission"
79+
);
80+
continue;
81+
}
7482
} catch (\Exception $e) {
7583
error_log(
7684
"There was a problem instantiating the instrument ".

modules/instruments/php/module.class.inc

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,12 @@ class Module extends \Module
8383
$commentID,
8484
$page
8585
);
86+
$user = $request->getAttribute("user");
87+
if ($instrument->_hasAccess($user) == false) {
88+
return (new \Laminas\Diactoros\Response())
89+
->withStatus(403)
90+
->withBody(new \LORIS\Http\StringStream("Permission denied"));
91+
}
8692

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

@@ -165,9 +171,15 @@ class Module extends \Module
165171
$tests = array_keys(\Utility::getAllInstruments());
166172

167173
$dict = [];
174+
$user = \User::singleton();
168175
foreach ($tests as $testname) {
169176
try {
170177
$inst = \NDB_BVL_Instrument::factory($loris, $testname, "", "");
178+
if ($inst->_hasAccess($user) == false) {
179+
$this->logger->debug("Skipping dictionary for $testname"
180+
. " due to permissions.");
181+
continue;
182+
}
171183
$cat = new \LORIS\Data\Dictionary\Category(
172184
$testname,
173185
$inst->getFullName()

modules/instruments/php/visitsummary.class.inc

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ class VisitSummary extends \NDB_Page
5252
return $this->_handleGET(
5353
new CandID($params['CandID']),
5454
$params['VisitLabel'],
55+
$user,
5556
);
5657
default:
5758
return new \LORIS\Http\Response\JSON\MethodNotAllowed(
@@ -61,15 +62,16 @@ class VisitSummary extends \NDB_Page
6162
}
6263

6364
/**
64-
* Helper to specifically handle PATCH HTTP methods to the endpoint.
65+
* Helper to specifically handle GET HTTP methods to the endpoint.
6566
*
6667
* @param CandID $candid The CandID for the visit.
6768
* @param string $visitLabel The visit label string.
69+
* @param \User $user The user accessing the endpoint
6870
*
6971
* @return ResponseInterface
7072
*/
7173
private function _handleGET(
72-
CandID $candid, string $visitLabel
74+
CandID $candid, string $visitLabel, \User $user
7375
) : ResponseInterface {
7476
$DB = \NDB_Factory::singleton()->database();
7577

@@ -111,6 +113,9 @@ class VisitSummary extends \NDB_Page
111113
'',
112114
false
113115
);
116+
if ($instrument->_hasAccess($user) == false) {
117+
continue;
118+
}
114119
if ($instrument === null) {
115120
$bvl_result[$key]['Completion'] = 0;
116121
} else {

php/libraries/NDB_BVL_Instrument.class.inc

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -308,17 +308,6 @@ abstract class NDB_BVL_Instrument extends NDB_Page
308308
$base . "project/instruments/$instrument.rules"
309309
);
310310

311-
$user = \User::singleton();
312-
$access = $obj->_hasAccess($user);
313-
314-
// check that user has access
315-
if ($access == false) {
316-
throw new Exception(
317-
"You do not have access to this page.",
318-
403
319-
);
320-
}
321-
322311
return $obj;
323312
}
324313

@@ -380,9 +369,7 @@ abstract class NDB_BVL_Instrument extends NDB_Page
380369
return true;
381370
}
382371
}
383-
384-
//no user permissions match required instrument permissions
385-
throw new Exception("You do not have access to this page.", 403);
372+
return false;
386373
}
387374
}
388375

0 commit comments

Comments
 (0)