From a4137b3d6fe1c00f8491fe24f5492f3a7b7b6f6e Mon Sep 17 00:00:00 2001 From: konrad Date: Sat, 6 Oct 2018 13:25:37 +0200 Subject: [PATCH] Added more checks to not leave an error unpassed when checking for rights --- models/list_items_rights.go | 12 ++++++++++-- models/list_rights.go | 2 ++ models/namespace_rights.go | 16 ++++++++++++---- models/namespace_users_rights.go | 18 +++++++++++++++--- models/team_members_rights.go | 7 +++++-- models/team_namespace_rights.go | 18 +++++++++++++++--- models/teams_rights.go | 19 +++++++++++++++---- 7 files changed, 74 insertions(+), 18 deletions(-) diff --git a/models/list_items_rights.go b/models/list_items_rights.go index 18e0458aa..5380745f7 100644 --- a/models/list_items_rights.go +++ b/models/list_items_rights.go @@ -3,7 +3,11 @@ package models // CanDelete checks if the user can delete an task func (i *ListTask) CanDelete(doer *User) bool { // Get the task - lI, _ := GetListTaskByID(i.ID) + lI, err := GetListTaskByID(i.ID) + if err != nil { + Log.Error("Error occurred during CanDelete for ListTask: %s", err) + return false + } // A user can delete an task if he has write acces to its list list := &List{ID: lI.ListID} @@ -14,7 +18,11 @@ func (i *ListTask) CanDelete(doer *User) bool { // CanUpdate determines if a user has the right to update a list task func (i *ListTask) CanUpdate(doer *User) bool { // Get the task - lI, _ := GetListTaskByID(i.ID) + lI, err := GetListTaskByID(i.ID) + if err != nil { + Log.Error("Error occurred during CanDelete for ListTask: %s", err) + return false + } // A user can update an task if he has write acces to its list list := &List{ID: lI.ListID} diff --git a/models/list_rights.go b/models/list_rights.go index be91a688e..9a70e2412 100644 --- a/models/list_rights.go +++ b/models/list_rights.go @@ -82,6 +82,7 @@ func (l *List) checkListTeamRight(user *User, r TeamRight) bool { user.ID, r, user.ID, r, l.ID). Exist(&List{}) if err != nil { + Log.Error("Error occurred during checkListTeamRight for List: %s", err) return false } @@ -102,6 +103,7 @@ func (l *List) checkListUserRight(user *User, r UserRight) bool { user.ID, r, user.ID, r, user.ID, l.ID). Exist(&List{}) if err != nil { + Log.Error("Error occurred during checkListUserRight for List: %s", err) return false } diff --git a/models/namespace_rights.go b/models/namespace_rights.go index 4ab7fbb61..bf1160102 100644 --- a/models/namespace_rights.go +++ b/models/namespace_rights.go @@ -50,13 +50,21 @@ func (n *Namespace) CanRead(user *User) bool { // CanUpdate checks if the user can update the namespace func (n *Namespace) CanUpdate(user *User) bool { - nn, _ := GetNamespaceByID(n.ID) + nn, err := GetNamespaceByID(n.ID) + if err != nil { + Log.Error("Error occurred during CanUpdate for Namespace: %s", err) + return false + } return nn.IsAdmin(user) } // CanDelete checks if the user can delete a namespace func (n *Namespace) CanDelete(user *User) bool { - nn, _ := GetNamespaceByID(n.ID) + nn, err := GetNamespaceByID(n.ID) + if err != nil { + Log.Error("Error occurred during CanDelete for Namespace: %s", err) + return false + } return nn.IsAdmin(user) } @@ -75,8 +83,8 @@ func (n *Namespace) checkTeamRights(user *User, r TeamRight) bool { "AND ((team_members.user_id = ? AND team_namespaces.right = ?) "+ "OR namespaces.owner_id = ? ", n.ID, user.ID, r, user.ID). Get(&Namespace{}) - if err != nil { + Log.Error("Error occurred during checkTeamRights for Namespace: %s, TeamRight: %d", err, r) return false } @@ -91,8 +99,8 @@ func (n *Namespace) checkUserRights(user *User, r UserRight) bool { "namespaces.owner_id = ? "+ "OR (users_namespace.user_id = ? AND users_namespace.right = ?))", n.ID, user.ID, user.ID, r). Get(&Namespace{}) - if err != nil { + Log.Error("Error occurred during checkUserRights for Namespace: %s, UserRight: %d", err, r) return false } diff --git a/models/namespace_users_rights.go b/models/namespace_users_rights.go index 2c7685a26..80cd1771b 100644 --- a/models/namespace_users_rights.go +++ b/models/namespace_users_rights.go @@ -3,20 +3,32 @@ package models // CanCreate checks if the user can create a new user <-> namespace relation func (nu *NamespaceUser) CanCreate(doer *User) bool { // Get the namespace and check if the user has write access on it - n, _ := GetNamespaceByID(nu.NamespaceID) + n, err := GetNamespaceByID(nu.NamespaceID) + if err != nil { + Log.Error("Error occurred during CanCreate for NamespaceUser: %s", err) + return false + } return n.CanWrite(doer) } // CanDelete checks if the user can delete a user <-> namespace relation func (nu *NamespaceUser) CanDelete(doer *User) bool { // Get the namespace and check if the user has write access on it - n, _ := GetNamespaceByID(nu.NamespaceID) + n, err := GetNamespaceByID(nu.NamespaceID) + if err != nil { + Log.Error("Error occurred during CanCreate for NamespaceUser: %s", err) + return false + } return n.CanWrite(doer) } // CanUpdate checks if the user can update a user <-> namespace relation func (nu *NamespaceUser) CanUpdate(doer *User) bool { // Get the namespace and check if the user has write access on it - n, _ := GetNamespaceByID(nu.NamespaceID) + n, err := GetNamespaceByID(nu.NamespaceID) + if err != nil { + Log.Error("Error occurred during CanCreate for NamespaceUser: %s", err) + return false + } return n.CanWrite(doer) } diff --git a/models/team_members_rights.go b/models/team_members_rights.go index 6f23fc489..a97548bbf 100644 --- a/models/team_members_rights.go +++ b/models/team_members_rights.go @@ -12,9 +12,12 @@ func (tm *TeamMember) CanDelete(user *User) bool { // IsAdmin checks if the user is team admin func (tm *TeamMember) IsAdmin(user *User) bool { - // A user can add a member to a team if he is admin of that team - exists, _ := x.Where("user_id = ? AND team_id = ? AND admin = ?", user.ID, tm.TeamID, true). + exists, err := x.Where("user_id = ? AND team_id = ? AND admin = ?", user.ID, tm.TeamID, true). Get(&TeamMember{}) + if err != nil { + Log.Error("Error occurred during IsAdmin for TeamMember: %s", err) + return false + } return exists } diff --git a/models/team_namespace_rights.go b/models/team_namespace_rights.go index 4a4903e60..ba1f3b053 100644 --- a/models/team_namespace_rights.go +++ b/models/team_namespace_rights.go @@ -2,18 +2,30 @@ package models // CanCreate checks if one can create a new team <-> namespace relation func (tn *TeamNamespace) CanCreate(user *User) bool { - n, _ := GetNamespaceByID(tn.NamespaceID) + n, err := GetNamespaceByID(tn.NamespaceID) + if err != nil { + Log.Error("Error occurred during CanCreate for TeamNamespace: %s", err) + return false + } return n.IsAdmin(user) } // CanDelete checks if a user can remove a team from a namespace. Only namespace admins can do that. func (tn *TeamNamespace) CanDelete(user *User) bool { - n, _ := GetNamespaceByID(tn.NamespaceID) + n, err := GetNamespaceByID(tn.NamespaceID) + if err != nil { + Log.Error("Error occurred during CanDelete for TeamNamespace: %s", err) + return false + } return n.IsAdmin(user) } // CanUpdate checks if a user can update a team from a namespace. Only namespace admins can do that. func (tn *TeamNamespace) CanUpdate(user *User) bool { - n, _ := GetNamespaceByID(tn.NamespaceID) + n, err := GetNamespaceByID(tn.NamespaceID) + if err != nil { + Log.Error("Error occurred during CanUpdate for TeamNamespace: %s", err) + return false + } return n.IsAdmin(user) } diff --git a/models/teams_rights.go b/models/teams_rights.go index 1e82cd157..5a1fbe405 100644 --- a/models/teams_rights.go +++ b/models/teams_rights.go @@ -36,34 +36,45 @@ func (t *Team) CanCreate(user *User) bool { func (t *Team) CanUpdate(user *User) bool { // Check if the current user is in the team and has admin rights in it - exists, _ := x.Where("team_id = ?", t.ID). + exists, err := x.Where("team_id = ?", t.ID). And("user_id = ?", user.ID). And("admin = ?", true). Get(&TeamMember{}) + if err != nil { + 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(user *User) bool { - //t.ID = id return t.IsAdmin(user) } // IsAdmin returns true when the user is admin of a team func (t *Team) IsAdmin(user *User) bool { - exists, _ := x.Where("team_id = ?", t.ID). + exists, err := x.Where("team_id = ?", t.ID). And("user_id = ?", user.ID). And("admin = ?", true). Get(&TeamMember{}) + if err != nil { + 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(user *User) bool { // Check if the user is in the team - exists, _ := x.Where("team_id = ?", t.ID). + exists, err := x.Where("team_id = ?", t.ID). And("user_id = ?", user.ID). Get(&TeamMember{}) + if err != nil { + Log.Error("Error occurred during CanUpdate for Team: %s", err) + return false + } return exists }