From 5525ee032868cf22e20327d05bdea39e40f20f16 Mon Sep 17 00:00:00 2001 From: konrad Date: Sun, 24 Mar 2019 13:17:36 +0000 Subject: [PATCH] Refactored canRead method to get the list before checking the right (#65) --- go.mod | 2 +- go.sum | 2 ++ pkg/models/label_rights.go | 1 - pkg/models/list_rights.go | 5 +++-- vendor/code.vikunja.io/web/Readme.md | 6 ++---- vendor/code.vikunja.io/web/handler/read_one.go | 13 ++++++------- vendor/modules.txt | 2 +- 7 files changed, 15 insertions(+), 16 deletions(-) diff --git a/go.mod b/go.mod index b8a66db5e6c..9b3d89bfb5f 100644 --- a/go.mod +++ b/go.mod @@ -18,7 +18,7 @@ module code.vikunja.io/api require ( cloud.google.com/go v0.34.0 // indirect - code.vikunja.io/web v0.0.0-20190324105229-0933ac082307 + code.vikunja.io/web v0.0.0-20190324123058-62b466dd1311 github.com/BurntSushi/toml v0.3.1 // indirect github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc github.com/asaskevich/govalidator v0.0.0-20180720115003-f9ffefc3facf diff --git a/go.sum b/go.sum index a7a5b05e44f..7c5627c88ee 100644 --- a/go.sum +++ b/go.sum @@ -11,6 +11,8 @@ code.vikunja.io/web v0.0.0-20190324080741-7bd881d9892a h1:nB+kG5/gq0njK9/fEtYgzv code.vikunja.io/web v0.0.0-20190324080741-7bd881d9892a/go.mod h1:PmGEu9qI7nbEKDn38H0SWgCoGO4GLdbjdlnWSzFi2PA= code.vikunja.io/web v0.0.0-20190324105229-0933ac082307 h1:t2E9v+k56RbvM5WNJF5BFFJDZrzM5l1Ua8qWdZYJAdA= code.vikunja.io/web v0.0.0-20190324105229-0933ac082307/go.mod h1:PmGEu9qI7nbEKDn38H0SWgCoGO4GLdbjdlnWSzFi2PA= +code.vikunja.io/web v0.0.0-20190324123058-62b466dd1311 h1:3VRszH3NCTNUh+8y2ImA50ALJiE1e9KNoowv9y8mzvA= +code.vikunja.io/web v0.0.0-20190324123058-62b466dd1311/go.mod h1:PmGEu9qI7nbEKDn38H0SWgCoGO4GLdbjdlnWSzFi2PA= github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= github.com/PuerkitoBio/purell v1.1.0 h1:rmGxhojJlM0tuKtfdvliR84CFHljx9ag64t2xmVkjK4= diff --git a/pkg/models/label_rights.go b/pkg/models/label_rights.go index 70fdcf4c4bc..8419b0c03b0 100644 --- a/pkg/models/label_rights.go +++ b/pkg/models/label_rights.go @@ -69,7 +69,6 @@ func (l *Label) hasAccessToLabel(a web.Auth) (bool, error) { Where("label_task.label_id != null OR labels.created_by_id = ?", u.ID). Or(builder.In("label_task.task_id", taskIDs)). And("labels.id = ?", l.ID). - GroupBy("labels.id"). Exist(&labels) return has, err } diff --git a/pkg/models/list_rights.go b/pkg/models/list_rights.go index 0a81051d53b..e1ac568bac2 100644 --- a/pkg/models/list_rights.go +++ b/pkg/models/list_rights.go @@ -45,8 +45,9 @@ func (l *List) CanRead(a web.Auth) (bool, error) { user := getUserForRights(a) // Check if the user is either owner or can read - // We can do this without first looking up the list because CanRead() is called after ReadOne() - // So are sure the list exists + if err := l.GetSimpleByID(); err != nil { + return false, err + } if l.isOwner(user) { return true, nil } diff --git a/vendor/code.vikunja.io/web/Readme.md b/vendor/code.vikunja.io/web/Readme.md index 01a4a35333f..5783050f911 100644 --- a/vendor/code.vikunja.io/web/Readme.md +++ b/vendor/code.vikunja.io/web/Readme.md @@ -102,10 +102,8 @@ type Rights interface { } ``` -When using the standard web handler, all methods except `CanRead()` are called before their `CRUD` counterparts. `CanRead()` -is called after `ReadOne()` was invoked as this would otherwise mean getting an object from the db to check if the user has the -right to see it and then getting it again if thats the case. Calling the function afterwards means we only have to get the -object once. +When using the standard web handler, all methods are called before their `CRUD` counterparts. +Use pointers for methods like `CanRead()` to get the base data of the model first, then check the right and then add addintional data. ## Handler Config diff --git a/vendor/code.vikunja.io/web/handler/read_one.go b/vendor/code.vikunja.io/web/handler/read_one.go index c548d685f18..64c64ca77a9 100644 --- a/vendor/code.vikunja.io/web/handler/read_one.go +++ b/vendor/code.vikunja.io/web/handler/read_one.go @@ -30,14 +30,7 @@ func (c *WebHandler) ReadOneWeb(ctx echo.Context) error { return echo.NewHTTPError(http.StatusBadRequest, "No or invalid model provided.") } - // Get our object - err := currentStruct.ReadOne() - if err != nil { - return HandleHTTPError(err, ctx) - } - // Check rights - // We can only check the rights on a full object, which is why we need to check it afterwards currentAuth, err := config.AuthProvider.AuthObject(ctx) if err != nil { return echo.NewHTTPError(http.StatusInternalServerError, "Could not determine the current user.") @@ -51,5 +44,11 @@ func (c *WebHandler) ReadOneWeb(ctx echo.Context) error { return echo.NewHTTPError(http.StatusForbidden, "You don't have the right to see this") } + // Get our object + err = currentStruct.ReadOne() + if err != nil { + return HandleHTTPError(err, ctx) + } + return ctx.JSON(http.StatusOK, currentStruct) } diff --git a/vendor/modules.txt b/vendor/modules.txt index 6b3e5b84443..ff2794b2489 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -1,4 +1,4 @@ -# code.vikunja.io/web v0.0.0-20190324105229-0933ac082307 +# code.vikunja.io/web v0.0.0-20190324123058-62b466dd1311 code.vikunja.io/web code.vikunja.io/web/handler # github.com/BurntSushi/toml v0.3.1