Add webhooks for instant package updates - Fix #10 #414

WebFreak001 · 2019-02-19T20:23:33Z

Depends on New tabbed edit package UI #410 (contains that PR already for having new location in package edit page)

I put a http-update.md during development in the root folder just to collect some thoughts how I wanted to implement it, that file will need to be moved to some proper API documentation page (which I don't think we have yet)

wilzbach · 2019-02-19T20:59:46Z

http-update.md

+
+## `POST /api/packages/:packageName/update/github`
+
+Queues an update for the specified package. Compatible with GitHub webhooks and only triggers on `create` events. Must pass secret as query param and not in GitHub webhook settings.


Hmm, you still need to save the GitHub secret here to verify that it's the actual owner (though this is theoretically not needed).

Also, we need to rate-limit this.
And lastly it would be ideal to not use extra GitHub API requests for every event, but get as much of the info as possible.

I wanted to implement github properly at first, but vibe.d with rest interface doesn't give me a way to get the full request body as string, which I need for the SHA1 HMAC verification with the GitHub secret field (at least I didn't find any way)

Also I don't think any of the other dub API currently has rate limiting yet. This will only queue a recheck and is basically the same as spamming the "check for updates" button on your project page

I wanted to implement github properly at first,

Don't reinvent the wheel -> https://github.com/dlang/dlang-bot

basically the same as spamming the "check for updates" button on your project page

Fair point, but by providing hooks we make it a lot easier for people to spam / run us into the rate-limits of GitHub.

I wanted to implement github properly at first,

Don't reinvent the wheel -> dlang/dlang-bot

I more accurately meant I wanted to support the "Secret" field on GitHub which sends a Signature of the body text as SHA1 HMAC with the Secret as password, but with vibe.d the rest interface implementation already ate the body so I couldn't access the raw string from code to hash. For hashing the stdlib provides everything needed

Regarding the rate limiting, if we have web hook support, I would propose to stretch the polling over a much larger period than now (e.g. 6 hours vs. 30 minutes), so that it merely acts as a fallback in case the web hooks fail. This should reduce the risk of running into the rate limit considerable when compared to the current state.

Non-webhook repositories could be kept at the 30 min. polling interval, of course.

Regarding the signature issue, I believe this should work using an @before parameter that reads the body, validates the signature, and then parses and returns the JSON.

wilzbach · 2019-02-19T21:00:54Z

http-update.md

+`header`: string (optional) provide which header is used to check the secret (must start with X-)
+
+Body params: (application/x-www-form-urlencoded, multipart/form-data or application/json)
+`secret`: string (optional) provide the secret as body param


Hmm, what's the use case for this? Isn't this prone to be over-used/falsely used?
(I'm referring to the API method itself)

I wanted to make sure it's easy to incoperate this into any service you might be using for webhooks, so I'm offering to make the API callable with a secret per query string, body param or per HTTP Header.

views/my_packages.register.dt

s-ludwig · 2020-05-25T13:04:03Z

I missed this PR at the time, but I think it would be fine to add it like this. It wouldn't completely "fix" the original issue due to its manual nature, but it would be nice to have as a convenience feature for those willing to do some configuration work.

Just getting a decent percentage of the packages being updated by push notifications would still require OAuth+PubSubHubub or a similar solution that works more or less seamlessly.

WebFreak001

notes to self to fix

WebFreak001 · 2020-05-25T13:03:24Z

source/dubregistry/registry.d

+
+		char[24] token;
+		foreach (ref c; token)
+			c = lowerHexDigits[uniform(0, $)];


TODO: use secure random instead of uniform

WebFreak001 · 2020-05-25T13:04:44Z

source/dubregistry/api.d

@@ -187,6 +245,18 @@ private:
 	}
 }

+private string updateSecretReader(scope HTTPServerRequest req, scope HTTPServerResponse res)


this needs a better name

WebFreak001 · 2020-05-25T13:05:18Z

source/dubregistry/api.d

+	if (header.length)
+		return req.headers.get(header);
+
+	string ret = req.contentType == "application/json" ? req.json["secret"].get!string : req.form.get("secret", "");


