Skip to content

Move items to s3 #1504

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
tarunnjoshi wants to merge 3 commits into
base: dev
Choose a base branch
from
Open

Move items to s3 #1504

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
51cf65c
Move items to s3
tarunnjoshi Dec 2, 2025
233f8f6
Merge branch 'dev' of https://github.com/ColoredCow/goonj into move-i…
tarunnjoshi Dec 2, 2025
84b6e9f
push images to s3
tarunnjoshi Dec 2, 2025
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
113 changes: 113 additions & 0 deletions wp-content/civi-extensions/goonjcustom/Civi/NavigationPermissionService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,123 @@ public static function getSubscribedEvents() {
['hideButtonsForMMT'],
['hideAPIKeyTab'],
['hideContributionFields'],
['testing']
],
];
}

/**
* Override file and contact-photo output to use S3.
*/
public function testing(&$page) {
error_log('Page class: ' . get_class($page));

/*
* ----------------------------------------------------
* 1️⃣ SEARCHKIT + FILE FIELDS (File Handler Override)
* ----------------------------------------------------
*/
if (get_class($page) === 'CRM_Core_Page_File') {
error_log("File handler override triggered");

$fileId = \CRM_Utils_Request::retrieve('id', 'Integer');
error_log("Retrieved fileId from URL: " . print_r($fileId, true));
if (!$fileId) return;

try {
$file = civicrm_api3('File', 'getsingle', ['id' => $fileId]);
} catch (\Exception $e) {
error_log("Error fetching civicrm_file: " . $e->getMessage());
return;
}

$filename = $file['uri'];
$mime = $file['mime_type'];
$s3Url = "https://goonj-uploads-items.s3.ap-south-1.amazonaws.com/custom/" . $filename;
error_log("S3 URL built: " . $s3Url);

$image = @file_get_contents($s3Url);
if ($image === FALSE) {
error_log("S3 file not found at URL: " . $s3Url);
return;
}

header("Content-Type: {$mime}");
header("Content-Length: " . strlen($image));
echo $image;
\CRM_Utils_System::civiExit();
}
Comment on lines +141 to +178

@coderabbitai coderabbitai Bot Dec 2, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

S3 file download override lacks explicit permission checks and uses brittle patterns

In the CRM_Core_Page_File branch you pull a file by ID and stream it from S3 directly:

  • There is no explicit ACL/permission check before serving the file. Anyone who can hit CRM_Core_Page_File with a valid id will get the file contents from S3, regardless of entity‑level access rules. For attachments or sensitive files this is a security risk.
  • Using get_class($page) === 'CRM_Core_Page_File' is brittle if Civi ever subclasses this page; instanceof or $page->getVar('_name') (as used elsewhere in this class) is more robust.
  • Suppressing errors with @file_get_contents hides useful diagnostics and can make production issues harder to debug.

Suggestions:

  • Before streaming from S3, enforce the same permission model Civi uses for that file (e.g. via the related entity/ACLs or by delegating to existing helpers) so that this override does not weaken access control.
  • Prefer instanceof CRM_Core_Page_File or $page->getVar('_name') for page detection.
  • Drop the error suppression and rely on your explicit FALSE check and logging.
🤖 Prompt for AI Agents 
In wp-content/civi-extensions/goonjcustom/Civi/NavigationPermissionService.php
around lines 30–67, replace the brittle page check and silent file fetch with a
secure, robust flow: use "if ($page instanceof CRM_Core_Page_File)" (or
$page->getVar('_name')) instead of get_class(...), remove the "@" error
suppression on file_get_contents and log any failure, and before streaming the
S3 object enforce Civi's access model by resolving the File's related entity
(via the File API/BAO) and verifying the current user has permission to view
that entity/file (use existing Civi permission helpers / ACL checks or delegate
to the standard file download helper); only stream the S3 contents after the
permission check passes and return/log and exit on denial or fetch errors.


if (get_class($page) === 'CRM_Contact_Page_View_Summary') {
error_log("Contact Summary page detected");

$contactId = $page->getVar('_contactId');
error_log("Contact ID: " . print_r($contactId, true));
if (!$contactId) return;

// Get custom QR code file ID
try {
$contacts = \Civi\Api4\Contact::get(TRUE)
->addSelect('Contact_QR_Code.QR_Code')
->addWhere('id', '=', $contactId)
->execute()
->first();
} catch (\Exception $e) {
error_log("Error fetching contact QR code: " . $e->getMessage());
return;
}

$fileId = $contacts['Contact_QR_Code.QR_Code'];
if (!$fileId) return;

// Load civicrm_file record
try {
$file = civicrm_api3('File', 'getsingle', ['id' => $fileId]);
} catch (\Exception $e) {
error_log("Error fetching civicrm_file: " . $e->getMessage());
return;
}

$s3Url = "https://goonj-uploads-items.s3.ap-south-1.amazonaws.com/custom/" . $file['uri'];
error_log("S3 URL for contact QR code: " . $s3Url);

// Assign S3 URL to custom field token and fileUrl
$customFields = \Civi\Api4\CustomField::get(TRUE)
->addSelect('id')
->addWhere('custom_group_id:name', '=', 'Contact_QR_Code')
->addWhere('name', '=', 'QR_Code')
->setLimit(1)
->execute()
->first();

$tokenName = 'custom_' . $customFields['id'];
$page->assign($tokenName, $s3Url);
$page->assign("fileUrl_{$fileId}", $s3Url);
error_log("Custom field image assigned successfully to token and fileUrl");

// ----------------------------
// Inject JS safely via CRM_Core_Resources
// ----------------------------
$js = <<<JS
document.addEventListener('DOMContentLoaded', function() {
var qrDiv = document.querySelector('#custom-set-content-3 .crm-content.crm-custom-data');
if(qrDiv && !qrDiv.querySelector('img')) {
qrDiv.innerHTML = '<img src="{$s3Url}" alt="QR Code" style="max-width:150px;height:auto;">';
console.log('QR code injected for contact {$contactId}');
}
});
JS;

\CRM_Core_Resources::singleton()->addScript($js, 0, 'inline');
error_log("JS injected via CRM_Core_Resources for inline rendering");
}
Comment on lines +180 to +242

@coderabbitai coderabbitai Bot Dec 2, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Contact summary S3 QR rendering needs stronger error handling and is DOM‑fragile

In the CRM_Contact_Page_View_Summary branch:

  • $contacts = ...->first(); is assumed to return an array with Contact_QR_Code.QR_Code. If the custom group/field is disabled, renamed, or the API returns no rows, $contacts may be NULL or missing that key, leading to notices or errors.
  • \Civi\Api4\CustomField::get(TRUE)...->first(); is not wrapped in a try/catch, so any API failure will bubble up as a fatal and break the contact summary page.
  • The JS uses a hard‑coded selector '#custom-set-content-3 .crm-content.crm-custom-data' and replaces innerHTML. This is brittle to layout/order changes and will wipe any other content in that container, which can surprise future maintainers.

Consider:

-      $contacts = \Civi\Api4\Contact::get(TRUE)
+      $contacts = \Civi\Api4\Contact::get(TRUE)
           ->addSelect('Contact_QR_Code.QR_Code')
           ->addWhere('id', '=', $contactId)
           ->execute()
           ->first();
-      $fileId = $contacts['Contact_QR_Code.QR_Code'];
-      if (!$fileId) return;
+      if (empty($contacts) || empty($contacts['Contact_QR_Code.QR_Code'])) {
+          return;
+      }
+      $fileId = $contacts['Contact_QR_Code.QR_Code'];

and wrapping the CustomField::get chain in a try/catch similar to the other API calls.

For the JS, appending an <img> element instead of replacing innerHTML, and targeting a more stable hook (e.g. a dedicated wrapper id/class for the QR field) would make this less fragile.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (get_class($page) === 'CRM_Contact_Page_View_Summary') {
error_log("Contact Summary page detected");
$contactId = $page->getVar('_contactId');
error_log("Contact ID: " . print_r($contactId, true));
if (!$contactId) return;
// Get custom QR code file ID
try {
$contacts = \Civi\Api4\Contact::get(TRUE)
->addSelect('Contact_QR_Code.QR_Code')
->addWhere('id', '=', $contactId)
->execute()
->first();
} catch (\Exception $e) {
error_log("Error fetching contact QR code: " . $e->getMessage());
return;
}
$fileId = $contacts['Contact_QR_Code.QR_Code'];
if (!$fileId) return;
// Load civicrm_file record
try {
$file = civicrm_api3('File', 'getsingle', ['id' => $fileId]);
} catch (\Exception $e) {
error_log("Error fetching civicrm_file: " . $e->getMessage());
return;
}
$s3Url = "https://goonj-uploads-items.s3.ap-south-1.amazonaws.com/custom/" . $file['uri'];
error_log("S3 URL for contact QR code: " . $s3Url);
// Assign S3 URL to custom field token and fileUrl
$customFields = \Civi\Api4\CustomField::get(TRUE)
->addSelect('id')
->addWhere('custom_group_id:name', '=', 'Contact_QR_Code')
->addWhere('name', '=', 'QR_Code')
->setLimit(1)
->execute()
->first();
$tokenName = 'custom_' . $customFields['id'];
$page->assign($tokenName, $s3Url);
$page->assign("fileUrl_{$fileId}", $s3Url);
error_log("Custom field image assigned successfully to token and fileUrl");
// ----------------------------
// Inject JS safely via CRM_Core_Resources
// ----------------------------
$js = <<<JS
document.addEventListener('DOMContentLoaded', function() {
var qrDiv = document.querySelector('#custom-set-content-3 .crm-content.crm-custom-data');
if(qrDiv && !qrDiv.querySelector('img')) {
qrDiv.innerHTML = '<img src="{$s3Url}" alt="QR Code" style="max-width:150px;height:auto;">';
console.log('QR code injected for contact {$contactId}');
}
});
JS;
\CRM_Core_Resources::singleton()->addScript($js, 0, 'inline');
error_log("JS injected via CRM_Core_Resources for inline rendering");
}
if (get_class($page) === 'CRM_Contact_Page_View_Summary') {
error_log("Contact Summary page detected");
$contactId = $page->getVar('_contactId');
error_log("Contact ID: " . print_r($contactId, true));
if (!$contactId) return;
// Get custom QR code file ID
try {
$contacts = \Civi\Api4\Contact::get(TRUE)
->addSelect('Contact_QR_Code.QR_Code')
->addWhere('id', '=', $contactId)
->execute()
->first();
} catch (\Exception $e) {
error_log("Error fetching contact QR code: " . $e->getMessage());
return;
}
if (empty($contacts) || empty($contacts['Contact_QR_Code.QR_Code'])) {
return;
}
$fileId = $contacts['Contact_QR_Code.QR_Code'];
// Load civicrm_file record
try {
$file = civicrm_api3('File', 'getsingle', ['id' => $fileId]);
} catch (\Exception $e) {
error_log("Error fetching civicrm_file: " . $e->getMessage());
return;
}
$s3Url = "https://goonj-uploads-items.s3.ap-south-1.amazonaws.com/custom/" . $file['uri'];
error_log("S3 URL for contact QR code: " . $s3Url);
// Assign S3 URL to custom field token and fileUrl
$customFields = \Civi\Api4\CustomField::get(TRUE)
->addSelect('id')
->addWhere('custom_group_id:name', '=', 'Contact_QR_Code')
->addWhere('name', '=', 'QR_Code')
->setLimit(1)
->execute()
->first();
$tokenName = 'custom_' . $customFields['id'];
$page->assign($tokenName, $s3Url);
$page->assign("fileUrl_{$fileId}", $s3Url);
error_log("Custom field image assigned successfully to token and fileUrl");
// ----------------------------
// Inject JS safely via CRM_Core_Resources
// ----------------------------
$js = <<<JS
document.addEventListener('DOMContentLoaded', function() {
var qrDiv = document.querySelector('#custom-set-content-3 .crm-content.crm-custom-data');
if(qrDiv && !qrDiv.querySelector('img')) {
qrDiv.innerHTML = '<img src="{$s3Url}" alt="QR Code" style="max-width:150px;height:auto;">';
console.log('QR code injected for contact {$contactId}');
}
});
JS;
\CRM_Core_Resources::singleton()->addScript($js, 0, 'inline');
error_log("JS injected via CRM_Core_Resources for inline rendering");
}




}



/**
*
*/
Expand Down