diff --git a/modules/structs/hook.go b/modules/structs/hook.go index 6e35c739254..84f492b45c0 100644 --- a/modules/structs/hook.go +++ b/modules/structs/hook.go @@ -120,12 +120,12 @@ type CreateHookOption struct { type EditHookOption struct { // Configuration settings for the webhook Config map[string]string `json:"config"` - // List of events that trigger this webhook + // List of events that trigger this webhook, omitted or empty keeps the current events Events []string `json:"events"` - // Branch filter pattern to determine which branches trigger the webhook - BranchFilter string `json:"branch_filter" binding:"GlobPattern"` - // Authorization header to include in webhook requests - AuthorizationHeader string `json:"authorization_header"` + // Branch filter pattern to determine which branches trigger the webhook, omitted keeps the current value + BranchFilter *string `json:"branch_filter" binding:"GlobPattern"` + // Authorization header to include in webhook requests, omitted keeps the current value unless the url changes + AuthorizationHeader *string `json:"authorization_header"` // Whether the webhook is active and will be triggered Active *bool `json:"active"` // Optional human-readable name diff --git a/routers/api/v1/utils/hook.go b/routers/api/v1/utils/hook.go index b0156b705a0..7974c5b2a77 100644 --- a/routers/api/v1/utils/hook.go +++ b/routers/api/v1/utils/hook.go @@ -344,6 +344,9 @@ func editHook(ctx *context.APIContext, form *api.EditHookOption, w *webhook.Webh ctx.APIError(http.StatusUnprocessableEntity, "Invalid url") return false } + if url != w.URL { + w.HeaderAuthorizationEncrypted = "" // the API never reveals the header, so it must not follow the hook to a new url + } w.URL = url } if ct, ok := form.Config["content_type"]; ok { @@ -371,17 +374,20 @@ func editHook(ctx *context.APIContext, form *api.EditHookOption, w *webhook.Webh } } - // Update events - w.HookEvents = updateHookEvents(form.Events) - w.PushOnly = false - w.SendEverything = false - w.ChooseEvents = true - w.BranchFilter = form.BranchFilter - - err := w.SetHeaderAuthorization(form.AuthorizationHeader) - if err != nil { - ctx.APIErrorInternal(err) - return false + if len(form.Events) > 0 { + w.HookEvents = updateHookEvents(form.Events) + w.PushOnly = false + w.SendEverything = false + w.ChooseEvents = true + } + if form.BranchFilter != nil { + w.BranchFilter = *form.BranchFilter + } + if form.AuthorizationHeader != nil { + if err := w.SetHeaderAuthorization(*form.AuthorizationHeader); err != nil { + ctx.APIErrorInternal(err) + return false + } } if err := w.UpdateEvent(); err != nil { diff --git a/routers/api/v1/utils/hook_test.go b/routers/api/v1/utils/hook_test.go index 0dc54e461bf..86f55add151 100644 --- a/routers/api/v1/utils/hook_test.go +++ b/routers/api/v1/utils/hook_test.go @@ -7,11 +7,15 @@ import ( "net/http" "testing" + "gitea.dev/models/db" "gitea.dev/models/unittest" + "gitea.dev/models/webhook" "gitea.dev/modules/structs" + webhook_module "gitea.dev/modules/webhook" "gitea.dev/services/contexttest" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestTestHookValidation(t *testing.T) { @@ -43,3 +47,46 @@ func TestTestHookValidation(t *testing.T) { }) } } + +func TestEditHook(t *testing.T) { + unittest.PrepareTestEnv(t) + + hook := &webhook.Webhook{ + RepoID: 1, + URL: "https://example.com/webhook", + ContentType: webhook.ContentTypeJSON, + IsActive: true, + HookEvent: &webhook_module.HookEvent{ + BranchFilter: "main", + ChooseEvents: true, + HookEvents: webhook_module.HookEvents{webhook_module.HookEventIssues: true}, + }, + } + require.NoError(t, hook.SetHeaderAuthorization("Bearer secret")) + require.NoError(t, hook.UpdateEvent()) + require.NoError(t, db.Insert(t.Context(), hook)) + + t.Run("KeepsOmittedFields", func(t *testing.T) { + ctx, _ := contexttest.MockAPIContext(t, "user2/repo1/hooks") + assert.True(t, editHook(ctx, &structs.EditHookOption{ + Active: new(false), + Events: []string{}, + Config: map[string]string{"url": hook.URL}, + }, hook)) + + updated := unittest.AssertExistsAndLoadBean(t, &webhook.Webhook{ID: hook.ID}) + assert.False(t, updated.IsActive) + assert.Equal(t, "main", updated.BranchFilter) + assert.True(t, updated.HasEvent(webhook_module.HookEventIssues)) + assert.False(t, updated.HasEvent(webhook_module.HookEventPush)) + header, err := updated.HeaderAuthorization() + require.NoError(t, err) + assert.Equal(t, "Bearer secret", header) + }) + + t.Run("NewURLClearsAuthorizationHeader", func(t *testing.T) { + ctx, _ := contexttest.MockAPIContext(t, "user2/repo1/hooks") + assert.True(t, editHook(ctx, &structs.EditHookOption{Config: map[string]string{"url": "https://example.org/webhook"}}, hook)) + assert.Empty(t, unittest.AssertExistsAndLoadBean(t, &webhook.Webhook{ID: hook.ID}).HeaderAuthorizationEncrypted) + }) +} diff --git a/templates/swagger/v1-openapi3.generated.json b/templates/swagger/v1-openapi3.generated.json index eaddae96fec..c10701c9b75 100644 --- a/templates/swagger/v1-openapi3.generated.json +++ b/templates/swagger/v1-openapi3.generated.json @@ -5543,12 +5543,12 @@ "x-go-name": "Active" }, "authorization_header": { - "description": "Authorization header to include in webhook requests", + "description": "Authorization header to include in webhook requests, omitted keeps the current value unless the url changes", "type": "string", "x-go-name": "AuthorizationHeader" }, "branch_filter": { - "description": "Branch filter pattern to determine which branches trigger the webhook", + "description": "Branch filter pattern to determine which branches trigger the webhook, omitted keeps the current value", "type": "string", "x-go-name": "BranchFilter" }, @@ -5561,7 +5561,7 @@ "x-go-name": "Config" }, "events": { - "description": "List of events that trigger this webhook", + "description": "List of events that trigger this webhook, omitted or empty keeps the current events", "items": { "type": "string" }, diff --git a/templates/swagger/v1-swagger.generated.json b/templates/swagger/v1-swagger.generated.json index 8ac00362fc1..15baa596f6a 100644 --- a/templates/swagger/v1-swagger.generated.json +++ b/templates/swagger/v1-swagger.generated.json @@ -27932,12 +27932,12 @@ "x-go-name": "Active" }, "authorization_header": { - "description": "Authorization header to include in webhook requests", + "description": "Authorization header to include in webhook requests, omitted keeps the current value unless the url changes", "type": "string", "x-go-name": "AuthorizationHeader" }, "branch_filter": { - "description": "Branch filter pattern to determine which branches trigger the webhook", + "description": "Branch filter pattern to determine which branches trigger the webhook, omitted keeps the current value", "type": "string", "x-go-name": "BranchFilter" }, @@ -27950,7 +27950,7 @@ "x-go-name": "Config" }, "events": { - "description": "List of events that trigger this webhook", + "description": "List of events that trigger this webhook, omitted or empty keeps the current events", "type": "array", "items": { "type": "string"