fedora-copr / copr

RPM build system - upstream for https://copr.fedorainfracloud.org/
113 stars 61 forks source link

Store payload in Webhook History & make webhook ID clickable to show corresponding payload #3467

Open jaitjacob opened 1 month ago

jaitjacob commented 1 month ago

In this PR,

  1. Webhook Payload is stored in Webhook History table.
  2. The Webhook ID is rendered as a link to open the payload in new tab. The JSON data is passed as a HTML Data Attribute along with the markup. image
  3. Payload opened in new tab using javascript function showJson, image

Reference, https://developer.mozilla.org/en-US/docs/Learn/HTML/Howto/Use_data_attributes

Fixed #634

praiskup commented 1 month ago

Thank you very much for your work on this!

Team: Could someone start a discussion on the Fedora Devel mailing list regarding whether it's considered "safe enough" to make the payload publicly available? My initial feeling is that payloads from Forges should be safe to publish, yet Forges don't typically do so. It would be helpful to get input from the involved parties and let them help us decide.

FrostyX commented 1 month ago

We also discussed that we could make the payloads downloadable only for project admins. Which is already done because the whole Settings > Integrations page requires login. The problem is that we do daily database dumps and allow anyone to download them. So we would have to exclude this table from the dumps. There is a precedent for this, which is for example a table with user API keys for copr-cli.

We talked that it would make our life easier if the webhook_history.payload column stored not the payload itself but only a link or command where to get it. I did some investigation how that would work for GitHub.

So, not very user-friendly.

praiskup commented 1 month ago

They don't allow user+password authentication and require a token. Then curl or gh can be used. But this is more setup steps than I would like

If we provide documentation on how to do the boilerplate setup (token?), and the table self-links this documentation, I don't think it is toooo bad. :+1:

jaitjacob commented 1 month ago

sorry marked it ready for review by mistake. please ignore.

praiskup commented 18 hours ago

@jaitjacob hello, how is this PR going? Do you think we could document the way(s) how to "to get the payload"?