diff --git a/Makefile b/Makefile index 22717665d..da566ef98 100644 --- a/Makefile +++ b/Makefile @@ -202,7 +202,7 @@ gocyclo-check: go get -u github.com/fzipp/gocyclo; \ go install $(GOFLAGS) github.com/fzipp/gocyclo; \ 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 static-check: diff --git a/go.mod b/go.mod index 4ba3e147c..b8a66db5e 100644 --- a/go.mod +++ b/go.mod @@ -18,7 +18,7 @@ module code.vikunja.io/api require ( 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/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc github.com/asaskevich/govalidator v0.0.0-20180720115003-f9ffefc3facf diff --git a/go.sum b/go.sum index 5e167727a..a7a5b05e4 100644 --- a/go.sum +++ b/go.sum @@ -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-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-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/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= github.com/PuerkitoBio/purell v1.1.0 h1:rmGxhojJlM0tuKtfdvliR84CFHljx9ag64t2xmVkjK4= diff --git a/pkg/models/bulk_list_task.go b/pkg/models/bulk_list_task.go index fc93b7200..6b4190d97 100644 --- a/pkg/models/bulk_list_task.go +++ b/pkg/models/bulk_list_task.go @@ -17,7 +17,6 @@ package models import ( - "code.vikunja.io/api/pkg/log" "code.vikunja.io/web" "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 -func (bt *BulkTask) CanUpdate(a web.Auth) bool { +func (bt *BulkTask) CanUpdate(a web.Auth) (bool, error) { err := bt.checkIfTasksAreOnTheSameList() if err != nil { - log.Log.Error("Error occurred during CanUpdate for BulkTask: %s", err) - return false + return false, err } // A user can update an task if he has write acces to its list diff --git a/pkg/models/bulk_list_task_test.go b/pkg/models/bulk_list_task_test.go index 751b474c1..e6c6c7f64 100644 --- a/pkg/models/bulk_list_task_test.go +++ b/pkg/models/bulk_list_task_test.go @@ -57,7 +57,7 @@ func TestBulkTask_Update(t *testing.T) { Tasks: tt.fields.Tasks, ListTask: tt.fields.ListTask, } - allowed := bt.CanUpdate(tt.fields.User) + allowed, _ := bt.CanUpdate(tt.fields.User) if !allowed != tt.wantForbidden { t.Errorf("BulkTask.Update() want forbidden, got %v, want %v", allowed, tt.wantForbidden) } diff --git a/pkg/models/label_rights.go b/pkg/models/label_rights.go index 9df2877f9..70fdcf4c4 100644 --- a/pkg/models/label_rights.go +++ b/pkg/models/label_rights.go @@ -17,51 +17,48 @@ package models import ( - "code.vikunja.io/api/pkg/log" "code.vikunja.io/web" "github.com/go-xorm/builder" ) // 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 } // 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 } // 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) } // CanCreate checks if the user can create a label // Currently a dummy. -func (l *Label) CanCreate(a web.Auth) bool { - return true +func (l *Label) CanCreate(a web.Auth) (bool, error) { + return true, nil } -func (l *Label) isLabelOwner(a web.Auth) bool { +func (l *Label) isLabelOwner(a web.Auth) (bool, error) { u := getUserForRights(a) lorig, err := getLabelByIDSimple(l.ID) if err != nil { - log.Log.Errorf("Error occurred during isLabelOwner for Label: %v", err) - return false + return false, err } - return lorig.CreatedByID == u.ID + return lorig.CreatedByID == u.ID, nil } // 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) // Get all tasks taskIDs, err := getUserTaskIDs(u) if err != nil { - log.Log.Errorf("Error occurred during hasAccessToLabel for Label: %v", err) - return false + return false, err } // Get all labels associated with these tasks @@ -74,10 +71,5 @@ func (l *Label) hasAccessToLabel(a web.Auth) bool { And("labels.id = ?", l.ID). GroupBy("labels.id"). Exist(&labels) - if err != nil { - log.Log.Errorf("Error occurred during hasAccessToLabel for Label: %v", err) - return false - } - - return has + return has, err } diff --git a/pkg/models/label_task.go b/pkg/models/label_task.go index cda8c9f68..bcda1c1ea 100644 --- a/pkg/models/label_task.go +++ b/pkg/models/label_task.go @@ -114,7 +114,11 @@ func (lt *LabelTask) ReadAll(search string, a web.Auth, page int) (labels interf 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} } @@ -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 - if !label.hasAccessToLabel(creator) { + hasAccessToLabel, err := label.hasAccessToLabel(creator) + if err != nil { + return err + } + if !hasAccessToLabel { user, _ := creator.(*User) return ErrUserHasNoAccessToLabel{LabelID: l.ID, UserID: user.ID} } diff --git a/pkg/models/label_task_rights.go b/pkg/models/label_task_rights.go index 034253832..237034307 100644 --- a/pkg/models/label_task_rights.go +++ b/pkg/models/label_task_rights.go @@ -17,51 +17,61 @@ package models import ( - "code.vikunja.io/api/pkg/log" "code.vikunja.io/web" ) // 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) if err != nil { - log.Log.Errorf("Error during CanCreate for LabelTask: %v", err) - return false + return false, err } - 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 -func (lt *LabelTask) CanDelete(a web.Auth) bool { - if !canDoLabelTask(lt.TaskID, a) { - return false +func (lt *LabelTask) CanDelete(a web.Auth) (bool, error) { + canDoLabelTask, err := canDoLabelTask(lt.TaskID, a) + 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, // throw an error. exists, err := x.Exist(&LabelTask{LabelID: lt.LabelID, TaskID: lt.TaskID}) if err != nil { - log.Log.Errorf("Error during CanDelete for LabelTask: %v", err) - return false + return false, err } - return exists + return exists, err } // 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) } // Helper function to check if a user can write to a task // + is able to see the label // 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 task, err := getTaskByIDSimple(taskID) if err != nil { - log.Log.Error("Error occurred during canDoLabelTask for LabelTask: %v", err) - return false + return false, err } return task.CanUpdate(a) } diff --git a/pkg/models/label_task_test.go b/pkg/models/label_task_test.go index 4956bbf7b..60b53b283 100644 --- a/pkg/models/label_task_test.go +++ b/pkg/models/label_task_test.go @@ -178,7 +178,8 @@ func TestLabelTask_Create(t *testing.T) { CRUDable: tt.fields.CRUDable, 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) } err := l.Create(tt.args.a) @@ -264,7 +265,8 @@ func TestLabelTask_Delete(t *testing.T) { CRUDable: tt.fields.CRUDable, 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) } err := l.Delete() diff --git a/pkg/models/label_test.go b/pkg/models/label_test.go index 39ea96e5c..07c78f7b0 100644 --- a/pkg/models/label_test.go +++ b/pkg/models/label_test.go @@ -217,7 +217,8 @@ func TestLabel_ReadOne(t *testing.T) { 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) } err := l.ReadOne() @@ -283,7 +284,8 @@ func TestLabel_Create(t *testing.T) { CRUDable: tt.fields.CRUDable, 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) } 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, 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) } if err := l.Update(); (err != nil) != tt.wantErr { @@ -441,7 +444,8 @@ func TestLabel_Delete(t *testing.T) { CRUDable: tt.fields.CRUDable, 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) } if err := l.Delete(); (err != nil) != tt.wantErr { diff --git a/pkg/models/list_create_test.go b/pkg/models/list_create_test.go index 57ef395cf..09afac518 100644 --- a/pkg/models/list_create_test.go +++ b/pkg/models/list_create_test.go @@ -37,7 +37,8 @@ func TestList_Create(t *testing.T) { } // Check if the user can create - assert.True(t, dummylist.CanCreate(&doer)) + allowed, _ := dummylist.CanCreate(&doer) + assert.True(t, allowed) // Create it err = dummylist.Create(&doer) @@ -52,16 +53,19 @@ func TestList_Create(t *testing.T) { assert.Equal(t, dummylist.OwnerID, doer.ID) // 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 - assert.True(t, dummylist.CanUpdate(&doer)) + allowed, _ = dummylist.CanUpdate(&doer) + assert.True(t, allowed) dummylist.Description = "Lorem Ipsum dolor sit amet." err = dummylist.Update() assert.NoError(t, err) // Delete it - assert.True(t, dummylist.CanDelete(&doer)) + allowed, _ = dummylist.CanDelete(&doer) + assert.True(t, allowed) err = dummylist.Delete() assert.NoError(t, err) diff --git a/pkg/models/list_rights.go b/pkg/models/list_rights.go index b6f24439e..0a81051d5 100644 --- a/pkg/models/list_rights.go +++ b/pkg/models/list_rights.go @@ -17,71 +17,76 @@ package models import ( - "code.vikunja.io/api/pkg/log" "code.vikunja.io/web" "github.com/go-xorm/builder" ) // 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 originalList := &List{ID: l.ID} err := originalList.GetSimpleByID() if err != nil { - log.Log.Error("Error occurred during CanWrite for List: %s", err) - return false + return false, err } user := getUserForRights(a) - // Check all the things // 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 -func (l *List) CanRead(a web.Auth) bool { +func (l *List) CanRead(a web.Auth) (bool, error) { user := getUserForRights(a) - // Check all the things // 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() // 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 -func (l *List) CanUpdate(a web.Auth) bool { +func (l *List) CanUpdate(a web.Auth) (bool, error) { return l.CanWrite(a) } // 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) } // 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 n := &Namespace{ID: l.NamespaceID} return n.CanWrite(a) } // 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) originalList := &List{ID: l.ID} err := originalList.GetSimpleByID() if err != nil { - log.Log.Error("Error occurred during IsAdmin for List: %s", err) - return false + return false, err } // Check all the things // Check if the user is either owner or can write to the list // 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 @@ -90,7 +95,7 @@ func (l *List) isOwner(u *User) bool { } // 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: @@ -149,10 +154,5 @@ func (l *List) checkRight(user *User, rights ...Right) bool { builder.Eq{"l.id": l.ID}, )). Exist(&List{}) - if err != nil { - log.Log.Error("Error occurred during checkRight for list: %s", err) - return false - } - - return exists + return exists, err } diff --git a/pkg/models/list_task_assignees.go b/pkg/models/list_task_assignees.go index 835f597f8..93398cfcb 100644 --- a/pkg/models/list_task_assignees.go +++ b/pkg/models/list_task_assignees.go @@ -175,7 +175,11 @@ func (t *ListTask) addNewAssigneeByID(newAssigneeID int64, list *List) (err erro if err != nil { return err } - if !list.CanRead(&newAssignee) { + canRead, err := list.CanRead(&newAssignee) + if err != nil { + return err + } + if !canRead { return ErrUserDoesNotHaveAccessToList{list.ID, newAssigneeID} } diff --git a/pkg/models/list_task_assignees_rights.go b/pkg/models/list_task_assignees_rights.go index 8c2496c16..d25371e67 100644 --- a/pkg/models/list_task_assignees_rights.go +++ b/pkg/models/list_task_assignees_rights.go @@ -17,31 +17,29 @@ package models import ( - "code.vikunja.io/api/pkg/log" "code.vikunja.io/web" ) // 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) } // 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) } // 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) } -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 list, err := GetListSimplByTaskID(taskID) if err != nil { - log.Log.Errorf("Error during canDoListTaskAssingee for ListTaskAssginee: %v", err) - return false + return false, err } return list.CanCreate(a) } diff --git a/pkg/models/list_tasks_rights.go b/pkg/models/list_tasks_rights.go index 94f3b494e..dce77d7ad 100644 --- a/pkg/models/list_tasks_rights.go +++ b/pkg/models/list_tasks_rights.go @@ -17,22 +17,21 @@ package models import ( - "code.vikunja.io/api/pkg/log" "code.vikunja.io/web" ) // 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) } // 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) } // 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) // 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 -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 list := &List{ID: t.ListID} err := list.GetSimpleByID() if err != nil { - log.Log.Error("Error occurred during CanRead for ListTask: %s", err) - return false + return false, err } return list.CanRead(a) } // 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) // Get the task lI, err := getTaskByIDSimple(t.ID) if err != nil { - log.Log.Error("Error occurred during canDoListTask (getTaskByIDSimple) for ListTask: %s", err) - return false + return false, err } // A user can do a task if he has write acces to its list diff --git a/pkg/models/list_tasks_test.go b/pkg/models/list_tasks_test.go index ee522919a..b13ea09b9 100644 --- a/pkg/models/list_tasks_test.go +++ b/pkg/models/list_tasks_test.go @@ -35,14 +35,16 @@ func TestListTask_Create(t *testing.T) { doer, err := GetUserByID(1) assert.NoError(t, err) - assert.True(t, listtask.CanCreate(&doer)) + allowed, _ := listtask.CanCreate(&doer) + assert.True(t, allowed) err = listtask.Create(&doer) assert.NoError(t, err) // Update it listtask.Text = "Test34" - assert.True(t, listtask.CanUpdate(&doer)) + allowed, _ = listtask.CanUpdate(&doer) + assert.True(t, allowed) err = listtask.Update() assert.NoError(t, err) @@ -52,7 +54,8 @@ func TestListTask_Create(t *testing.T) { assert.Equal(t, li.Text, "Test34") // Delete the task - assert.True(t, listtask.CanDelete(&doer)) + allowed, _ = listtask.CanDelete(&doer) + assert.True(t, allowed) err = listtask.Delete() assert.NoError(t, err) diff --git a/pkg/models/list_users_readall.go b/pkg/models/list_users_readall.go index 665c4978e..e60ad3572 100644 --- a/pkg/models/list_users_readall.go +++ b/pkg/models/list_users_readall.go @@ -43,7 +43,11 @@ func (lu *ListUser) ReadAll(search string, a web.Auth, page int) (interface{}, e if err := l.GetSimpleByID(); err != nil { 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} } diff --git a/pkg/models/list_users_rights.go b/pkg/models/list_users_rights.go index 925266baa..6876ef1bc 100644 --- a/pkg/models/list_users_rights.go +++ b/pkg/models/list_users_rights.go @@ -21,21 +21,21 @@ import ( ) // 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 l := List{ID: lu.ListID} return l.CanWrite(a) } // 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 l := List{ID: lu.ListID} return l.CanWrite(a) } // 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 l := List{ID: lu.ListID} return l.CanWrite(a) diff --git a/pkg/models/list_users_rights_test.go b/pkg/models/list_users_rights_test.go index 1e1ac7365..ccd2a33f9 100644 --- a/pkg/models/list_users_rights_test.go +++ b/pkg/models/list_users_rights_test.go @@ -85,13 +85,13 @@ func TestListUser_CanDoSomething(t *testing.T) { CRUDable: tt.fields.CRUDable, 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"]) } - 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"]) } - 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"]) } }) diff --git a/pkg/models/namespace_rights.go b/pkg/models/namespace_rights.go index a386a5f8a..5eb1780b3 100644 --- a/pkg/models/namespace_rights.go +++ b/pkg/models/namespace_rights.go @@ -17,59 +17,71 @@ package models import ( - "code.vikunja.io/api/pkg/log" "code.vikunja.io/web" "github.com/go-xorm/builder" ) // 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 originalNamespace := &Namespace{ID: n.ID} err := originalNamespace.GetSimpleByID() if err != nil { - log.Log.Error("Error occurred during CanWrite for Namespace: %s", err) - return false + return false, err } 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 -func (n *Namespace) IsAdmin(a web.Auth) bool { +func (n *Namespace) IsAdmin(a web.Auth) (bool, error) { originalNamespace := &Namespace{ID: n.ID} err := originalNamespace.GetSimpleByID() if err != nil { - log.Log.Error("Error occurred during IsAdmin for Namespace: %s", err) - return false + return false, err } 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 -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) - 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 -func (n *Namespace) CanUpdate(a web.Auth) bool { +func (n *Namespace) CanUpdate(a web.Auth) (bool, error) { return n.IsAdmin(a) } // 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) } // 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. - return true + return true, nil } // 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 } -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: @@ -124,10 +136,5 @@ func (n *Namespace) checkRight(user *User, rights ...Right) bool { builder.Eq{"namespaces.id": n.ID}, )). Exist(&List{}) - if err != nil { - log.Log.Error("Error occurred during checkRight for namespace: %s", err) - return false - } - - return exists + return exists, err } diff --git a/pkg/models/namespace_test.go b/pkg/models/namespace_test.go index f224df797..590d3f447 100644 --- a/pkg/models/namespace_test.go +++ b/pkg/models/namespace_test.go @@ -37,12 +37,14 @@ func TestNamespace_Create(t *testing.T) { assert.NoError(t, err) // Try creating it - assert.True(t, dummynamespace.CanCreate(&doer)) + allowed, _ := dummynamespace.CanCreate(&doer) + assert.True(t, allowed) err = dummynamespace.Create(&doer) assert.NoError(t, err) // check if it really exists - assert.True(t, dummynamespace.CanRead(&doer)) + allowed, _ = dummynamespace.CanRead(&doer) + assert.True(t, allowed) newOne := Namespace{ID: dummynamespace.ID} err = newOne.ReadOne() assert.NoError(t, err) @@ -62,7 +64,8 @@ func TestNamespace_Create(t *testing.T) { assert.True(t, IsErrUserDoesNotExist(err)) // Update it - assert.True(t, dummynamespace.CanUpdate(&doer)) + allowed, _ = dummynamespace.CanUpdate(&doer) + assert.True(t, allowed) dummynamespace.Description = "Dolor sit amet." err = dummynamespace.Update() assert.NoError(t, err) @@ -86,7 +89,8 @@ func TestNamespace_Create(t *testing.T) { assert.True(t, IsErrNamespaceDoesNotExist(err)) // Delete it - assert.True(t, dummynamespace.CanDelete(&doer)) + allowed, _ = dummynamespace.CanDelete(&doer) + assert.True(t, allowed) err = dummynamespace.Delete() assert.NoError(t, err) diff --git a/pkg/models/namespace_users_readall.go b/pkg/models/namespace_users_readall.go index 750bed40a..a7faa8682 100644 --- a/pkg/models/namespace_users_readall.go +++ b/pkg/models/namespace_users_readall.go @@ -43,7 +43,11 @@ func (nu *NamespaceUser) ReadAll(search string, a web.Auth, page int) (interface if err != nil { return nil, err } - if !l.CanRead(u) { + canRead, err := l.CanRead(u) + if err != nil { + return nil, err + } + if !canRead { return nil, ErrNeedToHaveNamespaceReadAccess{} } diff --git a/pkg/models/namespace_users_rights.go b/pkg/models/namespace_users_rights.go index bba1c5ded..2f94f0639 100644 --- a/pkg/models/namespace_users_rights.go +++ b/pkg/models/namespace_users_rights.go @@ -21,19 +21,19 @@ import ( ) // 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} return n.CanWrite(a) } // 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} return n.CanWrite(a) } // 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} return n.CanWrite(a) } diff --git a/pkg/models/namespace_users_rights_test.go b/pkg/models/namespace_users_rights_test.go index a30be33eb..f8a1f8682 100644 --- a/pkg/models/namespace_users_rights_test.go +++ b/pkg/models/namespace_users_rights_test.go @@ -85,13 +85,13 @@ func TestNamespaceUser_CanDoSomething(t *testing.T) { CRUDable: tt.fields.CRUDable, 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"]) } - 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"]) } - 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"]) } }) diff --git a/pkg/models/team_list_readall.go b/pkg/models/team_list_readall.go index ce26fc135..cc9262699 100644 --- a/pkg/models/team_list_readall.go +++ b/pkg/models/team_list_readall.go @@ -43,7 +43,11 @@ func (tl *TeamList) ReadAll(search string, a web.Auth, page int) (interface{}, e if err := l.GetSimpleByID(); err != nil { 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} } diff --git a/pkg/models/team_list_rights.go b/pkg/models/team_list_rights.go index efaa9b907..dabe6cc2c 100644 --- a/pkg/models/team_list_rights.go +++ b/pkg/models/team_list_rights.go @@ -21,7 +21,7 @@ import ( ) // 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) 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 -func (tl *TeamList) CanDelete(a web.Auth) bool { +func (tl *TeamList) CanDelete(a web.Auth) (bool, error) { user := getUserForRights(a) 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 -func (tl *TeamList) CanUpdate(a web.Auth) bool { +func (tl *TeamList) CanUpdate(a web.Auth) (bool, error) { user := getUserForRights(a) l := List{ID: tl.ListID} diff --git a/pkg/models/team_list_test.go b/pkg/models/team_list_test.go index 193fe4d23..dec28529d 100644 --- a/pkg/models/team_list_test.go +++ b/pkg/models/team_list_test.go @@ -37,7 +37,8 @@ func TestTeamList(t *testing.T) { assert.NoError(t, err) // Check normal creation - assert.True(t, tl.CanCreate(&u)) + allowed, _ := tl.CanCreate(&u) + assert.True(t, allowed) err = tl.Create(&u) assert.NoError(t, err) @@ -93,7 +94,8 @@ func TestTeamList(t *testing.T) { assert.True(t, IsErrNeedToHaveListReadAccess(err)) // Delete - assert.True(t, tl.CanDelete(&u)) + allowed, _ = tl.CanDelete(&u) + assert.True(t, allowed) err = tl.Delete() assert.NoError(t, err) diff --git a/pkg/models/team_members_rights.go b/pkg/models/team_members_rights.go index 075a52b4d..9ac857dd5 100644 --- a/pkg/models/team_members_rights.go +++ b/pkg/models/team_members_rights.go @@ -17,30 +17,25 @@ package models import ( - "code.vikunja.io/api/pkg/log" "code.vikunja.io/web" ) // 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) } // 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) } // 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) // 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). Get(&TeamMember{}) - if err != nil { - log.Log.Error("Error occurred during IsAdmin for TeamMember: %s", err) - return false - } - return exists + return exists, err } diff --git a/pkg/models/team_members_test.go b/pkg/models/team_members_test.go index 2a46274d3..90ce1c346 100644 --- a/pkg/models/team_members_test.go +++ b/pkg/models/team_members_test.go @@ -34,7 +34,8 @@ func TestTeamMember_Create(t *testing.T) { assert.NoError(t, err) // Insert a new team member - assert.True(t, dummyteammember.CanCreate(&doer)) + allowed, _ := dummyteammember.CanCreate(&doer) + assert.True(t, allowed) err = dummyteammember.Create(&doer) assert.NoError(t, err) @@ -50,7 +51,8 @@ func TestTeamMember_Create(t *testing.T) { assert.True(t, IsErrUserIsMemberOfTeam(err)) // Delete it - assert.True(t, dummyteammember.CanDelete(&doer)) + allowed, _ = dummyteammember.CanDelete(&doer) + assert.True(t, allowed) err = dummyteammember.Delete() assert.NoError(t, err) diff --git a/pkg/models/team_namespace_readall.go b/pkg/models/team_namespace_readall.go index b4fac9c2a..16a4d7d36 100644 --- a/pkg/models/team_namespace_readall.go +++ b/pkg/models/team_namespace_readall.go @@ -43,7 +43,11 @@ func (tn *TeamNamespace) ReadAll(search string, a web.Auth, page int) (interface if err != nil { 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} } diff --git a/pkg/models/team_namespace_rights.go b/pkg/models/team_namespace_rights.go index 65e1e2f5b..dc89672a6 100644 --- a/pkg/models/team_namespace_rights.go +++ b/pkg/models/team_namespace_rights.go @@ -21,19 +21,19 @@ import ( ) // 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} return n.IsAdmin(a) } // 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} return n.IsAdmin(a) } // 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} return n.IsAdmin(a) } diff --git a/pkg/models/team_namespace_rights_test.go b/pkg/models/team_namespace_rights_test.go index f919dd230..67399db77 100644 --- a/pkg/models/team_namespace_rights_test.go +++ b/pkg/models/team_namespace_rights_test.go @@ -85,13 +85,13 @@ func TestTeamNamespace_CanDoSomething(t *testing.T) { CRUDable: tt.fields.CRUDable, 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"]) } - 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"]) } - 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"]) } }) diff --git a/pkg/models/team_namespace_test.go b/pkg/models/team_namespace_test.go index 4ed427501..512ee639b 100644 --- a/pkg/models/team_namespace_test.go +++ b/pkg/models/team_namespace_test.go @@ -36,7 +36,8 @@ func TestTeamNamespace(t *testing.T) { assert.NoError(t, err) // Test normal creation - assert.True(t, tn.CanCreate(&dummyuser)) + allowed, _ := tn.CanCreate(&dummyuser) + assert.True(t, allowed) err = tn.Create(&dummyuser) assert.NoError(t, err) @@ -85,7 +86,8 @@ func TestTeamNamespace(t *testing.T) { assert.True(t, IsErrNeedToHaveNamespaceReadAccess(err)) // Delete it - assert.True(t, tn.CanDelete(&dummyuser)) + allowed, _ = tn.CanDelete(&dummyuser) + assert.True(t, allowed) err = tn.Delete() assert.NoError(t, err) diff --git a/pkg/models/teams_rights.go b/pkg/models/teams_rights.go index 06b778d2f..0d034da5c 100644 --- a/pkg/models/teams_rights.go +++ b/pkg/models/teams_rights.go @@ -17,64 +17,47 @@ package models import ( - "code.vikunja.io/api/pkg/log" "code.vikunja.io/web" ) // 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. - return true + return true, nil } // 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) // 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("admin = ?", true). 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 -func (t *Team) CanDelete(a web.Auth) bool { +func (t *Team) CanDelete(a web.Auth) (bool, error) { return t.IsAdmin(a) } // 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) - exists, err := x.Where("team_id = ?", t.ID). + return x.Where("team_id = ?", t.ID). And("user_id = ?", u.ID). And("admin = ?", true). 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 -func (t *Team) CanRead(a web.Auth) bool { +func (t *Team) CanRead(a web.Auth) (bool, error) { user := getUserForRights(a) // 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). Get(&TeamMember{}) - if err != nil { - log.Log.Error("Error occurred during CanUpdate for Team: %s", err) - return false - } - return exists } diff --git a/pkg/models/teams_rights_test.go b/pkg/models/teams_rights_test.go index 2b25d5aeb..940a79909 100644 --- a/pkg/models/teams_rights_test.go +++ b/pkg/models/teams_rights_test.go @@ -90,19 +90,19 @@ func TestTeam_CanDoSomething(t *testing.T) { 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"]) } - 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"]) } - 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"]) } - 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"] { + if got, _ := tm.IsAdmin(tt.args.a); got != tt.want["IsAdmin"] { t.Errorf("Team.IsAdmin() = %v, want %v", got, tt.want["IsAdmin"]) } }) diff --git a/pkg/models/teams_test.go b/pkg/models/teams_test.go index d32d7ec5d..6800df085 100644 --- a/pkg/models/teams_test.go +++ b/pkg/models/teams_test.go @@ -34,7 +34,8 @@ func TestTeam_Create(t *testing.T) { assert.NoError(t, err) // Insert it - assert.True(t, dummyteam.CanCreate(&doer)) + allowed, _ := dummyteam.CanCreate(&doer) + assert.True(t, allowed) err = dummyteam.Create(&doer) assert.NoError(t, err) @@ -45,7 +46,8 @@ func TestTeam_Create(t *testing.T) { assert.Equal(t, 1, len(tm.Members)) assert.Equal(t, doer.ID, tm.Members[0].User.ID) 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 _, err = GetTeamByID(-1) @@ -66,7 +68,8 @@ func TestTeam_Create(t *testing.T) { assert.True(t, IsErrTeamNameCannotBeEmpty(err)) // update it (still no name, should fail) - assert.True(t, dummyteam.CanUpdate(&doer)) + allowed, _ = dummyteam.CanUpdate(&doer) + assert.True(t, allowed) err = dummyteam.Update() assert.Error(t, err) assert.True(t, IsErrTeamNameCannotBeEmpty(err)) @@ -77,7 +80,8 @@ func TestTeam_Create(t *testing.T) { assert.NoError(t, err) // Delete it - assert.True(t, dummyteam.CanDelete(&doer)) + allowed, _ = dummyteam.CanDelete(&doer) + assert.True(t, allowed) err = dummyteam.Delete() assert.NoError(t, err) diff --git a/pkg/routes/api/v1/list_by_namespace.go b/pkg/routes/api/v1/list_by_namespace.go index 55f460e23..b0af5a083 100644 --- a/pkg/routes/api/v1/list_by_namespace.go +++ b/pkg/routes/api/v1/list_by_namespace.go @@ -91,7 +91,11 @@ func getNamespace(c echo.Context) (namespace models.Namespace, err error) { if err != nil { return } - if !namespace.CanRead(user) { + canRead, err := namespace.CanRead(user) + if err != nil { + return namespace, err + } + if !canRead { return } diff --git a/vendor/code.vikunja.io/web/Readme.md b/vendor/code.vikunja.io/web/Readme.md index 57b0a950e..01a4a3533 100644 --- a/vendor/code.vikunja.io/web/Readme.md +++ b/vendor/code.vikunja.io/web/Readme.md @@ -38,10 +38,11 @@ other handler implementations, enabling a lot of flexibility while develeoping. ### TODOs -* [ ] Improve docs/Merge with the ones of Vikunja -* [ ] Description of web.HTTPError -* [ ] Rights methods should return errors (I know, this will break a lot of existing stuff) +* [x] Improve docs/Merge with the ones of Vikunja +* [x] Description of web.HTTPError +* [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 +* [ ] "Magic": Check if a passed struct implements Crudable methods and use a general (user defined) function if not ## 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. +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 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 -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: ```go type Rights interface { - IsAdmin(Auth) bool - CanWrite(Auth) bool - CanRead(Auth) bool - CanDelete(Auth) bool - CanUpdate(Auth) bool - CanCreate(Auth) bool + IsAdmin(Auth) (bool, error) + CanWrite(Auth) (bool, error) + CanRead(Auth) (bool, error) + CanDelete(Auth) (bool, error) + CanUpdate(Auth) (bool, error) + CanCreate(Auth) (bool, error) } ``` diff --git a/vendor/code.vikunja.io/web/handler/create.go b/vendor/code.vikunja.io/web/handler/create.go index 0c78060a2..603338d2b 100644 --- a/vendor/code.vikunja.io/web/handler/create.go +++ b/vendor/code.vikunja.io/web/handler/create.go @@ -42,7 +42,11 @@ func (c *WebHandler) CreateWeb(ctx echo.Context) error { } // 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) return echo.NewHTTPError(http.StatusForbidden) } diff --git a/vendor/code.vikunja.io/web/handler/delete.go b/vendor/code.vikunja.io/web/handler/delete.go index fef4aa9a9..c671b1af2 100644 --- a/vendor/code.vikunja.io/web/handler/delete.go +++ b/vendor/code.vikunja.io/web/handler/delete.go @@ -40,7 +40,11 @@ func (c *WebHandler) DeleteWeb(ctx echo.Context) error { if err != nil { 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) return echo.NewHTTPError(http.StatusForbidden) } diff --git a/vendor/code.vikunja.io/web/handler/read_one.go b/vendor/code.vikunja.io/web/handler/read_one.go index f18a6316a..c548d685f 100644 --- a/vendor/code.vikunja.io/web/handler/read_one.go +++ b/vendor/code.vikunja.io/web/handler/read_one.go @@ -42,7 +42,11 @@ func (c *WebHandler) ReadOneWeb(ctx echo.Context) error { if err != nil { 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) return echo.NewHTTPError(http.StatusForbidden, "You don't have the right to see this") } diff --git a/vendor/code.vikunja.io/web/handler/update.go b/vendor/code.vikunja.io/web/handler/update.go index d65181a21..57fa5ec5a 100644 --- a/vendor/code.vikunja.io/web/handler/update.go +++ b/vendor/code.vikunja.io/web/handler/update.go @@ -41,7 +41,11 @@ func (c *WebHandler) UpdateWeb(ctx echo.Context) error { if err != nil { 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) return echo.NewHTTPError(http.StatusForbidden) } diff --git a/vendor/code.vikunja.io/web/web.go b/vendor/code.vikunja.io/web/web.go index e37ee7396..4231ab436 100644 --- a/vendor/code.vikunja.io/web/web.go +++ b/vendor/code.vikunja.io/web/web.go @@ -19,12 +19,12 @@ import "github.com/labstack/echo" // Rights defines rights methods type Rights interface { - IsAdmin(Auth) bool - CanWrite(Auth) bool - CanRead(Auth) bool - CanDelete(Auth) bool - CanUpdate(Auth) bool - CanCreate(Auth) bool + IsAdmin(Auth) (bool, error) + CanWrite(Auth) (bool, error) + CanRead(Auth) (bool, error) + CanDelete(Auth) (bool, error) + CanUpdate(Auth) (bool, error) + CanCreate(Auth) (bool, error) } // CRUDable defines the crud methods diff --git a/vendor/modules.txt b/vendor/modules.txt index 941d94108..6b3e5b844 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -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/handler # github.com/BurntSushi/toml v0.3.1