Page MenuHomePhabricator

Put User accounts in Spaces so some users can not see one another
Open, WishlistPublic

Description

For us it is a requirement that users only see users that have access to their own space. The reasons are information privacy laws for one, but even more so corporate secrets: It might be inadvisable to disclose to employees of company A that an employee of their competitor B also works with us.

I suggest to filter all typeahead fields, lists of users and /people/ to show only those users which have access to spaces which the current user also has access to. That way functionality is not impaired (assigning users which do not have access to an object's space to that object has no effect anyway), but users cannot peek into disjunct spaces (in terms of users).

Event Timeline

devurandom raised the priority of this task from to Needs Triage.
devurandom updated the task description. (Show Details)
devurandom added a project: Spaces.
devurandom added a subscriber: devurandom.

It's vaguely possible we may implement this eventually, but it's not going to happen any time soon and almost certainly not this year.

epriestley triaged this task as Wishlist priority.Jul 30 2015, 2:13 PM
epriestley renamed this task from Filter user lists by visible spaces to Put User accounts in Spaces so some users can not see one another.Jul 30 2015, 2:20 PM
devurandom updated the task description. (Show Details)Jul 30 2015, 2:23 PM

Would abovementioned approach be viable? I.e. filtering the list of users in /people/, and those which typeahead and similar functions provide? Would you accept a patch that implements it that way? Do you have any other suggestions on how to implement this efficiently?

This isn't a priority, which means I don't want to spend time planning, reviewing, merging, or supporting it right now, not just developing it.

Writing a patch is only a tiny fraction of the total cost of making a change. When we spend time on something less important, that time is taken away from something more important, regardless of whether I'm planning/reviewing the less-important thing or actually writing the less-important patch. It's still taking up my time working on something less important instead of something more important. See discussion here:

https://secure.phabricator.com/book/phabcontrib/article/contributing_code/

devurandom added a comment.EditedJul 31 2015, 10:11 AM

I am now using following patch locally:

See below for the current, complete patch

devurandom added a comment.EditedJul 31 2015, 5:43 PM

I added following patch to allow admins to add users into spaces, if currently that user is in no space at all:

See below for the current, complete patch

Otherwise it would be impossible to add these users to a project (and thus space).

You might argue that this just workarounds a configuration speciality on our instance: Users have no access to any space by default. This is because the default space is already restricted to employees only, which gives a lot of control over external users, because they are unable to login until someone explicitly puts them into a space.

epriestley moved this task from Backlog to Future on the Spaces board.Aug 11 2015, 12:58 PM

I updated the patch yet again. In this iteration, anyone can add users to projects, if these users are not in any space at all.

commit da493478448a19a2f2ae1d9528e0ac89c3fcfcc9
Author: Dennis Schridde <dennis.schridde@uni-heidelberg.de>
Date:   Fri Jul 31 12:10:08 2015 +0200

    Hide users from typeahead boxes which are not in any of the viewer's spaces
    
    These checks are ignored for users, who are not in any space (and thus no common space), to allow people to add them to projects anyway.
    
    Ref: T9021

diff --git a/src/applications/people/typeahead/PhabricatorPeopleDatasource.php b/src/applications/people/typeahead/PhabricatorPeopleDatasource.php
index cdd6898..5ffd703 100644
--- a/src/applications/people/typeahead/PhabricatorPeopleDatasource.php
+++ b/src/applications/people/typeahead/PhabricatorPeopleDatasource.php
@@ -40,6 +40,32 @@ final class PhabricatorPeopleDatasource
 
     $users = $this->executeQuery($query);
 
+    $filtered_users = array();
+    foreach ($users as $user) {
+      $spaces = PhabricatorSpacesNamespaceQuery::getViewerActiveSpaces($user);
+
+      if (empty($spaces)) {
+        $filtered_users[] = $user;
+        continue;
+      }
+
+      foreach ($spaces as $space) {
+        $policy = $space->getPolicy(PhabricatorPolicyCapability::CAN_VIEW);
+
+        try {
+          PhabricatorPolicyFilter::requireCapabilityWithForcedPolicy($viewer, $user, PhabricatorPolicyCapability::CAN_VIEW, $policy);
+        }
+        catch (PhabricatorPolicyException $ex) {
+          continue;
+        }
+
+        $filtered_users[] = $user;
+        break;
+      }
+    }
+
+    $users = $filtered_users;
+
     if ($this->enrichResults && $users) {
       $phids = mpull($users, 'getPHID');
       $handles = id(new PhabricatorHandleQuery())

One issue might be projects and their member list. Users from other spaces should probably be replaced with a "question mark" placeholder, so that others don't know who it is, but see that the project has some members. That way generic cross-space groups don't become a privacy/security issue.

Mayeu awarded a token.Sep 14 2015, 4:17 PM
Mayeu added a subscriber: Mayeu.
avivey removed a subscriber: avivey.Sep 14 2016, 5:54 PM
Luke081515.2 added a subscriber: Luke081515.2.
PicoCreator added a comment.EditedSep 14 2016, 6:42 PM

@devurandom - I am facing a similar problem in my installation regarding "corporate secrets". How to I get this and perhaps other similar patches up and running?

urzds added a subscriber: urzds.Jan 5 2017, 11:00 AM
urzds added a comment.Jan 5 2017, 11:03 AM

@PicoCreator:

  1. Paste the patch into a $FILE
  2. cd $PHABRICATOR_SRC
  3. patch -p1 < $FILE