fix(api): close five real authorization gaps, and skip the 48-site sweep that would have closed none

apps/admin/package.json

@@ -1,6 +1,6 @@
 {
   "name": "admin",
-  "version": "1.4.38",
+  "version": "1.4.39",
   "private": true,
   "description": "Admin UI for Pace",
   "license": "AGPL-3.0",

apps/api/internal/access/membership.go

@@ -0,0 +1,57 @@
+// Package access holds the project membership predicate every application's queries are written around, because all three of them ask the same question and the answer is an authorization decision.
+//
+// Two columns on project_members look like two spellings of one thing and are not. `is_active = FALSE` means this person was removed from the project: it is written synchronously, by project member removal, by workspace member removal and by account deactivation, and none of those three touches deleted_at. `deleted_at IS NOT NULL` means the project or workspace the membership belonged to was soft-deleted: it is written by one path only, the worker's cascade over the relation graph, which runs after the delete has already answered and after the parent row's own deleted_at is set.
+//
+// So the membership test is is_active, and it is unconditional. The membership's own soft delete is a lagging copy of a parent-liveness signal, and whether a query carries it is a question about which queryset the query is written from: Django's default manager filters the model's own queryset and never a join traversed through it, so a predicate written from ProjectMember's manager carries deleted_at IS NULL and a relation traversal from another model's queryset does not. That is why the option below is an option rather than the default — the sites that leave it out are reproducing the port's rule, and the base row's own deleted_at, stamped by the same cascade, is what excludes a cascaded row from those queries anyway.
+package access
+
+import (
+	"context"
+
+	"gorm.io/gorm"
+)
+
+// MemberJoin is the membership join a project-scoped queryset carries: the caller's own active membership of the project the base row belongs to. It is an inner join, so a row whose reader is not a member of its project does not survive it.
+//
+// alias and projectColumn say where the project id is read from, which is not always <alias>.project_id — a query over projects joins on p.id, and a page reaches its project through the link table, so it joins on pp.project_id. The member id stays a bind argument of the call site, which reads Joins(access.MemberJoin("i", "project_id"), user.ID).
+func MemberJoin(alias, projectColumn string, options ...Option) string {
+	var settings joinSettings
+	for _, option := range options {
+		option(&settings)
+	}
+	join := "JOIN project_members pm ON pm.project_id = " + alias + "." + projectColumn +
+		" AND pm.member_id = ? AND pm.is_active = TRUE"
+	if settings.membershipSoftDelete {
+		join += " AND pm.deleted_at IS NULL"
+	}
+	return join
+}
+
+// Option narrows what MemberJoin writes. There is one, and the reason it exists is in this package's doc comment.
+type Option func(*joinSettings)
+
+// joinSettings is unexported so that the only options are the ones named here.
+type joinSettings struct {
+	membershipSoftDelete bool
+}
+
+// WithMembershipSoftDelete adds the membership row's own soft-delete check, for the queries whose predicate is written from project_members' own manager rather than traversed into from another model's queryset. It narrows nothing the parent's deleted_at does not narrow first, and it narrows it later, because the cascade that writes the one has already written the other.
+func WithMembershipSoftDelete() Option {
+	return func(settings *joinSettings) { settings.membershipSoftDelete = true }
+}
+
+// ProjectRole reads one person's role in one project and reports whether they are an active member of it at all. It is the permission gate itself rather than a predicate joined into a larger query — the membership is the whole subject — so it filters the membership's own soft delete, as MemberJoin does only when asked to.
+//
+// It answers with the role rather than a count of rows matching a role, which the callers that used to count rely on: project_members carries a unique index on (project_id, member_id) where deleted_at is null (migrate/testdata/schema.tsv:2697), and the workspace is joined on its primary key, so at most one row can match and "no row with this role" and "the one row has another role" are the same answer.
+func ProjectRole(ctx context.Context, db *gorm.DB, slug, projectID, memberID string) (int, bool, error) {
+	var roles []int
+	err := db.WithContext(ctx).Table("project_members pm").
+		Joins("JOIN workspaces w ON w.id = pm.workspace_id").
+		Where("w.slug = ? AND pm.project_id = ? AND pm.member_id = ? AND pm.is_active = TRUE AND pm.deleted_at IS NULL",
+			slug, projectID, memberID).
+		Limit(1).Pluck("pm.role", &roles).Error
+	if err != nil || len(roles) == 0 {
+		return 0, false, err
+	}
+	return roles[0], true, nil
+}

apps/api/internal/access/membership_test.go

@@ -0,0 +1,52 @@
+package access
+
+import (
+	"strings"
+	"testing"
+)
+
+// The expected strings here are the literals the call sites carried before they were replaced, copied out of them character for character. That is the point of the test: the join this package writes has to be the same SQL those queries have always sent, for each of the three shapes of project column the tree uses.
+func TestTheMembershipJoinIsWhatTheCallSitesWrote(t *testing.T) {
+	for _, test := range []struct {
+		name string
+		got  string
+		want string
+	}{
+		{
+			name: "the base row's project",
+			got:  MemberJoin("i", "project_id"),
+			want: "JOIN project_members pm ON pm.project_id = i.project_id AND pm.member_id = ? AND pm.is_active = TRUE",
+		},
+		{
+			name: "a query over projects themselves",
+			got:  MemberJoin("p", "id"),
+			want: "JOIN project_members pm ON pm.project_id = p.id AND pm.member_id = ? AND pm.is_active = TRUE",
+		},
+		{
+			name: "a page, which reaches its project through the link table",
+			got:  MemberJoin("pp", "project_id"),
+			want: "JOIN project_members pm ON pm.project_id = pp.project_id AND pm.member_id = ? AND pm.is_active = TRUE",
+		},
+		{
+			name: "the membership's own soft delete, where the queryset is project_members'",
+			got:  MemberJoin("l", "project_id", WithMembershipSoftDelete()),
+			want: "JOIN project_members pm ON pm.project_id = l.project_id AND pm.member_id = ? AND pm.is_active = TRUE AND pm.deleted_at IS NULL",
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			if test.got != test.want {
+				t.Errorf("the join renders\n  %s\nwant\n  %s", test.got, test.want)
+			}
+		})
+	}
+}
+
+// The option is the only thing that adds the membership's soft delete, so a site that does not pass it cannot acquire the filter by accident.
+func TestTheMembershipSoftDeleteIsOptIn(t *testing.T) {
+	if got := MemberJoin("i", "project_id"); strings.Contains(got, "pm.deleted_at") {
+		t.Errorf("the plain join filters the membership's soft delete:\n  %s", got)
+	}
+	if got := MemberJoin("i", "project_id", WithMembershipSoftDelete()); !strings.Contains(got, "AND pm.deleted_at IS NULL") {
+		t.Errorf("the option did not add the membership's soft delete:\n  %s", got)
+	}
+}

