From 1e050d01c0ca44b9c1a34d00a3cc28faef22aa7a Mon Sep 17 00:00:00 2001 From: oliverpool Date: Thu, 21 Mar 2024 14:15:56 +0100 Subject: [PATCH 1/5] [REFACTOR] merge once-called functions --- routers/web/repo/setting/webhook.go | 99 ++++++++++------------------- routers/web/web.go | 29 ++++----- 2 files changed, 44 insertions(+), 84 deletions(-) diff --git a/routers/web/repo/setting/webhook.go b/routers/web/repo/setting/webhook.go index e89fcef39a..e6e869d8f6 100644 --- a/routers/web/repo/setting/webhook.go +++ b/routers/web/repo/setting/webhook.go @@ -191,18 +191,6 @@ func ParseHookEvent(form forms.WebhookForm) *webhook_module.HookEvent { } } -type webhookParams struct { - // Type should be imported from webhook package (webhook.XXX) - Type string - - URL string - ContentType webhook.HookContentType - Secret string - HTTPMethod string - WebhookForm forms.WebhookForm - Meta any -} - func WebhookCreate(ctx *context.Context) { typ := ctx.Params(":type") handler := webhook_service.GetWebhookHandler(typ) @@ -213,25 +201,14 @@ func WebhookCreate(ctx *context.Context) { fields := handler.FormFields(func(form any) { errs := binding.Bind(ctx.Req, form) - middleware.Validate(errs, ctx.Data, form, ctx.Locale) // error will be checked later in ctx.HasError + middleware.Validate(errs, ctx.Data, form, ctx.Locale) // error checked below in ctx.HasError }) - createWebhook(ctx, webhookParams{ - Type: typ, - URL: fields.URL, - ContentType: fields.ContentType, - Secret: fields.Secret, - HTTPMethod: fields.HTTPMethod, - WebhookForm: fields.WebhookForm, - Meta: fields.Metadata, - }) -} -func createWebhook(ctx *context.Context, params webhookParams) { ctx.Data["Title"] = ctx.Tr("repo.settings.add_webhook") ctx.Data["PageIsSettingsHooks"] = true ctx.Data["PageIsSettingsHooksNew"] = true ctx.Data["Webhook"] = webhook.Webhook{HookEvent: &webhook_module.HookEvent{}} - ctx.Data["HookType"] = params.Type + ctx.Data["HookType"] = typ orCtx, err := getOwnerRepoCtx(ctx) if err != nil { @@ -247,8 +224,8 @@ func createWebhook(ctx *context.Context, params webhookParams) { } var meta []byte - if params.Meta != nil { - meta, err = json.Marshal(params.Meta) + if fields.Metadata != nil { + meta, err = json.Marshal(fields.Metadata) if err != nil { ctx.ServerError("Marshal", err) return @@ -257,18 +234,18 @@ func createWebhook(ctx *context.Context, params webhookParams) { w := &webhook.Webhook{ RepoID: orCtx.RepoID, - URL: params.URL, - HTTPMethod: params.HTTPMethod, - ContentType: params.ContentType, - Secret: params.Secret, - HookEvent: ParseHookEvent(params.WebhookForm), - IsActive: params.WebhookForm.Active, - Type: params.Type, + URL: fields.URL, + HTTPMethod: fields.HTTPMethod, + ContentType: fields.ContentType, + Secret: fields.Secret, + HookEvent: ParseHookEvent(fields.WebhookForm), + IsActive: fields.WebhookForm.Active, + Type: typ, Meta: string(meta), OwnerID: orCtx.OwnerID, IsSystemWebhook: orCtx.IsSystemWebhook, } - err = w.SetHeaderAuthorization(params.WebhookForm.AuthorizationHeader) + err = w.SetHeaderAuthorization(fields.WebhookForm.AuthorizationHeader) if err != nil { ctx.ServerError("SetHeaderAuthorization", err) return @@ -286,29 +263,6 @@ func createWebhook(ctx *context.Context, params webhookParams) { } func WebhookUpdate(ctx *context.Context) { - typ := ctx.Params(":type") - handler := webhook_service.GetWebhookHandler(typ) - if handler == nil { - ctx.NotFound("GetWebhookHandler", nil) - return - } - - fields := handler.FormFields(func(form any) { - errs := binding.Bind(ctx.Req, form) - middleware.Validate(errs, ctx.Data, form, ctx.Locale) // error will be checked later in ctx.HasError - }) - editWebhook(ctx, webhookParams{ - Type: typ, - URL: fields.URL, - ContentType: fields.ContentType, - Secret: fields.Secret, - HTTPMethod: fields.HTTPMethod, - WebhookForm: fields.WebhookForm, - Meta: fields.Metadata, - }) -} - -func editWebhook(ctx *context.Context, params webhookParams) { ctx.Data["Title"] = ctx.Tr("repo.settings.update_webhook") ctx.Data["PageIsSettingsHooks"] = true ctx.Data["PageIsSettingsHooksEdit"] = true @@ -319,6 +273,17 @@ func editWebhook(ctx *context.Context, params webhookParams) { } ctx.Data["Webhook"] = w + handler := webhook_service.GetWebhookHandler(w.Type) + if handler == nil { + ctx.NotFound("GetWebhookHandler", nil) + return + } + + fields := handler.FormFields(func(form any) { + errs := binding.Bind(ctx.Req, form) + middleware.Validate(errs, ctx.Data, form, ctx.Locale) // error checked below in ctx.HasError + }) + if ctx.HasError() { ctx.HTML(http.StatusUnprocessableEntity, orCtx.NewTemplate) return @@ -326,23 +291,23 @@ func editWebhook(ctx *context.Context, params webhookParams) { var meta []byte var err error - if params.Meta != nil { - meta, err = json.Marshal(params.Meta) + if fields.Metadata != nil { + meta, err = json.Marshal(fields.Metadata) if err != nil { ctx.ServerError("Marshal", err) return } } - w.URL = params.URL - w.ContentType = params.ContentType - w.Secret = params.Secret - w.HookEvent = ParseHookEvent(params.WebhookForm) - w.IsActive = params.WebhookForm.Active - w.HTTPMethod = params.HTTPMethod + w.URL = fields.URL + w.ContentType = fields.ContentType + w.Secret = fields.Secret + w.HookEvent = ParseHookEvent(fields.WebhookForm) + w.IsActive = fields.WebhookForm.Active + w.HTTPMethod = fields.HTTPMethod w.Meta = string(meta) - err = w.SetHeaderAuthorization(params.WebhookForm.AuthorizationHeader) + err = w.SetHeaderAuthorization(fields.WebhookForm.AuthorizationHeader) if err != nil { ctx.ServerError("SetHeaderAuthorization", err) return diff --git a/routers/web/web.go b/routers/web/web.go index 348b9546c7..b2677f1a65 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -400,15 +400,6 @@ func registerRoutes(m *web.Route) { } } - addWebhookAddRoutes := func() { - m.Get("/{type}/new", repo_setting.WebhooksNew) - m.Post("/{type}/new", repo_setting.WebhookCreate) - } - - addWebhookEditRoutes := func() { - m.Post("/{type}/{id:[0-9]+}", repo_setting.WebhookUpdate) - } - addSettingsVariablesRoutes := func() { m.Group("/variables", func() { m.Get("", repo_setting.Variables) @@ -618,12 +609,13 @@ func registerRoutes(m *web.Route) { m.Group("/hooks", func() { m.Get("", user_setting.Webhooks) m.Post("/delete", user_setting.DeleteWebhook) - addWebhookAddRoutes() + m.Get("/{type}/new", repo_setting.WebhooksNew) + m.Post("/{type}/new", repo_setting.WebhookCreate) m.Group("/{id}", func() { m.Get("", repo_setting.WebHooksEdit) m.Post("/replay/{uuid}", repo_setting.ReplayWebhook) }) - addWebhookEditRoutes() + m.Post("/{type}/{id:[0-9]+}", repo_setting.WebhookUpdate) }, webhooksEnabled) m.Group("/blocked_users", func() { @@ -725,11 +717,12 @@ func registerRoutes(m *web.Route) { m.Get("", repo_setting.WebHooksEdit) m.Post("/replay/{uuid}", repo_setting.ReplayWebhook) }) - addWebhookEditRoutes() + m.Post("/{type}/{id:[0-9]+}", repo_setting.WebhookUpdate) }, webhooksEnabled) m.Group("/{configType:default-hooks|system-hooks}", func() { - addWebhookAddRoutes() + m.Get("/{type}/new", repo_setting.WebhooksNew) + m.Post("/{type}/new", repo_setting.WebhookCreate) }) m.Group("/auths", func() { @@ -887,12 +880,13 @@ func registerRoutes(m *web.Route) { m.Group("/hooks", func() { m.Get("", org.Webhooks) m.Post("/delete", org.DeleteWebhook) - addWebhookAddRoutes() + m.Get("/{type}/new", repo_setting.WebhooksNew) + m.Post("/{type}/new", repo_setting.WebhookCreate) m.Group("/{id}", func() { m.Get("", repo_setting.WebHooksEdit) m.Post("/replay/{uuid}", repo_setting.ReplayWebhook) }) - addWebhookEditRoutes() + m.Post("/{type}/{id:[0-9]+}", repo_setting.WebhookUpdate) }, webhooksEnabled) m.Group("/labels", func() { @@ -1061,13 +1055,14 @@ func registerRoutes(m *web.Route) { m.Group("/hooks", func() { m.Get("", repo_setting.Webhooks) m.Post("/delete", repo_setting.DeleteWebhook) - addWebhookAddRoutes() + m.Get("/{type}/new", repo_setting.WebhooksNew) + m.Post("/{type}/new", repo_setting.WebhookCreate) m.Group("/{id}", func() { m.Get("", repo_setting.WebHooksEdit) m.Post("/test", repo_setting.TestWebhook) m.Post("/replay/{uuid}", repo_setting.ReplayWebhook) }) - addWebhookEditRoutes() + m.Post("/{type}/{id:[0-9]+}", repo_setting.WebhookUpdate) }, webhooksEnabled) m.Group("/keys", func() { From 635230ca5debd9f230fcf216add83fa5658bc3c2 Mon Sep 17 00:00:00 2001 From: oliverpool Date: Sat, 23 Mar 2024 22:43:44 +0100 Subject: [PATCH 2/5] [TESTS] webhook forms keep submitted data when invalid --- routers/web/repo/setting/webhook.go | 44 +++++++++++++------ templates/repo/settings/webhook/settings.tmpl | 2 +- tests/integration/repo_webhook_test.go | 11 ++++- 3 files changed, 42 insertions(+), 15 deletions(-) diff --git a/routers/web/repo/setting/webhook.go b/routers/web/repo/setting/webhook.go index e6e869d8f6..29d2573451 100644 --- a/routers/web/repo/setting/webhook.go +++ b/routers/web/repo/setting/webhook.go @@ -219,6 +219,22 @@ func WebhookCreate(ctx *context.Context) { ctx.Data["BaseLinkNew"] = orCtx.LinkNew if ctx.HasError() { + // pre-fill the form with the submitted data + var w webhook.Webhook + w.URL = fields.URL + w.ContentType = fields.ContentType + w.Secret = fields.Secret + w.HookEvent = ParseHookEvent(fields.WebhookForm) + w.IsActive = fields.WebhookForm.Active + w.HTTPMethod = fields.HTTPMethod + err := w.SetHeaderAuthorization(fields.WebhookForm.AuthorizationHeader) + if err != nil { + ctx.ServerError("SetHeaderAuthorization", err) + return + } + ctx.Data["Webhook"] = w + ctx.Data["HookMetadata"] = fields.Metadata + ctx.HTML(http.StatusUnprocessableEntity, orCtx.NewTemplate) return } @@ -284,13 +300,27 @@ func WebhookUpdate(ctx *context.Context) { middleware.Validate(errs, ctx.Data, form, ctx.Locale) // error checked below in ctx.HasError }) + // pre-fill the form with the submitted data + w.URL = fields.URL + w.ContentType = fields.ContentType + w.Secret = fields.Secret + w.HookEvent = ParseHookEvent(fields.WebhookForm) + w.IsActive = fields.WebhookForm.Active + w.HTTPMethod = fields.HTTPMethod + + err := w.SetHeaderAuthorization(fields.WebhookForm.AuthorizationHeader) + if err != nil { + ctx.ServerError("SetHeaderAuthorization", err) + return + } + if ctx.HasError() { + ctx.Data["HookMetadata"] = fields.Metadata ctx.HTML(http.StatusUnprocessableEntity, orCtx.NewTemplate) return } var meta []byte - var err error if fields.Metadata != nil { meta, err = json.Marshal(fields.Metadata) if err != nil { @@ -299,20 +329,8 @@ func WebhookUpdate(ctx *context.Context) { } } - w.URL = fields.URL - w.ContentType = fields.ContentType - w.Secret = fields.Secret - w.HookEvent = ParseHookEvent(fields.WebhookForm) - w.IsActive = fields.WebhookForm.Active - w.HTTPMethod = fields.HTTPMethod w.Meta = string(meta) - err = w.SetHeaderAuthorization(fields.WebhookForm.AuthorizationHeader) - if err != nil { - ctx.ServerError("SetHeaderAuthorization", err) - return - } - if err := w.UpdateEvent(); err != nil { ctx.ServerError("UpdateEvent", err) return diff --git a/templates/repo/settings/webhook/settings.tmpl b/templates/repo/settings/webhook/settings.tmpl index 3ef8894444..2cbbef3e40 100644 --- a/templates/repo/settings/webhook/settings.tmpl +++ b/templates/repo/settings/webhook/settings.tmpl @@ -259,7 +259,7 @@ -
+
{{if ne .HookType "matrix"}}{{/* Matrix doesn't make the authorization optional but it is implied by the help string, should be changed.*/}} diff --git a/tests/integration/repo_webhook_test.go b/tests/integration/repo_webhook_test.go index 47c7a1e436..3264f3894e 100644 --- a/tests/integration/repo_webhook_test.go +++ b/tests/integration/repo_webhook_test.go @@ -330,7 +330,16 @@ func testWebhookForms(name string, session *TestSession, validFields map[string] } } - session.MakeRequest(t, NewRequestWithValues(t, "POST", "/user2/repo1/settings/hooks/"+name+"/new", payload), http.StatusUnprocessableEntity) + resp := session.MakeRequest(t, NewRequestWithValues(t, "POST", "/user2/repo1/settings/hooks/"+name+"/new", payload), http.StatusUnprocessableEntity) + // check that the invalid form is pre-filled + htmlForm = NewHTMLParser(t, resp.Body).Find(`form[action^="/user2/repo1/settings/hooks/"]`) + for k, v := range payload { + if k == "_csrf" || k == "events" || v == "" { + // the 'events' is a radio input, which is buggy below + continue + } + assert.Equal(t, v, assertInput(t, htmlForm, k), "input %q did not contain value %q", k, v) + } if t.Failed() { t.Log(invalidPatch) } From c0dd92e9c5bd32541420c4055ec67f3cd7d173e2 Mon Sep 17 00:00:00 2001 From: oliverpool Date: Thu, 21 Mar 2024 14:33:59 +0100 Subject: [PATCH 3/5] [REFACTOR] webhook move edit endpoints --- routers/web/web.go | 8 ++++---- templates/repo/settings/webhook/dingtalk.tmpl | 2 +- templates/repo/settings/webhook/discord.tmpl | 2 +- templates/repo/settings/webhook/feishu.tmpl | 2 +- templates/repo/settings/webhook/forgejo.tmpl | 2 +- templates/repo/settings/webhook/gitea.tmpl | 2 +- templates/repo/settings/webhook/gogs.tmpl | 2 +- templates/repo/settings/webhook/matrix.tmpl | 2 +- templates/repo/settings/webhook/msteams.tmpl | 2 +- templates/repo/settings/webhook/packagist.tmpl | 2 +- templates/repo/settings/webhook/slack.tmpl | 2 +- templates/repo/settings/webhook/telegram.tmpl | 2 +- templates/repo/settings/webhook/wechatwork.tmpl | 2 +- tests/integration/links_test.go | 4 ++-- tests/integration/repo_webhook_test.go | 2 +- 15 files changed, 19 insertions(+), 19 deletions(-) diff --git a/routers/web/web.go b/routers/web/web.go index b2677f1a65..ec7f6749bf 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -613,9 +613,9 @@ func registerRoutes(m *web.Route) { m.Post("/{type}/new", repo_setting.WebhookCreate) m.Group("/{id}", func() { m.Get("", repo_setting.WebHooksEdit) + m.Post("", repo_setting.WebhookUpdate) m.Post("/replay/{uuid}", repo_setting.ReplayWebhook) }) - m.Post("/{type}/{id:[0-9]+}", repo_setting.WebhookUpdate) }, webhooksEnabled) m.Group("/blocked_users", func() { @@ -715,9 +715,9 @@ func registerRoutes(m *web.Route) { m.Post("/delete", admin.DeleteDefaultOrSystemWebhook) m.Group("/{id}", func() { m.Get("", repo_setting.WebHooksEdit) + m.Post("", repo_setting.WebhookUpdate) m.Post("/replay/{uuid}", repo_setting.ReplayWebhook) }) - m.Post("/{type}/{id:[0-9]+}", repo_setting.WebhookUpdate) }, webhooksEnabled) m.Group("/{configType:default-hooks|system-hooks}", func() { @@ -884,9 +884,9 @@ func registerRoutes(m *web.Route) { m.Post("/{type}/new", repo_setting.WebhookCreate) m.Group("/{id}", func() { m.Get("", repo_setting.WebHooksEdit) + m.Post("", repo_setting.WebhookUpdate) m.Post("/replay/{uuid}", repo_setting.ReplayWebhook) }) - m.Post("/{type}/{id:[0-9]+}", repo_setting.WebhookUpdate) }, webhooksEnabled) m.Group("/labels", func() { @@ -1059,10 +1059,10 @@ func registerRoutes(m *web.Route) { m.Post("/{type}/new", repo_setting.WebhookCreate) m.Group("/{id}", func() { m.Get("", repo_setting.WebHooksEdit) + m.Post("", repo_setting.WebhookUpdate) m.Post("/test", repo_setting.TestWebhook) m.Post("/replay/{uuid}", repo_setting.ReplayWebhook) }) - m.Post("/{type}/{id:[0-9]+}", repo_setting.WebhookUpdate) }, webhooksEnabled) m.Group("/keys", func() { diff --git a/templates/repo/settings/webhook/dingtalk.tmpl b/templates/repo/settings/webhook/dingtalk.tmpl index 0ba99e98ee..f1cd6a1932 100644 --- a/templates/repo/settings/webhook/dingtalk.tmpl +++ b/templates/repo/settings/webhook/dingtalk.tmpl @@ -1,6 +1,6 @@ {{if eq .HookType "dingtalk"}}

