Make it possible not to allow anyone to edit Diffusion identities.
Make it possible not to allow anyone to view other users' email addresses.
Closes T15443
Details
Details
- Reviewers
speck valerio.bozzolan - Group Reviewers
O1: Blessed Committers - Maniphest Tasks
- T15443: Add Diffusion policy capability "Can Edit and View Identities"
- Commits
- rP90f651d669e4: Add Diffusion policy capability "Can Edit and View Identities"
- As an admin, go to /applications/view/PhabricatorDiffusionApplication/ and see new policy "Can Edit and View Identities" set to "All Users" (as implicitly before)
- As an admin, go to /applications/view/PhabricatorDiffusionApplication/ and change "Can Edit and View Identities" from "All Users" to "Administrators"
- As a non-admin, go to /diffusion/identity/ and try to select the disabled "Create Identity" button; get an error message clicking it due to lack of permissions
- Given there is at least one identity defined, as a non-admin, go directly to /diffusion/identity/view/1/ and get "You do not have permission to view this object."
- Given there is at least one identity defined, as a non-admin, go directly to /diffusion/identity/edit/1/ and get "You do not have permission to view this object."
- As a non-admin, go directly to /diffusion/identity/edit/form/default/ and get "You do not have permission to edit this object."
- As a non-admin, go directly to /diffusion/identity/ and get "No Identities found." instead of seeing the existing identities listed.
- As an admin, go to /diffusion/identity/ and still see the existing identities listed.
- As an admin, go to /diffusion/identity/, select "Create Identity" to go to /diffusion/identity/edit/ and see the "Create Identity" page (though broken; see T15453)
- As an admin, go to /diffusion/identity/view/1/ and still see the existing identity.
- As an admin, go to /diffusion/identity/edit/1/ and successfully edit the existing identity.
Diff Detail
Diff Detail
- Repository
- rP Phorge
- Branch
- T15443diffusionIdentities (branched from master)
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 868 Build 868: arc lint + arc unit
Event Timeline
Comment Actions
Note that this patch does NOT default to admin as proposed in T15443#9918 as I could imagine confusion by overwriting that value on existing installations. Happy to adjust though...
Comment Actions
Maybe add some doc - the view and edit actions being lumped together are because it would be a larger change to split out that functionality right now, correct?
In general I’m not familiar with the upgrade/migration of policies and what needs considered. E.g this policy doesn’t exist today at all (or, the value for the application is hardcoded) and so a database migration wouldn’t be necessary. After this change lands the selected policy would exist in the database somewhere - if we do eventually split the view and edit policy into separate policies would that require a database migration?
Comment Actions
Create, view, and edit are lumped together because I do not see a use case to split them. (In which situations do average users usually want to see/browse Diffusion identities?)
In general I’m not familiar with the upgrade/migration of policies and what needs considered. E.g this policy doesn’t exist today at all (or, the value for the application is hardcoded) and so a database migration wouldn’t be necessary.
That also my understanding; similar past changes like rP7ed35123a347a05c70c97eba2bec2b36eb2b3218 did not require database handling.
After this change lands the selected policy would exist in the database somewhere - if we do eventually split the view and edit policy into separate policies would that require a database migration?
I do not know either what a potential future split would imply. I'd like to see arguments why to split these policies further.
