Refactored canRead method to get the list before checking the right (#65)

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

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=

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
 }

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()
+	if err := l.GetSimpleByID(); err != nil {
-	// So are sure the list exists
+		return false, err
+	}
 	if l.isOwner(user) {
 		return true, nil
 	}

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()`
+When using the standard web handler, all methods are called before their `CRUD` counterparts.
-is called after `ReadOne()` was invoked as this would otherwise mean getting an object from the db to check if the user has the
+Use pointers for methods like `CanRead()` to get the base data of the model first, then check the right and then add addintional data.
-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.
 
 ## Handler Config
 

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)
 }

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