From be18247682f3ce348e6f06c64320585ecc5b7cbe Mon Sep 17 00:00:00 2001 From: konrad Date: Tue, 12 Jun 2018 18:35:36 +0200 Subject: [PATCH] Added check to only let a user delete his own list --- Featurecreep.md | 2 ++ models/error.go | 16 ++++++++++++++++ models/list_items.go | 30 +++++++++++++++++++++++++++--- routes/api/v1/item_delete.go | 12 +++++++++++- routes/api/v1/lists_list.go | 5 ----- 5 files changed, 56 insertions(+), 9 deletions(-) diff --git a/Featurecreep.md b/Featurecreep.md index 21dbe6eef45..e48eef38522 100644 --- a/Featurecreep.md +++ b/Featurecreep.md @@ -40,6 +40,8 @@ Ab v0.3 können wir mit clients anfangen. * [ ] Bearbeiten (abhaken) * [x] Löschen +* [ ] Überall nochmal überprüfen dass der Nutzer auch das Recht hat die Liste zu löschen + * [ ] Swaggerdocs !!!! #### v0.2 diff --git a/models/error.go b/models/error.go index f560470d252..d46396ee790 100644 --- a/models/error.go +++ b/models/error.go @@ -155,3 +155,19 @@ func IsErrListItemCannotBeEmpty(err error) bool { func (err ErrListItemCannotBeEmpty) Error() string { return fmt.Sprintf("List item text cannot be empty.") } + +// ErrListItemCannotBeEmpty represents a "ErrListDoesNotExist" kind of error. Used if the list does not exist. +type ErrListItemDoesNotExist struct{ + ID int64 +} + +// IsErrListItemCannotBeEmpty checks if an error is a ErrListDoesNotExist. +func IsErrListItemDoesNotExist(err error) bool { + _, ok := err.(ErrListItemDoesNotExist) + return ok +} + +func (err ErrListItemDoesNotExist) Error() string { + return fmt.Sprintf("List item does not exist. [ID: %d]", err.ID) +} + diff --git a/models/list_items.go b/models/list_items.go index 597c7ead681..9445598aa0f 100644 --- a/models/list_items.go +++ b/models/list_items.go @@ -65,9 +65,33 @@ func GetItemsByListID(listID int64) (items []*ListItem, err error) { return } -// DeleteListItemByID deletes a list item by its ID -func DeleteListItemByIDtemByID(itemID int64) (err error) { - _, err = x.ID(itemID).Delete(ListItem{}) +func GetListItemByID(listItemID int64) (listItem ListItem, err error) { + exists, err := x.ID(listItemID).Get(&listItem) + if err != nil { + return ListItem{}, err + } + + if !exists { + return ListItem{}, ErrListItemDoesNotExist{listItemID} + } return } + +// DeleteListItemByID deletes a list item by its ID +func DeleteListItemByID(itemID int64, doer *User) (err error) { + + // Check if it exists + listitem, err := GetListItemByID(itemID) + if err != nil { + return + } + + // Check if the user hat the right to delete that item + if listitem.CreatedByID != doer.ID { + return + } + + _, err = x.ID(itemID).Delete(ListItem{}) + return +} diff --git a/routes/api/v1/item_delete.go b/routes/api/v1/item_delete.go index 04bef7943ca..f1044c72029 100644 --- a/routes/api/v1/item_delete.go +++ b/routes/api/v1/item_delete.go @@ -16,10 +16,20 @@ func DeleteListItemByIDtemByID(c echo.Context) error { return c.JSON(http.StatusBadRequest, models.Message{"Invalid ID."}) } - err = models.DeleteListItemByIDtemByID(itemID) + // Check if the user has the right to delete that list + user, err := models.GetCurrentUser(c) if err != nil { return c.JSON(http.StatusInternalServerError, models.Message{"An error occured."}) } + err = models.DeleteListItemByID(itemID, &user) + if err != nil { + if models.IsErrListItemDoesNotExist(err) { + return c.JSON(http.StatusNotFound, models.Message{"List item does not exist."}) + } + + return c.JSON(http.StatusInternalServerError, models.Message{"An error occured."}) + } + return c.JSON(http.StatusOK, models.Message{"The item was deleted with success."}) } \ No newline at end of file diff --git a/routes/api/v1/lists_list.go b/routes/api/v1/lists_list.go index 62407d93466..2789a8e557a 100644 --- a/routes/api/v1/lists_list.go +++ b/routes/api/v1/lists_list.go @@ -16,11 +16,6 @@ func GetListsByUser(c echo.Context) error { allLists, err := models.GetListsByUser(¤tUser) if err != nil { - - if models.IsErrListDoesNotExist(err) { - - } - return c.JSON(http.StatusInternalServerError, models.Message{"Could not get lists."}) }