Rights performance improvements for lists and namespaces (#54)

Featurecreep.md

@@ -131,8 +131,9 @@ Sorry for some of them being in German, I'll tranlate them at some point.
 ### General features
 
 * [x] Deps nach mod umziehen
-* [ ] Performance bei rechtchecks verbessern
+* [x] Performance bei rechtchecks verbessern
   * User & Teamright sollte sich für n rechte in einer Funktion testen lassen
+* [ ] Endpoint um die Rechte mit Beschreibung und code zu kriegen
 * [ ] "Smart Lists", Listen nach bestimmten Kriterien gefiltert -> nur UI?
 * [ ] "Performance-Statistik" -> Wie viele Tasks man in bestimmten Zeiträumen so geschafft hat etc
 * [ ] IMAP-Integration -> Man schickt eine email an Vikunja und es macht daraus dann nen task -> Achtung missbrauchsmöglichkeiten

REST-Tests/lists.http

@@ -5,7 +5,7 @@ Authorization: Bearer {{auth_token}}
 ###
 
 # Get one list
-GET http://localhost:8080/api/v1/lists/1172
+GET http://localhost:8080/api/v1/lists/27
 Authorization: Bearer {{auth_token}}
 
 ###

REST-Tests/namespaces.http

@@ -5,7 +5,7 @@ Authorization: Bearer {{auth_token}}
 ###
 
 # Get one namespaces
-GET http://localhost:8080/api/v1/namespaces/125476
+GET http://localhost:8080/api/v1/namespaces/-1
 Authorization: Bearer {{auth_token}}
 
 ###

docs/docs.go

@@ -1,6 +1,6 @@
 // GENERATED BY THE COMMAND ABOVE; DO NOT EDIT
 // This file was generated by swaggo/swag at
-// 2019-01-14 23:02:47.994258682 +0100 CET m=+0.170768570
+// 2019-01-18 12:59:51.359642373 +0100 CET m=+0.125821859
 
 package docs
 
@@ -14,7 +14,7 @@ import (
 var doc = `{
     "swagger": "2.0",
     "info": {
-        "description": "\u003c!-- ReDoc-Inject: \u003csecurity-definitions\u003e --\u003e",
+        "description": "This is the documentation for the [Vikunja](http://vikunja.io) API. Vikunja is a cross-plattform Todo-application with a lot of features, such as sharing lists with users or teams. \u003c!-- ReDoc-Inject: \u003csecurity-definitions\u003e --\u003e\n# Authorization\n**JWT-Auth:** Main authorization method, used for most of the requests. Needs ` + "`" + `Authorization: Bearer \u003cjwt-token\u003e` + "`" + `-header to authenticate successfully.\n\n**BasicAuth:** Only used when requesting tasks via caldav.\n\u003c!-- ReDoc-Inject: \u003csecurity-definitions\u003e --\u003e",
         "title": "Vikunja API",
         "contact": {
             "name": "General Vikunja contact",
@@ -391,7 +391,7 @@ var doc = `{
                         "JWTKeyAuth": []
                     }
                 ],
-                "description": "Returns a list by its ID.",
+                "description": "Returns a team by its ID.",
                 "consumes": [
                     "application/json"
                 ],
@@ -399,13 +399,13 @@ var doc = `{
                     "application/json"
                 ],
                 "tags": [
-                    "list"
+                    "team"
                 ],
-                "summary": "Gets one list",
+                "summary": "Gets one team",
                 "parameters": [
                     {
                         "type": "integer",
-                        "description": "List ID",
+                        "description": "Team ID",
                         "name": "id",
                         "in": "path",
                         "required": true
@@ -413,14 +413,14 @@ var doc = `{
                 ],
                 "responses": {
                     "200": {
-                        "description": "The list",
+                        "description": "The team",
                         "schema": {
                             "type": "object",
-                            "$ref": "#/definitions/models.List"
+                            "$ref": "#/definitions/models.Team"
                         }
                     },
                     "403": {
-                        "description": "The user does not have access to the list",
+                        "description": "The user does not have access to the team",
                         "schema": {
                             "type": "object",
                             "$ref": "#/definitions/code.vikunja.io.web.HTTPError"

docs/errors.md

@@ -32,14 +32,13 @@ This document describes the different errors Vikunja can return.
 | 5011 | 409 | This user has already access to that namespace. |
 | 6001 | 400 | The team name cannot be emtpy. |
 | 6002 | 404 | The team does not exist. |
-| 6003 | 400 | The provided team right is invalid. |
 | 6004 | 409 | The team already has access to that namespace or list. |
 | 6005 | 409 | The user is already a member of that team. |
 | 6006 | 400 | Cannot delete the last team member. |
 | 6007 | 403 | The team does not have access to the list to perform that action. |
-| 7001 | 400 | The user right is invalid. |
 | 7002 | 409 | The user already has access to that list. |
 | 7003 | 403 | The user does not have access to that list. |
 | 8001 | 403 | This label already exists on that task. |
 | 8002 | 404 | The label does not exist. |
 | 8003 | 403 | The user does not have access to this label. |
+| 9001 | 403 | The right is invalid. | 

docs/swagger/swagger.json

@@ -1,7 +1,7 @@
 {
     "swagger": "2.0",
     "info": {
-        "description": "\u003c!-- ReDoc-Inject: \u003csecurity-definitions\u003e --\u003e",
+        "description": "This is the documentation for the [Vikunja](http://vikunja.io) API. Vikunja is a cross-plattform Todo-application with a lot of features, such as sharing lists with users or teams. \u003c!-- ReDoc-Inject: \u003csecurity-definitions\u003e --\u003e\n# Authorization\n**JWT-Auth:** Main authorization method, used for most of the requests. Needs ` + \"`\" + `Authorization: Bearer \u003cjwt-token\u003e` + \"`\" + `-header to authenticate successfully.\n\n**BasicAuth:** Only used when requesting tasks via caldav.\n\u003c!-- ReDoc-Inject: \u003csecurity-definitions\u003e --\u003e",
         "title": "Vikunja API",
         "contact": {
             "name": "General Vikunja contact",
@@ -378,7 +378,7 @@
                         "JWTKeyAuth": []
                     }
                 ],
-                "description": "Returns a list by its ID.",
+                "description": "Returns a team by its ID.",
                 "consumes": [
                     "application/json"
                 ],
@@ -386,13 +386,13 @@
                     "application/json"
                 ],
                 "tags": [
-                    "list"
+                    "team"
                 ],
-                "summary": "Gets one list",
+                "summary": "Gets one team",
                 "parameters": [
                     {
                         "type": "integer",
-                        "description": "List ID",
+                        "description": "Team ID",
                         "name": "id",
                         "in": "path",
                         "required": true
@@ -400,14 +400,14 @@
                 ],
                 "responses": {
                     "200": {
-                        "description": "The list",
+                        "description": "The team",
                         "schema": {
                             "type": "object",
-                            "$ref": "#/definitions/models.List"
+                            "$ref": "#/definitions/models.Team"
                         }
                     },
                     "403": {
-                        "description": "The user does not have access to the list",
+                        "description": "The user does not have access to the team",
                         "schema": {
                             "type": "object",
                             "$ref": "#/definitions/code.vikunja.io/web.HTTPError"

docs/swagger/swagger.yaml

@@ -622,7 +622,13 @@ info:
     email: hello@vikunja.io
     name: General Vikunja contact
     url: http://vikunja.io/en/contact/
-  description: '<!-- ReDoc-Inject: <security-definitions> -->'
+  description: |-
+    This is the documentation for the [Vikunja](http://vikunja.io) API. Vikunja is a cross-plattform Todo-application with a lot of features, such as sharing lists with users or teams. <!-- ReDoc-Inject: <security-definitions> -->
+    # Authorization
+    **JWT-Auth:** Main authorization method, used for most of the requests. Needs ` + "`" + `Authorization: Bearer <jwt-token>` + "`" + `-header to authenticate successfully.
+
+    **BasicAuth:** Only used when requesting tasks via caldav.
+    <!-- ReDoc-Inject: <security-definitions> -->
   license:
     name: GPLv3
     url: http://code.vikunja.io/api/src/branch/master/LICENSE
@@ -906,9 +912,9 @@ paths:
     get:
       consumes:
       - application/json
-      description: Returns a list by its ID.
+      description: Returns a team by its ID.
       parameters:
-      - description: List ID
+      - description: Team ID
         in: path
         name: id
         required: true
@@ -917,12 +923,12 @@ paths:
       - application/json
       responses:
         "200":
-          description: The list
+          description: The team
           schema:
-            $ref: '#/definitions/models.List'
+            $ref: '#/definitions/models.Team'
             type: object
         "403":
-          description: The user does not have access to the list
+          description: The user does not have access to the team
           schema:
             $ref: '#/definitions/code.vikunja.io/web.HTTPError'
             type: object
@@ -933,9 +939,9 @@ paths:
             type: object
       security:
       - JWTKeyAuth: []
-      summary: Gets one list
+      summary: Gets one team
       tags:
-      - list
+      - team
     post:
       consumes:
       - application/json

pkg/models/error.go

@@ -699,29 +699,6 @@ func (err ErrTeamDoesNotExist) HTTPError() web.HTTPError {
 	return web.HTTPError{HTTPCode: http.StatusNotFound, Code: ErrCodeTeamDoesNotExist, Message: "This team does not exist."}
 }
 
-// ErrInvalidTeamRight represents an error where a team right is invalid
-type ErrInvalidTeamRight struct {
-	Right TeamRight
-}
-
-// IsErrInvalidTeamRight checks if an error is ErrInvalidTeamRight.
-func IsErrInvalidTeamRight(err error) bool {
-	_, ok := err.(ErrInvalidTeamRight)
-	return ok
-}
-
-func (err ErrInvalidTeamRight) Error() string {
-	return fmt.Sprintf("Team right invalid [Right: %d]", err.Right)
-}
-
-// ErrCodeInvalidTeamRight holds the unique world-error code of this error
-const ErrCodeInvalidTeamRight = 6003
-
-// HTTPError holds the http error description
-func (err ErrInvalidTeamRight) HTTPError() web.HTTPError {
-	return web.HTTPError{HTTPCode: http.StatusBadRequest, Code: ErrCodeInvalidTeamRight, Message: "The team right is invalid."}
-}
-
 // ErrTeamAlreadyHasAccess represents an error where a team already has access to a list/namespace
 type ErrTeamAlreadyHasAccess struct {
 	TeamID int64
@@ -822,29 +799,6 @@ func (err ErrTeamDoesNotHaveAccessToList) HTTPError() web.HTTPError {
 // User <-> List errors
 // ====================
 
-// ErrInvalidUserRight represents an error where a user right is invalid
-type ErrInvalidUserRight struct {
-	Right UserRight
-}
-
-// IsErrInvalidUserRight checks if an error is ErrInvalidUserRight.
-func IsErrInvalidUserRight(err error) bool {
-	_, ok := err.(ErrInvalidUserRight)
-	return ok
-}
-
-func (err ErrInvalidUserRight) Error() string {
-	return fmt.Sprintf("User right is invalid [Right: %d]", err.Right)
-}
-
-// ErrCodeInvalidUserRight holds the unique world-error code of this error
-const ErrCodeInvalidUserRight = 7001
-
-// HTTPError holds the http error description
-func (err ErrInvalidUserRight) HTTPError() web.HTTPError {
-	return web.HTTPError{HTTPCode: http.StatusBadRequest, Code: ErrCodeInvalidUserRight, Message: "The user right is invalid."}
-}
-
 // ErrUserAlreadyHasAccess represents an error where a user already has access to a list/namespace
 type ErrUserAlreadyHasAccess struct {
 	UserID int64
@@ -979,3 +933,34 @@ func (err ErrUserHasNoAccessToLabel) HTTPError() web.HTTPError {
 		Message:  "You don't have access to this label.",
 	}
 }
+
+// ========
+//  Rights
+// ========
+
+// ErrInvalidRight represents an error where a right is invalid
+type ErrInvalidRight struct {
+	Right Right
+}
+
+// IsErrInvalidRight checks if an error is ErrInvalidRight.
+func IsErrInvalidRight(err error) bool {
+	_, ok := err.(ErrInvalidRight)
+	return ok
+}
+
+func (err ErrInvalidRight) Error() string {
+	return fmt.Sprintf(" right invalid [Right: %d]", err.Right)
+}
+
+// ErrCodeInvalidRight holds the unique world-error code of this error
+const ErrCodeInvalidRight = 9001
+
+// HTTPError holds the http error description
+func (err ErrInvalidRight) HTTPError() web.HTTPError {
+	return web.HTTPError{
+		HTTPCode: http.StatusBadRequest,
+		Code:     ErrCodeInvalidRight,
+		Message:  "The right is invalid.",
+	}
+}

pkg/models/list_rights.go

@@ -19,74 +19,35 @@ package models
 import (
 	"code.vikunja.io/api/pkg/log"
 	"code.vikunja.io/web"
+	"github.com/go-xorm/builder"
 )
 
-// IsAdmin returns whether the user has admin rights on the list or not
-func (l *List) IsAdmin(a web.Auth) bool {
-	u := getUserForRights(a)
-
-	// Owners are always admins
-	if l.OwnerID == u.ID {
-		return true
-	}
-
-	// Check individual rights
-	if l.checkListUserRight(u, UserRightAdmin) {
-		return true
-	}
-
-	return l.checkListTeamRight(u, TeamRightAdmin)
-}
-
 // CanWrite return whether the user can write on that list or not
 func (l *List) CanWrite(a web.Auth) bool {
 	user := getUserForRights(a)
 
-	// Admins always have write access
+	// Check all the things
-	if l.IsAdmin(user) {
+	// Check if the user is either owner or can write to the list
-		return true
+	return l.isOwner(user) || l.checkRight(user, RightWrite, RightAdmin)
-	}
-
-	// Check individual rights
-	if l.checkListUserRight(user, UserRightWrite) {
-		return true
-	}
-
-	return l.checkListTeamRight(user, TeamRightWrite)
 }
 
 // CanRead checks if a user has read access to a list
 func (l *List) CanRead(a web.Auth) bool {
 	user := getUserForRights(a)
 
-	// Admins always have read access
+	// Check all the things
-	if l.IsAdmin(user) {
+	// Check if the user is either owner or can read
-		return true
+	return l.isOwner(user) || l.checkRight(user, RightRead, RightWrite, RightAdmin)
 }
 
-	// Check individual rights
+// CanUpdate checks if the user can update a list
-	if l.checkListUserRight(user, UserRightRead) {
+func (l *List) CanUpdate(a web.Auth) bool {
-		return true
-	}
-
-	if l.checkListTeamRight(user, TeamRightRead) {
-		return true
-	}
-
-	// Users who are able to write should also be able to read
 	return l.CanWrite(a)
 }
 
 // CanDelete checks if the user can delete a list
 func (l *List) CanDelete(a web.Auth) bool {
-	doer := getUserForRights(a)
+	return l.IsAdmin(a)
-	return l.IsAdmin(doer)
-}
-
-// CanUpdate checks if the user can update a list
-func (l *List) CanUpdate(a web.Auth) bool {
-	doer := getUserForRights(a)
-	return l.CanWrite(doer)
 }
 
 // CanCreate checks if the user can update a list
@@ -96,40 +57,83 @@ func (l *List) CanCreate(a web.Auth) bool {
 	return n.CanWrite(a)
 }
 
-func (l *List) checkListTeamRight(user *User, r TeamRight) bool {
+// IsAdmin returns whether the user has admin rights on the list or not
+func (l *List) IsAdmin(a web.Auth) bool {
+	user := getUserForRights(a)
+
+	// Check all the things
+	// Check if the user is either owner or can write to the list
+	// Owners are always admins
+	return l.isOwner(user) || l.checkRight(user, RightAdmin)
+}
+
+// Little helper function to check if a user is list owner
+func (l *List) isOwner(u *User) bool {
+	return l.OwnerID == u.ID
+}
+
+// Checks n different rights for any given user
+func (l *List) checkRight(user *User, rights ...Right) bool {
+
+	/*
+			The following loop creates an sql condition like this one:
+
+		    (ul.user_id = 1 AND ul.right = 1) OR (un.user_id = 1 AND un.right = 1) OR
+			(tm.user_id = 1 AND tn.right = 1) OR (tm2.user_id = 1 AND tl.right = 1) OR
+
+			for each passed right. That way, we can check with a single sql query (instead if 8)
+			if the user has the right to see the list or not.
+	*/
+
+	var conds []builder.Cond
+	for _, r := range rights {
+		// User conditions
+		// If the list was shared directly with the user and the user has the right
+		conds = append(conds, builder.And(
+			builder.Eq{"ul.user_id": user.ID},
+			builder.Eq{"ul.right": r},
+		))
+		// If the namespace this list belongs to was shared directly with the user and the user has the right
+		conds = append(conds, builder.And(
+			builder.Eq{"un.user_id": user.ID},
+			builder.Eq{"un.right": r},
+		))
+
+		// Team rights
+		// If the list was shared directly with the team and the team has the right
+		conds = append(conds, builder.And(
+			builder.Eq{"tm2.user_id": user.ID},
+			builder.Eq{"tl.right": r},
+		))
+		// If the namespace this list belongs to was shared directly with the team and the team has the right
+		conds = append(conds, builder.And(
+			builder.Eq{"tm.user_id": user.ID},
+			builder.Eq{"tn.right": r},
+		))
+	}
+
 	exists, err := x.Select("l.*").
 		Table("list").
 		Alias("l").
+		// User stuff
+		Join("LEFT", []string{"users_namespace", "un"}, "un.namespace_id = l.namespace_id").
+		Join("LEFT", []string{"users_list", "ul"}, "ul.list_id = l.id").
+		Join("LEFT", []string{"namespaces", "n"}, "n.id = l.namespace_id").
+		// Team stuff
 		Join("LEFT", []string{"team_namespaces", "tn"}, " l.namespace_id = tn.namespace_id").
 		Join("LEFT", []string{"team_members", "tm"}, "tm.team_id = tn.team_id").
 		Join("LEFT", []string{"team_list", "tl"}, "l.id = tl.list_id").
 		Join("LEFT", []string{"team_members", "tm2"}, "tm2.team_id = tl.team_id").
-		Where("((tm.user_id = ? AND tn.right = ?) OR (tm2.user_id = ? AND tl.right = ?)) AND l.id = ?",
+		// The actual condition
-			user.ID, r, user.ID, r, l.ID).
+		Where(builder.And(
+			builder.Or(
+				conds...,
+			),
+			builder.Eq{"l.id": l.ID},
+		)).
 		Exist(&List{})
 	if err != nil {
-		log.Log.Error("Error occurred during checkListTeamRight for List: %s", err)
+		log.Log.Error("Error occurred during checkRight for list: %s", err)
-		return false
-	}
-
-	return exists
-}
-
-func (l *List) checkListUserRight(user *User, r UserRight) bool {
-	exists, err := x.Select("l.*").
-		Table("list").
-		Alias("l").
-		Join("LEFT", []string{"users_namespace", "un"}, "un.namespace_id = l.namespace_id").
-		Join("LEFT", []string{"users_list", "ul"}, "ul.list_id = l.id").
-		Join("LEFT", []string{"namespaces", "n"}, "n.id = l.namespace_id").
-		Where("((ul.user_id = ? AND ul.right = ?) "+
-			"OR (un.user_id = ? AND un.right = ?) "+
-			"OR n.owner_id = ?)"+
-			"AND l.id = ?",
-			user.ID, r, user.ID, r, user.ID, l.ID).
-		Exist(&List{})
-	if err != nil {
-		log.Log.Error("Error occurred during checkListUserRight for List: %s", err)
 		return false
 	}
 

pkg/models/list_users.go

@@ -27,7 +27,7 @@ type ListUser struct {
 	// The list id.
 	ListID int64 `xorm:"int(11) not null INDEX" json:"-" param:"list"`
 	// The right this user has. 0 = Read only, 1 = Read & Write, 2 = Admin. See the docs for more details.
-	Right UserRight `xorm:"int(11) INDEX" json:"right" valid:"length(0|2)" maximum:"2" default:"0"`
+	Right Right `xorm:"int(11) INDEX" json:"right" valid:"length(0|2)" maximum:"2" default:"0"`
 
 	// A unix timestamp when this relation was created. You cannot change this value.
 	Created int64 `xorm:"created" json:"created"`
@@ -46,5 +46,5 @@ func (ListUser) TableName() string {
 // UserWithRight represents a user in combination with the right it can have on a list/namespace
 type UserWithRight struct {
 	User  `xorm:"extends"`
-	Right UserRight `json:"right"`
+	Right Right `json:"right"`
 }

pkg/models/list_users_rights_test.go

@@ -27,7 +27,7 @@ func TestListUser_CanDoSomething(t *testing.T) {
 		ID       int64
 		UserID   int64
 		ListID   int64
-		Right    UserRight
+		Right    Right
 		Created  int64
 		Updated  int64
 		CRUDable web.CRUDable

pkg/models/list_users_test.go

@@ -29,7 +29,7 @@ func TestListUser_Create(t *testing.T) {
 		ID       int64
 		UserID   int64
 		ListID   int64
-		Right    UserRight
+		Right    Right
 		Created  int64
 		Updated  int64
 		CRUDable web.CRUDable
@@ -69,7 +69,7 @@ func TestListUser_Create(t *testing.T) {
 				Right:  500,
 			},
 			wantErr: true,
-			errType: IsErrInvalidUserRight,
+			errType: IsErrInvalidRight,
 		},
 		{
 			name: "ListUsers Create with inexisting list",
@@ -127,7 +127,7 @@ func TestListUser_ReadAll(t *testing.T) {
 		ID       int64
 		UserID   int64
 		ListID   int64
-		Right    UserRight
+		Right    Right
 		Created  int64
 		Updated  int64
 		CRUDable web.CRUDable
@@ -162,7 +162,7 @@ func TestListUser_ReadAll(t *testing.T) {
 						Password: "1234",
 						Email:    "user1@example.com",
 					},
-					Right: UserRightRead,
+					Right: RightRead,
 				},
 				{
 					User: User{
@@ -171,7 +171,7 @@ func TestListUser_ReadAll(t *testing.T) {
 						Password: "1234",
 						Email:    "user2@example.com",
 					},
-					Right: UserRightRead,
+					Right: RightRead,
 				},
 			},
 		},
@@ -218,7 +218,7 @@ func TestListUser_Update(t *testing.T) {
 		ID       int64
 		UserID   int64
 		ListID   int64
-		Right    UserRight
+		Right    Right
 		Created  int64
 		Updated  int64
 		CRUDable web.CRUDable
@@ -235,7 +235,7 @@ func TestListUser_Update(t *testing.T) {
 			fields: fields{
 				ListID: 3,
 				UserID: 1,
-				Right:  UserRightAdmin,
+				Right:  RightAdmin,
 			},
 		},
 		{
@@ -243,7 +243,7 @@ func TestListUser_Update(t *testing.T) {
 			fields: fields{
 				ListID: 3,
 				UserID: 1,
-				Right:  UserRightWrite,
+				Right:  RightWrite,
 			},
 		},
 		{
@@ -251,7 +251,7 @@ func TestListUser_Update(t *testing.T) {
 			fields: fields{
 				ListID: 3,
 				UserID: 1,
-				Right:  UserRightRead,
+				Right:  RightRead,
 			},
 		},
 		{
@@ -262,7 +262,7 @@ func TestListUser_Update(t *testing.T) {
 				Right:  500,
 			},
 			wantErr: true,
-			errType: IsErrInvalidUserRight,
+			errType: IsErrInvalidRight,
 		},
 	}
 	for _, tt := range tests {
@@ -293,7 +293,7 @@ func TestListUser_Delete(t *testing.T) {
 		ID       int64
 		UserID   int64
 		ListID   int64
-		Right    UserRight
+		Right    Right
 		Created  int64
 		Updated  int64
 		CRUDable web.CRUDable

pkg/models/namespace_rights.go

@@ -19,84 +19,29 @@ package models
 import (
 	"code.vikunja.io/api/pkg/log"
 	"code.vikunja.io/web"
+	"github.com/go-xorm/builder"
 )
 
-// IsAdmin returns true or false if the user is admin on that namespace or not
-func (n *Namespace) IsAdmin(a web.Auth) bool {
-	u := getUserForRights(a)
-
-	// Owners always have admin rights
-	if u.ID == n.Owner.ID {
-		return true
-	}
-
-	// Check user rights
-	if n.checkUserRights(u, UserRightAdmin) {
-		return true
-	}
-
-	// Check if that user is in a team which has admin rights to that namespace
-	return n.checkTeamRights(u, TeamRightAdmin)
-}
-
 // CanWrite checks if a user has write access to a namespace
 func (n *Namespace) CanWrite(a web.Auth) bool {
 	u := getUserForRights(a)
-
+	return n.isOwner(u) || n.checkRight(u, RightWrite, RightAdmin)
-	// Admins always have write access
-	if n.IsAdmin(u) {
-		return true
-	}
-
-	// Check user rights
-	if n.checkUserRights(u, UserRightWrite) {
-		return true
-	}
-
-	// Check if that user is in a team which has write rights to that namespace
-	return n.checkTeamRights(u, TeamRightWrite)
 }
 
 // CanRead checks if a user has read access to that namespace
 func (n *Namespace) CanRead(a web.Auth) bool {
 	u := getUserForRights(a)
-
+	return n.isOwner(u) || n.checkRight(u, RightRead, RightWrite, RightAdmin)
-	// Admins always have read access
-	if n.IsAdmin(u) {
-		return true
-	}
-
-	// Check user rights
-	if n.checkUserRights(u, UserRightRead) {
-		return true
-	}
-
-	// Check if the user is in a team which has access to the namespace
-	return n.checkTeamRights(u, TeamRightRead)
 }
 
 // CanUpdate checks if the user can update the namespace
 func (n *Namespace) CanUpdate(a web.Auth) bool {
-	u := getUserForRights(a)
+	return n.IsAdmin(a)
-
-	nn, err := GetNamespaceByID(n.ID)
-	if err != nil {
-		log.Log.Error("Error occurred during CanUpdate for Namespace: %s", err)
-		return false
-	}
-	return nn.IsAdmin(u)
 }
 
 // CanDelete checks if the user can delete a namespace
 func (n *Namespace) CanDelete(a web.Auth) bool {
-	u := getUserForRights(a)
+	return n.IsAdmin(a)
-
-	nn, err := GetNamespaceByID(n.ID)
-	if err != nil {
-		log.Log.Error("Error occurred during CanDelete for Namespace: %s", err)
-		return false
-	}
-	return nn.IsAdmin(u)
 }
 
 // CanCreate checks if the user can create a new namespace
@@ -105,33 +50,66 @@ func (n *Namespace) CanCreate(a web.Auth) bool {
 	return true
 }
 
-func (n *Namespace) checkTeamRights(u *User, r TeamRight) bool {
+// IsAdmin returns true or false if the user is admin on that namespace or not
+func (n *Namespace) IsAdmin(a web.Auth) bool {
+	u := getUserForRights(a)
+	return n.isOwner(u) || n.checkRight(u, RightAdmin)
+}
+
+// Small helper function to check if a user owns the namespace
+func (n *Namespace) isOwner(user *User) bool {
+	return n.OwnerID == user.ID
+}
+
+func (n *Namespace) checkRight(user *User, rights ...Right) bool {
+
+	/*
+		The following loop creates an sql condition like this one:
+
+		namespaces.owner_id = 1 OR
+		(users_namespace.user_id = 1 AND users_namespace.right = 1) OR
+		(team_members.user_id = 1 AND team_namespaces.right = 1) OR
+
+
+		for each passed right. That way, we can check with a single sql query (instead if 8)
+		if the user has the right to see the list or not.
+	*/
+
+	var conds []builder.Cond
+	conds = append(conds, builder.Eq{"namespaces.owner_id": user.ID})
+	for _, r := range rights {
+		// User conditions
+		// If the namespace was shared directly with the user and the user has the right
+		conds = append(conds, builder.And(
+			builder.Eq{"users_namespace.user_id": user.ID},
+			builder.Eq{"users_namespace.right": r},
+		))
+
+		// Team rights
+		// If the namespace was shared directly with the team and the team has the right
+		conds = append(conds, builder.And(
+			builder.Eq{"team_members.user_id": user.ID},
+			builder.Eq{"team_namespaces.right": r},
+		))
+	}
+
 	exists, err := x.Select("namespaces.*").
 		Table("namespaces").
+		// User stuff
+		Join("LEFT", "users_namespace", "users_namespace.namespace_id = namespaces.id").
+		// Teams stuff
 		Join("LEFT", "team_namespaces", "namespaces.id = team_namespaces.namespace_id").
 		Join("LEFT", "team_members", "team_members.team_id = team_namespaces.team_id").
-		Where("namespaces.id = ? AND ("+
+		// The actual condition
-			"(team_members.user_id = ? AND team_namespaces.right = ?) "+
+		Where(builder.And(
-			"OR namespaces.owner_id = ?)", n.ID, u.ID, r, u.ID).
+			builder.Or(
-		Get(&Namespace{})
+				conds...,
+			),
+			builder.Eq{"namespaces.id": n.ID},
+		)).
+		Exist(&List{})
 	if err != nil {
-		log.Log.Error("Error occurred during checkTeamRights for Namespace: %s, TeamRight: %d", err, r)
+		log.Log.Error("Error occurred during checkRight for namespace: %s", err)
-		return false
-	}
-
-	return exists
-}
-
-func (n *Namespace) checkUserRights(u *User, r UserRight) bool {
-	exists, err := x.Select("namespaces.*").
-		Table("namespaces").
-		Join("LEFT", "users_namespace", "users_namespace.namespace_id = namespaces.id").
-		Where("namespaces.id = ? AND ("+
-			"(users_namespace.user_id = ? AND users_namespace.right = ?) "+
-			"OR namespaces.owner_id = ?)", n.ID, u.ID, r, u.ID).
-		Get(&Namespace{})
-	if err != nil {
-		log.Log.Error("Error occurred during checkUserRights for Namespace: %s, UserRight: %d", err, r)
 		return false
 	}
 

pkg/models/namespace_users.go

@@ -27,7 +27,7 @@ type NamespaceUser struct {
 	// The namespace id
 	NamespaceID int64 `xorm:"int(11) not null INDEX" json:"-" param:"namespace"`
 	// The right this user has. 0 = Read only, 1 = Read & Write, 2 = Admin. See the docs for more details.
-	Right UserRight `xorm:"int(11) INDEX" json:"right" valid:"length(0|2)" maximum:"2" default:"0"`
+	Right Right `xorm:"int(11) INDEX" json:"right" valid:"length(0|2)" maximum:"2" default:"0"`
 
 	// A unix timestamp when this relation was created. You cannot change this value.
 	Created int64 `xorm:"created" json:"created"`

pkg/models/namespace_users_rights_test.go

@@ -27,7 +27,7 @@ func TestNamespaceUser_CanDoSomething(t *testing.T) {
 		ID          int64
 		UserID      int64
 		NamespaceID int64
-		Right       UserRight
+		Right       Right
 		Created     int64
 		Updated     int64
 		CRUDable    web.CRUDable

pkg/models/namespace_users_test.go

@@ -30,7 +30,7 @@ func TestNamespaceUser_Create(t *testing.T) {
 		ID          int64
 		UserID      int64
 		NamespaceID int64
-		Right       UserRight
+		Right       Right
 		Created     int64
 		Updated     int64
 		CRUDable    web.CRUDable
@@ -70,7 +70,7 @@ func TestNamespaceUser_Create(t *testing.T) {
 				Right:       500,
 			},
 			wantErr: true,
-			errType: IsErrInvalidUserRight,
+			errType: IsErrInvalidRight,
 		},
 		{
 			name: "NamespaceUsers Create with inexisting list",
@@ -128,7 +128,7 @@ func TestNamespaceUser_ReadAll(t *testing.T) {
 		ID          int64
 		UserID      int64
 		NamespaceID int64
-		Right       UserRight
+		Right       Right
 		Created     int64
 		Updated     int64
 		CRUDable    web.CRUDable
@@ -163,7 +163,7 @@ func TestNamespaceUser_ReadAll(t *testing.T) {
 						Password: "1234",
 						Email:    "user1@example.com",
 					},
-					Right: UserRightRead,
+					Right: RightRead,
 				},
 				{
 					User: User{
@@ -172,7 +172,7 @@ func TestNamespaceUser_ReadAll(t *testing.T) {
 						Password: "1234",
 						Email:    "user2@example.com",
 					},
-					Right: UserRightRead,
+					Right: RightRead,
 				},
 			},
 		},
@@ -220,7 +220,7 @@ func TestNamespaceUser_Update(t *testing.T) {
 		ID          int64
 		UserID      int64
 		NamespaceID int64
-		Right       UserRight
+		Right       Right
 		Created     int64
 		Updated     int64
 		CRUDable    web.CRUDable
@@ -237,7 +237,7 @@ func TestNamespaceUser_Update(t *testing.T) {
 			fields: fields{
 				NamespaceID: 3,
 				UserID:      1,
-				Right:       UserRightAdmin,
+				Right:       RightAdmin,
 			},
 		},
 		{
@@ -245,7 +245,7 @@ func TestNamespaceUser_Update(t *testing.T) {
 			fields: fields{
 				NamespaceID: 3,
 				UserID:      1,
-				Right:       UserRightWrite,
+				Right:       RightWrite,
 			},
 		},
 		{
@@ -253,7 +253,7 @@ func TestNamespaceUser_Update(t *testing.T) {
 			fields: fields{
 				NamespaceID: 3,
 				UserID:      1,
-				Right:       UserRightRead,
+				Right:       RightRead,
 			},
 		},
 		{
@@ -264,7 +264,7 @@ func TestNamespaceUser_Update(t *testing.T) {
 				Right:       500,
 			},
 			wantErr: true,
-			errType: IsErrInvalidUserRight,
+			errType: IsErrInvalidRight,
 		},
 	}
 	for _, tt := range tests {
@@ -295,7 +295,7 @@ func TestNamespaceUser_Delete(t *testing.T) {
 		ID          int64
 		UserID      int64
 		NamespaceID int64
-		Right       UserRight
+		Right       Right
 		Created     int64
 		Updated     int64
 		CRUDable    web.CRUDable

pkg/models/rights.go

@@ -1,5 +1,5 @@
 //  Vikunja is a todo-list application to facilitate your life.
-//  Copyright 2018 Vikunja and contributors. All rights reserved.
+//  Copyright 2019 Vikunja and contributors. All rights reserved.
 //
 //  This program is free software: you can redistribute it and/or modify
 //  it under the terms of the GNU General Public License as published by
@@ -16,27 +16,27 @@
 
 package models
 
-// TeamRight defines the rights teams can have for lists/namespaces
+// Right defines the rights users/teams can have for lists/namespaces
-type TeamRight int
+type Right int
 
-// define unknown team right
+// define unknown right
 const (
-	TeamRightUnknown = -1
+	RightUnknown = -1
 )
 
 // Enumerate all the team rights
 const (
-	// Can read lists in a Team
+	// Can read lists in a
-	TeamRightRead TeamRight = iota
+	RightRead Right = iota
-	// Can write tasks in a Team like lists and todo tasks. Cannot create new lists.
+	// Can write in a like lists and todo tasks. Cannot create new lists.
-	TeamRightWrite
+	RightWrite
 	// Can manage a list/namespace, can do everything
-	TeamRightAdmin
+	RightAdmin
 )
 
-func (r TeamRight) isValid() error {
+func (r Right) isValid() error {
-	if r != TeamRightAdmin && r != TeamRightRead && r != TeamRightWrite {
+	if r != RightAdmin && r != RightRead && r != RightWrite {
-		return ErrInvalidTeamRight{r}
+		return ErrInvalidRight{r}
 	}
 
 	return nil

pkg/models/team_list.go

@@ -27,7 +27,7 @@ type TeamList struct {
 	// The list id.
 	ListID int64 `xorm:"int(11) not null INDEX" json:"-" param:"list"`
 	// The right this team has. 0 = Read only, 1 = Read & Write, 2 = Admin. See the docs for more details.
-	Right TeamRight `xorm:"int(11) INDEX" json:"right" valid:"length(0|2)" maximum:"2" default:"0"`
+	Right Right `xorm:"int(11) INDEX" json:"right" valid:"length(0|2)" maximum:"2" default:"0"`
 
 	// A unix timestamp when this relation was created. You cannot change this value.
 	Created int64 `xorm:"created" json:"created"`
@@ -46,5 +46,5 @@ func (TeamList) TableName() string {
 // TeamWithRight represents a team, combined with rights.
 type TeamWithRight struct {
 	Team  `xorm:"extends"`
-	Right TeamRight `json:"right"`
+	Right Right `json:"right"`
 }

pkg/models/team_list_test.go

@@ -29,7 +29,7 @@ func TestTeamList(t *testing.T) {
 	tl := TeamList{
 		TeamID: 1,
 		ListID: 1,
-		Right:  TeamRightAdmin,
+		Right:  RightAdmin,
 	}
 
 	// Dummyuser
@@ -48,10 +48,10 @@ func TestTeamList(t *testing.T) {
 
 	// Check with wrong rights
 	tl2 := tl
-	tl2.Right = TeamRightUnknown
+	tl2.Right = RightUnknown
 	err = tl2.Create(&u)
 	assert.Error(t, err)
-	assert.True(t, IsErrInvalidTeamRight(err))
+	assert.True(t, IsErrInvalidRight(err))
 
 	// Check with inexistant team
 	tl3 := tl
@@ -113,7 +113,7 @@ func TestTeamList_Update(t *testing.T) {
 		ID       int64
 		TeamID   int64
 		ListID   int64
-		Right    TeamRight
+		Right    Right
 		Created  int64
 		Updated  int64
 		CRUDable web.CRUDable
@@ -130,7 +130,7 @@ func TestTeamList_Update(t *testing.T) {
 			fields: fields{
 				ListID: 3,
 				TeamID: 1,
-				Right:  TeamRightAdmin,
+				Right:  RightAdmin,
 			},
 		},
 		{
@@ -138,7 +138,7 @@ func TestTeamList_Update(t *testing.T) {
 			fields: fields{
 				ListID: 3,
 				TeamID: 1,
-				Right:  TeamRightWrite,
+				Right:  RightWrite,
 			},
 		},
 		{
@@ -146,7 +146,7 @@ func TestTeamList_Update(t *testing.T) {
 			fields: fields{
 				ListID: 3,
 				TeamID: 1,
-				Right:  TeamRightRead,
+				Right:  RightRead,
 			},
 		},
 		{
@@ -157,7 +157,7 @@ func TestTeamList_Update(t *testing.T) {
 				Right:  500,
 			},
 			wantErr: true,
-			errType: IsErrInvalidTeamRight,
+			errType: IsErrInvalidRight,
 		},
 	}
 	for _, tt := range tests {

pkg/models/team_namespace.go

@@ -27,7 +27,7 @@ type TeamNamespace struct {
 	// The namespace id.
 	NamespaceID int64 `xorm:"int(11) not null INDEX" json:"-" param:"namespace"`
 	// The right this team has. 0 = Read only, 1 = Read & Write, 2 = Admin. See the docs for more details.
-	Right TeamRight `xorm:"int(11) INDEX" json:"right" valid:"length(0|2)" maximum:"2" default:"0"`
+	Right Right `xorm:"int(11) INDEX" json:"right" valid:"length(0|2)" maximum:"2" default:"0"`
 
 	// A unix timestamp when this relation was created. You cannot change this value.
 	Created int64 `xorm:"created" json:"created"`

pkg/models/team_namespace_rights_test.go

@@ -27,7 +27,7 @@ func TestTeamNamespace_CanDoSomething(t *testing.T) {
 		ID          int64
 		TeamID      int64
 		NamespaceID int64
-		Right       TeamRight
+		Right       Right
 		Created     int64
 		Updated     int64
 		CRUDable    web.CRUDable

pkg/models/team_namespace_test.go

@@ -29,7 +29,7 @@ func TestTeamNamespace(t *testing.T) {
 	tn := TeamNamespace{
 		TeamID:      1,
 		NamespaceID: 1,
-		Right:       TeamRightAdmin,
+		Right:       RightAdmin,
 	}
 
 	dummyuser, err := GetUserByID(1)
@@ -47,10 +47,10 @@ func TestTeamNamespace(t *testing.T) {
 
 	// Test with invalid team right
 	tn2 := tn
-	tn2.Right = TeamRightUnknown
+	tn2.Right = RightUnknown
 	err = tn2.Create(&dummyuser)
 	assert.Error(t, err)
-	assert.True(t, IsErrInvalidTeamRight(err))
+	assert.True(t, IsErrInvalidRight(err))
 
 	// Check with inexistant team
 	tn3 := tn
@@ -105,7 +105,7 @@ func TestTeamNamespace_Update(t *testing.T) {
 		ID          int64
 		TeamID      int64
 		NamespaceID int64
-		Right       TeamRight
+		Right       Right
 		Created     int64
 		Updated     int64
 		CRUDable    web.CRUDable
@@ -122,7 +122,7 @@ func TestTeamNamespace_Update(t *testing.T) {
 			fields: fields{
 				NamespaceID: 3,
 				TeamID:      1,
-				Right:       TeamRightAdmin,
+				Right:       RightAdmin,
 			},
 		},
 		{
@@ -130,7 +130,7 @@ func TestTeamNamespace_Update(t *testing.T) {
 			fields: fields{
 				NamespaceID: 3,
 				TeamID:      1,
-				Right:       TeamRightWrite,
+				Right:       RightWrite,
 			},
 		},
 		{
@@ -138,7 +138,7 @@ func TestTeamNamespace_Update(t *testing.T) {
 			fields: fields{
 				NamespaceID: 3,
 				TeamID:      1,
-				Right:       TeamRightRead,
+				Right:       RightRead,
 			},
 		},
 		{
@@ -149,7 +149,7 @@ func TestTeamNamespace_Update(t *testing.T) {
 				Right:       500,
 			},
 			wantErr: true,
-			errType: IsErrInvalidTeamRight,
+			errType: IsErrInvalidRight,
 		},
 	}
 	for _, tt := range tests {

pkg/models/teams_test.go

@@ -92,15 +92,15 @@ func TestTeam_Create(t *testing.T) {
 	assert.True(t, IsErrTeamDoesNotExist(err))
 }
 
-func TestIsErrInvalidTeamRight(t *testing.T) {
+func TestIsErrInvalidRight(t *testing.T) {
-	assert.NoError(t, TeamRightAdmin.isValid())
+	assert.NoError(t, RightAdmin.isValid())
-	assert.NoError(t, TeamRightRead.isValid())
+	assert.NoError(t, RightRead.isValid())
-	assert.NoError(t, TeamRightWrite.isValid())
+	assert.NoError(t, RightWrite.isValid())
 
 	// Check invalid
-	var tr TeamRight
+	var tr Right
 	tr = 938
 	err := tr.isValid()
 	assert.Error(t, err)
-	assert.True(t, IsErrInvalidTeamRight(err))
+	assert.True(t, IsErrInvalidRight(err))
 }

pkg/models/user_right.go

@@ -1,43 +0,0 @@
-//  Vikunja is a todo-list application to facilitate your life.
-//  Copyright 2018 Vikunja and contributors. All rights reserved.
-//
-//  This program is free software: you can redistribute it and/or modify
-//  it under the terms of the GNU General Public License as published by
-//  the Free Software Foundation, either version 3 of the License, or
-//  (at your option) any later version.
-//
-//  This program is distributed in the hope that it will be useful,
-//  but WITHOUT ANY WARRANTY; without even the implied warranty of
-//  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-//  GNU General Public License for more details.
-//
-//  You should have received a copy of the GNU General Public License
-//  along with this program.  If not, see <https://www.gnu.org/licenses/>.
-
-package models
-
-// UserRight defines the rights users can have for lists/namespaces
-type UserRight int
-
-// define unknown user right
-const (
-	UserRightUnknown = -1
-)
-
-// Enumerate all the user rights
-const (
-	// Can read lists in a User
-	UserRightRead UserRight = iota
-	// Can write tasks in a User like lists and todo tasks. Cannot create new lists.
-	UserRightWrite
-	// Can manage a list/namespace, can do everything
-	UserRightAdmin
-)
-
-func (r UserRight) isValid() error {
-	if r != UserRightAdmin && r != UserRightRead && r != UserRightWrite {
-		return ErrInvalidUserRight{r}
-	}
-
-	return nil
-}