{{ctx.Locale.Tr "repo.settings.add_web_hook_desc" "https://dingtalk.com" (ctx.Locale.Tr "repo.settings.web_hook_name_dingtalk")}}

-
+ {{.CsrfTokenHtml}}
diff --git a/templates/repo/settings/webhook/discord.tmpl b/templates/repo/settings/webhook/discord.tmpl index d390e739be..95a3806c89 100644 --- a/templates/repo/settings/webhook/discord.tmpl +++ b/templates/repo/settings/webhook/discord.tmpl @@ -1,6 +1,6 @@ {{if eq .HookType "discord"}}

{{ctx.Locale.Tr "repo.settings.add_web_hook_desc" "https://discord.com" (ctx.Locale.Tr "repo.settings.web_hook_name_discord")}}

- + {{.CsrfTokenHtml}}
diff --git a/templates/repo/settings/webhook/feishu.tmpl b/templates/repo/settings/webhook/feishu.tmpl index d80deab26f..cd6aaf935a 100644 --- a/templates/repo/settings/webhook/feishu.tmpl +++ b/templates/repo/settings/webhook/feishu.tmpl @@ -1,7 +1,7 @@ {{if eq .HookType "feishu"}}

{{ctx.Locale.Tr "repo.settings.add_web_hook_desc" "https://feishu.cn" (ctx.Locale.Tr "repo.settings.web_hook_name_feishu")}}

