Skip to content

Add an endpoint saying who a project's work may be assigned to - #373

Merged
haksungjang merged 2 commits into
mainfrom
er65-team-members
Sep 4, 2026
Merged

Add an endpoint saying who a project's work may be assigned to#373
haksungjang merged 2 commits into
mainfrom
er65-team-members

Conversation

@haksungjang

Copy link
Copy Markdown
Contributor

The assignment work shipped self-assignment only, because no route below super admin listed users, so a screen could offer the signed-in person to themselves and nobody else. A team lead could not hand work to a team member. This is the missing half.

GET /v1/projects/{project_id}/assignable-members returns the people the assignment save accepts: on the team that owns the project, active, and not a service account. Addressed by project rather than by team, because that is what the caller holds and asking for a team id would mean handing out team ids to look work up by. Gated at developer to match the assignment PATCH it feeds: a caller who may perform an assignment has to be able to compose one, and a list gated higher would leave the write reachable only by somebody who already knew the id.

An id and a display name, and nothing else. The admin team view carries an email because an administrator auditing who can reach a team needs one; a picker does not, and shipping it would make every project page a place addresses can be collected from. Service accounts are absent rather than labelled, so the list is the set the write accepts rather than a superset the caller has to filter. Somebody who never set a display name comes back with full_name: null and is still listed, because they are assignable and hiding them would leave the write reachable only by id.

The list and the write share one query, and that is the part the security review changed. The first shape exported the three conditions as a predicate for both callers to use. The reviewer compiled select(User.id).where(assignable_predicate(team)) and got FROM users, memberships: a cross join returning every active person in the deployment rather than the team's, with no error, no warning, and every test still green, because the two call sites at the time happened to write the join by hand. Returning the query carries the join with it, so forgetting it is not available. A test asserts the statement has one final FROM, and builds the two-FROM form alongside so that assertion is known to distinguish something.

Three other review findings are recorded rather than fixed, each for a stated reason.

The 403 body names the owning team's UUID. The enumeration test checked for names and user ids and so certified "no member data in the refusal" while the refusal carried the team id; adding it to that tuple fails. It is pre-existing on all fifteen routes built on get_project, including GET /v1/projects/{project_id} at a lower grade, so this route discloses nothing new, and changing the message is not this pull request's to make. A separate test asserts the id is present, so the next person meets it as a decision instead of reading the first test as a guarantee it does not give.

The viewer denial is conditional. The effective grade is one number across every membership, so a viewer here who is a developer elsewhere clears the floor, and get_project then asks only for membership. The gate is unchanged on purpose: the same person reaches the assignment PATCH by the same mechanism, so narrowing only the list would create the divergence between offered and accepted that the shared query exists to prevent. The case is asserted at its actual result, so changing the role model to per-team grading fails here and asks whether the write moved too.

The premise that project access implies team membership is now a test rather than a comment. It switches a project to visibility=organization, asserts the switch took, and requires an outsider to still be refused. Only visibility=team is honoured today, so whoever wires organization-wide visibility gets a member-enumeration route for free; a comment needs reading and a test does not.

Left for separate work: the response has no cap, which is stated in the schema so a consumer does not have to discover it. Measured rather than guessed: the largest team in the reference deployment holds four people, so this is a shape to know about rather than one to work around, and the right bound depends on a picker screen that does not exist yet.

Verification: mypy over 916 files, ruff, and 470 tests across this endpoint, the assignment PATCH, obligations and the permission matrices. Reversing each design choice fails a different test: dropping the active, service-account or team condition, lowering the gate, removing the cross-team check, and taking the join back off the query.

Nothing below super admin listed users, so a screen could only offer the
signed-in person to themselves. This returns the people the assignment save
accepts: on the team, active, not a service account.

An id and a display name, and nothing else. The admin team view carries an
email because an administrator auditing who can reach a team needs one; a
picker does not, and shipping it would make every project page somewhere
addresses can be collected from.

Addressed by project rather than by team, so a caller never needs a team id to
look work up by, and gated at developer to match the assignment it feeds.

The list and the write share one query. A shared condition was the first
shape, and the security review showed it compiles to a cross join when a
caller forgets to join memberships, returning every active person in the
deployment with no error and no warning. Returning the query carries the join
with it.
The drift gate is a snapshot of the wire surface, so adding an endpoint has to
show as a reviewed line rather than as a passing test. One line added and
nothing else moved.
@haksungjang
haksungjang merged commit 1320111 into main Sep 4, 2026
24 checks passed
@haksungjang
haksungjang deleted the er65-team-members branch September 4, 2026 22:15
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.

1 participant