If the Webform Services webform/{UUID}/submissions endpoint includes some access control (e.g. session authentication) then there should be some user_access() checking of permissions before returning webform submissions. This does not happen currently.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joe-b’s picture

Here's a patch that adds this permissions check.

Nodz’s picture

I noticed the patch doesn't work as expected in all cases.

    // Only access submissions for which the user has permissions.
    if (user_access('access own webform results') && !user_access('access all webform results')) {
      global $user;
      $filters['u.uid'] = $user->uid;
    }

This works if you have access own webform results but if you don't have that right then no filtering takes place at all and you can see all submissions just like if you had access all webform results. This is probably not desired behavior.

Also it seems the webform permissions for submissions might be more appropriate:
'access own webform submissions' though it doesn't have a counterpart for access all submissions.

I've solved the problem with the following checks:

    // First check if we have access to all
    if (user_access('edit all webform submissions')) {
      // don't filter anything extra
    }
    // Then if we have access to our own
    else if (user_access('access own webform submissions')) {
      $filters['u.uid'] = $user->uid;
    }
    // We don't have any access so return error
    else {
      // Return a 401 Unauthorized.
      return services_error(t('User @userid does not have access to submissions', array('@userid' => $user->uid)), 401);
    }

Note: There's no access all webform submissiosn permission so i just used edit all, if you can edit you can basically access. Also when logged in as admin (user 1) you by default will see all.

I now wonder if the other resources (put and post) also lack similar access checks.

dureaghin’s picture

You forgot to add global $user;

Should be:

/**
 * Retrieve all submissions for a webform.
 */
function webform_service_submission_index($uuid, $page, $page_size, $parameters) {
  if ($webform = webform_service_resource_load($uuid)) {

    // Establish the index by setting the default nid filter, and then using any
    // additional parameters as filters when getting the submissions.
    $index = array();
    module_load_include('inc', 'webform', 'includes/webform.submissions');
    $parameters['nid'] = $webform->nid;
    // Set appropriate filters to get submissions.
    $filters = array('nid' => $webform->nid);
    // First check if we have access to all
    if (user_access('edit all webform submissions')) {
      // don't filter anything extra
    }
    // Then if we have access to our own
    else if (user_access('access own webform submissions')) {
      global $user;
      $filters['u.uid'] = $user->uid;
    }
    // We don't have any access so return error
    else {
      // Return a 401 Unauthorized.
      return services_error(t('User @userid does not have access to submissions', array('@userid' => $user->uid)), 401);
    }
    $submissions = webform_get_submissions($filters);

    // Iterate through each submission and get the submission.
    foreach ($submissions as $submission) {
      $index[] = webform_service_get_submission($webform, $submission);
    }
    return $index;
  }
  else {
    // Return a 404.
    return services_error(t('@uuid could not be found', array('@uuid' => $uuid)), 404);
  }
}

Thanks!

tyler.frankenstein’s picture

Status: Active » Needs work

Please re-roll the patch and I can look to get this committed, thank you.

dureaghin’s picture

I put global $user; to a wrong spot, I think the right place is the first line. I tested and now it works.

 function webform_service_submission_index($uuid, $page, $page_size, $parameters) {
  global $user;
tyler.frankenstein’s picture

We can't ignore $parameters here, many depend on the ability to use that. The use of $filters must be replaced with $parameters instead, and each filter should not overwrite any pre-existing $parameters coming in. Also, please post a patch instead of all the code in the comment.

dureaghin’s picture

tyler.frankenstein’s picture

Thank you, but the incoming $parameters is still being ignored, the filter values need to be added to parameters (and not overwrite any pre-existing incoming values). Essentially, use $parameters instead of $filters.

dureaghin’s picture

Thank you Tyler. Please test it.

tyler.frankenstein’s picture

Status: Needs work » Needs review
FileSize
1.85 KB

$parameters was still being overwritten, so I adjusted the patch a bit to not overwrite it, and to use the correct access permission check.

tyler.frankenstein’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.