models/webhook: restrict deletion to be explicitly with repo and org ID

7 files changed, 55 insertions, 48 deletions

diff --git a/README.md b/README.md
index debd753f9e..26b515d788 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 3cff3bf153..efb9c915e6 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 134ac10173..02bea5cf0b 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 4cda05c8b8..2811a4d29e 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 a938581c61..7900613b77 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 b2274033b0..22d5cfd2c6 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 457b52c984..c0497e65c5 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