Almost forgot about this issue.

From #2542790: Implement API for delivery reports for SMS Gateways #53:

One thing I'm concerned about, that does not have to necessarily be covered in this patch, is adding access control to the delivery report endpoint. Something along the lines of adding a secret token like Drupal cron does...

We dont want to have to deal with garbage/spam in our endpoints.

CommentFileSizeAuthor
#26 gateway-plugin-route-custom-2798171-26-bc839e5.patch25.74 KBdpi
#21 gateway-plugin-route-custom-2798171-21-552d3d7.patch24.39 KBdpi
#16 gateway-plugin-route-custom-2798171-16-bad41a7.patch24.05 KBdpi
#11 gateway-plugin-route-custom-2798171-11-a13e66f.patch23.99 KBdpi
#6 gateway-plugin-route-custom-2798171-6-51c97a5.patch21.6 KBdpi

Comments

dpi created an issue. See original summary.

almaudoh’s picture

almaudoh commented

Is this an 8.x-1.0 target?

dpi’s picture

dpi
Location Perth, Australia
commented
Issue tags: +release-8.x-1.1

I dont think it needs to be.

dpi’s picture

dpi
Location Perth, Australia
commented
Issue summary: View changes

The URL should be something completely random like cron URL?

And should we only respond to POST requests?

dpi’s picture

dpi
Location Perth, Australia
commented
Issue tags: +beta target

Add beta target, in case its an easy fix.

dpi’s picture

dpi
Location Perth, Australia
commented
Status: Active » Needs review
StatusFileSize
new gateway-plugin-route-custom-2798171-6-51c97a5.patch21.6 KB

https://github.com/dpi/smsframework/pull/57

Added configurable route for pushed delivery reports.

Generates a random URL on creation of a gateway instance.

Status: Needs review » Needs work

The last submitted patch, 6: gateway-plugin-route-custom-2798171-6-51c97a5.patch, failed testing.

The last submitted patch, 6: gateway-plugin-route-custom-2798171-6-51c97a5.patch, failed testing.

The last submitted patch, 6: gateway-plugin-route-custom-2798171-6-51c97a5.patch, failed testing.

The last submitted patch, 6: gateway-plugin-route-custom-2798171-6-51c97a5.patch, failed testing.

dpi’s picture

dpi
Location Perth, Australia
commented
Status: Needs work » Needs review
StatusFileSize
new gateway-plugin-route-custom-2798171-11-a13e66f.patch23.99 KB

Status: Needs review » Needs work

The last submitted patch, 11: gateway-plugin-route-custom-2798171-11-a13e66f.patch, failed testing.

The last submitted patch, 11: gateway-plugin-route-custom-2798171-11-a13e66f.patch, failed testing.

The last submitted patch, 11: gateway-plugin-route-custom-2798171-11-a13e66f.patch, failed testing.

The last submitted patch, 11: gateway-plugin-route-custom-2798171-11-a13e66f.patch, failed testing.

dpi’s picture

dpi
Location Perth, Australia
commented
Status: Needs work » Needs review
StatusFileSize
new gateway-plugin-route-custom-2798171-16-bad41a7.patch24.05 KB

Status: Needs review » Needs work

The last submitted patch, 16: gateway-plugin-route-custom-2798171-16-bad41a7.patch, failed testing.

The last submitted patch, 16: gateway-plugin-route-custom-2798171-16-bad41a7.patch, failed testing.

The last submitted patch, 16: gateway-plugin-route-custom-2798171-16-bad41a7.patch, failed testing.

The last submitted patch, 16: gateway-plugin-route-custom-2798171-16-bad41a7.patch, failed testing.

dpi’s picture

dpi
Location Perth, Australia
commented
Status: Needs work » Needs review
StatusFileSize
new gateway-plugin-route-custom-2798171-21-552d3d7.patch24.39 KB

Status: Needs review » Needs work

The last submitted patch, 21: gateway-plugin-route-custom-2798171-21-552d3d7.patch, failed testing.

The last submitted patch, 21: gateway-plugin-route-custom-2798171-21-552d3d7.patch, failed testing.

The last submitted patch, 21: gateway-plugin-route-custom-2798171-21-552d3d7.patch, failed testing.

The last submitted patch, 21: gateway-plugin-route-custom-2798171-21-552d3d7.patch, failed testing.

dpi’s picture

dpi
Location Perth, Australia
commented
Status: Needs work » Needs review
StatusFileSize
new gateway-plugin-route-custom-2798171-26-bc839e5.patch25.74 KB
almaudoh’s picture

almaudoh commented

Made some comment on the PR. Repeating it here.

This is a great change overall. Some few nitpicks below. We could additionally secure the transaction by attaching generated tokens to the delivery report path sent to the SMSC for each transaction. Of course this could be an optional feature because some providers require the delivery report url to be preconfigured and not sent in the transaction.

Maybe in another issue.

dpi’s picture

dpi
Location Perth, Australia
commented

Yeh this sounds like it would require a bit of investigation. Out of scope for now.

dpi’s picture

dpi
Location Perth, Australia
commented

One thing to look look into is allowing gateway plugins to alter the pushed delivery report route definition inside of the plugin file. Currently they have to create their own altering-routesubscriber.

  • dpi committed de3ff51 on 8.x-1.x 
    Issue #2798171 by dpi: Improve route access control of delivery report...
dpi’s picture

dpi
Location Perth, Australia
commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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