Page MenuHomePhabricator

Update protect as security issue to use acl*security
Closed, ResolvedPublic

Description

@mmodell. Sorry for getting this in late, as I know you were just merging a change here.

Here is the pickle. We want #Security-related to be named security but the mechanism for Protect as security issue calls the project by name. I have added the alias for acl*security (seems to noralize to acl_security) to the current #security project details.

If we update the mechanism from

$projects = WMFSecurityPolicy::getProjectByName('security');

to

$projects = WMFSecurityPolicy::getProjectByName('acl_security');

will this do the Right Thing (TM) with just the alias? I think yes, and if so could you please update this?

Event Timeline

@chasemp WMFSecurityPolicy::getProjectByName uses the project name only and it will not match on hashtags. In fact, there isn't even a convenient mechanism to query by the Additional Hashtags field. I can change the code to use a different project name or I can switch it to use PHIDs, however, that has one drawback which is that it makes it more difficult for testing in other instances of phab where the PHIDs won't match.

btw: I really don't like the the acl* convention because it doesn't normalize consistently. I really don't see why we can't use only the icon as the indicator of ACL projects and then we won't need a naming convention.

@chasemp WMFSecurityPolicy::getProjectByName uses the project name only and it will not match on hashtags. In fact, there isn't even a convenient mechanism to query by the Additional Hashtags field. I can change the code to use a different project name or I can switch it to use PHIDs, however, that has one drawback which is that it makes it more difficult for testing in other instances of phab where the PHIDs won't match.

OK, let me think on it for a day and we can figure out how to do a switcharoo of some kind.

btw: I really don't like the the acl* convention because it doesn't normalize consistently. I really don't see why we can't use only the icon as the indicator of ACL projects and then we won't need a naming convention.

I'm not in love with it either. I think the original idea was the prepend and special char would prevent it from polluting project search ahead, and it's more or less done that, but iirc you said we could do it another way if the icon is consistent? Seems sane to me. For the moment though, I don't want to mix that work and this. I'll make another task for addressing ACL naming convention.

btw: I really don't like the the acl* convention because it doesn't normalize consistently. I really don't see why we can't use only the icon as the indicator of ACL projects and then we won't need a naming convention.

I read almost everything via my email client (with plainlink email) and it would be bit of confusing times if I have to double check to see what kind of project this project is mentioning — is it acl? Or components? Dunno when you are on mail clients.

btw: I really don't like the the acl* convention because it doesn't normalize consistently. I really don't see why we can't use only the icon as the indicator of ACL projects and then we won't need a naming convention.

I'm not in love with it either. I think the original idea was the prepend and special char would prevent it from polluting project search ahead, and it's more or less done that, but iirc you said we could do it another way if the icon is consistent? Seems sane to me. For the moment though, I don't want to mix that work and this. I'll make another task for addressing ACL naming convention.

New users of Phabricator may not easily know what the padlock means. e.g. If there's a project named "stewards" someone may add the project if a task are related to steward work (i.e. Stewards-and-global-tools). But if there's "acl" in project name they will know it is not a normal project.

Also, this will reduce much naming conflict.

@chasemp: Another thing to consider: phabricator spaces might be the more appropriate way to handle this now, rather than projects and acls.

As the policy of space is absolute, other users can not be invited to the task.

@chasemp: The best way I can think of to make the transition easy (and ease future maintainability of the code) is to make the project names into config variables, then we can change the config (via web interface, not puppet) in sync with any changes to the actual project names.

@chasemp: The best way I can think of to make the transition easy (and ease future maintainability of the code) is to make the project names into config variables, then we can change the config (via web interface, not puppet) in sync with any changes to the actual project names.

Not opposed to that, the other things is that since I created Security as restricted with the same members of Security (current) I could move the name(s) around right before you deploy phab updates and nothing is at risk during. The only race condition is someone using the protect as feature in that window and pulling down the Security PHID for Security and then we want to open that group up. But that should be fairly transparent if it happened, and would be easily fixable.

It takes a bit of coordination but seems benign. Having the value be configurable is def preferable from a ease towards the future, but are we ever going to change this again? I can't think of when.

working in real time with @mmodell now to do this dance with minimal impact :)