respect expose_user_email configuration in datalibraries permissions list

[martenson]

part of #1177

ping @erasche

[martenson]

rebased, rebuilt and fixed indent

[hexylena]

Ok, tested this today, I'm happy with the changes. This is a much, much better sharing experience. 👍

[martenson]

Thanks for testing @erasche and sorry the sharing is still ... limited. We should do better.

[dannon]

s/divclass/div\ class/ ?

[dannon]

I'm probably doing it wrong, but I tested the following.

1. create a new data library as an admin ([email protected]).
2. granted "access" and "manage" permissions to a non-admin ([email protected])
3. (as [email protected] user, in a separate browser) I go to the permission management page, and I can't actually add more roles that can access the page. "No matches found" won't go away, preventing me from entering any additional roles here.

Note: with default expose_user_name/expose_user_email in galaxy.ini, so, False

[martenson]

@dannon did you flip the expose_user_email config?

[dannon]

@martenson No, I left it false on purpose to test. I should still be able to add emails that I know, without the assisted autocomplete, right?

[martenson]

@dannon No, you cannot search for specific emails/users nor can you add arbitrary string that Galaxy will check against the roles DB table.

[dannon]

Ok, so without 'expose' set, these are just completely disabled for non-admins? Can we add a hint that that's the case on the interface? (and not allow people to try)

[martenson]

@dannon They are not disabled, you only see your role, your sharing roles and other roles/groups you are part of. The logic is in the get_valid_roles method.

edit: The reason why you cannot see anything is that your only valid option is your own private role. But if you were part of an e.g. group you would see its role there.

[dannon]

I get that I can see my own role. This user is supposed to be able to administer this library though, and cannot, and the UI doesn't provide any sort of a clue why -- it just appears broken, which is bad UX.

(screenshot snipped, wasn't useful)

If we just add some text saying "Note that this instance is configured to hide usernames and email addresses, so you will not be able to add users that are not <blah>", it'd be fine.

[martenson]

@dannon That does not have much to do with this PR though. Feel free to contribute to the discussion here.

[dannon]

I took that issue to be about much higher level overall security design, which will of course eventually change the UX here, but I'm talking about my experience with this particular interface that this PR touches. It appears broken, to me, and a little additional text would explain that.

[martenson]

@dannon Similar interface is in history on every dataset's permissions and in fact on sharing of every object - workflows, pages, history etc.

I suggest opening a PR that alters labeling on all these, if you think we can do a better job explaining the current configuration and options.

This PR specifically addresses the following part of #1177 :

When modifying permissions as non-admin user, expose_user_name and expose_user_email config options are not respected

and thus, unless there are bugs related to that, I deem it finished.

[dannon]

You specifically mention workflows, pages, and histories, but they all have a simple "email address of user to share with", which allows me to (as expected) type in whatever address I want. Anyway, this was an open PR, I gave feedback, you're welcome to ignore it. I'm moving on.

[martenson]

@dannon Could you please stop holding my PR as hostage for what you deem is a label changing issue?

[dannon]

@martenson What am I holding hostage? I spent time to review this, reported one typo and a confusing interface. At first, I literally did not know if the interface was fixed or not because it's so confusing. I did not -1 this, and am not 'holding anything hostage'.

[hexylena]

I see where @dannon is coming from, but that can be kicked down the road slightly. I don't think we're regressing any here. I've added an item to #1177 relating to his comment.