In #828566: Webform Token support on confirmation message, a URL token was added to provide access control to submission confirmation pages, allowing Webform tokens to be used for anonymous users. This access token should also allow access in general to a particular submission. This would make it easier to allow anonymous users to download PDFs with FillPDF. I will write a patch.

CommentFileSizeAuthor
#11 webform-token_access-2470385-11-D7.patch2.38 KBLiam Morland
#10 webform-token_access-2470385-10-D7.patch2.44 KBLiam Morland
#8 webform-token_access-2470385-8-D7.patch2.44 KBLiam Morland
#1 webform-token_access-2470385-1-D7.patch2.58 KBLiam Morland
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Liam Morland’s picture

Liam Morland
Primary language English
Location Ontario, CA 🇨🇦
CreditAttribution: Liam Morland commented
Status: Active » Needs review
FileSize
webform-token_access-2470385-1-D7.patch2.58 KB
Liam Morland’s picture

Liam Morland
Primary language English
Location Ontario, CA 🇨🇦
CreditAttribution: Liam Morland commented
Parent issue: » #828566: Webform Token support on confirmation message

Status: Needs review » Needs work

The last submitted patch, 1: webform-token_access-2470385-1-D7.patch, failed testing.

Liam Morland’s picture

Liam Morland
Primary language English
Location Ontario, CA 🇨🇦
CreditAttribution: Liam Morland commented
Version: 8.x-4.x-dev » 7.x-4.x-dev
Status: Needs work » Needs review

Wrong core version.

I don't think the test failure is related to the patch.

DanChadwick queued 1: webform-token_access-2470385-1-D7.patch for re-testing.

Liam Morland’s picture

Liam Morland
Primary language English
Location Ontario, CA 🇨🇦
CreditAttribution: Liam Morland commented

Note that only third hunk is specific to this issue. The other hunks just create webform_get_submission_access_token() and put it in use.

DanChadwick’s picture

DanChadwick CreditAttribution: DanChadwick commented

A few quibbles.

  1. When including a type in @param, the type goes before the variable, C style.
  2. When is_object($object) isn't needed when you know $object is either NULL or an object. Just $object is sufficient.
  3. I don't see the upside in drupal_get_query_parameters. I'd be inclined to just access $_GET instead.
  4. I don't see the benefit of the type casting and use of type-specific === operator. This isn't the webform style.
Liam Morland’s picture

Liam Morland
Primary language English
Location Ontario, CA 🇨🇦
CreditAttribution: Liam Morland commented
FileSize
webform-token_access-2470385-8-D7.patch2.44 KB

Thanks. Updated with your changes. I thought drupal_get_query_parameters() was the proper Drupal way of accessing query parameters.

DanChadwick’s picture

DanChadwick CreditAttribution: DanChadwick commented

Doesn't work because of type hint:

Recoverable fatal error: Argument 1 passed to webform_get_submission_access_token() must be an instance of object, instance of stdClass given, called in C:\wamp\www\drupal7\sites\all\modules\custom\webform\webform.module on line 3197 and defined in webform_get_submission_access_token() (line 531 of C:\wamp\www\drupal7\sites\all\modules\custom\webform\webform.module).

Need to get rid of the object type hint.

http://stackoverflow.com/questions/7839059/type-hinting-for-any-object

Also, I'm not sure I understand the need for the sid in the query. The access function already has the submission, so if the token access is being checked, the sid has already been established by the caller (e.g. via the menu path or some other means). Any reason to not remove that test? The token is only good for that one sid anyhow.

Liam Morland’s picture

Liam Morland
Primary language English
Location Ontario, CA 🇨🇦
CreditAttribution: Liam Morland commented
FileSize
webform-token_access-2470385-10-D7.patch2.44 KB

Thanks.

Liam Morland’s picture

Liam Morland
Primary language English
Location Ontario, CA 🇨🇦
CreditAttribution: Liam Morland commented
FileSize
webform-token_access-2470385-11-D7.patch2.38 KB

With sid check removed.

DanChadwick’s picture

DanChadwick CreditAttribution: DanChadwick commented
Status: Needs review » Fixed

While I'm not completely clear about the use case where the $_SESSION isn't sufficient, I have no problem with this patch.

It makes it possible to share an anonymous submission with someone else.

#11 Committed to 7.x-4.x and 8.x.

Liam Morland’s picture

Liam Morland
Primary language English
Location Ontario, CA 🇨🇦
CreditAttribution: Liam Morland commented

Thanks.

Status: Fixed » Closed (fixed)

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