Let rights methods return errors (#64)

This commit is contained in:
konrad 2019-03-24 12:35:50 +00:00 committed by Gitea
parent 11e7c071ce
commit 47352d3ed4
44 changed files with 282 additions and 220 deletions

View File

@ -202,7 +202,7 @@ gocyclo-check:
go get -u github.com/fzipp/gocyclo; \ go get -u github.com/fzipp/gocyclo; \
go install $(GOFLAGS) github.com/fzipp/gocyclo; \ go install $(GOFLAGS) github.com/fzipp/gocyclo; \
fi fi
for S in $(GOFILES); do gocyclo -over 16 $$S || exit 1; done; for S in $(GOFILES); do gocyclo -over 17 $$S || exit 1; done;
.PHONY: static-check .PHONY: static-check
static-check: static-check:

2
go.mod
View File

@ -18,7 +18,7 @@ module code.vikunja.io/api
require ( require (
cloud.google.com/go v0.34.0 // indirect cloud.google.com/go v0.34.0 // indirect
code.vikunja.io/web v0.0.0-20190324080741-7bd881d9892a code.vikunja.io/web v0.0.0-20190324105229-0933ac082307
github.com/BurntSushi/toml v0.3.1 // indirect github.com/BurntSushi/toml v0.3.1 // indirect
github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc
github.com/asaskevich/govalidator v0.0.0-20180720115003-f9ffefc3facf github.com/asaskevich/govalidator v0.0.0-20180720115003-f9ffefc3facf

2
go.sum
View File

@ -9,6 +9,8 @@ code.vikunja.io/web v0.0.0-20190324080654-fcc8b45b7e2c h1:j1VTb5aOEQ3y4Y+u0RzU5l
code.vikunja.io/web v0.0.0-20190324080654-fcc8b45b7e2c/go.mod h1:PmGEu9qI7nbEKDn38H0SWgCoGO4GLdbjdlnWSzFi2PA= code.vikunja.io/web v0.0.0-20190324080654-fcc8b45b7e2c/go.mod h1:PmGEu9qI7nbEKDn38H0SWgCoGO4GLdbjdlnWSzFi2PA=
code.vikunja.io/web v0.0.0-20190324080741-7bd881d9892a h1:nB+kG5/gq0njK9/fEtYgzvLfd+U8i1I4m3CvYC+aN9k= code.vikunja.io/web v0.0.0-20190324080741-7bd881d9892a h1:nB+kG5/gq0njK9/fEtYgzvLfd+U8i1I4m3CvYC+aN9k=
code.vikunja.io/web v0.0.0-20190324080741-7bd881d9892a/go.mod h1:PmGEu9qI7nbEKDn38H0SWgCoGO4GLdbjdlnWSzFi2PA= code.vikunja.io/web v0.0.0-20190324080741-7bd881d9892a/go.mod h1:PmGEu9qI7nbEKDn38H0SWgCoGO4GLdbjdlnWSzFi2PA=
code.vikunja.io/web v0.0.0-20190324105229-0933ac082307 h1:t2E9v+k56RbvM5WNJF5BFFJDZrzM5l1Ua8qWdZYJAdA=
code.vikunja.io/web v0.0.0-20190324105229-0933ac082307/go.mod h1:PmGEu9qI7nbEKDn38H0SWgCoGO4GLdbjdlnWSzFi2PA=
github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ= github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=
github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU=
github.com/PuerkitoBio/purell v1.1.0 h1:rmGxhojJlM0tuKtfdvliR84CFHljx9ag64t2xmVkjK4= github.com/PuerkitoBio/purell v1.1.0 h1:rmGxhojJlM0tuKtfdvliR84CFHljx9ag64t2xmVkjK4=

View File

@ -17,7 +17,6 @@
package models package models
import ( import (
"code.vikunja.io/api/pkg/log"
"code.vikunja.io/web" "code.vikunja.io/web"
"github.com/imdario/mergo" "github.com/imdario/mergo"
) )
@ -53,12 +52,11 @@ func (bt *BulkTask) checkIfTasksAreOnTheSameList() (err error) {
} }
// CanUpdate checks if a user is allowed to update a task // CanUpdate checks if a user is allowed to update a task
func (bt *BulkTask) CanUpdate(a web.Auth) bool { func (bt *BulkTask) CanUpdate(a web.Auth) (bool, error) {
err := bt.checkIfTasksAreOnTheSameList() err := bt.checkIfTasksAreOnTheSameList()
if err != nil { if err != nil {
log.Log.Error("Error occurred during CanUpdate for BulkTask: %s", err) return false, err
return false
} }
// A user can update an task if he has write acces to its list // A user can update an task if he has write acces to its list

View File

@ -57,7 +57,7 @@ func TestBulkTask_Update(t *testing.T) {
Tasks: tt.fields.Tasks, Tasks: tt.fields.Tasks,
ListTask: tt.fields.ListTask, ListTask: tt.fields.ListTask,
} }
allowed := bt.CanUpdate(tt.fields.User) allowed, _ := bt.CanUpdate(tt.fields.User)
if !allowed != tt.wantForbidden { if !allowed != tt.wantForbidden {
t.Errorf("BulkTask.Update() want forbidden, got %v, want %v", allowed, tt.wantForbidden) t.Errorf("BulkTask.Update() want forbidden, got %v, want %v", allowed, tt.wantForbidden)
} }

View File

@ -17,51 +17,48 @@
package models package models
import ( import (
"code.vikunja.io/api/pkg/log"
"code.vikunja.io/web" "code.vikunja.io/web"
"github.com/go-xorm/builder" "github.com/go-xorm/builder"
) )
// CanUpdate checks if a user can update a label // CanUpdate checks if a user can update a label
func (l *Label) CanUpdate(a web.Auth) bool { func (l *Label) CanUpdate(a web.Auth) (bool, error) {
return l.isLabelOwner(a) // Only owners should be allowed to update a label return l.isLabelOwner(a) // Only owners should be allowed to update a label
} }
// CanDelete checks if a user can delete a label // CanDelete checks if a user can delete a label
func (l *Label) CanDelete(a web.Auth) bool { func (l *Label) CanDelete(a web.Auth) (bool, error) {
return l.isLabelOwner(a) // Only owners should be allowed to delete a label return l.isLabelOwner(a) // Only owners should be allowed to delete a label
} }
// CanRead checks if a user can read a label // CanRead checks if a user can read a label
func (l *Label) CanRead(a web.Auth) bool { func (l *Label) CanRead(a web.Auth) (bool, error) {
return l.hasAccessToLabel(a) return l.hasAccessToLabel(a)
} }
// CanCreate checks if the user can create a label // CanCreate checks if the user can create a label
// Currently a dummy. // Currently a dummy.
func (l *Label) CanCreate(a web.Auth) bool { func (l *Label) CanCreate(a web.Auth) (bool, error) {
return true return true, nil
} }
func (l *Label) isLabelOwner(a web.Auth) bool { func (l *Label) isLabelOwner(a web.Auth) (bool, error) {
u := getUserForRights(a) u := getUserForRights(a)
lorig, err := getLabelByIDSimple(l.ID) lorig, err := getLabelByIDSimple(l.ID)
if err != nil { if err != nil {
log.Log.Errorf("Error occurred during isLabelOwner for Label: %v", err) return false, err
return false
} }
return lorig.CreatedByID == u.ID return lorig.CreatedByID == u.ID, nil
} }
// Helper method to check if a user can see a specific label // Helper method to check if a user can see a specific label
func (l *Label) hasAccessToLabel(a web.Auth) bool { func (l *Label) hasAccessToLabel(a web.Auth) (bool, error) {
u := getUserForRights(a) u := getUserForRights(a)
// Get all tasks // Get all tasks
taskIDs, err := getUserTaskIDs(u) taskIDs, err := getUserTaskIDs(u)
if err != nil { if err != nil {
log.Log.Errorf("Error occurred during hasAccessToLabel for Label: %v", err) return false, err
return false
} }
// Get all labels associated with these tasks // Get all labels associated with these tasks
@ -74,10 +71,5 @@ func (l *Label) hasAccessToLabel(a web.Auth) bool {
And("labels.id = ?", l.ID). And("labels.id = ?", l.ID).
GroupBy("labels.id"). GroupBy("labels.id").
Exist(&labels) Exist(&labels)
if err != nil { return has, err
log.Log.Errorf("Error occurred during hasAccessToLabel for Label: %v", err)
return false
}
return has
} }

View File

@ -114,7 +114,11 @@ func (lt *LabelTask) ReadAll(search string, a web.Auth, page int) (labels interf
return nil, err return nil, err
} }
if !task.CanRead(a) { canRead, err := task.CanRead(a)
if err != nil {
return nil, err
}
if !canRead {
return nil, ErrNoRightToSeeTask{lt.TaskID, u.ID} return nil, ErrNoRightToSeeTask{lt.TaskID, u.ID}
} }
@ -256,7 +260,11 @@ func (t *ListTask) updateTaskLabels(creator web.Auth, labels []*Label) (err erro
} }
// Check if the user has the rights to see the label he is about to add // Check if the user has the rights to see the label he is about to add
if !label.hasAccessToLabel(creator) { hasAccessToLabel, err := label.hasAccessToLabel(creator)
if err != nil {
return err
}
if !hasAccessToLabel {
user, _ := creator.(*User) user, _ := creator.(*User)
return ErrUserHasNoAccessToLabel{LabelID: l.ID, UserID: user.ID} return ErrUserHasNoAccessToLabel{LabelID: l.ID, UserID: user.ID}
} }

View File

@ -17,51 +17,61 @@
package models package models
import ( import (
"code.vikunja.io/api/pkg/log"
"code.vikunja.io/web" "code.vikunja.io/web"
) )
// CanCreate checks if a user can add a label to a task // CanCreate checks if a user can add a label to a task
func (lt *LabelTask) CanCreate(a web.Auth) bool { func (lt *LabelTask) CanCreate(a web.Auth) (bool, error) {
label, err := getLabelByIDSimple(lt.LabelID) label, err := getLabelByIDSimple(lt.LabelID)
if err != nil { if err != nil {
log.Log.Errorf("Error during CanCreate for LabelTask: %v", err) return false, err
return false
} }
return label.hasAccessToLabel(a) && canDoLabelTask(lt.TaskID, 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
}
canDoLabelTask, err := canDoLabelTask(lt.TaskID, a)
if err != nil {
return false, err
}
return hasAccessTolabel && canDoLabelTask, nil
} }
// CanDelete checks if a user can delete a label from a task // CanDelete checks if a user can delete a label from a task
func (lt *LabelTask) CanDelete(a web.Auth) bool { func (lt *LabelTask) CanDelete(a web.Auth) (bool, error) {
if !canDoLabelTask(lt.TaskID, a) { canDoLabelTask, err := canDoLabelTask(lt.TaskID, a)
return false if err != nil {
return false, err
}
if !canDoLabelTask {
return false, nil
} }
// We don't care here if the label exists or not. The only relevant thing here is if the relation already exists, // We don't care here if the label exists or not. The only relevant thing here is if the relation already exists,
// throw an error. // throw an error.
exists, err := x.Exist(&LabelTask{LabelID: lt.LabelID, TaskID: lt.TaskID}) exists, err := x.Exist(&LabelTask{LabelID: lt.LabelID, TaskID: lt.TaskID})
if err != nil { if err != nil {
log.Log.Errorf("Error during CanDelete for LabelTask: %v", err) return false, err
return false
} }
return exists return exists, err
} }
// CanCreate determines if a user can update a labeltask // CanCreate determines if a user can update a labeltask
func (ltb *LabelTaskBulk) CanCreate(a web.Auth) bool { func (ltb *LabelTaskBulk) CanCreate(a web.Auth) (bool, error) {
return canDoLabelTask(ltb.TaskID, a) return canDoLabelTask(ltb.TaskID, a)
} }
// Helper function to check if a user can write to a task // Helper function to check if a user can write to a task
// + is able to see the label // + is able to see the label
// always the same check for either deleting or adding a label to a task // always the same check for either deleting or adding a label to a task
func canDoLabelTask(taskID int64, a web.Auth) bool { func canDoLabelTask(taskID int64, a web.Auth) (bool, error) {
// A user can add a label to a task if he can write to the task // A user can add a label to a task if he can write to the task
task, err := getTaskByIDSimple(taskID) task, err := getTaskByIDSimple(taskID)
if err != nil { if err != nil {
log.Log.Error("Error occurred during canDoLabelTask for LabelTask: %v", err) return false, err
return false
} }
return task.CanUpdate(a) return task.CanUpdate(a)
} }

View File

@ -178,7 +178,8 @@ func TestLabelTask_Create(t *testing.T) {
CRUDable: tt.fields.CRUDable, CRUDable: tt.fields.CRUDable,
Rights: tt.fields.Rights, Rights: tt.fields.Rights,
} }
if !l.CanCreate(tt.args.a) && !tt.wantForbidden { allowed, _ := l.CanCreate(tt.args.a)
if !allowed && !tt.wantForbidden {
t.Errorf("LabelTask.CanCreate() forbidden, want %v", tt.wantForbidden) t.Errorf("LabelTask.CanCreate() forbidden, want %v", tt.wantForbidden)
} }
err := l.Create(tt.args.a) err := l.Create(tt.args.a)
@ -264,7 +265,8 @@ func TestLabelTask_Delete(t *testing.T) {
CRUDable: tt.fields.CRUDable, CRUDable: tt.fields.CRUDable,
Rights: tt.fields.Rights, Rights: tt.fields.Rights,
} }
if !l.CanDelete(tt.auth) && !tt.wantForbidden { allowed, _ := l.CanDelete(tt.auth)
if !allowed && !tt.wantForbidden {
t.Errorf("LabelTask.CanDelete() forbidden, want %v", tt.wantForbidden) t.Errorf("LabelTask.CanDelete() forbidden, want %v", tt.wantForbidden)
} }
err := l.Delete() err := l.Delete()

