diff options
author | oliverpool <3864879+oliverpool@users.noreply.github.com> | 2022-11-03 19:23:20 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-11-03 20:23:20 +0200 |
commit | b6e81357bd6fb80f8ba94c513f89a210beb05313 (patch) | |
tree | b495f05c492c4876a5f8ded3a85d9985e0ec22a8 /routers | |
parent | 085f717529008c31b147f76ea7eeaf06ca8801bd (diff) | |
download | gitea-b6e81357bd6fb80f8ba94c513f89a210beb05313.tar.gz gitea-b6e81357bd6fb80f8ba94c513f89a210beb05313.zip |
Add Webhook authorization header (#20926)
_This is a different approach to #20267, I took the liberty of adapting
some parts, see below_
## Context
In some cases, a weebhook endpoint requires some kind of authentication.
The usual way is by sending a static `Authorization` header, with a
given token. For instance:
- Matrix expects a `Bearer <token>` (already implemented, by storing the
header cleartext in the metadata - which is buggy on retry #19872)
- TeamCity #18667
- Gitea instances #20267
- SourceHut https://man.sr.ht/graphql.md#authentication-strategies (this
is my actual personal need :)
## Proposed solution
Add a dedicated encrypt column to the webhook table (instead of storing
it as meta as proposed in #20267), so that it gets available for all
present and future hook types (especially the custom ones #19307).
This would also solve the buggy matrix retry #19872.
As a first step, I would recommend focusing on the backend logic and
improve the frontend at a later stage. For now the UI is a simple
`Authorization` field (which could be later customized with `Bearer` and
`Basic` switches):
![2022-08-23-142911](https://user-images.githubusercontent.com/3864879/186162483-5b721504-eef5-4932-812e-eb96a68494cc.png)
The header name is hard-coded, since I couldn't fine any usecase
justifying otherwise.
## Questions
- What do you think of this approach? @justusbunsi @Gusted @silverwind
- ~~How are the migrations generated? Do I have to manually create a new
file, or is there a command for that?~~
- ~~I started adding it to the API: should I complete it or should I
drop it? (I don't know how much the API is actually used)~~
## Done as well:
- add a migration for the existing matrix webhooks and remove the
`Authorization` logic there
_Closes #19872_
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: Gusted <williamzijl7@hotmail.com>
Co-authored-by: delvh <dev.lh@web.de>
Diffstat (limited to 'routers')
-rw-r--r-- | routers/api/v1/org/hook.go | 14 | ||||
-rw-r--r-- | routers/api/v1/repo/hook.go | 13 | ||||
-rw-r--r-- | routers/api/v1/utils/hook.go | 52 | ||||
-rw-r--r-- | routers/web/repo/webhook.go | 12 |
4 files changed, 80 insertions, 11 deletions
diff --git a/routers/api/v1/org/hook.go b/routers/api/v1/org/hook.go index 2ddb2b2d2b..f8ea5a876c 100644 --- a/routers/api/v1/org/hook.go +++ b/routers/api/v1/org/hook.go @@ -59,7 +59,11 @@ func ListHooks(ctx *context.APIContext) { hooks := make([]*api.Hook, len(orgHooks)) for i, hook := range orgHooks { - hooks[i] = convert.ToHook(ctx.Org.Organization.AsUser().HomeLink(), hook) + hooks[i], err = convert.ToHook(ctx.Org.Organization.AsUser().HomeLink(), hook) + if err != nil { + ctx.InternalServerError(err) + return + } } ctx.SetTotalCountHeader(count) @@ -95,7 +99,13 @@ func GetHook(ctx *context.APIContext) { if err != nil { return } - ctx.JSON(http.StatusOK, convert.ToHook(org.AsUser().HomeLink(), hook)) + + apiHook, err := convert.ToHook(org.AsUser().HomeLink(), hook) + if err != nil { + ctx.InternalServerError(err) + return + } + ctx.JSON(http.StatusOK, apiHook) } // CreateHook create a hook for an organization diff --git a/routers/api/v1/repo/hook.go b/routers/api/v1/repo/hook.go index 5956fe9da9..57fff7d326 100644 --- a/routers/api/v1/repo/hook.go +++ b/routers/api/v1/repo/hook.go @@ -69,7 +69,11 @@ func ListHooks(ctx *context.APIContext) { apiHooks := make([]*api.Hook, len(hooks)) for i := range hooks { - apiHooks[i] = convert.ToHook(ctx.Repo.RepoLink, hooks[i]) + apiHooks[i], err = convert.ToHook(ctx.Repo.RepoLink, hooks[i]) + if err != nil { + ctx.InternalServerError(err) + return + } } ctx.SetTotalCountHeader(count) @@ -112,7 +116,12 @@ func GetHook(ctx *context.APIContext) { if err != nil { return } - ctx.JSON(http.StatusOK, convert.ToHook(repo.RepoLink, hook)) + apiHook, err := convert.ToHook(repo.RepoLink, hook) + if err != nil { + ctx.InternalServerError(err) + return + } + ctx.JSON(http.StatusOK, apiHook) } // TestHook tests a hook diff --git a/routers/api/v1/utils/hook.go b/routers/api/v1/utils/hook.go index 7e4dfca9ad..aa922f4f5b 100644 --- a/routers/api/v1/utils/hook.go +++ b/routers/api/v1/utils/hook.go @@ -72,18 +72,39 @@ func CheckCreateHookOption(ctx *context.APIContext, form *api.CreateHookOption) func AddOrgHook(ctx *context.APIContext, form *api.CreateHookOption) { org := ctx.Org.Organization hook, ok := addHook(ctx, form, org.ID, 0) - if ok { - ctx.JSON(http.StatusCreated, convert.ToHook(org.AsUser().HomeLink(), hook)) + if !ok { + return + } + apiHook, ok := toAPIHook(ctx, org.AsUser().HomeLink(), hook) + if !ok { + return } + ctx.JSON(http.StatusCreated, apiHook) } // AddRepoHook add a hook to a repo. Writes to `ctx` accordingly func AddRepoHook(ctx *context.APIContext, form *api.CreateHookOption) { repo := ctx.Repo hook, ok := addHook(ctx, form, 0, repo.Repository.ID) - if ok { - ctx.JSON(http.StatusCreated, convert.ToHook(repo.RepoLink, hook)) + if !ok { + return + } + apiHook, ok := toAPIHook(ctx, repo.RepoLink, hook) + if !ok { + return + } + ctx.JSON(http.StatusCreated, apiHook) +} + +// toAPIHook converts the hook to its API representation. +// If there is an error, write to `ctx` accordingly. Return (hook, ok) +func toAPIHook(ctx *context.APIContext, repoLink string, hook *webhook.Webhook) (*api.Hook, bool) { + apiHook, err := convert.ToHook(repoLink, hook) + if err != nil { + ctx.Error(http.StatusInternalServerError, "ToHook", err) + return nil, false } + return apiHook, true } func issuesHook(events []string, event string) bool { @@ -135,6 +156,11 @@ func addHook(ctx *context.APIContext, form *api.CreateHookOption, orgID, repoID IsActive: form.Active, Type: form.Type, } + err := w.SetHeaderAuthorization(form.AuthorizationHeader) + if err != nil { + ctx.Error(http.StatusInternalServerError, "SetHeaderAuthorization", err) + return nil, false + } if w.Type == webhook.SLACK { channel, ok := form.Config["channel"] if !ok { @@ -185,7 +211,11 @@ func EditOrgHook(ctx *context.APIContext, form *api.EditHookOption, hookID int64 if err != nil { return } - ctx.JSON(http.StatusOK, convert.ToHook(org.AsUser().HomeLink(), updated)) + apiHook, ok := toAPIHook(ctx, org.AsUser().HomeLink(), updated) + if !ok { + return + } + ctx.JSON(http.StatusOK, apiHook) } // EditRepoHook edit webhook `w` according to `form`. Writes to `ctx` accordingly @@ -202,7 +232,11 @@ func EditRepoHook(ctx *context.APIContext, form *api.EditHookOption, hookID int6 if err != nil { return } - ctx.JSON(http.StatusOK, convert.ToHook(repo.RepoLink, updated)) + apiHook, ok := toAPIHook(ctx, repo.RepoLink, updated) + if !ok { + return + } + ctx.JSON(http.StatusOK, apiHook) } // editHook edit the webhook `w` according to `form`. If an error occurs, write @@ -254,6 +288,12 @@ func editHook(ctx *context.APIContext, form *api.EditHookOption, w *webhook.Webh w.Release = util.IsStringInSlice(string(webhook.HookEventRelease), form.Events, true) w.BranchFilter = form.BranchFilter + err := w.SetHeaderAuthorization(form.AuthorizationHeader) + if err != nil { + ctx.Error(http.StatusInternalServerError, "SetHeaderAuthorization", err) + return false + } + // Issues w.Issues = issuesHook(form.Events, "issues_only") w.IssueAssign = issuesHook(form.Events, string(webhook.HookEventIssueAssign)) diff --git a/routers/web/repo/webhook.go b/routers/web/repo/webhook.go index ee980333b7..5496496e80 100644 --- a/routers/web/repo/webhook.go +++ b/routers/web/repo/webhook.go @@ -239,6 +239,11 @@ func createWebhook(ctx *context.Context, params webhookParams) { OrgID: orCtx.OrgID, IsSystemWebhook: orCtx.IsSystemWebhook, } + err = w.SetHeaderAuthorization(params.WebhookForm.AuthorizationHeader) + if err != nil { + ctx.ServerError("SetHeaderAuthorization", err) + return + } if err := w.UpdateEvent(); err != nil { ctx.ServerError("UpdateEvent", err) return @@ -285,6 +290,12 @@ func editWebhook(ctx *context.Context, params webhookParams) { w.HTTPMethod = params.HTTPMethod w.Meta = string(meta) + err = w.SetHeaderAuthorization(params.WebhookForm.AuthorizationHeader) + if err != nil { + ctx.ServerError("SetHeaderAuthorization", err) + return + } + if err := w.UpdateEvent(); err != nil { ctx.ServerError("UpdateEvent", err) return @@ -445,7 +456,6 @@ func matrixHookParams(ctx *context.Context) webhookParams { Meta: &webhook_service.MatrixMeta{ HomeserverURL: form.HomeserverURL, Room: form.RoomID, - AccessToken: form.AccessToken, MessageType: form.MessageType, }, } |