Skip to content

security: Enforce candidate/attachment access checks and centralize authorization logic #828

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

Open
RussH wants to merge 1 commit into
base: master
Choose a base branch
from
Open

security: Enforce candidate/attachment access checks and centralize authorization logic #828

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
58 changes: 58 additions & 0 deletions lib/CandidateAuthorization.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
<?php
/**
* Shared authorization helpers for candidate object-level access checks.
*/

include_once(LEGACY_ROOT . '/lib/Candidates.php');
include_once(LEGACY_ROOT . '/lib/Attachments.php');

class CandidateAuthorization
{
public static function getCandidate($candidateID)
{
$candidates = new Candidates();
return $candidates->get($candidateID);
}

public static function canAccessCandidate($candidateID, &$candidate = null)

Check warning on line 17 in lib/CandidateAuthorization.php

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

lib/CandidateAuthorization.php#L17 

canAccessCandidate accesses the super-global variable $_SESSION.
{
$candidate = self::getCandidate($candidateID);
if (empty($candidate))
{
return false;
}

if ($candidate['isAdminHidden'] == 1 &&
$_SESSION['CATS']->getAccessLevel('candidates.hidden') < ACCESS_LEVEL_SA)
{
return false;
}

return true;
}

public static function getAttachment($attachmentID)
{
$attachments = new Attachments();
return $attachments->get($attachmentID);
}

public static function canAccessCandidateAttachment($attachmentID, &$attachment = null)
{
$attachment = self::getAttachment($attachmentID);
if (!isset($attachment['attachmentID']))
{
return false;
}

if ($attachment['dataItemType'] == DATA_ITEM_CANDIDATE)
{
$candidate = null;
return self::canAccessCandidate($attachment['dataItemID'], $candidate);
}

return true;
}
}

?>
12 changes: 11 additions & 1 deletion modules/attachments/AttachmentsUI.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

include_once(LEGACY_ROOT . '/lib/CommonErrors.php');
include_once(LEGACY_ROOT . '/lib/Attachments.php');
include_once(LEGACY_ROOT . '/lib/CandidateAuthorization.php');

class AttachmentsUI extends UserInterface
{
Expand Down Expand Up @@ -83,14 +84,23 @@
$attachments = new Attachments();
$rs = $attachments->get($attachmentID, false);

if (empty($rs) || md5($rs['directoryName']) != $_GET['directoryNameHash'])
if (!isset($rs['attachmentID']) || md5($rs['directoryName']) != $_GET['directoryNameHash'])
{
CommonErrors::fatal(
COMMONERROR_BADFIELDS,
$this,
'Invalid id / directory / filename, or you do not have permission to access this attachment.'
);
}

if (!CandidateAuthorization::canAccessCandidateAttachment($attachmentID, $rs))

Check warning on line 96 in modules/attachments/AttachmentsUI.php

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

modules/attachments/AttachmentsUI.php#L96 

Avoid using static access to class 'CandidateAuthorization' in method 'getAttachment'.
{
CommonErrors::fatal(
COMMONERROR_PERMISSION,
$this,
'Invalid user level for action.'
);
}

$directoryName = $rs['directoryName'];
$fileName = $rs['storedFilename'];
Expand Down
96 changes: 59 additions & 37 deletions modules/candidates/CandidatesUI.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
include_once(LEGACY_ROOT . '/lib/ResultSetUtility.php');
include_once(LEGACY_ROOT . '/lib/DateUtility.php'); /* Depends on StringUtility. */
include_once(LEGACY_ROOT . '/lib/Candidates.php');
include_once(LEGACY_ROOT . '/lib/CandidateAuthorization.php');
include_once(LEGACY_ROOT . '/lib/Pipelines.php');
include_once(LEGACY_ROOT . '/lib/Attachments.php');
include_once(LEGACY_ROOT . '/lib/ActivityEntries.php');
Expand Down Expand Up @@ -890,8 +891,7 @@ private function add($contents = '', $fields = array())
{
$associatedAttachment = $_GET['attachmentID'];

$attachments = new Attachments();
$associatedAttachmentRS = $attachments->get($associatedAttachment);
$associatedAttachmentRS = $this->enforceAttachmentCandidateHiddenAccess($associatedAttachment);

/* Show an attachment icon based on the document's file type. */
$attachmentIcon = strtolower(
Expand Down Expand Up @@ -1277,6 +1277,9 @@ private function onEdit()
return;
}

$candidateID = $_POST['candidateID'];
$this->enforceCandidateHiddenAccess($candidateID);

/* Bail out if we don't have a valid owner user ID. */
if (!$this->isOptionalIDValid('owner', $_POST))
{
Expand Down Expand Up @@ -1339,7 +1342,6 @@ private function onEdit()
$phoneWork = $this->getTrimmedInput('phoneWork', $_POST);
}

$candidateID = $_POST['candidateID'];
$owner = $_POST['owner'];

/* Can Relocate */
Expand Down Expand Up @@ -1520,6 +1522,7 @@ private function onDelete()
}

$candidateID = $_POST['candidateID'];
$this->enforceCandidateHiddenAccess($candidateID);

if (!eval(Hooks::get('CANDIDATE_DELETE'))) return;

Expand Down Expand Up @@ -1574,6 +1577,8 @@ private function considerForJobSearch($candidateIDArray = array())
CommonErrors::fatalModal(COMMONERROR_BADINDEX, $this, 'Invalid candidate ID.');
return;
}

