Webhooks 5.10beta1 incorrectly registers plugin events and does not renew its event cache in time

[John Heenan]

Blesta: 5.10.beta1
PHP: 8.2.18
OS: Debian 12.5 (stable, bookworm)

Webhooks reads available events from files with Observer derived classes in core/Util/Events/Observers directory and in top level plugins directories.

1) Even if a plugin is not installed (but its files are present), or is installed but disabled, Webhooks still incorrectly reads these file for available events.

2) Webhooks caches its list of availabe events using the Cache class from minphp. Once the cache is setup it is not renewed again. This mean without manually deleting the cache, the cache cannot read new available events from new or edited plugins. Also it cannot remove events no longer available.

 

[John Heenan]

How to reproduce the two bugs above with one sample generated plugin:

Using the extensions generator plugin, create an advanced Form Type Plugin with name 'Sample A' and add in an event 'SampleA.add' with callback SampleaAdd

Adding in event:

The plugin that was generated and that can now be edited and regenerated:

The plugin without being installed:

Just exists as files in plugins directory:

 

Add in the following to the end of file sample_a_plugin.php:

use Blesta\Core\Util\Events\Observer;
class SampleAObserver extends Observer
{
  public static function add(EventInterface $event)
  {
    return parent::triggerEvent($event);
  }
}

 

Delete the webhooks record of available events with

rm cache/1/plugins/webhooks/*

Without installing the Sample A plugin:

1) Refresh Webhooks Tools page anf click on '+' at top right for 'Add Webhook'.
2) Open the Event drop down list
3) Scroll to end

The 'SampleA.add' event is listed as available despite plugin 'Sample A' not being installed:

Install the plugin to confirm the event is recognised by Blesta:

 

1) Uninstall the plugin
2) Delete the sample_a directory in plugins
3) Refresh the Webhooks Tools page as above
4) The 'SampleA.add' event is still listed as available despite the plugin uninstalled and deleted from the file system

Remove the Webhooks cache as before with

rm cache/1/plugins/webhooks/*

Refresh the Webhooks Tools page as before

The 'SampleA.add' event is no longer listed as available

 

[John Heenan]

A quick review of Webhooks source code reveals why the bugs exist. In fact it was a review of the Webhooks source code that prompted tests to confirm these bugs do exist, rather than behavioural observations. I was lucky I reviewed the code before starting tests.

Just to be clear, 'Sample A'  exists to prove bugs and does not include a triggerEvent function, but it can easily be added into a top level model file. Blesta likes to keep Observer derived classes in separate files and include these classes in parent triggerEvent functions through a load mechansim. The triggerEvent function is required for incoming Webhooks and I assume other functionality. Also the use of an Observer derived class, as above, is also required for Webhooks functionality and I also assume other functionality. It does  not matter what file it is on as long it is in a plugin top level .php file.

While Observer derived classes in separate files  is fine for complex core functionaliy, I would personally like to see use use of a less complex mechanism, like including the Observer class in the same top level model file as the triggerEvent function for simpler use cases, while keeping an option to change. 

[John Heenan]

There is a default cache refresh time of two hours set in config/blesta.php.

Of course for development purposes with webhooks this is not practical, so the webhooks cache needs to be busted, as above during development.

[John Heenan]

Some notes on progress.

1. I added in the following two functions below the automatically generated getEvents() function in sample_a_plugin.php (part of SampleAPlugin class)

public function SampleaAdd(EventInterface $event)
  {
    $params = $event->getParams();
    $this->logger->info("SampleaAdd $params", [$params]);
    $a = (int) ($params['a'] ?? 0);
    $b = (int) ($params['b'] ?? 0);
    $c = $a + $b;
    $resultadd['a'] = $a;
    $resultadd['b'] = $b;
    $resultadd['c'] = "a ($a) + b ($b) = c ($c)";
    $event->setReturnValue($resultadd);
  }

  public function triggerEvent($name, array $params = [])
  {

    $this->logger->info("triggerEvent", [$name, $params]);
    $eventFactory = new EventFactory();
    $eventListener = $eventFactory->listener();
    $eventListener->register($name, [$this, $name]);
    $event = $eventListener->trigger($eventFactory->event($name, $params));


    // Get the event return value
    $returnValue = $event->getReturnValue();

    // Put return in a special index
    $return = ['__return__' => $returnValue];

    // Any return values that match the submitted params should be put in their own index to support extract() calls
    if (is_array($returnValue)) {
      foreach ($returnValue as $key => $data) {
        if (array_key_exists($key, $params)) {
          $return[$key] = $data;
        }
      }
    }
    return $return;
  }

2. I left the SampleAObserver class where I had previously added it in as above (at the end of sample_a_plugin.php and outside SampleAPlugin class).

3. I could only get incoming webbhook events triggered with a GET request. A POST_JSON request causes confusion because an object gets passed through instead of an array. I could not get a plain POST with parameters recognised, the parameters were ignored.

4. The incoming webhook only work ONCE after the cache is busted. After this the event is not triggered again until the cache is busted again.

5. In my example, setting an outgoing POST_JSON webhook for the same incoming webhook resulted in the data form the incoming request getting echoed, not the results of the event triggered. This makes sense. The triggerEvent function can be expanded to fire a second 'after' event which can include both the original paremeters and the result, which can be subscribed to instead.

6. The outgoing webhook, which echoes incoming parameters, only activates if if the event is triggered as above. Hence the same requirement to bust the cache is required.

7. Following is the result of making a GET request to the incoming webhook to trigger the 'Sample A' add event to add 1 and 2 with URI path /admin/plugin/webhooks/trigger/index/SampleaAdd?a=1&b=2

8. The result shown from above to add 1+2 is {"__return__":{"a":1,"b":2,"c":"a (1) + b (2) = c (3)"},"a":1,"b":2}. Again, it will only work once after each cache bust as above.

9. So, I did manage to simplify as noted above. All additions are to a single file and there is no use of a load function to include the Observer derived class. Ironically I have not so far being able to trigger incoming webhook events in the domain module or in core events.

[John Heenan]

On 4/27/2024 at 4:40 AM, John Heenan said:

I could not get a plain POST with parameters recognised, the parameters were ignored.

With retesting, POST does work, POSTJSON does not work without source code alterations as the JSON is passed as a class object and not an array.

New topic posted at 

 

[John Heenan]

There is a one line fix for cache requiring deletion (for incoming webhooks to work) at:

There is also another posted fix to allow incoming POSTJSON to work.