From b1de837c4f34a9659edc38e872a2a16965f7da85 Mon Sep 17 00:00:00 2001 From: kolaente Date: Mon, 9 Jul 2018 23:17:19 +0200 Subject: [PATCH] Implemented Create and Update methods on list --- Featurecreep.md | 5 +++ models/crudable.go | 4 +- models/error.go | 11 ++--- models/list_create_update.go | 55 +++++++++++++++++++------ models/list_delete.go | 2 +- models/list_items_create_update.go | 2 - models/lists.go | 17 ++++++++ models/namespaces.go | 11 +++++ models/rights.go | 2 + routes/CRUD/CRUD_helper.go | 66 ++++++++++++++++++++++++++++-- routes/api/v1/list_delete.go | 2 +- routes/routes.go | 4 +- 12 files changed, 152 insertions(+), 29 deletions(-) diff --git a/Featurecreep.md b/Featurecreep.md index 6ffbdc001..f1361b5c8 100644 --- a/Featurecreep.md +++ b/Featurecreep.md @@ -145,6 +145,11 @@ Teams sind global, d.h. Ein Team kann mehrere Namespaces verwalten. ## Anderes +* [ ] Refactor!!!! Alle Funktionen raus, die nicht mehr grbaucht werden + Funktionen vereinfachen/zusammenführen. +Wenn ein Objekt 5x hin und hergereicht wird, und jedesmal nur geringfügig was dran geändert wird sollte das +doch auch in einer Funktion machbar sein. + * [ ] ganz viel in eigene neue Dateien + Packages auslagern, am besten eine package pro model mit allen methoden etc. + * [ ] Bessere Lösung der Rechteüberprüfung überlegen? * [ ] CI aufsetzen * [ ] Tests schreiben * [ ] Namen finden diff --git a/models/crudable.go b/models/crudable.go index d05865386..6b6a29667 100644 --- a/models/crudable.go +++ b/models/crudable.go @@ -1,9 +1,9 @@ package models type CRUDable interface { - Create() + Create(*User) (error) ReadOne(int64) error ReadAll(*User) (interface{}, error) - Update() + Update(int64, *User) error Delete() } diff --git a/models/error.go b/models/error.go index 058b1f227..584df6fb9 100644 --- a/models/error.go +++ b/models/error.go @@ -143,19 +143,19 @@ func (err ErrListDoesNotExist) Error() string { return fmt.Sprintf("List does not exist [ID: %d]", err.ID) } -// ErrNeedToBeListOwner represents an error, where the user is not the owner of that list (used i.e. when deleting a list) -type ErrNeedToBeListOwner struct { +// ErrNeedToBeListAdmin represents an error, where the user is not the owner of that list (used i.e. when deleting a list) +type ErrNeedToBeListAdmin struct { ListID int64 UserID int64 } // IsErrListDoesNotExist checks if an error is a ErrListDoesNotExist. -func IsErrNeedToBeListOwner(err error) bool { - _, ok := err.(ErrNeedToBeListOwner) +func IsErrNeedToBeListAdmin(err error) bool { + _, ok := err.(ErrNeedToBeListAdmin) return ok } -func (err ErrNeedToBeListOwner) Error() string { +func (err ErrNeedToBeListAdmin) Error() string { return fmt.Sprintf("You need to be list owner to do that [ListID: %d, UserID: %d]", err.ListID, err.UserID) } @@ -289,3 +289,4 @@ func IsErrUserDoesNotHaveWriteAccessToNamespace(err error) bool { func (err ErrUserDoesNotHaveWriteAccessToNamespace) Error() string { return fmt.Sprintf("You need to have write access to this namespace to do that [NamespaceID: %d, UserID: %d]", err.NamespaceID, err.UserID) } + diff --git a/models/list_create_update.go b/models/list_create_update.go index 9f3138e33..5d1ba87cb 100644 --- a/models/list_create_update.go +++ b/models/list_create_update.go @@ -2,18 +2,6 @@ package models // CreateOrUpdateList updates a list or creates it if it doesn't exist func CreateOrUpdateList(list *List) (err error) { - // Check if it exists - _, err = GetListByID(list.ID) - if err != nil { - return - } - - // Check if the user exists - list.Owner, _, err = GetUserByID(list.Owner.ID) - if err != nil { - return - } - list.OwnerID = list.Owner.ID if list.ID == 0 { _, err = x.Insert(list) @@ -30,3 +18,46 @@ func CreateOrUpdateList(list *List) (err error) { return } + +func (l *List) Update(id int64, doer *User) (err error) { + l.ID = id + + // Check if it exists + oldList, 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) +} + +func (l *List) Create(doer *User) (err error) { + // Check rights + user, _, err := GetUserByID(doer.ID) + if err != nil { + return + } + + // Get the namespace of the list to check if the user can write to it + namespace, err := GetNamespaceByID(l.NamespaceID) + if err != nil { + return + } + if !namespace.CanWrite(doer) { + return ErrUserDoesNotHaveWriteAccessToNamespace{UserID:user.ID, NamespaceID:namespace.ID} + } + + l.Owner.ID = user.ID + + return CreateOrUpdateList(l) +} \ No newline at end of file diff --git a/models/list_delete.go b/models/list_delete.go index f1034b422..b063b0404 100644 --- a/models/list_delete.go +++ b/models/list_delete.go @@ -9,7 +9,7 @@ func DeleteListByID(listID int64, doer *User) (err error) { } if list.Owner.ID != doer.ID { - return ErrNeedToBeListOwner{ListID: listID, UserID: doer.ID} + return ErrNeedToBeListAdmin{ListID: listID, UserID: doer.ID} } // Delete the list diff --git a/models/list_items_create_update.go b/models/list_items_create_update.go index e9f36eea8..3c082a5ec 100644 --- a/models/list_items_create_update.go +++ b/models/list_items_create_update.go @@ -16,8 +16,6 @@ func CreateOrUpdateListItem(item *ListItem) (newItem *ListItem, err error) { } item.CreatedByID = item.CreatedBy.ID - // TODO: Check if the user has the right to add/update an item to that list - if item.ID != 0 { _, err = x.ID(item.ID).Update(item) if err != nil { diff --git a/models/lists.go b/models/lists.go index 3145c4b1a..14f454257 100644 --- a/models/lists.go +++ b/models/lists.go @@ -15,6 +15,7 @@ type List struct { Updated int64 `xorm:"updated" json:"updated"` CRUDable `xorm:"-" json:"-"` + Rights `xorm:"-" json:"-"` } // Lists is a multiple of list @@ -73,3 +74,19 @@ func (l *List) ReadOne(id int64) (err error) { *l, err = GetListByID(id) return } + +func (l *List) IsAdmin(user *User) bool { + // Owners are always admins + if l.Owner.ID == user.ID { + return true + } + + // Check Team rights + // aka "is the user in a team which has admin rights?" + // TODO + + // Check Namespace rights + // TODO + + return false +} \ No newline at end of file diff --git a/models/namespaces.go b/models/namespaces.go index 561efc7d9..f99515fe8 100644 --- a/models/namespaces.go +++ b/models/namespaces.go @@ -11,6 +11,9 @@ type Namespace struct { Created int64 `xorm:"created" json:"created"` Updated int64 `xorm:"updated" json:"updated"` + + CRUDable `xorm:"-" json:"-"` + Rights `xorm:"-" json:"-"` } // TableName makes beautiful table names @@ -58,6 +61,14 @@ func (user *User) HasNamespaceAccess(namespace *Namespace) (err error) { return ErrUserDoesNotHaveAccessToNamespace{UserID: user.ID, NamespaceID: namespace.ID} } +func (n *Namespace) CanWrite(user *User) bool { + if err := user.HasNamespaceAccess(n); err != nil { + return false + } + + return true +} + func (user *User) HasNamespaceWriteAccess(namespace *Namespace) (err error) { // Owners always have access diff --git a/models/rights.go b/models/rights.go index 208314371..5f40f164c 100644 --- a/models/rights.go +++ b/models/rights.go @@ -2,4 +2,6 @@ package models type Rights interface { IsAdmin(*User) bool + CanWrite(*User) bool + CanRead(*User) bool } diff --git a/routes/CRUD/CRUD_helper.go b/routes/CRUD/CRUD_helper.go index 340ae7303..7154eaca5 100644 --- a/routes/CRUD/CRUD_helper.go +++ b/routes/CRUD/CRUD_helper.go @@ -5,19 +5,21 @@ import ( "git.kolaente.de/konrad/list/models" "github.com/labstack/echo" "net/http" - "strconv" ) // This does web stuff, aka returns json etc. Uses CRUDable Methods to get the data type CRUDWebHandler struct { - CObject interface{ models.CRUDable } + CObject interface{ + models.CRUDable + models.Rights + } } // This does json, handles the request func (c *CRUDWebHandler) ReadOneWeb(ctx echo.Context) error { // Get the ID - id, err := strconv.ParseInt(ctx.Param("id"), 10, 64) + id, err := models.GetIntURLParam("id", ctx) if err != nil { return ctx.JSON(http.StatusBadRequest, models.Message{"Invalid ID."}) } @@ -37,7 +39,7 @@ func (c *CRUDWebHandler) ReadOneWeb(ctx echo.Context) error { return ctx.JSON(http.StatusOK, c.CObject) } -// +// ReadAllWeb returns all elements of a type func (c *CRUDWebHandler) ReadAllWeb(ctx echo.Context) error { currentUser, err := models.GetCurrentUser(ctx) if err != nil { @@ -52,3 +54,59 @@ func (c *CRUDWebHandler) ReadAllWeb(ctx echo.Context) error { return ctx.JSON(http.StatusOK, lists) } + +// UpdateWeb is the webhandler to update an object +func (c *CRUDWebHandler) UpdateWeb(ctx echo.Context) error { + // Get the object + if err := ctx.Bind(&c.CObject); err != nil { + return ctx.JSON(http.StatusBadRequest, models.Message{"No model provided."}) + } + + // Get the ID + var err error + id, err := models.GetIntURLParam("id", ctx) + if err != nil { + return ctx.JSON(http.StatusBadRequest, models.Message{"Invalid ID."}) + } + + // Check if the user has the right to do that + currentUser, err := models.GetCurrentUser(ctx) + if err != nil { + return ctx.JSON(http.StatusInternalServerError, models.Message{"Could not determine the current user."}) + } + + // Do the update + err = c.CObject.Update(id, ¤tUser) + if err != nil { + if models.IsErrNeedToBeListAdmin(err) { + return echo.NewHTTPError(http.StatusForbidden, "You need to be list admin to do that.") + } + + return echo.NewHTTPError(http.StatusInternalServerError) + } + + return ctx.JSON(http.StatusOK, c.CObject) +} + +// CreateWeb is the handler to create an object +func (c *CRUDWebHandler) CreateWeb(ctx echo.Context) error { + // Get the object + if err := ctx.Bind(&c.CObject); err != nil { + return ctx.JSON(http.StatusBadRequest, models.Message{"No model provided."}) + } + + // Get the user to pass for later checks + currentUser, err := models.GetCurrentUser(ctx) + if err != nil { + return echo.NewHTTPError(http.StatusInternalServerError, "Could not determine the current user.") + } + + // Create + err = c.CObject.Create(¤tUser) + if err != nil { + fmt.Println(err) + return echo.NewHTTPError(http.StatusInternalServerError) + } + + return ctx.JSON(http.StatusOK, c.CObject) +} \ No newline at end of file diff --git a/routes/api/v1/list_delete.go b/routes/api/v1/list_delete.go index 691bb9ab1..f0b259a41 100644 --- a/routes/api/v1/list_delete.go +++ b/routes/api/v1/list_delete.go @@ -49,7 +49,7 @@ func DeleteListByID(c echo.Context) error { err = models.DeleteListByID(itemID, &user) if err != nil { - if models.IsErrNeedToBeListOwner(err) { + if models.IsErrNeedToBeListAdmin(err) { return c.JSON(http.StatusForbidden, models.Message{"You need to be the list owner to delete a list."}) } diff --git a/routes/routes.go b/routes/routes.go index 118b82d20..85ad81bd9 100644 --- a/routes/routes.go +++ b/routes/routes.go @@ -92,7 +92,7 @@ func RegisterRoutes(e *echo.Echo) { } a.GET("/lists", listHandler.ReadAllWeb) a.GET("/lists/:id", listHandler.ReadOneWeb) - a.POST("/lists/:id", apiv1.UpdateList) + a.POST("/lists/:id", listHandler.UpdateWeb) a.PUT("/lists/:id", apiv1.AddListItem) a.DELETE("/lists/:id", apiv1.DeleteListByID) @@ -105,5 +105,5 @@ func RegisterRoutes(e *echo.Echo) { a.POST("/namespaces/:id", apiv1.UpdateNamespace) a.DELETE("/namespaces/:id", apiv1.DeleteNamespaceByID) a.GET("/namespaces/:id/lists", apiv1.GetListsByNamespaceID) - a.PUT("/namespaces/:id/lists", apiv1.AddList) + a.PUT("/namespaces/:id/lists", listHandler.CreateWeb) }