From 60110adc06ef85e533f48a52a744d43c63a818bc Mon Sep 17 00:00:00 2001 From: Unknwon Date: Sun, 17 Jul 2016 08:33:59 +0800 Subject: [PATCH] models/webhook: restrict deletion to be explicitly with repo and org ID --- README.md | 2 +- gogs.go | 2 +- models/webhook.go | 57 +++++++++++++++++++++++++------------ routers/api/v1/repo/hook.go | 18 ++++++------ routers/org/setting.go | 18 ++---------- routers/repo/webhook.go | 4 +-- templates/.VERSION | 2 +- 7 files changed, 55 insertions(+), 48 deletions(-) diff --git a/README.md b/README.md index debd753f9..26b515d78 100644 --- a/README.md +++ b/README.md @@ -3,7 +3,7 @@ Gogs - Go Git Service [![Build Status](https://travis-ci.org/gogits/gogs.svg?bra ![](https://github.com/gogits/gogs/blob/master/public/img/gogs-large-resize.png?raw=true) -##### Current tip version: 0.9.45 (see [Releases](https://github.com/gogits/gogs/releases) for binary versions) +##### Current tip version: 0.9.46 (see [Releases](https://github.com/gogits/gogs/releases) for binary versions) | Web | UI | Preview | |:-------------:|:-------:|:-------:| diff --git a/gogs.go b/gogs.go index 3cff3bf15..efb9c915e 100644 --- a/gogs.go +++ b/gogs.go @@ -17,7 +17,7 @@ import ( "github.com/gogits/gogs/modules/setting" ) -const APP_VER = "0.9.45.0716" +const APP_VER = "0.9.46.0717" func init() { runtime.GOMAXPROCS(runtime.NumCPU()) diff --git a/models/webhook.go b/models/webhook.go index 134ac1017..02bea5cf0 100644 --- a/models/webhook.go +++ b/models/webhook.go @@ -174,28 +174,32 @@ func CreateWebhook(w *Webhook) error { return err } -// GetWebhookByRepoID returns webhook of repository by given ID. -func GetWebhookByRepoID(repoID, id int64) (*Webhook, error) { - w := new(Webhook) - has, err := x.Id(id).And("repo_id=?", repoID).Get(w) +// getWebhook uses argument bean as query condition, +// ID must be specified and do not assign unnecessary fields. +func getWebhook(bean *Webhook) (*Webhook, error) { + has, err := x.Get(bean) if err != nil { return nil, err } else if !has { - return nil, ErrWebhookNotExist{id} + return nil, ErrWebhookNotExist{bean.ID} } - return w, nil + return bean, nil +} + +// GetWebhookByRepoID returns webhook of repository by given ID. +func GetWebhookByRepoID(repoID, id int64) (*Webhook, error) { + return getWebhook(&Webhook{ + ID: id, + RepoID: repoID, + }) } // GetWebhookByOrgID returns webhook of organization by given ID. func GetWebhookByOrgID(orgID, id int64) (*Webhook, error) { - w := new(Webhook) - has, err := x.Id(id).And("org_id=?", orgID).Get(w) - if err != nil { - return nil, err - } else if !has { - return nil, ErrWebhookNotExist{id} - } - return w, nil + return getWebhook(&Webhook{ + ID: id, + OrgID: orgID, + }) } // GetActiveWebhooksByRepoID returns all active webhooks of repository. @@ -216,23 +220,40 @@ func UpdateWebhook(w *Webhook) error { return err } -// DeleteWebhook deletes webhook of repository. -func DeleteWebhook(id int64) (err error) { +// deleteWebhook uses argument bean as query condition, +// ID must be specified and do not assign unnecessary fields. +func deleteWebhook(bean *Webhook) (err error) { sess := x.NewSession() defer sessionRelease(sess) if err = sess.Begin(); err != nil { return err } - if _, err = sess.Delete(&Webhook{ID: id}); err != nil { + if _, err = sess.Delete(bean); err != nil { return err - } else if _, err = sess.Delete(&HookTask{HookID: id}); err != nil { + } else if _, err = sess.Delete(&HookTask{HookID: bean.ID}); err != nil { return err } return sess.Commit() } +// DeleteWebhookByRepoID deletes webhook of repository by given ID. +func DeleteWebhookByRepoID(repoID, id int64) (error) { + return deleteWebhook(&Webhook{ + ID: id, + RepoID: repoID, + }) +} + +// DeleteWebhookByOrgID deletes webhook of organization by given ID. +func DeleteWebhookByOrgID(orgID, id int64) (error) { + return deleteWebhook(&Webhook{ + ID: id, + OrgID: orgID, + }) +} + // GetWebhooksByOrgID returns all webhooks for an organization. func GetWebhooksByOrgID(orgID int64) (ws []*Webhook, err error) { err = x.Find(&ws, &Webhook{OrgID: orgID}) diff --git a/routers/api/v1/repo/hook.go b/routers/api/v1/repo/hook.go index 4cda05c8b..2811a4d29 100644 --- a/routers/api/v1/repo/hook.go +++ b/routers/api/v1/repo/hook.go @@ -96,15 +96,6 @@ func CreateHook(ctx *context.APIContext, form api.CreateHookOption) { ctx.JSON(201, convert.ToHook(ctx.Repo.RepoLink, w)) } -func DeleteHook(ctx *context.APIContext) { - if err := models.DeleteWebhook(ctx.ParamsInt64(":id")); err != nil { - ctx.Error(500, "DeleteWebhook", err) - return - } - - ctx.Status(204) -} - // https://github.com/gogits/go-gogs-client/wiki/Repositories#edit-a-hook func EditHook(ctx *context.APIContext, form api.EditHookOption) { w, err := models.GetWebhookByRepoID(ctx.Repo.Repository.ID, ctx.ParamsInt64(":id")) @@ -171,3 +162,12 @@ func EditHook(ctx *context.APIContext, form api.EditHookOption) { ctx.JSON(200, convert.ToHook(ctx.Repo.RepoLink, w)) } + +func DeleteHook(ctx *context.APIContext) { + if err := models.DeleteWebhookByRepoID(ctx.Repo.Repository.ID, ctx.ParamsInt64(":id")); err != nil { + ctx.Error(500, "DeleteWebhookByRepoID", err) + return + } + + ctx.Status(204) +} diff --git a/routers/org/setting.go b/routers/org/setting.go index a938581c6..7900613b7 100644 --- a/routers/org/setting.go +++ b/routers/org/setting.go @@ -7,8 +7,6 @@ package org import ( "strings" - "github.com/Unknwon/com" - "github.com/gogits/gogs/models" "github.com/gogits/gogs/modules/auth" "github.com/gogits/gogs/modules/base" @@ -142,18 +140,6 @@ func Webhooks(ctx *context.Context) { ctx.Data["BaseLink"] = ctx.Org.OrgLink ctx.Data["Description"] = ctx.Tr("org.settings.hooks_desc") - // Delete web hook. - remove := com.StrTo(ctx.Query("remove")).MustInt64() - if remove > 0 { - if err := models.DeleteWebhook(remove); err != nil { - ctx.Handle(500, "DeleteWebhook", err) - return - } - ctx.Flash.Success(ctx.Tr("repo.settings.remove_hook_success")) - ctx.Redirect(ctx.Org.OrgLink + "/settings/hooks") - return - } - ws, err := models.GetWebhooksByOrgID(ctx.Org.Organization.Id) if err != nil { ctx.Handle(500, "GetWebhooksByOrgId", err) @@ -165,8 +151,8 @@ func Webhooks(ctx *context.Context) { } func DeleteWebhook(ctx *context.Context) { - if err := models.DeleteWebhook(ctx.QueryInt64("id")); err != nil { - ctx.Flash.Error("DeleteWebhook: " + err.Error()) + if err := models.DeleteWebhookByOrgID(ctx.Org.Organization.Id, ctx.QueryInt64("id")); err != nil { + ctx.Flash.Error("DeleteWebhookByOrgID: " + err.Error()) } else { ctx.Flash.Success(ctx.Tr("repo.settings.webhook_deletion_success")) } diff --git a/routers/repo/webhook.go b/routers/repo/webhook.go index b2274033b..22d5cfd2c 100644 --- a/routers/repo/webhook.go +++ b/routers/repo/webhook.go @@ -384,8 +384,8 @@ func TestWebhook(ctx *context.Context) { } func DeleteWebhook(ctx *context.Context) { - if err := models.DeleteWebhook(ctx.QueryInt64("id")); err != nil { - ctx.Flash.Error("DeleteWebhook: " + err.Error()) + if err := models.DeleteWebhookByRepoID(ctx.Repo.Repository.ID, ctx.QueryInt64("id")); err != nil { + ctx.Flash.Error("DeleteWebhookByRepoID: " + err.Error()) } else { ctx.Flash.Success(ctx.Tr("repo.settings.webhook_deletion_success")) } diff --git a/templates/.VERSION b/templates/.VERSION index 457b52c98..c0497e65c 100644 --- a/templates/.VERSION +++ b/templates/.VERSION @@ -1 +1 @@ -0.9.45.0716 \ No newline at end of file +0.9.46.0717 \ No newline at end of file