systopia / CiviProxy

A security proxy for CiviCRM
GNU Affero General Public License v3.0
7 stars 18 forks source link

Callbacks #26

Open michaelmcandrew opened 6 years ago

michaelmcandrew commented 6 years ago

This PR adds callback handling to CIviProxy.

Callbacks should be configured in the third party with a URL in the proxy in the following format:

{$proxy_base}/callback.php?source=CALLBACK_KEY&secret=CALLBACK_SECRET

Where:

Other validations you can define include

Some notes on the implementation:

  1. The callback feature is turned on by setting $callbacks_enabled to true. This is a slightly different mechanism to other CiviProxy features that are turned on and off by the presence of a target url. I didn't think that made sense in this case as callbacks define multiple target urls. I appreciate that this isn't great for usability. It might be better to refactor how features are turned on and off but I didn't want to go there.

  2. There is a civiproxy_callback_validate_body() function that could be used to validate the payload. I haven't implemented this but can see that you might want to if you are very security conscious. Not sure what the best way to validate that would be. Some form of json schema validation might work for most json based callbacks, though that would involved the inclusion of another library, and might get complicated if the service is posting lots of different flavours of callback. Though this could be handled in a similar way to $rest_allowed_actions where we define a series of definitions per callback. Another approach would be to allow users to define (or select a predefined) extra validation functions for callbacks.

  3. At the moment, the only supported request methods are GET and POST. I think this covers 99% of callbacks thought I guess some might want to use DELETE or PUT or something else. We can extend it if so.

  4. At the moment, I am using my own redirect functions specifically for callbacks - one for POST and one for GET. I wrote these because the civiproxy_redirect() did not handle the body of post requests (even when the method used was POST). @systopia - you may want to refactor civiproxy_redirect so it can handle the body of post requests. Up to you how you want to handle it.

  5. The CALLBACK_SECRET is another layer of security that I have implemented to allow the proxy to fail early. This might interfere with callbacks that pass data via query params but in my experience these are few and far between (the CiviCRM rest API being a notable exception to the rule, and not good practice IMO, but nothing we have to worry about here since it is handled separately by the rest target).

  6. Note that I did not add any IP restrictions. In my experience, these services are operating at 'cloud scale' and do not guarantee an originating IP. Instead they use methods like addings secrets/tokens to the http headers, etc.