From 24bd087d2a227cc37636446996b7149f1a9d36e3 Mon Sep 17 00:00:00 2001 From: Evan Hu Date: Fri, 18 Sep 2026 23:15:05 +0900 Subject: [PATCH 1/7] test(api): pin the membership join's predicate split before refactoring it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The membership join that gates every project-scoped queryset is held in string literals that nothing reads: go build says nothing about a string, and the queries themselves only run under a database the suite skips unless one of the seven *_TEST_DATABASE_URL gates is set. That makes any rewrite of those literals unreviewable, so these tests render the statements first and read the fragment back out of them. They render rather than reproduce. A gorm session opened with DryRun and DisableAutomaticPing builds a statement without dialling the DSN, so each scope builder can be asked what SQL it would send, with the driver's numbered bind markers put back as question marks. Twelve of the call sites are reachable that way — six scope builders in the session API and six in the external API; the rest are assembled inside a handler body that also executes, and refactoring a handler to make it testable is a larger change than the one these tests exist to guard. What they pin is a split that reads like drift and is not. `pm.is_active = TRUE` is the membership test and is written everywhere. `pm.deleted_at IS NULL` is the membership row's own soft delete, which member removal never sets — only the worker cascade does, when the parent project or workspace is soft-deleted — and it is written only where the queryset reads project_members through its own manager. A join traversed through another model's queryset does not apply the joined model's manager, which is the rule already stated at issue_ordering.go:28 and already asserted for the analytics scope by advance_analytics_test.go. So each of these twelve leaves it out on purpose, and the assertion that it stays out is as load-bearing as the assertion that is_active stays in. --- .../externalapi/membership_predicate_test.go | 101 ++++++++++++++ .../project/membership_predicate_test.go | 128 ++++++++++++++++++ 2 files changed, 229 insertions(+) create mode 100644 apps/api/internal/externalapi/membership_predicate_test.go create mode 100644 apps/api/internal/project/membership_predicate_test.go diff --git a/apps/api/internal/externalapi/membership_predicate_test.go b/apps/api/internal/externalapi/membership_predicate_test.go new file mode 100644 index 00000000..c204b486 --- /dev/null +++ b/apps/api/internal/externalapi/membership_predicate_test.go @@ -0,0 +1,101 @@ +package externalapi + +import ( + "net/http" + "net/http/httptest" + "regexp" + "strings" + "testing" + + "github.com/gin-gonic/gin" + "github.com/yldm-tech/pace/apps/api/internal/auth" + "gorm.io/driver/postgres" + "gorm.io/gorm" +) + +// These tests pin the membership join every external-API queryset carries. The join is the whole gate on these routes — the API-key middleware at auth.go checks the key and the user, not the project — and it lives in string literals nothing else reads, so this renders the statement and reads the fragment back out of it. +// +// As in the session API, the membership row's own soft delete is deliberately absent: it is set only by the worker cascade when the parent project or workspace is soft-deleted, and a join traversed through another model's queryset does not apply the joined model's manager. The base row's `deleted_at IS NULL` in the same statement is what excludes a cascaded row, and it is stamped by the same cascade, so the filter left out here would change no row. + +// placeholderNumbers matches the bind markers the postgres driver numbers, which are put back as question marks so a fragment reads the way the call site wrote it. +var placeholderNumbers = regexp.MustCompile(`\$\d+`) + +// dryRunHandler is a Handler whose queries render to SQL instead of reaching a database. DisableAutomaticPing is what keeps gorm.Open from dialling the DSN, which is never connected to. +func dryRunHandler(t *testing.T) *Handler { + t.Helper() + database, err := gorm.Open( + postgres.New(postgres.Config{DSN: "postgres://render@127.0.0.1:1/render"}), + &gorm.Config{DryRun: true, DisableAutomaticPing: true}, + ) + if err != nil { + t.Fatalf("opening a dry-run database: %v", err) + } + return &Handler{db: database} +} + +// renderedSQL is the statement a scope becomes. +func renderedSQL(t *testing.T, scope *gorm.DB) string { + t.Helper() + var rows []map[string]any + statement := scope.Find(&rows).Statement + if statement.SQL.Len() == 0 { + t.Fatal("the scope rendered no SQL") + } + return placeholderNumbers.ReplaceAllString(statement.SQL.String(), "?") +} + +// dryRunContext is a request the scope builders can read their route parameters off. +func dryRunContext(t *testing.T) *gin.Context { + t.Helper() + context, _ := gin.CreateTestContext(httptest.NewRecorder()) + context.Request = httptest.NewRequest(http.MethodGet, "/", nil) + context.Params = gin.Params{ + {Key: "slug", Value: "acme"}, + {Key: "project", Value: "00000000-0000-0000-0000-0000000000p1"}, + {Key: "issue", Value: "00000000-0000-0000-0000-0000000000i1"}, + } + return context +} + +// Every queryset the external API reads through joins the caller's membership of the project in the url, on its own base table's project column. +func TestTheExternalScopesJoinTheCallersMembership(t *testing.T) { + handler := dryRunHandler(t) + context := dryRunContext(t) + user := &auth.User{ID: "00000000-0000-0000-0000-0000000000u1"} + + for _, test := range []struct { + name string + scope *gorm.DB + want string + }{ + {"stateScope", handler.stateScope(context, user), + "JOIN project_members pm ON pm.project_id = s.project_id AND pm.member_id = ? AND pm.is_active = TRUE"}, + {"labelScope", handler.labelScope(context, user), + "JOIN project_members pm ON pm.project_id = l.project_id AND pm.member_id = ? AND pm.is_active = TRUE"}, + {"issueLinkScope", handler.issueLinkScope(context, user), + "JOIN project_members pm ON pm.project_id = l.project_id AND pm.member_id = ? AND pm.is_active = TRUE"}, + {"issueActivityScope", handler.issueActivityScope(context, user), + "JOIN project_members pm ON pm.project_id = a.project_id AND pm.member_id = ? AND pm.is_active = TRUE"}, + {"issueCommentScope", handler.issueCommentScope(context, user), + "JOIN project_members pm ON pm.project_id = cm.project_id AND pm.member_id = ? AND pm.is_active = TRUE"}, + {"cycleScope", handler.cycleScope(context, user), + "JOIN project_members pm ON pm.project_id = c.project_id AND pm.member_id = ? AND pm.is_active = TRUE"}, + } { + t.Run(test.name, func(t *testing.T) { + rendered := renderedSQL(t, test.scope) + if !strings.Contains(rendered, test.want) { + t.Errorf("%s does not carry\n %s\nit renders\n %s", test.name, test.want, rendered) + } + if strings.Contains(rendered, "pm.deleted_at") { + t.Errorf("%s checks the membership's soft delete, and Django's join does not", test.name) + } + if strings.Contains(rendered, "LEFT JOIN project_members") { + t.Errorf("%s joins the membership outer, which lets a non-member's row through", test.name) + } + // Whatever a cascaded membership would have hidden, the base row's own soft delete hides first: the two are stamped by the same cascade. + if !strings.Contains(rendered, "deleted_at IS NULL") { + t.Errorf("%s filters no soft-deleted row at all:\n %s", test.name, rendered) + } + }) + } +} diff --git a/apps/api/internal/project/membership_predicate_test.go b/apps/api/internal/project/membership_predicate_test.go new file mode 100644 index 00000000..54571414 --- /dev/null +++ b/apps/api/internal/project/membership_predicate_test.go @@ -0,0 +1,128 @@ +package project + +import ( + "net/http" + "net/http/httptest" + "regexp" + "strings" + "testing" + + "github.com/gin-gonic/gin" + "github.com/yldm-tech/pace/apps/api/internal/auth" + "gorm.io/driver/postgres" + "gorm.io/gorm" +) + +// These tests pin the membership join every project-scoped queryset carries, one alias at a time, because the join is held in string literals that nothing else reads: go build says nothing about a string, and the queries themselves only run under a database this suite skips by default. +// +// What they pin is a split that looks like drift and is not. `pm.is_active = TRUE` is the membership test, and it is written everywhere. `pm.deleted_at IS NULL` is the *membership row's* soft delete, which is set only when the parent project or workspace is soft-deleted and only by the worker cascade, and it is written at the sites whose queryset reads project_members through its own manager. A join traversed through another model's queryset carries no such filter — the rule stated at issue_ordering.go:28 and asserted for the analytics scope at advance_analytics_test.go:100 — so every scope below leaves it out on purpose. Adding it here would be a sweep that changes no row (the same cascade stamps the base row, which each of these queries already filters) while breaking parity with Django. +// +// The SQL is rendered rather than reproduced: a dry-run session builds the statement without connecting to anything, so the assertion is over what the database would have been sent. + +// placeholderNumbers matches the bind markers the postgres driver numbers, which are put back as question marks so a fragment reads the way the call site wrote it. +var placeholderNumbers = regexp.MustCompile(`\$\d+`) + +// dryRunHandler is a Handler whose queries render to SQL instead of reaching a database. DisableAutomaticPing is what keeps gorm.Open from dialling the DSN, which is never connected to. +func dryRunHandler(t *testing.T) *Handler { + t.Helper() + database, err := gorm.Open( + postgres.New(postgres.Config{DSN: "postgres://render@127.0.0.1:1/render"}), + &gorm.Config{DryRun: true, DisableAutomaticPing: true}, + ) + if err != nil { + t.Fatalf("opening a dry-run database: %v", err) + } + return &Handler{db: database} +} + +// renderedSQL is the statement a scope becomes. +func renderedSQL(t *testing.T, scope *gorm.DB) string { + t.Helper() + var rows []map[string]any + statement := scope.Find(&rows).Statement + if statement.SQL.Len() == 0 { + t.Fatal("the scope rendered no SQL") + } + return placeholderNumbers.ReplaceAllString(statement.SQL.String(), "?") +} + +// dryRunContext is a request the scope builders can read their route parameters off. +func dryRunContext(t *testing.T) *gin.Context { + t.Helper() + context, _ := gin.CreateTestContext(httptest.NewRecorder()) + context.Request = httptest.NewRequest(http.MethodGet, "/", nil) + context.Params = gin.Params{ + {Key: "slug", Value: "acme"}, + {Key: "id", Value: "00000000-0000-0000-0000-0000000000p1"}, + {Key: "module", Value: "00000000-0000-0000-0000-0000000000m1"}, + } + return context +} + +// Every queryset the session API's project routes read through joins the caller's membership, and the alias it joins on is the one its own base table names. +func TestTheProjectScopesJoinTheCallersMembership(t *testing.T) { + handler := dryRunHandler(t) + context := dryRunContext(t) + user := &auth.User{ID: "00000000-0000-0000-0000-0000000000u1"} + const project = "00000000-0000-0000-0000-0000000000p1" + + for _, test := range []struct { + name string + scope *gorm.DB + want string + }{ + {"memberIssueScope", handler.memberIssueScope(context, user, "acme"), + "JOIN project_members pm ON pm.project_id = i.project_id AND pm.member_id = ? AND pm.is_active = TRUE"}, + {"memberIssueScopeAnyProject", handler.memberIssueScopeAnyProject(context, user, "acme"), + "JOIN project_members pm ON pm.project_id = i.project_id AND pm.member_id = ? AND pm.is_active = TRUE"}, + {"projectScopedTable", handler.projectScopedTable(context, user, "cycles", "acme", project), + "JOIN project_members pm ON pm.project_id = t.project_id AND pm.member_id = ? AND pm.is_active = TRUE"}, + {"viewScope", handler.viewScope(context, user, "acme", project), + "JOIN project_members pm ON pm.project_id = v.project_id AND pm.member_id = ? AND pm.is_active = TRUE"}, + // A page reaches its project through the link table, so the membership is joined on the link's project rather than the page's. + {"pageScope", handler.pageScope(context, user, "acme", project), + "JOIN project_members pm ON pm.project_id = pp.project_id AND pm.member_id = ? AND pm.is_active = TRUE"}, + {"moduleLinkScope", handler.moduleLinkScope(context, user), + "JOIN project_members pm ON pm.project_id = l.project_id AND pm.member_id = ? AND pm.is_active = TRUE"}, + } { + t.Run(test.name, func(t *testing.T) { + rendered := renderedSQL(t, test.scope) + if !strings.Contains(rendered, test.want) { + t.Errorf("%s does not carry\n %s\nit renders\n %s", test.name, test.want, rendered) + } + // The membership's own soft delete belongs to project_members' manager, and a join traversed through another queryset does not apply it. + if strings.Contains(rendered, "pm.deleted_at") { + t.Errorf("%s checks the membership's soft delete, and Django's join does not", test.name) + } + }) + } +} + +// The membership join narrows rather than widens: none of these scopes can reach a row through a LEFT JOIN of it, and each keeps the project-liveness filter that is what actually closes a deleted project. +func TestTheProjectScopesJoinTheMembershipInner(t *testing.T) { + handler := dryRunHandler(t) + context := dryRunContext(t) + user := &auth.User{ID: "00000000-0000-0000-0000-0000000000u1"} + const project = "00000000-0000-0000-0000-0000000000p1" + + for _, test := range []struct { + name string + scope *gorm.DB + want string + }{ + {"memberIssueScope", handler.memberIssueScope(context, user, "acme"), "JOIN projects p ON p.id = i.project_id AND p.archived_at IS NULL"}, + {"viewScope", handler.viewScope(context, user, "acme", project), "JOIN projects p ON p.id = v.project_id AND p.archived_at IS NULL"}, + {"pageScope", handler.pageScope(context, user, "acme", project), "JOIN project_pages pp ON pp.page_id = p.id AND pp.deleted_at IS NULL"}, + {"moduleLinkScope", handler.moduleLinkScope(context, user), "JOIN projects p ON p.id = l.project_id AND p.archived_at IS NULL"}, + } { + t.Run(test.name, func(t *testing.T) { + rendered := renderedSQL(t, test.scope) + if strings.Contains(rendered, "LEFT JOIN project_members") { + t.Errorf("%s joins the membership outer, which lets a non-member's row through", test.name) + } + if !strings.Contains(rendered, test.want) { + t.Errorf("%s does not carry\n %s\nit renders\n %s", test.name, test.want, rendered) + } + }) + } +} From 9c4b1327ca4fcaa6fefaee0248df690bd0e2134a Mon Sep 17 00:00:00 2001 From: Evan Hu Date: Fri, 18 Sep 2026 23:18:46 +0900 Subject: [PATCH 2/7] refactor(api): put the project membership join in one place, with its soft-delete rule as an argument MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The join that decides whether a caller may read a project's rows was written out as a string literal at thirty-six call sites across internal/project, internal/externalapi and internal/worker. Thirty-one of them end at `pm.is_active = TRUE` and five go on to `AND pm.deleted_at IS NULL`, and nothing at any of those sites says which group it is in or why — the difference is a few characters inside otherwise identical strings, and reading one site tells you nothing about the rule. That is how the same predicate came to diverge in the workspace-role lookups, and it is why the omission keeps being read as an oversight. access.MemberJoin writes the join now and its package comment is where the two columns are explained: is_active is membership, written synchronously by member removal, workspace member removal and account deactivation; deleted_at on the membership row is the worker cascade's mark that the parent project or workspace was soft-deleted, written after the fact and never by member removal. The five sites whose queryset reads project_members through its own manager pass WithMembershipSoftDelete(), so the exception is an argument a reader can see rather than a substring they have to notice. No SQL changed. Thirty-two of the thirty-six render byte for byte what the literal they replaced held; the four in workspace_aggregate_handler.go were backtick strings wrapped across two lines, so their newline and indentation collapse to the single space that was already there logically — same tokens, same order, same bind argument. The join predicates the twelve renderable scope builders send are asserted character for character by the tests added in the previous commit, and they pass unchanged. The remaining hand-written copies are deliberately untouched: the EXISTS fragments and correlated subqueries are embedded in Select strings, the five in workspace_user_handler.go split the predicate between a Joins and a Where, and the assignee joins in the annotations are not authorization at all. The guard in internal/server keeps the literal from regrowing, because one place for this predicate is only worth having if it stays the only place. --- apps/api/internal/access/membership.go | 35 +++++++++++++ apps/api/internal/access/membership_test.go | 52 +++++++++++++++++++ .../externalapi/cycle_crud_handler.go | 3 +- .../api/internal/externalapi/cycle_handler.go | 3 +- .../externalapi/cycle_issue_handler.go | 3 +- .../externalapi/issue_activity_handler.go | 3 +- .../externalapi/issue_comment_handler.go | 3 +- .../externalapi/issue_link_handler.go | 3 +- .../externalapi/issue_search_handler.go | 3 +- .../api/internal/externalapi/label_handler.go | 3 +- .../externalapi/module_issue_handler.go | 3 +- .../api/internal/externalapi/state_handler.go | 3 +- .../project/archived_cycle_detail_handler.go | 3 +- .../project/archived_cycle_handler.go | 3 +- .../internal/project/cycle_list_handler.go | 5 +- .../internal/project/entity_search_handler.go | 7 +-- apps/api/internal/project/exporter_handler.go | 3 +- .../internal/project/global_search_handler.go | 9 ++-- .../project/issue_activity_handler.go | 5 +- .../internal/project/issue_search_handler.go | 3 +- apps/api/internal/project/module_handler.go | 3 +- .../internal/project/module_list_handler.go | 3 +- apps/api/internal/project/page_handler.go | 5 +- apps/api/internal/project/state_handler.go | 3 +- apps/api/internal/project/view_handler.go | 3 +- .../project/workspace_aggregate_handler.go | 13 ++--- .../internal/server/membership_join_test.go | 19 +++++++ apps/api/internal/worker/export_rows.go | 3 +- 28 files changed, 167 insertions(+), 40 deletions(-) create mode 100644 apps/api/internal/access/membership.go create mode 100644 apps/api/internal/access/membership_test.go create mode 100644 apps/api/internal/server/membership_join_test.go diff --git a/apps/api/internal/access/membership.go b/apps/api/internal/access/membership.go new file mode 100644 index 00000000..5869c5d4 --- /dev/null +++ b/apps/api/internal/access/membership.go @@ -0,0 +1,35 @@ +// 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 + +// 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 .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 } +} diff --git a/apps/api/internal/access/membership_test.go b/apps/api/internal/access/membership_test.go new file mode 100644 index 00000000..3cf4ad66 --- /dev/null +++ b/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) + } +} diff --git a/apps/api/internal/externalapi/cycle_crud_handler.go b/apps/api/internal/externalapi/cycle_crud_handler.go index 4ee5e65c..f14c898d 100644 --- a/apps/api/internal/externalapi/cycle_crud_handler.go +++ b/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")) diff --git a/apps/api/internal/externalapi/cycle_handler.go b/apps/api/internal/externalapi/cycle_handler.go index ca9c5667..c3e70e2e 100644 --- a/apps/api/internal/externalapi/cycle_handler.go +++ b/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"). diff --git a/apps/api/internal/externalapi/cycle_issue_handler.go b/apps/api/internal/externalapi/cycle_issue_handler.go index 60f2b791..6dc65a7b 100644 --- a/apps/api/internal/externalapi/cycle_issue_handler.go +++ b/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 diff --git a/apps/api/internal/externalapi/issue_activity_handler.go b/apps/api/internal/externalapi/issue_activity_handler.go index 9e7b3602..0a69b3d4 100644 --- a/apps/api/internal/externalapi/issue_activity_handler.go +++ b/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. diff --git a/apps/api/internal/externalapi/issue_comment_handler.go b/apps/api/internal/externalapi/issue_comment_handler.go index 1d14a0bf..ad3e85c5 100644 --- a/apps/api/internal/externalapi/issue_comment_handler.go +++ b/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")) } diff --git a/apps/api/internal/externalapi/issue_link_handler.go b/apps/api/internal/externalapi/issue_link_handler.go index 7a790c3a..84e23bc7 100644 --- a/apps/api/internal/externalapi/issue_link_handler.go +++ b/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")) } diff --git a/apps/api/internal/externalapi/issue_search_handler.go b/apps/api/internal/externalapi/issue_search_handler.go index aa18e1aa..02c0128b 100644 --- a/apps/api/internal/externalapi/issue_search_handler.go +++ b/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 diff --git a/apps/api/internal/externalapi/label_handler.go b/apps/api/internal/externalapi/label_handler.go index dec6c7d1..64912316 100644 --- a/apps/api/internal/externalapi/label_handler.go +++ b/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")) } diff --git a/apps/api/internal/externalapi/module_issue_handler.go b/apps/api/internal/externalapi/module_issue_handler.go index c2af23fd..293a42f8 100644 --- a/apps/api/internal/externalapi/module_issue_handler.go +++ b/apps/api/internal/externalapi/module_issue_handler.go @@ -7,6 +7,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" @@ -251,7 +252,7 @@ func (handler *Handler) moduleIssueLinks(c *gin.Context, user *auth.User) ([]mod Select(moduleIssueSelection()). Joins("JOIN workspaces w ON w.id = mi.workspace_id"). Joins("JOIN projects p ON p.id = mi.project_id AND p.archived_at IS NULL"). - Joins("JOIN project_members pm ON pm.project_id = mi.project_id AND pm.member_id = ? AND pm.is_active = TRUE", user.ID). + Joins(access.MemberJoin("mi", "project_id"), user.ID). Where("w.slug = ? AND mi.project_id = ? AND mi.module_id = ? AND mi.deleted_at IS NULL", c.Param("slug"), c.Param("project"), c.Param("module")). Order("mi.created_at DESC").Scan(&rows).Error diff --git a/apps/api/internal/externalapi/state_handler.go b/apps/api/internal/externalapi/state_handler.go index 6ad0ce38..ad813387 100644 --- a/apps/api/internal/externalapi/state_handler.go +++ b/apps/api/internal/externalapi/state_handler.go @@ -7,6 +7,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/pagination" @@ -325,7 +326,7 @@ func (handler *Handler) stateScope(c *gin.Context, user *auth.User) *gorm.DB { return handler.db.WithContext(c.Request.Context()).Table("states s").Select("s.*"). Joins("JOIN workspaces w ON w.id = s.workspace_id"). Joins("JOIN projects p ON p.id = s.project_id AND p.archived_at IS NULL"). - Joins("JOIN project_members pm ON pm.project_id = s.project_id AND pm.member_id = ? AND pm.is_active = TRUE", user.ID). + Joins(access.MemberJoin("s", "project_id"), user.ID). Where("w.slug = ? AND s.project_id = ? AND s.is_triage = FALSE AND s.deleted_at IS NULL", c.Param("slug"), c.Param("project")) } diff --git a/apps/api/internal/project/archived_cycle_detail_handler.go b/apps/api/internal/project/archived_cycle_detail_handler.go index c2d833e0..28131a23 100644 --- a/apps/api/internal/project/archived_cycle_detail_handler.go +++ b/apps/api/internal/project/archived_cycle_detail_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" ) @@ -105,7 +106,7 @@ func (handler *Handler) archivedCycleRowByID(c *gin.Context, slug, projectID, us Select(selection, userID, projectID, slug, now, now, now, now, projectID). Joins("JOIN workspaces w ON w.id = c.workspace_id"). Joins("JOIN projects p ON p.id = c.project_id AND p.archived_at IS NULL"). - Joins("JOIN project_members pm ON pm.project_id = c.project_id AND pm.member_id = ? AND pm.is_active = TRUE", userID). + Joins(access.MemberJoin("c", "project_id"), userID). Where("w.slug = ? AND c.project_id = ? AND c.id = ? AND c.deleted_at IS NULL AND c.archived_at IS NOT NULL", slug, projectID, cycleID). Group("c.id").Limit(1).Scan(&rows).Error diff --git a/apps/api/internal/project/archived_cycle_handler.go b/apps/api/internal/project/archived_cycle_handler.go index e1b682e1..40cf6b51 100644 --- a/apps/api/internal/project/archived_cycle_handler.go +++ b/apps/api/internal/project/archived_cycle_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" ) @@ -27,7 +28,7 @@ func (handler *Handler) archivedCycleList(c *gin.Context, user *auth.User) { Select(archivedCycleAnnotations(), user.ID, projectID, slug, now, now, now, now). Joins("JOIN workspaces w ON w.id = c.workspace_id"). Joins("JOIN projects p ON p.id = c.project_id AND p.archived_at IS NULL"). - 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). Where("w.slug = ? AND c.project_id = ? AND c.deleted_at IS NULL AND c.archived_at IS NOT NULL", slug, projectID). Group("c.id").Order("is_favorite DESC, c.created_at DESC").Scan(&rows).Error if err != nil { diff --git a/apps/api/internal/project/cycle_list_handler.go b/apps/api/internal/project/cycle_list_handler.go index dc4f9719..bafe600e 100644 --- a/apps/api/internal/project/cycle_list_handler.go +++ b/apps/api/internal/project/cycle_list_handler.go @@ -7,6 +7,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" "gorm.io/gorm" @@ -89,7 +90,7 @@ func (handler *Handler) cycleRows(c *gin.Context, slug, projectID, userID string Select(cycleAnnotations(), userID, projectID, slug, now, now, now, now). Joins("JOIN workspaces w ON w.id = c.workspace_id"). Joins("JOIN projects p ON p.id = c.project_id AND p.archived_at IS NULL"). - Joins("JOIN project_members pm ON pm.project_id = c.project_id AND pm.member_id = ? AND pm.is_active = TRUE", userID). + Joins(access.MemberJoin("c", "project_id"), userID). Where("w.slug = ? AND c.project_id = ? AND c.deleted_at IS NULL AND c.archived_at IS NULL", slug, projectID) if currentOnly { query = query.Where("c.start_date <= ? AND c.end_date >= ?", now, now) @@ -116,7 +117,7 @@ func (handler *Handler) cycleRowsByID(c *gin.Context, slug, projectID, userID, c Select(selection, arguments...). Joins("JOIN workspaces w ON w.id = c.workspace_id"). Joins("JOIN projects p ON p.id = c.project_id AND p.archived_at IS NULL"). - Joins("JOIN project_members pm ON pm.project_id = c.project_id AND pm.member_id = ? AND pm.is_active = TRUE", userID). + Joins(access.MemberJoin("c", "project_id"), userID). Where("w.slug = ? AND c.project_id = ? AND c.id = ? AND c.deleted_at IS NULL", slug, projectID, cycleID). Group("c.id").Limit(1).Scan(&rows).Error return rows, err diff --git a/apps/api/internal/project/entity_search_handler.go b/apps/api/internal/project/entity_search_handler.go index 67163321..bff04dc2 100644 --- a/apps/api/internal/project/entity_search_handler.go +++ b/apps/api/internal/project/entity_search_handler.go @@ -7,6 +7,7 @@ import ( "strings" "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" @@ -200,7 +201,7 @@ func (handler *Handler) memberIssueScopeAnyProject(c *gin.Context, user *auth.Us return 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"). - 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). Where("w.slug = ?", slug) } @@ -276,7 +277,7 @@ func (handler *Handler) projectScopedTable(c *gin.Context, user *auth.User, tabl database := handler.db.WithContext(c.Request.Context()).Table(table+" t"). Joins("JOIN workspaces w ON w.id = t.workspace_id"). Joins("JOIN projects p ON p.id = t.project_id"). - Joins("JOIN project_members pm ON pm.project_id = t.project_id AND pm.member_id = ? AND pm.is_active = TRUE", user.ID). + Joins(access.MemberJoin("t", "project_id"), user.ID). Where("w.slug = ? AND t.deleted_at IS NULL", slug) if projectID != "" { database = database.Where("t.project_id = ?", projectID) @@ -290,7 +291,7 @@ func (handler *Handler) searchMentionablePages(c *gin.Context, user *auth.User, Joins("JOIN workspaces w ON w.id = pg.workspace_id"). Joins("JOIN project_pages pp ON pp.page_id = pg.id"). Joins("JOIN projects p ON p.id = pp.project_id"). - Joins("JOIN project_members pm ON pm.project_id = pp.project_id AND pm.member_id = ? AND pm.is_active = TRUE", user.ID). + Joins(access.MemberJoin("pp", "project_id"), user.ID). Where("w.slug = ? AND pg.deleted_at IS NULL AND pg.access = ?", slug, pagePublicAccess) if projectID != "" { database = database.Where("pp.project_id = ?", projectID) diff --git a/apps/api/internal/project/exporter_handler.go b/apps/api/internal/project/exporter_handler.go index e8d88f8e..c06a54de 100644 --- a/apps/api/internal/project/exporter_handler.go +++ b/apps/api/internal/project/exporter_handler.go @@ -10,6 +10,7 @@ import ( "github.com/gin-gonic/gin" "github.com/google/uuid" "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" "github.com/yldm-tech/pace/apps/api/internal/pagination" @@ -90,7 +91,7 @@ func (handler *Handler) exporterCreate(c *gin.Context, user *auth.User) { if len(projectIDs) == 0 { // Nobody named a project, so it is every project in the workspace this person is still a member of and which has not been archived. err = handler.db.WithContext(c.Request.Context()).Table("projects p"). - Joins("JOIN project_members pm ON pm.project_id = p.id AND pm.member_id = ? AND pm.is_active = TRUE AND pm.deleted_at IS NULL", user.ID). + Joins(access.MemberJoin("p", "id", access.WithMembershipSoftDelete()), user.ID). Where("p.workspace_id = ? AND p.archived_at IS NULL AND p.deleted_at IS NULL", workspaceID). Distinct().Pluck("p.id", &projectIDs).Error if err != nil { diff --git a/apps/api/internal/project/global_search_handler.go b/apps/api/internal/project/global_search_handler.go index 5036e93e..85d87782 100644 --- a/apps/api/internal/project/global_search_handler.go +++ b/apps/api/internal/project/global_search_handler.go @@ -7,6 +7,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" "gorm.io/gorm" @@ -135,7 +136,7 @@ func (handler *Handler) searchWorkspaces(c *gin.Context, user *auth.User, search func (handler *Handler) searchProjects(c *gin.Context, user *auth.User, search, slug string) ([]gin.H, error) { query := handler.db.WithContext(c.Request.Context()).Table("projects p"). Joins("JOIN workspaces w ON w.id = p.workspace_id"). - Joins("JOIN project_members pm ON pm.project_id = p.id AND pm.member_id = ? AND pm.is_active = TRUE", user.ID). + Joins(access.MemberJoin("p", "id"), user.ID). Where("w.slug = ? AND p.archived_at IS NULL AND p.deleted_at IS NULL", slug) if search != "" { query = query.Where("p.name ILIKE ? OR p.identifier ILIKE ?", "%"+search+"%", "%"+search+"%") @@ -190,7 +191,7 @@ func (handler *Handler) memberIssueScope(c *gin.Context, user *auth.User, slug s return 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). Where("w.slug = ?", slug) } @@ -225,7 +226,7 @@ func (handler *Handler) searchNamed(c *gin.Context, user *auth.User, search, slu query := handler.db.WithContext(c.Request.Context()).Table(table+" t"). Joins("JOIN workspaces w ON w.id = t.workspace_id"). Joins("JOIN projects p ON p.id = t.project_id AND p.archived_at IS NULL"). - Joins("JOIN project_members pm ON pm.project_id = t.project_id AND pm.member_id = ? AND pm.is_active = TRUE", user.ID). + Joins(access.MemberJoin("t", "project_id"), user.ID). Where("w.slug = ? AND t.deleted_at IS NULL", slug) if search != "" { query = query.Where("t.name ILIKE ?", "%"+search+"%") @@ -261,7 +262,7 @@ func (handler *Handler) searchPages(c *gin.Context, user *auth.User, search, slu Joins("JOIN workspaces w ON w.id = pg.workspace_id"). Joins("JOIN project_pages pp ON pp.page_id = pg.id"). Joins("JOIN projects p ON p.id = pp.project_id AND p.archived_at IS NULL"). - Joins("JOIN project_members pm ON pm.project_id = pp.project_id AND pm.member_id = ? AND pm.is_active = TRUE", user.ID). + Joins(access.MemberJoin("pp", "project_id"), user.ID). Where("w.slug = ? AND pg.deleted_at IS NULL", slug) if search != "" { query = query.Where("pg.name ILIKE ?", "%"+search+"%") diff --git a/apps/api/internal/project/issue_activity_handler.go b/apps/api/internal/project/issue_activity_handler.go index e1346a8b..aedddcd6 100644 --- a/apps/api/internal/project/issue_activity_handler.go +++ b/apps/api/internal/project/issue_activity_handler.go @@ -8,6 +8,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" "gorm.io/gorm" @@ -93,7 +94,7 @@ func (handler *Handler) issueActivityRows(ctx context.Context, slug, projectID, query := handler.db.WithContext(ctx).Table("issue_activities ia"). Joins("JOIN workspaces w ON w.id = ia.workspace_id"). Joins("JOIN projects p ON p.id = ia.project_id"). - Joins("JOIN project_members pm ON pm.project_id = ia.project_id AND pm.member_id = ? AND pm.is_active = TRUE", userID). + Joins(access.MemberJoin("ia", "project_id"), userID). Where(`ia.issue_id = ? AND ia.deleted_at IS NULL AND w.slug = ? AND p.archived_at IS NULL AND (ia.field IS NULL OR ia.field NOT IN ?)`, issueID, slug, hiddenActivityFields) if since != nil { @@ -270,7 +271,7 @@ func (handler *Handler) issueActivityComments(ctx context.Context, slug, project query := handler.db.WithContext(ctx).Table("issue_comments ic"). Joins("JOIN workspaces w ON w.id = ic.workspace_id"). Joins("JOIN projects p ON p.id = ic.project_id"). - Joins("JOIN project_members pm ON pm.project_id = ic.project_id AND pm.member_id = ? AND pm.is_active = TRUE", userID). + Joins(access.MemberJoin("ic", "project_id"), userID). Where("ic.issue_id = ? AND ic.deleted_at IS NULL AND w.slug = ? AND p.archived_at IS NULL", issueID, slug) if since != nil { query = query.Where("ic.created_at > ?", *since) diff --git a/apps/api/internal/project/issue_search_handler.go b/apps/api/internal/project/issue_search_handler.go index 179d49a8..d6856bf0 100644 --- a/apps/api/internal/project/issue_search_handler.go +++ b/apps/api/internal/project/issue_search_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" @@ -24,7 +25,7 @@ func (handler *Handler) issueSearch(c *gin.Context, user *auth.User) { 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). Where("w.slug = ?", slug). Where(issueObjectsPredicate("i")) diff --git a/apps/api/internal/project/module_handler.go b/apps/api/internal/project/module_handler.go index a22f914d..6e8fdfbc 100644 --- a/apps/api/internal/project/module_handler.go +++ b/apps/api/internal/project/module_handler.go @@ -6,6 +6,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" "github.com/yldm-tech/pace/apps/api/internal/validate" @@ -330,7 +331,7 @@ func (handler *Handler) moduleLinkScope(c *gin.Context, user *auth.User) *gorm.D return handler.db.WithContext(c.Request.Context()).Table("module_links 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.module_id = ? AND l.deleted_at IS NULL", c.Param("slug"), c.Param("id"), c.Param("module")) } diff --git a/apps/api/internal/project/module_list_handler.go b/apps/api/internal/project/module_list_handler.go index 5a4d0f48..4e92d5c5 100644 --- a/apps/api/internal/project/module_list_handler.go +++ b/apps/api/internal/project/module_list_handler.go @@ -6,6 +6,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" ) @@ -65,7 +66,7 @@ func (handler *Handler) moduleRows(c *gin.Context, userID, moduleID string) ([]m Select(moduleAnnotations(), userID, c.Param("id"), c.Param("slug")). Joins("JOIN workspaces w ON w.id = m.workspace_id"). Joins("JOIN projects p ON p.id = m.project_id AND p.archived_at IS NULL"). - Joins("JOIN project_members pm ON pm.project_id = m.project_id AND pm.member_id = ? AND pm.is_active = TRUE", userID). + Joins(access.MemberJoin("m", "project_id"), userID). Where("w.slug = ? AND m.project_id = ? AND m.deleted_at IS NULL AND m.archived_at IS NULL", c.Param("slug"), c.Param("id")) if moduleID != "" { diff --git a/apps/api/internal/project/page_handler.go b/apps/api/internal/project/page_handler.go index cb14db8a..6b56289c 100644 --- a/apps/api/internal/project/page_handler.go +++ b/apps/api/internal/project/page_handler.go @@ -8,6 +8,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" "gorm.io/gorm" @@ -150,7 +151,7 @@ func (handler *Handler) pageScope(c *gin.Context, user *auth.User, slug, project Joins("JOIN workspaces w ON w.id = p.workspace_id"). Joins("JOIN project_pages pp ON pp.page_id = p.id AND pp.deleted_at IS NULL"). Joins("JOIN projects pr ON pr.id = pp.project_id AND pr.archived_at IS NULL"). - Joins("JOIN project_members pm ON pm.project_id = pp.project_id AND pm.member_id = ? AND pm.is_active = TRUE", user.ID). + Joins(access.MemberJoin("pp", "project_id"), user.ID). Where("w.slug = ? AND p.deleted_at IS NULL AND p.parent_id IS NULL", slug). Where("p.owned_by_id = ? OR p.access = ?", user.ID, pagePublicAccess). // The project filter is an annotation that is then filtered on, so the page has to be linked to *this* project while still being reachable through any of them. @@ -587,7 +588,7 @@ func (handler *Handler) pageSummary(c *gin.Context, user *auth.User) { Joins("JOIN workspaces w ON w.id = p.workspace_id"). Joins("JOIN project_pages pp ON pp.page_id = p.id"). Joins("JOIN projects pr ON pr.id = pp.project_id AND pr.archived_at IS NULL"). - Joins("JOIN project_members pm ON pm.project_id = pp.project_id AND pm.member_id = ? AND pm.is_active = TRUE", user.ID). + Joins(access.MemberJoin("pp", "project_id"), user.ID). Where("w.slug = ? AND p.deleted_at IS NULL AND p.parent_id IS NULL", slug). Where("p.owned_by_id = ? OR p.access = ?", user.ID, pagePublicAccess). Where(`EXISTS (SELECT 1 FROM project_pages fp WHERE fp.page_id = p.id AND fp.project_id = ?)`, projectID) diff --git a/apps/api/internal/project/state_handler.go b/apps/api/internal/project/state_handler.go index d21deed0..9ea8c744 100644 --- a/apps/api/internal/project/state_handler.go +++ b/apps/api/internal/project/state_handler.go @@ -7,6 +7,7 @@ import ( "sort" "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" @@ -92,7 +93,7 @@ func (handler *Handler) projectStates(c *gin.Context, user *auth.User) ([]State, err := handler.db.WithContext(c.Request.Context()).Table("states s").Select("s.*"). Joins("JOIN workspaces w ON w.id = s.workspace_id"). Joins("JOIN projects p ON p.id = s.project_id AND p.archived_at IS NULL"). - Joins("JOIN project_members pm ON pm.project_id = s.project_id AND pm.member_id = ? AND pm.is_active = TRUE", user.ID). + Joins(access.MemberJoin("s", "project_id"), user.ID). Where("w.slug = ? AND s.project_id = ? AND s.is_triage = FALSE AND s.deleted_at IS NULL", c.Param("slug"), c.Param("id")). Order("s.sequence").Scan(&states).Error diff --git a/apps/api/internal/project/view_handler.go b/apps/api/internal/project/view_handler.go index 59f80f27..5303d275 100644 --- a/apps/api/internal/project/view_handler.go +++ b/apps/api/internal/project/view_handler.go @@ -9,6 +9,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" @@ -113,7 +114,7 @@ func (handler *Handler) viewScope(c *gin.Context, user *auth.User, slug, project AND uf.project_id = ? AND uw.slug = ? AND uf.deleted_at IS NULL) AS is_favorite`, user.ID, projectID, slug). Joins("JOIN workspaces w ON w.id = v.workspace_id"). Joins("JOIN projects p ON p.id = v.project_id AND p.archived_at IS NULL"). - Joins("JOIN project_members pm ON pm.project_id = v.project_id AND pm.member_id = ? AND pm.is_active = TRUE", user.ID). + Joins(access.MemberJoin("v", "project_id"), user.ID). Where("w.slug = ? AND v.project_id = ? AND v.deleted_at IS NULL", slug, projectID). // Access 1 is public and 0 is private, and a private view is visible to the person who made it. Where("v.owned_by_id = ? OR v.access = 1", user.ID) diff --git a/apps/api/internal/project/workspace_aggregate_handler.go b/apps/api/internal/project/workspace_aggregate_handler.go index a13fdb95..65a74cc7 100644 --- a/apps/api/internal/project/workspace_aggregate_handler.go +++ b/apps/api/internal/project/workspace_aggregate_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" ) @@ -26,8 +27,7 @@ func (handler *Handler) workspaceLabels(c *gin.Context, user *auth.User) { err := 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 AND pm.deleted_at IS NULL`, user.ID). + Joins(access.MemberJoin("l", "project_id", access.WithMembershipSoftDelete()), user.ID). Where("w.slug = ? AND l.deleted_at IS NULL", c.Param("slug")). Order("l.created_at").Scan(&labels).Error if err != nil { @@ -52,8 +52,7 @@ func (handler *Handler) workspaceStates(c *gin.Context, user *auth.User) { err := handler.db.WithContext(c.Request.Context()).Table("states s").Select("s.*"). Joins("JOIN workspaces w ON w.id = s.workspace_id"). Joins("JOIN projects p ON p.id = s.project_id AND p.archived_at IS NULL"). - Joins(`JOIN project_members pm ON pm.project_id = s.project_id - AND pm.member_id = ? AND pm.is_active = TRUE AND pm.deleted_at IS NULL`, user.ID). + Joins(access.MemberJoin("s", "project_id", access.WithMembershipSoftDelete()), user.ID). Where("w.slug = ? AND s.is_triage = FALSE AND s.deleted_at IS NULL", c.Param("slug")). Order("s.sequence").Scan(&states).Error if err != nil { @@ -93,8 +92,7 @@ func (handler *Handler) workspaceCycles(c *gin.Context, user *auth.User) { Select(cycleAnnotations(), user.ID, nil, c.Param("slug"), now, now, now, now). Joins("JOIN workspaces w ON w.id = c.workspace_id"). Joins("JOIN projects p ON p.id = c.project_id AND p.archived_at IS NULL"). - Joins(`JOIN project_members pm ON pm.project_id = c.project_id - AND pm.member_id = ? AND pm.is_active = TRUE AND pm.deleted_at IS NULL`, user.ID). + Joins(access.MemberJoin("c", "project_id", access.WithMembershipSoftDelete()), user.ID). Where("w.slug = ? AND c.archived_at IS NULL AND c.deleted_at IS NULL", c.Param("slug")). Group("c.id").Order("c.created_at DESC").Scan(&rows).Error if err != nil { @@ -128,8 +126,7 @@ func (handler *Handler) workspaceModules(c *gin.Context, user *auth.User) { Select(moduleAnnotations(), user.ID, nil, c.Param("slug")). Joins("JOIN workspaces w ON w.id = m.workspace_id"). Joins("JOIN projects p ON p.id = m.project_id AND p.archived_at IS NULL"). - Joins(`JOIN project_members pm ON pm.project_id = m.project_id - AND pm.member_id = ? AND pm.is_active = TRUE AND pm.deleted_at IS NULL`, user.ID). + Joins(access.MemberJoin("m", "project_id", access.WithMembershipSoftDelete()), user.ID). Where("w.slug = ? AND m.archived_at IS NULL AND m.deleted_at IS NULL", c.Param("slug")). Group("m.id").Order("m.created_at DESC").Scan(&rows).Error if err != nil { diff --git a/apps/api/internal/server/membership_join_test.go b/apps/api/internal/server/membership_join_test.go new file mode 100644 index 00000000..c3e82fb5 --- /dev/null +++ b/apps/api/internal/server/membership_join_test.go @@ -0,0 +1,19 @@ +package server + +import ( + "regexp" + "strings" + "testing" +) + +// TestTheMembershipJoinIsNotWrittenOutByHand keeps the join that gates every project-scoped queryset in one place. +// +// It was written out as a literal at thirty-six call sites in three packages, which is how the five that carry the membership's own soft delete came to be indistinguishable from the thirty-one that leave it out on purpose: the difference was a few characters inside otherwise identical strings. access.MemberJoin writes it now, and the exception is an argument you can see. A new query that spells the join out again is not wrong SQL, but it puts the semantics back where nobody can find them, so it fails here instead. +// +// The pattern is the shape that joins the project id first and the member id second, which is the authorization join. The assignee joins in the work-item annotations (pm2.member_id = ia.assignee_id) decide which assignees to render rather than who may read, and they are deliberately not this. +func TestTheMembershipJoinIsNotWrittenOutByHand(t *testing.T) { + handWritten := regexp.MustCompile(`JOIN project_members \w+ ON \w+\.project_id = \w+\.\w+ AND \w+\.member_id = \?`) + if offenders := walk(t, handWritten); len(offenders) > 0 { + t.Errorf("the membership join belongs to access.MemberJoin, which is where its two soft-delete rules are written down:\n %s", strings.Join(offenders, "\n ")) + } +} diff --git a/apps/api/internal/worker/export_rows.go b/apps/api/internal/worker/export_rows.go index be9d3417..d0fa29ac 100644 --- a/apps/api/internal/worker/export_rows.go +++ b/apps/api/internal/worker/export_rows.go @@ -6,6 +6,7 @@ import ( "strings" "time" + "github.com/yldm-tech/pace/apps/api/internal/access" "github.com/yldm-tech/pace/apps/api/internal/drf" "gorm.io/gorm" ) @@ -53,7 +54,7 @@ func loadExportIssues(ctx context.Context, db *gorm.DB, workspaceID string, proj } err := db.WithContext(ctx).Table("issues i"). Joins("JOIN projects p ON p.id = i.project_id"). - Joins("JOIN project_members pm ON pm.project_id = i.project_id AND pm.member_id = ? AND pm.is_active = TRUE", memberID). + Joins(access.MemberJoin("i", "project_id"), memberID). Joins("LEFT JOIN states s ON s.id = i.state_id"). Joins("LEFT JOIN users cu ON cu.id = i.created_by_id"). Joins("LEFT JOIN estimate_points ep ON ep.id = i.estimate_point_id"). From e909b4ce8943a7d2f0cbad41718ce7869ea81cd9 Mon Sep 17 00:00:00 2001 From: Evan Hu Date: Fri, 18 Sep 2026 23:19:56 +0900 Subject: [PATCH 3/7] refactor(api): one workspace-role lookup in package project instead of three MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit handler.workspaceRole, workspace_user_handler.activeWorkspaceRole and member_handler.workspaceMemberRole ran the same query — same table, same join, same predicate `w.slug = ? AND wm.member_id = ? AND wm.is_active = TRUE AND wm.deleted_at IS NULL` — and differed only in how they reported a miss and whether they read the role through Scan or Pluck. Three copies of an authorization predicate in one package is how the fourth copy, in internal/externalapi, came to be missing two filters without anyone noticing, so the next change to this predicate should have one place to happen. workspaceMemberRole is now the only implementation. activeWorkspaceRole is gone and its single caller reads workspaceMemberRole directly, which is an exact swap: both returned the role with a found flag, both returned (0, false, nil) on a miss and both surfaced the error as (0, false, err). workspaceRole keeps its signature and its five callers, as a two-line wrapper — it reported a miss as role 0, and role 0 is not one of the three roles, so discarding the found flag reproduces it including the error path. The only difference between the bodies that is not visible in the result is Pluck into []int against Scan into *int, and workspace_members.role is NOT NULL (migrate/testdata/schema.tsv:1412), so no row can render the two differently. The predicate itself is unchanged. Whether package project's copy should also filter `w.deleted_at IS NULL`, as internal/workspace's does, is a separate question from removing the duplication, and this commit deliberately does not answer it. --- apps/api/internal/project/handler.go | 15 +++------------ .../internal/project/workspace_user_handler.go | 15 +-------------- 2 files changed, 4 insertions(+), 26 deletions(-) diff --git a/apps/api/internal/project/handler.go b/apps/api/internal/project/handler.go index c287da0f..93bad64c 100644 --- a/apps/api/internal/project/handler.go +++ b/apps/api/internal/project/handler.go @@ -633,19 +633,10 @@ func (handler *Handler) createDefaultStates(tx *gorm.DB, project Project, actorI return projects.CreateDefaultStates(tx, project.ID, project.WorkspaceID, actorID, now, newUUID) } +// workspaceRole is the role lookup for the callers that treat "not a member" and "no matching role" alike: a miss is role 0, which is not one of the three roles, so every allowlist refuses it. The callers that have to tell a missing membership from a role they do not want read workspaceMemberRole, which is the one query behind both. func (handler *Handler) workspaceRole(ctx context.Context, slug, userID string) (int, error) { - var role *int - err := handler.db.WithContext(ctx).Table("workspace_members wm"). - Joins("JOIN workspaces w ON w.id = wm.workspace_id"). - Where("w.slug = ? AND wm.member_id = ? AND wm.is_active = TRUE AND wm.deleted_at IS NULL", slug, userID). - Select("wm.role").Limit(1).Scan(&role).Error - if err != nil { - return 0, err - } - if role == nil { - return 0, nil - } - return *role, nil + role, _, err := handler.workspaceMemberRole(ctx, slug, userID) + return role, err } func (handler *Handler) requireWorkspaceRole(c *gin.Context, user *auth.User, allowed ...int) bool { diff --git a/apps/api/internal/project/workspace_user_handler.go b/apps/api/internal/project/workspace_user_handler.go index ec3b39a7..22f02261 100644 --- a/apps/api/internal/project/workspace_user_handler.go +++ b/apps/api/internal/project/workspace_user_handler.go @@ -38,7 +38,7 @@ func (handler *Handler) registerWorkspaceUserRoutes(router gin.IRouter) { // It guards nothing but the session. What stands in for a permission is the pair of lookups it opens with: the caller has to be an active member of the workspace and so does the person being asked about, and either one missing is a 404 rather than a 403. func (handler *Handler) workspaceUserProfile(c *gin.Context, user *auth.User) { slug, target := c.Param("slug"), c.Param("user") - callerRole, found, err := handler.activeWorkspaceRole(c.Request.Context(), slug, user.ID) + callerRole, found, err := handler.workspaceMemberRole(c.Request.Context(), slug, user.ID) if err != nil { handler.internalError(c, err) return @@ -725,19 +725,6 @@ func (handler *Handler) workspaceProjectMembers(c *gin.Context, user *auth.User) drf.Respond(c, http.StatusOK, grouped) } -// activeWorkspaceRole reads the caller's role, and reports whether they are a member at all. -func (handler *Handler) activeWorkspaceRole(ctx context.Context, slug, userID string) (int, bool, error) { - var roles []int - err := handler.db.WithContext(ctx).Table("workspace_members wm"). - Joins("JOIN workspaces w ON w.id = wm.workspace_id"). - Where("w.slug = ? AND wm.member_id = ? AND wm.is_active = TRUE AND wm.deleted_at IS NULL", slug, userID). - Limit(1).Pluck("wm.role", &roles).Error - if err != nil || len(roles) == 0 { - return 0, false, err - } - return roles[0], true, nil -} - // workspaceMemberProfile is the eight fields the profile reports about a person, which stop short of anything an account needs. func (handler *Handler) workspaceMemberProfile(ctx context.Context, slug, memberID string) (gin.H, bool, error) { var rows []struct { From c35ab2bd3f089d0a9d7a86b5e8b80ca19083c372 Mon Sep 17 00:00:00 2001 From: Evan Hu Date: Fri, 18 Sep 2026 23:22:47 +0900 Subject: [PATCH 4/7] fix(api): the external API's project gate read memberships of soft-deleted workspaces MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit requireProjectBase is ProjectBasePermission, and the workspace-role lookup behind it is the entire gate on the external API's project list, create, update and delete. Its predicate was `w.slug = ? AND wm.member_id = ? AND wm.is_active = TRUE`: alone among the five copies of this lookup in the tree it filtered neither the workspace's own soft delete nor the membership's, so a member of a deleted workspace kept their role there, and every route behind this gate kept answering for them. The exposure is narrow but real. workspaceDelete stamps the workspace's deleted_at and renames its slug to `__` in one transaction, so the caller has to know the mangled slug to get this far — which anyone who saw the workspace before it was deleted can compute, and a client that cached the rename cannot avoid sending. Nothing downstream compensates: the project lookups behind this gate are keyed on the slug and the project id and do not filter the workspace either. The fix is internal/workspace's predicate verbatim, which is the strictest copy: `w.slug = ? AND w.deleted_at IS NULL AND wm.member_id = ? AND wm.is_active = TRUE AND wm.deleted_at IS NULL`. The workspace filter is the load-bearing half — deleted_at on the workspace row is written synchronously, inside the delete's transaction, so the window closes at once rather than when the worker drains the cascade that eventually stamps the membership. It can only narrow: a live workspace has a null deleted_at, and a member of one has a null deleted_at on their membership. The test that comes with it runs the gate against a session that records statements instead of sending them, so the predicate can be asserted without a database — this package has no database-backed test at all, and its gates cover sixty-odd routes. It asserts both halves: which filters the statement carries, and that the gate answers 403 rather than admitting the caller when the lookup matches nothing. Two sibling gates in this package have the same omission and are deliberately left alone here: requireWorkspaceAdmin in project_handler.go and requireWorkspaceUser in sticky_handler.go both filter `wm.is_active = TRUE` without either deleted_at. They guard different routes and belong in their own change rather than inside this one. --- .../internal/externalapi/permissions_test.go | 108 ++++++++++++++++++ .../externalapi/project_detail_handler.go | 5 +- 2 files changed, 112 insertions(+), 1 deletion(-) create mode 100644 apps/api/internal/externalapi/permissions_test.go diff --git a/apps/api/internal/externalapi/permissions_test.go b/apps/api/internal/externalapi/permissions_test.go new file mode 100644 index 00000000..3109b3c2 --- /dev/null +++ b/apps/api/internal/externalapi/permissions_test.go @@ -0,0 +1,108 @@ +package externalapi + +import ( + "context" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/gin-gonic/gin" + "github.com/yldm-tech/pace/apps/api/internal/auth" + "gorm.io/driver/postgres" + "gorm.io/gorm" + "gorm.io/gorm/logger" +) + +// The external API's permission gates are the whole of its authorization: the API-key middleware in auth.go authenticates the key and the user and says nothing about the workspace or the project, so whatever the gate's query does not filter, the route does not filter either. There is no database-backed test in this package to check them against, and these gates are not pure functions, so what these tests do is run the gate against a session that records the statement instead of sending it, and read the predicate back out of the record. +// +// That gives two assertions per gate, and both matter. The recorded SQL says which rows the gate would have accepted. The gate's own answer, against a session where no query matches anything, says which way it fails when it finds nothing — and the answer has to be no. + +// recordedStatements is a gorm logger that keeps the SQL a session would have sent. Under DryRun nothing is sent, and the trace still carries the statement with its bind values inlined. +type recordedStatements struct { + statements []string +} + +func (recorded *recordedStatements) LogMode(logger.LogLevel) logger.Interface { return recorded } + +func (recorded *recordedStatements) Info(context.Context, string, ...any) {} + +func (recorded *recordedStatements) Warn(context.Context, string, ...any) {} + +func (recorded *recordedStatements) Error(context.Context, string, ...any) {} + +func (recorded *recordedStatements) Trace(_ context.Context, _ time.Time, sql func() (string, int64), _ error) { + statement, _ := sql() + recorded.statements = append(recorded.statements, statement) +} + +// contains reports whether any recorded statement carries the fragment. +func (recorded *recordedStatements) contains(fragment string) bool { + for _, statement := range recorded.statements { + if strings.Contains(statement, fragment) { + return true + } + } + return false +} + +func (recorded *recordedStatements) String() string { + return strings.Join(recorded.statements, "\n ") +} + +// recordingHandler is a Handler whose queries are built and recorded rather than run. DisableAutomaticPing is what keeps gorm.Open from dialling the DSN, which is never connected to. +func recordingHandler(t *testing.T) (*Handler, *recordedStatements) { + t.Helper() + recorded := &recordedStatements{} + database, err := gorm.Open( + postgres.New(postgres.Config{DSN: "postgres://render@127.0.0.1:1/render"}), + &gorm.Config{DryRun: true, DisableAutomaticPing: true, Logger: recorded}, + ) + if err != nil { + t.Fatalf("opening a dry-run database: %v", err) + } + return &Handler{db: database}, recorded +} + +// gateRequest is a request carrying the two route parameters every project gate reads. +func gateRequest(t *testing.T, method string) (*gin.Context, *httptest.ResponseRecorder) { + t.Helper() + recorder := httptest.NewRecorder() + context, _ := gin.CreateTestContext(recorder) + context.Request = httptest.NewRequest(method, "/", nil) + context.Params = gin.Params{ + {Key: "slug", Value: "acme"}, + {Key: "project", Value: "00000000-0000-0000-0000-0000000000p1"}, + } + return context, recorder +} + +// gateUser is the caller every gate below is asked about. +func gateUser() *auth.User { + return &auth.User{ID: "00000000-0000-0000-0000-0000000000u1"} +} + +// ProjectBasePermission reads the caller's workspace role, and a soft-deleted workspace has no members to read: the workspace's own deleted_at is written in the same transaction as the delete, so this is the filter that closes the window rather than narrowing it eventually. +// +// This gate used to filter neither the workspace's soft delete nor the membership's, alone among the five copies of the lookup in the tree, and it is the one that admits the external API's project list, create, update and delete. +func TestTheProjectBaseGateWillNotReadASoftDeletedWorkspace(t *testing.T) { + handler, recorded := recordingHandler(t) + context, recorder := gateRequest(t, http.MethodGet) + + if handler.requireProjectBase(context, gateUser(), http.MethodGet) { + t.Error("the gate admitted a caller whose membership lookup matched nothing") + } + if recorder.Code != http.StatusForbidden { + t.Errorf("the gate answered %d, want %d", recorder.Code, http.StatusForbidden) + } + for _, fragment := range []string{ + "w.deleted_at IS NULL", + "wm.is_active = TRUE", + "wm.deleted_at IS NULL", + } { + if !recorded.contains(fragment) { + t.Errorf("the workspace role lookup does not filter %s:\n %s", fragment, recorded) + } + } +} diff --git a/apps/api/internal/externalapi/project_detail_handler.go b/apps/api/internal/externalapi/project_detail_handler.go index e893712e..473ca74c 100644 --- a/apps/api/internal/externalapi/project_detail_handler.go +++ b/apps/api/internal/externalapi/project_detail_handler.go @@ -324,11 +324,14 @@ func (handler *Handler) requireProjectBase(c *gin.Context, user *auth.User, meth return false } +// workspaceRole reads the caller's role in the workspace, and reports whether they are a member of it at all. +// +// The predicate is internal/workspace's, which is the strictest of the copies in the tree, and this one used to be the loosest: it filtered neither the workspace's own soft delete nor the membership's. A soft-deleted workspace is renamed to `__` in the same transaction that stamps its deleted_at, so reaching one takes knowing the mangled slug — but it was reachable, and this lookup is the whole of ProjectBasePermission, which gates the project list, create, update and delete. The workspace filter is the one that matters: deleted_at on the workspace is written synchronously, while the membership's arrives later with the worker's cascade. func (handler *Handler) workspaceRole(c *gin.Context, user *auth.User, slug string) (int, bool, error) { var roles []int err := handler.db.WithContext(c.Request.Context()).Table("workspace_members wm"). Joins("JOIN workspaces w ON w.id = wm.workspace_id"). - Where("w.slug = ? AND wm.member_id = ? AND wm.is_active = TRUE", slug, user.ID). + Where("w.slug = ? AND w.deleted_at IS NULL AND wm.member_id = ? AND wm.is_active = TRUE AND wm.deleted_at IS NULL", slug, user.ID). Limit(1).Pluck("wm.role", &roles).Error if err != nil || len(roles) == 0 { return 0, false, err From cacf219eca609cab1765262674f35def472cf87c Mon Sep 17 00:00:00 2001 From: Evan Hu Date: Fri, 18 Sep 2026 23:25:54 +0900 Subject: [PATCH 5/7] fix(api): the four project permission gates kept answering for a deleted project MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These four queries are the only ones in the tree whose whole subject is the membership row and which carry no other liveness condition at all — no project deleted_at, no workspace deleted_at, no base row to filter: externalapi's requireProjectMember (ProjectEntityPermission, the gate on sixty-odd routes including every write), externalapi's requireProjectAdmin (the three member write routes), externalapi's projectRole (which requireProjectBase asks for updates and deletes), and internal/workspace's requireProjectMember (the project asset routes). With nothing in them about the project, they answered with the caller's stored role forever after the project was deleted, and the handlers behind them do not compensate: the external API's state create, for one, looks its project up on `w.slug = ? AND p.id = ?` with no deleted_at and inserts. Each now filters `pm.deleted_at IS NULL`. That is the parity-correct spelling as well as the narrowing one — in Django these four read ProjectMember's own manager, which is why the other six queries of this shape in the tree already carry it, and why the thirty-one join sites that traverse the membership from another model's queryset deliberately do not. It cannot refuse a live member: a membership row of a live project has a null deleted_at, and nothing but the cascade ever writes one. What it does not do is close the window. The membership's deleted_at is stamped by the worker draining the soft-delete cascade, after the delete has already answered 204 and after the project's own deleted_at is set, so between those two moments these gates still admit the caller. Closing it immediately takes joining projects and filtering p.deleted_at, which is a stronger predicate than the one Django writes for these permissions; that is a parity decision rather than a bug fix, and it is left for its own change. Adding the filter these four were missing is the fail-closed half, and it is the half that needs no decision. view_handler.go's guest count is the fifth query of this shape and is deliberately not touched. It counts guest memberships to decide whether a caller is restricted to their own views, so fewer matching rows means fewer restrictions: adding the filter there loosens access rather than tightening it, and it has no business inside this commit. The tests run each gate against a session that records statements instead of sending them, which is the only way to check a permission query in packages whose database-backed tests are env-gated — internal/externalapi has no such test at all. Each asserts both what the statement filters and that the gate answers 403 when the lookup matches nothing. --- .../internal/externalapi/intake_handler.go | 3 +- .../internal/externalapi/member_handler.go | 4 +- apps/api/internal/externalapi/permissions.go | 4 +- .../internal/externalapi/permissions_test.go | 39 +++++++++ .../workspace/project_asset_handler.go | 4 +- .../project_asset_permission_test.go | 87 +++++++++++++++++++ 6 files changed, 137 insertions(+), 4 deletions(-) create mode 100644 apps/api/internal/workspace/project_asset_permission_test.go diff --git a/apps/api/internal/externalapi/intake_handler.go b/apps/api/internal/externalapi/intake_handler.go index 0a35b6a0..57683d75 100644 --- a/apps/api/internal/externalapi/intake_handler.go +++ b/apps/api/internal/externalapi/intake_handler.go @@ -490,11 +490,12 @@ 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 requireProjectBase asks when a write is neither a create nor a safe method, so like the other gates over project_members it filters the membership's own soft delete: nothing else in the query says the project still exists. 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", + Where("w.slug = ? AND pm.project_id = ? AND pm.member_id = ? AND pm.is_active = TRUE AND pm.deleted_at IS NULL", c.Param("slug"), c.Param("project"), user.ID). Limit(1).Pluck("pm.role", &roles).Error if err != nil || len(roles) == 0 { diff --git a/apps/api/internal/externalapi/member_handler.go b/apps/api/internal/externalapi/member_handler.go index 968b6678..21ece56f 100644 --- a/apps/api/internal/externalapi/member_handler.go +++ b/apps/api/internal/externalapi/member_handler.go @@ -352,11 +352,13 @@ func validMemberRole(role int) bool { } // requireProjectAdmin is ProjectAdminPermission, which the three write routes swap in for the read one. +// +// Like requireProjectMember it filters the membership's own soft delete, because the project's liveness is not tested anywhere else in the query. 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", + Where("w.slug = ? AND pm.project_id = ? AND pm.member_id = ? AND pm.role = ? AND pm.is_active = TRUE AND pm.deleted_at IS NULL", c.Param("slug"), c.Param("project"), user.ID, roleAdmin). Count(&admins).Error if err != nil { diff --git a/apps/api/internal/externalapi/permissions.go b/apps/api/internal/externalapi/permissions.go index 0bd73e9b..c9549652 100644 --- a/apps/api/internal/externalapi/permissions.go +++ b/apps/api/internal/externalapi/permissions.go @@ -20,11 +20,13 @@ const ( ) // requireProjectMember is ProjectEntityPermission: a safe method wants any active member, and a write wants an admin or a member. +// +// The membership's own soft delete is filtered here, where a join traversed through another model's queryset would leave it out, because this query reads project_members through its own manager and nothing else in it is about the project still existing. Without it the gate answers for a member of a soft-deleted project forever. func (handler *Handler) requireProjectMember(c *gin.Context, user *auth.User, method string) bool { 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", + Where("w.slug = ? AND pm.project_id = ? AND pm.member_id = ? AND pm.is_active = TRUE AND pm.deleted_at IS NULL", c.Param("slug"), c.Param("project"), user.ID). Limit(1).Pluck("pm.role", &roles).Error if err != nil { diff --git a/apps/api/internal/externalapi/permissions_test.go b/apps/api/internal/externalapi/permissions_test.go index 3109b3c2..fa4ed907 100644 --- a/apps/api/internal/externalapi/permissions_test.go +++ b/apps/api/internal/externalapi/permissions_test.go @@ -106,3 +106,42 @@ func TestTheProjectBaseGateWillNotReadASoftDeletedWorkspace(t *testing.T) { } } } + +// The three gates over project_members are the only queries in this app whose whole subject is the membership: they read it through its own manager and carry no other liveness check, so the membership's own soft delete is the one thing standing between a soft-deleted project and a caller who used to be in it. Every other project-scoped query here traverses the membership as a join from a base row it already filters, and leaves the filter out to match Django. +// +// This takes effect when the worker drains the cascade that stamps the membership, not when the project is deleted. The gates do not test the project row itself, so the window between the two stays open; closing it takes a join onto projects, which is a departure from the predicate Django writes for these permissions and is deliberately not made here. +func TestTheProjectGatesRefuseACascadedMembership(t *testing.T) { + for _, test := range []struct { + name string + method string + gate func(*Handler, *gin.Context) bool + }{ + {"requireProjectMember", http.MethodGet, func(handler *Handler, context *gin.Context) bool { + return handler.requireProjectMember(context, gateUser(), http.MethodGet) + }}, + {"requireProjectAdmin", http.MethodPost, func(handler *Handler, context *gin.Context) bool { + return handler.requireProjectAdmin(context, gateUser()) + }}, + {"projectRole", http.MethodPatch, func(handler *Handler, context *gin.Context) bool { + _, found, err := handler.projectRole(context, gateUser()) + if err != nil { + t.Fatalf("the role lookup failed: %v", err) + } + return found + }}, + } { + t.Run(test.name, func(t *testing.T) { + handler, recorded := recordingHandler(t) + context, _ := gateRequest(t, test.method) + + if test.gate(handler, context) { + t.Error("the gate admitted a caller whose membership lookup matched nothing") + } + for _, fragment := range []string{"pm.is_active = TRUE", "pm.deleted_at IS NULL"} { + if !recorded.contains(fragment) { + t.Errorf("the gate does not filter %s:\n %s", fragment, recorded) + } + } + }) + } +} diff --git a/apps/api/internal/workspace/project_asset_handler.go b/apps/api/internal/workspace/project_asset_handler.go index ae7ce285..f5b40219 100644 --- a/apps/api/internal/workspace/project_asset_handler.go +++ b/apps/api/internal/workspace/project_asset_handler.go @@ -436,11 +436,13 @@ func (handler *Handler) projectAsset(c *gin.Context, user *auth.User, uploadedOn } // requireProjectMember is the project branch of allow_permission: an active membership of the project named in the url, at any role. +// +// The membership's own soft delete is filtered because this query is the whole gate: it reads project_members through its own manager and carries no other check that the project or the workspace still exists. func (handler *Handler) requireProjectMember(c *gin.Context, user *auth.User) bool { var count 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.is_active = TRUE", + Where("w.slug = ? AND pm.project_id = ? AND pm.member_id = ? AND pm.is_active = TRUE AND pm.deleted_at IS NULL", c.Param("slug"), c.Param("id"), user.ID).Count(&count).Error if err != nil { handler.internalError(c, err) diff --git a/apps/api/internal/workspace/project_asset_permission_test.go b/apps/api/internal/workspace/project_asset_permission_test.go new file mode 100644 index 00000000..87e5b982 --- /dev/null +++ b/apps/api/internal/workspace/project_asset_permission_test.go @@ -0,0 +1,87 @@ +package workspace + +import ( + "context" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/gin-gonic/gin" + "github.com/yldm-tech/pace/apps/api/internal/auth" + "gorm.io/driver/postgres" + "gorm.io/gorm" + "gorm.io/gorm/logger" +) + +// requireProjectMember is the whole gate on the project asset routes, and its query is the only thing in them that reads the caller's membership. It is not a pure function and this package's database-backed tests are skipped unless WORKSPACE_TEST_DATABASE_URL is set, so what this does is run the gate against a session that records the statement instead of sending it, and read the predicate back out of the record. + +// recordedStatements is a gorm logger that keeps the SQL a session would have sent. Under DryRun nothing is sent, and the trace still carries the statement with its bind values inlined. +type recordedStatements struct { + statements []string +} + +func (recorded *recordedStatements) LogMode(logger.LogLevel) logger.Interface { return recorded } + +func (recorded *recordedStatements) Info(context.Context, string, ...any) {} + +func (recorded *recordedStatements) Warn(context.Context, string, ...any) {} + +func (recorded *recordedStatements) Error(context.Context, string, ...any) {} + +func (recorded *recordedStatements) Trace(_ context.Context, _ time.Time, sql func() (string, int64), _ error) { + statement, _ := sql() + recorded.statements = append(recorded.statements, statement) +} + +func (recorded *recordedStatements) contains(fragment string) bool { + for _, statement := range recorded.statements { + if strings.Contains(statement, fragment) { + return true + } + } + return false +} + +func (recorded *recordedStatements) String() string { + return strings.Join(recorded.statements, "\n ") +} + +// recordingHandler is a Handler whose queries are built and recorded rather than run. DisableAutomaticPing is what keeps gorm.Open from dialling the DSN, which is never connected to. +func recordingHandler(t *testing.T) (*Handler, *recordedStatements) { + t.Helper() + recorded := &recordedStatements{} + database, err := gorm.Open( + postgres.New(postgres.Config{DSN: "postgres://render@127.0.0.1:1/render"}), + &gorm.Config{DryRun: true, DisableAutomaticPing: true, Logger: recorded}, + ) + if err != nil { + t.Fatalf("opening a dry-run database: %v", err) + } + return &Handler{db: database}, recorded +} + +// The gate asks project_members through its own manager and has no other liveness condition in it, so the membership's own soft delete is what stops a member of a soft-deleted project from reading and writing its assets. It arrives with the worker's cascade rather than with the delete, which is a lag and not a hole: the project row itself is stamped first, and no route behind this gate is reachable without the project. +func TestTheProjectAssetGateRefusesACascadedMembership(t *testing.T) { + handler, recorded := recordingHandler(t) + recorder := httptest.NewRecorder() + context, _ := gin.CreateTestContext(recorder) + context.Request = httptest.NewRequest(http.MethodGet, "/", nil) + context.Params = gin.Params{ + {Key: "slug", Value: "acme"}, + {Key: "id", Value: "00000000-0000-0000-0000-0000000000p1"}, + } + + if handler.requireProjectMember(context, &auth.User{ID: "00000000-0000-0000-0000-0000000000u1"}) { + t.Error("the gate admitted a caller whose membership lookup matched nothing") + } + if recorder.Code != http.StatusForbidden { + t.Errorf("the gate answered %d, want %d", recorder.Code, http.StatusForbidden) + } + for _, fragment := range []string{"pm.is_active = TRUE", "pm.deleted_at IS NULL"} { + if !recorded.contains(fragment) { + t.Errorf("the gate does not filter %s:\n %s", fragment, recorded) + } + } +} From 4a9f94acca48a5cb4dd80812bdaf4ce2e64df4d7 Mon Sep 17 00:00:00 2001 From: Evan Hu Date: Fri, 18 Sep 2026 23:29:07 +0900 Subject: [PATCH 6/7] refactor(api): one project-role lookup behind the five permission gates that asked for it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five queries in four packages asked the same question — what is this person's role in this project, if they are still in it — in four different shapes: Pluck into a slice and read element zero, Count and compare with zero, Count with the role in the predicate and compare with zero, and the same again with the member and project arguments in the other order. access.ProjectRole is that question now, and it is the place a future change to this predicate has to be made once instead of five times. Two of the five counted rows matching a role rather than reading the role back, and turning `COUNT(role = admin) > 0` into `role == admin` is only sound because at most one row can match: 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. That is also why this comes after the commit that added `pm.deleted_at IS NULL` to the gates and not before — without that filter a member of a soft-deleted project can have several cascaded rows with different roles, and the two forms are then not the same question. The predicate access.ProjectRole writes is the one all five now carry: `w.slug = ? AND pm.project_id = ? AND pm.member_id = ? AND pm.is_active = TRUE AND pm.deleted_at IS NULL`. The only textual change is in internal/project's workspaceOrProjectAdmin, which wrote the same conditions with the member before the project. view_handler.go's guest count is the sixth query of this shape and stays where it is. It counts guest memberships to decide whether a caller may only see their own views, so it is the one site where filtering the membership's soft delete widens what the caller can reach — folding it into a helper that filters would quietly change a permission in the direction nobody wants. --- apps/api/internal/access/membership.go | 22 +++++++++++++++++++ .../internal/externalapi/intake_handler.go | 14 +++--------- .../internal/externalapi/member_handler.go | 12 +++------- apps/api/internal/externalapi/permissions.go | 14 ++++-------- apps/api/internal/project/handler.go | 9 +++----- .../workspace/project_asset_handler.go | 13 ++++------- 6 files changed, 39 insertions(+), 45 deletions(-) diff --git a/apps/api/internal/access/membership.go b/apps/api/internal/access/membership.go index 5869c5d4..c0ce9454 100644 --- a/apps/api/internal/access/membership.go +++ b/apps/api/internal/access/membership.go @@ -5,6 +5,12 @@ // 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 .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). @@ -33,3 +39,19 @@ type joinSettings struct { 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 +} diff --git a/apps/api/internal/externalapi/intake_handler.go b/apps/api/internal/externalapi/intake_handler.go index 57683d75..6af17581 100644 --- a/apps/api/internal/externalapi/intake_handler.go +++ b/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,18 +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 requireProjectBase asks when a write is neither a create nor a safe method, so like the other gates over project_members it filters the membership's own soft delete: nothing else in the query says the project still exists. +// 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 AND pm.deleted_at IS NULL", - 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. diff --git a/apps/api/internal/externalapi/member_handler.go b/apps/api/internal/externalapi/member_handler.go index 21ece56f..afa4a72d 100644 --- a/apps/api/internal/externalapi/member_handler.go +++ b/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" @@ -352,20 +353,13 @@ func validMemberRole(role int) bool { } // requireProjectAdmin is ProjectAdminPermission, which the three write routes swap in for the read one. -// -// Like requireProjectMember it filters the membership's own soft delete, because the project's liveness is not tested anywhere else in the query. 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 AND pm.deleted_at IS NULL", - 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.", }) diff --git a/apps/api/internal/externalapi/permissions.go b/apps/api/internal/externalapi/permissions.go index c9549652..c242f821 100644 --- a/apps/api/internal/externalapi/permissions.go +++ b/apps/api/internal/externalapi/permissions.go @@ -7,6 +7,7 @@ import ( "github.com/gin-gonic/gin" "github.com/google/uuid" "github.com/jackc/pgx/v5/pgconn" + "github.com/yldm-tech/pace/apps/api/internal/access" "github.com/yldm-tech/pace/apps/api/internal/auth" "gorm.io/gorm/clause" @@ -20,20 +21,13 @@ const ( ) // requireProjectMember is ProjectEntityPermission: a safe method wants any active member, and a write wants an admin or a member. -// -// The membership's own soft delete is filtered here, where a join traversed through another model's queryset would leave it out, because this query reads project_members through its own manager and nothing else in it is about the project still existing. Without it the gate answers for a member of a soft-deleted project forever. func (handler *Handler) requireProjectMember(c *gin.Context, user *auth.User, method string) bool { - 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 AND pm.deleted_at IS NULL", - c.Param("slug"), c.Param("project"), user.ID). - Limit(1).Pluck("pm.role", &roles).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 len(roles) == 0 { + if !member { c.AbortWithStatusJSON(http.StatusForbidden, gin.H{ "detail": "You do not have permission to perform this action.", }) @@ -42,7 +36,7 @@ func (handler *Handler) requireProjectMember(c *gin.Context, user *auth.User, me if method == http.MethodGet || method == http.MethodHead || method == http.MethodOptions { return true } - if roles[0] == roleAdmin || roles[0] == roleMember { + if role == roleAdmin || role == roleMember { return true } c.AbortWithStatusJSON(http.StatusForbidden, gin.H{ diff --git a/apps/api/internal/project/handler.go b/apps/api/internal/project/handler.go index 93bad64c..b3c665ff 100644 --- a/apps/api/internal/project/handler.go +++ b/apps/api/internal/project/handler.go @@ -10,6 +10,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/projects" @@ -666,12 +667,8 @@ func (handler *Handler) workspaceOrProjectAdmin(ctx context.Context, slug, proje if workspaceAdmins > 0 { return true, nil } - var projectAdmins int64 - err = handler.db.WithContext(ctx).Table("project_members pm"). - Joins("JOIN workspaces w ON w.id = pm.workspace_id"). - Where("w.slug = ? AND pm.member_id = ? AND pm.project_id = ? AND pm.role = ? AND pm.is_active = TRUE AND pm.deleted_at IS NULL", slug, userID, projectID, roleAdmin). - Count(&projectAdmins).Error - return projectAdmins > 0, err + role, member, err := access.ProjectRole(ctx, handler.db, slug, projectID, userID) + return member && role == roleAdmin, err } // projectMemberIDsIncludingBots backs the membership check in retrieve, which diff --git a/apps/api/internal/workspace/project_asset_handler.go b/apps/api/internal/workspace/project_asset_handler.go index f5b40219..49f9b937 100644 --- a/apps/api/internal/workspace/project_asset_handler.go +++ b/apps/api/internal/workspace/project_asset_handler.go @@ -6,6 +6,7 @@ import ( "strings" "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/uploads" @@ -435,20 +436,14 @@ func (handler *Handler) projectAsset(c *gin.Context, user *auth.User, uploadedOn return assets[0], true } -// requireProjectMember is the project branch of allow_permission: an active membership of the project named in the url, at any role. -// -// The membership's own soft delete is filtered because this query is the whole gate: it reads project_members through its own manager and carries no other check that the project or the workspace still exists. +// requireProjectMember is the project branch of allow_permission: an active membership of the project named in the url, at any role. The project is the :id parameter on these routes rather than the :project the external API uses. func (handler *Handler) requireProjectMember(c *gin.Context, user *auth.User) bool { - var count 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.is_active = TRUE AND pm.deleted_at IS NULL", - c.Param("slug"), c.Param("id"), user.ID).Count(&count).Error + _, member, err := access.ProjectRole(c.Request.Context(), handler.db, c.Param("slug"), c.Param("id"), user.ID) if err != nil { handler.internalError(c, err) return false } - if count == 0 { + if !member { forbidden(c) return false } From 5c6c5087e2c616f9a2463b63b313eec5c2ca40b0 Mon Sep 17 00:00:00 2001 From: Evan Hu Date: Fri, 18 Sep 2026 23:49:54 +0900 Subject: [PATCH 7/7] chore: bump the workspace version, which check-version requires of every PR --- apps/admin/package.json | 2 +- apps/space/package.json | 2 +- apps/web/package.json | 2 +- package.json | 2 +- packages/codemods/package.json | 2 +- packages/constants/package.json | 2 +- packages/editor/package.json | 2 +- packages/hooks/package.json | 2 +- packages/i18n/package.json | 2 +- packages/propel/package.json | 2 +- packages/services/package.json | 2 +- packages/shared-state/package.json | 2 +- packages/tailwind-config/package.json | 2 +- packages/types/package.json | 2 +- packages/typescript-config/package.json | 2 +- packages/ui/package.json | 2 +- packages/utils/package.json | 2 +- 17 files changed, 17 insertions(+), 17 deletions(-) diff --git a/apps/admin/package.json b/apps/admin/package.json index e3be06b7..e2361a51 100644 --- a/apps/admin/package.json +++ b/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", diff --git a/apps/space/package.json b/apps/space/package.json index f773d059..7fe586f1 100644 --- a/apps/space/package.json +++ b/apps/space/package.json @@ -1,6 +1,6 @@ { "name": "space", - "version": "1.4.38", + "version": "1.4.39", "private": true, "license": "AGPL-3.0", "type": "module", diff --git a/apps/web/package.json b/apps/web/package.json index 08ee07aa..cd55c38a 100644 --- a/apps/web/package.json +++ b/apps/web/package.json @@ -1,6 +1,6 @@ { "name": "web", - "version": "1.4.38", + "version": "1.4.39", "private": true, "license": "AGPL-3.0", "type": "module", diff --git a/package.json b/package.json index fa3dbd24..6291a891 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "pace", - "version": "1.4.38", + "version": "1.4.39", "private": true, "description": "Project management that keeps pace with the work", "license": "AGPL-3.0", diff --git a/packages/codemods/package.json b/packages/codemods/package.json index eff21b10..e20d849a 100644 --- a/packages/codemods/package.json +++ b/packages/codemods/package.json @@ -1,6 +1,6 @@ { "name": "@pace/codemods", - "version": "1.4.38", + "version": "1.4.39", "private": true, "scripts": { "check:format": "oxfmt --check .", diff --git a/packages/constants/package.json b/packages/constants/package.json index 6b7e5f02..b58b4f76 100644 --- a/packages/constants/package.json +++ b/packages/constants/package.json @@ -1,6 +1,6 @@ { "name": "@pace/constants", - "version": "1.4.38", + "version": "1.4.39", "private": true, "license": "AGPL-3.0", "type": "module", diff --git a/packages/editor/package.json b/packages/editor/package.json index e3aab465..cc755645 100644 --- a/packages/editor/package.json +++ b/packages/editor/package.json @@ -1,6 +1,6 @@ { "name": "@pace/editor", - "version": "1.4.38", + "version": "1.4.39", "private": true, "description": "Core Editor that powers Pace", "keywords": [ diff --git a/packages/hooks/package.json b/packages/hooks/package.json index 7db38f89..a465e16a 100644 --- a/packages/hooks/package.json +++ b/packages/hooks/package.json @@ -1,6 +1,6 @@ { "name": "@pace/hooks", - "version": "1.4.38", + "version": "1.4.39", "private": true, "description": "React hooks that are shared across multiple apps internally", "license": "AGPL-3.0", diff --git a/packages/i18n/package.json b/packages/i18n/package.json index 5c179910..afd8f563 100644 --- a/packages/i18n/package.json +++ b/packages/i18n/package.json @@ -1,6 +1,6 @@ { "name": "@pace/i18n", - "version": "1.4.38", + "version": "1.4.39", "private": true, "description": "I18n shared across multiple apps internally", "license": "AGPL-3.0", diff --git a/packages/propel/package.json b/packages/propel/package.json index 5cf9843f..1a404e58 100644 --- a/packages/propel/package.json +++ b/packages/propel/package.json @@ -1,6 +1,6 @@ { "name": "@pace/propel", - "version": "1.4.38", + "version": "1.4.39", "private": true, "license": "AGPL-3.0", "type": "module", diff --git a/packages/services/package.json b/packages/services/package.json index f0af3409..b2d62b85 100644 --- a/packages/services/package.json +++ b/packages/services/package.json @@ -1,6 +1,6 @@ { "name": "@pace/services", - "version": "1.4.38", + "version": "1.4.39", "private": true, "license": "AGPL-3.0", "type": "module", diff --git a/packages/shared-state/package.json b/packages/shared-state/package.json index 98cd1e7c..a1f21d63 100644 --- a/packages/shared-state/package.json +++ b/packages/shared-state/package.json @@ -1,6 +1,6 @@ { "name": "@pace/shared-state", - "version": "1.4.38", + "version": "1.4.39", "private": true, "description": "Shared state shared across multiple apps internally", "license": "AGPL-3.0", diff --git a/packages/tailwind-config/package.json b/packages/tailwind-config/package.json index 35406e35..4fef73fa 100644 --- a/packages/tailwind-config/package.json +++ b/packages/tailwind-config/package.json @@ -1,6 +1,6 @@ { "name": "@pace/tailwind-config", - "version": "1.4.38", + "version": "1.4.39", "private": true, "description": "common tailwind configuration across monorepo", "license": "AGPL-3.0", diff --git a/packages/types/package.json b/packages/types/package.json index 4f252210..f16a1dfd 100644 --- a/packages/types/package.json +++ b/packages/types/package.json @@ -1,6 +1,6 @@ { "name": "@pace/types", - "version": "1.4.38", + "version": "1.4.39", "private": true, "license": "AGPL-3.0", "type": "module", diff --git a/packages/typescript-config/package.json b/packages/typescript-config/package.json index 1df10826..a65311ee 100644 --- a/packages/typescript-config/package.json +++ b/packages/typescript-config/package.json @@ -1,6 +1,6 @@ { "name": "@pace/typescript-config", - "version": "1.4.38", + "version": "1.4.39", "private": true, "license": "AGPL-3.0", "files": [ diff --git a/packages/ui/package.json b/packages/ui/package.json index 8f128831..d3f99689 100644 --- a/packages/ui/package.json +++ b/packages/ui/package.json @@ -1,6 +1,6 @@ { "name": "@pace/ui", - "version": "1.4.38", + "version": "1.4.39", "private": true, "description": "UI components shared across multiple apps internally", "license": "AGPL-3.0", diff --git a/packages/utils/package.json b/packages/utils/package.json index 274f4cc1..84fd1eca 100644 --- a/packages/utils/package.json +++ b/packages/utils/package.json @@ -1,6 +1,6 @@ { "name": "@pace/utils", - "version": "1.4.38", + "version": "1.4.39", "private": true, "description": "Helper functions shared across multiple apps internally", "license": "AGPL-3.0",