$this->enforceCandidateHiddenAccess($candidateID);
}

/* Bail out to prevent an error if the POST string doesn't even contain
Expand Down Expand Up @@ -1713,6 +1718,11 @@ private function onAddToPipeline()
}


foreach ($candidateIDArray as $candidateID)
{
$this->enforceCandidateHiddenAccess($candidateID);
}

$jobOrderID = $_POST['jobOrderID'];

if (!eval(Hooks::get('CANDIDATE_ADD_TO_PIPELINE_PRE'))) return;
Expand Down Expand Up @@ -1785,16 +1795,7 @@ private function addActivity()
$selectedJobOrderID = -1;
}
$candidateID = $_GET['candidateID'];

$candidates = new Candidates();
$candidateData = $candidates->get($candidateID);

/* Bail out if we got an empty result set. */
if (empty($candidateData))
{
CommonErrors::fatalModal(COMMONERROR_BADINDEX, $this);
return;
}
$candidateData = $this->enforceCandidateHiddenAccess($candidateID);

$pipelines = new Pipelines();
$pipelineRS = $pipelines->getNonClosedCandidatePipeline($candidateID);
Expand Down Expand Up @@ -1875,16 +1876,7 @@ private function changeStatus()
$selectedJobOrderID = -1;
}
$candidateID = $_GET['candidateID'];

$candidates = new Candidates();
$candidateData = $candidates->get($candidateID);

/* Bail out if we got an empty result set. */
if (empty($candidateData))
{
CommonErrors::fatalModal(COMMONERROR_BADINDEX, $this);
return;
}
$candidateData = $this->enforceCandidateHiddenAccess($candidateID);

$pipelines = new Pipelines();
$pipelineRS = $pipelines->getCandidatePipeline($candidateID);
Expand Down Expand Up @@ -2002,6 +1994,7 @@ private function onAddCandidateTags()
}

$candidateID = $_POST['candidateID'];
$this->enforceCandidateHiddenAccess($candidateID);
$tagIDs = $_POST['candidate_tags'];

$tags = new Tags();
Expand All @@ -2025,19 +2018,7 @@ private function addCandidateTags()
}

$candidateID = $_GET['candidateID'];

$candidates = new Candidates();
$candidateData = $candidates->get($candidateID);

/* Bail out if we got an empty result set. */
if (empty($candidateData))
{
CommonErrors::fatalModal(COMMONERROR_BADINDEX, $this);
return;
/*$this->fatalModal(
'The specified candidate ID could not be found.'
);*/
}
$this->enforceCandidateHiddenAccess($candidateID);

$tags = new Tags();
$tagsRS = $tags->getAll();
Expand Down Expand Up @@ -2099,6 +2080,7 @@ private function onRemoveFromPipeline()
}

$candidateID = $_POST['candidateID'];
$this->enforceCandidateHiddenAccess($candidateID);
$jobOrderID = $_POST['jobOrderID'];

if (!eval(Hooks::get('CANDIDATE_REMOVE_FROM_PIPELINE_PRE'))) return;
Expand Down Expand Up @@ -2460,6 +2442,8 @@ private function viewResume()

if (!empty($data))
{
$this->enforceCandidateHiddenAccess($data['candidateID']);

/* Keyword highlighting. */
$data['text'] = SearchUtility::makePreview($query, $data['text']);
}
Expand All @@ -2471,6 +2455,28 @@ private function viewResume()
$this->_template->display('./modules/candidates/ResumeView.tpl');
}

