ami/tl-vikunja
1
0
Fork 0
Code Activity

Return rights when reading a single item (#626)

Browse Source 
Fix lint

Update docs

Fix loading all rights (list & namespace)

Add tests

Update web framework

Make tests run again

Update all calls to CanRead methods

Update task attachment & task comment & task rights to return the max right

Update team rights to return the max right

Update namespace rights to return the max right

Update list rights to return the max right

Update link share rights to return the max right

Update label rights to return the max right

Update web dependency

Co-authored-by: kolaente <k@knt.li>
Reviewed-on: https://kolaente.dev/vikunja/api/pulls/626
This commit is contained in:
konrad
2020-08-10 12:11:43 +00:00
parent 28b8cabea5
commit bd8c1c3bb7
36 changed files with 165 additions and 68 deletions
Download Patch File Download Diff File Expand all files Collapse all files

29
pkg/models/label_rights.go
View File

@ -33,7 +33,7 @@ func (l *Label) CanDelete(a web.Auth) (bool, error) {
}
// CanRead checks if a user can read a label
func (l *Label) CanRead(a web.Auth) (bool, error) {
func (l *Label) CanRead(a web.Auth) (bool, int, error) {
return l.hasAccessToLabel(a)
}
@ -61,24 +61,37 @@ func (l *Label) isLabelOwner(a web.Auth) (bool, error) {
}
// Helper method to check if a user can see a specific label
func (l *Label) hasAccessToLabel(a web.Auth) (bool, error) {
func (l *Label) hasAccessToLabel(a web.Auth) (has bool, maxRight int, err error) {
// TODO: add an extra check for link share handling
// Get all tasks
taskIDs, err := getUserTaskIDs(&user.User{ID: a.GetID()})
if err != nil {
return false, err
return false, 0, err
}
// Get all labels associated with these tasks
var labels []*Label
has, err := x.Table("labels").
Select("labels.*").
ll := &LabelTask{}
has, err = x.Table("labels").
Select("label_task.*").
Join("LEFT", "label_task", "label_task.label_id = labels.id").
Where("label_task.label_id is not null OR labels.created_by_id = ?", a.GetID()).
Or(builder.In("label_task.task_id", taskIDs)).
And("labels.id = ?", l.ID).
Exist(&labels)
return has, err
Exist(ll)
if err != nil {
return
}
// Since the right depends on the task the label is associated with, we need to check that too.
if ll.TaskID > 0 {
t := &Task{ID: ll.TaskID}
_, maxRight, err = t.CanRead(a)
if err != nil {
return
}
}
return
}

4
pkg/models/label_task.go
View File

@ -113,7 +113,7 @@ func (lt *LabelTask) Create(a web.Auth) (err error) {
func (lt *LabelTask) ReadAll(a web.Auth, search string, page int, perPage int) (result interface{}, resultCount int, numberOfTotalItems int64, err error) {
// Check if the user has the right to see the task
task := Task{ID: lt.TaskID}
canRead, err := task.CanRead(a)
canRead, _, err := task.CanRead(a)
if err != nil {
return nil, 0, 0, err
}
@ -291,7 +291,7 @@ func (t *Task) updateTaskLabels(creator web.Auth, labels []*Label) (err error) {
}
// Check if the user has the rights to see the label he is about to add
hasAccessToLabel, err := label.hasAccessToLabel(creator)
hasAccessToLabel, _, err := label.hasAccessToLabel(creator)
if err != nil {
return err
}

2
pkg/models/label_task_rights.go
View File

@ -27,7 +27,7 @@ func (lt *LabelTask) CanCreate(a web.Auth) (bool, error) {
return false, err
}
hasAccessTolabel, err := label.hasAccessToLabel(a)
hasAccessTolabel, _, err := label.hasAccessToLabel(a)
if err != nil || !hasAccessTolabel { // If the user doesn't have access to the label, we can error out here
return false, err
}

2
pkg/models/label_test.go
View File

@ -240,7 +240,7 @@ func TestLabel_ReadOne(t *testing.T) {
Rights: tt.fields.Rights,
}
allowed, _ := l.CanRead(tt.auth)
allowed, _, _ := l.CanRead(tt.auth)
if !allowed && !tt.wantForbidden {
t.Errorf("Label.CanRead() forbidden, want %v", tt.wantForbidden)
}

2
pkg/models/link_sharing.go
View File

@ -153,7 +153,7 @@ func (share *LinkSharing) ReadOne() (err error) {
// @Router /lists/{list}/shares [get]
func (share *LinkSharing) ReadAll(a web.Auth, search string, page int, perPage int) (result interface{}, resultCount int, totalItems int64, err error) {
list := &List{ID: share.ListID}
can, err := list.CanRead(a)
can, _, err := list.CanRead(a)
if err != nil {
return nil, 0, 0, err
}

6
pkg/models/link_sharing_rights.go
View File

@ -19,15 +19,15 @@ package models
import "code.vikunja.io/web"
// CanRead implements the read right check for a link share
func (share *LinkSharing) CanRead(a web.Auth) (bool, error) {
func (share *LinkSharing) CanRead(a web.Auth) (bool, int, error) {
// Don't allow creating link shares if the user itself authenticated with a link share
if _, is := a.(*LinkSharing); is {
return false, nil
return false, 0, nil
}
l, err := GetListByShareHash(share.Hash)
if err != nil {
return false, err
return false, 0, err
}
return l.CanRead(a)
}

2
pkg/models/list_duplicate.go
View File

@ -41,7 +41,7 @@ type ListDuplicate struct {
func (ld *ListDuplicate) CanCreate(a web.Auth) (canCreate bool, err error) {
// List Exists + user has read access to list
ld.List = &List{ID: ld.ListID}
canRead, err := ld.List.CanRead(a)
canRead, _, err := ld.List.CanRead(a)
if err != nil || !canRead {
return canRead, err
}

46
pkg/models/list_rights.go
View File

@ -54,7 +54,7 @@ func (l *List) CanWrite(a web.Auth) (bool, error) {
return canWrite, errIsArchived
}
canWrite, err = originalList.checkRight(a, RightWrite, RightAdmin)
canWrite, _, err = originalList.checkRight(a, RightWrite, RightAdmin)
if err != nil {
return false, err
}
@ -62,21 +62,21 @@ func (l *List) CanWrite(a web.Auth) (bool, error) {
}
// CanRead checks if a user has read access to a list
func (l *List) CanRead(a web.Auth) (bool, error) {
func (l *List) CanRead(a web.Auth) (bool, int, error) {
// Check if the user is either owner or can read
if err := l.GetSimpleByID(); err != nil {
return false, err
return false, 0, err
}
// Check if we're dealing with a share auth
shareAuth, ok := a.(*LinkSharing)
if ok {
return l.ID == shareAuth.ListID &&
(shareAuth.Right == RightRead || shareAuth.Right == RightWrite || shareAuth.Right == RightAdmin), nil
(shareAuth.Right == RightRead || shareAuth.Right == RightWrite || shareAuth.Right == RightAdmin), int(shareAuth.Right), nil
}
if l.isOwner(&user.User{ID: a.GetID()}) {
return true, nil
return true, int(RightAdmin), nil
}
return l.checkRight(a, RightRead, RightWrite, RightAdmin)
}
@ -123,7 +123,8 @@ func (l *List) IsAdmin(a web.Auth) (bool, error) {
if originalList.isOwner(&user.User{ID: a.GetID()}) {
return true, nil
}
return originalList.checkRight(a, RightAdmin)
is, _, err := originalList.checkRight(a, RightAdmin)
return is, err
}
// Little helper function to check if a user is list owner
@ -132,7 +133,7 @@ func (l *List) isOwner(u *user.User) bool {
}
// Checks n different rights for any given user
func (l *List) checkRight(a web.Auth, rights ...Right) (bool, error) {
func (l *List) checkRight(a web.Auth, rights ...Right) (bool, int, error) {
/*
The following loop creates an sql condition like this one:
@ -174,7 +175,17 @@ func (l *List) checkRight(a web.Auth, rights ...Right) (bool, error) {
// If the user is the owner of a namespace, it has any right, all the time
conds = append(conds, builder.Eq{"n.owner_id": a.GetID()})
exists, err := x.Select("l.*").
type allListRights struct {
UserNamespace NamespaceUser `xorm:"extends"`
UserList ListUser `xorm:"extends"`
TeamNamespace TeamNamespace `xorm:"extends"`
TeamList TeamList `xorm:"extends"`
}
r := &allListRights{}
var maxRight = 0
exists, err := x.
Table("list").
Alias("l").
// User stuff
@ -193,6 +204,21 @@ func (l *List) checkRight(a web.Auth, rights ...Right) (bool, error) {
),
builder.Eq{"l.id": l.ID},
)).
Exist(&List{})
return exists, err
Get(r)
// Figure out the max right and return it
if int(r.UserNamespace.Right) > maxRight {
maxRight = int(r.UserNamespace.Right)
}
if int(r.UserList.Right) > maxRight {
maxRight = int(r.UserList.Right)
}
if int(r.TeamNamespace.Right) > maxRight {
maxRight = int(r.TeamNamespace.Right)
}
if int(r.TeamList.Right) > maxRight {
maxRight = int(r.TeamList.Right)
}
return exists, maxRight, err
}

2
pkg/models/list_team.go
View File

@ -168,7 +168,7 @@ func (tl *TeamList) Delete() (err error) {
func (tl *TeamList) ReadAll(a web.Auth, search string, page int, perPage int) (result interface{}, resultCount int, totalItems int64, err error) {
// Check if the user can read the namespace
l := &List{ID: tl.ListID}
canRead, err := l.CanRead(a)
canRead, _, err := l.CanRead(a)
if err != nil {
return nil, 0, 0, err
}

2
pkg/models/list_users.go
View File

@ -174,7 +174,7 @@ func (lu *ListUser) Delete() (err error) {
func (lu *ListUser) ReadAll(a web.Auth, search string, page int, perPage int) (result interface{}, resultCount int, numberOfTotalItems int64, err error) {
// Check if the user has access to the list
l := &List{ID: lu.ListID}
canRead, err := l.CanRead(a)
canRead, _, err := l.CanRead(a)
if err != nil {
return nil, 0, 0, err
}

38
pkg/models/namespace_rights.go
View File

@ -23,16 +23,18 @@ import (
// CanWrite checks if a user has write access to a namespace
func (n *Namespace) CanWrite(a web.Auth) (bool, error) {
return n.checkRight(a, RightWrite, RightAdmin)
can, _, err := n.checkRight(a, RightWrite, RightAdmin)
return can, err
}
// IsAdmin returns true or false if the user is admin on that namespace or not
func (n *Namespace) IsAdmin(a web.Auth) (bool, error) {
return n.checkRight(a, RightAdmin)
is, _, err := n.checkRight(a, RightAdmin)
return is, err
}
// CanRead checks if a user has read access to that namespace
func (n *Namespace) CanRead(a web.Auth) (bool, error) {
func (n *Namespace) CanRead(a web.Auth) (bool, int, error) {
return n.checkRight(a, RightRead, RightWrite, RightAdmin)
}
@ -56,22 +58,22 @@ func (n *Namespace) CanCreate(a web.Auth) (bool, error) {
return true, nil
}
func (n *Namespace) checkRight(a web.Auth, rights ...Right) (bool, error) {
func (n *Namespace) checkRight(a web.Auth, rights ...Right) (bool, int, error) {
// If the auth is a link share, don't do anything
if _, is := a.(*LinkSharing); is {
return false, nil
return false, 0, nil
}
// Get the namespace and check the right
nn := &Namespace{ID: n.ID}
err := nn.GetSimpleByID()
if err != nil {
return false, err
return false, 0, err
}
if a.GetID() == n.OwnerID {
return true, nil
return true, int(RightAdmin), nil
}
/*
@ -104,7 +106,14 @@ func (n *Namespace) checkRight(a web.Auth, rights ...Right) (bool, error) {
))
}
exists, err := x.Select("namespaces.*").
type allRights struct {
UserNamespace NamespaceUser `xorm:"extends"`
TeamNamespace TeamNamespace `xorm:"extends"`
}
var maxRights = 0
r := &allRights{}
exists, err := x.Select("*").
Table("namespaces").
// User stuff
Join("LEFT", "users_namespace", "users_namespace.namespace_id = namespaces.id").
@ -118,6 +127,15 @@ func (n *Namespace) checkRight(a web.Auth, rights ...Right) (bool, error) {
),
builder.Eq{"namespaces.id": n.ID},
)).
Exist(&List{})
return exists, err
Exist(r)
// Figure out the max right and return it
if int(r.UserNamespace.Right) > maxRights {
maxRights = int(r.UserNamespace.Right)
}
if int(r.TeamNamespace.Right) > maxRights {
maxRights = int(r.TeamNamespace.Right)
}
return exists, maxRights, err
}

2
pkg/models/namespace_team.go
View File

@ -153,7 +153,7 @@ func (tn *TeamNamespace) Delete() (err error) {
func (tn *TeamNamespace) ReadAll(a web.Auth, search string, page int, perPage int) (result interface{}, resultCount int, numberOfTotalItems int64, err error) {
// Check if the user can read the namespace
n := Namespace{ID: tn.NamespaceID}
canRead, err := n.CanRead(a)
canRead, _, err := n.CanRead(a)
if err != nil {
return nil, 0, 0, err
}

6
pkg/models/namespace_test.go
View File

@ -47,7 +47,7 @@ func TestNamespace_Create(t *testing.T) {
assert.NoError(t, err)
// check if it really exists
allowed, err = dummynamespace.CanRead(doer)
allowed, _, err = dummynamespace.CanRead(doer)
assert.NoError(t, err)
assert.True(t, allowed)
err = dummynamespace.ReadOne()
@ -78,7 +78,7 @@ func TestNamespace_Create(t *testing.T) {
// Check if it was updated
assert.Equal(t, "Dolor sit amet.", dummynamespace.Description)
// Get it and check it again
allowed, err = dummynamespace.CanRead(doer)
allowed, _, err = dummynamespace.CanRead(doer)
assert.NoError(t, err)
assert.True(t, allowed)
err = dummynamespace.ReadOne()
@ -116,7 +116,7 @@ func TestNamespace_Create(t *testing.T) {
assert.True(t, IsErrNamespaceDoesNotExist(err))
// Check if it was successfully deleted
allowed, err = dummynamespace.CanRead(doer)
allowed, _, err = dummynamespace.CanRead(doer)
assert.False(t, allowed)
assert.Error(t, err)
assert.True(t, IsErrNamespaceDoesNotExist(err))

2
pkg/models/namespace_users.go
View File

@ -160,7 +160,7 @@ func (nu *NamespaceUser) Delete() (err error) {
func (nu *NamespaceUser) ReadAll(a web.Auth, search string, page int, perPage int) (result interface{}, resultCount int, numberOfTotalItems int64, err error) {
// Check if the user has access to the namespace
l := Namespace{ID: nu.NamespaceID}
canRead, err := l.CanRead(a)
canRead, _, err := l.CanRead(a)
if err != nil {
return nil, 0, 0, err
}

4
pkg/models/task_assignees.go
View File

@ -207,7 +207,7 @@ func (t *Task) addNewAssigneeByID(newAssigneeID int64, list *List) (err error) {
if err != nil {
return err
}
canRead, err := list.CanRead(newAssignee)
canRead, _, err := list.CanRead(newAssignee)
if err != nil {
return err
}
@ -247,7 +247,7 @@ func (la *TaskAssginee) ReadAll(a web.Auth, search string, page int, perPage int
return nil, 0, 0, err
}
can, err := task.CanRead(a)
can, _, err := task.CanRead(a)
if err != nil {
return nil, 0, 0, err
}

2
pkg/models/task_attachment_rights.go
View File

@ -19,7 +19,7 @@ package models
import "code.vikunja.io/web"
// CanRead checks if the user can see an attachment
func (ta *TaskAttachment) CanRead(a web.Auth) (bool, error) {
func (ta *TaskAttachment) CanRead(a web.Auth) (bool, int, error) {
t := &Task{ID: ta.TaskID}
return t.CanRead(a)
}

4
pkg/models/task_attachment_test.go
View File

@ -165,14 +165,14 @@ func TestTaskAttachment_Rights(t *testing.T) {
t.Run("Allowed", func(t *testing.T) {
db.LoadAndAssertFixtures(t)
ta := &TaskAttachment{TaskID: 1}
can, err := ta.CanRead(u)
can, _, err := ta.CanRead(u)
assert.NoError(t, err)
assert.True(t, can)
})
t.Run("Forbidden", func(t *testing.T) {
db.LoadAndAssertFixtures(t)
ta := &TaskAttachment{TaskID: 14}
can, err := ta.CanRead(u)
can, _, err := ta.CanRead(u)
assert.NoError(t, err)
assert.False(t, can)
})

2
pkg/models/task_collection.go
View File

@ -166,7 +166,7 @@ func (tf *TaskCollection) ReadAll(a web.Auth, search string, page int, perPage i
} else {
// Check the list exists and the user has acess on it
list := &List{ID: tf.ListID}
canRead, err := list.CanRead(a)
canRead, _, err := list.CanRead(a)
if err != nil {
return nil, 0, 0, err
}

2
pkg/models/task_comment_rights.go
View File

@ -20,7 +20,7 @@ package models
import "code.vikunja.io/web"
// CanRead checks if a user can read a comment
func (tc *TaskComment) CanRead(a web.Auth) (bool, error) {
func (tc *TaskComment) CanRead(a web.Auth) (bool, int, error) {
t := Task{ID: tc.TaskID}
return t.CanRead(a)
}

2
pkg/models/task_comments.go
View File

@ -165,7 +165,7 @@ func (tc *TaskComment) ReadOne() (err error) {
func (tc *TaskComment) ReadAll(auth web.Auth, search string, page int, perPage int) (result interface{}, resultCount int, numberOfTotalItems int64, err error) {
// Check if the user has access to the task
canRead, err := tc.CanRead(auth)
canRead, _, err := tc.CanRead(auth)
if err != nil {
return nil, 0, 0, err
}

2
pkg/models/task_relation_rights.go
View File

@ -42,7 +42,7 @@ func (rel *TaskRelation) CanCreate(a web.Auth) (bool, error) {
// We explicitly don't check if the two tasks are on the same list.
otherTask := &Task{ID: rel.OtherTaskID}
has, err = otherTask.CanRead(a)
has, _, err = otherTask.CanRead(a)
if err != nil {
return false, err
}

2
pkg/models/tasks_rights.go
View File

@ -38,7 +38,7 @@ func (t *Task) CanCreate(a web.Auth) (bool, error) {
}
// CanRead determines if a user can read a task
func (t *Task) CanRead(a web.Auth) (canRead bool, err error) {
func (t *Task) CanRead(a web.Auth) (canRead bool, maxRight int, err error) {
//return t.canDoTask(a)
// Get the task, error out if it doesn't exist
*t, err = GetTaskByIDSimple(t.ID)

14
pkg/models/teams_rights.go
View File

@ -60,9 +60,17 @@ func (t *Team) IsAdmin(a web.Auth) (bool, error) {
}
// CanRead returns true if the user has read access to the team
func (t *Team) CanRead(a web.Auth) (bool, error) {
func (t *Team) CanRead(a web.Auth) (bool, int, error) {
// Check if the user is in the team
return x.Where("team_id = ?", t.ID).
tm := &TeamMember{}
can, err := x.Where("team_id = ?", t.ID).
And("user_id = ?", a.GetID()).
Get(&TeamMember{})
Get(tm)
maxRights := 0
if tm.Admin {
maxRights = int(RightAdmin)
}
return can, maxRights, err
}

2
pkg/models/teams_rights_test.go
View File

@ -104,7 +104,7 @@ func TestTeam_CanDoSomething(t *testing.T) {
if got, _ := tm.CanUpdate(tt.args.a); got != tt.want["CanUpdate"] {
t.Errorf("Team.CanUpdate() = %v, want %v", got, tt.want["CanUpdate"])
}
if got, _ := tm.CanRead(tt.args.a); got != tt.want["CanRead"] {
if got, _, _ := tm.CanRead(tt.args.a); got != tt.want["CanRead"] {
t.Errorf("Team.CanRead() = %v, want %v", got, tt.want["CanRead"])
}
if got, _ := tm.IsAdmin(tt.args.a); got != tt.want["IsAdmin"] {