Skip to content

Commit 51bac3a

Browse files
AlexTugarevroboquat
authored andcommitted
[gitlab/webhook] return code 200 on Unauthorized
Otherwise we provoke permanent deactivation of the webhook. In future we need to make the Unauthorized situation fixable from Gitpod.
1 parent 90a25f5 commit 51bac3a

File tree

1 file changed

+7
-4
lines changed

1 file changed

+7
-4
lines changed

components/server/ee/src/prebuilds/gitlab-app.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ export class GitLabApp {
4141
* - never return 500 status responses if the event has been handled
4242
* - prefer to return 200; indicate that the webhook is asynchronous by returning 201
4343
* - to support fast response times, perform I/O or computationally intensive operations asynchronously
44+
* - if a webhook fails repeatedly, it may be disabled automatically
45+
* - webhooks that return failure codes in the 4xx range are understood to be misconfigured, and these are disabled (permanently)
4446
*/
4547
this._router.post("/", async (req, res) => {
4648
const eventType = req.header("X-Gitlab-Event");
@@ -60,7 +62,7 @@ export class GitLabApp {
6062
if (eventType !== "Push Hook" || !secretToken) {
6163
log.warn("Unhandled GitLab event.", { event: eventType, secretToken: !!secretToken });
6264
res.status(200).send("Unhandled event.");
63-
await this.webhookEvents.updateEvent(event.id, { status: "dismissed_unauthorized" });
65+
await this.webhookEvents.updateEvent(event.id, { status: "ignored" });
6466
return;
6567
}
6668

@@ -75,11 +77,12 @@ export class GitLabApp {
7577
TraceContext.setError({ span }, error);
7678
}
7779
if (!user) {
78-
// If the webhook installer is no longer found in Gitpod's DB
79-
// we should send a UNAUTHORIZED signal.
80+
// webhooks are not supposed to return 4xx codes on application issues.
81+
// sending "Unauthorized" as content to support inspection of webhook delivery logs.
8082
span.finish();
81-
res.status(401).send("Unauthorized.");
83+
res.status(200).send("Unauthorized.");
8284
await this.webhookEvents.updateEvent(event.id, { status: "dismissed_unauthorized" });
85+
// TODO(at) explore ways to mark a project having issues with permissions.
8386
return;
8487
}
8588
/** no await */ this.handlePushHook({ span }, context, user, event).catch((error) => {

0 commit comments

Comments
 (0)