add safeguards to handlers

api/server/handlers/project/update_collaborator_roles.go

@@ -90,91 +90,77 @@ func (p *UpdateCollaboratorRolesHandler) ServeHTTP(w http.ResponseWriter, r *htt
 		return
 	}
 
+	if len(userRoles) == 0 {
+		p.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(
+			fmt.Errorf("user is not a collaborator in this project"), http.StatusBadRequest,
+		))
+		return
+	}
+
 	userRolesMap := make(map[string]bool)
 
 	for _, role := range userRoles {
 		userRolesMap[role.UniqueID] = true
 	}
 
-	if len(userRoles) == 0 {
-		for _, uid := range request.RoleUIDs {
-			role, err := p.Repo().ProjectRole().ReadProjectRole(proj.ID, uid)
-
-			if err != nil {
-				p.HandleAPIError(w, r, apierrors.NewErrInternal(err))
-				return
-			}
+	var rolesToAdd []string
+	var rolesToRemove []string
 
-			userIDs := role.GetUserIDs()
-			userIDs = append(userIDs, userID)
-
-			err = p.Repo().ProjectRole().UpdateUsersInProjectRole(proj.ID, uid, userIDs)
-
-			if err != nil {
-				p.HandleAPIError(w, r, apierrors.NewErrInternal(err))
-				return
-			}
-		}
-	} else {
-		var rolesToAdd []string
-		var rolesToRemove []string
-
-		for _, uid := range userRoles {
-			if _, ok := rolesMap[uid.UniqueID]; !ok {
-				// user had this role, should be removed from
-				rolesToRemove = append(rolesToRemove, uid.UniqueID)
-			}
+	for _, uid := range userRoles {
+		if _, ok := rolesMap[uid.UniqueID]; !ok {
+			// user had this role, should be removed from
+			rolesToRemove = append(rolesToRemove, uid.UniqueID)
 		}
+	}
 
-		for _, uid := range request.RoleUIDs {
-			if _, ok := userRolesMap[uid]; !ok {
-				// user does not have this role, should be added to
-				rolesToAdd = append(rolesToAdd, uid)
-			}
+	for _, uid := range request.RoleUIDs {
+		if _, ok := userRolesMap[uid]; !ok {
+			// user does not have this role, should be added to
+			rolesToAdd = append(rolesToAdd, uid)
 		}
+	}
 
-		for _, uid := range rolesToAdd {
-			role, err := p.Repo().ProjectRole().ReadProjectRole(proj.ID, uid)
+	for _, uid := range rolesToAdd {
+		role, err := p.Repo().ProjectRole().ReadProjectRole(proj.ID, uid)
 
-			if err != nil {
-				p.HandleAPIError(w, r, apierrors.NewErrInternal(err))
-				return
-			}
+		if err != nil {
+			p.HandleAPIError(w, r, apierrors.NewErrInternal(err))
+			return
+		}
 
-			userIDs := role.GetUserIDs()
-			userIDs = append(userIDs, userID)
+		userIDs := role.GetUserIDs()
+		userIDs = append(userIDs, userID)
 
-			err = p.Repo().ProjectRole().UpdateUsersInProjectRole(proj.ID, uid, userIDs)
+		err = p.Repo().ProjectRole().UpdateUsersInProjectRole(proj.ID, uid, userIDs)
 
-			if err != nil {
-				p.HandleAPIError(w, r, apierrors.NewErrInternal(err))
-				return
-			}
+		if err != nil {
+			p.HandleAPIError(w, r, apierrors.NewErrInternal(err))
+			return
 		}
+	}
 
-		for _, uid := range rolesToRemove {
-			role, err := p.Repo().ProjectRole().ReadProjectRole(proj.ID, uid)
+	for _, uid := range rolesToRemove {
+		role, err := p.Repo().ProjectRole().ReadProjectRole(proj.ID, uid)
 
-			if err != nil {
-				p.HandleAPIError(w, r, apierrors.NewErrInternal(err))
-				return
-			}
+		if err != nil {
+			p.HandleAPIError(w, r, apierrors.NewErrInternal(err))
+			return
+		}
 
-			userIDs := role.GetUserIDs()
-			var newUserIDs []uint
+		userIDs := role.GetUserIDs()
+		var newUserIDs []uint
 
-			for _, id := range userIDs {
-				if id != userID {
-					newUserIDs = append(newUserIDs, id)
-				}
+		for _, id := range userIDs {
+			if id != userID {
+				newUserIDs = append(newUserIDs, id)
 			}
+		}
 
-			err = p.Repo().ProjectRole().UpdateUsersInProjectRole(proj.ID, uid, newUserIDs)
+		err = p.Repo().ProjectRole().UpdateUsersInProjectRole(proj.ID, uid, newUserIDs)
 
-			if err != nil {
-				p.HandleAPIError(w, r, apierrors.NewErrInternal(err))
-				return
-			}
+		if err != nil {
+			p.HandleAPIError(w, r, apierrors.NewErrInternal(err))
+			return
 		}
 	}
 }

api/server/handlers/project_role/create.go

@@ -2,6 +2,7 @@ package project_role
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"net/http"
 
@@ -12,6 +13,8 @@ import (
 	"github.com/porter-dev/porter/api/types"
 	"github.com/porter-dev/porter/internal/encryption"
 	"github.com/porter-dev/porter/internal/models"
+	"github.com/porter-dev/porter/internal/repository"
+	"gorm.io/gorm"
 )
 
 type CreateProjectRoleHandler struct {
@@ -100,13 +103,18 @@ func (c *CreateProjectRoleHandler) ServeHTTP(w http.ResponseWriter, r *http.Requ
 	}
 
 	if len(request.Users) > 0 {
+		for _, u := range request.Users {
+			err := validateUserForProjectRole(c.Repo(), u, project.ID)
+
+			if err != nil {
+				c.HandleAPIError(w, r, err)
+				return
+			}
+		}
+
 		err = c.Repo().ProjectRole().UpdateUsersInProjectRole(project.ID, role.UniqueID, request.Users)
 
 		if err != nil {
-			// we need to delete the policy and project role we just created
-			c.Repo().Policy().DeletePolicy(policy)
-			c.Repo().ProjectRole().DeleteProjectRole(role)
-
 			c.HandleAPIError(w, r, apierrors.NewErrInternal(err))
 			return
 		}
@@ -114,3 +122,28 @@ func (c *CreateProjectRoleHandler) ServeHTTP(w http.ResponseWriter, r *http.Requ
 
 	w.WriteHeader(http.StatusCreated)
 }
+
+func validateUserForProjectRole(repo repository.Repository, userID, projectID uint) apierrors.RequestError {
+	// check for valid user
+	_, err := repo.User().ReadUser(userID)
+
+	if err != nil && errors.Is(err, gorm.ErrRecordNotFound) {
+		return apierrors.NewErrNotFound(fmt.Errorf("user with id %d does not exist", userID))
+	} else if err != nil {
+		return apierrors.NewErrInternal(err)
+	}
+
+	// a user needs to have been a collaborator with at least one role already in a project to be added to a new role
+	roles, err := repo.ProjectRole().ListAllRolesForUser(projectID, userID)
+
+	if err != nil {
+		return apierrors.NewErrInternal(err)
+	}
+
+	if len(roles) == 0 {
+		return apierrors.NewErrPassThroughToClient(fmt.Errorf("user is not a collaborator in this project"),
+			http.StatusBadRequest)
+	}
+
+	return nil
+}

api/server/handlers/project_role/update.go

@@ -58,7 +58,14 @@ func (c *UpdateProjectRoleHandler) ServeHTTP(w http.ResponseWriter, r *http.Requ
 		return
 	}
 
-	if !role.IsDefaultRole() && request.Name != "" && request.Name != role.Name {
+	if role.IsDefaultRole() {
+		c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(
+			fmt.Errorf("cannot update default project roles"), http.StatusBadRequest,
+		))
+		return
+	}
+
+	if request.Name != "" && request.Name != role.Name {
 		if request.Name == string(types.RoleAdmin) ||
 			request.Name == string(types.RoleDeveloper) ||
 			request.Name == string(types.RoleViewer) {
@@ -86,6 +93,15 @@ func (c *UpdateProjectRoleHandler) ServeHTTP(w http.ResponseWriter, r *http.Requ
 			return
 		}
 	} else {
+		for _, u := range request.Users {
+			err := validateUserForProjectRole(c.Repo(), u, project.ID)
+
+			if err != nil {
+				c.HandleAPIError(w, r, err)
+				return
+			}
+		}
+
 		err = c.Repo().ProjectRole().UpdateUsersInProjectRole(project.ID, roleUID, request.Users)
 
 		if err != nil {
@@ -94,7 +110,7 @@ func (c *UpdateProjectRoleHandler) ServeHTTP(w http.ResponseWriter, r *http.Requ
 		}
 	}
 
-	if !role.IsDefaultRole() && request.Policy != nil {
+	if request.Policy != nil {
 		policy, err := c.Repo().Policy().ReadPolicy(project.ID, role.PolicyUID)
 
 		if err != nil {