Refactor webhooks to remove need for gate configuration and add event

Open on Drupal.org →

Created on 2 August 2023, about 1 year ago

Updated 11 July 2024, about 2 months ago

Problem/Motivation

The current webhook system is tedious and confusing to set up, especially when you have more than one gateway. You'd need to set up webhooks for every configured gateway, and then for every transaction, all but the current one will complain about the unknown transaction for example or things might even get processed more than once.

Steps to reproduce

Create two gateways, for example one for credit card and the other for twint payments.

Proposed resolution

Remove the gate context, look up the relevant gateway based on the passed context, which is mostly the space id.

We did this for the forked commerce_postfinance_checkout module and it has worked well there, this is basically just backporting this to the current API in this module.

Additionally, this patch adds an event subscriber so that custom code can easily listen to additional events, which is actually my main reason for the backport. I also removed two bogus and unused classes in the EventSubscriber namespace that don't do anything.

I believe there is no security aspect to the gate configuration, per https://app-wallee.com/de-de/doc/webhooks#_integration_details, no data that needs to be secured is transported, worst case is that you can cause a site to send a lot of requests to wallee, but you still need to find an actually valid transaction and space id for that.

Existing webhook urls with the gate url should keep working, it will just no longer use that.

Remaining tasks

As a possible follow-up, the module could also provide a button to set up the url and listeners automatically as it knows what exactly it needs.

This is a draft/work in progress, I still need to implement and verify the hook.

User interface changes

API changes

Data model changes

📌 Task

Status

Needs review

Version

2.0

Component

Code

Created by

🇨🇭Switzerland Berdir Switzerland

Live updates comments and jobs are added and updated live.

Sign in to follow issues

Merge Requests

!9Refactor webhooks to remove need for gate configuration and add event
Open
🇨🇭Switzerland Berdir
updated about 2 months ago

Comments & Activities

Issue created by @Berdir
Status changed to Needs review about 1 year ago1:50pm 2 August 2023
Comment about 1 year ago →
🇨🇭Switzerland Berdir Switzerland
First patch, again, reminder that this is work in progress.
Comment about 1 year ago →
🇨🇭Switzerland Berdir Switzerland
forgot to change the route name.
Assigned to ruvus
Comment about 1 year ago →
🇨🇭Switzerland cola
Comment 7 months ago →
🇨🇭Switzerland Berdir Switzerland
Reroll.

The sleep issue is moved to 🐛 Hardcoded sleep on webhook callback blocks processes Needs review . The patch there conflicts with this though.

This is working quite well for us, but note that it changes PaymentSdkInterface, so technically has BC breaking changes.
Comment 6 months ago →
🇨🇭Switzerland Berdir Switzerland
This is now finally in production for our client and so far is working well.

It's backwards compatible as the path stays the same, the gate query parameter just doesn't matter anymore.

I saw there's an option to use a sign, which might be a good idea, this doesn't use any data from the request but you could still at least get the system to do many possibly slow API requests.
Merge request !9Resolve #3378753 Refactor webhooks to remove need for gate configuration and add event → (Open) created by Berdir
Comment about 2 months ago →
🇨🇭Switzerland Berdir Switzerland
Switched to a merge request and fixed a wrong route name there.

contrib.social Blog FAQ Discussions

Production build 0.71.5 2024