{{ctx.Locale.Tr "repo.settings.add_web_hook_desc" "https://larksuite.com" (ctx.Locale.Tr "repo.settings.web_hook_name_larksuite")}}

- + {{.CsrfTokenHtml}}
diff --git a/templates/repo/settings/webhook/forgejo.tmpl b/templates/repo/settings/webhook/forgejo.tmpl index 0bfb99115c..5c8233a638 100644 --- a/templates/repo/settings/webhook/forgejo.tmpl +++ b/templates/repo/settings/webhook/forgejo.tmpl @@ -1,6 +1,6 @@ {{if eq .HookType "forgejo"}}

{{ctx.Locale.Tr "repo.settings.add_web_hook_desc" "https://forgejo.org/docs/latest/user/webhooks/" (ctx.Locale.Tr "repo.settings.web_hook_name_forgejo")}}

- + {{template "base/disable_form_autofill"}} {{.CsrfTokenHtml}}
diff --git a/templates/repo/settings/webhook/gitea.tmpl b/templates/repo/settings/webhook/gitea.tmpl index 38ec29c784..e2a80d3c78 100644 --- a/templates/repo/settings/webhook/gitea.tmpl +++ b/templates/repo/settings/webhook/gitea.tmpl @@ -1,6 +1,6 @@ {{if eq .HookType "gitea"}}