private function enforceCandidateHiddenAccess($candidateID)
{
$candidate = null;
if (!CandidateAuthorization::canAccessCandidate($candidateID, $candidate))
{
CommonErrors::fatalModal(COMMONERROR_PERMISSION, $this, 'Invalid user level for action.');
}

return $candidate;
}

private function enforceAttachmentCandidateHiddenAccess($attachmentID)
{
$attachment = null;
if (!CandidateAuthorization::canAccessCandidateAttachment($attachmentID, $attachment))
{
CommonErrors::fatalModal(COMMONERROR_PERMISSION, $this, 'Invalid user level for action.');
}

return $attachment;
}

private function addEditImage()
{
/* Bail out if we don't have a valid candidate ID. */
Expand All @@ -2480,6 +2486,7 @@ private function addEditImage()
}

$candidateID = $_GET['candidateID'];
$this->enforceCandidateHiddenAccess($candidateID);

$attachments = new Attachments();
$attachmentsRS = $attachments->getAll(
Expand Down Expand Up @@ -2508,6 +2515,7 @@ private function onAddEditImage()
}

$candidateID = $_POST['candidateID'];
$this->enforceCandidateHiddenAccess($candidateID);

if (!eval(Hooks::get('CANDIDATE_ON_ADD_EDIT_IMAGE_PRE'))) return;

Expand Down Expand Up @@ -2545,6 +2553,7 @@ private function createAttachment()
}

$candidateID = $_GET['candidateID'];
$this->enforceCandidateHiddenAccess($candidateID);

if (!eval(Hooks::get('CANDIDATE_CREATE_ATTACHMENT'))) return;

Expand Down Expand Up @@ -2574,6 +2583,7 @@ private function onCreateAttachment()
}

$candidateID = $_POST['candidateID'];
$this->enforceCandidateHiddenAccess($candidateID);

if ($_POST['resume'] == '1')
{
Expand Down Expand Up @@ -2639,6 +2649,15 @@ private function onDeleteAttachment()
$candidateID = $_POST['candidateID'];
$attachmentID = $_POST['attachmentID'];

$attachment = $this->enforceAttachmentCandidateHiddenAccess($attachmentID);
if ($attachment['dataItemType'] != DATA_ITEM_CANDIDATE ||
$attachment['dataItemID'] != $candidateID)
{
CommonErrors::fatalModal(COMMONERROR_BADINDEX, $this, 'Invalid attachment ID.');
}

$this->enforceCandidateHiddenAccess($attachment['dataItemID']);

if (!eval(Hooks::get('CANDIDATE_ON_DELETE_ATTACHMENT_PRE'))) return;

$attachments = new Attachments();
Expand Down Expand Up @@ -2958,6 +2977,7 @@ private function _addCandidate($isModal, $directoryOverride = '')
if (isset($_POST['associatedAttachment']))
{
$attachmentID = $_POST['associatedAttachment'];
$this->enforceAttachmentCandidateHiddenAccess($attachmentID);

$attachments = new Attachments();
$attachments->setDataItemID($attachmentID, $candidateID, DATA_ITEM_CANDIDATE);
Expand Down Expand Up @@ -3160,6 +3180,7 @@ private function _addActivity($isJobOrdersMode, $regardingID,
}

$candidateID = $_POST['candidateID'];
$this->enforceCandidateHiddenAccess($candidateID);

if (!eval(Hooks::get('CANDIDATE_ON_ADD_ACTIVITY_CHANGE_STATUS_PRE'))) return;

Expand Down Expand Up @@ -3531,6 +3552,7 @@ private function _changeStatus($isJobOrdersMode, $regardingID,
}

$candidateID = $_POST['candidateID'];
$this->enforceCandidateHiddenAccess($candidateID);

if (!eval(Hooks::get('CANDIDATE_ON_ADD_ACTIVITY_CHANGE_STATUS_PRE'))) return;

Expand Down Expand Up @@ -3786,8 +3808,8 @@ private function onShowQuestionnaire()
CommonErrors::fatal(COMMONERROR_BADINDEX, $this, 'Bad Server Information.');
}

$cData = $this->enforceCandidateHiddenAccess($candidateID);
$candidates = new Candidates();
$cData = $candidates->get($candidateID);

$questionnaire = new Questionnaire();
$qData = $questionnaire->getCandidateQuestionnaire($candidateID, $title);
Expand Down
Loading
Loading