Page MenuHomePhabricator

Fix various policy issues with project member materialization around interactions between "Project Members" policies and Subprojects/Milestones
Open, LowPublic

Description

See T10779. Previously, see T13462.

There are two remaining policy issues here:


Extended Policies vs Policy Proxying

When you have project P with milestone M and the parent project has its edit policy set to "Editable By: Project Members", the policy is inherited by the milestone.

However, it is inherited directly, by calling $this->getParentProject()->getEditPolicy(). This means when it is an object policy (like "Project Members") it evaluates in the context of M, not P. So the effective policy that is evaluated when trying to edit M with the edit policy "Project Members" is "Members of M" (that is, using the object itself), not "Members of P" (that is, inheriting the effective parent policy).

In theory, both evaluate the same way because both should have the same members, but in practice they evaluate differently because of the issue below.

To reproduce this:

  • Create project P.
    • Do not join project P yet.
  • Create milestone M of project P.
  • Join Project P.
  • Set "Editable By" on Project P to "Project Members".
  • Try to edit M.

Expected behavior:

  • You can edit M.

Actual behavior:

  • You can not edit M.

Notes:

  • The order of operations (create a milestone first, then join the project) is important because project members are incorrectly copied into milestones as raw members when a milestone is created. See below.
  • This is the issue in T10779.

Materialized vs Raw Members

The actual policy rule also tests that the viewer is a raw member of the project, not a materialized member. "Materialized" members are: virtual memberships in a parent project created by membership in a subproject; or virtual memberships in a milestone created by membership in a parent project.

This creates problems when the "Project Members" rule is evaluated against parent projects or milestones. To reproduce:

  • Create project P.
  • Create subproject S.
  • Join subproject S.
  • Set "Editable By" for P to "Project Members" without joining it. (This isn't normally possible in the web UI, and requires modifying the database.)
  • Try to edit P.

Expected behavior:

  • You can edit P.

Actual behavior:

  • You can not edit P.

Notes:

  • This state is hard to reach because it's hard (impossible?) to do in the UI, but it's technically a valid/consistent state.
  • A simpler set of reproduction instructions does not work because parent project raw members are not removed during subproject conversion. See below.

Copying Raw Members

Something is afoot here. To reproduce this:

  • Create project P.
  • Join project P.
  • Create milestone M of P.

Expected behavior:

  • M has no raw members as edges.

Actual behavior:

  • M has a (mostly-ignored) copy of P's members as edges.

This usually has little impact on behavior, but is incorrect and generally throws a wrench into the gears here. Offhand, I'm not actually sure where/why this is happening.


Leftover Raw Members

When the first subproject S of parent project P is created, we do not remove the raw member edges from P. This leads to the "Members" policy continuing to evaluate members at the time P was mutated.

To reproduce:

  • Create project P.
    • Join project P.
    • Set "Editable By" to "Project Members".
  • Create subproject S of P.
  • Leave subproject S.

Expected behavior:

  • You can not edit P.

Actual behavior:

  • You can edit P (your previous membership has been "saved" incorrectly).

See also:

Event Timeline

epriestley created this task.
epriestley renamed this task from Fix various policy issues issues with project member materialization around interactions between "Project Members" policies and Subprojects/Milestones to Fix various policy issues with project member materialization around interactions between "Project Members" policies and Subprojects/Milestones.Jan 17 2020, 5:19 PM

Probably related: According to https://phabricator.wikimedia.org/T261642, it seems that when leaving a project, phabricator leaves behind some cruft in the form of materialized memberships for milestones of that project.

Leftover Raw Members

Just for my own notes, I also saw this while testing the change in T13596, so it didn't magically fix itself or anything.