Rights performance improvements for lists and namespaces #54

Merged
konrad merged 14 commits from improvement/rights-performance into master 2019-01-21 22:08:05 +00:00
4 changed files with 112 additions and 81 deletions
Showing only changes of commit d69a101cec - Show all commits

View File

@ -31,6 +31,7 @@ This document describes the different errors Vikunja can return.
| 5009 | 403 | The user needs to have namespace read access to perform that action. |
| 5010 | 403 | This team does not have access to that namespace. |
| 5011 | 409 | This user has already access to that namespace. |
| 5012 | 403 | The namespace right is invalid. |
| 6001 | 400 | The team name cannot be emtpy. |
| 6002 | 404 | The team does not exist. |
| 6003 | 400 | The provided team right is invalid. |

View File

@ -672,6 +672,33 @@ func (err ErrUserAlreadyHasNamespaceAccess) HTTPError() web.HTTPError {
return web.HTTPError{HTTPCode: http.StatusConflict, Code: ErrCodeUserAlreadyHasNamespaceAccess, Message: "This user already has access to this namespace."}
}
// ErrInvalidNamespaceRight represents an error where a namespace right is invalid
type ErrInvalidNamespaceRight struct {
Right NamespaceRight
}
// IsErrInvalidNamespaceRight checks if an error is ErrInvalidNamespaceRight.
func IsErrInvalidNamespaceRight(err error) bool {
_, ok := err.(ErrInvalidNamespaceRight)
return ok
}
func (err ErrInvalidNamespaceRight) Error() string {
return fmt.Sprintf("Namespace right is invalid [Right: %d]", err.Right)
}
// ErrCodeInvalidNamespaceRight holds the unique world-error code of this error
const ErrCodeInvalidNamespaceRight = 5012
// HTTPError holds the http error description
func (err ErrInvalidNamespaceRight) HTTPError() web.HTTPError {
return web.HTTPError{
HTTPCode: http.StatusBadRequest,
Code: ErrCodeInvalidNamespaceRight,
Message: "The namespace right is invalid.",
}
}
// ============
// Team errors
// ============

View File

@ -70,10 +70,7 @@ func (l *List) IsAdmin(a web.Auth) bool {
// Little helper function to check if a user is list owner
func (l *List) isOwner(u *User) bool {
if l.OwnerID == u.ID {
return true
}
return false
return l.OwnerID == u.ID
}
// Checks n different rights for any given user
@ -137,7 +134,7 @@ func (l *List) checkRight(user *User, rights ...ListRight) bool {
)).
Exist(&List{})
if err != nil {
log.Log.Error("Error occurred during checkListRight for List: %s", err)
log.Log.Error("Error occurred during checkRight for list: %s", err)
return false
}

View File

@ -19,84 +19,31 @@ 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, NamespaceRightWrite, NamespaceRightAdmin)
}
// 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, NamespaceRightRead, NamespaceRightWrite, NamespaceRightAdmin)
}
// 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.isOwner(u) || n.checkRight(u, NamespaceRightAdmin)
}
// 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.isOwner(u) || n.checkRight(u, NamespaceRightAdmin)
}
// CanCreate checks if the user can create a new namespace
@ -105,35 +52,94 @@ 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, NamespaceRightAdmin)
}
// 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 ...NamespaceRight) 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)
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
// NamespaceRight defines the rights users/teams can have for namespaces
type NamespaceRight int
// define unknown namespace right
const (
NamespaceRightUnknown = -1
)
// Enumerate all the namespace rights
const (
// Can read namespaces
NamespaceRightRead NamespaceRight = iota
// Can write/edit tasks in a namespace
NamespaceRightWrite
// Can manage a namespace, can do everything
NamespaceRightAdmin
)
func (r NamespaceRight) isValid() error {
if r != NamespaceRightAdmin && r != NamespaceRightRead && r != NamespaceRightWrite {
return ErrInvalidNamespaceRight{r}
}
return exists
return nil
}