diff --git a/Featurecreep.md b/Featurecreep.md index 82c241e8a..7c2cad26b 100644 --- a/Featurecreep.md +++ b/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 diff --git a/REST-Tests/lists.http b/REST-Tests/lists.http index 0257046ef..c84017aa7 100644 --- a/REST-Tests/lists.http +++ b/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}} ### diff --git a/REST-Tests/namespaces.http b/REST-Tests/namespaces.http index 87bd39c67..d0eef8f11 100644 --- a/REST-Tests/namespaces.http +++ b/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}} ### diff --git a/docs/docs.go b/docs/docs.go index 8dfd805c2..85a71c1ec 100644 --- a/docs/docs.go +++ b/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" diff --git a/docs/errors.md b/docs/errors.md index 5ee8d3626..f4aace61d 100644 --- a/docs/errors.md +++ b/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. | \ No newline at end of file +| 8003 | 403 | The user does not have access to this label. | +| 9001 | 403 | The right is invalid. | \ No newline at end of file diff --git a/docs/swagger/swagger.json b/docs/swagger/swagger.json index d74a5754c..29077a74f 100644 --- a/docs/swagger/swagger.json +++ b/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" diff --git a/docs/swagger/swagger.yaml b/docs/swagger/swagger.yaml index 70f9568fa..76e9f9e7d 100644 --- a/docs/swagger/swagger.yaml +++ b/docs/swagger/swagger.yaml @@ -622,7 +622,13 @@ info: email: hello@vikunja.io name: General Vikunja contact url: http://vikunja.io/en/contact/ - description: '' + 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. + # Authorization + **JWT-Auth:** Main authorization method, used for most of the requests. Needs ` + "`" + `Authorization: Bearer ` + "`" + `-header to authenticate successfully. + + **BasicAuth:** Only used when requesting tasks via caldav. + 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 diff --git a/pkg/models/error.go b/pkg/models/error.go index 124191dba..b1ebbdb71 100644 --- a/pkg/models/error.go +++ b/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.", + } +} diff --git a/pkg/models/list_rights.go b/pkg/models/list_rights.go index c3a2bdc83..48ef5c50b 100644 --- a/pkg/models/list_rights.go +++ b/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 - if l.IsAdmin(user) { - return true - } - - // Check individual rights - if l.checkListUserRight(user, UserRightWrite) { - return true - } - - return l.checkListTeamRight(user, TeamRightWrite) + // Check all the things + // Check if the user is either owner or can write to the list + return l.isOwner(user) || l.checkRight(user, RightWrite, RightAdmin) } // 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 - if l.IsAdmin(user) { - return true - } + // Check all the things + // Check if the user is either owner or can read + return l.isOwner(user) || l.checkRight(user, RightRead, RightWrite, RightAdmin) +} - // Check individual rights - if l.checkListUserRight(user, UserRightRead) { - return true - } - - if l.checkListTeamRight(user, TeamRightRead) { - return true - } - - // Users who are able to write should also be able to read +// CanUpdate checks if the user can update a list +func (l *List) CanUpdate(a web.Auth) bool { 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(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) + return l.IsAdmin(a) } // 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 = ?", - user.ID, r, user.ID, r, l.ID). + // The actual condition + 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) - 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) + log.Log.Error("Error occurred during checkRight for list: %s", err) return false } diff --git a/pkg/models/list_users.go b/pkg/models/list_users.go index 19beb4da0..b458ca8a7 100644 --- a/pkg/models/list_users.go +++ b/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"` } diff --git a/pkg/models/list_users_rights_test.go b/pkg/models/list_users_rights_test.go index 96f3bfa1a..1e1ac7365 100644 --- a/pkg/models/list_users_rights_test.go +++ b/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 diff --git a/pkg/models/list_users_test.go b/pkg/models/list_users_test.go index 748347aa6..6f60a89d9 100644 --- a/pkg/models/list_users_test.go +++ b/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 diff --git a/pkg/models/namespace_rights.go b/pkg/models/namespace_rights.go index 937752bbb..209ef698a 100644 --- a/pkg/models/namespace_rights.go +++ b/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) - - // 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) + return n.isOwner(u) || n.checkRight(u, RightWrite, RightAdmin) } // CanRead checks if a user has read access to that namespace func (n *Namespace) CanRead(a web.Auth) bool { u := getUserForRights(a) - - // 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) + return n.isOwner(u) || n.checkRight(u, RightRead, RightWrite, RightAdmin) } // CanUpdate checks if the user can update the namespace func (n *Namespace) CanUpdate(a web.Auth) bool { - u := getUserForRights(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) + return n.IsAdmin(a) } // CanDelete checks if the user can delete a namespace func (n *Namespace) CanDelete(a web.Auth) bool { - u := getUserForRights(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) + return n.IsAdmin(a) } // 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 ("+ - "(team_members.user_id = ? AND team_namespaces.right = ?) "+ - "OR namespaces.owner_id = ?)", n.ID, u.ID, r, u.ID). - Get(&Namespace{}) + // The actual condition + Where(builder.And( + builder.Or( + 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) - 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) + log.Log.Error("Error occurred during checkRight for namespace: %s", err) return false } diff --git a/pkg/models/namespace_users.go b/pkg/models/namespace_users.go index 63cea2018..c9230c014 100644 --- a/pkg/models/namespace_users.go +++ b/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"` diff --git a/pkg/models/namespace_users_rights_test.go b/pkg/models/namespace_users_rights_test.go index 8883b67a0..a30be33eb 100644 --- a/pkg/models/namespace_users_rights_test.go +++ b/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 diff --git a/pkg/models/namespace_users_test.go b/pkg/models/namespace_users_test.go index c1d310174..ec0265f3a 100644 --- a/pkg/models/namespace_users_test.go +++ b/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 diff --git a/pkg/models/team_right.go b/pkg/models/rights.go similarity index 62% rename from pkg/models/team_right.go rename to pkg/models/rights.go index d15080a5a..1423b4753 100644 --- a/pkg/models/team_right.go +++ b/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 -type TeamRight int +// Right defines the rights users/teams can have for lists/namespaces +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 - TeamRightRead TeamRight = iota - // Can write tasks in a Team like lists and todo tasks. Cannot create new lists. - TeamRightWrite + // Can read lists in a + RightRead Right = iota + // Can write in a like lists and todo tasks. Cannot create new lists. + RightWrite // Can manage a list/namespace, can do everything - TeamRightAdmin + RightAdmin ) -func (r TeamRight) isValid() error { - if r != TeamRightAdmin && r != TeamRightRead && r != TeamRightWrite { - return ErrInvalidTeamRight{r} +func (r Right) isValid() error { + if r != RightAdmin && r != RightRead && r != RightWrite { + return ErrInvalidRight{r} } return nil diff --git a/pkg/models/team_list.go b/pkg/models/team_list.go index d882d2780..244b295f9 100644 --- a/pkg/models/team_list.go +++ b/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"` } diff --git a/pkg/models/team_list_test.go b/pkg/models/team_list_test.go index 83516707a..193fe4d23 100644 --- a/pkg/models/team_list_test.go +++ b/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 { diff --git a/pkg/models/team_namespace.go b/pkg/models/team_namespace.go index 3b2f5b5db..b8d9db6b8 100644 --- a/pkg/models/team_namespace.go +++ b/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"` diff --git a/pkg/models/team_namespace_rights_test.go b/pkg/models/team_namespace_rights_test.go index 19c70cb61..f919dd230 100644 --- a/pkg/models/team_namespace_rights_test.go +++ b/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 diff --git a/pkg/models/team_namespace_test.go b/pkg/models/team_namespace_test.go index 085f67b0a..4ed427501 100644 --- a/pkg/models/team_namespace_test.go +++ b/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 { diff --git a/pkg/models/teams_test.go b/pkg/models/teams_test.go index 801505f4b..1f3e0d632 100644 --- a/pkg/models/teams_test.go +++ b/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) { - assert.NoError(t, TeamRightAdmin.isValid()) - assert.NoError(t, TeamRightRead.isValid()) - assert.NoError(t, TeamRightWrite.isValid()) +func TestIsErrInvalidRight(t *testing.T) { + assert.NoError(t, RightAdmin.isValid()) + assert.NoError(t, RightRead.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)) } diff --git a/pkg/models/user_right.go b/pkg/models/user_right.go deleted file mode 100644 index 1f31fd6e9..000000000 --- a/pkg/models/user_right.go +++ /dev/null @@ -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 . - -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 -}