{{ctx.Locale.Tr "repo.settings.add_web_hook_desc" "https://forgejo.org/docs/latest/user/webhooks/" (ctx.Locale.Tr "repo.settings.web_hook_name_gitea")}}

- + {{template "base/disable_form_autofill"}} {{.CsrfTokenHtml}}
diff --git a/templates/repo/settings/webhook/gogs.tmpl b/templates/repo/settings/webhook/gogs.tmpl index ff1742aa18..3c60033650 100644 --- a/templates/repo/settings/webhook/gogs.tmpl +++ b/templates/repo/settings/webhook/gogs.tmpl @@ -1,6 +1,6 @@ {{if eq .HookType "gogs"}}

{{ctx.Locale.Tr "repo.settings.add_web_hook_desc" "https://forgejo.org/docs/latest/user/webhooks/" (ctx.Locale.Tr "repo.settings.web_hook_name_gogs")}}

- + {{template "base/disable_form_autofill"}} {{.CsrfTokenHtml}}
diff --git a/templates/repo/settings/webhook/matrix.tmpl b/templates/repo/settings/webhook/matrix.tmpl index 43df8fd5bd..57a78d3c81 100644 --- a/templates/repo/settings/webhook/matrix.tmpl +++ b/templates/repo/settings/webhook/matrix.tmpl @@ -1,6 +1,6 @@ {{if eq .HookType "matrix"}}

