summaryrefslogtreecommitdiffstats
path: root/api4
diff options
context:
space:
mode:
authorGeorge Goldberg <george@gberg.me>2018-09-14 16:21:05 +0100
committerChristopher Speller <crspeller@gmail.com>2018-09-14 08:21:05 -0700
commitfdbb6de3d52a5f41f075812e3b87616685a21b9b (patch)
tree1e434c28bf56b82148124f15958f36423e02828c /api4
parent83a00ae9251475b3ef39284ba922cb3ca55a6fbe (diff)
downloadchat-fdbb6de3d52a5f41f075812e3b87616685a21b9b.tar.gz
chat-fdbb6de3d52a5f41f075812e3b87616685a21b9b.tar.bz2
chat-fdbb6de3d52a5f41f075812e3b87616685a21b9b.zip
MM-11520: Make entity ID checks consistent across api4. (#9395)
* MM-11520: Make entity ID checks consistent across api4. * Update tests.
Diffstat (limited to 'api4')
-rw-r--r--api4/oauth.go6
-rw-r--r--api4/post.go6
-rw-r--r--api4/status.go6
-rw-r--r--api4/status_test.go6
-rw-r--r--api4/team.go6
-rw-r--r--api4/team_test.go7
-rw-r--r--api4/user.go6
-rw-r--r--api4/webhook.go45
8 files changed, 68 insertions, 20 deletions
diff --git a/api4/oauth.go b/api4/oauth.go
index 990f292e9..2ed6641ee 100644
--- a/api4/oauth.go
+++ b/api4/oauth.go
@@ -91,6 +91,12 @@ func updateOAuthApp(c *Context, w http.ResponseWriter, r *http.Request) {
return
}
+ // The app being updated in the payload must be the same one as indicated in the URL.
+ if oauthApp.Id != c.Params.AppId {
+ c.SetInvalidParam("app_id")
+ return
+ }
+
c.LogAudit("attempt")
oldOauthApp, err := c.App.GetOAuthApp(c.Params.AppId)
diff --git a/api4/post.go b/api4/post.go
index 014174b37..46c688014 100644
--- a/api4/post.go
+++ b/api4/post.go
@@ -382,6 +382,12 @@ func updatePost(c *Context, w http.ResponseWriter, r *http.Request) {
return
}
+ // The post being updated in the payload must be the same one as indicated in the URL.
+ if post.Id != c.Params.PostId {
+ c.SetInvalidParam("post_id")
+ return
+ }
+
if !c.App.SessionHasPermissionToChannelByPost(c.Session, c.Params.PostId, model.PERMISSION_EDIT_POST) {
c.SetPermissionError(model.PERMISSION_EDIT_POST)
return
diff --git a/api4/status.go b/api4/status.go
index edba5460b..67b4d10bc 100644
--- a/api4/status.go
+++ b/api4/status.go
@@ -68,6 +68,12 @@ func updateUserStatus(c *Context, w http.ResponseWriter, r *http.Request) {
return
}
+ // The user being updated in the payload must be the same one as indicated in the URL.
+ if status.UserId != c.Params.UserId {
+ c.SetInvalidParam("user_id")
+ return
+ }
+
if !c.App.SessionHasPermissionToUser(c.Session, c.Params.UserId) {
c.SetPermissionError(model.PERMISSION_EDIT_OTHER_USERS)
return
diff --git a/api4/status_test.go b/api4/status_test.go
index afff8526d..ddfc879ae 100644
--- a/api4/status_test.go
+++ b/api4/status_test.go
@@ -121,7 +121,7 @@ func TestUpdateUserStatus(t *testing.T) {
defer th.TearDown()
Client := th.Client
- toUpdateUserStatus := &model.Status{Status: "online"}
+ toUpdateUserStatus := &model.Status{Status: "online", UserId: th.BasicUser.Id}
updateUserStatus, resp := Client.UpdateUserStatus(th.BasicUser.Id, toUpdateUserStatus)
CheckNoError(t, resp)
if updateUserStatus.Status != "online" {
@@ -150,6 +150,7 @@ func TestUpdateUserStatus(t *testing.T) {
}
toUpdateUserStatus.Status = "online"
+ toUpdateUserStatus.UserId = th.BasicUser2.Id
_, resp = Client.UpdateUserStatus(th.BasicUser2.Id, toUpdateUserStatus)
CheckForbiddenStatus(t, resp)
@@ -159,6 +160,9 @@ func TestUpdateUserStatus(t *testing.T) {
t.Fatal("Should return online status")
}
+ _, resp = Client.UpdateUserStatus(th.BasicUser.Id, toUpdateUserStatus)
+ CheckBadRequestStatus(t, resp)
+
Client.Logout()
_, resp = Client.UpdateUserStatus(th.BasicUser2.Id, toUpdateUserStatus)
diff --git a/api4/team.go b/api4/team.go
index 2c229dce1..5a387ee35 100644
--- a/api4/team.go
+++ b/api4/team.go
@@ -137,7 +137,11 @@ func updateTeam(c *Context, w http.ResponseWriter, r *http.Request) {
return
}
- team.Id = c.Params.TeamId
+ // The team being updated in the payload must be the same one as indicated in the URL.
+ if team.Id != c.Params.TeamId {
+ c.SetInvalidParam("team_id")
+ return
+ }
if !c.App.SessionHasPermissionToTeam(c.Session, c.Params.TeamId, model.PERMISSION_MANAGE_TEAM) {
c.SetPermissionError(model.PERMISSION_MANAGE_TEAM)
diff --git a/api4/team_test.go b/api4/team_test.go
index 8304c979d..547b8d4a9 100644
--- a/api4/team_test.go
+++ b/api4/team_test.go
@@ -322,11 +322,8 @@ func TestUpdateTeam(t *testing.T) {
originalTeamId := team.Id
team.Id = model.NewId()
- if r, err := Client.DoApiPut(Client.GetTeamRoute(originalTeamId), team.ToJson()); err != nil {
- t.Fatal(err)
- } else {
- uteam = model.TeamFromJson(r.Body)
- }
+ r, _ := Client.DoApiPut(Client.GetTeamRoute(originalTeamId), team.ToJson())
+ assert.Equal(t, http.StatusBadRequest, r.StatusCode)
if uteam.Id != originalTeamId {
t.Fatal("wrong team id")
diff --git a/api4/user.go b/api4/user.go
index 8d4a264f8..5e97122d7 100644
--- a/api4/user.go
+++ b/api4/user.go
@@ -583,6 +583,12 @@ func updateUser(c *Context, w http.ResponseWriter, r *http.Request) {
return
}
+ // The user being updated in the payload must be the same one as indicated in the URL.
+ if user.Id != c.Params.UserId {
+ c.SetInvalidParam("user_id")
+ return
+ }
+
if !c.App.SessionHasPermissionToUser(c.Session, user.Id) {
c.SetPermissionError(model.PERMISSION_EDIT_OTHER_USERS)
return
diff --git a/api4/webhook.go b/api4/webhook.go
index 90d9a7dc8..05426e776 100644
--- a/api4/webhook.go
+++ b/api4/webhook.go
@@ -67,17 +67,21 @@ func updateIncomingHook(c *Context, w http.ResponseWriter, r *http.Request) {
return
}
- hookId := c.Params.HookId
-
updatedHook := model.IncomingWebhookFromJson(r.Body)
if updatedHook == nil {
c.SetInvalidParam("incoming_webhook")
return
}
+ // The hook being updated in the payload must be the same one as indicated in the URL.
+ if updatedHook.Id != c.Params.HookId {
+ c.SetInvalidParam("hook_id")
+ return
+ }
+
c.LogAudit("attempt")
- oldHook, err := c.App.GetIncomingWebhook(hookId)
+ oldHook, err := c.App.GetIncomingWebhook(c.Params.HookId)
if err != nil {
c.Err = err
return
@@ -247,34 +251,49 @@ func updateOutgoingHook(c *Context, w http.ResponseWriter, r *http.Request) {
return
}
- toUpdateHook := model.OutgoingWebhookFromJson(r.Body)
- if toUpdateHook == nil {
+ updatedHook := model.OutgoingWebhookFromJson(r.Body)
+ if updatedHook == nil {
c.SetInvalidParam("outgoing_webhook")
return
}
+ // The hook being updated in the payload must be the same one as indicated in the URL.
+ if updatedHook.Id != c.Params.HookId {
+ c.SetInvalidParam("hook_id")
+ return
+ }
+
c.LogAudit("attempt")
- toUpdateHook.CreatorId = c.Session.UserId
+ oldHook, err := c.App.GetOutgoingWebhook(c.Params.HookId)
+ if err != nil {
+ c.Err = err
+ return
+ }
+
+ if updatedHook.TeamId == "" {
+ updatedHook.TeamId = oldHook.TeamId
+ }
- if !c.App.SessionHasPermissionToTeam(c.Session, toUpdateHook.TeamId, model.PERMISSION_MANAGE_WEBHOOKS) {
- c.SetPermissionError(model.PERMISSION_MANAGE_WEBHOOKS)
+ if updatedHook.TeamId != oldHook.TeamId {
+ c.Err = model.NewAppError("updateOutgoingHook", "api.webhook.team_mismatch.app_error", nil, "user_id="+c.Session.UserId, http.StatusBadRequest)
return
}
- oldHook, err := c.App.GetOutgoingWebhook(toUpdateHook.Id)
- if err != nil {
- c.Err = err
+ if !c.App.SessionHasPermissionToTeam(c.Session, updatedHook.TeamId, model.PERMISSION_MANAGE_WEBHOOKS) {
+ c.SetPermissionError(model.PERMISSION_MANAGE_WEBHOOKS)
return
}
- if c.Session.UserId != oldHook.CreatorId && !c.App.SessionHasPermissionToTeam(c.Session, oldHook.TeamId, model.PERMISSION_MANAGE_OTHERS_WEBHOOKS) {
+ if c.Session.UserId != oldHook.CreatorId && !c.App.SessionHasPermissionToTeam(c.Session, updatedHook.TeamId, model.PERMISSION_MANAGE_OTHERS_WEBHOOKS) {
c.LogAudit("fail - inappropriate permissions")
c.SetPermissionError(model.PERMISSION_MANAGE_OTHERS_WEBHOOKS)
return
}
- rhook, err := c.App.UpdateOutgoingWebhook(oldHook, toUpdateHook)
+ updatedHook.CreatorId = c.Session.UserId
+
+ rhook, err := c.App.UpdateOutgoingWebhook(oldHook, updatedHook)
if err != nil {
c.Err = err
return