apps/api/internal/externalapi/cycle_crud_handler.go

@@ -6,6 +6,7 @@ import (
 	"time"
 
 	"github.com/gin-gonic/gin"
+	"github.com/yldm-tech/pace/apps/api/internal/access"
 	"github.com/yldm-tech/pace/apps/api/internal/auth"
 	"github.com/yldm-tech/pace/apps/api/internal/cycles"
 	"github.com/yldm-tech/pace/apps/api/internal/drf"
@@ -102,7 +103,7 @@ func (handler *Handler) cycleRetrieve(c *gin.Context, user *auth.User, _ APIToke
 func (handler *Handler) cycleScope(c *gin.Context, user *auth.User) *gorm.DB {
 	return handler.db.WithContext(c.Request.Context()).Table("cycles c").
 		Joins("JOIN workspaces w ON w.id = c.workspace_id").
-		Joins("JOIN project_members pm ON pm.project_id = c.project_id AND pm.member_id = ? AND pm.is_active = TRUE", user.ID).
+		Joins(access.MemberJoin("c", "project_id"), user.ID).
 		Joins("LEFT JOIN cycle_issues ci ON ci.cycle_id = c.id").
 		Joins("LEFT JOIN issues ii ON ii.id = ci.issue_id").
 		Where("w.slug = ? AND c.project_id = ? AND c.deleted_at IS NULL", c.Param("slug"), c.Param("project"))

apps/api/internal/externalapi/cycle_handler.go

@@ -5,6 +5,7 @@ import (
 	"time"
 
 	"github.com/gin-gonic/gin"
+	"github.com/yldm-tech/pace/apps/api/internal/access"
 	"github.com/yldm-tech/pace/apps/api/internal/auth"
 	"gorm.io/gorm"
 )
@@ -98,7 +99,7 @@ func (handler *Handler) archivedCycleList(c *gin.Context, user *auth.User, _ API
 	err := handler.db.WithContext(c.Request.Context()).Table("cycles c").
 		Select(externalCycleAnnotations()).
 		Joins("JOIN workspaces w ON w.id = c.workspace_id").
-		Joins("JOIN project_members pm ON pm.project_id = c.project_id AND pm.member_id = ? AND pm.is_active = TRUE", user.ID).
+		Joins(access.MemberJoin("c", "project_id"), user.ID).
 		Joins("LEFT JOIN cycle_issues ci ON ci.cycle_id = c.id").
 		Joins("LEFT JOIN issues ii ON ii.id = ci.issue_id").
 		Joins("LEFT JOIN estimate_points ep ON ep.id = ii.estimate_point_id").

apps/api/internal/externalapi/cycle_issue_handler.go

@@ -6,6 +6,7 @@ import (
 	"time"
 
 	"github.com/gin-gonic/gin"
+	"github.com/yldm-tech/pace/apps/api/internal/access"
 	"github.com/yldm-tech/pace/apps/api/internal/auth"
 	"github.com/yldm-tech/pace/apps/api/internal/drf"
 	"gorm.io/gorm"
@@ -285,7 +286,7 @@ func (handler *Handler) cycleIssueLinks(c *gin.Context, user *auth.User) ([]cycl
 	err := handler.db.WithContext(c.Request.Context()).Table("cycle_issues ci").
 		Select(cycleIssueSelection()).
 		Joins("JOIN workspaces w ON w.id = ci.workspace_id").
-		Joins("JOIN project_members pm ON pm.project_id = ci.project_id AND pm.member_id = ? AND pm.is_active = TRUE", user.ID).
+		Joins(access.MemberJoin("ci", "project_id"), user.ID).
 		Where("w.slug = ? AND ci.project_id = ? AND ci.cycle_id = ? AND ci.deleted_at IS NULL",
 			c.Param("slug"), c.Param("project"), c.Param("cycle")).
 		Order("ci.created_at DESC").Scan(&rows).Error

apps/api/internal/externalapi/intake_handler.go

@@ -6,6 +6,7 @@ import (
 	"time"
 
 	"github.com/gin-gonic/gin"
+	"github.com/yldm-tech/pace/apps/api/internal/access"
 	"github.com/yldm-tech/pace/apps/api/internal/auth"
 	"github.com/yldm-tech/pace/apps/api/internal/drf"
 	"github.com/yldm-tech/pace/apps/api/internal/htmlsanitizer"
@@ -490,17 +491,9 @@ func (handler *Handler) requireProjectLite(c *gin.Context, user *auth.User) bool
 	return true
 }
 
+// projectRole reads the caller's role in the project named in the url, and reports whether they are a member of it at all. It is what ProjectLitePermission asks, and what requireProjectBase asks for a write that is neither a create nor a safe method.
 func (handler *Handler) projectRole(c *gin.Context, user *auth.User) (int, bool, error) {
-	var roles []int
-	err := handler.db.WithContext(c.Request.Context()).Table("project_members pm").
-		Joins("JOIN workspaces w ON w.id = pm.workspace_id").
-		Where("w.slug = ? AND pm.project_id = ? AND pm.member_id = ? AND pm.is_active = TRUE",
-			c.Param("slug"), c.Param("project"), user.ID).
-		Limit(1).Pluck("pm.role", &roles).Error
-	if err != nil || len(roles) == 0 {
-		return 0, false, err
-	}
-	return roles[0], true, nil
+	return access.ProjectRole(c.Request.Context(), handler.db, c.Param("slug"), c.Param("project"), user.ID)
 }
 
 // projectIntakeIfEnabled reports the project's intake and whether the list should read it at all.

apps/api/internal/externalapi/issue_activity_handler.go

@@ -5,6 +5,7 @@ import (
 	"time"
 
 	"github.com/gin-gonic/gin"
+	"github.com/yldm-tech/pace/apps/api/internal/access"
 	"github.com/yldm-tech/pace/apps/api/internal/auth"
 	"github.com/yldm-tech/pace/apps/api/internal/drf"
 	"gorm.io/gorm"
@@ -117,7 +118,7 @@ func (handler *Handler) issueActivityScope(c *gin.Context, user *auth.User) *gor
 	return handler.db.WithContext(c.Request.Context()).Table("issue_activities a").Select("a.*").
 		Joins("JOIN workspaces w ON w.id = a.workspace_id").
 		Joins("JOIN projects p ON p.id = a.project_id AND p.archived_at IS NULL").
-		Joins("JOIN project_members pm ON pm.project_id = a.project_id AND pm.member_id = ? AND pm.is_active = TRUE", user.ID).
+		Joins(access.MemberJoin("a", "project_id"), user.ID).
 		Where("w.slug = ? AND a.project_id = ? AND a.issue_id = ? AND a.deleted_at IS NULL",
 			c.Param("slug"), c.Param("project"), c.Param("issue")).
 		// The exclusion is a NOT IN, so an entry whose field is null survives it: a null is not in any list.

apps/api/internal/externalapi/issue_comment_handler.go

@@ -6,6 +6,7 @@ import (
 	"time"
 
 	"github.com/gin-gonic/gin"
+	"github.com/yldm-tech/pace/apps/api/internal/access"
 	"github.com/yldm-tech/pace/apps/api/internal/auth"
 	"github.com/yldm-tech/pace/apps/api/internal/drf"
 	"github.com/yldm-tech/pace/apps/api/internal/htmlsanitizer"
@@ -346,7 +347,7 @@ func (handler *Handler) issueCommentScope(c *gin.Context, user *auth.User) *gorm
 			c.Param("project"), user.ID, c.Param("slug")).
 		Joins("JOIN workspaces w ON w.id = cm.workspace_id").
 		Joins("JOIN projects p ON p.id = cm.project_id AND p.archived_at IS NULL").
-		Joins("JOIN project_members pm ON pm.project_id = cm.project_id AND pm.member_id = ? AND pm.is_active = TRUE", user.ID).
+		Joins(access.MemberJoin("cm", "project_id"), user.ID).
 		Where("w.slug = ? AND cm.project_id = ? AND cm.issue_id = ? AND cm.deleted_at IS NULL",
 			c.Param("slug"), c.Param("project"), c.Param("issue"))
 }

apps/api/internal/externalapi/issue_link_handler.go

@@ -8,6 +8,7 @@ import (
 	"time"
 
 	"github.com/gin-gonic/gin"
+	"github.com/yldm-tech/pace/apps/api/internal/access"
 	"github.com/yldm-tech/pace/apps/api/internal/auth"
 	"github.com/yldm-tech/pace/apps/api/internal/drf"
 	"gorm.io/gorm"
@@ -261,7 +262,7 @@ func (handler *Handler) issueLinkScope(c *gin.Context, user *auth.User) *gorm.DB
 	return handler.db.WithContext(c.Request.Context()).Table("issue_links l").Select("l.*").
 		Joins("JOIN workspaces w ON w.id = l.workspace_id").
 		Joins("JOIN projects p ON p.id = l.project_id AND p.archived_at IS NULL").
-		Joins("JOIN project_members pm ON pm.project_id = l.project_id AND pm.member_id = ? AND pm.is_active = TRUE", user.ID).
+		Joins(access.MemberJoin("l", "project_id"), user.ID).
 		Where("w.slug = ? AND l.project_id = ? AND l.issue_id = ? AND l.deleted_at IS NULL",
 			c.Param("slug"), c.Param("project"), c.Param("issue"))
 }

apps/api/internal/externalapi/issue_search_handler.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/gin-gonic/gin"
 	"github.com/lib/pq"
+	"github.com/yldm-tech/pace/apps/api/internal/access"
 	"github.com/yldm-tech/pace/apps/api/internal/auth"
 	"github.com/yldm-tech/pace/apps/api/internal/drf"
 )
@@ -56,7 +57,7 @@ func (handler *Handler) externalIssueSearch(c *gin.Context, user *auth.User, _ A
 	query := handler.db.WithContext(c.Request.Context()).Table("issues i").
 		Joins("JOIN workspaces w ON w.id = i.workspace_id").
 		Joins("JOIN projects p ON p.id = i.project_id AND p.archived_at IS NULL").
-		Joins("JOIN project_members pm ON pm.project_id = i.project_id AND pm.member_id = ? AND pm.is_active = TRUE", user.ID).
+		Joins(access.MemberJoin("i", "project_id"), user.ID).
 		Joins("LEFT JOIN states s ON s.id = i.state_id").
 		Where("w.slug = ?", c.Param("slug")).
 		Where(`i.deleted_at IS NULL AND i.archived_at IS NULL AND i.is_draft = FALSE

apps/api/internal/externalapi/label_handler.go

@@ -6,6 +6,7 @@ import (
 	"time"
 
 	"github.com/gin-gonic/gin"
+	"github.com/yldm-tech/pace/apps/api/internal/access"
 	"github.com/yldm-tech/pace/apps/api/internal/auth"
 	"github.com/yldm-tech/pace/apps/api/internal/drf"
 	"gorm.io/gorm"
@@ -236,7 +237,7 @@ func (handler *Handler) labelScope(c *gin.Context, user *auth.User) *gorm.DB {
 	return handler.db.WithContext(c.Request.Context()).Table("labels l").Select("l.*").
 		Joins("JOIN workspaces w ON w.id = l.workspace_id").
 		Joins("JOIN projects p ON p.id = l.project_id AND p.archived_at IS NULL").
-		Joins("JOIN project_members pm ON pm.project_id = l.project_id AND pm.member_id = ? AND pm.is_active = TRUE", user.ID).
+		Joins(access.MemberJoin("l", "project_id"), user.ID).
 		Where("w.slug = ? AND l.project_id = ? AND l.deleted_at IS NULL", c.Param("slug"), c.Param("project"))
 }
 

apps/api/internal/externalapi/member_handler.go

@@ -4,6 +4,7 @@ import (
 	"net/http"
 
 	"github.com/gin-gonic/gin"
+	"github.com/yldm-tech/pace/apps/api/internal/access"
 	"github.com/yldm-tech/pace/apps/api/internal/auth"
 	"github.com/yldm-tech/pace/apps/api/internal/drf"
 	"gorm.io/gorm"
@@ -353,17 +354,12 @@ func validMemberRole(role int) bool {
 
 // requireProjectAdmin is ProjectAdminPermission, which the three write routes swap in for the read one.
 func (handler *Handler) requireProjectAdmin(c *gin.Context, user *auth.User) bool {
-	var admins int64
-	err := handler.db.WithContext(c.Request.Context()).Table("project_members pm").
-		Joins("JOIN workspaces w ON w.id = pm.workspace_id").
-		Where("w.slug = ? AND pm.project_id = ? AND pm.member_id = ? AND pm.role = ? AND pm.is_active = TRUE",
-			c.Param("slug"), c.Param("project"), user.ID, roleAdmin).
-		Count(&admins).Error
+	role, member, err := access.ProjectRole(c.Request.Context(), handler.db, c.Param("slug"), c.Param("project"), user.ID)
 	if err != nil {
 		handler.serverError(c, err)
 		return false
 	}
-	if admins == 0 {
+	if !member || role != roleAdmin {
 		c.AbortWithStatusJSON(http.StatusForbidden, gin.H{
 			"detail": "You do not have permission to perform this action.",
 		})