Page MenuHomePhabricator

Should not allow searching for users if "Can Browse User Directory" is not allowed
Open, Needs TriagePublic

Description

even If "Can Browse User Directory", a logged-out user can still go to the quick-search, or advanced search, and search for users there.

This might be hypothetical, but T4358 is similar.

(Works on admin.phacility.com right now - open https://admin.phacility.com/search/query/8P5PAHn4RfZw/ in incognito)

Event Timeline

avivey created this task.Jun 27 2017, 9:13 PM
avivey created this object with visibility "Custom Policy".

This is fundamentally unfixable:

https://admin.phacility.com/typeahead/class/PhabricatorPeopleUserFunctionDatasource/

If you select users with a typeahead -- a capability we must reasonably provide -- you can necessarily enumerate them.

You can also use user.search to enumerate them even more easily in a programmatic way.

The "Can Browse User Directory" permission is basically just to make search engine indexing moderately less exhaustive, although search engines will still find user profiles for any users who have ever left a comment, created an object, etc.

Possibly we should just remove this permission since it's inevitably somewhat confusing and not ultimately very meaningful.

This is fundamentally unfixable

This statement is overly strong, but I think any fix for this requires giving users real visibility, and opens a huge tangled web of product questions.

For example, when I want to assign a task to you, what allows me to do so? Do I need to know your email address? Do I need to add you as a friend somehow?

I don't think there's any answer to this which works well for admin.phacility.com or any open source install but is meaningfully more restrictive than "everyone can see all user accounts".

It could be "anyone who is logged in can see all user accounts", but I think we more or less have to cripple "public" mode.

We could possibly tailor this on admin.phacility.com, which is a little weird in how it deals with some of its top-level policy stuff. I think policy.allow-public is enabled only so blog.phacility.com can work?

chad added a subscriber: chad.Jun 28 2017, 6:18 AM

Spaces for user accounts?

If we put spaces on user accounts, how do I get into a Space you have permission to see so you can add me as a Manager on the payment account for phacility.phacility.com in Phortune using the typeahead?

If I'm part of business.phacilty.com (main job) and non-profit.phacility.com (side project) on admin.phacility.com, which Space is my central account in?

If users are in a Space which isn't public, every task will render as "Restricted user created this task. Restricted user added a comment: ...", etc. Would any install with policy.allow-public want this / be happy about this change?


It's definitely possible to put policies and/or Spaces on user accounts, but I think this will make nearly every interaction dramatically more complicated and/or way worse, and it only helps installs that set policy.allow-public but don't want accounts to be enumerable, which is sort of inherently contradictory.

Also note that we already have this warning:


I think adding this option was probably a mistake, since it implies it does something substantive, but it doesn't really, and I don't think it can be made to.

I can't think of any solution to this short of full policy + complex policy rules such as "users that share project/space/payment account/instance with this user", which is complex (and probably slow to compute, or requires lots of extra stuff to maintain).

This whole thing is a little hypothetical, but T4358 mentions a use case without allow-public, for consulting firms.

I only looked at this myself because I'm thinking of using the platform in a way that's way unsupported, and I'd want to basically disable all user browsing (Except for non-super-admin), and I would be forking everything related to user-visibility quite hard anyway.
I can see some org not wanting their users to be searchable on admin.phacility, but this org would probably be on private-cluster anyway,

I guess the TL;DR is:

  • We don't properly answer the use-case described in T4358, although we talk about it
  • We can support this use-case using either (1) lots and lots of code and complexity, or (2) a warning to self-hosted installs that we don't support this use-case, and possibly (3) similar warning in Phacility docs.
  • While option 1 might be desirable product-wise, the complexity considerations should push it far down the road.

The "consulting firms" use case is explicitly disavowed by, e.g., the Spaces documentation, here:

https://secure.phabricator.com/book/phabricator/article/spaces/#limitations-and-caveats

I think we're already consistent about saying that this use case doesn't work and isn't supported, although this option is an outlier and a little misleading since it provides a glimmer of hope that it might work.

I don't currently plan to ever support this -- T11597#193665 is roughly the state of the world. If we do want to support these "secret client portals", I think building tiny walled-off areas in Nuance that interact with the main install via a support barrier is probably the most promising way forward. I'm not excited about that either, but more excited about it than trying to put policies on users, projects, etc., and still leaking their existence at the end of the day because two users can't both be named @epriestley.

I'm touching some adjacent code for a "Disable User" permission in T13164.

Motivated mostly by making my life and the narrow use case on admin.phacility.com easier, I'm inclined to possibly expand this permission from "Can Browse User Directory" to "Can View User Profiles". You would then need this capability to:

  • Browse the user directory.
  • Find users in global search.
  • Find users in the global search typeahead.
  • View user profiles (/p/xyz/).

This would not prevent enumeration of user accounts by logged-out users: they can still query the typeahead datasource.

But, in theory, we could actually lock this down too, I think:

  • Let datasources define required permission to query them.
  • Aggregate datasources require aggregate permission? Or only query the permitted datasources (how do we explain that the typeahead is half-functional to users?).
  • If a user lacks permission on a datasource, the control disables itself with some explanation (e.g., "log in to search for tasks by author").
  • Modern SearchEngine / *.search stuff is smart enough that it could disable the entire constraint based on datasource permissions.

This would also solve "find users in the global search typeahead" automatically. Maybe that use case motivates "partially disable aggregate datasources".

You can still try to register accounts to enumerate usernames, but this would at least be a higher barrier.

epriestley changed the visibility from "Custom Policy" to "Public (No Login Required)".Aug 27 2018, 4:32 PM
epriestley added a project: People.

I made this public since I've disclosed/discussed this elsewhere, including an indirect reference in the Spaces documentation, and I'm going to schedule it alongside some other stuff.

(For anyone getting added by Herald, this is a year old, it was just private when filed.)