From da3107b07ff990704b3fec45675b628eccc8798a Mon Sep 17 00:00:00 2001 From: SURESH CHOUKSEY Date: Fri, 15 May 2026 10:06:51 +0530 Subject: [PATCH 1/2] fix: harden Civil ID S3 handling --- .../v1/controllers/AccountController.php | 262 ++++++++++-------- common/components/S3ResourceManager.php | 14 +- common/config/main.php | 8 +- common/models/Candidate.php | 40 ++- .../prod-railway/common/config/main-local.php | 8 +- tests/check-s3-civil-id-hardening.sh | 34 +++ 6 files changed, 230 insertions(+), 136 deletions(-) create mode 100644 tests/check-s3-civil-id-hardening.sh diff --git a/candidate/modules/v1/controllers/AccountController.php b/candidate/modules/v1/controllers/AccountController.php index 171d01ff..5b4fcdf2 100644 --- a/candidate/modules/v1/controllers/AccountController.php +++ b/candidate/modules/v1/controllers/AccountController.php @@ -73,14 +73,14 @@ public function actions() ]; return $actions; } - + /** * return profile details */ public function actionProfile() { return Candidate::findOne(Yii::$app->user->getId()); } - + /** * update candidate experiences * @return array @@ -90,15 +90,15 @@ public function actionUpdateExperiences() $experiences = Yii::$app->request->getBodyParam("experiences"); $experiences = $experiences? explode(',', $experiences): []; - - if (empty($experiences) || count($experiences) == 0) + + if (empty($experiences) || count($experiences) == 0) { return [ "operation" => "error", "message" => Yii::t('candidate', "Experiences Required") ]; } - + CandidateExperience::deleteAll([ 'candidate_id' => Yii::$app->user->getId() ]); @@ -117,11 +117,11 @@ public function actionUpdateExperiences() } } } - + $experienceList = CandidateExperience::find() ->andWhere([ 'candidate_id' => Yii::$app->user->getId() - ]) + ]) ->all(); Yii::$app->user->identity->updateAlgoliaIndex(false); @@ -133,13 +133,13 @@ public function actionUpdateExperiences() "lang" => Yii::$app->language ]; } - + public function actionToggleTwoStepAuth() { $candidate = Yii::$app->user->identity; $candidate->enable_two_step_auth = !$candidate->enable_two_step_auth; - + if (!$candidate->save()) { return [ 'operation' => 'error', @@ -150,10 +150,10 @@ public function actionToggleTwoStepAuth() { return [ 'operation' => 'success', "enable_two_step_auth" => $candidate->enable_two_step_auth, - 'message' => Yii::t('candidate', $candidate->enable_two_step_auth ? - 'Two-step authentication enabled' : + 'message' => Yii::t('candidate', $candidate->enable_two_step_auth ? + 'Two-step authentication enabled' : 'Two-step authentication disabled') - ]; + ]; } /** @@ -175,7 +175,7 @@ public function actionUpdateSkills() "message" => Yii::t('candidate',"Skills Required") ]; } - + CandidateSkill::deleteAll([ 'candidate_id' => Yii::$app->user->getId() ]); @@ -194,11 +194,11 @@ public function actionUpdateSkills() } } } - + $skillList = CandidateSkill::find() ->andWhere([ 'candidate_id' => Yii::$app->user->getId() - ]) + ]) ->all(); Yii::$app->user->identity->updateAlgoliaIndex(false); @@ -209,11 +209,11 @@ public function actionUpdateSkills() "skills" => $skillList ]; } - + /** * send updated candidate video status from db */ - public function actionVideoStatus() + public function actionVideoStatus() { $model = Candidate::findOne(Yii::$app->user->getId()); @@ -224,7 +224,7 @@ public function actionVideoStatus() } /** - * mark video as processed + * mark video as processed */ public function actionVideoByWebhook() { @@ -304,7 +304,7 @@ public function actionVideoByWebhook() { $model->candidate_video = explode('.', $fileName)[0]; $model->candidate_video_processed = true; - + if(!$model->save(false)) { return [ 'operation' => 'error', @@ -322,7 +322,7 @@ public function actionVideoByWebhook() { return [ 'operation' => 'success', - ]; + ]; } /** @@ -334,7 +334,7 @@ public function actionRemoveVideo() { if ($model->candidate_video) { $model->deleteVideo(); } - + $model->candidate_video = null; $model->scenario = 'changeVideo'; @@ -359,7 +359,7 @@ public function actionRemovePhoto() { if ($model->candidate_personal_photo) { $model->deleteProfilePhotoFromCloudinary(); } - + $model->candidate_personal_photo = null; $model->scenario = 'changeProfilePhoto'; @@ -380,21 +380,30 @@ public function actionRemovePhoto() { * Remove civil photo back */ public function actionRemoveCivilPhotoBack() { - + $model = Candidate::findOne(Yii::$app->user->getId()); - - if ($model->candidate_civil_photo_back) { - $model->deleteFile('civil-id', 'back'); - } - - $model->candidate_civil_photo_back = null; - - $model->scenario = 'updateCivilPhotoBack'; - if (!$model->save(false)) { + try { + if ($model->candidate_civil_photo_back) { + $model->deleteFile('civil-id', 'back'); + } + + $model->candidate_civil_photo_back = null; + $model->candidate_civil_need_verification = true; + + $model->scenario = 'updateCivilPhotoBack'; + + if (!$model->save(false)) { + return [ + 'operation' => 'error', + 'message' => $model->getErrors() + ]; + } + } catch (\Exception $e) { + Yii::error($e->getMessage(), 'candidate'); return [ 'operation' => 'error', - 'message' => $model->getErrors() + 'message' => Yii::t('app', 'Unable to remove civil photo back.') ]; } @@ -402,25 +411,34 @@ public function actionRemoveCivilPhotoBack() { 'operation' => 'success', ]; } - + /** * Remove civil photo front */ public function actionRemoveCivilPhotoFront() { $model = Candidate::findOne(Yii::$app->user->getId()); - if ($model->candidate_civil_photo_front) { - $model->deleteFile('civil-id', 'front'); - } - - $model->candidate_civil_photo_front = null; - - $model->scenario = 'updateCivilPhotoFront'; + try { + if ($model->candidate_civil_photo_front) { + $model->deleteFile('civil-id', 'front'); + } + + $model->candidate_civil_photo_front = null; + $model->candidate_civil_need_verification = true; - if (!$model->save(false)) { + $model->scenario = 'updateCivilPhotoFront'; + + if (!$model->save(false)) { + return [ + 'operation' => 'error', + 'message' => $model->getErrors() + ]; + } + } catch (\Exception $e) { + Yii::error($e->getMessage(), 'candidate'); return [ 'operation' => 'error', - 'message' => $model->getErrors() + 'message' => Yii::t('app', 'Unable to remove civil photo front.') ]; } @@ -430,11 +448,11 @@ public function actionRemoveCivilPhotoFront() { } /** - * Update candidate email address + * Update candidate email address * @return type */ public function actionUpdateEmail() { - + $candidate = Candidate::findOne(Yii::$app->user->getId()); $new_email = Yii::$app->request->getBodyParam("email"); @@ -556,7 +574,7 @@ public function actionUpdateBankDetail() { } /** - * Set language preference + * Set language preference */ public function actionLanguagePref() { $language_pref = Yii::$app->request->getBodyParam('language_pref'); @@ -577,15 +595,15 @@ public function actionLanguagePref() { 'operation' => 'success', ]; } - + /** * return job status * @return type */ - public function actionGetJobSearchStatus() + public function actionGetJobSearchStatus() { $model = Yii::$app->user->identity; - + return [ 'candidate_job_search_status' => (int) $model->candidate_job_search_status, 'isProfileCompleted' => $model->isProfileCompleted(), @@ -594,12 +612,12 @@ public function actionGetJobSearchStatus() 'parent_company' => (isset($model->company->parentCompany)) ? $model->company->parentCompany : null ]; } - + /** * Set job search status */ public function actionJobSearchStatus() { - + $job_search_status = Yii::$app->request->getBodyParam('job_search_status'); $model = Candidate::findOne(Yii::$app->user->getId()); @@ -625,7 +643,7 @@ public function actionJobSearchStatus() { 'operation' => 'success', ]; } - + /** * Return a List of Salary transfers */ @@ -718,35 +736,35 @@ public function actionChangePassword() } $model->scenario = 'changePassword'; - - $model->setPassword($newPassword); - + + $model->setPassword($newPassword); + if (!$model->save()) { return [ "operation" => "error", "message" => $model->getErrors() ]; } - + return [ "operation" => "success", "message" => Yii::t('candidate',"Password changed successfully!") ]; } - + /** * update nationality * @return type * @throws \yii\web\HttpException */ public function actionUpdateNationality() { - + $candidate = Candidate::findOne(Yii::$app->user->getId()); if (!$candidate) { throw new \yii\web\HttpException(404, Yii::t('candidate', 'The requested Item could not be found.')); } - + $candidate->country_id = Yii::$app->request->getBodyParam('country_id'); $candidate->scenario = "updateNationality"; @@ -764,20 +782,20 @@ public function actionUpdateNationality() { "message" => Yii::t('candidate', "Candidate Nationality Info Updated Successfully"), ]; } - + /** * update candidate driving license * @return type * @throws \yii\web\HttpException */ public function actionUpdateDrivingLicense() { - + $candidate = Candidate::findOne(Yii::$app->user->getId()); if (!$candidate) { throw new \yii\web\HttpException(404, Yii::t('candidate', 'The requested Item could not be found.')); } - + $candidate->candidate_driving_license = Yii::$app->request->getBodyParam('driving_license'); $candidate->scenario = "updateDrivingLicense"; @@ -795,12 +813,12 @@ public function actionUpdateDrivingLicense() { "message" => Yii::t('candidate', "Candidate Driving License Info Updated Successfully"), ]; } - + /** * Update introductory video */ public function actionVideo() { - + $model = Yii::$app->user->identity; // deleting old video @@ -837,9 +855,9 @@ public function actionVideo() { 'message' => Yii::t('candidate', 'Video Uploaded Successfully') ]; } - + /** - * Update personal photo + * Update personal photo */ public function actionProfilePhoto() { $model = Yii::$app->user->identity; @@ -902,13 +920,13 @@ public function actionUpdateNames() { * @throws \yii\web\HttpException */ public function actionUpdateName() { - + $candidate = Candidate::findOne(Yii::$app->user->getId()); if (!$candidate) { throw new \yii\web\HttpException(404, Yii::t('candidate', 'The requested Item could not be found.')); } - + $candidate->candidate_name = Yii::$app->request->getBodyParam('name'); $candidate->scenario = "updateName"; @@ -964,13 +982,13 @@ public function actionProfileUrl() { * @throws \yii\web\HttpException */ public function actionUpdateNameAr() { - + $candidate = Candidate::findOne(Yii::$app->user->getId()); if (!$candidate) { throw new \yii\web\HttpException(404, Yii::t('candidate', 'The requested Item could not be found.')); } - + $candidate->candidate_name_ar = Yii::$app->request->getBodyParam('name_ar'); $candidate->scenario = "updateNameAr"; @@ -993,7 +1011,7 @@ public function actionUpdateNameAr() { * Update candidate location info */ public function actionUpdateLocation() { - + $candidate = Candidate::findOne(Yii::$app->user->getId()); if (!$candidate) { @@ -1027,35 +1045,35 @@ public function actionUpdateLocation() { } /** - * Return area by geolocation + * Return area by geolocation * @return type */ - public function actionAreaByLocation() + public function actionAreaByLocation() { $latitude = Yii::$app->request->get("latitude"); $longitude = Yii::$app->request->get("longitude"); $area_name = Yii::$app->request->get("area"); - // call google api to get country name, lat, long - + // call google api to get country name, lat, long + $url = 'https://maps.googleapis.com/maps/api/geocode/json?latlng=' . $latitude .','. $longitude; - + return Area::addByGoogleAPIResponse($url, $area_name); } - + /** * update candidate civil id number * @return type * @throws \yii\web\HttpException */ public function actionUpdateCivilId() { - + $candidate = Candidate::findOne(Yii::$app->user->getId()); if (!$candidate) { throw new \yii\web\HttpException(404, Yii::t('candidate', 'The requested Item could not be found.')); } - + $candidate->candidate_civil_id = Yii::$app->request->getBodyParam('civil_id'); $candidate->candidate_civil_need_verification = true; @@ -1074,21 +1092,21 @@ public function actionUpdateCivilId() { "message" => Yii::t('candidate', "Candidate Civil ID Info Updated Successfully"), ]; } - - + + /** * update candidate intro * @return type * @throws \yii\web\HttpException */ public function actionUpdateIntro() { - + $candidate = Candidate::findOne(Yii::$app->user->getId()); if (!$candidate) { throw new \yii\web\HttpException(404, Yii::t('candidate', 'The requested Item could not be found.')); } - + $candidate->candidate_intro = Yii::$app->request->getBodyParam('intro'); $candidate->scenario = "updateIntro"; @@ -1113,13 +1131,13 @@ public function actionUpdateIntro() { * @throws \yii\web\HttpException */ public function actionUpdateObjective() { - + $candidate = Candidate::findOne(Yii::$app->user->getId()); if (!$candidate) { throw new \yii\web\HttpException(404, Yii::t('candidate', 'The requested Item could not be found.')); } - + $candidate->candidate_objective = Yii::$app->request->getBodyParam('objective'); $candidate->scenario = "updateObjective"; @@ -1137,20 +1155,20 @@ public function actionUpdateObjective() { "message" => Yii::t('candidate', "Candidate Objective Info Updated Successfully"), ]; } - + /** * update candidate gender * @return type * @throws \yii\web\HttpException */ public function actionUpdateGender() { - + $candidate = Candidate::findOne(Yii::$app->user->getId()); if (!$candidate) { throw new \yii\web\HttpException(404, Yii::t('candidate', 'The requested Item could not be found.')); } - + $candidate->candidate_gender = Yii::$app->request->getBodyParam('gender'); $candidate->scenario = "updateGender"; @@ -1168,20 +1186,20 @@ public function actionUpdateGender() { "message" => Yii::t('candidate', "Candidate Gender Info Updated Successfully"), ]; } - + /** * candidate university * @return type * @throws \yii\web\HttpException */ public function actionUpdateUniversity() { - + $candidate = Candidate::findOne(Yii::$app->user->getId()); if (!$candidate) { throw new \yii\web\HttpException(404, Yii::t('candidate', 'The requested Item could not be found.')); } - + $candidate->university_id = Yii::$app->request->getBodyParam('university_id'); $candidate->scenario = "updateUniversity"; @@ -1199,20 +1217,20 @@ public function actionUpdateUniversity() { "message" => Yii::t('candidate', "Candidate University Info Updated Successfully"), ]; } - + /** * update resume * @return type * @throws \yii\web\HttpException */ public function actionUpdateResume() { - + $model = Candidate::findOne(Yii::$app->user->getId()); if (!$model) { throw new \yii\web\HttpException(404, Yii::t('candidate', 'The requested Item could not be found.')); } - + if ($model->candidate_resume) { $model->deleteResume(); } @@ -1235,7 +1253,7 @@ public function actionUpdateResume() { ]) ]; } - + if (!$model->updateResume()) { return [ 'operation' => 'error', @@ -1281,7 +1299,7 @@ public function actionRemoveResume() { * @throws \yii\web\HttpException */ public function actionUpdateCivilPhotoBack() { - + $model = Candidate::findOne(Yii::$app->user->getId()); if (!$model) { @@ -1289,7 +1307,7 @@ public function actionUpdateCivilPhotoBack() { } $model->scenario = "updateCivilPhotoBack"; - + $model->candidate_civil_photo_back = urldecode(Yii::$app->request->getBodyParam('civil_photo_back')); if(!$model->candidate_civil_photo_back || $model->candidate_civil_photo_back == "undefined") { @@ -1300,7 +1318,7 @@ public function actionUpdateCivilPhotoBack() { ]) ]; } - + $model->updateCivilId('back'); //reset to remove old id's data @@ -1326,14 +1344,14 @@ public function actionUpdateCivilPhotoBack() { 'message' => Yii::t('candidate', 'Civil Photo Back Uploaded Successfully') ]; } - + /** * update civil photo front * @return type * @throws \yii\web\HttpException */ public function actionUpdateCivilPhotoFront() { - + $model = Candidate::findOne(Yii::$app->user->getId()); if (!$model) { @@ -1341,7 +1359,7 @@ public function actionUpdateCivilPhotoFront() { } $model->scenario = "updateCivilPhotoFront"; - + $model->candidate_civil_photo_front = urldecode(Yii::$app->request->getBodyParam('civil_photo_front')); if(!$model->candidate_civil_photo_front || $model->candidate_civil_photo_front == "undefined") { @@ -1352,7 +1370,7 @@ public function actionUpdateCivilPhotoFront() { ]) ]; } - + $model->updateCivilId('front'); //reset to remove old id's data @@ -1379,20 +1397,20 @@ public function actionUpdateCivilPhotoFront() { 'message' => Yii::t('candidate', 'Civil Photo Front Uploaded Successfully') ]; } - + /** * update civil id expiry date * @return type * @throws \yii\web\HttpException */ public function actionUpdateCivilExpiryDate() { - + $candidate = Candidate::findOne(Yii::$app->user->getId()); if (!$candidate) { throw new \yii\web\HttpException(404, Yii::t('candidate', 'The requested Item could not be found.')); } - + $candidate_civil_expiry_date = Yii::$app->request->getBodyParam('civil_expiry_date'); if($candidate_civil_expiry_date) @@ -1430,24 +1448,38 @@ public function actionUpdateCivilIdExpiryDate() { throw new \yii\web\HttpException(404, Yii::t('candidate', 'The requested Item could not be found.')); } - $candidate_civil_id = Yii::$app->request->getBodyParam('civil_id'); + $candidate_civil_id = trim((string) Yii::$app->request->getBodyParam('civil_id')); $candidate_civil_expiry_date = Yii::$app->request->getBodyParam('civil_expiry_date'); + if (!$candidate_civil_id || !$candidate_civil_expiry_date || strtotime($candidate_civil_expiry_date) === false) { + return [ + "operation" => "error", + "message" => Yii::t('app', 'Invalid civil ID or expiry date') + ]; + } + $candidate->candidate_civil_id = $candidate_civil_id; - if($candidate_civil_expiry_date) - $candidate->candidate_civil_expiry_date = date('Y-m-d', strtotime($candidate_civil_expiry_date)); + $candidate->candidate_civil_expiry_date = date('Y-m-d', strtotime($candidate_civil_expiry_date)); $candidate->candidate_civil_need_verification = true; $candidate->scenario = "updateCivilExpiryDateAndCivilID"; - if (!$candidate->save()) { + try { + if (!$candidate->save()) { + return [ + "operation" => "error", + "message" => $candidate->errors + ]; + } + } catch (\Exception $e) { + Yii::error($e->getMessage(), 'candidate'); return [ "operation" => "error", - "message" => $candidate->errors + "message" => Yii::t('candidate', "Civil ID And Expiry Date could not be updated") ]; } @@ -1457,22 +1489,22 @@ public function actionUpdateCivilIdExpiryDate() { "message" => Yii::t('candidate', "Civil ID And Expiry Date Updated Successfully"), ]; } - + /** - * update birth date + * update birth date * @return type * @throws \yii\web\HttpException */ public function actionUpdateBirthDate() { - + $candidate = Candidate::findOne(Yii::$app->user->getId()); if (!$candidate) { throw new \yii\web\HttpException(404, Yii::t('candidate', 'The requested Item could not be found.')); } - + $birth_date = Yii::$app->request->getBodyParam('birth_date'); - + $candidate->candidate_birth_date = empty($birth_date)? date('Y-m-d'): date('Y-m-d', strtotime($birth_date)); $candidate->scenario = "updateBirthDate"; diff --git a/common/components/S3ResourceManager.php b/common/components/S3ResourceManager.php index 608a407e..f4e1060c 100644 --- a/common/components/S3ResourceManager.php +++ b/common/components/S3ResourceManager.php @@ -162,7 +162,7 @@ public function delete($name) 'Key' => $name ]); - return $result['DeleteMarker']; + return isset($result['DeleteMarker']) ? $result['DeleteMarker'] : true; } /** @@ -178,6 +178,18 @@ public function fileExists($filenameOrUrl) $isUrl = true; } + if (!$isUrl) { + try { + $this->getClient()->headObject([ + 'Bucket' => $this->bucket, + 'Key' => $filenameOrUrl, + ]); + return true; + } catch (AwsException $e) { + return false; + } + } + $http = new \GuzzleHttp\Client(['base_uri' => $isUrl ? $filenameOrUrl : $this->getUrl($filenameOrUrl)]); try { $response = $http->request('HEAD'); diff --git a/common/config/main.php b/common/config/main.php index 793bdb35..598b4272 100644 --- a/common/config/main.php +++ b/common/config/main.php @@ -7,10 +7,10 @@ 'components' => [ 'temporaryBucketResourceManager' => [ 'class' => 'common\components\S3ResourceManager', - 'region' => 'eu-west-2', // Bucket based in London - 'key' => 'AKIAWMITDJRKVN5ODY2X', - 'secret' => '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', - 'bucket' => 'studenthub-public-anyone-can-upload-24hr-expiry' + 'region' => getenv('AWS_TEMP_BUCKET_REGION') ?: 'eu-west-2', + 'key' => getenv('AWS_TEMP_BUCKET_KEY') ?: '', + 'secret' => getenv('AWS_TEMP_BUCKET_SECRET') ?: '', + 'bucket' => getenv('AWS_TEMP_BUCKET_NAME') ?: 'studenthub-public-anyone-can-upload-24hr-expiry' /** * You can access the Temporary bucket with: * https://studenthub-public-anyone-can-upload-24hr-expiry.s3.amazonaws.com/ diff --git a/common/models/Candidate.php b/common/models/Candidate.php index b1c5694b..ed1a2360 100644 --- a/common/models/Candidate.php +++ b/common/models/Candidate.php @@ -118,8 +118,8 @@ class Candidate extends \yii\db\ActiveRecord implements \yii\web\IdentityInterfa // Array of attribute names and folder names to store them in the permanent bucket public $FILE_ATTRIBUTES = [ 'candidate_personal_photo' => 'photos', - 'candidate_civil_photo_front' => 'civil-id', - 'candidate_civil_photo_back' => 'civil-id' + 'candidate_civil_photo_front' => 'photos', + 'candidate_civil_photo_back' => 'photos' ]; /** @@ -2705,13 +2705,17 @@ public function deleteFile($type = 'resume', $side = 'front') { if ($type == 'resume' && isset($this->oldAttributes['candidate_resume'])) { $file = "candidate-resume/" . $this->oldAttributes['candidate_resume']; } else if ($type == 'civil-id' && $side == 'front' && isset($this->oldAttributes['candidate_civil_photo_front'])) { - $file = "candidate-civil-id/" . $this->oldAttributes['candidate_civil_photo_front']; + $file = "photos/" . $this->oldAttributes['candidate_civil_photo_front']; } else if ($type == 'civil-id' && $side == 'back' && isset($this->oldAttributes['candidate_civil_photo_back'])) { - $file = "candidate-civil-id/" . $this->oldAttributes['candidate_civil_photo_back']; + $file = "photos/" . $this->oldAttributes['candidate_civil_photo_back']; } if ($file) { - Yii::$app->resourceManager->delete($file); + if (Yii::$app->resourceManager->fileExists($file)) { + Yii::$app->resourceManager->delete($file); + } else { + Yii::warning("Civil ID file already missing during delete: " . $file, 'candidate'); + } } } @@ -2719,7 +2723,9 @@ public function deleteFile($type = 'resume', $side = 'front') { Yii::error($e->getMessage(), 'candidate'); - $this->addError('candidate_resume', Yii::t('app', 'file not available to delete.')); + if ($type != 'civil-id') { + $this->addError('candidate_resume', Yii::t('app', 'file not available to delete.')); + } return false; @@ -2727,7 +2733,9 @@ public function deleteFile($type = 'resume', $side = 'front') { Yii::error($e->getMessage(), 'candidate'); - $this->addError('candidate_resume', Yii::t('app', 'file not available to delete.')); + if ($type != 'civil-id') { + $this->addError('candidate_resume', Yii::t('app', 'file not available to delete.')); + } return false; } @@ -2740,10 +2748,6 @@ public function updateCivilId($side = 'front') { $idSide = ($side == 'front') ? 'candidate_civil_photo_front' : 'candidate_civil_photo_back'; - if (!empty($this->oldAttributes[$idSide])) { - $this->deleteFile('civil-id', $side); - } - $fileName = $this->$idSide; $sourceBucket = Yii::$app->temporaryBucketResourceManager->bucket; @@ -2754,7 +2758,19 @@ public function updateCivilId($side = 'front') { try { - return Yii::$app->resourceManager->copy($fileName, $targetPath, $sourceBucket); + $result = Yii::$app->resourceManager->copy($fileName, $targetPath, $sourceBucket); + + if (!Yii::$app->resourceManager->fileExists($targetPath)) { + Yii::error("Civil ID copy verification failed for {$idSide}: {$targetPath}", 'candidate'); + $this->addError($idSide, Yii::t('app', 'file not available to save.')); + return false; + } + + if (!empty($this->oldAttributes[$idSide])) { + $this->deleteFile('civil-id', $side); + } + + return $result; } catch (\Aws\S3\Exception\S3Exception $e) { diff --git a/environments/prod-railway/common/config/main-local.php b/environments/prod-railway/common/config/main-local.php index f8b14d07..3e5e0bca 100644 --- a/environments/prod-railway/common/config/main-local.php +++ b/environments/prod-railway/common/config/main-local.php @@ -152,10 +152,10 @@ 'resourceManager' => [ 'class' => 'common\components\S3ResourceManager', 'authMethod' => \common\components\S3ResourceManager::AUTH_VIA_KEY_AND_SECRET, - 'region' => 'eu-west-2', // Bucket based in London - 'bucket' => 'studenthub-uploads', - 'key' => 'AKIAWMITDJRKWZZEWCUM',//railway-s3-access - 'secret' => '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', + 'region' => getenv('AWS_PERMANENT_S3_REGION') ?: 'eu-west-2', + 'bucket' => getenv('AWS_PERMANENT_S3_BUCKET') ?: 'studenthub-uploads', + 'key' => getenv('AWS_PERMANENT_S3_ACCESS_KEY_ID') ?: '', + 'secret' => getenv('AWS_PERMANENT_S3_SECRET_ACCESS_KEY') ?: '', /** * For Local Development, we access using key and secret * For Dev and Production servers, access is via server embedded IAM roles so no key/secret required diff --git a/tests/check-s3-civil-id-hardening.sh b/tests/check-s3-civil-id-hardening.sh new file mode 100644 index 00000000..6929778e --- /dev/null +++ b/tests/check-s3-civil-id-hardening.sh @@ -0,0 +1,34 @@ +#!/bin/sh +set -eu + +candidate_model="common/models/Candidate.php" +account_controller="candidate/modules/v1/controllers/AccountController.php" +s3_manager="common/components/S3ResourceManager.php" +common_config="common/config/main.php" +railway_config="environments/prod-railway/common/config/main-local.php" + +grep -q "'candidate_civil_photo_front' => 'photos'" "$candidate_model" +grep -q "'candidate_civil_photo_back' => 'photos'" "$candidate_model" +grep -q '"photos/" . $this->oldAttributes' "$candidate_model" +grep -q 'Civil ID copy verification failed' "$candidate_model" +grep -q 'fileExists($targetPath)' "$candidate_model" + +grep -q 'headObject' "$s3_manager" +grep -q "return isset(\$result\\['DeleteMarker'\\]) ? \$result\\['DeleteMarker'\\] : true;" "$s3_manager" + +grep -q 'candidate_civil_need_verification = true' "$account_controller" +grep -q 'Invalid civil ID or expiry date' "$account_controller" +grep -q 'try {' "$account_controller" + +grep -q 'AWS_TEMP_BUCKET_KEY' "$common_config" +grep -q 'AWS_TEMP_BUCKET_SECRET' "$common_config" +grep -q 'AWS_PERMANENT_S3_ACCESS_KEY_ID' "$railway_config" +grep -q 'AWS_PERMANENT_S3_SECRET_ACCESS_KEY' "$railway_config" + +if grep -R "AKIAWMITDJRKVN5ODY2X\\|AKIAWMITDJRKWZZEWCUM" \ + "$common_config" "$railway_config"; then + echo "hardcoded S3 key suffix remains in patched configs" >&2 + exit 1 +fi + +echo "PASS S3 Civil ID hardening checks" From 06858e22158c4716459a6b2d0ee14a1257a431ab Mon Sep 17 00:00:00 2001 From: SURESH CHOUKSEY Date: Fri, 15 May 2026 10:59:31 +0530 Subject: [PATCH 2/2] fix: address Civil ID S3 review feedback --- .../v1/controllers/AccountController.php | 26 ++++++++++++++++--- common/config/main.php | 4 +-- .../prod-railway/common/config/main-local.php | 4 +-- tests/check-s3-civil-id-hardening.sh | 13 ++++++++-- 4 files changed, 37 insertions(+), 10 deletions(-) diff --git a/candidate/modules/v1/controllers/AccountController.php b/candidate/modules/v1/controllers/AccountController.php index 5b4fcdf2..287ef615 100644 --- a/candidate/modules/v1/controllers/AccountController.php +++ b/candidate/modules/v1/controllers/AccountController.php @@ -383,6 +383,10 @@ public function actionRemoveCivilPhotoBack() { $model = Candidate::findOne(Yii::$app->user->getId()); + if (!$model) { + throw new \yii\web\NotFoundHttpException(Yii::t('candidate', 'The requested Item could not be found.')); + } + try { if ($model->candidate_civil_photo_back) { $model->deleteFile('civil-id', 'back'); @@ -399,7 +403,7 @@ public function actionRemoveCivilPhotoBack() { 'message' => $model->getErrors() ]; } - } catch (\Exception $e) { + } catch (\Throwable $e) { Yii::error($e->getMessage(), 'candidate'); return [ 'operation' => 'error', @@ -418,6 +422,10 @@ public function actionRemoveCivilPhotoBack() { public function actionRemoveCivilPhotoFront() { $model = Candidate::findOne(Yii::$app->user->getId()); + if (!$model) { + throw new \yii\web\NotFoundHttpException(Yii::t('candidate', 'The requested Item could not be found.')); + } + try { if ($model->candidate_civil_photo_front) { $model->deleteFile('civil-id', 'front'); @@ -434,7 +442,7 @@ public function actionRemoveCivilPhotoFront() { 'message' => $model->getErrors() ]; } - } catch (\Exception $e) { + } catch (\Throwable $e) { Yii::error($e->getMessage(), 'candidate'); return [ 'operation' => 'error', @@ -1319,7 +1327,12 @@ public function actionUpdateCivilPhotoBack() { ]; } - $model->updateCivilId('back'); + if (!$model->updateCivilId('back')) { + return [ + 'operation' => 'error', + 'message' => $model->getErrors() + ]; + } //reset to remove old id's data $model->candidate_civil_expiry_date = null; @@ -1371,7 +1384,12 @@ public function actionUpdateCivilPhotoFront() { ]; } - $model->updateCivilId('front'); + if (!$model->updateCivilId('front')) { + return [ + 'operation' => 'error', + 'message' => $model->getErrors() + ]; + } //reset to remove old id's data $model->candidate_civil_expiry_date = null; diff --git a/common/config/main.php b/common/config/main.php index 598b4272..4d6c96be 100644 --- a/common/config/main.php +++ b/common/config/main.php @@ -8,8 +8,8 @@ 'temporaryBucketResourceManager' => [ 'class' => 'common\components\S3ResourceManager', 'region' => getenv('AWS_TEMP_BUCKET_REGION') ?: 'eu-west-2', - 'key' => getenv('AWS_TEMP_BUCKET_KEY') ?: '', - 'secret' => getenv('AWS_TEMP_BUCKET_SECRET') ?: '', + 'key' => getenv('AWS_TEMP_BUCKET_KEY') ?: null, + 'secret' => getenv('AWS_TEMP_BUCKET_SECRET') ?: null, 'bucket' => getenv('AWS_TEMP_BUCKET_NAME') ?: 'studenthub-public-anyone-can-upload-24hr-expiry' /** * You can access the Temporary bucket with: diff --git a/environments/prod-railway/common/config/main-local.php b/environments/prod-railway/common/config/main-local.php index 3e5e0bca..2378adcc 100644 --- a/environments/prod-railway/common/config/main-local.php +++ b/environments/prod-railway/common/config/main-local.php @@ -154,8 +154,8 @@ 'authMethod' => \common\components\S3ResourceManager::AUTH_VIA_KEY_AND_SECRET, 'region' => getenv('AWS_PERMANENT_S3_REGION') ?: 'eu-west-2', 'bucket' => getenv('AWS_PERMANENT_S3_BUCKET') ?: 'studenthub-uploads', - 'key' => getenv('AWS_PERMANENT_S3_ACCESS_KEY_ID') ?: '', - 'secret' => getenv('AWS_PERMANENT_S3_SECRET_ACCESS_KEY') ?: '', + 'key' => getenv('AWS_PERMANENT_S3_ACCESS_KEY_ID') ?: null, + 'secret' => getenv('AWS_PERMANENT_S3_SECRET_ACCESS_KEY') ?: null, /** * For Local Development, we access using key and secret * For Dev and Production servers, access is via server embedded IAM roles so no key/secret required diff --git a/tests/check-s3-civil-id-hardening.sh b/tests/check-s3-civil-id-hardening.sh index 6929778e..527abc24 100644 --- a/tests/check-s3-civil-id-hardening.sh +++ b/tests/check-s3-civil-id-hardening.sh @@ -14,16 +14,25 @@ grep -q 'Civil ID copy verification failed' "$candidate_model" grep -q 'fileExists($targetPath)' "$candidate_model" grep -q 'headObject' "$s3_manager" -grep -q "return isset(\$result\\['DeleteMarker'\\]) ? \$result\\['DeleteMarker'\\] : true;" "$s3_manager" +grep -q 'DeleteMarker' "$s3_manager" grep -q 'candidate_civil_need_verification = true' "$account_controller" grep -q 'Invalid civil ID or expiry date' "$account_controller" -grep -q 'try {' "$account_controller" +grep -q 'actionRemoveCivilPhotoBack' "$account_controller" +grep -q 'actionRemoveCivilPhotoFront' "$account_controller" +grep -q 'Unable to remove civil photo back.' "$account_controller" +grep -q 'Unable to remove civil photo front.' "$account_controller" +grep -q "if (!\$model->updateCivilId('back'))" "$account_controller" +grep -q "if (!\$model->updateCivilId('front'))" "$account_controller" grep -q 'AWS_TEMP_BUCKET_KEY' "$common_config" grep -q 'AWS_TEMP_BUCKET_SECRET' "$common_config" +grep -q "'key' => getenv('AWS_TEMP_BUCKET_KEY') ?: null" "$common_config" +grep -q "'secret' => getenv('AWS_TEMP_BUCKET_SECRET') ?: null" "$common_config" grep -q 'AWS_PERMANENT_S3_ACCESS_KEY_ID' "$railway_config" grep -q 'AWS_PERMANENT_S3_SECRET_ACCESS_KEY' "$railway_config" +grep -q "'key' => getenv('AWS_PERMANENT_S3_ACCESS_KEY_ID') ?: null" "$railway_config" +grep -q "'secret' => getenv('AWS_PERMANENT_S3_SECRET_ACCESS_KEY') ?: null" "$railway_config" if grep -R "AKIAWMITDJRKVN5ODY2X\\|AKIAWMITDJRKWZZEWCUM" \ "$common_config" "$railway_config"; then