feat: scope GET /opportunity to an AGENT caller's own agents

[ivannissimrch]

Description

fe 934 asks that NGO users only see opportunities belonging to their own organisation. Today GET /opportunity applies no caller-based scoping at all, so an NGO user sees every organisation's.

The issue says both "their own operator (their email domain)" and "for their own shelter", which give different lists. Nadav settled it on fe 950: "anyone from the same NGO/agent can see the opportunities there of that NGO/agent". So this scopes by agent. agent.organization_id is NULL for every agent on local and dev so that operator scoping would return an empty list for everyone today anyway.

GET /opportunity now resolves the caller's agent memberships server-side and restricts the result to those agents when the caller's role is AGENT. Coordinators and admins are unaffected.

Only ACTIVE memberships count. A PENDING membership is one a coordinator has not approved yet, so it grants nothing.

fe 950 is the frontend half and needs this to be safe to ship.

Related Issues

fe 934
fe 950

Changes

• getCallerAgentIds in src/server/utils/data/: a person's ACTIVE agent memberships, deduped
• GET /opportunity: when the caller is an AGENT, constrain where.agent.id
  to those IDs
• An AGENT with no ACTIVE membership gets an empty list rather than falling
  through to an unfiltered query, so the failure mode is closed, not open
• 6 tests in src/test/server/routes/opportunity-agent-scope.routes.test.ts

Checklist

• WITHIN THE SCOPE OF AN ISSUE; No unnecessary files included
• Tests added/updated
• Documentation updated
• CI passes

[arturasmckwcz]

Code review findings

Correctness

1. src/server/utils/pii/visible-persons.ts:68 — resolveCallerVisibility resolves an AGENT caller's agentIds without filtering on ACTIVE membership status, while the new getCallerAgentIds does filter on AgentMembershipStatus.ACTIVE. Two disagreeing definitions of "the agents this caller belongs to" now exist. A person with only a PENDING membership at Agent X gets zero opportunities from the newly-scoped list endpoint, but resolveCallerVisibility still treats them as an Agent X member for PII-masking elsewhere (GET /opportunity/:id, comments, contact PII) — contradicting this PR's own stated principle that a PENDING membership grants nothing.

2. src/server/routes/opportunity/opportunity.routes.ts:614 (mirrored at :439 for POST / and :725 for the contact check) — the AGENT ownership check via agentPersonRepository.findOneBy has no status: ACTIVE filter, unlike getCallerAgentIds. A person with a PENDING membership at an agent can still create/update opportunities for that agent, even though the read path now excludes PENDING members — a more permissive write path sitting right next to the newly-tightened read path.

3. src/server/routes/opportunity/opportunity.routes.ts:153 — GET /opportunity/:id has no agent-ownership scoping for AGENT callers at all. An AGENT scoped out of another org's opportunities in the list endpoint can still fetch that opportunity directly by ID, undercutting the stated security goal (fe#934).

4. src/server/routes/opportunity/legacy.routes.ts:199 — the legacy GET / opportunity endpoint is public: true (bypasses fastify.authenticate entirely) and returns all NEW/ACTIVE/SEARCHING opportunities unmasked, including PII, to unauthenticated callers. Pre-existing, but it fully defeats the purpose of the scoping just added to the sibling list endpoint.

5. src/server/routes/opportunity/opportunity-volunteer.routes.ts:42 — GET /:id/volunteer-linked has no agent-ownership check on opportunityId. An AGENT scoped out of an opportunity by the new list restriction can still call this endpoint directly with that opportunity's ID and see matched-volunteer data.

Test coverage

6. src/test/server/routes/opportunity-agent-scope.routes.test.ts:91 — otherAgentId = Number(owned[1].agentId) indexes into a GROUP BY result with no check that ≥2 distinct agents exist in test data. If fewer, owned[1] is undefined and .agentId throws in beforeAll, failing the whole suite with a cryptic TypeError instead of a clear precondition error.

7. src/server/routes/opportunity/opportunity.routes.ts:286 — the where.agent spread is written to compose with a filter getOpportunityWhere might someday set, but getOpportunityWhere never sets where.agent today, so this branch is untested/unreachable and the "scope wins over filter" test passes trivially. If filter.agentId support is added later with an incompatible shape, the AGENT scope could be silently bypassed without this test catching it.

Simplification / reuse

8. src/server/utils/data/get-caller-agent-ids.ts:1 — duplicates agent-membership resolution already present in resolveCallerVisibility instead of reusing/extending it (see finding 1 — the two have already diverged on ACTIVE-status handling), contrary to the repo's reuse-before-create rule.

Minor

9. src/server/utils/data/get-caller-agent-ids.ts:8 — if (!personId) return []; treats personId === 0 as absent. Not exploitable today (Person.id starts at 1), but a latent trap if a sequence were ever reset to 0.

10. src/server/routes/opportunity/opportunity.routes.ts:271 — getCallerAgentIds re-queries agent_person for the caller's agentIds in the handler, duplicating the query resolveCallerVisibility already runs in the preSerialization PII-masking hook for the same request — a redundant DB round trip on every AGENT list request.

---

🤖 Posted by Claude Code