Added better permission checks to item linking, attachment settings

Author: Stefan Ernst

internal/handlers/base.go

@@ -3,8 +3,12 @@ package handlers
 import (
 	"database/sql"
 	"errors"
+	"fmt"
+	"log/slog"
 	"net/http"
+	"strings"
 	"windshift/internal/database"
+	"windshift/internal/middleware"
 	"windshift/internal/models"
 	"windshift/internal/services"
 	"windshift/internal/utils"
@@ -164,3 +168,95 @@ func AuthorizeUserRequest(w http.ResponseWriter, r *http.Request, targetUserID i
 
 	return currentUser
 }
+
+// CheckItemPermission verifies the user has the given permission on the item's workspace.
+// Returns 404 on both not-found and no-permission to prevent item existence leakage.
+func CheckItemPermission(w http.ResponseWriter, r *http.Request, db database.Database,
+	permService *services.PermissionService, itemID int, permission string) bool {
+	user, ok := r.Context().Value(middleware.ContextKeyUser).(*models.User)
+	if !ok {
+		respondUnauthorized(w, r)
+		return false
+	}
+	var workspaceID int
+	err := db.QueryRow("SELECT workspace_id FROM items WHERE id = ?", itemID).Scan(&workspaceID)
+	if err != nil {
+		respondNotFound(w, r, "Item")
+		return false
+	}
+	hasPermission, err := permService.HasWorkspacePermission(user.ID, workspaceID, permission)
+	if err != nil || !hasPermission {
+		respondNotFound(w, r, "Item") // 404, not 403 — prevents existence leakage
+		return false
+	}
+	return true
+}
+
+// GetAccessibleWorkspaceIDs returns IDs of active workspaces the user can view.
+func GetAccessibleWorkspaceIDs(user *models.User, db database.Database,
+	permService *services.PermissionService) ([]int, error) {
+	if user == nil || permService == nil {
+		return []int{}, nil
+	}
+	rows, err := db.Query("SELECT id FROM workspaces WHERE active = 1")
+	if err != nil {
+		return nil, fmt.Errorf("failed to query workspaces: %w", err)
+	}
+	defer rows.Close()
+	var ids []int
+	for rows.Next() {
+		var id int
+		if err := rows.Scan(&id); err != nil {
+			continue
+		}
+		hasView, err := permService.HasWorkspacePermission(user.ID, id, models.PermissionItemView)
+		if err != nil {
+			slog.Error("error checking view permission", slog.Int("workspace_id", id), slog.Any("error", err))
+			continue
+		}
+		if hasView {
+			ids = append(ids, id)
+		}
+	}
+	return ids, rows.Err()
+}
+
+// GetAccessibleWorkspaceKeys returns a set of workspace keys the user can view.
+func GetAccessibleWorkspaceKeys(user *models.User, db database.Database,
+	permService *services.PermissionService) (map[string]bool, error) {
+	if user == nil || permService == nil {
+		return map[string]bool{}, nil
+	}
+	rows, err := db.Query("SELECT id, key FROM workspaces WHERE active = 1")
+	if err != nil {
+		return nil, fmt.Errorf("failed to query workspaces: %w", err)
+	}
+	defer rows.Close()
+	keys := make(map[string]bool)
+	for rows.Next() {
+		var id int
+		var key string
+		if err := rows.Scan(&id, &key); err != nil {
+			continue
+		}
+		hasView, err := permService.HasWorkspacePermission(user.ID, id, models.PermissionItemView)
+		if err != nil {
+			continue
+		}
+		if hasView {
+			keys[key] = true
+		}
+	}
+	return keys, rows.Err()
+}
+
+// BuildWorkspaceIDPlaceholders builds a parameterized IN clause for workspace IDs.
+func BuildWorkspaceIDPlaceholders(ids []int) (string, []interface{}) {
+	placeholders := make([]string, len(ids))
+	args := make([]interface{}, len(ids))
+	for i, id := range ids {
+		placeholders[i] = "?"
+		args[i] = id
+	}
+	return strings.Join(placeholders, ", "), args
+}

internal/handlers/diagram.go

@@ -11,18 +11,26 @@ import (
 	"windshift/internal/database"
 	"windshift/internal/middleware"
 	"windshift/internal/models"
+	"windshift/internal/services"
 )
 
 type DiagramHandler struct {
-	db database.Database
+	db                database.Database
+	permissionService *services.PermissionService
 }
 
-func NewDiagramHandler(db database.Database) *DiagramHandler {
+func NewDiagramHandler(db database.Database, permissionService *services.PermissionService) *DiagramHandler {
 	return &DiagramHandler{
-		db: db,
+		db:                db,
+		permissionService: permissionService,
 	}
 }
 
+// checkItemEditPermission checks if the current user can edit the given item
+func (h *DiagramHandler) checkItemEditPermission(w http.ResponseWriter, r *http.Request, itemID int) bool {
+	return CheckItemPermission(w, r, h.db, h.permissionService, itemID, models.PermissionItemEdit)
+}
+
 // Create creates a new diagram for an item
 func (h *DiagramHandler) Create(w http.ResponseWriter, r *http.Request) {
 	itemIDStr := r.PathValue("itemId")
@@ -32,6 +40,10 @@ func (h *DiagramHandler) Create(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	if !h.checkItemEditPermission(w, r, itemID) {
+		return
+	}
+
 	var req struct {
 		Name        string `json:"name"`
 		DiagramData string `json:"diagram_data"`
@@ -271,6 +283,10 @@ func (h *DiagramHandler) Update(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	if !h.checkItemEditPermission(w, r, itemID) {
+		return
+	}
+
 	query := `
 		UPDATE item_diagrams
 		SET name = ?, diagram_data = ?, updated_at = ?, updated_by = ?
@@ -385,6 +401,10 @@ func (h *DiagramHandler) Delete(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	if !h.checkItemEditPermission(w, r, itemID) {
+		return
+	}
+
 	// Record history before deletion
 	if userID != nil {
 		if err := h.recordDiagramHistory(itemID, userID, "diagram_deleted", &diagramName, 0, diagramName); err != nil {

internal/handlers/item_links.go

@@ -16,6 +16,7 @@ import (
 
 type ItemLinkHandler struct {
 	db                  database.Database
+	permissionService   *services.PermissionService
 	notificationService interface {
 		EmitEvent(event *services.NotificationEvent)
 	} // Notification service for async notification processing (optional, can be nil)
@@ -26,13 +27,19 @@ type ItemLinkHandler struct {
 
 func NewItemLinkHandler(db database.Database, notificationService interface {
 	EmitEvent(event *services.NotificationEvent)
-}) *ItemLinkHandler {
+}, permissionService *services.PermissionService) *ItemLinkHandler {
 	return &ItemLinkHandler{
 		db:                  db,
 		notificationService: notificationService,
+		permissionService:   permissionService,
 	}
 }
 
+// checkItemEditPermission checks if the current user can edit the given item
+func (h *ItemLinkHandler) checkItemEditPermission(w http.ResponseWriter, r *http.Request, itemID int) bool {
+	return CheckItemPermission(w, r, h.db, h.permissionService, itemID, models.PermissionItemEdit)
+}
+
 // SetActionService sets the action service for automation workflows
 func (h *ItemLinkHandler) SetActionService(actionService interface {
 	EmitActionEvent(event *models.ActionEvent)
@@ -49,6 +56,23 @@ func (h *ItemLinkHandler) GetLinksForItem(w http.ResponseWriter, r *http.Request
 		return
 	}
 
+	user := h.getUserFromContext(r)
+	if user == nil {
+		respondUnauthorized(w, r)
+		return
+	}
+
+	// Check item.view permission if it's a work item
+	var workspaceID int
+	isWorkItem := h.db.QueryRow("SELECT workspace_id FROM items WHERE id = ?", id).Scan(&workspaceID) == nil
+	if isWorkItem {
+		hasView, _ := h.permissionService.HasWorkspacePermission(user.ID, workspaceID, models.PermissionItemView)
+		if !hasView {
+			respondNotFound(w, r, "item")
+			return
+		}
+	}
+
 	// Convert URL path to internal type
 	internalType := "item"
 	if itemType == "test-cases" {
@@ -69,6 +93,11 @@ func (h *ItemLinkHandler) GetLinksForItem(w http.ResponseWriter, r *http.Request
 		return
 	}
 
+	// Filter linked items by accessible workspaces
+	accessibleKeys, _ := GetAccessibleWorkspaceKeys(user, h.db, h.permissionService)
+	outgoingLinks = filterLinksByAccessibleWorkspaces(outgoingLinks, accessibleKeys)
+	incomingLinks = filterLinksByAccessibleWorkspaces(incomingLinks, accessibleKeys)
+
 	response := map[string]interface{}{
 		"outgoing": outgoingLinks,
 		"incoming": incomingLinks,
@@ -78,6 +107,21 @@ func (h *ItemLinkHandler) GetLinksForItem(w http.ResponseWriter, r *http.Request
 	json.NewEncoder(w).Encode(response)
 }
 
+// filterLinksByAccessibleWorkspaces removes links pointing to items in inaccessible workspaces
+func filterLinksByAccessibleWorkspaces(links []models.ItemLink, accessibleKeys map[string]bool) []models.ItemLink {
+	filtered := make([]models.ItemLink, 0, len(links))
+	for _, link := range links {
+		if link.SourceType == "item" && link.SourceWorkspaceKey != "" && !accessibleKeys[link.SourceWorkspaceKey] {
+			continue
+		}
+		if link.TargetType == "item" && link.TargetWorkspaceKey != "" && !accessibleKeys[link.TargetWorkspaceKey] {
+			continue
+		}
+		filtered = append(filtered, link)
+	}
+	return filtered
+}
+
 // CreateLink creates a new link between items
 func (h *ItemLinkHandler) CreateLink(w http.ResponseWriter, r *http.Request) {
 	var link models.ItemLink
@@ -146,6 +190,19 @@ func (h *ItemLinkHandler) CreateLink(w http.ResponseWriter, r *http.Request) {
 	}
 	createdBy := currentUser.ID
 
+	// Check item.edit on source (authorizes modifying the source by adding a link)
+	if link.SourceType == "item" {
+		if !CheckItemPermission(w, r, h.db, h.permissionService, link.SourceID, models.PermissionItemEdit) {
+			return
+		}
+	}
+	// Check item.view on target (verifies user can see it — prevents existence leakage)
+	if link.TargetType == "item" {
+		if !CheckItemPermission(w, r, h.db, h.permissionService, link.TargetID, models.PermissionItemView) {
+			return
+		}
+	}
+
 	// Create link via service (handles link type validation + insert)
 	linkSvc := services.NewItemLinkService(h.db)
 	id, err := linkSvc.CreateLink(services.CreateItemLinkParams{
@@ -270,6 +327,13 @@ func (h *ItemLinkHandler) DeleteLink(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// Check item.edit permission for item-type source
+	if sourceType == "item" {
+		if !h.checkItemEditPermission(w, r, sourceID) {
+			return
+		}
+	}
+
 	result, err := h.db.ExecWrite("DELETE FROM item_links WHERE id = ?", id)
 	if err != nil {
 		respondInternalError(w, r, err)
@@ -333,6 +397,10 @@ func (h *ItemLinkHandler) GetLinkedAssets(w http.ResponseWriter, r *http.Request
 		return
 	}
 
+	if !CheckItemPermission(w, r, h.db, h.permissionService, id, models.PermissionItemView) {
+		return
+	}
+
 	// Get assets where item is the source
 	outgoingQuery := `
 		SELECT a.id, a.title, COALESCE(a.description, '') AS description,
@@ -441,6 +509,18 @@ func (h *ItemLinkHandler) GetLinkedAssets(w http.ResponseWriter, r *http.Request
 
 // SearchLinkableItems searches for items that can be linked
 func (h *ItemLinkHandler) SearchLinkableItems(w http.ResponseWriter, r *http.Request) {
+	user := h.getUserFromContext(r)
+	if user == nil {
+		respondUnauthorized(w, r)
+		return
+	}
+
+	accessibleWorkspaceIDs, err := GetAccessibleWorkspaceIDs(user, h.db, h.permissionService)
+	if err != nil {
+		respondInternalError(w, r, err)
+		return
+	}
+
 	query := r.URL.Query().Get("q")
 	itemType := r.URL.Query().Get("type") // "item", "test_case", "asset", or empty for all
 	limit := 20
@@ -455,7 +535,7 @@ func (h *ItemLinkHandler) SearchLinkableItems(w http.ResponseWriter, r *http.Req
 
 	// Search work items
 	if itemType == "" || itemType == "item" {
-		workItems, err := h.searchWorkItems(query, limit)
+		workItems, err := h.searchWorkItems(query, limit, accessibleWorkspaceIDs)
 		if err != nil {
 			respondInternalError(w, r, err)
 			return
@@ -569,11 +649,16 @@ func (h *ItemLinkHandler) getLinkByID(id int) (*models.ItemLink, error) {
 	return &links[0], nil
 }
 
-func (h *ItemLinkHandler) searchWorkItems(query string, limit int) ([]models.LinkableItem, error) {
-	sqlQuery := `
-		SELECT 
-			i.id, 
-			i.title, 
+func (h *ItemLinkHandler) searchWorkItems(query string, limit int, accessibleWorkspaceIDs []int) ([]models.LinkableItem, error) {
+	if len(accessibleWorkspaceIDs) == 0 {
+		return []models.LinkableItem{}, nil
+	}
+
+	placeholders, wsArgs := BuildWorkspaceIDPlaceholders(accessibleWorkspaceIDs)
+	sqlQuery := fmt.Sprintf(`
+		SELECT
+			i.id,
+			i.title,
 			COALESCE(i.description, '') AS description,
 			i.workspace_id,
 			w.name AS workspace_name,
@@ -583,13 +668,17 @@ func (h *ItemLinkHandler) searchWorkItems(query string, limit int) ([]models.Lin
 		LEFT JOIN workspaces w ON i.workspace_id = w.id
 		LEFT JOIN statuses s ON i.status_id = s.id
 		LEFT JOIN priorities p ON i.priority_id = p.id
-		WHERE i.title LIKE ? OR i.description LIKE ?
+		WHERE (i.title LIKE ? OR i.description LIKE ?)
+		  AND i.workspace_id IN (%s)
 		ORDER BY i.title
 		LIMIT ?
-	`
+	`, placeholders)
 
 	searchTerm := "%" + query + "%"
-	rows, err := h.db.Query(sqlQuery, searchTerm, searchTerm, limit)
+	args := []interface{}{searchTerm, searchTerm}
+	args = append(args, wsArgs...)
+	args = append(args, limit)
+	rows, err := h.db.Query(sqlQuery, args...)
 	if err != nil {
 		return nil, err
 	}