{{ctx.Locale.Tr "repo.settings.add_web_hook_desc" "https://matrix.org/" (ctx.Locale.Tr "repo.settings.web_hook_name_matrix")}}

- + {{.CsrfTokenHtml}}
diff --git a/templates/repo/settings/webhook/msteams.tmpl b/templates/repo/settings/webhook/msteams.tmpl index 62ea24e763..771183c13c 100644 --- a/templates/repo/settings/webhook/msteams.tmpl +++ b/templates/repo/settings/webhook/msteams.tmpl @@ -1,6 +1,6 @@ {{if eq .HookType "msteams"}}

{{ctx.Locale.Tr "repo.settings.add_web_hook_desc" "https://teams.microsoft.com" (ctx.Locale.Tr "repo.settings.web_hook_name_msteams")}}

- + {{.CsrfTokenHtml}}
diff --git a/templates/repo/settings/webhook/packagist.tmpl b/templates/repo/settings/webhook/packagist.tmpl index 447f0d7e6b..aad1ab5917 100644 --- a/templates/repo/settings/webhook/packagist.tmpl +++ b/templates/repo/settings/webhook/packagist.tmpl @@ -1,6 +1,6 @@ {{if eq .HookType "packagist"}}

{{ctx.Locale.Tr "repo.settings.add_web_hook_desc" "https://packagist.org" (ctx.Locale.Tr "repo.settings.web_hook_name_packagist")}}

- + {{.CsrfTokenHtml}}
diff --git a/templates/repo/settings/webhook/slack.tmpl b/templates/repo/settings/webhook/slack.tmpl index 22c5f6e72f..1000f4a633 100644 --- a/templates/repo/settings/webhook/slack.tmpl +++ b/templates/repo/settings/webhook/slack.tmpl @@ -1,6 +1,6 @@ {{if eq .HookType "slack"}}

{{ctx.Locale.Tr "repo.settings.add_web_hook_desc" "https://slack.com" (ctx.Locale.Tr "repo.settings.web_hook_name_slack")}}

- + {{.CsrfTokenHtml}}
diff --git a/templates/repo/settings/webhook/telegram.tmpl b/templates/repo/settings/webhook/telegram.tmpl index 46f96ce495..84bc2dc49d 100644 --- a/templates/repo/settings/webhook/telegram.tmpl +++ b/templates/repo/settings/webhook/telegram.tmpl @@ -1,6 +1,6 @@ {{if eq .HookType "telegram"}}

