Skip to content

Commit

Permalink
[API] Fix PUT request for Candidates \visit endpoint and integration …
Browse files Browse the repository at this point in the history 
…test (#7088)

The PUT request should never allow to edit a candidate from a Site where the User has no affiliation with. In this case, the test User has only affiliation with Data Coordinating Center (DCC). It is however possible to do so if the new Site is DCC.

For example, in raisinbread, candidate 300001 (https://<hostname>.loris.ca/api/v0.0.4-dev/candidates/300001) is from site Montreal. The test user should not be able to modify this candidate, because the candidate is from Montreal, but is able to.

Also adds automatic the integration tests to test the changes suggested.


    Resolves #7106
  • Loading branch information
@spell00
spell00 authored Nov 9, 2020
1 parent 70ae672 commit df29ed8
Show file tree
Hide file tree
Showing 3 changed files with 202 additions and 2 deletions.
9 changes: 8 additions & 1 deletion modules/api/php/endpoints/candidate/visit/visit.class.inc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,11 +215,18 @@ class Visit extends Endpoint implements \LORIS\Middleware\ETagCalculator
$this->_candidate->getListOfVisitLabels()
);

if (!$this->_candidate->isAccessibleBy($user)) {
return new \LORIS\Http\Response\JSON\Forbidden(
"You can't create or modify that candidate"
);
}

$centerid = array_search($visitinfo['Site'], \Utility::getSiteList());

if (!in_array($centerid, $user->getCenterIDs())) {
return new \LORIS\Http\Response\JSON\Forbidden(
'You can`t create candidates visit for that site'
"You can't create or modify candidates visit for the site " .
$centerid
);
}

Expand Down
58 changes: 58 additions & 0 deletions raisinbread/test/api/LorisApiAuthenticatedTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,59 @@ public function setUp()
$this->DB->update('Config', $set, $where);

$this->apiLogin('UnitTester', $this->validPassword);


$this->DB->insert(
"candidate",
[
'CandID' => '900000',
'PSCID' => 'TST0001',
'RegistrationCenterID' => 1,
'RegistrationProjectID' => 1,
'Active' => 'Y',
'UserID' => 1,
'Entity_type' => 'Human',
'Sex' => 'Female'
]
);
$this->DB->insert(
'session',
[
'ID' => '999999',
'CandID' => '900000',
'Visit_label' => 'V1',
'CenterID' => 1,
'ProjectID' => 1,
'Current_stage' => 'Not Started',
]
);
$this->DB->insert(
'test_names',
[
'ID' => '999999',
'Test_name' => 'testtest',
'Full_name' => 'Test Test',
'Sub_group' => 1,
]
);
$this->DB->insert(
'flag',
[
'ID' => '999999',
'SessionID' => '999999',
'Test_name' => 'testtest',
'CommentID' => '11111111111111111',
]
);
$this->DB->insert(
'flag',
[
'ID' => '999999',
'SessionID' => '999999',
'Test_name' => 'testtest',
'CommentID' => 'DDE_11111111111111111',
]
);
}

/**
Expand Down Expand Up @@ -131,6 +184,11 @@ function testLoginSuccess()
*/
public function tearDown()
{
$this->DB->delete("session", ['CandID' => '900000']);
$this->DB->delete("candidate", ['CandID' => '900000']);
$this->DB->delete("flag", ['ID' => '999999']);
$this->DB->delete("test_names", ['ID' => '999999']);

$set = [
'Value' => $this->originalJwtKey
];
Expand Down
137 changes: 136 additions & 1 deletion raisinbread/test/api/LorisApiVisitsTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,142 @@ public function testGetCandidatesCandidVisit(): void
*/
public function testPutCandidatesCandidVisit(): void
{
$this->markTestSkipped('No access to create visits for any site');
// Test changing the Project & Battery
$json = ['CandID' => '900000',
'Visit' => "V1",
'Site' => "Data Coordinating Center",
'Battery' => "High Yeast",
'Project' => "Rye",
];
$response = $this->client->request(
'PUT',
"candidates/900000/V1",
[
'headers' => $this->headers,
'json' => $json
]
);
// Verify the status code
$this->assertEquals(204, $response->getStatusCode());
// Verify the endpoint has a body
$body = $response->getBody();
$this->assertNotEmpty($body);

/**
* Test changing from a site with no affiliation to a site with affiliation
* Candidate 400266 is from site Rome. The test user only has access to
* Data Coordinating Center. He should not be able to modify a visit
* for a candidate from a site he has no access to.
*/
$json = ['CandID' => '400266',
'Visit' => "V3",
'Site' => "Data Coordinating Center",
'Battery' => "Stale",
'Project' => "Pumpernickel",
];
$response = $this->client->request(
'PUT',
"candidates/400266/V3",
[
'headers' => $this->headers,
'json' => $json,
'http_errors' => false
]
);
// Verify the status code
$this->assertEquals(403, $response->getStatusCode());
// Verify the endpoint has a body
$body = $response->getBody();
$this->assertNotEmpty($body);

// Test changing the Battery from a visit that is already initiated.
$json = ['CandID' => "115788",
'Visit' => "V3",
'Site' => "Data Coordinating Center",
'Battery' => "Stale",
'Project' => "Pumpernickel",
];
$response = $this->client->request(
'PUT',
"candidates/115788/V3",
[
'headers' => $this->headers,
'json' => $json,
'http_errors' => false
]
);
// verify the status code
$this->assertequals(409, $response->getstatuscode());
// verify the endpoint has a body
$body = $response->getbody();
$this->assertnotempty($body);

// Test assigning site with no affiliation. It changes the site from
// Data Coordinating Center, which the test user has its only affiliation,
// to Montreal, where he has no affiliation.
$json = ['CandID' => '900000',
'Visit' => "V1",
'Site' => "Montreal",
'Battery' => "Stale",
'Project' => "Pumpernickel",
];
$response = $this->client->request(
'PUT',
"candidates/900000/V1",
[
'headers' => $this->headers,
'json' => $json,
'http_errors' => false
]
);
// Verify the status code
$this->assertEquals(403, $response->getStatusCode());
// Verify the endpoint has a body
$body = $response->getBody();
$this->assertNotEmpty($body);

// Test what happen when a field is missing (here, Battery)
$json = ['CandID' => $this->candidTest,
'Visit' => $this->visitTest,
'Site' => "Data Coordinating Center",
'Project' => "Pumpernickel",
];
$response = $this->client->request(
'PUT',
"candidates/$this->candidTest/$this->visitTest",
[
'headers' => $this->headers,
'json' => $json,
'http_errors' => false
]
);
// verify the status code
$this->assertequals(400, $response->getstatuscode());
// verify the endpoint has a body
$body = $response->getbody();
$this->assertnotempty($body);

// Test CandID in URL should match CandID in the request fields
$json = ['CandID' => $this->candidTest,
'Visit' => $this->visitTest,
'Site' => "Montreal",
'Battery' => "Low Yeast",
'Project' => "Pumpernickel",
];
$response = $this->client->request(
'PUT',
"candidates/300001/$this->visitTest",
[
'headers' => $this->headers,
'json' => $json,
'http_errors' => false
]
);
// verify the status code
$this->assertequals(400, $response->getstatuscode());
// verify the endpoint has a body
$body = $response->getbody();
$this->assertnotempty($body);
}

/**
Expand Down

0 comments on commit df29ed8

Please sign in to comment.