View File

@ -217,7 +217,8 @@ func TestLabel_ReadOne(t *testing.T) {
Rights: tt.fields.Rights, Rights: tt.fields.Rights,
} }
if !l.CanRead(tt.auth) && !tt.wantForbidden { allowed, _ := l.CanRead(tt.auth)
if !allowed && !tt.wantForbidden {
t.Errorf("Label.CanRead() forbidden, want %v", tt.wantForbidden) t.Errorf("Label.CanRead() forbidden, want %v", tt.wantForbidden)
} }
err := l.ReadOne() err := l.ReadOne()
@ -283,7 +284,8 @@ func TestLabel_Create(t *testing.T) {
CRUDable: tt.fields.CRUDable, CRUDable: tt.fields.CRUDable,
Rights: tt.fields.Rights, Rights: tt.fields.Rights,
} }
if !l.CanCreate(tt.args.a) && !tt.wantForbidden { allowed, _ := l.CanCreate(tt.args.a)
if !allowed && !tt.wantForbidden {
t.Errorf("Label.CanCreate() forbidden, want %v", tt.wantForbidden) t.Errorf("Label.CanCreate() forbidden, want %v", tt.wantForbidden)
} }
if err := l.Create(tt.args.a); (err != nil) != tt.wantErr { if err := l.Create(tt.args.a); (err != nil) != tt.wantErr {
@ -364,7 +366,8 @@ func TestLabel_Update(t *testing.T) {
CRUDable: tt.fields.CRUDable, CRUDable: tt.fields.CRUDable,
Rights: tt.fields.Rights, Rights: tt.fields.Rights,
} }
if !l.CanUpdate(tt.auth) && !tt.wantForbidden { allowed, _ := l.CanUpdate(tt.auth)
if !allowed && !tt.wantForbidden {
t.Errorf("Label.CanUpdate() forbidden, want %v", tt.wantForbidden) t.Errorf("Label.CanUpdate() forbidden, want %v", tt.wantForbidden)
} }
if err := l.Update(); (err != nil) != tt.wantErr { if err := l.Update(); (err != nil) != tt.wantErr {
@ -441,7 +444,8 @@ func TestLabel_Delete(t *testing.T) {
CRUDable: tt.fields.CRUDable, CRUDable: tt.fields.CRUDable,
Rights: tt.fields.Rights, Rights: tt.fields.Rights,
} }
if !l.CanDelete(tt.auth) && !tt.wantForbidden { allowed, _ := l.CanDelete(tt.auth)
if !allowed && !tt.wantForbidden {
t.Errorf("Label.CanDelete() forbidden, want %v", tt.wantForbidden) t.Errorf("Label.CanDelete() forbidden, want %v", tt.wantForbidden)
} }
if err := l.Delete(); (err != nil) != tt.wantErr { if err := l.Delete(); (err != nil) != tt.wantErr {

View File

@ -37,7 +37,8 @@ func TestList_Create(t *testing.T) {
} }
// Check if the user can create // Check if the user can create
assert.True(t, dummylist.CanCreate(&doer)) allowed, _ := dummylist.CanCreate(&doer)
assert.True(t, allowed)
// Create it // Create it
err = dummylist.Create(&doer) err = dummylist.Create(&doer)
@ -52,16 +53,19 @@ func TestList_Create(t *testing.T) {
assert.Equal(t, dummylist.OwnerID, doer.ID) assert.Equal(t, dummylist.OwnerID, doer.ID)
// Check if the user can see it // Check if the user can see it
assert.True(t, dummylist.CanRead(&doer)) allowed, _ = dummylist.CanRead(&doer)
assert.True(t, allowed)
// Try updating a list // Try updating a list
assert.True(t, dummylist.CanUpdate(&doer)) allowed, _ = dummylist.CanUpdate(&doer)
assert.True(t, allowed)
dummylist.Description = "Lorem Ipsum dolor sit amet." dummylist.Description = "Lorem Ipsum dolor sit amet."
err = dummylist.Update() err = dummylist.Update()
assert.NoError(t, err) assert.NoError(t, err)
// Delete it // Delete it
assert.True(t, dummylist.CanDelete(&doer)) allowed, _ = dummylist.CanDelete(&doer)
assert.True(t, allowed)
err = dummylist.Delete() err = dummylist.Delete()
assert.NoError(t, err) assert.NoError(t, err)

View File

@ -17,71 +17,76 @@
package models package models
import ( import (
"code.vikunja.io/api/pkg/log"
"code.vikunja.io/web" "code.vikunja.io/web"
"github.com/go-xorm/builder" "github.com/go-xorm/builder"
) )
// CanWrite return whether the user can write on that list or not // CanWrite return whether the user can write on that list or not
func (l *List) CanWrite(a web.Auth) bool { func (l *List) CanWrite(a web.Auth) (bool, error) {
// Get the list and check the right // Get the list and check the right
originalList := &List{ID: l.ID} originalList := &List{ID: l.ID}
err := originalList.GetSimpleByID() err := originalList.GetSimpleByID()
if err != nil { if err != nil {
log.Log.Error("Error occurred during CanWrite for List: %s", err) return false, err
return false
} }
user := getUserForRights(a) user := getUserForRights(a)
// Check all the things
// Check if the user is either owner or can write to the list // Check if the user is either owner or can write to the list
return originalList.isOwner(user) || originalList.checkRight(user, RightWrite, RightAdmin) if originalList.isOwner(user) {
return true, nil
}
return originalList.checkRight(user, RightWrite, RightAdmin)
} }
// CanRead checks if a user has read access to a list // CanRead checks if a user has read access to a list
func (l *List) CanRead(a web.Auth) bool { func (l *List) CanRead(a web.Auth) (bool, error) {
user := getUserForRights(a) user := getUserForRights(a)
// Check all the things
// Check if the user is either owner or can read // Check if the user is either owner or can read
// We can do this without first looking up the list because CanRead() is called after ReadOne() // We can do this without first looking up the list because CanRead() is called after ReadOne()
// So are sure the list exists // So are sure the list exists
return l.isOwner(user) || l.checkRight(user, RightRead, RightWrite, RightAdmin) if l.isOwner(user) {
return true, nil
}
return l.checkRight(user, RightRead, RightWrite, RightAdmin)
} }
// CanUpdate checks if the user can update a list // CanUpdate checks if the user can update a list
func (l *List) CanUpdate(a web.Auth) bool { func (l *List) CanUpdate(a web.Auth) (bool, error) {
return l.CanWrite(a) return l.CanWrite(a)
} }
// CanDelete checks if the user can delete a list // CanDelete checks if the user can delete a list
func (l *List) CanDelete(a web.Auth) bool { func (l *List) CanDelete(a web.Auth) (bool, error) {
return l.IsAdmin(a) return l.IsAdmin(a)
} }
// CanCreate checks if the user can update a list // CanCreate checks if the user can update a list
func (l *List) CanCreate(a web.Auth) bool { func (l *List) CanCreate(a web.Auth) (bool, error) {
// A user can create a list if he has write access to the namespace // A user can create a list if he has write access to the namespace
n := &Namespace{ID: l.NamespaceID} n := &Namespace{ID: l.NamespaceID}
return n.CanWrite(a) return n.CanWrite(a)
} }
// IsAdmin returns whether the user has admin rights on the list or not // IsAdmin returns whether the user has admin rights on the list or not
func (l *List) IsAdmin(a web.Auth) bool { func (l *List) IsAdmin(a web.Auth) (bool, error) {
user := getUserForRights(a) user := getUserForRights(a)
originalList := &List{ID: l.ID} originalList := &List{ID: l.ID}
err := originalList.GetSimpleByID() err := originalList.GetSimpleByID()
if err != nil { if err != nil {
log.Log.Error("Error occurred during IsAdmin for List: %s", err) return false, err
return false
} }
// Check all the things // Check all the things
// Check if the user is either owner or can write to the list // Check if the user is either owner or can write to the list
// Owners are always admins // Owners are always admins
return originalList.isOwner(user) || originalList.checkRight(user, RightAdmin) if originalList.isOwner(user) {
return true, nil
}
return originalList.checkRight(user, RightAdmin)
} }
// Little helper function to check if a user is list owner // Little helper function to check if a user is list owner
@ -90,7 +95,7 @@ func (l *List) isOwner(u *User) bool {
} }
// Checks n different rights for any given user // Checks n different rights for any given user
func (l *List) checkRight(user *User, rights ...Right) bool { func (l *List) checkRight(user *User, rights ...Right) (bool, error) {
/* /*
The following loop creates an sql condition like this one: The following loop creates an sql condition like this one:
@ -149,10 +154,5 @@ func (l *List) checkRight(user *User, rights ...Right) bool {
builder.Eq{"l.id": l.ID}, builder.Eq{"l.id": l.ID},
)). )).
Exist(&List{}) Exist(&List{})
if err != nil { return exists, err
log.Log.Error("Error occurred during checkRight for list: %s", err)
return false
}
return exists
} }

View File

@ -175,7 +175,11 @@ func (t *ListTask) addNewAssigneeByID(newAssigneeID int64, list *List) (err erro
if err != nil { if err != nil {
return err return err
} }
if !list.CanRead(&newAssignee) { canRead, err := list.CanRead(&newAssignee)
if err != nil {
return err
}
if !canRead {
return ErrUserDoesNotHaveAccessToList{list.ID, newAssigneeID} return ErrUserDoesNotHaveAccessToList{list.ID, newAssigneeID}
} }

View File

@ -17,31 +17,29 @@
package models package models
import ( import (
"code.vikunja.io/api/pkg/log"
"code.vikunja.io/web" "code.vikunja.io/web"
) )
// CanCreate checks if a user can add a new assignee // CanCreate checks if a user can add a new assignee
func (la *ListTaskAssginee) CanCreate(a web.Auth) bool { func (la *ListTaskAssginee) CanCreate(a web.Auth) (bool, error) {
return canDoListTaskAssingee(la.TaskID, a) return canDoListTaskAssingee(la.TaskID, a)
} }
// CanCreate checks if a user can add a new assignee // CanCreate checks if a user can add a new assignee
func (ba *BulkAssignees) CanCreate(a web.Auth) bool { func (ba *BulkAssignees) CanCreate(a web.Auth) (bool, error) {
return canDoListTaskAssingee(ba.TaskID, a) return canDoListTaskAssingee(ba.TaskID, a)
} }
// CanDelete checks if a user can delete an assignee // CanDelete checks if a user can delete an assignee
func (la *ListTaskAssginee) CanDelete(a web.Auth) bool { func (la *ListTaskAssginee) CanDelete(a web.Auth) (bool, error) {
return canDoListTaskAssingee(la.TaskID, a) return canDoListTaskAssingee(la.TaskID, a)
} }
func canDoListTaskAssingee(taskID int64, a web.Auth) bool { func canDoListTaskAssingee(taskID int64, a web.Auth) (bool, error) {
// Check if the current user can edit the list // Check if the current user can edit the list
list, err := GetListSimplByTaskID(taskID) list, err := GetListSimplByTaskID(taskID)
if err != nil { if err != nil {
log.Log.Errorf("Error during canDoListTaskAssingee for ListTaskAssginee: %v", err) return false, err
return false
} }
return list.CanCreate(a) return list.CanCreate(a)
} }

View File

@ -17,22 +17,21 @@
package models package models
import ( import (
"code.vikunja.io/api/pkg/log"
"code.vikunja.io/web" "code.vikunja.io/web"
) )
// CanDelete checks if the user can delete an task // CanDelete checks if the user can delete an task
func (t *ListTask) CanDelete(a web.Auth) bool { func (t *ListTask) CanDelete(a web.Auth) (bool, error) {
return t.canDoListTask(a) return t.canDoListTask(a)
} }
// CanUpdate determines if a user has the right to update a list task // CanUpdate determines if a user has the right to update a list task
func (t *ListTask) CanUpdate(a web.Auth) bool { func (t *ListTask) CanUpdate(a web.Auth) (bool, error) {
return t.canDoListTask(a) return t.canDoListTask(a)
} }
// CanCreate determines if a user has the right to create a list task // CanCreate determines if a user has the right to create a list task
func (t *ListTask) CanCreate(a web.Auth) bool { func (t *ListTask) CanCreate(a web.Auth) (bool, error) {
doer := getUserForRights(a) doer := getUserForRights(a)
// A user can do a task if he has write acces to its list // A user can do a task if he has write acces to its list
@ -41,26 +40,24 @@ func (t *ListTask) CanCreate(a web.Auth) bool {
} }
// CanRead determines if a user can read a task // CanRead determines if a user can read a task
func (t *ListTask) CanRead(a web.Auth) bool { func (t *ListTask) CanRead(a web.Auth) (bool, error) {
// A user can read a task if it has access to the list // A user can read a task if it has access to the list
list := &List{ID: t.ListID} list := &List{ID: t.ListID}
err := list.GetSimpleByID() err := list.GetSimpleByID()
if err != nil { if err != nil {
log.Log.Error("Error occurred during CanRead for ListTask: %s", err) return false, err
return false
} }
return list.CanRead(a) return list.CanRead(a)
} }
// Helper function to check if a user can do stuff on a list task // Helper function to check if a user can do stuff on a list task
func (t *ListTask) canDoListTask(a web.Auth) bool { func (t *ListTask) canDoListTask(a web.Auth) (bool, error) {
doer := getUserForRights(a) doer := getUserForRights(a)
// Get the task // Get the task
lI, err := getTaskByIDSimple(t.ID) lI, err := getTaskByIDSimple(t.ID)
if err != nil { if err != nil {
log.Log.Error("Error occurred during canDoListTask (getTaskByIDSimple) for ListTask: %s", err) return false, err
return false
} }
// A user can do a task if he has write acces to its list // A user can do a task if he has write acces to its list

View File

@ -35,14 +35,16 @@ func TestListTask_Create(t *testing.T) {
doer, err := GetUserByID(1) doer, err := GetUserByID(1)
assert.NoError(t, err) assert.NoError(t, err)
assert.True(t, listtask.CanCreate(&doer)) allowed, _ := listtask.CanCreate(&doer)
assert.True(t, allowed)
err = listtask.Create(&doer) err = listtask.Create(&doer)
assert.NoError(t, err) assert.NoError(t, err)
// Update it // Update it
listtask.Text = "Test34" listtask.Text = "Test34"
assert.True(t, listtask.CanUpdate(&doer)) allowed, _ = listtask.CanUpdate(&doer)
assert.True(t, allowed)
err = listtask.Update() err = listtask.Update()
assert.NoError(t, err) assert.NoError(t, err)
@ -52,7 +54,8 @@ func TestListTask_Create(t *testing.T) {
assert.Equal(t, li.Text, "Test34") assert.Equal(t, li.Text, "Test34")
// Delete the task // Delete the task
assert.True(t, listtask.CanDelete(&doer)) allowed, _ = listtask.CanDelete(&doer)
assert.True(t, allowed)
err = listtask.Delete() err = listtask.Delete()
assert.NoError(t, err) assert.NoError(t, err)

View File

@ -43,7 +43,11 @@ func (lu *ListUser) ReadAll(search string, a web.Auth, page int) (interface{}, e
if err := l.GetSimpleByID(); err != nil { if err := l.GetSimpleByID(); err != nil {
return nil, err return nil, err
} }
if !l.CanRead(u) { canRead, err := l.CanRead(u)
if err != nil {
return nil, err
}
if !canRead {
return nil, ErrNeedToHaveListReadAccess{UserID: u.ID, ListID: lu.ListID} return nil, ErrNeedToHaveListReadAccess{UserID: u.ID, ListID: lu.ListID}
} }

View File

@ -21,21 +21,21 @@ import (
) )
// CanCreate checks if the user can create a new user <-> list relation // CanCreate checks if the user can create a new user <-> list relation
func (lu *ListUser) CanCreate(a web.Auth) bool { func (lu *ListUser) CanCreate(a web.Auth) (bool, error) {
// Get the list and check if the user has write access on it // Get the list and check if the user has write access on it
l := List{ID: lu.ListID} l := List{ID: lu.ListID}
return l.CanWrite(a) return l.CanWrite(a)
} }
// CanDelete checks if the user can delete a user <-> list relation // CanDelete checks if the user can delete a user <-> list relation
func (lu *ListUser) CanDelete(a web.Auth) bool { func (lu *ListUser) CanDelete(a web.Auth) (bool, error) {
// Get the list and check if the user has write access on it // Get the list and check if the user has write access on it
l := List{ID: lu.ListID} l := List{ID: lu.ListID}
return l.CanWrite(a) return l.CanWrite(a)
} }
// CanUpdate checks if the user can update a user <-> list relation // CanUpdate checks if the user can update a user <-> list relation
func (lu *ListUser) CanUpdate(a web.Auth) bool { func (lu *ListUser) CanUpdate(a web.Auth) (bool, error) {
// Get the list and check if the user has write access on it // Get the list and check if the user has write access on it
l := List{ID: lu.ListID} l := List{ID: lu.ListID}
return l.CanWrite(a) return l.CanWrite(a)

View File

@ -85,13 +85,13 @@ func TestListUser_CanDoSomething(t *testing.T) {
CRUDable: tt.fields.CRUDable, CRUDable: tt.fields.CRUDable,
Rights: tt.fields.Rights, Rights: tt.fields.Rights,
} }
if got := lu.CanCreate(tt.args.a); got != tt.want["CanCreate"] { if got, _ := lu.CanCreate(tt.args.a); got != tt.want["CanCreate"] {
t.Errorf("ListUser.CanCreate() = %v, want %v", got, tt.want["CanCreate"]) t.Errorf("ListUser.CanCreate() = %v, want %v", got, tt.want["CanCreate"])
} }
if got := lu.CanDelete(tt.args.a); got != tt.want["CanDelete"] { if got, _ := lu.CanDelete(tt.args.a); got != tt.want["CanDelete"] {
t.Errorf("ListUser.CanDelete() = %v, want %v", got, tt.want["CanDelete"]) t.Errorf("ListUser.CanDelete() = %v, want %v", got, tt.want["CanDelete"])
} }
if got := lu.CanUpdate(tt.args.a); got != tt.want["CanUpdate"] { if got, _ := lu.CanUpdate(tt.args.a); got != tt.want["CanUpdate"] {
t.Errorf("ListUser.CanUpdate() = %v, want %v", got, tt.want["CanUpdate"]) t.Errorf("ListUser.CanUpdate() = %v, want %v", got, tt.want["CanUpdate"])
} }
}) })

View File

@ -17,59 +17,71 @@
package models package models
import ( import (
"code.vikunja.io/api/pkg/log"
"code.vikunja.io/web" "code.vikunja.io/web"
"github.com/go-xorm/builder" "github.com/go-xorm/builder"
) )
// CanWrite checks if a user has write access to a namespace // CanWrite checks if a user has write access to a namespace
func (n *Namespace) CanWrite(a web.Auth) bool { func (n *Namespace) CanWrite(a web.Auth) (bool, error) {
// Get the namespace and check the right // Get the namespace and check the right
originalNamespace := &Namespace{ID: n.ID} originalNamespace := &Namespace{ID: n.ID}
err := originalNamespace.GetSimpleByID() err := originalNamespace.GetSimpleByID()
if err != nil { if err != nil {
log.Log.Error("Error occurred during CanWrite for Namespace: %s", err) return false, err
return false
} }
u := getUserForRights(a) u := getUserForRights(a)
return originalNamespace.isOwner(u) || originalNamespace.checkRight(u, RightWrite, RightAdmin) if originalNamespace.isOwner(u) {
return true, nil
}
return originalNamespace.checkRight(u, RightWrite, RightAdmin)
} }
// IsAdmin returns true or false if the user is admin on that namespace or not // IsAdmin returns true or false if the user is admin on that namespace or not
func (n *Namespace) IsAdmin(a web.Auth) bool { func (n *Namespace) IsAdmin(a web.Auth) (bool, error) {
originalNamespace := &Namespace{ID: n.ID} originalNamespace := &Namespace{ID: n.ID}
err := originalNamespace.GetSimpleByID() err := originalNamespace.GetSimpleByID()
if err != nil { if err != nil {
log.Log.Error("Error occurred during IsAdmin for Namespace: %s", err) return false, err
return false
} }
u := getUserForRights(a) u := getUserForRights(a)
return originalNamespace.isOwner(u) || originalNamespace.checkRight(u, RightAdmin) if originalNamespace.isOwner(u) {
return true, nil
}
return originalNamespace.checkRight(u, RightAdmin)
} }
// CanRead checks if a user has read access to that namespace // CanRead checks if a user has read access to that namespace
func (n *Namespace) CanRead(a web.Auth) bool { func (n *Namespace) CanRead(a web.Auth) (bool, error) {
originalNamespace := &Namespace{ID: n.ID}
err := originalNamespace.GetSimpleByID()
if err != nil {
return false, err
}
u := getUserForRights(a) u := getUserForRights(a)
return n.isOwner(u) || n.checkRight(u, RightRead, RightWrite, RightAdmin) if originalNamespace.isOwner(u) {
return true, nil
}
return n.checkRight(u, RightRead, RightWrite, RightAdmin)
} }
// CanUpdate checks if the user can update the namespace // CanUpdate checks if the user can update the namespace
func (n *Namespace) CanUpdate(a web.Auth) bool { func (n *Namespace) CanUpdate(a web.Auth) (bool, error) {
return n.IsAdmin(a) return n.IsAdmin(a)
} }
// CanDelete checks if the user can delete a namespace // CanDelete checks if the user can delete a namespace
func (n *Namespace) CanDelete(a web.Auth) bool { func (n *Namespace) CanDelete(a web.Auth) (bool, error) {
return n.IsAdmin(a) return n.IsAdmin(a)
} }
// CanCreate checks if the user can create a new namespace // CanCreate checks if the user can create a new namespace
func (n *Namespace) CanCreate(a web.Auth) bool { func (n *Namespace) CanCreate(a web.Auth) (bool, error) {
// This is currently a dummy function, later on we could imagine global limits etc. // This is currently a dummy function, later on we could imagine global limits etc.
return true return true, nil
} }
// Small helper function to check if a user owns the namespace // Small helper function to check if a user owns the namespace
@ -77,7 +89,7 @@ func (n *Namespace) isOwner(user *User) bool {
return n.OwnerID == user.ID return n.OwnerID == user.ID
} }
func (n *Namespace) checkRight(user *User, rights ...Right) bool { func (n *Namespace) checkRight(user *User, rights ...Right) (bool, error) {
/* /*
The following loop creates an sql condition like this one: The following loop creates an sql condition like this one:
@ -124,10 +136,5 @@ func (n *Namespace) checkRight(user *User, rights ...Right) bool {
builder.Eq{"namespaces.id": n.ID}, builder.Eq{"namespaces.id": n.ID},
)). )).
Exist(&List{}) Exist(&List{})
if err != nil { return exists, err
log.Log.Error("Error occurred during checkRight for namespace: %s", err)
return false
}
return exists
} }

View File

@ -37,12 +37,14 @@ func TestNamespace_Create(t *testing.T) {
assert.NoError(t, err) assert.NoError(t, err)
// Try creating it // Try creating it
assert.True(t, dummynamespace.CanCreate(&doer)) allowed, _ := dummynamespace.CanCreate(&doer)
assert.True(t, allowed)
err = dummynamespace.Create(&doer) err = dummynamespace.Create(&doer)
assert.NoError(t, err) assert.NoError(t, err)
// check if it really exists // check if it really exists
assert.True(t, dummynamespace.CanRead(&doer)) allowed, _ = dummynamespace.CanRead(&doer)
assert.True(t, allowed)
newOne := Namespace{ID: dummynamespace.ID} newOne := Namespace{ID: dummynamespace.ID}
err = newOne.ReadOne() err = newOne.ReadOne()
assert.NoError(t, err) assert.NoError(t, err)
@ -62,7 +64,8 @@ func TestNamespace_Create(t *testing.T) {
assert.True(t, IsErrUserDoesNotExist(err)) assert.True(t, IsErrUserDoesNotExist(err))
// Update it // Update it
assert.True(t, dummynamespace.CanUpdate(&doer)) allowed, _ = dummynamespace.CanUpdate(&doer)
assert.True(t, allowed)
dummynamespace.Description = "Dolor sit amet." dummynamespace.Description = "Dolor sit amet."
err = dummynamespace.Update() err = dummynamespace.Update()
assert.NoError(t, err) assert.NoError(t, err)
@ -86,7 +89,8 @@ func TestNamespace_Create(t *testing.T) {
assert.True(t, IsErrNamespaceDoesNotExist(err)) assert.True(t, IsErrNamespaceDoesNotExist(err))
// Delete it // Delete it
assert.True(t, dummynamespace.CanDelete(&doer)) allowed, _ = dummynamespace.CanDelete(&doer)
assert.True(t, allowed)
err = dummynamespace.Delete() err = dummynamespace.Delete()
assert.NoError(t, err) assert.NoError(t, err)

View File

@ -43,7 +43,11 @@ func (nu *NamespaceUser) ReadAll(search string, a web.Auth, page int) (interface
if err != nil { if err != nil {
return nil, err return nil, err
} }
if !l.CanRead(u) { canRead, err := l.CanRead(u)
if err != nil {
return nil, err
}
if !canRead {
return nil, ErrNeedToHaveNamespaceReadAccess{} return nil, ErrNeedToHaveNamespaceReadAccess{}
} }

View File

@ -21,19 +21,19 @@ import (
) )
// CanCreate checks if the user can create a new user <-> namespace relation // CanCreate checks if the user can create a new user <-> namespace relation
func (nu *NamespaceUser) CanCreate(a web.Auth) bool { func (nu *NamespaceUser) CanCreate(a web.Auth) (bool, error) {
n := &Namespace{ID: nu.NamespaceID} n := &Namespace{ID: nu.NamespaceID}
return n.CanWrite(a) return n.CanWrite(a)
} }
// CanDelete checks if the user can delete a user <-> namespace relation // CanDelete checks if the user can delete a user <-> namespace relation
func (nu *NamespaceUser) CanDelete(a web.Auth) bool { func (nu *NamespaceUser) CanDelete(a web.Auth) (bool, error) {
n := &Namespace{ID: nu.NamespaceID} n := &Namespace{ID: nu.NamespaceID}
return n.CanWrite(a) return n.CanWrite(a)
} }
// CanUpdate checks if the user can update a user <-> namespace relation // CanUpdate checks if the user can update a user <-> namespace relation
func (nu *NamespaceUser) CanUpdate(a web.Auth) bool { func (nu *NamespaceUser) CanUpdate(a web.Auth) (bool, error) {
n := &Namespace{ID: nu.NamespaceID} n := &Namespace{ID: nu.NamespaceID}
return n.CanWrite(a) return n.CanWrite(a)
} }

View File

@ -85,13 +85,13 @@ func TestNamespaceUser_CanDoSomething(t *testing.T) {
CRUDable: tt.fields.CRUDable, CRUDable: tt.fields.CRUDable,
Rights: tt.fields.Rights, Rights: tt.fields.Rights,
} }
if got := nu.CanCreate(tt.args.a); got != tt.want["CanCreate"] { if got, _ := nu.CanCreate(tt.args.a); got != tt.want["CanCreate"] {
t.Errorf("NamespaceUser.CanCreate() = %v, want %v", got, tt.want["CanCreate"]) t.Errorf("NamespaceUser.CanCreate() = %v, want %v", got, tt.want["CanCreate"])
} }
if got := nu.CanDelete(tt.args.a); got != tt.want["CanDelete"] { if got, _ := nu.CanDelete(tt.args.a); got != tt.want["CanDelete"] {
t.Errorf("NamespaceUser.CanDelete() = %v, want %v", got, tt.want["CanDelete"]) t.Errorf("NamespaceUser.CanDelete() = %v, want %v", got, tt.want["CanDelete"])
} }
if got := nu.CanUpdate(tt.args.a); got != tt.want["CanUpdate"] { if got, _ := nu.CanUpdate(tt.args.a); got != tt.want["CanUpdate"] {
t.Errorf("NamespaceUser.CanUpdate() = %v, want %v", got, tt.want["CanUpdate"]) t.Errorf("NamespaceUser.CanUpdate() = %v, want %v", got, tt.want["CanUpdate"])
} }
}) })

View File

@ -43,7 +43,11 @@ func (tl *TeamList) ReadAll(search string, a web.Auth, page int) (interface{}, e
if err := l.GetSimpleByID(); err != nil { if err := l.GetSimpleByID(); err != nil {
return nil, err return nil, err
} }
if !l.CanRead(u) { canRead, err := l.CanRead(u)
if err != nil {
return nil, err
}
if !canRead {
return nil, ErrNeedToHaveListReadAccess{ListID: tl.ListID, UserID: u.ID} return nil, ErrNeedToHaveListReadAccess{ListID: tl.ListID, UserID: u.ID}
} }

View File

@ -21,7 +21,7 @@ import (
) )
// CanCreate checks if the user can create a team <-> list relation // CanCreate checks if the user can create a team <-> list relation
func (tl *TeamList) CanCreate(a web.Auth) bool { func (tl *TeamList) CanCreate(a web.Auth) (bool, error) {
u := getUserForRights(a) u := getUserForRights(a)
l := List{ID: tl.ListID} l := List{ID: tl.ListID}
@ -29,7 +29,7 @@ func (tl *TeamList) CanCreate(a web.Auth) bool {
} }
// CanDelete checks if the user can delete a team <-> list relation // CanDelete checks if the user can delete a team <-> list relation
func (tl *TeamList) CanDelete(a web.Auth) bool { func (tl *TeamList) CanDelete(a web.Auth) (bool, error) {
user := getUserForRights(a) user := getUserForRights(a)
l := List{ID: tl.ListID} l := List{ID: tl.ListID}
@ -37,7 +37,7 @@ func (tl *TeamList) CanDelete(a web.Auth) bool {
} }
// CanUpdate checks if the user can update a team <-> list relation // CanUpdate checks if the user can update a team <-> list relation
func (tl *TeamList) CanUpdate(a web.Auth) bool { func (tl *TeamList) CanUpdate(a web.Auth) (bool, error) {
user := getUserForRights(a) user := getUserForRights(a)
l := List{ID: tl.ListID} l := List{ID: tl.ListID}

View File

@ -37,7 +37,8 @@ func TestTeamList(t *testing.T) {
assert.NoError(t, err) assert.NoError(t, err)
// Check normal creation // Check normal creation
assert.True(t, tl.CanCreate(&u)) allowed, _ := tl.CanCreate(&u)
assert.True(t, allowed)
err = tl.Create(&u) err = tl.Create(&u)
assert.NoError(t, err) assert.NoError(t, err)
@ -93,7 +94,8 @@ func TestTeamList(t *testing.T) {
assert.True(t, IsErrNeedToHaveListReadAccess(err)) assert.True(t, IsErrNeedToHaveListReadAccess(err))
// Delete // Delete
assert.True(t, tl.CanDelete(&u)) allowed, _ = tl.CanDelete(&u)
assert.True(t, allowed)
err = tl.Delete() err = tl.Delete()
assert.NoError(t, err) assert.NoError(t, err)

View File

@ -17,30 +17,25 @@
package models package models
import ( import (
"code.vikunja.io/api/pkg/log"
"code.vikunja.io/web" "code.vikunja.io/web"
) )
// CanCreate checks if the user can add a new tem member // CanCreate checks if the user can add a new tem member
func (tm *TeamMember) CanCreate(a web.Auth) bool { func (tm *TeamMember) CanCreate(a web.Auth) (bool, error) {
return tm.IsAdmin(a) return tm.IsAdmin(a)
} }
// CanDelete checks if the user can delete a new team member // CanDelete checks if the user can delete a new team member
func (tm *TeamMember) CanDelete(a web.Auth) bool { func (tm *TeamMember) CanDelete(a web.Auth) (bool, error) {
return tm.IsAdmin(a) return tm.IsAdmin(a)
} }
// IsAdmin checks if the user is team admin // IsAdmin checks if the user is team admin
func (tm *TeamMember) IsAdmin(a web.Auth) bool { func (tm *TeamMember) IsAdmin(a web.Auth) (bool, error) {
u := getUserForRights(a) u := getUserForRights(a)
// A user can add a member to a team if he is admin of that team // A user can add a member to a team if he is admin of that team
exists, err := x.Where("user_id = ? AND team_id = ? AND admin = ?", u.ID, tm.TeamID, true). exists, err := x.Where("user_id = ? AND team_id = ? AND admin = ?", u.ID, tm.TeamID, true).
Get(&TeamMember{}) Get(&TeamMember{})
if err != nil { return exists, err
log.Log.Error("Error occurred during IsAdmin for TeamMember: %s", err)
return false
}
return exists
} }

View File

@ -34,7 +34,8 @@ func TestTeamMember_Create(t *testing.T) {
assert.NoError(t, err) assert.NoError(t, err)
// Insert a new team member // Insert a new team member
assert.True(t, dummyteammember.CanCreate(&doer)) allowed, _ := dummyteammember.CanCreate(&doer)
assert.True(t, allowed)
err = dummyteammember.Create(&doer) err = dummyteammember.Create(&doer)
assert.NoError(t, err) assert.NoError(t, err)
@ -50,7 +51,8 @@ func TestTeamMember_Create(t *testing.T) {
assert.True(t, IsErrUserIsMemberOfTeam(err)) assert.True(t, IsErrUserIsMemberOfTeam(err))
// Delete it // Delete it
assert.True(t, dummyteammember.CanDelete(&doer)) allowed, _ = dummyteammember.CanDelete(&doer)
assert.True(t, allowed)
err = dummyteammember.Delete() err = dummyteammember.Delete()
assert.NoError(t, err) assert.NoError(t, err)

View File

@ -43,7 +43,11 @@ func (tn *TeamNamespace) ReadAll(search string, a web.Auth, page int) (interface
if err != nil { if err != nil {
return nil, err return nil, err
} }
if !n.CanRead(user) { canRead, err := n.CanRead(user)
if err != nil {
return nil, err
}
if !canRead {
return nil, ErrNeedToHaveNamespaceReadAccess{NamespaceID: tn.NamespaceID, UserID: user.ID} return nil, ErrNeedToHaveNamespaceReadAccess{NamespaceID: tn.NamespaceID, UserID: user.ID}
} }

View File

@ -21,19 +21,19 @@ import (
) )
// CanCreate checks if one can create a new team <-> namespace relation // CanCreate checks if one can create a new team <-> namespace relation
func (tn *TeamNamespace) CanCreate(a web.Auth) bool { func (tn *TeamNamespace) CanCreate(a web.Auth) (bool, error) {
n := &Namespace{ID: tn.NamespaceID} n := &Namespace{ID: tn.NamespaceID}
return n.IsAdmin(a) return n.IsAdmin(a)
} }
// CanDelete checks if a user can remove a team from a namespace. Only namespace admins can do that. // CanDelete checks if a user can remove a team from a namespace. Only namespace admins can do that.
func (tn *TeamNamespace) CanDelete(a web.Auth) bool { func (tn *TeamNamespace) CanDelete(a web.Auth) (bool, error) {
n := &Namespace{ID: tn.NamespaceID} n := &Namespace{ID: tn.NamespaceID}
return n.IsAdmin(a) return n.IsAdmin(a)
} }
// CanUpdate checks if a user can update a team from a Only namespace admins can do that. // CanUpdate checks if a user can update a team from a Only namespace admins can do that.
func (tn *TeamNamespace) CanUpdate(a web.Auth) bool { func (tn *TeamNamespace) CanUpdate(a web.Auth) (bool, error) {
n := &Namespace{ID: tn.NamespaceID} n := &Namespace{ID: tn.NamespaceID}
return n.IsAdmin(a) return n.IsAdmin(a)
} }

View File

@ -85,13 +85,13 @@ func TestTeamNamespace_CanDoSomething(t *testing.T) {
CRUDable: tt.fields.CRUDable, CRUDable: tt.fields.CRUDable,
Rights: tt.fields.Rights, Rights: tt.fields.Rights,
} }
if got := tn.CanCreate(tt.args.a); got != tt.want["CanCreate"] { if got, _ := tn.CanCreate(tt.args.a); got != tt.want["CanCreate"] {
t.Errorf("TeamNamespace.CanCreate() = %v, want %v", got, tt.want["CanCreate"]) t.Errorf("TeamNamespace.CanCreate() = %v, want %v", got, tt.want["CanCreate"])
} }
if got := tn.CanDelete(tt.args.a); got != tt.want["CanDelete"] { if got, _ := tn.CanDelete(tt.args.a); got != tt.want["CanDelete"] {
t.Errorf("TeamNamespace.CanDelete() = %v, want %v", got, tt.want["CanDelete"]) t.Errorf("TeamNamespace.CanDelete() = %v, want %v", got, tt.want["CanDelete"])
} }
if got := tn.CanUpdate(tt.args.a); got != tt.want["CanUpdate"] { if got, _ := tn.CanUpdate(tt.args.a); got != tt.want["CanUpdate"] {
t.Errorf("TeamNamespace.CanUpdate() = %v, want %v", got, tt.want["CanUpdate"]) t.Errorf("TeamNamespace.CanUpdate() = %v, want %v", got, tt.want["CanUpdate"])
} }
}) })

View File

@ -36,7 +36,8 @@ func TestTeamNamespace(t *testing.T) {
assert.NoError(t, err) assert.NoError(t, err)
// Test normal creation // Test normal creation
assert.True(t, tn.CanCreate(&dummyuser)) allowed, _ := tn.CanCreate(&dummyuser)
assert.True(t, allowed)
err = tn.Create(&dummyuser) err = tn.Create(&dummyuser)
assert.NoError(t, err) assert.NoError(t, err)
@ -85,7 +86,8 @@ func TestTeamNamespace(t *testing.T) {
assert.True(t, IsErrNeedToHaveNamespaceReadAccess(err)) assert.True(t, IsErrNeedToHaveNamespaceReadAccess(err))
// Delete it // Delete it
assert.True(t, tn.CanDelete(&dummyuser)) allowed, _ = tn.CanDelete(&dummyuser)
assert.True(t, allowed)
err = tn.Delete() err = tn.Delete()
assert.NoError(t, err) assert.NoError(t, err)

View File

@ -17,64 +17,47 @@
package models package models
import ( import (
"code.vikunja.io/api/pkg/log"
"code.vikunja.io/web" "code.vikunja.io/web"
) )
// CanCreate checks if the user can create a new team // CanCreate checks if the user can create a new team
func (t *Team) CanCreate(a web.Auth) bool { func (t *Team) CanCreate(a web.Auth) (bool, error) {
// This is currently a dummy function, later on we could imagine global limits etc. // This is currently a dummy function, later on we could imagine global limits etc.
return true return true, nil
} }
// CanUpdate checks if the user can update a team // CanUpdate checks if the user can update a team
func (t *Team) CanUpdate(a web.Auth) bool { func (t *Team) CanUpdate(a web.Auth) (bool, error) {
u := getUserForRights(a) u := getUserForRights(a)
// Check if the current user is in the team and has admin rights in it // Check if the current user is in the team and has admin rights in it
exists, err := x.Where("team_id = ?", t.ID). return x.Where("team_id = ?", t.ID).
And("user_id = ?", u.ID). And("user_id = ?", u.ID).
And("admin = ?", true). And("admin = ?", true).
Get(&TeamMember{}) Get(&TeamMember{})
if err != nil {
log.Log.Error("Error occurred during CanUpdate for Team: %s", err)
return false
}
return exists
} }
// CanDelete checks if a user can delete a team // CanDelete checks if a user can delete a team
func (t *Team) CanDelete(a web.Auth) bool { func (t *Team) CanDelete(a web.Auth) (bool, error) {
return t.IsAdmin(a) return t.IsAdmin(a)
} }
// IsAdmin returns true when the user is admin of a team // IsAdmin returns true when the user is admin of a team
func (t *Team) IsAdmin(a web.Auth) bool { func (t *Team) IsAdmin(a web.Auth) (bool, error) {
u := getUserForRights(a) u := getUserForRights(a)
exists, err := x.Where("team_id = ?", t.ID). return x.Where("team_id = ?", t.ID).
And("user_id = ?", u.ID). And("user_id = ?", u.ID).
And("admin = ?", true). And("admin = ?", true).
Get(&TeamMember{}) Get(&TeamMember{})
if err != nil {
log.Log.Error("Error occurred during CanUpdate for Team: %s", err)
return false
}
return exists
} }
// CanRead returns true if the user has read access to the team // CanRead returns true if the user has read access to the team
func (t *Team) CanRead(a web.Auth) bool { func (t *Team) CanRead(a web.Auth) (bool, error) {
user := getUserForRights(a) user := getUserForRights(a)
// Check if the user is in the team // Check if the user is in the team
exists, err := x.Where("team_id = ?", t.ID). return x.Where("team_id = ?", t.ID).
And("user_id = ?", user.ID). And("user_id = ?", user.ID).
Get(&TeamMember{}) Get(&TeamMember{})
if err != nil {
log.Log.Error("Error occurred during CanUpdate for Team: %s", err)
return false
}
return exists
} }

View File

@ -90,19 +90,19 @@ func TestTeam_CanDoSomething(t *testing.T) {
Rights: tt.fields.Rights, Rights: tt.fields.Rights,
} }
if got := tm.CanCreate(tt.args.a); got != tt.want["CanCreate"] { // CanCreate is currently always true if got, _ := tm.CanCreate(tt.args.a); got != tt.want["CanCreate"] { // CanCreate is currently always true
t.Errorf("Team.CanCreate() = %v, want %v", got, tt.want["CanCreate"]) t.Errorf("Team.CanCreate() = %v, want %v", got, tt.want["CanCreate"])
} }
if got := tm.CanDelete(tt.args.a); got != tt.want["CanDelete"] { if got, _ := tm.CanDelete(tt.args.a); got != tt.want["CanDelete"] {
t.Errorf("Team.CanDelete() = %v, want %v", got, tt.want["CanDelete"]) t.Errorf("Team.CanDelete() = %v, want %v", got, tt.want["CanDelete"])
} }
if got := tm.CanUpdate(tt.args.a); got != tt.want["CanUpdate"] { if got, _ := tm.CanUpdate(tt.args.a); got != tt.want["CanUpdate"] {
t.Errorf("Team.CanUpdate() = %v, want %v", 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"]) t.Errorf("Team.CanRead() = %v, want %v", got, tt.want["CanRead"])
} }
if got := tm.IsAdmin(tt.args.a); got != tt.want["IsAdmin"] { if got, _ := tm.IsAdmin(tt.args.a); got != tt.want["IsAdmin"] {
t.Errorf("Team.IsAdmin() = %v, want %v", got, tt.want["IsAdmin"]) t.Errorf("Team.IsAdmin() = %v, want %v", got, tt.want["IsAdmin"])
} }
}) })

View File

@ -34,7 +34,8 @@ func TestTeam_Create(t *testing.T) {
assert.NoError(t, err) assert.NoError(t, err)
// Insert it // Insert it
assert.True(t, dummyteam.CanCreate(&doer)) allowed, _ := dummyteam.CanCreate(&doer)
assert.True(t, allowed)
err = dummyteam.Create(&doer) err = dummyteam.Create(&doer)
assert.NoError(t, err) assert.NoError(t, err)
@ -45,7 +46,8 @@ func TestTeam_Create(t *testing.T) {
assert.Equal(t, 1, len(tm.Members)) assert.Equal(t, 1, len(tm.Members))
assert.Equal(t, doer.ID, tm.Members[0].User.ID) assert.Equal(t, doer.ID, tm.Members[0].User.ID)
assert.True(t, tm.Members[0].Admin) assert.True(t, tm.Members[0].Admin)
assert.True(t, dummyteam.CanRead(&doer)) allowed, _ = dummyteam.CanRead(&doer)
assert.True(t, allowed)
// Try getting a team with an ID < 0 // Try getting a team with an ID < 0
_, err = GetTeamByID(-1) _, err = GetTeamByID(-1)
@ -66,7 +68,8 @@ func TestTeam_Create(t *testing.T) {
assert.True(t, IsErrTeamNameCannotBeEmpty(err)) assert.True(t, IsErrTeamNameCannotBeEmpty(err))
// update it (still no name, should fail) // update it (still no name, should fail)
assert.True(t, dummyteam.CanUpdate(&doer)) allowed, _ = dummyteam.CanUpdate(&doer)
assert.True(t, allowed)
err = dummyteam.Update() err = dummyteam.Update()
assert.Error(t, err) assert.Error(t, err)
assert.True(t, IsErrTeamNameCannotBeEmpty(err)) assert.True(t, IsErrTeamNameCannotBeEmpty(err))
@ -77,7 +80,8 @@ func TestTeam_Create(t *testing.T) {
assert.NoError(t, err) assert.NoError(t, err)
// Delete it // Delete it
assert.True(t, dummyteam.CanDelete(&doer)) allowed, _ = dummyteam.CanDelete(&doer)
assert.True(t, allowed)
err = dummyteam.Delete() err = dummyteam.Delete()
assert.NoError(t, err) assert.NoError(t, err)

View File

@ -91,7 +91,11 @@ func getNamespace(c echo.Context) (namespace models.Namespace, err error) {
if err != nil { if err != nil {
return return
} }
if !namespace.CanRead(user) { canRead, err := namespace.CanRead(user)
if err != nil {
return namespace, err
}
if !canRead {
return return
} }

25
vendor/code.vikunja.io/web/Readme.md generated vendored
View File

@ -38,10 +38,11 @@ other handler implementations, enabling a lot of flexibility while develeoping.
### TODOs ### TODOs
* [ ] Improve docs/Merge with the ones of Vikunja * [x] Improve docs/Merge with the ones of Vikunja
* [ ] Description of web.HTTPError * [x] Description of web.HTTPError
* [ ] Rights methods should return errors (I know, this will break a lot of existing stuff) * [x] Rights methods should return errors (I know, this will break a lot of existing stuff)
* [ ] optional Before- and after-{load|update|create} methods which do some preprocessing/after processing like making human-readable names from automatically up counting consts * [ ] optional Before- and after-{load|update|create} methods which do some preprocessing/after processing like making human-readable names from automatically up counting consts
* [ ] "Magic": Check if a passed struct implements Crudable methods and use a general (user defined) function if not
## Installation ## Installation
@ -49,6 +50,8 @@ Using the web handler in your application is pretty straight forward, simply run
In order to use the common web handler, the struct must implement the `web.CRUDable` and `web.Rights` interface. In order to use the common web handler, the struct must implement the `web.CRUDable` and `web.Rights` interface.
To learn how to use the handler, take a look at the [handler config](#handler-config) [defining routes](#defining-routes-using-the-standard-web-handler)
## CRUDable ## CRUDable
This interface defines methods to Create/Read/ReadAll/Update/Delete something. It is defined as followed: This interface defines methods to Create/Read/ReadAll/Update/Delete something. It is defined as followed:
@ -82,18 +85,20 @@ way to do this, don't hesitate to [drop me a message](https://vikunja.io/en/cont
## Rights ## Rights
This interface defines methods to check for rights on structs. They accept an `Auth`-element as parameter and return a `bool`. This interface defines methods to check for rights on structs. They accept an `Auth`-element as parameter and return a `bool` and `error`.
The `error` is handled [as usual](#errors).
The interface is defined as followed: The interface is defined as followed:
```go ```go
type Rights interface { type Rights interface {
IsAdmin(Auth) bool IsAdmin(Auth) (bool, error)
CanWrite(Auth) bool CanWrite(Auth) (bool, error)
CanRead(Auth) bool CanRead(Auth) (bool, error)
CanDelete(Auth) bool CanDelete(Auth) (bool, error)
CanUpdate(Auth) bool CanUpdate(Auth) (bool, error)
CanCreate(Auth) bool CanCreate(Auth) (bool, error)
} }
``` ```

View File

@ -42,7 +42,11 @@ func (c *WebHandler) CreateWeb(ctx echo.Context) error {
} }
// Check rights // Check rights
if !currentStruct.CanCreate(currentAuth) { canRead, err := currentStruct.CanCreate(currentAuth)
if err != nil {
return HandleHTTPError(err, ctx)
}
if canRead {
config.LoggingProvider.Noticef("Tried to create while not having the rights for it (User: %v)", currentAuth) config.LoggingProvider.Noticef("Tried to create while not having the rights for it (User: %v)", currentAuth)
return echo.NewHTTPError(http.StatusForbidden) return echo.NewHTTPError(http.StatusForbidden)
} }

View File

@ -40,7 +40,11 @@ func (c *WebHandler) DeleteWeb(ctx echo.Context) error {
if err != nil { if err != nil {
return echo.NewHTTPError(http.StatusInternalServerError) return echo.NewHTTPError(http.StatusInternalServerError)
} }
if !currentStruct.CanDelete(currentAuth) { canDelete, err := currentStruct.CanDelete(currentAuth)
if err != nil {
return HandleHTTPError(err, ctx)
}
if canDelete {
config.LoggingProvider.Noticef("Tried to create while not having the rights for it (User: %v)", currentAuth) config.LoggingProvider.Noticef("Tried to create while not having the rights for it (User: %v)", currentAuth)
return echo.NewHTTPError(http.StatusForbidden) return echo.NewHTTPError(http.StatusForbidden)
} }

View File

@ -42,7 +42,11 @@ func (c *WebHandler) ReadOneWeb(ctx echo.Context) error {
if err != nil { if err != nil {
return echo.NewHTTPError(http.StatusInternalServerError, "Could not determine the current user.") return echo.NewHTTPError(http.StatusInternalServerError, "Could not determine the current user.")
} }
if !currentStruct.CanRead(currentAuth) { canRead, err := currentStruct.CanRead(currentAuth)
if err != nil {
return HandleHTTPError(err, ctx)
}
if canRead {
config.LoggingProvider.Noticef("Tried to create while not having the rights for it (User: %v)", currentAuth) config.LoggingProvider.Noticef("Tried to create while not having the rights for it (User: %v)", currentAuth)
return echo.NewHTTPError(http.StatusForbidden, "You don't have the right to see this") return echo.NewHTTPError(http.StatusForbidden, "You don't have the right to see this")
} }

View File

@ -41,7 +41,11 @@ func (c *WebHandler) UpdateWeb(ctx echo.Context) error {
if err != nil { if err != nil {
return echo.NewHTTPError(http.StatusInternalServerError, "Could not determine the current user.") return echo.NewHTTPError(http.StatusInternalServerError, "Could not determine the current user.")
} }
if !currentStruct.CanUpdate(currentAuth) { canUpdate, err := currentStruct.CanUpdate(currentAuth)
if err != nil {
return HandleHTTPError(err, ctx)
}
if canUpdate {
config.LoggingProvider.Noticef("Tried to create while not having the rights for it (User: %v)", currentAuth) config.LoggingProvider.Noticef("Tried to create while not having the rights for it (User: %v)", currentAuth)
return echo.NewHTTPError(http.StatusForbidden) return echo.NewHTTPError(http.StatusForbidden)
} }

12
vendor/code.vikunja.io/web/web.go generated vendored
View File

@ -19,12 +19,12 @@ import "github.com/labstack/echo"
// Rights defines rights methods // Rights defines rights methods
type Rights interface { type Rights interface {
IsAdmin(Auth) bool IsAdmin(Auth) (bool, error)
CanWrite(Auth) bool CanWrite(Auth) (bool, error)
CanRead(Auth) bool CanRead(Auth) (bool, error)
CanDelete(Auth) bool CanDelete(Auth) (bool, error)
CanUpdate(Auth) bool CanUpdate(Auth) (bool, error)
CanCreate(Auth) bool CanCreate(Auth) (bool, error)
} }
// CRUDable defines the crud methods // CRUDable defines the crud methods

2
vendor/modules.txt vendored
View File

@ -1,4 +1,4 @@
# code.vikunja.io/web v0.0.0-20190324080741-7bd881d9892a # code.vikunja.io/web v0.0.0-20190324105229-0933ac082307
code.vikunja.io/web code.vikunja.io/web
code.vikunja.io/web/handler code.vikunja.io/web/handler
# github.com/BurntSushi/toml v0.3.1 # github.com/BurntSushi/toml v0.3.1