{{ctx.Locale.Tr "repo.settings.add_web_hook_desc" "https://core.telegram.org/bots" (ctx.Locale.Tr "repo.settings.web_hook_name_telegram")}}

- + {{.CsrfTokenHtml}}
diff --git a/templates/repo/settings/webhook/wechatwork.tmpl b/templates/repo/settings/webhook/wechatwork.tmpl index 78a1617123..bee753bf6b 100644 --- a/templates/repo/settings/webhook/wechatwork.tmpl +++ b/templates/repo/settings/webhook/wechatwork.tmpl @@ -1,6 +1,6 @@ {{if eq .HookType "wechatwork"}}

{{ctx.Locale.Tr "repo.settings.add_web_hook_desc" "https://work.weixin.qq.com" (ctx.Locale.Tr "repo.settings.web_hook_name_wechatwork")}}

- + {{.CsrfTokenHtml}}
diff --git a/tests/integration/links_test.go b/tests/integration/links_test.go index 6edcbbc71b..68d7008e02 100644 --- a/tests/integration/links_test.go +++ b/tests/integration/links_test.go @@ -193,8 +193,8 @@ func TestRedirectsWebhooks(t *testing.T) { {from: "/user2/repo1/settings/hooks/" + kind + "/new", to: "/", verb: "POST"}, {from: "/admin/system-hooks/" + kind + "/new", to: "/", verb: "POST"}, {from: "/admin/default-hooks/" + kind + "/new", to: "/", verb: "POST"}, - {from: "/user2/repo1/settings/hooks/" + kind + "/1", to: "/", verb: "POST"}, - {from: "/admin/hooks/" + kind + "/1", to: "/", verb: "POST"}, + {from: "/user2/repo1/settings/hooks/1", to: "/", verb: "POST"}, + {from: "/admin/hooks/1", to: "/", verb: "POST"}, } for _, info := range redirects { req := NewRequest(t, info.verb, info.from) diff --git a/tests/integration/repo_webhook_test.go b/tests/integration/repo_webhook_test.go index 3264f3894e..15da511758 100644 --- a/tests/integration/repo_webhook_test.go +++ b/tests/integration/repo_webhook_test.go @@ -52,7 +52,7 @@ func TestNewWebHookLink(t *testing.T) { htmlDoc := NewHTMLParser(t, resp.Body) assert.Equal(t, webhooksLen, htmlDoc.Find(`a[href^="`+baseurl+`/"][href$="/new"]`).Length(), "not all webhooks are listed in the 'new' dropdown on failure") - resp = session.MakeRequest(t, NewRequestWithValues(t, "POST", baseurl+"/gitea/1", map[string]string{"_csrf": csrfToken}), http.StatusUnprocessableEntity) + resp = session.MakeRequest(t, NewRequestWithValues(t, "POST", baseurl+"/1", map[string]string{"_csrf": csrfToken}), http.StatusUnprocessableEntity) htmlDoc = NewHTMLParser(t, resp.Body) assert.Equal(t, webhooksLen, htmlDoc.Find(`a[href^="`+baseurl+`/"][href$="/new"]`).Length(), "not all webhooks are listed in the 'new' dropdown on failure") } From 9c3611ec50d4b2c5100f3b99b1bfa58a3dc41859 Mon Sep 17 00:00:00 2001 From: oliverpool Date: Thu, 21 Mar 2024 14:39:02 +0100 Subject: [PATCH 4/5] [REFACTOR] simplify checkHookType --- modules/setting/webhook.go | 2 -- routers/web/repo/setting/webhook.go | 31 ++++++++--------------------- 2 files changed, 8 insertions(+), 25 deletions(-) diff --git a/modules/setting/webhook.go b/modules/setting/webhook.go index b56c55c439..7b1ab4db1f 100644 --- a/modules/setting/webhook.go +++ b/modules/setting/webhook.go @@ -15,7 +15,6 @@ var Webhook = struct { DeliverTimeout int SkipTLSVerify bool AllowedHostList string - Types []string PagingNum int ProxyURL string ProxyURLFixed *url.URL @@ -35,7 +34,6 @@ func loadWebhookFrom(rootCfg ConfigProvider) { Webhook.DeliverTimeout = sec.Key("DELIVER_TIMEOUT").MustInt(5) Webhook.SkipTLSVerify = sec.Key("SKIP_TLS_VERIFY").MustBool() Webhook.AllowedHostList = sec.Key("ALLOWED_HOST_LIST").MustString("") - Webhook.Types = []string{"forgejo", "gitea", "gogs", "slack", "discord", "dingtalk", "telegram", "msteams", "feishu", "matrix", "wechatwork", "packagist"} Webhook.PagingNum = sec.Key("PAGING_NUM").MustInt(10) Webhook.ProxyURL = sec.Key("PROXY_URL").MustString("") if Webhook.ProxyURL != "" { diff --git a/routers/web/repo/setting/webhook.go b/routers/web/repo/setting/webhook.go index 29d2573451..58c9d2634c 100644 --- a/routers/web/repo/setting/webhook.go +++ b/routers/web/repo/setting/webhook.go @@ -10,7 +10,6 @@ import ( "net/http" "net/url" "path" - "strings" "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/perm" @@ -22,7 +21,6 @@ import ( "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" - "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web/middleware" webhook_module "code.gitea.io/gitea/modules/webhook" "code.gitea.io/gitea/services/context" @@ -111,15 +109,6 @@ func getOwnerRepoCtx(ctx *context.Context) (*ownerRepoCtx, error) { return nil, errors.New("unable to set OwnerRepo context") } -func checkHookType(ctx *context.Context) string { - hookType := strings.ToLower(ctx.Params(":type")) - if !util.SliceContainsString(setting.Webhook.Types, hookType, true) { - ctx.NotFound("checkHookType", nil) - return "" - } - return hookType -} - // WebhooksNew render creating webhook page func WebhooksNew(ctx *context.Context) { ctx.Data["Title"] = ctx.Tr("repo.settings.add_webhook") @@ -142,16 +131,12 @@ func WebhooksNew(ctx *context.Context) { ctx.Data["PageIsSettingsHooksNew"] = true } - hookType := checkHookType(ctx) - ctx.Data["HookType"] = hookType - if ctx.Written() { + hookType := ctx.Params(":type") + if webhook_service.GetWebhookHandler(hookType) == nil { + ctx.NotFound("GetWebhookHandler", nil) return } - if hookType == "discord" { - ctx.Data["DiscordHook"] = map[string]any{ - "Username": "Gitea", - } - } + ctx.Data["HookType"] = hookType ctx.Data["BaseLink"] = orCtx.LinkNew ctx.Data["BaseLinkNew"] = orCtx.LinkNew @@ -192,8 +177,8 @@ func ParseHookEvent(form forms.WebhookForm) *webhook_module.HookEvent { } func WebhookCreate(ctx *context.Context) { - typ := ctx.Params(":type") - handler := webhook_service.GetWebhookHandler(typ) + hookType := ctx.Params(":type") + handler := webhook_service.GetWebhookHandler(hookType) if handler == nil { ctx.NotFound("GetWebhookHandler", nil) return @@ -208,7 +193,7 @@ func WebhookCreate(ctx *context.Context) { ctx.Data["PageIsSettingsHooks"] = true ctx.Data["PageIsSettingsHooksNew"] = true ctx.Data["Webhook"] = webhook.Webhook{HookEvent: &webhook_module.HookEvent{}} - ctx.Data["HookType"] = typ + ctx.Data["HookType"] = hookType orCtx, err := getOwnerRepoCtx(ctx) if err != nil { @@ -256,7 +241,7 @@ func WebhookCreate(ctx *context.Context) { Secret: fields.Secret, HookEvent: ParseHookEvent(fields.WebhookForm), IsActive: fields.WebhookForm.Active, - Type: typ, + Type: hookType, Meta: string(meta), OwnerID: orCtx.OwnerID, IsSystemWebhook: orCtx.IsSystemWebhook, From bc36f85b5fcf14c12d071f5fe6fa46c9bfadf235 Mon Sep 17 00:00:00 2001 From: oliverpool Date: Thu, 21 Mar 2024 14:43:43 +0100 Subject: [PATCH 5/5] [REFACTOR] webhook repo naming consistency --- routers/web/repo/setting/webhook.go | 24 +++++++++++------------ routers/web/web.go | 30 ++++++++++++++--------------- 2 files changed, 27 insertions(+), 27 deletions(-) diff --git a/routers/web/repo/setting/webhook.go b/routers/web/repo/setting/webhook.go index 58c9d2634c..bf7770bfdb 100644 --- a/routers/web/repo/setting/webhook.go +++ b/routers/web/repo/setting/webhook.go @@ -39,8 +39,8 @@ const ( tplAdminHookNew base.TplName = "admin/hook_new" ) -// Webhooks render web hooks list page -func Webhooks(ctx *context.Context) { +// WebhookList render web hooks list page +func WebhookList(ctx *context.Context) { ctx.Data["Title"] = ctx.Tr("repo.settings.hooks") ctx.Data["PageIsSettingsHooks"] = true ctx.Data["BaseLink"] = ctx.Repo.RepoLink + "/settings/hooks" @@ -109,8 +109,8 @@ func getOwnerRepoCtx(ctx *context.Context) (*ownerRepoCtx, error) { return nil, errors.New("unable to set OwnerRepo context") } -// WebhooksNew render creating webhook page -func WebhooksNew(ctx *context.Context) { +// WebhookNew render creating webhook page +func WebhookNew(ctx *context.Context) { ctx.Data["Title"] = ctx.Tr("repo.settings.add_webhook") ctx.Data["Webhook"] = webhook.Webhook{HookEvent: &webhook_module.HookEvent{}} @@ -367,8 +367,8 @@ func checkWebhook(ctx *context.Context) (*ownerRepoCtx, *webhook.Webhook) { return orCtx, w } -// WebHooksEdit render editing web hook page -func WebHooksEdit(ctx *context.Context) { +// WebhookEdit render editing web hook page +func WebhookEdit(ctx *context.Context) { ctx.Data["Title"] = ctx.Tr("repo.settings.update_webhook") ctx.Data["PageIsSettingsHooks"] = true ctx.Data["PageIsSettingsHooksEdit"] = true @@ -382,8 +382,8 @@ func WebHooksEdit(ctx *context.Context) { ctx.HTML(http.StatusOK, orCtx.NewTemplate) } -// TestWebhook test if web hook is work fine -func TestWebhook(ctx *context.Context) { +// WebhookTest test if web hook is work fine +func WebhookTest(ctx *context.Context) { hookID := ctx.ParamsInt64(":id") w, err := webhook.GetWebhookByRepoID(ctx, ctx.Repo.Repository.ID, hookID) if err != nil { @@ -443,8 +443,8 @@ func TestWebhook(ctx *context.Context) { } } -// ReplayWebhook replays a webhook -func ReplayWebhook(ctx *context.Context) { +// WebhookReplay replays a webhook +func WebhookReplay(ctx *context.Context) { hookTaskUUID := ctx.Params(":uuid") orCtx, w := checkWebhook(ctx) @@ -465,8 +465,8 @@ func ReplayWebhook(ctx *context.Context) { ctx.Redirect(fmt.Sprintf("%s/%d", orCtx.Link, w.ID)) } -// DeleteWebhook delete a webhook -func DeleteWebhook(ctx *context.Context) { +// WebhookDelete delete a webhook +func WebhookDelete(ctx *context.Context) { if err := webhook.DeleteWebhookByRepoID(ctx, ctx.Repo.Repository.ID, ctx.FormInt64("id")); err != nil { ctx.Flash.Error("DeleteWebhookByRepoID: " + err.Error()) } else { diff --git a/routers/web/web.go b/routers/web/web.go index ec7f6749bf..7329acd155 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -609,12 +609,12 @@ func registerRoutes(m *web.Route) { m.Group("/hooks", func() { m.Get("", user_setting.Webhooks) m.Post("/delete", user_setting.DeleteWebhook) - m.Get("/{type}/new", repo_setting.WebhooksNew) + m.Get("/{type}/new", repo_setting.WebhookNew) m.Post("/{type}/new", repo_setting.WebhookCreate) m.Group("/{id}", func() { - m.Get("", repo_setting.WebHooksEdit) + m.Get("", repo_setting.WebhookEdit) m.Post("", repo_setting.WebhookUpdate) - m.Post("/replay/{uuid}", repo_setting.ReplayWebhook) + m.Post("/replay/{uuid}", repo_setting.WebhookReplay) }) }, webhooksEnabled) @@ -714,14 +714,14 @@ func registerRoutes(m *web.Route) { m.Get("", admin.DefaultOrSystemWebhooks) m.Post("/delete", admin.DeleteDefaultOrSystemWebhook) m.Group("/{id}", func() { - m.Get("", repo_setting.WebHooksEdit) + m.Get("", repo_setting.WebhookEdit) m.Post("", repo_setting.WebhookUpdate) - m.Post("/replay/{uuid}", repo_setting.ReplayWebhook) + m.Post("/replay/{uuid}", repo_setting.WebhookReplay) }) }, webhooksEnabled) m.Group("/{configType:default-hooks|system-hooks}", func() { - m.Get("/{type}/new", repo_setting.WebhooksNew) + m.Get("/{type}/new", repo_setting.WebhookNew) m.Post("/{type}/new", repo_setting.WebhookCreate) }) @@ -880,12 +880,12 @@ func registerRoutes(m *web.Route) { m.Group("/hooks", func() { m.Get("", org.Webhooks) m.Post("/delete", org.DeleteWebhook) - m.Get("/{type}/new", repo_setting.WebhooksNew) + m.Get("/{type}/new", repo_setting.WebhookNew) m.Post("/{type}/new", repo_setting.WebhookCreate) m.Group("/{id}", func() { - m.Get("", repo_setting.WebHooksEdit) + m.Get("", repo_setting.WebhookEdit) m.Post("", repo_setting.WebhookUpdate) - m.Post("/replay/{uuid}", repo_setting.ReplayWebhook) + m.Post("/replay/{uuid}", repo_setting.WebhookReplay) }) }, webhooksEnabled) @@ -1053,15 +1053,15 @@ func registerRoutes(m *web.Route) { }, context.GitHookService()) m.Group("/hooks", func() { - m.Get("", repo_setting.Webhooks) - m.Post("/delete", repo_setting.DeleteWebhook) - m.Get("/{type}/new", repo_setting.WebhooksNew) + m.Get("", repo_setting.WebhookList) + m.Post("/delete", repo_setting.WebhookDelete) + m.Get("/{type}/new", repo_setting.WebhookNew) m.Post("/{type}/new", repo_setting.WebhookCreate) m.Group("/{id}", func() { - m.Get("", repo_setting.WebHooksEdit) + m.Get("", repo_setting.WebhookEdit) m.Post("", repo_setting.WebhookUpdate) - m.Post("/test", repo_setting.TestWebhook) - m.Post("/replay/{uuid}", repo_setting.ReplayWebhook) + m.Post("/test", repo_setting.WebhookTest) + m.Post("/replay/{uuid}", repo_setting.WebhookReplay) }) }, webhooksEnabled)