diff --git a/models/crudable.go b/models/crudable.go index 90d0bb27b..4defc7f59 100644 --- a/models/crudable.go +++ b/models/crudable.go @@ -5,6 +5,6 @@ type CRUDable interface { Create(*User, int64) error ReadOne(int64) error ReadAll(*User) (interface{}, error) - Update(int64, *User) error + Update(int64) error Delete(int64) error } diff --git a/models/list_create_update.go b/models/list_create_update.go index 17063b3ce..8e1a9c21e 100644 --- a/models/list_create_update.go +++ b/models/list_create_update.go @@ -20,25 +20,15 @@ func CreateOrUpdateList(list *List) (err error) { } // Update implements the update method of CRUDable -func (l *List) Update(id int64, doer *User) (err error) { +func (l *List) Update(id int64) (err error) { l.ID = id // Check if it exists - oldList, err := GetListByID(l.ID) + _, err = GetListByID(l.ID) if err != nil { return } - // Check rights - user, _, err := GetUserByID(doer.ID) - if err != nil { - return - } - - if !oldList.IsAdmin(&user) { - return ErrNeedToBeListAdmin{ListID: id, UserID: user.ID} - } - return CreateOrUpdateList(l) } diff --git a/models/list_items.go b/models/list_items.go index 5dd0e82e7..e98f9e8b9 100644 --- a/models/list_items.go +++ b/models/list_items.go @@ -92,10 +92,3 @@ func GetListItemByID(listItemID int64) (listItem ListItem, err error) { return } - -// CanDelete checks if the user can delete an item -func (i *ListItem) CanDelete(doer *User) (bool) { - // A user can delete an item if he has write acces to its list - list, _ := GetListByID(i.ListID) - return list.CanWrite(doer) -} diff --git a/models/list_items_create_update.go b/models/list_items_create_update.go index fb3f0e036..1b6d32f84 100644 --- a/models/list_items_create_update.go +++ b/models/list_items_create_update.go @@ -5,30 +5,24 @@ func (i *ListItem) Create(doer *User, lID int64) (err error) { i.ListID = lID i.ID = 0 - return createOrUpdateListItem(i, doer, lID) + return createOrUpdateListItem(i, doer) } // Update updates a list item -func (i *ListItem) Update(ID int64, doer *User) (err error) { +func (i *ListItem) Update(ID int64) (err error) { i.ID = ID - // Get the full item - fullItem, err := GetListItemByID(ID) + // Check if the item exists + _, err = GetListItemByID(ID) if err != nil { return } - return createOrUpdateListItem(i, doer, fullItem.ListID) + return createOrUpdateListItem(i, &User{}) } // Helper function for creation or updating of new lists as both methods share most of their logic -func createOrUpdateListItem(i *ListItem, doer *User, lID int64) (err error) { - - // Check rights - user, err := listItemPreCheck(i, doer, lID) - if err != nil { - return - } +func createOrUpdateListItem(i *ListItem, doer *User) (err error) { // Check if we have at least a text if i.Text == "" { @@ -39,6 +33,11 @@ func createOrUpdateListItem(i *ListItem, doer *User, lID int64) (err error) { if i.ID != 0 { _, err = x.ID(i.ID).Update(i) } else { + user, _, err := GetUserByID(doer.ID) + if err != nil { + return err + } + i.CreatedByID = user.ID i.CreatedBy = user _, err = x.Insert(i) @@ -46,25 +45,3 @@ func createOrUpdateListItem(i *ListItem, doer *User, lID int64) (err error) { return } - -// This helper function checks if the user has the right to edit a list item. -// It is used in Create/Update/Delete. -func listItemPreCheck(i *ListItem, doer *User, lID int64) (user User, err error) { - // Check rights - user, _, err = GetUserByID(doer.ID) - if err != nil { - return - } - - // Get the list to check if the user has the right to write to that list - list, err := GetListByID(lID) // TODO: Get the list with one query by item ID - if err != nil { - return - } - - if !list.CanWrite(&user) { - return user, ErrNeedToBeListWriter{ListID: i.ListID, UserID: user.ID} - } - - return -} diff --git a/models/list_items_rights.go b/models/list_items_rights.go new file mode 100644 index 000000000..c8bb7ffe2 --- /dev/null +++ b/models/list_items_rights.go @@ -0,0 +1,18 @@ +package models + +// CanDelete checks if the user can delete an item +func (i *ListItem) CanDelete(doer *User) bool { + // A user can delete an item if he has write acces to its list + list, _ := GetListByID(i.ListID) + return list.CanWrite(doer) +} + +// CanUpdate determines if a user has the right to update a list item +func (i *ListItem) CanUpdate(doer *User, id int64) bool { + // Get the item + lI, _ := GetListItemByID(id) + + // A user can update an item if he has write acces to its list + list, _ := GetListByID(lI.ListID) + return list.CanWrite(doer) +} diff --git a/models/list_rights.go b/models/list_rights.go index 99a4d30f2..8c1697697 100644 --- a/models/list_rights.go +++ b/models/list_rights.go @@ -81,6 +81,12 @@ func (l *List) CanRead(user *User) bool { } // CanDelete checks if the user can delete a list -func (l *List) CanDelete(doer *User) (bool) { +func (l *List) CanDelete(doer *User) bool { return l.IsAdmin(doer) -} \ No newline at end of file +} + +// CanUpdate checks if the user can update a list +func (l *List) CanUpdate(doer *User, id int64) bool { + list, _ := GetListByID(id) + return list.CanWrite(doer) +} diff --git a/models/namespaces_add_update.go b/models/namespaces_add_update.go index eb0eeee16..102294cf4 100644 --- a/models/namespaces_add_update.go +++ b/models/namespaces_add_update.go @@ -20,10 +20,10 @@ func (n *Namespace) Create(doer *User, _ int64) (err error) { } // Update implements the update method via the interface -func (n *Namespace) Update(id int64, doer *User) (err error) { +func (n *Namespace) Update(id int64) (err error) { // Check if we have at least a name if n.Name == "" { - return ErrNamespaceNameCannotBeEmpty{NamespaceID: id, UserID: doer.ID} + return ErrNamespaceNameCannotBeEmpty{NamespaceID: id} } n.ID = id @@ -35,22 +35,12 @@ func (n *Namespace) Update(id int64, doer *User) (err error) { // Check if the (new) owner exists if currentNamespace.OwnerID != n.OwnerID { - n.Owner, _, err = GetUserByID(doer.ID) + n.Owner, _, err = GetUserByID(n.OwnerID) if err != nil { return } } - // Check rights - user, _, err := GetUserByID(doer.ID) - if err != nil { - return - } - - if !currentNamespace.IsAdmin(&user) { - return ErrNeedToBeNamespaceAdmin{NamespaceID: id, UserID: doer.ID} - } - // Do the actual update _, err = x.ID(currentNamespace.ID).Update(n) return diff --git a/models/namespaces_rights.go b/models/namespaces_rights.go index a9ac14ed9..a3224f6c4 100644 --- a/models/namespaces_rights.go +++ b/models/namespaces_rights.go @@ -81,3 +81,9 @@ func (n *Namespace) CanRead(user *User) bool { return exists } + +// CanUpdate checks if the user can update the namespace +func (n *Namespace) CanUpdate(user *User, id int64) bool { + nn, _ := GetNamespaceByID(id) + return nn.IsAdmin(user) +} diff --git a/models/rights.go b/models/rights.go index d38412f45..70149cfb3 100644 --- a/models/rights.go +++ b/models/rights.go @@ -6,4 +6,5 @@ type Rights interface { CanWrite(*User) bool CanRead(*User) bool CanDelete(*User) bool + CanUpdate(*User, int64) bool } diff --git a/routes/crud/update.go b/routes/crud/update.go index 042489a99..c4bca31de 100644 --- a/routes/crud/update.go +++ b/routes/crud/update.go @@ -24,9 +24,12 @@ func (c *WebHandler) UpdateWeb(ctx echo.Context) error { if err != nil { return echo.NewHTTPError(http.StatusInternalServerError, "Could not determine the current user.") } + if !c.CObject.CanUpdate(¤tUser, id) { + return echo.NewHTTPError(http.StatusForbidden) + } // Do the update - err = c.CObject.Update(id, ¤tUser) + err = c.CObject.Update(id) if err != nil { if models.IsErrNeedToBeListAdmin(err) { return echo.NewHTTPError(http.StatusForbidden, "You need to be list admin to do that.")