Page MenuHomePhabricator

Fix people being able to see /people/create/
AbandonedPublic

Authored by chad on Aug 3 2015, 3:21 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Nov 20, 2:39 PM
Unknown Object (File)
Wed, Nov 20, 12:30 PM
Unknown Object (File)
Sat, Nov 16, 3:09 PM
Unknown Object (File)
Tue, Nov 12, 2:12 PM
Unknown Object (File)
Mon, Nov 11, 5:18 AM
Unknown Object (File)
Fri, Nov 8, 9:52 AM
Unknown Object (File)
Thu, Nov 7, 2:28 AM
Unknown Object (File)
Oct 14 2024, 12:57 PM
Tokens
"Orange Medal" token, awarded by joshuaspence.

Details

Summary

Fixes T9041. Let's non-admins see the page. Not sure how this ever worked?

Test Plan

Test on a non admin account, can see page, but not create users. Give permissions to All Users, See ability to create a real user.

Diff Detail

Repository
rP Phabricator
Branch
people-perms
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 7471
Build 7982: [Placeholder Plan] Wait for 30 Seconds
Build 7981: arc lint + arc unit

Event Timeline

chad retitled this revision from to Fix people being able to see /people/create/.
chad updated this object.
chad edited the test plan for this revision. (Show Details)
chad added a reviewer: epriestley.
This comment was removed by eadler.
epriestley edited edge metadata.

This "works" because we added it for the Phacility cluster and use it to reduce access there (from "Administrators" to "No One").

I think this patch isn't correct. In particular, I believe it:

  • allows any user to see this screen and try to create bot and mailing list users, even if they are not administrators; and
  • doesn't actually allow creation of new users by non-administrators, since it just sends you to PhabricatorPeopleNewController after you make a choice, which also requires administrative access.

The "Can Create (non-bot) Users" permission doesn't mean "can create users", it means "assuming you can already create users because you are an administrator, can you create 'standard' (non-bot, non-list) users?". We added it to prevent Phacility instances from creating new unlinked user accounts, and it probably isn't very useful for much outside of that.

I'll follow up in T9041. I don't think there's a way forward here right now.

This revision now requires changes to proceed.Aug 3 2015, 1:13 PM