Page MenuHomePhorge

Add Diffusion policy capability "Can Edit and View Identities"
Closed, ResolvedPublic

Description

Add a repository.identity.create capability to allow limiting creating and editing Diffusion identities.

This can reduce vandalism in open Phorge installations in which anyone can create an account (and thus edit identities) while you still want anyone to be able to view identities.

See https://phabricator.wikimedia.org/T327105 and https://phabricator.wikimedia.org/diffusion/identity/view/33242/ for a vandalism example.

Event Timeline

Cannot properly test locally (too many exceptions on PHP8.2 trying to create a local Git repo and commits to be indexed in Diffusion) how much this change would actually affect both editing and creating (or not) so I dumped an untested patch into P13

As an approach this seems good to me. Would it make sense to put creating identities behind the existing Edit policy of the repository?

Would it make sense to put creating identities behind the existing Edit policy of the repository?

@speck: What would be your per-repo policy use case, in contrast to a global policy use case? In my case there's an open account registration and we want everyone to see/browse repos but not everyone to be able to manually edit user data.

Would it make sense to put creating identities behind the existing Edit policy of the repository?

I think Identity is "global" - not per-repository, so no.

It also might make sense to hide the actual list from the general public (it's a mapping of emails to users).

P13 looks generally ok, but should probably default to "Admin".

It also might make sense to hide the actual list from the general public

I agree. Wouldn't it make sense to put it behind repository.identity.view?

The "Create Identity" button on /diffusion/identity/ should be guarded by this new policy access, though currently that form is not functional - see T15453

In T15443#9918, @avivey wrote:

It also might make sense to hide the actual list from the general public (it's a mapping of emails to users).

Could you file a separate feature request please? Also I have no idea in which place I'd have to add some getViewPolicy or CAN_VIEW code (similar to getCreateNewObjectPolicy in PhabricatorRepositoryidentityEditEngine.php)...

Edit: Argh. Found src/applications/diffusion/controller/DiffusionIdentityViewController.php right after adding this comment. So I guess I should also adjust the naming scheme.

Patch in P13 is incomplete, policy does not cover going to /diffusion/identity/edit/1/ and setting Assigned To to another user but should.

It's important to restrict the ability to create Diffusion IDs.

I don't get the code.
src/applications/diffusion/controller/DiffusionIdentityViewController.php (note the View in its name) includes stuff like
$edit_uri = $this->getApplicationURI("identity/edit/{$id}/") defining ->setName(pht('Edit Identity')) (note the Edit here).
Also, this mashes up Diffusion and Repositories to add confusion (DiffusionIdentityEditController calls PhabricatorRepositoryIdentityEditEngine?).
I think I give up.

Also, this mashes up Diffusion and Repositories to add confusion

Yeah, the "Repository viewing/managing" application changed names at some point, and is now split across 2 directories with a bunch of confusing names. It's basically a single application that has inconsistent file names.

aklapper renamed this task from Add Diffusion policy capability "Can create and edit Identities" to Add Diffusion policy capability "Can Edit and View Identities".Oct 26 2023, 19:36