Skip to content

Commit

Permalink
ISSUE-214 --- Use the AMI set user ID for checking access to entity o…
Browse files Browse the repository at this point in the history 
…perations in AmiUtilityService::preprocessAmiSet and AmiUtilityService::getProcessedAmiSetNodeUUids. Fixes cases where the process is executing under cron or hydroponics and the user is anonymous. (#215)
  • Loading branch information
@patdunlavey
patdunlavey authored Aug 30, 2024
1 parent f9d012e commit 567fea5
Showing 1 changed file with 8 additions and 2 deletions.
10 changes: 8 additions & 2 deletions src/AmiUtilityService.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1759,6 +1759,9 @@ public function createAmiSet(\stdClass $data) {
*/
public function preprocessAmiSet(File $file, \stdClass $data, array &$invalid = [], $strict = FALSE): array {

// Use the AMI set user ID for checking access to entity operations.
$account = $data->info['uid'] == \Drupal::currentUser()->id() ? \Drupal::currentUser() : $this->entityTypeManager->getStorage('user')->load($data->info['uid']);

This comment has been minimized.

Sign in to view

Copy link
@patdunlavey

patdunlavey Sep 5, 2024

Author Contributor

Oops - it seems that $data->info['uid'] is not defined here when called from Drupal\ami\Form\amiSetEntityProcessForm->submitForm, resulting in a fatal error:

AssertionError: Cannot load the "user" entity with NULL ID. in assert() (line 261 of core/lib/Drupal/Core/Entity/EntityStorageBase.php).
Drupal\Core\Entity\EntityStorageBase->load(NULL) (Line: 1763)
Drupal\ami\AmiUtilityService->preprocessAmiSet(Object, Object, Array, ) (Line: 173)
Drupal\ami\Form\amiSetEntityProcessForm->submitForm(Array, Object)
call_user_func_array(Array, Array) (Line: 129)
Drupal\Core\Form\FormSubmitter->executeSubmitHandlers(Array, Object) (Line: 67)
Drupal\Core\Form\FormSubmitter->doSubmitForm(Array, Object) (Line: 597)
Drupal\Core\Form\FormBuilder->processForm('ami_set_entity_process_form', Array, Object) (Line: 325)
Drupal\Core\Form\FormBuilder->buildForm(Object, Object) (Line: 73)
Drupal\Core\Controller\FormController->getContentResult(Object, Object)
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 627)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 43)
Drupal\strawberryfield\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 181)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 76)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 58)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 28)
Drupal\Core\StackMiddleware\ContentLength->handle(Object, 1, 1) (Line: 106)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 85)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 36)
Drupal\Core\StackMiddleware\AjaxPageState->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle(Object, 1, 1) (Line: 704)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

This comment has been minimized.

Sign in to view

Copy link
@DiegoPino

DiegoPino Sep 5, 2024

Member

@patdunlavey ok, that can be fixed easily by actually adding that key in the form.... give me a minute

This comment has been minimized.

Sign in to view

Copy link
@DiegoPino

DiegoPino Sep 5, 2024

Member

I will also check In the actual call just in case.

This comment has been minimized.

Sign in to view

Copy link
@patdunlavey

patdunlavey Sep 5, 2024

Author Contributor

I'm not sure if it will also be a problem in getProcessedAmiSetNodeUUids, which is called from \Drupal\ami\Form\amiSetEntityDeleteProcessedForm::submitForm

This comment has been minimized.

Sign in to view

Copy link
@patdunlavey

patdunlavey Sep 5, 2024

Author Contributor

I can confirm that it is also causing a white screen when submitting "Delete Processed ADOs"

This comment has been minimized.

Sign in to view

Copy link
@DiegoPino

DiegoPino Sep 5, 2024

Member

@patdunlavey can you please check #218 and see if that covers all the regressions? thanks (want to move fast bc I updated deployment to that previous hash!(

$file_data_all = $this->csv_read($file);
// We want to validate here if the found Headers match at least the
// Mapped ones during AMI setup. If not we will return an empty array
Expand Down Expand Up @@ -1839,7 +1842,7 @@ public function preprocessAmiSet(File $file, \stdClass $data, array &$invalid =
// In case access changes later of course
// Processors do NOT delete. So we only check for Update.
$existing_object = $existing_objects && count($existing_objects) == 1 ? reset($existing_objects) : NULL;
if (!$existing_object || !$existing_object->access('update')) {
if (!$existing_object || !$existing_object->access('update', $account)) {
unset($ado);
$invalid = $invalid + [$index => $index];
}
Expand Down Expand Up @@ -2122,6 +2125,9 @@ protected function validateAmiSet(array $file_data_all, \stdClass $data, $strict
*/
public function getProcessedAmiSetNodeUUids(File $file, \stdClass $data, $op = NULL) {

// Use the AMI set user ID for checking access to entity operations.
$account = $data->info['uid'] == \Drupal::currentUser()->id() ? \Drupal::currentUser() : $this->entityTypeManager->getStorage('user')->load($data->info['uid']);

$file_data_all = $this->csv_read($file);
// we may want to check if saved metadata headers == csv ones first.
// $data->column_keys
Expand All @@ -2142,7 +2148,7 @@ public function getProcessedAmiSetNodeUUids(File $file, \stdClass $data, $op = N
// In case access changes later of course
// This does NOT delete. So we only check for Update.
$existing_object = $existing_objects && count($existing_objects) == 1 ? reset($existing_objects) : NULL;
if ($existing_object && $existing_object->access($op)) {
if ($existing_object && $existing_object->access($op, $account)) {
$uuids[] = $possibleUUID;
}
}
Expand Down

0 comments on commit 567fea5

Please sign in to comment.