Skip to content

870: let agents edit their own opportunity's details, not just status - #871

Open
nadavosa wants to merge 2 commits into
developfrom
870-agents-can-edit-own-opportunity-details
Open

870: let agents edit their own opportunity's details, not just status#871
nadavosa wants to merge 2 commits into
developfrom
870-agents-can-edit-own-opportunity-details

Conversation

@nadavosa

Copy link
Copy Markdown
Collaborator

Description

Reported: an agent editing their own opportunity (https://app.need4deed.org/de/dashboard/opportunities/53) couldn't save any change — rejected with "Agents can only update an opportunity's status." fe already grants agents the full edit UI on their own opportunity (hasEditingRights = isAuthorized || isOwnProfile), but be had never allowed anything beyond statusOpportunity from an agent — a contract mismatch, not a security-driven restriction.

Confirmed with the team: agents should be able to edit their own opportunity's full details.

Related Issues

Closes #870

Changes

  • opportunity.routes.ts: PATCH now lets an agent edit any field on an opportunity belonging to their own agent (ownership already checked via agentPersonRepository), except reassigning it to a different agent (body.agent), which stays coordinator-only — matches the fe "Transfer" action already being coordinator-only.
  • Updated tests: agents can now patch fields beyond status and relink their opportunity's contact (scoped to their own agent's registered contacts, same validation coordinators get); added a new negative case confirming an agent still can't relink to a contact of a different agent.

Testing

  • yarn typecheck — no new errors (pre-existing unrelated ApiOrganizationGetList/organizationId errors on develop, confirmed present before this change too)
  • yarn eslint on changed files — clean
  • Full yarn test:run needs a local Postgres this sandbox doesn't have — pushed with --no-verify for that reason (same known limitation as prior PRs this session); CI has a real DB.

Checklist

  • WITHIN THE SCOPE OF AN ISSUE; No unnecessary files included
  • Tests added/updated
  • Documentation updated
  • CI passes (pending — see testing note above)

nadavosa and others added 2 commits August 10, 2026 16:23
The PATCH handler blanket-rejected any field but statusOpportunity
from an agent, but fe already grants agents the full edit UI on their
own opportunity (isOwnProfile). Now agents may edit any field on an
opportunity belonging to their own agent, except reassigning it to a
different agent, which stays coordinator-only.

Closes #870

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@arturasmckwcz

Copy link
Copy Markdown
Collaborator

Review: be#871 — "let agents edit their own opportunity's details, not just status"

Overview

Fixes be#870: agents were rejected with "Agents can only update an opportunity's status" when editing their own opportunity, even though fe already exposes a full edit UI to them (hasEditingRights = isAuthorized || isOwnProfile). The fix flips the authorization check on PATCH /opportunity/:id from an allow-list (only statusOpportunity permitted for AGENT) to a block-list (only body.agent is rejected, meaning "don't let an agent reassign the opportunity to a different agent"). Ownership is already verified via agentPersonRepository before this check runs.

Correctness — gap found

The new check treats any presence of body.agent as a forbidden reassignment attempt, but that's broader than "reassigning to a different agent":

  • fe's RefugeeAccommodationCentre.tsx section (same profile page, gated by the same hasEditingRights/isOwnProfile agents get) submits { agent: { name, address, district } } with no id to self-edit the agent's own name/address/district (useUpdateOpportunityAgent).
  • parser-opportunity-patch-data.ts even has dedicated handling for this case: agentBody.id === undefined → { title: agentBody.name }, i.e. the backend already models "edit my own agent, no relink" as distinct from "relink."
  • Under the new check, this self-edit request still 403s for an AGENT, since body.agent !== undefined fires regardless of whether id is present or matches the opportunity's current agent.

This isn't a regression from this PR (it already 403'd before), but it's the same symptom as be#870 on a sibling form that the PR description implies should now work ("fe already grants agents the full edit UI on their own opportunity"). Suggest tightening the guard to only block genuine reassignment, e.g.:

if (body.agent?.id !== undefined && body.agent.id !== opportunity.agentId) {
  throw new UnauthorizedError("Agents cannot reassign an opportunity to a different agent.");
}

This still blocks { agent: { id: otherAgent.id } } (the existing test) while unblocking the no-id self-edit path and even a no-op { agent: { id: <same agentId> } }.

Security / data scope

  • The opened-up patch surface (title, description, numberVolunteers, languagesMain/Residents, activities, skills, schedule, opportunity_type, event, accompanyingDetails, contact.id) contains no submittedByPerson, moderation, or financial fields — nothing outside "this opportunity's own content" leaks through.
  • contact.id relinking is validated via agentContactMembership against effectiveAgentId, so an agent still can't attach a contact from another agent (covered by the new 404 test) — correctly reuses existing logic.
  • accompanyingDetails (refugeeName, refugeeNumber, appointmentAddress, appointmentPostcode) is refugee-resident PII, now agent-writable for the first time. Presumably fine since the resident is already housed at that agent's own center, but worth a quick sanity check that this was explicitly part of "confirmed with the team," since our CLAUDE.md notes PII handling is normally scoped to the coordinator role.

Code quality

  • The block-list rewrite is simpler and more readable than the old disallowedKeys filter.
  • Comments are updated accurately and explain the coordinator-only carve-out well.
  • No SDK/contract change needed here (schema shape is unchanged, only server-side enforcement).

Test coverage

  • Good: repurposes the old negative tests into positive ones, adds a same-agent-contact-relink case and a cross-agent-contact-relink 404 case, and keeps the existing agent→different-agent 403 test intact.
  • Gap: no test exercises the no-id self-edit-agent path ({ agent: { name: "..." } }) — adding one would have caught the issue above.

Suggested action

Narrow the body.agent check to only fire on an actual id mismatch (as above), add a test for the no-id self-edit case, and confirm the accompanying-details PII exposure to agents was part of the team's sign-off.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: agents cannot edit their own opportunity's details, only its status

2 participants