Page MenuHomePhabricator
Create Task
Maniphest T7404

Projects that are joinable by no-one or require legalpad documents should require special privileges to add users
Closed, WontfixPublic
Actions

Description

As a user of Phabricator in a data-sensitive industry we are trying to limit access to projects in a very strict manner.

We had setup a project in a way to restrict membership as best as we know how... There are two configurations that non-admin users were still able to add users that conflicts with my understanding of the policies.

Configuration 1

  • Created Projects Secret Project.
  • Secret Project can only be edited by members of Secret Project
  • Secret Project can only be viewed by members of Secret Project
  • No one can join Secret Project

Configuration 2

  • Created Projects Secret Project.
  • Created #Legalpad document Secret Project Membeship.
  • Secret Project can only be edited by members of Secret Project
  • Secret Project can only be viewed by members of Secret Project
  • Users can only join Secret Project if they sign Secret Project Membership

After having either of these configurations, a non-admin member of Secret Project was able to add a user, who had not signed Scret Project Membership, to the project. This is somewhat related to T7403

Related Objects

Event Timeline

SimplyKnownAsG created this task.Feb 27 2015, 9:57 PM
SimplyKnownAsG raised the priority of this task from to Needs Triage.
SimplyKnownAsG updated the task description. (Show Details)
SimplyKnownAsG added a project: Projects.
SimplyKnownAsG added a subscriber: SimplyKnownAsG.
btrahan added a subscriber: btrahan.Feb 27 2015, 10:09 PM

As things work today, people who can edit can also edit the membership. Is there some reason you can't configure your policy such that the set of editors are also the set of people who could be adding (or removing) people from the membership?

chad added a subscriber: chad.Feb 27 2015, 10:11 PM

My assumption is "can join" isn't the same as "can be added". It it were, no one would be in Secret Project to begin with.

epriestley added a subscriber: epriestley.Feb 27 2015, 10:11 PM

In particular, if users who could edit the project could not edit the membership, they could still edit the "edit membership" policy, then add themselves to it, then edit members. "Edit Project" necessarily trumps all other edit policies.

SimplyKnownAsG added a comment.Feb 27 2015, 11:00 PM

Great, @btrahan's suggestion seems to work great; however, I still think it is odd that members can be added without signing a #Legalpad.

joshuaspence added a subscriber: joshuaspence.Feb 28 2015, 3:14 AM
chad added a comment.Feb 28 2015, 3:23 AM

T3820 or maybe T5681, but Spaces is our more generalized application plan to hard wall-off portions of Phabricator. I could see Legalpad being a requirement possibly there.

epriestley closed this task as Wontfix.Mar 1 2015, 3:28 PM
epriestley claimed this task.

One general thought is that we could prompt administrators to confirm that they want to add members who could not otherwise join the group, or prevent their addition outright, but I believe both of these workflows are very common (e.g., on this install we have a number of projects with "Can Join: No One" and a manual membership list; construction of these projects was intentional). And users who can edit projects could always get around these restrictions, so any measure here would only prevent mistakes, not malicious actors.

We could also add a "weak" edit permission which would let you update the description, icon, etc., but not let you update membership or policies. Or we could add CLI locking or something. However, we haven't seen other use cases for anything in this vein, and haven't heard about users making mistakes here in the wild.

We'll keep an ear out, but for now this is working as intended and we believe the product behavior is a good fit for the overwhelming majority of real-world use cases (particularly, that users manually adding members who could not join projects is essentially always an intentional action -- not a mistake -- on real installs today).

Phabricator · Built by Phacility