probably want to tryIndex the JSON

WebFreak001 · 2020-05-25T13:06:10Z

source/dubregistry/api.d

+			return "No secret sent";
+
+		string expected = m_registry.getPackageSecret(_name);
+		if (!expected.length || secret != expected)


use constant-time comparison

WebFreak001 · 2020-05-25T13:08:33Z

source/dubregistry/registry.d

+		foreach (ref c; token)
+			c = lowerHexDigits[uniform(0, $)];
+
+		m_db.setPackageSecret(pack_name, token[].idup);


can just allocate in the first place instead of stack allocating & idup

WebFreak001 · 2020-05-25T13:09:51Z

source/dubregistry/api.d

+			enforceBadRequest(eventsObj.type == Json.Type.array, "Hook events must be of type array");
+			auto events = eventsObj.get!(Json[]);
+
+			foreach (ev; events)


can simplify the eventsObj to events and just iterate over Json here

WebFreak001 · 2020-05-25T13:10:45Z

source/dubregistry/api.d

+			string expected = m_registry.getPackageSecret(_name);
+			if (expected.length && secret.length && secret == expected)
+				m_registry.addPackageError(_name,
+					"GitHub hook configuration is invalid. Hook is missing 'create' event. (Tags or branches)");


this test code will never be run if configuration is valid

WebFreak001 · 2020-05-25T13:11:20Z

source/dubregistry/api.d

+					return "valid";
+
+			string expected = m_registry.getPackageSecret(_name);
+			if (expected.length && secret.length && secret == expected)


use constant-time compare

WebFreak001 added the WIP label Feb 19, 2019

wilzbach reviewed Feb 19, 2019

View reviewed changes

views/my_packages.register.dt Outdated Show resolved Hide resolved

wilzbach temporarily deployed to dub-registry-staging-pr-414 February 19, 2019 21:29 Inactive

wilzbach deployed to dub-registry-staging-pr-414 February 25, 2019 23:19 View deployment

WebFreak001 changed the title ~~Add webhooks for instant package updates - Fix #412~~ Add webhooks for instant package updates - Fix #10 Sep 21, 2019

WebFreak001 force-pushed the fix-412 branch from 1243ce8 to 682feb9 Compare September 21, 2019 16:40

fix dlang#10 add webhooks for instant version updates

ff568d4

WebFreak001 force-pushed the fix-412 branch from 682feb9 to ff568d4 Compare September 21, 2019 16:43

WebFreak001 commented May 25, 2020

View reviewed changes

WebFreak001 mentioned this pull request Oct 19, 2021

Bump dsymbol to latest stable tag dlang-community/DCD#655

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add webhooks for instant package updates - Fix #10 #414

Add webhooks for instant package updates - Fix #10 #414

WebFreak001 commented Feb 19, 2019 •

edited

Loading

wilzbach Feb 19, 2019

WebFreak001 Feb 19, 2019

WebFreak001 Feb 19, 2019

wilzbach Feb 19, 2019

wilzbach Feb 19, 2019

WebFreak001 Feb 19, 2019

s-ludwig May 25, 2020

wilzbach Feb 19, 2019

WebFreak001 Feb 19, 2019

s-ludwig commented May 25, 2020

WebFreak001 left a comment

WebFreak001 May 25, 2020

WebFreak001 May 25, 2020

WebFreak001 May 25, 2020

WebFreak001 May 25, 2020

WebFreak001 May 25, 2020

WebFreak001 May 25, 2020

WebFreak001 May 25, 2020

WebFreak001 May 25, 2020


		## `POST /api/packages/:packageName/update/github`

		Queues an update for the specified package. Compatible with GitHub webhooks and only triggers on `create` events. Must pass secret as query param and not in GitHub webhook settings.

Add webhooks for instant package updates - Fix #10 #414

Are you sure you want to change the base?

Add webhooks for instant package updates - Fix #10 #414

Conversation

WebFreak001 commented Feb 19, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

s-ludwig commented May 25, 2020

WebFreak001 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WebFreak001 commented Feb 19, 2019 •

edited

Loading