Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
<?php

use Civi\Core\Event\GenericHookEvent;
use Civi\Core\Service\AutoService;

/**
* Provides ability to prefill afform fields from encrypted token.
*
* @service goonjcustom.afform_prefill
*/
class CRM_Goonjcustom_AfformFieldPrefillService extends AutoService {

public static function preprocess(\Civi\Angular\Manager $angular) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Avoid static methods for better testability

Static methods make unit testing difficult and violate dependency injection principles. Consider making this an instance method instead.

- public static function preprocess(\Civi\Angular\Manager $angular) {
+ public function preprocess(\Civi\Angular\Manager $angular) {
📝 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
public static function preprocess(\Civi\Angular\Manager $angular) {
public function preprocess(\Civi\Angular\Manager $angular) {
🤖 Prompt for AI Agents
In
wp-content/civi-extensions/goonjcustom/CRM/Goonjcustom/AfformFieldPrefillService.php
at line 13, the preprocess method is declared static, which hinders unit testing
and dependency injection. Change the method from static to an instance method by
removing the static keyword and update all calls to this method accordingly to
use an instance of the class instead of calling it statically.

Civi::log()->debug('[AfformPrefill] Preprocess started.');

$request = \CRM_Utils_Request::retrieve('token', 'String', CRM_Core_DAO::$_nullObject, FALSE, NULL, 'GET');

if (!$request) {
Civi::log()->warning('[AfformPrefill] No token found in URL.');
return;
}

try {
$decrypted = CRM_Utils_Crypt::decrypt($request);
$data = json_decode($decrypted, true);
Civi::log()->debug('[AfformPrefill] Token decrypted: ' . print_r($data, true));
} catch (\Exception $e) {
Civi::log()->error('[AfformPrefill] Token decryption failed: ' . $e->getMessage());
return;
}

if (!is_array($data)) {
Civi::log()->error('[AfformPrefill] Decrypted token is not valid JSON.');
return;
}

$changeSet = \Civi\Angular\ChangeSet::create('injectPrefillValues')
->alterHtml(';\\.aff\\.html$;', function ($doc, $path) use ($data) {
Civi::log()->debug("[AfformPrefill] Altering HTML template: {$path}");

foreach (pq('af-field', $doc) as $afField) {
$fieldName = $afField->getAttribute('name');

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

Sanitize field names to prevent potential attacks

Field names are used directly without validation, which could lead to security issues if malicious field names are processed.

$fieldName = $afField->getAttribute('name');

if (!$fieldName) {
  continue;
}

+// Validate field name format
+if (!preg_match('/^[a-zA-Z_][a-zA-Z0-9_\.]*$/', $fieldName)) {
+  Civi::log()->warning("[AfformPrefill] Invalid field name format: {$fieldName}");
+  continue;
+}

if (array_key_exists($fieldName, $data)) {

Also applies to: 48-48

🤖 Prompt for AI Agents
In
wp-content/civi-extensions/goonjcustom/CRM/Goonjcustom/AfformFieldPrefillService.php
at lines 42 and 48, the field names retrieved from getAttribute('name') are used
without sanitization, posing a security risk. To fix this, sanitize the field
names by validating and cleaning the input to allow only expected characters
(e.g., alphanumeric and underscores) before using them. This prevents potential
injection or other attacks from malicious field names.


if (!$fieldName) {
continue;
}

if (array_key_exists($fieldName, $data)) {
$fieldDefn = self::getFieldDefinition($afField);

if (!$fieldDefn) {
Civi::log()->warning("[AfformPrefill] No field definition for: {$fieldName}");
continue;
}

$fieldDefn['afform_default'] = $data[$fieldName];
$fieldDefn['readonly'] = true;

pq($afField)->attr('defn', htmlspecialchars(\CRM_Utils_JS::writeObject($fieldDefn), ENT_COMPAT));

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

Potential XSS vulnerability in field definition handling

The field definition is serialized and inserted into HTML attributes without proper validation of the input data values, which could lead to XSS attacks.

+// Sanitize the field value before setting
+$sanitizedValue = is_string($data[$fieldName]) ? 
+  htmlspecialchars($data[$fieldName], ENT_QUOTES, 'UTF-8') : 
+  $data[$fieldName];
+
+$fieldDefn['afform_default'] = $sanitizedValue;
-$fieldDefn['afform_default'] = $data[$fieldName];
$fieldDefn['readonly'] = true;

pq($afField)->attr('defn', htmlspecialchars(\CRM_Utils_JS::writeObject($fieldDefn), ENT_COMPAT));
📝 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
pq($afField)->attr('defn', htmlspecialchars(\CRM_Utils_JS::writeObject($fieldDefn), ENT_COMPAT));
// Sanitize the field value before setting
$sanitizedValue = is_string($data[$fieldName])
? htmlspecialchars($data[$fieldName], ENT_QUOTES, 'UTF-8')
: $data[$fieldName];
$fieldDefn['afform_default'] = $sanitizedValue;
$fieldDefn['readonly'] = true;
pq($afField)
->attr('defn', htmlspecialchars(\CRM_Utils_JS::writeObject($fieldDefn), ENT_COMPAT));
🤖 Prompt for AI Agents
In
wp-content/civi-extensions/goonjcustom/CRM/Goonjcustom/AfformFieldPrefillService.php
at line 59, the field definition is serialized and inserted into an HTML
attribute without sufficient validation, risking XSS vulnerabilities. To fix
this, ensure that all data within $fieldDefn is properly sanitized or escaped
before serialization, and consider using a safer encoding method that prevents
injection of malicious scripts when outputting into HTML attributes.

Civi::log()->info("[AfformPrefill] Prefilled field '{$fieldName}' with value: " . json_encode($data[$fieldName]));
} else {
Civi::log()->debug("[AfformPrefill] Field '{$fieldName}' not in token data, skipping.");
}
}
});

$angular->add($changeSet);
Civi::log()->debug('[AfformPrefill] ChangeSet injected successfully.');
}
Comment on lines +37 to +69

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Break down the monolithic preprocess method

This method violates the Single Responsibility Principle by handling token processing, template modification, and field updates all in one place. Consider extracting smaller, focused methods.

public function preprocess(\Civi\Angular\Manager $angular) {
-  // All the current logic here
+  $data = $this->processToken();
+  if (!$data) {
+    return;
+  }
+  
+  $changeSet = $this->createFieldPrefillChangeSet($data);
+  $angular->add($changeSet);
}

+private function processToken() {
+  // Token retrieval and decryption logic
+}

+private function createFieldPrefillChangeSet($data) {
+  // ChangeSet creation and field processing logic
+}

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In
wp-content/civi-extensions/goonjcustom/CRM/Goonjcustom/AfformFieldPrefillService.php
between lines 37 and 69, the current method is handling multiple
responsibilities including token processing, HTML template modification, and
field updates, which violates the Single Responsibility Principle. Refactor by
extracting distinct smaller methods for each responsibility: one for processing
tokens and preparing data, another for modifying the HTML template, and a third
for updating individual fields. Then call these methods sequentially to keep the
main method clean and focused.


public static function getFieldDefinition($afField) {
$existingFieldDefn = trim(pq($afField)->attr('defn') ?: '');

if ($existingFieldDefn && $existingFieldDefn[0] !== '{') {
return NULL;
}

return $existingFieldDefn ? \CRM_Utils_JS::getRawProps($existingFieldDefn) : [];
}
Comment on lines +71 to +79

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling for field definition parsing

The CRM_Utils_JS::getRawProps() call could throw exceptions that aren't handled, potentially breaking the entire prefill process.

-  return $existingFieldDefn ? \CRM_Utils_JS::getRawProps($existingFieldDefn) : [];
+  if (!$existingFieldDefn) {
+    return [];
+  }
+  
+  try {
+    return \CRM_Utils_JS::getRawProps($existingFieldDefn);
+  } catch (\Exception $e) {
+    Civi::log()->error('[AfformPrefill] Failed to parse field definition: ' . $e->getMessage());
+    return [];
+  }
📝 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
public static function getFieldDefinition($afField) {
$existingFieldDefn = trim(pq($afField)->attr('defn') ?: '');
if ($existingFieldDefn && $existingFieldDefn[0] !== '{') {
return NULL;
}
return $existingFieldDefn ? \CRM_Utils_JS::getRawProps($existingFieldDefn) : [];
}
public static function getFieldDefinition($afField) {
$existingFieldDefn = trim(pq($afField)->attr('defn') ?: '');
if ($existingFieldDefn && $existingFieldDefn[0] !== '{') {
return NULL;
}
if (!$existingFieldDefn) {
return [];
}
try {
return \CRM_Utils_JS::getRawProps($existingFieldDefn);
} catch (\Exception $e) {
Civi::log()->error('[AfformPrefill] Failed to parse field definition: ' . $e->getMessage());
return [];
}
}
🤖 Prompt for AI Agents
In
wp-content/civi-extensions/goonjcustom/CRM/Goonjcustom/AfformFieldPrefillService.php
around lines 71 to 79, the call to CRM_Utils_JS::getRawProps() may throw
exceptions that are currently unhandled. Wrap this call in a try-catch block to
catch any exceptions, and handle them gracefully by returning an empty array or
null to prevent the prefill process from breaking.

}
Original file line number Diff line number Diff line change
Expand Up @@ -543,32 +543,51 @@ public static function processDispatchEmail(string $op, string $objectName, $obj
*
*/
public static function sendDispatchEmail($email, $initiatorName, $droppingCenterId, $contactId, $goonjOffice, $goonjOfficeName) {
$homeUrl = \CRM_Utils_System::baseCMSURL();
$vehicleDispatchFormUrl = $homeUrl . '/vehicle-dispatch/#?Camp_Vehicle_Dispatch.Dropping_Center=' . $droppingCenterId . '&Camp_Vehicle_Dispatch.Filled_by=' . $contactId . '&Camp_Vehicle_Dispatch.To_which_PU_Center_material_is_being_sent=' . $goonjOffice . '&Camp_Vehicle_Dispatch.Goonj_Office_Name=' . $goonjOfficeName . '&Eck_Collection_Camp1=' . $droppingCenterId;
$homeUrl = \CRM_Utils_System::baseCMSURL();

// Step 1: Gather data
$dispatchData = [
'Camp_Vehicle_Dispatch.Dropping_Center' => $droppingCenterId,
'Camp_Vehicle_Dispatch.Filled_by' => $contactId,
'Camp_Vehicle_Dispatch.To_which_PU_Center_material_is_being_sent' => $goonjOffice,
'Camp_Vehicle_Dispatch.Goonj_Office_Name' => $goonjOfficeName,
'Eck_Collection_Camp1' => $droppingCenterId,
'id' => $droppingCenterId,
];
Comment on lines +549 to +557

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Validate input parameters before encryption

The method assumes all parameters are valid without verification. This could lead to issues if any required data is missing or malformed.

Add validation before constructing the dispatch data:

+  // Validate required parameters
+  if (empty($droppingCenterId) || empty($contactId) || empty($goonjOffice)) {
+    \Civi::log()->error('Missing required parameters for dispatch email');
+    return;
+  }
+
   // Step 1: Gather data
   $dispatchData = [
     'Camp_Vehicle_Dispatch.Dropping_Center' => $droppingCenterId,
🤖 Prompt for AI Agents
In wp-content/civi-extensions/goonjcustom/Civi/DroppingCenterService.php around
lines 549 to 556, the code constructs the $dispatchData array without validating
input parameters, which risks errors if data is missing or invalid. Add
validation checks for each input parameter (like $droppingCenterId, $contactId,
$goonjOffice, and $goonjOfficeName) before using them to ensure they are set and
correctly formatted. If any required parameter is missing or invalid, handle the
error appropriately before proceeding to construct $dispatchData.


// Step 2: Convert to JSON and encrypt
$json = json_encode($dispatchData);
$token = \CRM_Utils_Crypt::encrypt($json);
Comment on lines +548 to +561

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

Add error handling for encryption failures

The encryption operation could fail, but there's no error handling. If CRM_Utils_Crypt::encrypt() throws an exception or returns false, the email will be sent with a malformed URL.

Wrap the encryption in a try-catch block:

-  // Step 2: Convert to JSON and encrypt
-  $json = json_encode($dispatchData);
-  $token = \CRM_Utils_Crypt::encrypt($json);
+  // Step 2: Convert to JSON and encrypt
+  $json = json_encode($dispatchData);
+  try {
+    $token = \CRM_Utils_Crypt::encrypt($json);
+  } catch (\Exception $e) {
+    \Civi::log()->error('Failed to encrypt dispatch data: ' . $e->getMessage());
+    throw new \CRM_Core_Exception('Failed to generate dispatch email');
+  }
🤖 Prompt for AI Agents
In wp-content/civi-extensions/goonjcustom/Civi/DroppingCenterService.php around
lines 548 to 560, the encryption call to CRM_Utils_Crypt::encrypt() lacks error
handling, risking malformed URLs if encryption fails. Wrap the encryption call
in a try-catch block to catch exceptions and check if the result is false;
handle these cases by logging the error or aborting the operation to prevent
sending invalid data.


$emailHtml = "
<html>
<body>
// Step 3: Generate encrypted URL
$vehicleDispatchFormUrl = $homeUrl . '/vehicle-dispatch/?token=' . urlencode($token);

// Step 4: Compose HTML email
$emailHtml = "
<html>
<body>
<p>Dear {$initiatorName},</p>
<p>Thank you so much for your invaluable efforts in running the Goonj Dropping Center.
Your dedication plays a crucial role in our work, and we deeply appreciate your continued support.</p>
<p>Please fill out this Dispatch Form – <a href='{$vehicleDispatchFormUrl}'>Link</a> once the vehicle is loaded and ready to head to Goonj’s processing center.
This will help us to verify and acknowledge the materials as soon as they arrive.</p>
<p>We truly appreciate your cooperation and continued commitment to our cause.</p>
<p>Warm Regards,<br>Team Goonj..</p>
</body>
</html>
";
$from = HelperService::getDefaultFromEmail();
$mailParams = [
'subject' => 'Kindly fill the Dispatch Form for Material Pickup',
'from' => $from,
'toEmail' => $email,
'html' => $emailHtml,
];
</body>
</html>
";
Comment on lines +567 to +579

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Extract email HTML to a template file

Inline HTML violates the separation of concerns principle and makes the code harder to maintain. Consider moving this to a Smarty template for better maintainability and localization support.

Create a new template file templates/CRM/Goonjcustom/Email/DispatchForm.tpl and use CiviCRM's template system:

-  // Step 4: Compose HTML email
-  $emailHtml = "
-  <html>
-  <body>
-    <p>Dear {$initiatorName},</p>
-    <p>Thank you so much for your invaluable efforts in running the Goonj Dropping Center. 
-    Your dedication plays a crucial role in our work, and we deeply appreciate your continued support.</p>
-    <p>Please fill out this Dispatch Form – <a href='{$vehicleDispatchFormUrl}'>Link</a> once the vehicle is loaded and ready to head to Goonj's processing center. 
-    This will help us to verify and acknowledge the materials as soon as they arrive.</p>
-    <p>We truly appreciate your cooperation and continued commitment to our cause.</p>
-    <p>Warm Regards,<br>Team Goonj..</p>
-  </body>
-  </html>
-  ";
+  // Step 4: Compose HTML email
+  $tplParams = [
+    'initiatorName' => $initiatorName,
+    'vehicleDispatchFormUrl' => $vehicleDispatchFormUrl,
+  ];
+  $emailHtml = CRM_Core_Smarty::singleton()->fetchWith('CRM/Goonjcustom/Email/DispatchForm.tpl', $tplParams);
📝 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
$emailHtml = "
<html>
<body>
<p>Dear {$initiatorName},</p>
<p>Thank you so much for your invaluable efforts in running the Goonj Dropping Center.
Your dedication plays a crucial role in our work, and we deeply appreciate your continued support.</p>
<p>Please fill out this Dispatch Form – <a href='{$vehicleDispatchFormUrl}'>Link</a> once the vehicle is loaded and ready to head to Goonj’s processing center.
This will help us to verify and acknowledge the materials as soon as they arrive.</p>
<p>We truly appreciate your cooperation and continued commitment to our cause.</p>
<p>Warm Regards,<br>Team Goonj..</p>
</body>
</html>
";
$from = HelperService::getDefaultFromEmail();
$mailParams = [
'subject' => 'Kindly fill the Dispatch Form for Material Pickup',
'from' => $from,
'toEmail' => $email,
'html' => $emailHtml,
];
</body>
</html>
";
// Step 4: Compose HTML email
$tplParams = [
'initiatorName' => $initiatorName,
'vehicleDispatchFormUrl'=> $vehicleDispatchFormUrl,
];
$emailHtml = CRM_Core_Smarty::singleton()
->fetchWith('CRM/Goonjcustom/Email/DispatchForm.tpl', $tplParams);
🤖 Prompt for AI Agents
In wp-content/civi-extensions/goonjcustom/Civi/DroppingCenterService.php around
lines 566 to 578, the email HTML is hardcoded inline, which reduces
maintainability and localization support. To fix this, extract the HTML content
into a new Smarty template file named
templates/CRM/Goonjcustom/Email/DispatchForm.tpl. Then, update the PHP code to
load and render this template using CiviCRM's template system, passing necessary
variables like initiatorName and vehicleDispatchFormUrl to the template for
dynamic content rendering.


// Step 5: Send email
$from = HelperService::getDefaultFromEmail();
$mailParams = [
'subject' => 'Kindly fill the Dispatch Form for Material Pickup',
'from' => $from,
'toEmail' => $email,
'html' => $emailHtml,
];

\CRM_Utils_Mail::send($mailParams);
}
\CRM_Utils_Mail::send($mailParams);
}

/**
*
Expand Down
7 changes: 7 additions & 0 deletions wp-content/civi-extensions/goonjcustom/goonjcustom.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,13 @@ function goonjcustom_civicrm_config(&$config): void {
\Civi::dispatcher()->addSubscriber(new CRM_Goonjcustom_Token_IndividualGoonjActivities());
}

/**
* Implements hook_civicrm_alterAngular().
*/
function goonjcustom_civicrm_alterAngular($angular) {

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

Remove unused parameter $angular

The $angular parameter is not used in the function implementation.

Apply this diff to remove the unused parameter:

-function goonjcustom_civicrm_alterAngular($angular) {
+function goonjcustom_civicrm_alterAngular() {
📝 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
function goonjcustom_civicrm_alterAngular($angular) {
-function goonjcustom_civicrm_alterAngular($angular) {
+function goonjcustom_civicrm_alterAngular() {
🧰 Tools
🪛 PHPMD (2.15.0)

39-39: Avoid unused parameters such as '$angular'. (Unused Code Rules)

(UnusedFormalParameter)

🤖 Prompt for AI Agents
In wp-content/civi-extensions/goonjcustom/goonjcustom.php at line 39, the
function goonjcustom_civicrm_alterAngular declares a parameter $angular that is
not used anywhere in the function body. Remove the $angular parameter from the
function definition to clean up the code and avoid unused parameter warnings.

CRM_Goonjcustom_AfformFieldPrefillService::preprocess($angular);
}
Comment on lines +36 to +77

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add token validation and consider splitting responsibilities

The function handles multiple responsibilities and lacks token validation:

  1. Missing token expiration/integrity checks before decryption
  2. No validation of expected JSON structure after parsing
  3. Function combines token processing with resource injection

Consider:

  1. Add token validation (expiration, signature) before decryption
  2. Extract token processing into a separate service class
  3. Validate the decrypted data structure matches expected schema
🧰 Tools
🪛 PHPMD (2.15.0)

39-39: Avoid unused parameters such as '$angular'. (Unused Code Rules)

(UnusedFormalParameter)

🤖 Prompt for AI Agents
In wp-content/civi-extensions/goonjcustom/goonjcustom.php around lines 36 to 77,
the goonjcustom_civicrm_alterAngular function lacks token validation, does not
verify the decrypted JSON structure, and mixes token processing with resource
injection. To fix this, first add token validation checks such as expiration and
signature verification before attempting decryption. Then, refactor by
extracting the token processing logic into a separate service class to separate
concerns. Finally, validate that the decrypted data matches the expected JSON
schema before encoding and injecting it into the page.


/**
* Implements hook_civicrm_install().
*
Expand Down