Refactor webhook and fix feishu/lark secret (#34961)

options/locale/locale_en-US.ini

@@ -2355,6 +2355,7 @@ settings.payload_url = Target URL
 settings.http_method = HTTP Method
 settings.content_type = POST Content Type
 settings.secret = Secret
+settings.webhook_secret_desc = If the webhook server supports using secret, you can follow the webhook's manual and fill in a secret here.
 settings.slack_username = Username
 settings.slack_icon_url = Icon URL
 settings.slack_color = Color

routers/web/repo/setting/webhook.go

@@ -198,7 +198,6 @@ type webhookParams struct {
 
 	URL         string
 	ContentType webhook.HookContentType
-	Secret      string
 	HTTPMethod  string
 	WebhookForm forms.WebhookForm
 	Meta        any
@@ -237,7 +236,7 @@ func createWebhook(ctx *context.Context, params webhookParams) {
 		URL:             params.URL,
 		HTTPMethod:      params.HTTPMethod,
 		ContentType:     params.ContentType,
-		Secret:          params.Secret,
+		Secret:          params.WebhookForm.Secret,
 		HookEvent:       ParseHookEvent(params.WebhookForm),
 		IsActive:        params.WebhookForm.Active,
 		Type:            params.Type,
@@ -290,7 +289,7 @@ func editWebhook(ctx *context.Context, params webhookParams) {
 
 	w.URL = params.URL
 	w.ContentType = params.ContentType
-	w.Secret = params.Secret
+	w.Secret = params.WebhookForm.Secret
 	w.HookEvent = ParseHookEvent(params.WebhookForm)
 	w.IsActive = params.WebhookForm.Active
 	w.HTTPMethod = params.HTTPMethod
@@ -336,7 +335,6 @@ func giteaHookParams(ctx *context.Context) webhookParams {
 		Type:        webhook_module.GITEA,
 		URL:         form.PayloadURL,
 		ContentType: contentType,
-		Secret:      form.Secret,
 		HTTPMethod:  form.HTTPMethod,
 		WebhookForm: form.WebhookForm,
 	}
@@ -364,7 +362,6 @@ func gogsHookParams(ctx *context.Context) webhookParams {
 		Type:        webhook_module.GOGS,
 		URL:         form.PayloadURL,
 		ContentType: contentType,
-		Secret:      form.Secret,
 		WebhookForm: form.WebhookForm,
 	}
 }

services/forms/repo_form.go

@@ -238,6 +238,7 @@ type WebhookForm struct {
 	Active                   bool
 	BranchFilter             string `binding:"GlobPattern"`
 	AuthorizationHeader      string
+	Secret                   string
 }
 
 // PushOnly if the hook will be triggered when push
@@ -260,7 +261,6 @@ type NewWebhookForm struct {
 	PayloadURL  string `binding:"Required;ValidUrl"`
 	HTTPMethod  string `binding:"Required;In(POST,GET)"`
 	ContentType int    `binding:"Required"`
-	Secret      string
 	WebhookForm
 }
 
@@ -274,7 +274,6 @@ func (f *NewWebhookForm) Validate(req *http.Request, errs binding.Errors) bindin
 type NewGogshookForm struct {
 	PayloadURL  string `binding:"Required;ValidUrl"`
 	ContentType int    `binding:"Required"`
-	Secret      string
 	WebhookForm
 }
 

templates/admin/hooks.tmpl

@@ -1,9 +1,6 @@
 {{template "admin/layout_head" (dict "ctxData" . "pageClass" "admin hooks")}}
 	<div class="admin-setting-content">
-
 		{{template "repo/settings/webhook/base_list" .SystemWebhooks}}
 		{{template "repo/settings/webhook/base_list" .DefaultWebhooks}}
-
-		{{template "repo/settings/webhook/delete_modal" .}}
 	</div>
 {{template "admin/layout_footer" .}}

templates/org/settings/hooks.tmpl

@@ -1,5 +1,5 @@
 {{template "org/settings/layout_head" (dict "ctxData" . "pageClass" "organization settings webhooks")}}
 			<div class="org-setting-content">
-				{{template "repo/settings/webhook/list" .}}
+				{{template "repo/settings/webhook/base_list" .}}
 			</div>
 {{template "org/settings/layout_footer" .}}

templates/repo/settings/webhook/base.tmpl

@@ -1,5 +1,5 @@
 {{template "repo/settings/layout_head" (dict "ctxData" . "pageClass" "repository settings webhooks")}}
 	<div class="repo-setting-content">
-		{{template "repo/settings/webhook/list" .}}
+		{{template "repo/settings/webhook/base_list" .}}
 	</div>
 {{template "repo/settings/layout_footer" .}}

templates/repo/settings/webhook/base_list.tmpl

@@ -17,7 +17,10 @@
 					<a title="{{.URL}}" href="{{$.BaseLink}}/{{.ID}}">{{.URL}}</a>
 				</div>
 				<a class="muted tw-p-2" href="{{$.BaseLink}}/{{.ID}}">{{svg "octicon-pencil"}}</a>
-				<a class="delete-button tw-p-2" data-url="{{$.Link}}/delete" data-id="{{.ID}}">{{svg "octicon-trash"}}</a>
+				<a class="text red tw-p-2 link-action"
+					data-url="{{$.Link}}/delete?id={{.ID}}"
+					data-modal-confirm="{{ctx.Locale.Tr "repo.settings.webhook_deletion_desc"}}"
+				>{{svg "octicon-trash"}}</a>
 			</div>
 		{{end}}
 	</div>

templates/repo/settings/webhook/delete_modal.tmpl

@@ -1,10 +0,0 @@
-<div class="ui g-modal-confirm delete modal">
-	<div class="header">
-		{{svg "octicon-trash"}}
-		{{ctx.Locale.Tr "repo.settings.webhook_deletion"}}
-	</div>
-	<div class="content">
-		<p>{{ctx.Locale.Tr "repo.settings.webhook_deletion_desc"}}</p>
-	</div>
-	{{template "base/modal_actions_confirm" .}}
-</div>

templates/repo/settings/webhook/dingtalk.tmpl

@@ -6,6 +6,7 @@
 			<label for="payload_url">{{ctx.Locale.Tr "repo.settings.payload_url"}}</label>
 			<input id="payload_url" name="payload_url" type="url" value="{{.Webhook.URL}}" autofocus required>
 		</div>
-		{{template "repo/settings/webhook/settings" .}}
+		{{/* FIXME: support authorization header or not? */}}
+		{{template "repo/settings/webhook/settings" dict "BaseLink" .BaseLink "Webhook" .Webhook "UseAuthorizationHeader" "optional"}}
 	</form>
 {{end}}

templates/repo/settings/webhook/discord.tmpl

@@ -14,6 +14,7 @@
 			<label for="icon_url">{{ctx.Locale.Tr "repo.settings.discord_icon_url"}}</label>
 			<input id="icon_url" name="icon_url" value="{{.DiscordHook.IconURL}}" placeholder="https://example.com/assets/img/logo.svg">
 		</div>
-		{{template "repo/settings/webhook/settings" .}}
+		{{/* FIXME: support authorization header or not? */}}
+		{{template "repo/settings/webhook/settings" dict "BaseLink" .BaseLink "Webhook" .Webhook "UseAuthorizationHeader" "optional"}}
 	</form>
 {{end}}

templates/repo/settings/webhook/feishu.tmpl

@@ -1,12 +1,14 @@
 {{if eq .HookType "feishu"}}
-	<p>{{ctx.Locale.Tr "repo.settings.add_web_hook_desc" "https://feishu.cn" (ctx.Locale.Tr "repo.settings.web_hook_name_feishu")}}</p>
+	<p>
-	<p>{{ctx.Locale.Tr "repo.settings.add_web_hook_desc" "https://larksuite.com" (ctx.Locale.Tr "repo.settings.web_hook_name_larksuite")}}</p>
+		{{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")}}
+	</p>
 	<form class="ui form" action="{{.BaseLink}}/feishu/{{or .Webhook.ID "new"}}" method="post">
 		{{.CsrfTokenHtml}}
 		<div class="required field {{if .Err_PayloadURL}}error{{end}}">
 			<label for="payload_url">{{ctx.Locale.Tr "repo.settings.payload_url"}}</label>
 			<input id="payload_url" name="payload_url" type="url" value="{{.Webhook.URL}}" autofocus required>
 		</div>
-		{{template "repo/settings/webhook/settings" .}}
+		{{template "repo/settings/webhook/settings" dict "BaseLink" .BaseLink "Webhook" .Webhook "UseRequestSecret" "optional"}}
 	</form>
 {{end}}

templates/repo/settings/webhook/gitea.tmpl

@@ -31,10 +31,11 @@
 				</div>
 			</div>
 		</div>
-		<div class="field {{if .Err_Secret}}error{{end}}">
+		{{template "repo/settings/webhook/settings" dict
-			<label for="secret">{{ctx.Locale.Tr "repo.settings.secret"}}</label>
+			"BaseLink" .BaseLink
-			<input id="secret" name="secret" type="password" value="{{.Webhook.Secret}}" autocomplete="off">
+			"Webhook" .Webhook
-		</div>
+			"UseAuthorizationHeader" "optional"
-		{{template "repo/settings/webhook/settings" .}}
+			"UseRequestSecret" "optional"
+		}}
 	</form>
 {{end}}

templates/repo/settings/webhook/gogs.tmpl

@@ -19,10 +19,11 @@
 				</div>
 			</div>
 		</div>
-		<div class="field {{if .Err_Secret}}error{{end}}">
+		{{template "repo/settings/webhook/settings" dict
-			<label for="secret">{{ctx.Locale.Tr "repo.settings.secret"}}</label>
+			"BaseLink" .BaseLink
-			<input id="secret" name="secret" type="password" value="{{.Webhook.Secret}}" autocomplete="off">
+			"Webhook" .Webhook
-		</div>
+			"UseAuthorizationHeader" "optional"
-		{{template "repo/settings/webhook/settings" .}}
+			"UseRequestSecret" "optional"
+		}}
 	</form>
 {{end}}

templates/repo/settings/webhook/list.tmpl

@@ -1,4 +0,0 @@
-
-{{template "repo/settings/webhook/base_list" .}}
-
-{{template "repo/settings/webhook/delete_modal" .}}

templates/repo/settings/webhook/matrix.tmpl

@@ -22,6 +22,6 @@
 				</div>
 			</div>
 		</div>
-		{{template "repo/settings/webhook/settings" .}}
+		{{template "repo/settings/webhook/settings" dict "BaseLink" .BaseLink "Webhook" .Webhook "UseAuthorizationHeader" "required"}}
 	</form>
 {{end}}

templates/repo/settings/webhook/msteams.tmpl

@@ -6,6 +6,7 @@
 			<label for="payload_url">{{ctx.Locale.Tr "repo.settings.payload_url"}}</label>
 			<input id="payload_url" name="payload_url" type="url" value="{{.Webhook.URL}}" autofocus required>
 		</div>
-		{{template "repo/settings/webhook/settings" .}}
+		{{/* FIXME: support authorization header or not? */}}
+		{{template "repo/settings/webhook/settings" dict "BaseLink" .BaseLink "Webhook" .Webhook "UseAuthorizationHeader" "optional"}}
 	</form>
 {{end}}

templates/repo/settings/webhook/packagist.tmpl

@@ -14,6 +14,7 @@
 			<label for="package_url">{{ctx.Locale.Tr "repo.settings.packagist_package_url"}}</label>
 			<input id="package_url" name="package_url" value="{{.PackagistHook.PackageURL}}" placeholder="https://packagist.org/packages/laravel/framework" required>
 		</div>
-		{{template "repo/settings/webhook/settings" .}}
+		{{/* FIXME: support authorization header or not? */}}
+		{{template "repo/settings/webhook/settings" dict "BaseLink" .BaseLink "Webhook" .Webhook "UseAuthorizationHeader" "optional"}}
 	</form>
 {{end}}

templates/repo/settings/webhook/settings.tmpl

@@ -1,4 +1,52 @@
-{{$isNew:=or .PageIsSettingsHooksNew .PageIsAdminDefaultHooksNew .PageIsAdminSystemHooksNew}}
+{{/* Template attributes:
+- BaseLink: Base URL for the repository settings
+- WebHook: Webhook object containing details about the webhook
+- UseAuthorizationHeader: optional or required
+- UseRequestSecret: optional or required
+*/}}
+{{$isNew := not .Webhook.ID}}
+
+<div class="inline field">
+	<div class="ui checkbox">
+		<input name="active" type="checkbox" {{if or $isNew .Webhook.IsActive}}checked{{end}}>
+		<label>{{ctx.Locale.Tr "repo.settings.active"}}</label>
+		<span class="help">{{ctx.Locale.Tr "repo.settings.active_helper"}}</span>
+	</div>
+</div>
+
+<!-- Authorization Header -->
+{{if .UseAuthorizationHeader}}
+	{{$attributeValid := or (eq .UseAuthorizationHeader "optional") (eq .UseAuthorizationHeader "required")}}
+	{{if not $attributeValid}}<div class="ui error message">Invalid UseAuthorizationHeader: {{.UseAuthorizationHeader}}}</div>{{end}}
+	{{$required := eq .UseAuthorizationHeader "required"}}
+	<div class="field {{if $required}}required{{end}}">
+		<label>{{ctx.Locale.Tr "repo.settings.authorization_header"}}</label>
+		<input name="authorization_header" type="text" value="{{.Webhook.HeaderAuthorization}}" {{if $required}}required placeholder="Bearer $access_token"{{end}}>
+		{{if not $required}}
+			<span class="help">{{ctx.Locale.Tr "repo.settings.authorization_header_desc" (HTMLFormat "<code>%s</code>, <code>%s</code>" "Bearer token123456" "Basic YWxhZGRpbjpvcGVuc2VzYW1l")}}</span>
+		{{end}}
+	</div>
+{{end}}
+
+<!-- Secret -->
+{{if .UseRequestSecret}}
+	{{$attributeValid := or (eq .UseRequestSecret "optional") (eq .UseRequestSecret "required")}}
+	{{if not $attributeValid}}<div class="ui error message">Invalid UseRequestSecret: {{.UseRequestSecret}}}</div>{{end}}
+	{{$required := eq .UseRequestSecret "required"}}
+	<div class="field {{if $required}}required{{end}}">
+		<label>{{ctx.Locale.Tr "repo.settings.secret"}}</label>
+		<input name="secret" type="password" value="{{.Webhook.Secret}}" autocomplete="off" {{if $required}}required{{end}}>
+		<span class="help">{{ctx.Locale.Tr "repo.settings.webhook_secret_desc"}}</span>
+	</div>
+{{end}}
+
+<!-- Branch filter -->
+<div class="field">
+	<label>{{ctx.Locale.Tr "repo.settings.branch_filter"}}</label>
+	<input name="branch_filter" type="text" value="{{or .Webhook.BranchFilter "*"}}">
+	<span class="help">{{ctx.Locale.Tr "repo.settings.branch_filter_desc" "https://pkg.go.dev/github.com/gobwas/glob#Compile" "github.com/gobwas/glob"}}</span>
+</div>
+
 <div class="field">
 	<h4>{{ctx.Locale.Tr "repo.settings.event_desc"}}</h4>
 	<div class="grouped event type fields">
@@ -286,38 +334,14 @@
 	</div>
 </div>
 
-<!-- Branch filter -->
-<div class="field">
-	<label for="branch_filter">{{ctx.Locale.Tr "repo.settings.branch_filter"}}</label>
-	<input id="branch_filter" name="branch_filter" type="text" value="{{or .Webhook.BranchFilter "*"}}">
-	<span class="help">{{ctx.Locale.Tr "repo.settings.branch_filter_desc" "https://pkg.go.dev/github.com/gobwas/glob#Compile" "github.com/gobwas/glob"}}</span>
-</div>
-
-<!-- Authorization Header -->
-<div class="field{{if eq .HookType "matrix"}} required{{end}}">
-	<label for="authorization_header">{{ctx.Locale.Tr "repo.settings.authorization_header"}}</label>
-	<input id="authorization_header" name="authorization_header" type="text" value="{{.Webhook.HeaderAuthorization}}"{{if eq .HookType "matrix"}} placeholder="Bearer $access_token" required{{end}}>
-	{{if ne .HookType "matrix"}}{{/* Matrix doesn't make the authorization optional but it is implied by the help string, should be changed.*/}}
-		<span class="help">{{ctx.Locale.Tr "repo.settings.authorization_header_desc" (HTMLFormat "<code>%s</code>, <code>%s</code>" "Bearer token123456" "Basic YWxhZGRpbjpvcGVuc2VzYW1l")}}</span>
-	{{end}}
-</div>
-
-<div class="divider"></div>
-
-<div class="inline field">
-	<div class="ui checkbox">
-		<input name="active" type="checkbox" {{if or $isNew .Webhook.IsActive}}checked{{end}}>
-		<label>{{ctx.Locale.Tr "repo.settings.active"}}</label>
-		<span class="help">{{ctx.Locale.Tr "repo.settings.active_helper"}}</span>
-	</div>
-</div>
 <div class="field">
 	{{if $isNew}}
 		<button class="ui primary button">{{ctx.Locale.Tr "repo.settings.add_webhook"}}</button>
 	{{else}}
 		<button class="ui primary button">{{ctx.Locale.Tr "repo.settings.update_webhook"}}</button>
-		<a class="ui red delete-button button" data-url="{{.BaseLink}}/delete" data-id="{{.Webhook.ID}}">{{ctx.Locale.Tr "repo.settings.delete_webhook"}}</a>
+		<a class="ui red button link-action"
+			data-url="{{.BaseLink}}/delete?id={{.Webhook.ID}}"
+			data-modal-confirm="{{ctx.Locale.Tr "repo.settings.webhook_deletion_desc"}}"
+		>{{ctx.Locale.Tr "repo.settings.delete_webhook"}}</a>
 	{{end}}
 </div>
-
-{{template "repo/settings/webhook/delete_modal" .}}

templates/repo/settings/webhook/slack.tmpl

@@ -23,6 +23,7 @@
 			<label for="color">{{ctx.Locale.Tr "repo.settings.slack_color"}}</label>
 			<input id="color" name="color" value="{{.SlackHook.Color}}" placeholder="#dd4b39, good, warning, danger">
 		</div>
-		{{template "repo/settings/webhook/settings" .}}
+		{{/* FIXME: support authorization header or not? */}}
+		{{template "repo/settings/webhook/settings" dict "BaseLink" .BaseLink "Webhook" .Webhook "UseAuthorizationHeader" "optional"}}
 	</form>
 {{end}}

templates/repo/settings/webhook/telegram.tmpl

@@ -14,6 +14,7 @@
 			<label for="thread_id">{{ctx.Locale.Tr "repo.settings.thread_id"}}</label>
 			<input id="thread_id" name="thread_id" type="text" value="{{.TelegramHook.ThreadID}}">
 		</div>
-		{{template "repo/settings/webhook/settings" .}}
+		{{/* FIXME: support authorization header or not? */}}
+		{{template "repo/settings/webhook/settings" dict "BaseLink" .BaseLink "Webhook" .Webhook "UseAuthorizationHeader" "optional"}}
 	</form>
 {{end}}

templates/repo/settings/webhook/wechatwork.tmpl

@@ -6,6 +6,7 @@
 			<label for="payload_url">{{ctx.Locale.Tr "repo.settings.payload_url"}}</label>
 			<input id="payload_url" name="payload_url" type="url" value="{{.Webhook.URL}}" autofocus required>
 		</div>
-		{{template "repo/settings/webhook/settings" .}}
+		{{/* FIXME: support authorization header or not? */}}
+		{{template "repo/settings/webhook/settings" dict "BaseLink" .BaseLink "Webhook" .Webhook "UseAuthorizationHeader" "optional"}}
 	</form>
 {{end}}

templates/user/settings/hooks.tmpl

@@ -1,5 +1,5 @@
 {{template "user/settings/layout_head" (dict "ctxData" . "pageClass" "user settings webhooks")}}
 	<div class="user-setting-content">
-		{{template "repo/settings/webhook/list" .}}
+		{{template "repo/settings/webhook/base_list" .}}
 	</div>
 {{template "user/settings/layout_footer" .}}