diff --git a/models/webhook.go b/models/webhook.go index c9c3c21a46..8f5c561939 100644 --- a/models/webhook.go +++ b/models/webhook.go @@ -258,8 +258,10 @@ func deleteWebhook(bean *Webhook) (err error) { return err } - if _, err = sess.Delete(bean); err != nil { + if count, err := sess.Delete(bean); err != nil { return err + } else if count == 0 { + return ErrWebhookNotExist{ID: bean.ID} } else if _, err = sess.Delete(&HookTask{HookID: bean.ID}); err != nil { return err } diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 69b9a272e0..c30db1a33a 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -119,6 +119,44 @@ func reqRepoWriter() macaron.Handler { } } +func reqOrgMembership() macaron.Handler { + return func(ctx *context.APIContext) { + var orgID int64 + if ctx.Org.Organization != nil { + orgID = ctx.Org.Organization.ID + } else if ctx.Org.Team != nil { + orgID = ctx.Org.Team.OrgID + } else { + ctx.Error(500, "", "reqOrgMembership: unprepared context") + return + } + + if !models.IsOrganizationMember(orgID, ctx.User.ID) { + ctx.Error(403, "", "Must be an organization member") + return + } + } +} + +func reqOrgOwnership() macaron.Handler { + return func(ctx *context.APIContext) { + var orgID int64 + if ctx.Org.Organization != nil { + orgID = ctx.Org.Organization.ID + } else if ctx.Org.Team != nil { + orgID = ctx.Org.Team.OrgID + } else { + ctx.Error(500, "", "reqOrgOwnership: unprepared context") + return + } + + if !models.IsOrganizationOwner(orgID, ctx.User.ID) { + ctx.Error(403, "", "Must be an organization member") + return + } + } +} + func orgAssignment(args ...bool) macaron.Handler { var ( assignOrg bool @@ -362,9 +400,9 @@ func RegisterRoutes(m *macaron.Macaron) { m.Combo("").Get(org.ListHooks). Post(bind(api.CreateHookOption{}), org.CreateHook) m.Combo("/:id").Get(org.GetHook). - Patch(bind(api.EditHookOption{}), org.EditHook). - Delete(org.DeleteHook) - }) + Patch(reqOrgOwnership(), bind(api.EditHookOption{}), org.EditHook). + Delete(reqOrgOwnership(), org.DeleteHook) + }, reqOrgMembership()) }, orgAssignment(true)) m.Group("/teams/:teamid", func() { m.Get("", org.GetTeam) diff --git a/routers/api/v1/org/hook.go b/routers/api/v1/org/hook.go index ebea13e56a..3a0b74ac21 100644 --- a/routers/api/v1/org/hook.go +++ b/routers/api/v1/org/hook.go @@ -58,7 +58,11 @@ func DeleteHook(ctx *context.APIContext) { org := ctx.Org.Organization hookID := ctx.ParamsInt64(":id") if err := models.DeleteWebhookByOrgID(org.ID, hookID); err != nil { - ctx.Error(500, "DeleteWebhookByOrgID", err) + if models.IsErrWebhookNotExist(err) { + ctx.Status(404) + } else { + ctx.Error(500, "DeleteWebhookByOrgID", err) + } return } ctx.Status(204) diff --git a/routers/api/v1/repo/hook.go b/routers/api/v1/repo/hook.go index 51f64e3452..2e3b655a12 100644 --- a/routers/api/v1/repo/hook.go +++ b/routers/api/v1/repo/hook.go @@ -59,9 +59,12 @@ func EditHook(ctx *context.APIContext, form api.EditHookOption) { // DeleteHook delete a hook of a repository func DeleteHook(ctx *context.APIContext) { if err := models.DeleteWebhookByRepoID(ctx.Repo.Repository.ID, ctx.ParamsInt64(":id")); err != nil { - ctx.Error(500, "DeleteWebhookByRepoID", err) + if models.IsErrWebhookNotExist(err) { + ctx.Status(404) + } else { + ctx.Error(500, "DeleteWebhookByRepoID", err) + } return } - ctx.Status(204) }