1
0
mirror of https://github.com/mattermost/focalboard.git synced 2025-07-15 23:54:29 +02:00

Refactor error usage from the store level up and add API helpers (#3792)

* Refactor error usage from the store level up and add API helpers

* Complete API tests

* Fix merge errorResponse calls

* Remove ensure helpers to allow for custom messages on permission errors

* Fix bad import and call

* Remove bad user check on auth that was added as part of the main merge

* Fix empty list test

* Replace deprecated proxy calls to ioutil.ReadAll with io.ReadAll

* Add information to the NotFound errors

* Add context to all remaining errors and address review comments

* Fix linter

* Adapt the new card API endpoints to the error refactor

* Remove almost all customErrorResponse calls

* Add request entity too large to errorResponse and remove customErrorResponse

* Fix linter
This commit is contained in:
Miguel de la Cruz
2022-09-13 12:18:40 +02:00
committed by GitHub
parent ed655ac996
commit 08c0b7a2fd
68 changed files with 1349 additions and 922 deletions

View File

@ -2,13 +2,11 @@ package api
import (
"encoding/json"
"errors"
"fmt"
"io/ioutil"
"io"
"net/http"
"github.com/gorilla/mux"
"github.com/mattermost/focalboard/server/app"
"github.com/mattermost/focalboard/server/model"
"github.com/mattermost/focalboard/server/services/audit"
@ -51,22 +49,20 @@ func (a *API) handleCreateBoardsAndBlocks(w http.ResponseWriter, r *http.Request
userID := getUserID(r)
requestBody, err := ioutil.ReadAll(r.Body)
requestBody, err := io.ReadAll(r.Body)
if err != nil {
a.errorResponse(w, r.URL.Path, http.StatusInternalServerError, "", err)
a.errorResponse(w, r, err)
return
}
var newBab *model.BoardsAndBlocks
if err = json.Unmarshal(requestBody, &newBab); err != nil {
// a.errorResponse(w, r.URL.Path, http.StatusBadRequest, "", err)
a.errorResponse(w, r.URL.Path, http.StatusBadRequest, "", err)
a.errorResponse(w, r, err)
return
}
if len(newBab.Boards) == 0 {
message := "at least one board is required"
a.errorResponse(w, r.URL.Path, http.StatusBadRequest, message, nil)
a.errorResponse(w, r, model.NewErrBadRequest("at least one board is required"))
return
}
@ -81,30 +77,28 @@ func (a *API) handleCreateBoardsAndBlocks(w http.ResponseWriter, r *http.Request
}
if teamID != board.TeamID {
message := "cannot create boards for multiple teams"
a.errorResponse(w, r.URL.Path, http.StatusBadRequest, message, nil)
a.errorResponse(w, r, model.NewErrBadRequest("cannot create boards for multiple teams"))
return
}
if board.ID == "" {
message := "boards need an ID to be referenced from the blocks"
a.errorResponse(w, r.URL.Path, http.StatusBadRequest, message, nil)
a.errorResponse(w, r, model.NewErrBadRequest("boards need an ID to be referenced from the blocks"))
return
}
}
if !a.permissions.HasPermissionToTeam(userID, teamID, model.PermissionViewTeam) {
a.errorResponse(w, r.URL.Path, http.StatusForbidden, "", PermissionError{"access denied to board template"})
a.errorResponse(w, r, model.NewErrPermission("access denied to board template"))
return
}
isGuest, err := a.userIsGuest(userID)
if err != nil {
a.errorResponse(w, r.URL.Path, http.StatusInternalServerError, "", err)
a.errorResponse(w, r, err)
return
}
if isGuest {
a.errorResponse(w, r.URL.Path, http.StatusForbidden, "", PermissionError{"access denied to create board"})
a.errorResponse(w, r, model.NewErrPermission("access denied to create board"))
return
}
@ -112,25 +106,25 @@ func (a *API) handleCreateBoardsAndBlocks(w http.ResponseWriter, r *http.Request
// Error checking
if len(block.Type) < 1 {
message := fmt.Sprintf("missing type for block id %s", block.ID)
a.errorResponse(w, r.URL.Path, http.StatusBadRequest, message, nil)
a.errorResponse(w, r, model.NewErrBadRequest(message))
return
}
if block.CreateAt < 1 {
message := fmt.Sprintf("invalid createAt for block id %s", block.ID)
a.errorResponse(w, r.URL.Path, http.StatusBadRequest, message, nil)
a.errorResponse(w, r, model.NewErrBadRequest(message))
return
}
if block.UpdateAt < 1 {
message := fmt.Sprintf("invalid UpdateAt for block id %s", block.ID)
a.errorResponse(w, r.URL.Path, http.StatusBadRequest, message, nil)
a.errorResponse(w, r, model.NewErrBadRequest(message))
return
}
if !boardIDs[block.BoardID] {
message := fmt.Sprintf("invalid BoardID %s (not exists in the created boards)", block.BoardID)
a.errorResponse(w, r.URL.Path, http.StatusBadRequest, message, nil)
a.errorResponse(w, r, model.NewErrBadRequest(message))
return
}
}
@ -139,7 +133,7 @@ func (a *API) handleCreateBoardsAndBlocks(w http.ResponseWriter, r *http.Request
// linked and then regenerated by the server
newBab, err = model.GenerateBoardsAndBlocksIDs(newBab, a.logger)
if err != nil {
a.errorResponse(w, r.URL.Path, http.StatusBadRequest, err.Error(), err)
a.errorResponse(w, r, model.NewErrBadRequest(err.Error()))
return
}
@ -153,7 +147,7 @@ func (a *API) handleCreateBoardsAndBlocks(w http.ResponseWriter, r *http.Request
// create boards and blocks
bab, err := a.app.CreateBoardsAndBlocks(newBab, userID, true)
if err != nil {
a.errorResponse(w, r.URL.Path, http.StatusInternalServerError, err.Error(), err)
a.errorResponse(w, r, err)
return
}
@ -166,7 +160,7 @@ func (a *API) handleCreateBoardsAndBlocks(w http.ResponseWriter, r *http.Request
data, err := json.Marshal(bab)
if err != nil {
a.errorResponse(w, r.URL.Path, http.StatusInternalServerError, err.Error(), err)
a.errorResponse(w, r, err)
return
}
@ -205,20 +199,20 @@ func (a *API) handlePatchBoardsAndBlocks(w http.ResponseWriter, r *http.Request)
userID := getUserID(r)
requestBody, err := ioutil.ReadAll(r.Body)
requestBody, err := io.ReadAll(r.Body)
if err != nil {
a.errorResponse(w, r.URL.Path, http.StatusInternalServerError, "", err)
a.errorResponse(w, r, err)
return
}
var pbab *model.PatchBoardsAndBlocks
if err = json.Unmarshal(requestBody, &pbab); err != nil {
a.errorResponse(w, r.URL.Path, http.StatusBadRequest, "", err)
a.errorResponse(w, r, err)
return
}
if err = pbab.IsValid(); err != nil {
a.errorResponse(w, r.URL.Path, http.StatusBadRequest, "", err)
a.errorResponse(w, r, model.NewErrBadRequest(err.Error()))
return
}
@ -229,29 +223,25 @@ func (a *API) handlePatchBoardsAndBlocks(w http.ResponseWriter, r *http.Request)
patch := pbab.BoardPatches[i]
if err = patch.IsValid(); err != nil {
a.errorResponse(w, r.URL.Path, http.StatusBadRequest, "", err)
a.errorResponse(w, r, model.NewErrBadRequest(err.Error()))
return
}
if !a.permissions.HasPermissionToBoard(userID, boardID, model.PermissionManageBoardProperties) {
a.errorResponse(w, r.URL.Path, http.StatusForbidden, "", PermissionError{"access denied to modifying board properties"})
a.errorResponse(w, r, model.NewErrPermission("access denied to modifying board properties"))
return
}
if patch.Type != nil || patch.MinimumRole != nil {
if !a.permissions.HasPermissionToBoard(userID, boardID, model.PermissionManageBoardType) {
a.errorResponse(w, r.URL.Path, http.StatusForbidden, "", PermissionError{"access denied to modifying board type"})
a.errorResponse(w, r, model.NewErrPermission("access denied to modifying board type"))
return
}
}
board, err2 := a.app.GetBoard(boardID)
if err2 != nil {
a.errorResponse(w, r.URL.Path, http.StatusInternalServerError, "", err2)
return
}
if board == nil {
a.errorResponse(w, r.URL.Path, http.StatusBadRequest, "", nil)
a.errorResponse(w, r, err2)
return
}
@ -259,7 +249,7 @@ func (a *API) handlePatchBoardsAndBlocks(w http.ResponseWriter, r *http.Request)
teamID = board.TeamID
}
if teamID != board.TeamID {
a.errorResponse(w, r.URL.Path, http.StatusBadRequest, "", nil)
a.errorResponse(w, r, model.NewErrBadRequest("mismatched team ID"))
return
}
}
@ -267,21 +257,17 @@ func (a *API) handlePatchBoardsAndBlocks(w http.ResponseWriter, r *http.Request)
for _, blockID := range pbab.BlockIDs {
block, err2 := a.app.GetBlockByID(blockID)
if err2 != nil {
a.errorResponse(w, r.URL.Path, http.StatusInternalServerError, "", err2)
return
}
if block == nil {
a.errorResponse(w, r.URL.Path, http.StatusBadRequest, "", nil)
a.errorResponse(w, r, err2)
return
}
if _, ok := boardIDMap[block.BoardID]; !ok {
a.errorResponse(w, r.URL.Path, http.StatusBadRequest, "", nil)
a.errorResponse(w, r, model.NewErrBadRequest("missing BoardID="+block.BoardID))
return
}
if !a.permissions.HasPermissionToBoard(userID, block.BoardID, model.PermissionManageBoardCards) {
a.errorResponse(w, r.URL.Path, http.StatusForbidden, "", PermissionError{"access denied to modifying cards"})
a.errorResponse(w, r, model.NewErrPermission("access denied to modifying cards"))
return
}
}
@ -292,12 +278,8 @@ func (a *API) handlePatchBoardsAndBlocks(w http.ResponseWriter, r *http.Request)
auditRec.AddMeta("blocksCount", len(pbab.BlockIDs))
bab, err := a.app.PatchBoardsAndBlocks(pbab, userID)
if errors.Is(err, app.ErrPatchUpdatesLimitedCards) {
a.errorResponse(w, r.URL.Path, http.StatusForbidden, "", err)
return
}
if err != nil {
a.errorResponse(w, r.URL.Path, http.StatusInternalServerError, "", err)
a.errorResponse(w, r, err)
return
}
@ -308,7 +290,7 @@ func (a *API) handlePatchBoardsAndBlocks(w http.ResponseWriter, r *http.Request)
data, err := json.Marshal(bab)
if err != nil {
a.errorResponse(w, r.URL.Path, http.StatusInternalServerError, "", err)
a.errorResponse(w, r, err)
return
}
@ -345,15 +327,15 @@ func (a *API) handleDeleteBoardsAndBlocks(w http.ResponseWriter, r *http.Request
userID := getUserID(r)
requestBody, err := ioutil.ReadAll(r.Body)
requestBody, err := io.ReadAll(r.Body)
if err != nil {
a.errorResponse(w, r.URL.Path, http.StatusInternalServerError, "", err)
a.errorResponse(w, r, err)
return
}
var dbab *model.DeleteBoardsAndBlocks
if err = json.Unmarshal(requestBody, &dbab); err != nil {
a.errorResponse(w, r.URL.Path, http.StatusBadRequest, "", err)
a.errorResponse(w, r, model.NewErrBadRequest(err.Error()))
return
}
@ -366,24 +348,20 @@ func (a *API) handleDeleteBoardsAndBlocks(w http.ResponseWriter, r *http.Request
// all boards in the request should belong to the same team
board, err := a.app.GetBoard(boardID)
if err != nil {
a.errorResponse(w, r.URL.Path, http.StatusInternalServerError, "", err)
return
}
if board == nil {
a.errorResponse(w, r.URL.Path, http.StatusBadRequest, "", err)
a.errorResponse(w, r, err)
return
}
if teamID == "" {
teamID = board.TeamID
}
if teamID != board.TeamID {
a.errorResponse(w, r.URL.Path, http.StatusBadRequest, "", nil)
a.errorResponse(w, r, model.NewErrBadRequest("all boards should belong to the same team"))
return
}
// permission check
if !a.permissions.HasPermissionToBoard(userID, boardID, model.PermissionDeleteBoard) {
a.errorResponse(w, r.URL.Path, http.StatusForbidden, "", PermissionError{"access denied to delete board"})
a.errorResponse(w, r, model.NewErrPermission("access denied to delete board"))
return
}
}
@ -391,27 +369,23 @@ func (a *API) handleDeleteBoardsAndBlocks(w http.ResponseWriter, r *http.Request
for _, blockID := range dbab.Blocks {
block, err2 := a.app.GetBlockByID(blockID)
if err2 != nil {
a.errorResponse(w, r.URL.Path, http.StatusInternalServerError, "", err2)
return
}
if block == nil {
a.errorResponse(w, r.URL.Path, http.StatusBadRequest, "", nil)
a.errorResponse(w, r, err2)
return
}
if _, ok := boardIDMap[block.BoardID]; !ok {
a.errorResponse(w, r.URL.Path, http.StatusBadRequest, "", nil)
a.errorResponse(w, r, model.NewErrBadRequest("missing BoardID="+block.BoardID))
return
}
if !a.permissions.HasPermissionToBoard(userID, block.BoardID, model.PermissionManageBoardCards) {
a.errorResponse(w, r.URL.Path, http.StatusForbidden, "", PermissionError{"access denied to modifying cards"})
a.errorResponse(w, r, model.NewErrPermission("access denied to modifying cards"))
return
}
}
if err := dbab.IsValid(); err != nil {
a.errorResponse(w, r.URL.Path, http.StatusBadRequest, "", err)
a.errorResponse(w, r, model.NewErrBadRequest(err.Error()))
return
}
@ -421,7 +395,7 @@ func (a *API) handleDeleteBoardsAndBlocks(w http.ResponseWriter, r *http.Request
auditRec.AddMeta("blocksCount", len(dbab.Blocks))
if err := a.app.DeleteBoardsAndBlocks(dbab, userID); err != nil {
a.errorResponse(w, r.URL.Path, http.StatusInternalServerError, "", err)
a.errorResponse(w, r, err)
return
}