From f0a52f2bdaa05fbf7dfa09eebfe9627a847fcadb Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Tue, 4 Jun 2013 15:14:00 -0300 Subject: [PATCH 01/22] Initial implementation of the hooked access callback... ... Now have to make it get used everywhere. --- islandora.api.php | 39 +++++++++++++++++++++++++ islandora.module | 73 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 112 insertions(+) diff --git a/islandora.api.php b/islandora.api.php index 89e1e048..603525ec 100644 --- a/islandora.api.php +++ b/islandora.api.php @@ -443,3 +443,42 @@ function hook_islandora_ingest_steps(array $form_state) { */ function hook_CMODEL_PID_islandora_ingest_steps(array $form_state) { } + +/** + * Hookable access hook. + * + * @param string $op + * A string define an operation to check. Should be defined via + * hook_permission(). + * @param AbstractObject $object + * An object to check the operation on. + * @param object $user + * A loaded user object, as the global $user variable might contain. + * + * @return bool|NULL + * Either boolean TRUE or FALSE to explicitly allow or deny the operation on + * the given object, or NULL to indicate that we are making no assertion + * about the outcome. + */ +function hook_islandora_access($op, $object, $user) { + switch ($op) { + case 'create stuff': + return TRUE; + + case 'break stuff': + return FALSE; + + case 'do a barrel roll!': + return NULL; + + } +} + +/** + * Content model specific version of hook_islandora_access(). + * + * @see hook_islandora_access + */ +function hook_CMODEL_PID_islandora_access($op, $object, $user) { + +} diff --git a/islandora.module b/islandora.module index d6c89e99..dbec912d 100644 --- a/islandora.module +++ b/islandora.module @@ -1126,3 +1126,76 @@ function islandora_file_mimetype_mapping_alter(&$mapping) { $mapping['extensions'][$ext] = key($mapping['mimetypes']); } } + +/** + * Hookable access callback. + * + * @param string $op + * String identifying an operation to check. Should correspond to a + * permission declared via hook_permission(). + * @param AbstractObject $object + * An object to check for permissions. + * @param object $user + * An optional loaded user object. Defaults to the global $user. + * + * @return bool + * TRUE if at least one implementation of hook_islandora_access() returned + * TRUE, and no implementation return FALSE; FALSE otherwise. + */ +function islandora_access($op, $object = NULL, $user = NULL) { + $cache = &drupal_static(__FUNCTION__); + + if (empty($object)) { + $pid = variable_get('islandora_repository_pid', 'islandora:root'); + $object = islandora_object_load($pid); + } + if (!$object) { + // The object could not be loaded... Presumably, we don't have + // permission. + return FALSE; + } + if ($user === NULL) { + global $user; + } + + // Populate the cache on a miss. + if (!isset($cache[$op][$object->id][$user->uid])) { + module_load_include('inc', 'islandora', 'includes/utilities'); + + $results = islandora_invoke_hook_list('islandora_access', $object->models, array( + $op, + $object, + $user, + )); + + // Nothing returned FALSE, and something returned TRUE. + $cache[$op][$object->id][$user->uid] = (!in_array(FALSE, $results, TRUE) && in_array(TRUE, $results, TRUE)); + } + + return $cache[$op][$object->id][$user->uid]; +} + +/** + * Implements hook_islandora_access(). + * + * Denies according to PID namespace restrictions, passes according to + * user_access(), and makes no indication if namespace restrictions passed but + * user_access() returned a fail, to allow other modules to allow an operation. + */ +function islandora_islandora_access($op, $object, $user) { + module_load_include('inc', 'islandora', 'includes/utilities'); + $to_return = islandora_namespace_accessible($object->id); + + if ($to_return && user_access($op, $user)) { + // Straight Drupal permissions, let's allow it. + return TRUE; + } + elseif ($to_return === FALSE) { + // PID namespace is outside of those allowed. Forbid! + return FALSE; + } + else { + // Neither allowing of forbidding, to allow other modules to override. + return NULL; + } +} From 9459899105efaf841e7c71647ae6c1dc1575cf8d Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Tue, 4 Jun 2013 16:43:28 -0300 Subject: [PATCH 02/22] Add hook_islandora_datastream_access()... ... Also, start using our new access functions in a few places. --- includes/datastream.inc | 4 +-- islandora.api.php | 45 +++++++++++++++++++++++--- islandora.module | 72 ++++++++++++++++++++++++++++++++--------- 3 files changed, 99 insertions(+), 22 deletions(-) diff --git a/includes/datastream.inc b/includes/datastream.inc index 23eb44f9..a0fd7669 100644 --- a/includes/datastream.inc +++ b/includes/datastream.inc @@ -85,7 +85,7 @@ function islandora_datastream_get_url(AbstractDatastream $datastream, $type = 'd */ function islandora_datastream_get_delete_link(AbstractDatastream $datastream) { $datastreams = module_invoke_all('islandora_undeletable_datastreams', $datastream->parent->models); - $can_delete = !in_array($datastream->id, $datastreams); + $can_delete = !in_array($datastream->id, $datastreams) && islandora_datastream_access(FEDORA_PURGE, $datastream); return $can_delete ? l(t('delete'), "islandora/object/{$datastream->parent->id}/datastream/{$datastream->id}/delete") : ''; } @@ -97,7 +97,7 @@ function islandora_datastream_get_delete_link(AbstractDatastream $datastream) { */ function islandora_datastream_edit_get_link(AbstractDatastream $datastream) { $edit_registry = module_invoke_all('islandora_edit_datastream_registry', $datastream->parent, $datastream); - $can_edit = count($edit_registry) > 0 && user_access(FEDORA_METADATA_EDIT); + $can_edit = count($edit_registry) > 0 && islandora_datastream_access(FEDORA_METADATA_EDIT, $datastream); return $can_edit ? l(t('edit'), "islandora/object/{$datastream->parent->id}/datastream/{$datastream->id}/edit") : ''; } diff --git a/islandora.api.php b/islandora.api.php index 603525ec..aa6df5b6 100644 --- a/islandora.api.php +++ b/islandora.api.php @@ -460,7 +460,7 @@ function hook_CMODEL_PID_islandora_ingest_steps(array $form_state) { * the given object, or NULL to indicate that we are making no assertion * about the outcome. */ -function hook_islandora_access($op, $object, $user) { +function hook_islandora_object_access($op, $object, $user) { switch ($op) { case 'create stuff': return TRUE; @@ -475,10 +475,47 @@ function hook_islandora_access($op, $object, $user) { } /** - * Content model specific version of hook_islandora_access(). + * Content model specific version of hook_islandora_object_access(). * - * @see hook_islandora_access + * @see hook_islandora_object_access() */ -function hook_CMODEL_PID_islandora_access($op, $object, $user) { +function hook_CMODEL_PID_islandora_object_access($op, $object, $user) { +} + +/** + * Hookable access hook. + * + * @param string $op + * A string define an operation to check. Should be defined via + * hook_permission(). + * @param AbstractDatastream $object + * An object to check the operation on. + * @param object $user + * A loaded user object, as the global $user variable might contain. + * + * @return bool|NULL + * Either boolean TRUE or FALSE to explicitly allow or deny the operation on + * the given object, or NULL to indicate that we are making no assertion + * about the outcome. + */ +function hook_islandora_datastream_access($op, $object, $user) { + switch ($op) { + case 'create stuff': + return TRUE; + + case 'break stuff': + return FALSE; + + case 'do a barrel roll!': + return NULL; + + } +} +/** + * Content model specific version of hook_islandora_datastream_access(). + * + * @see hook_islandora_datastream_access() + */ +function hook_CMODEL_PID_islandora_datastream_access($op, $object, $user) { } diff --git a/islandora.module b/islandora.module index dbec912d..5f156ee3 100644 --- a/islandora.module +++ b/islandora.module @@ -475,7 +475,7 @@ function islandora_object_access_callback($perm, $object = NULL) { return FALSE; } - return user_access($perm) && is_object($object) && islandora_namespace_accessible($object->id); + return islandora_object_access($perm); } /** @@ -499,8 +499,7 @@ function islandora_object_access_callback($perm, $object = NULL) { * TRUE if the user is allowed to access this object, FALSE otherwise. */ function islandora_object_datastream_access_callback($perm, $object = NULL, $datastream = NULL, $account = NULL) { - module_load_include('inc', 'islandora', 'includes/utilities'); - return user_access($perm, $account) && is_object($object) && islandora_namespace_accessible($object->id) && is_object($datastream); + return islandora_datastream_access($perm, $datastream, $account); } /** @@ -554,10 +553,10 @@ function islandora_object_manage_access_callback($perms, $object = NULL) { $has_access = FALSE; for ($i = 0; $i < count($perms) && !$has_access; $i++) { - $has_access = $has_access || user_access($perms[$i]); + $has_access = $has_access || islandora_object_access($perms[$i], $object); } - return $has_access && is_object($object) && islandora_namespace_accessible($object->id); + return $has_access; } /** @@ -1128,7 +1127,7 @@ function islandora_file_mimetype_mapping_alter(&$mapping) { } /** - * Hookable access callback. + * Hookable object access callback. * * @param string $op * String identifying an operation to check. Should correspond to a @@ -1139,17 +1138,13 @@ function islandora_file_mimetype_mapping_alter(&$mapping) { * An optional loaded user object. Defaults to the global $user. * * @return bool - * TRUE if at least one implementation of hook_islandora_access() returned + * TRUE if at least one implementation of hook_islandora_object_access() returned * TRUE, and no implementation return FALSE; FALSE otherwise. */ -function islandora_access($op, $object = NULL, $user = NULL) { +function islandora_object_access($op, $object, $user = NULL) { $cache = &drupal_static(__FUNCTION__); - if (empty($object)) { - $pid = variable_get('islandora_repository_pid', 'islandora:root'); - $object = islandora_object_load($pid); - } - if (!$object) { + if (!is_object($object)) { // The object could not be loaded... Presumably, we don't have // permission. return FALSE; @@ -1162,7 +1157,7 @@ function islandora_access($op, $object = NULL, $user = NULL) { if (!isset($cache[$op][$object->id][$user->uid])) { module_load_include('inc', 'islandora', 'includes/utilities'); - $results = islandora_invoke_hook_list('islandora_access', $object->models, array( + $results = islandora_invoke_hook_list('islandora_object_access', $object->models, array( $op, $object, $user, @@ -1176,13 +1171,13 @@ function islandora_access($op, $object = NULL, $user = NULL) { } /** - * Implements hook_islandora_access(). + * Implements hook_islandora_object_access(). * * Denies according to PID namespace restrictions, passes according to * user_access(), and makes no indication if namespace restrictions passed but * user_access() returned a fail, to allow other modules to allow an operation. */ -function islandora_islandora_access($op, $object, $user) { +function islandora_islandora_object_access($op, $object, $user) { module_load_include('inc', 'islandora', 'includes/utilities'); $to_return = islandora_namespace_accessible($object->id); @@ -1199,3 +1194,48 @@ function islandora_islandora_access($op, $object, $user) { return NULL; } } + +/** + * Hookable access callback for datastreams. + * + * Requires the equivalent permissions on the object. + */ +function islandora_datastream_access($op, $datastream, $user = NULL) { + $cache = &drupal_static(__FUNCTION__); + + if (!$datastream) { + // The object could not be loaded... Presumably, we don't have + // permission. + return NULL; + } + if ($user === NULL) { + global $user; + } + + // Populate the cache on a miss. + if (!isset($cache[$op][$object->id][$user->uid])) { + if ($cache[$op][$datastream->parent->id][$datastream->id][$user->uid]) { + module_load_include('inc', 'islandora', 'includes/utilities'); + $object_results = islandora_invoke_hook_list('islandora_object_access', $datastream->parent->models, array( + $op, + $datastream->parent, + $user, + )); + + $datastream_results = islandora_invoke_hook_list('islandora_datastream_access', $datastream->parent->models, array( + $op, + $datastream, + $user, + )); + + // Neither the object nor the datastream check returned FALSE, and one in + // the object or datastream checks returned TRUE. + $cache[$op][$datastream->parent->id][$datastream->id][$user->uid] = + !in_array(FALSE, $object_results, TRUE) && + !in_array(FALSE, $datastream_results, TRUE) && + (in_array(TRUE, $object_results, TRUE) || in_array(TRUE, $datastream_results, TRUE)); + } + } + + return $cache[$op][$datastream->parent->id][$datastream->id][$user->uid]; +} From 368492a54e23f9d5dcd583fae2a157257e105641 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Wed, 5 Jun 2013 10:21:00 -0300 Subject: [PATCH 03/22] Add access checks to view/download links. --- theme/theme.inc | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/theme/theme.inc b/theme/theme.inc index 7c43deef..d03370d5 100644 --- a/theme/theme.inc +++ b/theme/theme.inc @@ -29,7 +29,9 @@ function islandora_preprocess_islandora_default_edit(array &$variables) { $rows[] = array( array( 'class' => 'datastream-id', - 'data' => l($ds->id, islandora_datastream_get_url($ds, 'view')), + 'data' => (islandora_datastream_access(FEDORA_VIEW_OBJECTS, $ds) ? + l($ds->id, islandora_datastream_get_url($ds, 'view')) : + ''), ), array( 'class' => 'datastream-label', @@ -49,7 +51,9 @@ function islandora_preprocess_islandora_default_edit(array &$variables) { ), array( 'class' => 'datastream-download', - 'data' => l(t('download'), islandora_datastream_get_url($ds, 'download')), + 'data' => (islandora_datastream_access(FEDORA_VIEW_OBJECTS, $ds) ? + l(t('download'), islandora_datastream_get_url($ds, 'download')) : + ''), ), array( 'class' => 'datstream-edit', From a8f13b0a46155c3f3dad8c9ea11ac5072e44e401 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Wed, 5 Jun 2013 16:27:10 -0300 Subject: [PATCH 04/22] Add access checks. --- includes/datastream.inc | 34 +++++++++++++++++++ includes/object_properties.form.inc | 1 + islandora.module | 51 ++++++++++++++--------------- theme/theme.inc | 30 +++++++++-------- 4 files changed, 76 insertions(+), 40 deletions(-) diff --git a/includes/datastream.inc b/includes/datastream.inc index a0fd7669..902d5670 100644 --- a/includes/datastream.inc +++ b/includes/datastream.inc @@ -144,3 +144,37 @@ function islandora_edit_datastream_registry_render(array $edit_registry) { '#markup' => $markup, ); } + +/** + * Get markup for a download link. + * + * @param AbstractDatastream $datastream + * The datastream for which to generate a link. + * + * @return string + * Either the link markup if the user has access or an empty string if the + * user is not allowed to see the given datastream. + */ +function islandora_datastream_get_download_link(AbstractDatastream $datastream) { + $label = t('download'); + return islandora_datastream_access(FEDORA_VIEW_OBJECTS, $datastream) ? + l($label, islandora_datastream_get_url($datastream, 'download')) : + ''; +} + +/** + * Get markup for a view link. + * + * @param AbstractDatastream $datastream + * The datastream for which to generate a link. + * + * @return string + * Either the link markup if the user has access or a string containing the + * datastream ID if the user is not allowed to see the given datastream. + */ +function islandora_datastream_get_view_link(AbstractDatastream $datastream) { + $label = check_plain($datastream->id); + return islandora_datastream_access(FEDORA_VIEW_OBJECTS, $datastream) ? + l($label, islandora_datastream_get_url($datastream, 'view')) : + $label; +} diff --git a/includes/object_properties.form.inc b/includes/object_properties.form.inc index 543fa6dd..44d016b7 100644 --- a/includes/object_properties.form.inc +++ b/includes/object_properties.form.inc @@ -60,6 +60,7 @@ function islandora_object_properties_form(array $form, array &$form_state, Abstr ), 'delete' => array( '#type' => 'submit', + '#access' => islandora_object_access(FEDORA_PURGE, $object), '#value' => t('Delete'), '#submit' => array('islandora_object_properties_form_delete'), '#limit_validation_errors' => array(array('pid')), diff --git a/islandora.module b/islandora.module index 5f156ee3..daa612a9 100644 --- a/islandora.module +++ b/islandora.module @@ -475,7 +475,7 @@ function islandora_object_access_callback($perm, $object = NULL) { return FALSE; } - return islandora_object_access($perm); + return islandora_object_access($perm, $object); } /** @@ -1138,8 +1138,8 @@ function islandora_file_mimetype_mapping_alter(&$mapping) { * An optional loaded user object. Defaults to the global $user. * * @return bool - * TRUE if at least one implementation of hook_islandora_object_access() returned - * TRUE, and no implementation return FALSE; FALSE otherwise. + * TRUE if at least one implementation of hook_islandora_object_access() + * returned TRUE, and no implementation return FALSE; FALSE otherwise. */ function islandora_object_access($op, $object, $user = NULL) { $cache = &drupal_static(__FUNCTION__); @@ -1203,7 +1203,7 @@ function islandora_islandora_object_access($op, $object, $user) { function islandora_datastream_access($op, $datastream, $user = NULL) { $cache = &drupal_static(__FUNCTION__); - if (!$datastream) { + if (!is_object($datastream)) { // The object could not be loaded... Presumably, we don't have // permission. return NULL; @@ -1213,28 +1213,27 @@ function islandora_datastream_access($op, $datastream, $user = NULL) { } // Populate the cache on a miss. - if (!isset($cache[$op][$object->id][$user->uid])) { - if ($cache[$op][$datastream->parent->id][$datastream->id][$user->uid]) { - module_load_include('inc', 'islandora', 'includes/utilities'); - $object_results = islandora_invoke_hook_list('islandora_object_access', $datastream->parent->models, array( - $op, - $datastream->parent, - $user, - )); - - $datastream_results = islandora_invoke_hook_list('islandora_datastream_access', $datastream->parent->models, array( - $op, - $datastream, - $user, - )); - - // Neither the object nor the datastream check returned FALSE, and one in - // the object or datastream checks returned TRUE. - $cache[$op][$datastream->parent->id][$datastream->id][$user->uid] = - !in_array(FALSE, $object_results, TRUE) && - !in_array(FALSE, $datastream_results, TRUE) && - (in_array(TRUE, $object_results, TRUE) || in_array(TRUE, $datastream_results, TRUE)); - } + if (!isset($cache[$op][$datastream->parent->id][$datastream->id][$user->uid])) { + module_load_include('inc', 'islandora', 'includes/utilities'); + $object_results = islandora_invoke_hook_list('islandora_object_access', $datastream->parent->models, array( + $op, + $datastream->parent, + $user, + )); + + $datastream_results = islandora_invoke_hook_list('islandora_datastream_access', $datastream->parent->models, array( + $op, + $datastream, + $user, + )); + + // Neither the object nor the datastream check returned FALSE, and one in + // the object or datastream checks returned TRUE. + $cache[$op][$datastream->parent->id][$datastream->id][$user->uid] = ( + !in_array(FALSE, $object_results, TRUE) && + !in_array(FALSE, $datastream_results, TRUE) && + (in_array(TRUE, $object_results, TRUE) || in_array(TRUE, $datastream_results, TRUE)) + ); } return $cache[$op][$datastream->parent->id][$datastream->id][$user->uid]; diff --git a/theme/theme.inc b/theme/theme.inc index d03370d5..e9db9f3e 100644 --- a/theme/theme.inc +++ b/theme/theme.inc @@ -29,9 +29,7 @@ function islandora_preprocess_islandora_default_edit(array &$variables) { $rows[] = array( array( 'class' => 'datastream-id', - 'data' => (islandora_datastream_access(FEDORA_VIEW_OBJECTS, $ds) ? - l($ds->id, islandora_datastream_get_url($ds, 'view')) : - ''), + 'data' => islandora_datastream_get_view_link($ds), ), array( 'class' => 'datastream-label', @@ -51,9 +49,7 @@ function islandora_preprocess_islandora_default_edit(array &$variables) { ), array( 'class' => 'datastream-download', - 'data' => (islandora_datastream_access(FEDORA_VIEW_OBJECTS, $ds) ? - l(t('download'), islandora_datastream_get_url($ds, 'download')) : - ''), + 'data' => islandora_datastream_get_download_link($ds), ), array( 'class' => 'datstream-edit', @@ -99,7 +95,9 @@ function islandora_preprocess_islandora_default(&$variables) { $download_path = islandora_datastream_get_url($ds, 'download'); $datastreams[$id]['id'] = $id; $datastreams[$id]['label'] = $label; - $datastreams[$id]['label_link'] = l($label, $download_path); + $datastreams[$id]['label_link'] = islandora_datastream_access(FEDORA_VIEW_OBJECTS, $ds) ? + l($label, $download_path) : + $label; $datastreams[$id]['download_url'] = $download_path; $datastreams[$id]['mimetype'] = $ds->mimetype; $datastreams[$id]['size'] = islandora_datastream_get_human_readable_size($ds); @@ -112,14 +110,14 @@ function islandora_preprocess_islandora_default(&$variables) { } $variables['datastreams'] = $datastreams; // Objects in fcrepo4 don't always contain a DC datastream. - if (isset($islandora_object['DC'])) { + if (isset($islandora_object['DC']) && islandora_datastream_access(FEDORA_VIEW_OBJECTS, $islandora_object['DC'])) { $dc_object = DublinCore::importFromXMLString($islandora_object['DC']->content); $dc_array = $dc_object->asArray(); } $variables['dc_array'] = isset($dc_array) ? $dc_array : array(); $variables['islandora_dublin_core'] = isset($dc_object) ? $dc_object : NULL; $variables['islandora_object_label'] = $islandora_object->label; - if (isset($islandora_object['TN'])) { + if (isset($islandora_object['TN']) && islandora_datastream_access(FEDORA_VIEW_OBJECTS, $islandora_object['TN'])) { $variables['islandora_thumbnail_url'] = url("islandora/object/{$islandora_object->id}/datastream/TN/view"); } } @@ -201,12 +199,16 @@ function islandora_preprocess_islandora_objects(array &$variables) { $o = islandora_object_load($o); $url = "islandora/object/{$o->id}"; $link_options = array('html' => TRUE, 'attributes' => array('title' => $o->label)); - $img = theme_image(array('path' => url("$url/datastream/TN/view"), 'attributes' => array())); + $img = islandora_datastream_access(FEDORA_VIEW_OBJECTS, $o['TN']) ? + theme('image', array('path' => url("$url/datastream/TN/view"), 'attributes' => array())) : + ''; $description = NULL; - $dc = DublinCore::importFromXMLString($o['DC']->content); - if ($dc) { - $dc = $dc->asArray(); - $description = $dc['dc:description']['value']; + if (islandora_datastream_access($o['DC'])) { + $dc = DublinCore::importFromXMLString($o['DC']->content); + if ($dc) { + $dc = $dc->asArray(); + $description = $dc['dc:description']['value']; + } } return array( 'label' => $o->label, From bc8492a7879b6599da54a064717905f002d66383 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Fri, 7 Jun 2013 10:46:59 -0300 Subject: [PATCH 05/22] Add a couple more access checks. --- includes/utilities.inc | 2 +- theme/theme.inc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/includes/utilities.inc b/includes/utilities.inc index cdbf8daa..4d69155d 100644 --- a/includes/utilities.inc +++ b/includes/utilities.inc @@ -389,7 +389,7 @@ function islandora_get_datastreams_requirements_from_models(array $models) { * - "optional": A boolean indicating if the given stream is optional. */ function islandora_get_datastreams_requirements_from_content_model(AbstractObject $object) { - if (empty($object[DS_COMP_STREAM])) { + if (empty($object[DS_COMP_STREAM]) || !islandora_datastream_access(FEDORA_VIEW_OBJECTS, $object[DS_COMP_STREAM])) { return array(); } $xml = new SimpleXMLElement($object[DS_COMP_STREAM]->content); diff --git a/theme/theme.inc b/theme/theme.inc index e9db9f3e..ce6f2659 100644 --- a/theme/theme.inc +++ b/theme/theme.inc @@ -203,7 +203,7 @@ function islandora_preprocess_islandora_objects(array &$variables) { theme('image', array('path' => url("$url/datastream/TN/view"), 'attributes' => array())) : ''; $description = NULL; - if (islandora_datastream_access($o['DC'])) { + if (isset($o['DC']) && islandora_datastream_access(FEDORA_VIEW_OBJECTS, $o['DC'])) { $dc = DublinCore::importFromXMLString($o['DC']->content); if ($dc) { $dc = $dc->asArray(); From 70333d8783e33de80c5b28c7eb4a8b77b7beb959 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Fri, 7 Jun 2013 13:32:42 -0300 Subject: [PATCH 06/22] Add the strict access flag. --- includes/admin.form.inc | 7 +++++++ islandora.module | 6 ++++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/includes/admin.form.inc b/includes/admin.form.inc index a1f22652..f4db785b 100644 --- a/includes/admin.form.inc +++ b/includes/admin.form.inc @@ -108,6 +108,13 @@ function islandora_repository_admin(array $form, array &$form_state) { '#required' => TRUE, ); + $form['islandora_tabs']['islandora_general']['islandora_strict_user_access_enforcement'] = array( + '#type' => 'checkbox', + '#title' => t('Strict User Access Enforcement'), + '#description' => t('Restrict permissions to the result of user_access(); other modules will be able to deny things, but other modules will not be able to allow operations not allowed via Drupal permissions.'), + '#default_value' => variable_get('islandora_strict_user_access_enforcement', TRUE), + ); + $form['islandora_tabs']['islandora_namespace'] = array( '#type' => 'fieldset', '#title' => t('Namespaces'), diff --git a/islandora.module b/islandora.module index daa612a9..b5319122 100644 --- a/islandora.module +++ b/islandora.module @@ -1181,11 +1181,13 @@ function islandora_islandora_object_access($op, $object, $user) { module_load_include('inc', 'islandora', 'includes/utilities'); $to_return = islandora_namespace_accessible($object->id); - if ($to_return && user_access($op, $user)) { + $user_access_result = user_access($op, $user); + + if ($to_return && $user_access_result) { // Straight Drupal permissions, let's allow it. return TRUE; } - elseif ($to_return === FALSE) { + elseif ($to_return === FALSE || variable_get('islandora_strict_user_access_enforcement', TRUE) && !$user_access_result) { // PID namespace is outside of those allowed. Forbid! return FALSE; } From ef4a0fe021850f4f3acd3bd44b98d745132079ca Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Mon, 10 Jun 2013 15:13:50 -0300 Subject: [PATCH 07/22] Use the hooked access callback in place of the any/all cases. Leaving the functions called in place, in the case that they're called elsewhere. --- islandora.module | 45 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 38 insertions(+), 7 deletions(-) diff --git a/islandora.module b/islandora.module index b5319122..96836f4c 100644 --- a/islandora.module +++ b/islandora.module @@ -432,24 +432,55 @@ function islandora_user_access($object, array $permissions, $content_models = ar } } // Determine what has been passed as $object. - if (is_subclass_of($object, 'FedoraObject')) { - $object = $object; + if (is_subclass_of($object, 'AbstractObject')) { + $datastream = NULL; + // $object stays $object... } - elseif (is_subclass_of($object, 'FedoraDatastream')) { + elseif (is_subclass_of($object, 'AbstractDatastream')) { $datastream = $object; $object = $datastream->parent; } + // Check for access. $accessible_namespace = islandora_namespace_accessible($object->id); if ($access_any) { - $has_required_permissions = islandora_user_access_any($permissions, $account); + $has_required_permissions = function ($permissions, $datastream, $object) { + foreach ($permissions as $p) { + if ($datastream !== NULL) { + $check = islandora_datastream_access($p, $datastream); + } + else { + $check = islandora_object_access($p, $object); + } + + if ($check) { + return TRUE; + } + } + }; $has_required_content_models = empty($content_models) ? TRUE : count(array_intersect($object->models, $content_models)) > 0; } else { - $has_required_permissions = islandora_user_access_all($permissions, $account); + $has_required_permissions = function ($permissions, $datastream, $object) { + foreach ($permissions as $p) { + if ($datastream !== NULL) { + $check = islandora_datastream_access($p, $datastream); + } + else { + $check = islandora_object_access($p, $object); + } + + if (!$check) { + return FALSE; + } + } + }; $has_required_content_models = count(array_diff($content_models, $object->models)) == 0; } - return $accessible_namespace && $has_required_permissions && $has_required_content_models; + + return $accessible_namespace && + $has_required_permissions($permissions, $datastream, $object) && + $has_required_content_models; } /** @@ -1187,7 +1218,7 @@ function islandora_islandora_object_access($op, $object, $user) { // Straight Drupal permissions, let's allow it. return TRUE; } - elseif ($to_return === FALSE || variable_get('islandora_strict_user_access_enforcement', TRUE) && !$user_access_result) { + elseif ($to_return === FALSE || (variable_get('islandora_strict_user_access_enforcement', TRUE) && !$user_access_result)) { // PID namespace is outside of those allowed. Forbid! return FALSE; } From 594adab668bea5a0c2ff62978bc9edde1c59fa1b Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Tue, 11 Jun 2013 11:52:54 -0300 Subject: [PATCH 08/22] Pass the user along as we should. --- islandora.module | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/islandora.module b/islandora.module index 96836f4c..9b81a388 100644 --- a/islandora.module +++ b/islandora.module @@ -444,13 +444,13 @@ function islandora_user_access($object, array $permissions, $content_models = ar // Check for access. $accessible_namespace = islandora_namespace_accessible($object->id); if ($access_any) { - $has_required_permissions = function ($permissions, $datastream, $object) { + $has_required_permissions = function ($permissions, $datastream, $object, $user) { foreach ($permissions as $p) { if ($datastream !== NULL) { - $check = islandora_datastream_access($p, $datastream); + $check = islandora_datastream_access($p, $datastream, $user); } else { - $check = islandora_object_access($p, $object); + $check = islandora_object_access($p, $object, $user); } if ($check) { @@ -461,13 +461,13 @@ function islandora_user_access($object, array $permissions, $content_models = ar $has_required_content_models = empty($content_models) ? TRUE : count(array_intersect($object->models, $content_models)) > 0; } else { - $has_required_permissions = function ($permissions, $datastream, $object) { + $has_required_permissions = function ($permissions, $datastream, $object, $user) { foreach ($permissions as $p) { if ($datastream !== NULL) { - $check = islandora_datastream_access($p, $datastream); + $check = islandora_datastream_access($p, $datastream, $user); } else { - $check = islandora_object_access($p, $object); + $check = islandora_object_access($p, $object, $user); } if (!$check) { @@ -479,7 +479,7 @@ function islandora_user_access($object, array $permissions, $content_models = ar } return $accessible_namespace && - $has_required_permissions($permissions, $datastream, $object) && + $has_required_permissions($permissions, $datastream, $object, $account) && $has_required_content_models; } @@ -1188,6 +1188,9 @@ function islandora_object_access($op, $object, $user = NULL) { if (!isset($cache[$op][$object->id][$user->uid])) { module_load_include('inc', 'islandora', 'includes/utilities'); + if (!($object instanceof AbstractObject)) { + ddebug_backtrace(); + } $results = islandora_invoke_hook_list('islandora_object_access', $object->models, array( $op, $object, From 2a654d6e837c4c8809d22b99bed731c1dc847e70 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Tue, 11 Jun 2013 13:25:50 -0300 Subject: [PATCH 09/22] Pass the content models... --- islandora.module | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/islandora.module b/islandora.module index 9b81a388..504d9940 100644 --- a/islandora.module +++ b/islandora.module @@ -444,7 +444,7 @@ function islandora_user_access($object, array $permissions, $content_models = ar // Check for access. $accessible_namespace = islandora_namespace_accessible($object->id); if ($access_any) { - $has_required_permissions = function ($permissions, $datastream, $object, $user) { + $has_required_permissions = function ($permissions, $content_models, $datastream, $object, $user) { foreach ($permissions as $p) { if ($datastream !== NULL) { $check = islandora_datastream_access($p, $datastream, $user); @@ -458,10 +458,10 @@ function islandora_user_access($object, array $permissions, $content_models = ar } } }; - $has_required_content_models = empty($content_models) ? TRUE : count(array_intersect($object->models, $content_models)) > 0; + $has_required_content_models = empty($content_models) ? TRUE : (count(array_intersect($object->models, $content_models)) > 0); } else { - $has_required_permissions = function ($permissions, $datastream, $object, $user) { + $has_required_permissions = function ($permissions, $content_models, $datastream, $object, $user) { foreach ($permissions as $p) { if ($datastream !== NULL) { $check = islandora_datastream_access($p, $datastream, $user); @@ -479,7 +479,7 @@ function islandora_user_access($object, array $permissions, $content_models = ar } return $accessible_namespace && - $has_required_permissions($permissions, $datastream, $object, $account) && + $has_required_permissions($permissions, $content_models, $datastream, $object, $account) && $has_required_content_models; } From bb635ccbcdfe27e22b3c5e04cb1fa1dd13193f50 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Tue, 11 Jun 2013 13:43:02 -0300 Subject: [PATCH 10/22] The namespace check is handled via the hooked access callback. --- islandora.module | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/islandora.module b/islandora.module index 504d9940..0a676c76 100644 --- a/islandora.module +++ b/islandora.module @@ -442,7 +442,6 @@ function islandora_user_access($object, array $permissions, $content_models = ar } // Check for access. - $accessible_namespace = islandora_namespace_accessible($object->id); if ($access_any) { $has_required_permissions = function ($permissions, $content_models, $datastream, $object, $user) { foreach ($permissions as $p) { @@ -478,8 +477,7 @@ function islandora_user_access($object, array $permissions, $content_models = ar $has_required_content_models = count(array_diff($content_models, $object->models)) == 0; } - return $accessible_namespace && - $has_required_permissions($permissions, $content_models, $datastream, $object, $account) && + return $has_required_permissions($permissions, $content_models, $datastream, $object, $account) && $has_required_content_models; } From b1479d5af78e909783aca21b9df708798ad980a1 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Tue, 11 Jun 2013 13:50:07 -0300 Subject: [PATCH 11/22] Revert "Pass the content models..." This reverts commit 2a654d6e837c4c8809d22b99bed731c1dc847e70... The content models weren't needed or used... Messed up matching braces in my head. Conflicts: islandora.module --- islandora.module | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/islandora.module b/islandora.module index 0a676c76..3997de63 100644 --- a/islandora.module +++ b/islandora.module @@ -443,7 +443,7 @@ function islandora_user_access($object, array $permissions, $content_models = ar // Check for access. if ($access_any) { - $has_required_permissions = function ($permissions, $content_models, $datastream, $object, $user) { + $has_required_permissions = function ($permissions, $datastream, $object, $user) { foreach ($permissions as $p) { if ($datastream !== NULL) { $check = islandora_datastream_access($p, $datastream, $user); @@ -457,10 +457,10 @@ function islandora_user_access($object, array $permissions, $content_models = ar } } }; - $has_required_content_models = empty($content_models) ? TRUE : (count(array_intersect($object->models, $content_models)) > 0); + $has_required_content_models = empty($content_models) ? TRUE : count(array_intersect($object->models, $content_models)) > 0; } else { - $has_required_permissions = function ($permissions, $content_models, $datastream, $object, $user) { + $has_required_permissions = function ($permissions, $datastream, $object, $user) { foreach ($permissions as $p) { if ($datastream !== NULL) { $check = islandora_datastream_access($p, $datastream, $user); @@ -477,7 +477,7 @@ function islandora_user_access($object, array $permissions, $content_models = ar $has_required_content_models = count(array_diff($content_models, $object->models)) == 0; } - return $has_required_permissions($permissions, $content_models, $datastream, $object, $account) && + return $has_required_permissions($permissions, $datastream, $object, $account) && $has_required_content_models; } From 54bee2e233c76f7939b118a7474bc359509b11a8 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Tue, 11 Jun 2013 13:56:27 -0300 Subject: [PATCH 12/22] Return the final result from the permission anonymous functions. ... Was missing... Also, the handling of an empty array of permissions was happening only implicitly. --- islandora.module | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/islandora.module b/islandora.module index 3997de63..b4f4ca7a 100644 --- a/islandora.module +++ b/islandora.module @@ -413,7 +413,7 @@ function islandora_user_access($object, array $permissions, $content_models = ar return FALSE; } } - if (!$is_repository_accessible || !is_object($object)) { + if (!$is_repository_accessible || !is_object($object) || empty($permissions)) { return FALSE; } // Determine the user account to test against. @@ -456,6 +456,7 @@ function islandora_user_access($object, array $permissions, $content_models = ar return TRUE; } } + return FALSE; }; $has_required_content_models = empty($content_models) ? TRUE : count(array_intersect($object->models, $content_models)) > 0; } @@ -473,6 +474,7 @@ function islandora_user_access($object, array $permissions, $content_models = ar return FALSE; } } + return TRUE; }; $has_required_content_models = count(array_diff($content_models, $object->models)) == 0; } From 023a410c714201484ae2548ae7e6e925d72e16a2 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Tue, 11 Jun 2013 15:20:37 -0300 Subject: [PATCH 13/22] Add some basic tests for the hooked access callback. --- islandora.info | 1 + tests/hooked_access.test | 181 ++++++++++++++++++++++ tests/islandora_hooked_access_test.info | 7 + tests/islandora_hooked_access_test.module | 31 ++++ 4 files changed, 220 insertions(+) create mode 100644 tests/hooked_access.test create mode 100644 tests/islandora_hooked_access_test.info create mode 100644 tests/islandora_hooked_access_test.module diff --git a/islandora.info b/islandora.info index 4f6a5858..ce6ba947 100644 --- a/islandora.info +++ b/islandora.info @@ -15,5 +15,6 @@ files[] = tests/islandora_web_test_case.inc files[] = tests/authtokens.test files[] = tests/hooks.test files[] = tests/ingest.test +files[] = tests/hooked_access.test files[] = tests/islandora_manage_permissions.test php = 5.3 diff --git a/tests/hooked_access.test b/tests/hooked_access.test new file mode 100644 index 00000000..0f7610e0 --- /dev/null +++ b/tests/hooked_access.test @@ -0,0 +1,181 @@ + 'Islandora Hooked Access Callback', + 'description' => 'Ensure that the hooked access callback returns appropriate results.', + 'group' => 'Islandora', + ); + } + + /** + * Creates an admin user and a connection to a fedora repository. + * + * @see IslandoraWebTestCase::setUp() + */ + public function setUp() { + parent::setUp('islandora_hooked_access_test'); + $this->repository = $this->admin->repository; + $this->objects = array( + 'test:testAccessHook', + ); + $this->op = FEDORA_VIEW_OBJECTS; + $this->other_op = FEDORA_INGEST; + $this->denied_op = FEDORA_PURGE; + $this->purgeTestObjects(); + $this->dsid = 'asdf'; + $this->createTestObjects(); + $this->object = $this->repository->getObject('test:testAccessHook'); + } + + /** + * Free any objects/resources created for this test. + * + * @see IslandoraWebTestCase::tearDown() + */ + public function tearDown() { + $this->purgeTestObjects(); + unset($this->repository); + unset($_SESSION['islandora_hooked_access_test']); + parent::tearDown(); + } + + public function createTestObjects() { + foreach ($this->objects as $object_id) { + $object = $this->repository->constructObject($object_id); + $object->label = $object_id; + + $datastream = $object->constructDatastream($this->dsid, 'M'); + $datastream->label = 'fdsa'; + $datastream->mimetype = 'text/plain'; + $datastream->content = 'Some kinda awesome content stuffs...'; + + $object->ingestDatastream($datastream); + $this->repository->ingestObject($object); + } + } + + /** + * Purge any objects created by the test's in this class. + */ + public function purgeTestObjects() { + foreach ($this->objects as $object) { + try { + $object = $this->repository->getObject($object); + $this->repository->purgeObject($object->id); + } + catch (Exception $e) { + // Meh... Either it didn't exist or the purge failed. + } + } + } + + /** + * Deny an object permission check without an object. + */ + public function testDenyBadObject() { + $this->assertFalse(islandora_object_access($this->op, 'this is not an object'), 'Deny bad objects.'); + } + + /** + * Deny a datastream permission check without a datastream. + */ + public function testDenyBadDatastream() { + $this->assertFalse(islandora_datastream_access($this->op, 'this is not a datastream'), 'Deny bad datastreams.'); + } + + /** + * Allow operation on object. + */ + public function testAllowObject() { + $user = $this->drupalCreateUser(array($this->op)); + + $_SESSION['islandora_hooked_access_test'] = array( + $this->op, + $this->object, + $user, + ); + $this->assertTrue(islandora_object_access($this->op, $this->object, $user), 'Allow object access.'); + } + + /** + * Allow operation on datastream. + */ + public function testAllowDatastream() { + $user = $this->drupalCreateUser(array($this->op)); + + $_SESSION['islandora_hooked_access_test'] = array( + $this->op, + $this->object['asdf'], + $user, + ); + $this->assertTrue(islandora_datastream_access($this->op, $this->object['asdf'], $user), 'Allow datastream access.'); + } + + /** + * Deny an operation which was not explicitly allowed on an object. + */ + public function testDenyObjectImplicit() { + $user = $this->drupalCreateUser(array($this->other_op)); + + // This variable forces either a TRUE or FALSE from Islandora's + // implementation of hook_islandora_object_access(). + variable_set('islandora_strict_user_access_enforcement', FALSE); + $_SESSION['islandora_hooked_access_test'] = array( + $this->other_op, + $this->object, + $user, + ); + $this->assertFalse(islandora_object_access($this->op, $this->object, $user), 'Implied denial of object access.'); + } + + /** + * Deny an operation which was not explicitly allowed on a datastream. + */ + public function testDenyDatastreamImplicit() { + $user = $this->drupalCreateUser(array($this->other_op)); + + // This variable forces either a TRUE or FALSE from Islandora's + // implementation of hook_islandora_object_access(). + variable_set('islandora_strict_user_access_enforcement', FALSE); + + $_SESSION['islandora_hooked_access_test'] = array( + $this->other_op, + $this->object['asdf'], + $user, + ); + $this->assertFalse(islandora_datastream_access($this->op, $this->object['asdf'], $user), 'Implied denial of datastream access.'); + } + + /** + * Deny an operation which was not explicitly allowed on an object. + */ + public function testDenyObjectExplicit() { + $this->assertFalse(islandora_object_access($this->denied_op, $this->object), 'Explicit denial of object access.'); + } + + /** + * Deny an operation which was not explicitly allowed on a datastream. + */ + public function testDenyDatastreamExplicit() { + $this->assertFalse(islandora_datastream_access($this->denied_op, $this->object['asdf']), 'Explicit denial of datastream access.'); + } +} diff --git a/tests/islandora_hooked_access_test.info b/tests/islandora_hooked_access_test.info new file mode 100644 index 00000000..6c585daf --- /dev/null +++ b/tests/islandora_hooked_access_test.info @@ -0,0 +1,7 @@ +name = Islandora Hooked Access Callback testing +description = Tests callback hooks. Do not enable. +core = 7.x +package = Testing +hidden = TRUE +files[] = islandora_hooks_test.module +dependencies[] = islandora diff --git a/tests/islandora_hooked_access_test.module b/tests/islandora_hooked_access_test.module new file mode 100644 index 00000000..de417738 --- /dev/null +++ b/tests/islandora_hooked_access_test.module @@ -0,0 +1,31 @@ + Date: Tue, 11 Jun 2013 16:12:40 -0300 Subject: [PATCH 14/22] Add missing function comment. --- tests/hooked_access.test | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/hooked_access.test b/tests/hooked_access.test index 0f7610e0..90a893c7 100644 --- a/tests/hooked_access.test +++ b/tests/hooked_access.test @@ -58,6 +58,9 @@ class IslandoraHookedAccessTestCase extends IslandoraWebTestCase { parent::tearDown(); } + /** + * Create the test object(s) to use during the test. + */ public function createTestObjects() { foreach ($this->objects as $object_id) { $object = $this->repository->constructObject($object_id); From 6384036901074155fed6bb0b39ed7044be6ced74 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Fri, 14 Jun 2013 10:51:39 -0300 Subject: [PATCH 15/22] Squash ddebug_backtrace(). --- islandora.module | 3 --- 1 file changed, 3 deletions(-) diff --git a/islandora.module b/islandora.module index b4f4ca7a..58e1a3a9 100644 --- a/islandora.module +++ b/islandora.module @@ -1188,9 +1188,6 @@ function islandora_object_access($op, $object, $user = NULL) { if (!isset($cache[$op][$object->id][$user->uid])) { module_load_include('inc', 'islandora', 'includes/utilities'); - if (!($object instanceof AbstractObject)) { - ddebug_backtrace(); - } $results = islandora_invoke_hook_list('islandora_object_access', $object->models, array( $op, $object, From daa15012924d71d0efbdb41b8a3e5cacc3a65a2a Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Wed, 19 Jun 2013 08:02:11 -0300 Subject: [PATCH 16/22] Change initial variable name. --- islandora.module | 53 ++++++++++++++++++++++++------------------------ 1 file changed, 27 insertions(+), 26 deletions(-) diff --git a/islandora.module b/islandora.module index 58e1a3a9..88664bd1 100644 --- a/islandora.module +++ b/islandora.module @@ -381,7 +381,7 @@ function islandora_forms($form_id) { * * @global $user * - * @param mixed $object + * @param mixed $object_or_datastream * The AbstractObject or AbstractDatastream to test for accessibility, if NULL * is given the object is assumed to not exist or be inaccessible. * @param array $permissions @@ -392,7 +392,7 @@ function islandora_forms($form_id) { * (optional) TRUE to grant access if any single requirement is met from both * the permissions and content models parameters. FALSE if all requirements * must be met from both the permissions and content model parameters. - * @param object $account + * @param object $user * (optional) The account to check, if not given check the GET parameters for * a token to restore the user. If no GET parameter is present use currently * logged in user. @@ -401,9 +401,10 @@ function islandora_forms($form_id) { * TRUE if the user is allowed to access this object/datastream, FALSE * otherwise. */ -function islandora_user_access($object, array $permissions, $content_models = array(), $access_any = TRUE, $account = NULL) { +function islandora_user_access($object_or_datastream, array $permissions, $content_models = array(), $access_any = TRUE, $user = NULL) { module_load_include('inc', 'islandora', 'includes/utilities'); $is_repository_accessible = &drupal_static(__FUNCTION__); + // If the repository is inaccessible then access always fails. if (!isset($is_repository_accessible)) { $is_repository_accessible = islandora_describe_repository(); @@ -413,37 +414,39 @@ function islandora_user_access($object, array $permissions, $content_models = ar return FALSE; } } - if (!$is_repository_accessible || !is_object($object) || empty($permissions)) { + if (!$is_repository_accessible || !is_object($object_or_datastream) || empty($permissions)) { return FALSE; } + + // Determine what has been passed as $object. + if (is_subclass_of($object_or_datastream, 'AbstractObject')) { + $object = $object_or_datastream; + $datastream = NULL; + } + elseif (is_subclass_of($object_or_datastream, 'AbstractDatastream')) { + $datastream = $object_or_datastream; + $object = $datastream->parent; + } + // Determine the user account to test against. - if (!isset($account)) { + if (!isset($user)) { $token = filter_input(INPUT_GET, 'token', FILTER_SANITIZE_STRING); if ($token) { module_load_include('inc', 'islandora', 'includes/authtokens'); - $user = islandora_validate_object_token($object->id, $datastream->id, $token); + $token_user = islandora_validate_object_token($object->id, $datastream->id, $token); if ($user) { - $account = user_load($user->uid); + $user = user_load($token_user->uid); } } else { global $user; - $account = $user; } } - // Determine what has been passed as $object. - if (is_subclass_of($object, 'AbstractObject')) { - $datastream = NULL; - // $object stays $object... - } - elseif (is_subclass_of($object, 'AbstractDatastream')) { - $datastream = $object; - $object = $datastream->parent; - } // Check for access. if ($access_any) { - $has_required_permissions = function ($permissions, $datastream, $object, $user) { + $has_required_content_models = empty($content_models) ? TRUE : count(array_intersect($object->models, $content_models)) > 0; + if ($has_required_content_models) { foreach ($permissions as $p) { if ($datastream !== NULL) { $check = islandora_datastream_access($p, $datastream, $user); @@ -457,11 +460,11 @@ function islandora_user_access($object, array $permissions, $content_models = ar } } return FALSE; - }; - $has_required_content_models = empty($content_models) ? TRUE : count(array_intersect($object->models, $content_models)) > 0; + } } else { - $has_required_permissions = function ($permissions, $datastream, $object, $user) { + $has_required_content_models = count(array_diff($content_models, $object->models)) == 0; + if ($has_required_content_models) { foreach ($permissions as $p) { if ($datastream !== NULL) { $check = islandora_datastream_access($p, $datastream, $user); @@ -474,13 +477,11 @@ function islandora_user_access($object, array $permissions, $content_models = ar return FALSE; } } + // Should already have failed if there are no $permissions. return TRUE; - }; - $has_required_content_models = count(array_diff($content_models, $object->models)) == 0; + } } - - return $has_required_permissions($permissions, $datastream, $object, $account) && - $has_required_content_models; + return FALSE; } /** From 344a4058947d1cfc0ef3ec62168e8734b891ead9 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Wed, 19 Jun 2013 08:05:06 -0300 Subject: [PATCH 17/22] Be somewhat more descriptive in API doc. --- islandora.api.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/islandora.api.php b/islandora.api.php index aa6df5b6..129787f4 100644 --- a/islandora.api.php +++ b/islandora.api.php @@ -445,7 +445,7 @@ function hook_CMODEL_PID_islandora_ingest_steps(array $form_state) { } /** - * Hookable access hook. + * Object-level access callback hook. * * @param string $op * A string define an operation to check. Should be defined via @@ -483,7 +483,7 @@ function hook_CMODEL_PID_islandora_object_access($op, $object, $user) { } /** - * Hookable access hook. + * Datastream-level access callback hook. * * @param string $op * A string define an operation to check. Should be defined via From 6f9b7b877ac75d0fb8592e3d31ba977e9b6794bc Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Wed, 19 Jun 2013 08:09:54 -0300 Subject: [PATCH 18/22] Get rid of non-strict policy enforcement. --- includes/admin.form.inc | 7 ------- islandora.module | 21 +++------------------ 2 files changed, 3 insertions(+), 25 deletions(-) diff --git a/includes/admin.form.inc b/includes/admin.form.inc index f4db785b..a1f22652 100644 --- a/includes/admin.form.inc +++ b/includes/admin.form.inc @@ -108,13 +108,6 @@ function islandora_repository_admin(array $form, array &$form_state) { '#required' => TRUE, ); - $form['islandora_tabs']['islandora_general']['islandora_strict_user_access_enforcement'] = array( - '#type' => 'checkbox', - '#title' => t('Strict User Access Enforcement'), - '#description' => t('Restrict permissions to the result of user_access(); other modules will be able to deny things, but other modules will not be able to allow operations not allowed via Drupal permissions.'), - '#default_value' => variable_get('islandora_strict_user_access_enforcement', TRUE), - ); - $form['islandora_tabs']['islandora_namespace'] = array( '#type' => 'fieldset', '#title' => t('Namespaces'), diff --git a/islandora.module b/islandora.module index 88664bd1..58a23a10 100644 --- a/islandora.module +++ b/islandora.module @@ -1205,28 +1205,13 @@ function islandora_object_access($op, $object, $user = NULL) { /** * Implements hook_islandora_object_access(). * - * Denies according to PID namespace restrictions, passes according to - * user_access(), and makes no indication if namespace restrictions passed but - * user_access() returned a fail, to allow other modules to allow an operation. + * Denies according to PID namespace restrictions, then passes or denies + * according to core Drupal permissions according to user_access(). */ function islandora_islandora_object_access($op, $object, $user) { module_load_include('inc', 'islandora', 'includes/utilities'); - $to_return = islandora_namespace_accessible($object->id); - $user_access_result = user_access($op, $user); - - if ($to_return && $user_access_result) { - // Straight Drupal permissions, let's allow it. - return TRUE; - } - elseif ($to_return === FALSE || (variable_get('islandora_strict_user_access_enforcement', TRUE) && !$user_access_result)) { - // PID namespace is outside of those allowed. Forbid! - return FALSE; - } - else { - // Neither allowing of forbidding, to allow other modules to override. - return NULL; - } + return islandora_namespace_accessible($object->id) && user_access($op, $user); } /** From 7aa11510018ed50ff8b35ddd0ac1d42f49bad2d4 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Wed, 19 Jun 2013 08:27:48 -0300 Subject: [PATCH 19/22] Deprecate old and use new functions. --- includes/utilities.inc | 6 ++++++ islandora.module | 30 ++++++++++++++++-------------- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/includes/utilities.inc b/includes/utilities.inc index 4d69155d..cc4845bf 100644 --- a/includes/utilities.inc +++ b/includes/utilities.inc @@ -673,6 +673,9 @@ function islandora_get_comp_ds_mappings($pid) { * TRUE if the account has all the given permissions, FALSE otherwise. */ function islandora_user_access_all(array $perms, $account = NULL) { + $message = islandora_deprecated('7.x-1.2', 'Roll your own code.'); + trigger_error($message, E_USER_DEPRECATED); + foreach ($perms as $perm) { if (!user_access($perm, $account)) { return FALSE; @@ -694,6 +697,9 @@ function islandora_user_access_all(array $perms, $account = NULL) { * otherwise. */ function islandora_user_access_any(array $perms, $account = NULL) { + $message = islandora_deprecated('7.x-1.2', 'Roll your own code.'); + trigger_error($message, E_USER_DEPRECATED); + foreach ($perms as $perm) { if (user_access($perm, $account)) { return TRUE; diff --git a/islandora.module b/islandora.module index 58a23a10..3c7d9419 100644 --- a/islandora.module +++ b/islandora.module @@ -190,8 +190,8 @@ function islandora_menu() { 'page arguments' => array(4, FALSE), 'type' => MENU_CALLBACK, 'file' => 'includes/datastream.inc', - 'access callback' => 'islandora_object_datastream_access_callback', - 'access arguments' => array(FEDORA_VIEW_OBJECTS, 2, 4), + 'access callback' => 'islandora_datastream_access', + 'access arguments' => array(FEDORA_VIEW_OBJECTS, 4), 'load arguments' => array(2), ); // This menu item uses token authentication in islandora_tokened_object. @@ -208,8 +208,8 @@ function islandora_menu() { 'page arguments' => array(4), 'type' => MENU_CALLBACK, 'file' => 'includes/datastream.inc', - 'access callback' => 'islandora_object_datastream_access_callback', - 'access arguments' => array(FEDORA_VIEW_OBJECTS, 2, 4), + 'access callback' => 'islandora_datastream_access', + 'access arguments' => array(FEDORA_VIEW_OBJECTS, 4), 'load arguments' => array(2), ); $items['islandora/object/%islandora_object/datastream/%islandora_datastream/edit'] = array( @@ -218,8 +218,8 @@ function islandora_menu() { 'page arguments' => array(4), 'type' => MENU_CALLBACK, 'file' => 'includes/datastream.inc', - 'access callback' => 'islandora_object_datastream_access_callback', - 'access arguments' => array(FEDORA_METADATA_EDIT, 2, 4), + 'access callback' => 'islandora_datastream_access', + 'access arguments' => array(FEDORA_METADATA_EDIT, 4), 'load arguments' => array(2), ); $items['islandora/object/%islandora_object/datastream/%islandora_datastream/delete'] = array( @@ -228,8 +228,8 @@ function islandora_menu() { 'page arguments' => array('islandora_delete_datastream_form', 4), 'file' => 'includes/delete_datastream.form.inc', 'type' => MENU_CALLBACK, - 'access callback' => 'islandora_object_datastream_access_callback', - 'access arguments' => array(FEDORA_PURGE, 2, 4), + 'access callback' => 'islandora_datastream_access', + 'access arguments' => array(FEDORA_PURGE, 4), 'load arguments' => array(2), ); $items['islandora/object/%islandora_object/print'] = array( @@ -237,7 +237,7 @@ function islandora_menu() { 'page callback' => 'islandora_print_object', 'page arguments' => array(2), 'type' => MENU_CALLBACK, - 'access callback' => 'islandora_object_access_callback', + 'access callback' => 'islandora_object_access', 'access arguments' => array(FEDORA_VIEW_OBJECTS, 2), 'load arguments' => array(2), ); @@ -245,7 +245,7 @@ function islandora_menu() { 'page callback' => 'islandora_download_clip', 'page arguments' => array(2), 'type' => MENU_CALLBACK, - 'access callback' => 'islandora_object_access_callback', + 'access callback' => 'islandora_object_access', 'access arguments' => array(FEDORA_VIEW_OBJECTS, 2), 'load arguments' => array(2), ); @@ -531,6 +531,10 @@ function islandora_object_access_callback($perm, $object = NULL) { * TRUE if the user is allowed to access this object, FALSE otherwise. */ function islandora_object_datastream_access_callback($perm, $object = NULL, $datastream = NULL, $account = NULL) { + module_load_include('inc', 'islandora', 'includes/utilities'); + $message = islandora_deprecated('7.x-1.2', 'Use islandora_datastream_access().'); + trigger_error($message, E_USER_DEPRECATED); + return islandora_datastream_access($perm, $datastream, $account); } @@ -539,10 +543,8 @@ function islandora_object_datastream_access_callback($perm, $object = NULL, $dat * * This function will validate and use a token if present in the GET parameters. * - * Checks for object existance, accessiblitly, namespace permissions, + * Checks for object existance, accessibility, namespace permissions, * and user permissions - * - * @see islandora_object_datastream_tokened_access_callback() */ function islandora_object_datastream_tokened_access_callback($perm, $object = NULL, $datastream = NULL) { module_load_include('inc', 'islandora', 'includes/utilities'); @@ -557,7 +559,7 @@ function islandora_object_datastream_tokened_access_callback($perm, $object = NU } } - return islandora_object_datastream_access_callback($perm, $object, $datastream, $token_account); + return islandora_datastream_access($perm, $datastream, $token_account); } /** From fa3449813077f7734419a5c6049e71bb81a1d7a3 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Wed, 19 Jun 2013 09:10:07 -0300 Subject: [PATCH 20/22] Themify datastream link creation. --- includes/datastream.inc | 38 +++++++++++------- islandora.module | 16 ++++++++ theme/theme.inc | 88 +++++++++++++++++++++++++++++++++++++++-- 3 files changed, 124 insertions(+), 18 deletions(-) diff --git a/includes/datastream.inc b/includes/datastream.inc index 902d5670..e8aa2fbd 100644 --- a/includes/datastream.inc +++ b/includes/datastream.inc @@ -84,9 +84,12 @@ function islandora_datastream_get_url(AbstractDatastream $datastream, $type = 'd * The datastream to generated the url to. */ function islandora_datastream_get_delete_link(AbstractDatastream $datastream) { - $datastreams = module_invoke_all('islandora_undeletable_datastreams', $datastream->parent->models); - $can_delete = !in_array($datastream->id, $datastreams) && islandora_datastream_access(FEDORA_PURGE, $datastream); - return $can_delete ? l(t('delete'), "islandora/object/{$datastream->parent->id}/datastream/{$datastream->id}/delete") : ''; + $message = islandora_deprecated('7.x-1.2', 'Use the "islandora_datastream_delete_link" theme implementation.'); + trigger_error($message, E_USER_DEPRECATED); + + return theme('islandora_datastream_delete_link', array( + 'datastream' => $datastream, + )); } /** @@ -96,9 +99,12 @@ function islandora_datastream_get_delete_link(AbstractDatastream $datastream) { * The datastream to generated the url to. */ function islandora_datastream_edit_get_link(AbstractDatastream $datastream) { - $edit_registry = module_invoke_all('islandora_edit_datastream_registry', $datastream->parent, $datastream); - $can_edit = count($edit_registry) > 0 && islandora_datastream_access(FEDORA_METADATA_EDIT, $datastream); - return $can_edit ? l(t('edit'), "islandora/object/{$datastream->parent->id}/datastream/{$datastream->id}/edit") : ''; + $message = islandora_deprecated('7.x-1.2', 'Use the "islandora_datastream_edit_link" theme implementation.'); + trigger_error($message, E_USER_DEPRECATED); + + return theme('islandora_datastream_edit_link', array( + 'datastream' => $datastream, + )); } /** @@ -156,10 +162,12 @@ function islandora_edit_datastream_registry_render(array $edit_registry) { * user is not allowed to see the given datastream. */ function islandora_datastream_get_download_link(AbstractDatastream $datastream) { - $label = t('download'); - return islandora_datastream_access(FEDORA_VIEW_OBJECTS, $datastream) ? - l($label, islandora_datastream_get_url($datastream, 'download')) : - ''; + $message = islandora_deprecated('7.x-1.2', 'Use the "islandora_datastream_download_link" theme implementation.'); + trigger_error($message, E_USER_DEPRECATED); + + return theme('islandora_datastream_download_link', array( + 'datastream' => $datastream, + )); } /** @@ -173,8 +181,10 @@ function islandora_datastream_get_download_link(AbstractDatastream $datastream) * datastream ID if the user is not allowed to see the given datastream. */ function islandora_datastream_get_view_link(AbstractDatastream $datastream) { - $label = check_plain($datastream->id); - return islandora_datastream_access(FEDORA_VIEW_OBJECTS, $datastream) ? - l($label, islandora_datastream_get_url($datastream, 'view')) : - $label; + $message = islandora_deprecated('7.x-1.2', 'Use the "islandora_datastream_view_link" theme implementation.'); + trigger_error($message, E_USER_DEPRECATED); + + return theme('islandora_datastream_view_link', array( + 'datastream' => $datastream, + )); } diff --git a/islandora.module b/islandora.module index 3c7d9419..baa1460c 100644 --- a/islandora.module +++ b/islandora.module @@ -322,6 +322,22 @@ function islandora_theme() { 'template' => 'theme/islandora-objects-list', 'variables' => array('objects' => NULL), ), + 'islandora_datastream_edit_link' => array( + 'file' => 'theme/theme.inc', + 'variables' => array('datastream' => NULL), + ), + 'islandora_datastream_delete_link' => array( + 'file' => 'theme/theme.inc', + 'variables' => array('datastream' => NULL), + ), + 'islandora_datastream_view_link' => array( + 'file' => 'theme/theme.inc', + 'variables' => array('datastream' => NULL), + ), + 'islandora_datastream_download_link' => array( + 'file' => 'theme/theme.inc', + 'variables' => array('datastream' => NULL), + ), ); } diff --git a/theme/theme.inc b/theme/theme.inc index ce6f2659..cf048745 100644 --- a/theme/theme.inc +++ b/theme/theme.inc @@ -29,7 +29,9 @@ function islandora_preprocess_islandora_default_edit(array &$variables) { $rows[] = array( array( 'class' => 'datastream-id', - 'data' => islandora_datastream_get_view_link($ds), + 'data' => theme('islandora_datastream_view_link', array( + 'datastream' => $ds, + )), ), array( 'class' => 'datastream-label', @@ -49,15 +51,21 @@ function islandora_preprocess_islandora_default_edit(array &$variables) { ), array( 'class' => 'datastream-download', - 'data' => islandora_datastream_get_download_link($ds), + 'data' => theme('islandora_datastream_download_link', array( + 'datastream' => $ds, + )), ), array( 'class' => 'datstream-edit', - 'data' => islandora_datastream_edit_get_link($ds), + 'data' => theme('islandora_datastream_edit_link', array( + 'datastream' => $ds, + )), ), array( 'class' => 'datastream-delete', - 'data' => islandora_datastream_get_delete_link($ds), + 'data' => theme('islandora_datastream_delete_link', array( + 'datastream' => $ds, + )), ), ); } @@ -224,3 +232,75 @@ function islandora_preprocess_islandora_objects(array &$variables) { $module_path = drupal_get_path('module', 'islandora'); drupal_add_css("$module_path/css/islandora.objects.css"); } + +/** + * Renders a link to allow downloading of a datatream. + * + * @param array $vars + * An array containing: + * - datastream: An AbstractDatastream for which to generate a download link. + */ +function theme_islandora_datastream_download_link(array $vars) { + $datastream = $vars['datastream']; + module_load_include('inc', 'islandora', 'includes/utilities'); + + $label = t('download'); + return islandora_datastream_access(FEDORA_VIEW_OBJECTS, $datastream) ? + l($label, islandora_datastream_get_url($datastream, 'download')) : + ''; +} + +/** + * Renders a link to allow viewing of a datatream. + * + * @param array $vars + * An array containing: + * - datastream: An AbstractDatastream for which to generate a view link. + */ +function theme_islandora_datastream_view_link(array $vars) { + $datastream = $vars['datastream']; + module_load_include('inc', 'islandora', 'includes/utilities'); + + $label = check_plain($datastream->label); + return islandora_datastream_access(FEDORA_VIEW_OBJECTS, $datastream) ? + l($label, islandora_datastream_get_url($datastream, 'view')) : + $label; +} + +/** + * Renders a link to allow deleting of a datatream. + * + * @param array $vars + * An array containing: + * - datastream: An AbstractDatastream for which to generate a delete link. + */ +function theme_islandora_datastream_delete_link(array $vars) { + $datastream = $vars['datastream']; + + $datastreams = module_invoke_all('islandora_undeletable_datastreams', $datastream->parent->models); + + $can_delete = !in_array($datastream->id, $datastreams) && islandora_datastream_access(FEDORA_PURGE, $datastream); + + return $can_delete ? + l(t('delete'), "islandora/object/{$datastream->parent->id}/datastream/{$datastream->id}/delete") : + ''; +} + +/** + * Renders a link to allow editing of a datatream. + * + * @param array $vars + * An array containing: + * - datastream: An AbstractDatastream for which to generate a edit link. + */ +function theme_islandora_datastream_edit_link(array $vars) { + $datastream = $vars['datastream']; + + $edit_registry = module_invoke_all('islandora_edit_datastream_registry', $datastream->parent, $datastream); + + $can_edit = count($edit_registry) > 0 && islandora_datastream_access(FEDORA_METADATA_EDIT, $datastream); + + return $can_edit ? + l(t('edit'), "islandora/object/{$datastream->parent->id}/datastream/{$datastream->id}/edit") : + ''; +} From 6d677c4319c4ca1a5b500db7af5e0452166d8ed0 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Wed, 19 Jun 2013 09:11:54 -0300 Subject: [PATCH 21/22] Get rid of implicit denial tests. Variable no longer exists, so these are no longer useful. --- tests/hooked_access.test | 35 ----------------------------------- 1 file changed, 35 deletions(-) diff --git a/tests/hooked_access.test b/tests/hooked_access.test index 90a893c7..0039460e 100644 --- a/tests/hooked_access.test +++ b/tests/hooked_access.test @@ -133,41 +133,6 @@ class IslandoraHookedAccessTestCase extends IslandoraWebTestCase { $this->assertTrue(islandora_datastream_access($this->op, $this->object['asdf'], $user), 'Allow datastream access.'); } - /** - * Deny an operation which was not explicitly allowed on an object. - */ - public function testDenyObjectImplicit() { - $user = $this->drupalCreateUser(array($this->other_op)); - - // This variable forces either a TRUE or FALSE from Islandora's - // implementation of hook_islandora_object_access(). - variable_set('islandora_strict_user_access_enforcement', FALSE); - $_SESSION['islandora_hooked_access_test'] = array( - $this->other_op, - $this->object, - $user, - ); - $this->assertFalse(islandora_object_access($this->op, $this->object, $user), 'Implied denial of object access.'); - } - - /** - * Deny an operation which was not explicitly allowed on a datastream. - */ - public function testDenyDatastreamImplicit() { - $user = $this->drupalCreateUser(array($this->other_op)); - - // This variable forces either a TRUE or FALSE from Islandora's - // implementation of hook_islandora_object_access(). - variable_set('islandora_strict_user_access_enforcement', FALSE); - - $_SESSION['islandora_hooked_access_test'] = array( - $this->other_op, - $this->object['asdf'], - $user, - ); - $this->assertFalse(islandora_datastream_access($this->op, $this->object['asdf'], $user), 'Implied denial of datastream access.'); - } - /** * Deny an operation which was not explicitly allowed on an object. */ From 09e9785ecfd4ad1b305f5a244101c9925d676cc5 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Wed, 19 Jun 2013 09:45:42 -0300 Subject: [PATCH 22/22] Add filter_xss() to trigger_error() calls... Hurray coder! :P --- includes/datastream.inc | 8 ++++---- includes/utilities.inc | 8 ++++---- islandora.module | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/includes/datastream.inc b/includes/datastream.inc index e8aa2fbd..dc01bdfe 100644 --- a/includes/datastream.inc +++ b/includes/datastream.inc @@ -85,7 +85,7 @@ function islandora_datastream_get_url(AbstractDatastream $datastream, $type = 'd */ function islandora_datastream_get_delete_link(AbstractDatastream $datastream) { $message = islandora_deprecated('7.x-1.2', 'Use the "islandora_datastream_delete_link" theme implementation.'); - trigger_error($message, E_USER_DEPRECATED); + trigger_error(filter_xss($message), E_USER_DEPRECATED); return theme('islandora_datastream_delete_link', array( 'datastream' => $datastream, @@ -100,7 +100,7 @@ function islandora_datastream_get_delete_link(AbstractDatastream $datastream) { */ function islandora_datastream_edit_get_link(AbstractDatastream $datastream) { $message = islandora_deprecated('7.x-1.2', 'Use the "islandora_datastream_edit_link" theme implementation.'); - trigger_error($message, E_USER_DEPRECATED); + trigger_error(filter_xss($message), E_USER_DEPRECATED); return theme('islandora_datastream_edit_link', array( 'datastream' => $datastream, @@ -163,7 +163,7 @@ function islandora_edit_datastream_registry_render(array $edit_registry) { */ function islandora_datastream_get_download_link(AbstractDatastream $datastream) { $message = islandora_deprecated('7.x-1.2', 'Use the "islandora_datastream_download_link" theme implementation.'); - trigger_error($message, E_USER_DEPRECATED); + trigger_error(filter_xss($message), E_USER_DEPRECATED); return theme('islandora_datastream_download_link', array( 'datastream' => $datastream, @@ -182,7 +182,7 @@ function islandora_datastream_get_download_link(AbstractDatastream $datastream) */ function islandora_datastream_get_view_link(AbstractDatastream $datastream) { $message = islandora_deprecated('7.x-1.2', 'Use the "islandora_datastream_view_link" theme implementation.'); - trigger_error($message, E_USER_DEPRECATED); + trigger_error(filter_xss($message), E_USER_DEPRECATED); return theme('islandora_datastream_view_link', array( 'datastream' => $datastream, diff --git a/includes/utilities.inc b/includes/utilities.inc index cc4845bf..38e7666f 100644 --- a/includes/utilities.inc +++ b/includes/utilities.inc @@ -673,8 +673,8 @@ function islandora_get_comp_ds_mappings($pid) { * TRUE if the account has all the given permissions, FALSE otherwise. */ function islandora_user_access_all(array $perms, $account = NULL) { - $message = islandora_deprecated('7.x-1.2', 'Roll your own code.'); - trigger_error($message, E_USER_DEPRECATED); + $message = islandora_deprecated('7.x-1.2', 'Roll your own code or use islandora_user_access().'); + trigger_error(filter_xss($message), E_USER_DEPRECATED); foreach ($perms as $perm) { if (!user_access($perm, $account)) { @@ -697,8 +697,8 @@ function islandora_user_access_all(array $perms, $account = NULL) { * otherwise. */ function islandora_user_access_any(array $perms, $account = NULL) { - $message = islandora_deprecated('7.x-1.2', 'Roll your own code.'); - trigger_error($message, E_USER_DEPRECATED); + $message = islandora_deprecated('7.x-1.2', 'Roll your own code or use islandora_user_access().'); + trigger_error(filter_xss($message), E_USER_DEPRECATED); foreach ($perms as $perm) { if (user_access($perm, $account)) { diff --git a/islandora.module b/islandora.module index baa1460c..ba6d972e 100644 --- a/islandora.module +++ b/islandora.module @@ -549,7 +549,7 @@ function islandora_object_access_callback($perm, $object = NULL) { function islandora_object_datastream_access_callback($perm, $object = NULL, $datastream = NULL, $account = NULL) { module_load_include('inc', 'islandora', 'includes/utilities'); $message = islandora_deprecated('7.x-1.2', 'Use islandora_datastream_access().'); - trigger_error($message, E_USER_DEPRECATED); + trigger_error(filter_xss($message), E_USER_DEPRECATED); return islandora_datastream_access($perm, $datastream, $account); }