Page MenuHomePhabricator

Grant shell user right with project memberships and remove autocreation of shell requests
Closed, ResolvedPublic

Related Objects

Event Timeline

yuvipanda raised the priority of this task from to Needs Triage.
yuvipanda updated the task description. (Show Details)
yuvipanda added subscribers: yuvipanda, Andrew.

The reason for human interaction is to prevent a bot (or malicious human) from suddenly allocating 100 accounts and using us to mine bitcoins or send spam or whatnot. That hasn't happened much, but I'm not sure that shows that it /can't/ happen.

Also, new users would still need to wait to be added to a project, so this doesn't necessarily save them a ton of time.

Can you explain what you would do with cgroups and/or ulimits to help?

@Andrew yeah but the people adding people to new projects is different from the people granting shell access, so it requires two manual intervention steps from two different groups of people now.

cgroups / ulimits will let us:

  1. Put a cap on CPU / Memory they can use
  2. Restrict which applicatications they can actually run. I'd imagine letting bastions do nothing but allowing ssh forwarding, maybe.
  3. We can also (independent of that) setup monitoring for new groups and then monitor / alert for spikes.

@Andrew we can also put in the same roadblocks in place against bot account creation that prod has (Captcha, AbuseFilter, Rate Limiting)

@Andrew we can also put in the same roadblocks in place against bot account creation that prod has (Captcha, AbuseFilter, Rate Limiting)

AbuseFilter requires LOTS of manual tweaking and a decently large sample size to test with, and our captcha has been broken for a loooong time.

We do not get much account spam atm with no captchas at all, and you just
use abusefilter after the fact. And remember what happened when the captcha
was turned off for a bit in MW.org a few months ago? It still catches a lot
of accounts.

I'm indifferent about this. I sometimes do not grant the shell user right because of a "bad feeling", and it's hard to prove if that makes a difference.

I would put it the other way: Shell access without belonging to a project is useless, so let's get rid of it. If you apply for a project, checking the bit is easily done. And it wouldn't confuse users who just register to use a watchlist or their preferred skin, and it wouldn't inspire others to see what they can do with shell access.

Yeah, ultimately I want us to get rid of 'shell access' as a manual step different from project access.

So now we just need to check when adding to a project and if shell bit isn't set set it. Let me see if I can write up a patch for that.

Change 220181 had a related patch set uploaded (by Yuvipanda):
Automatically add to shell group when adding to a project

https://gerrit.wikimedia.org/r/220181

^ should fix it. @Andrew thoughts / objections? It automatically adds to the shell group people the first time they get added to a specific project.

Change 220181 merged by jenkins-bot:
Automatically add to shell group when adding to a project

https://gerrit.wikimedia.org/r/220181

Change 220468 had a related patch set uploaded (by Yuvipanda):
Automatically add to shell group when adding to a project

https://gerrit.wikimedia.org/r/220468

Great! When the automation has been tested, we need to remove the "generate shell request page on account creation" logic and remove the various documentation references to "first, request shell access".

Yup, I have a patch for the removal as well.

Change 220468 merged by jenkins-bot:
Automatically add to shell group when adding to a project

https://gerrit.wikimedia.org/r/220468

Change 220480 had a related patch set uploaded (by Yuvipanda):
Automatically add to shell group when adding to a project

https://gerrit.wikimedia.org/r/220480

Change 220480 merged by jenkins-bot:
Automatically add to shell group when adding to a project

https://gerrit.wikimedia.org/r/220480

So this works, except that the user doesn't seem to be automatically added to the Bastion project? Is that a new problem or existing problem?

That would be a new problem. Why is the consequence of "if( $user->isAllowed( 'loginviashell' ) )" "$project = new OpenStackNovaProject( $wgOpenStackManagerBastionProjectName ); […] $project->addMember( $username );"? Shouldn't it be "$user->mediaWikiFunctionToAllow( 'loginviashell' )"?

(Reading Extension:OpenStackManager code always makes my head hurt, so please excuse if the answer is obvious :-).)

Sorry, wrong code location. I meant special/SpecialNovaProject.php's:

if ( !$user->isAllowed( 'loginviashell' ) ) {
        # Grant user the shell right if they have
        # successfully been added to a project
        $user->addGroup( 'shell' );
}

Okay, rereading the code made it now clear to me: The shell => loginviashell relation is defined in operations/mediawiki-config:wmf-config/InitialiseSettings.php.

Could it be that the UserRights hook (that itself calls manageShellAccess()) is not called by ->addGroup(), but only interactively?

I. e., manageShellAccess() shouldn't be hooked on UserRights, but on UserAddGroup and UserRemoveGroup?

Change 220635 had a related patch set uploaded (by Tim Landscheidt):
Use correct hooks for bastion project membership

https://gerrit.wikimedia.org/r/220635

Change 220635 merged by jenkins-bot:
Use correct hooks for bastion project membership

https://gerrit.wikimedia.org/r/220635

Change 221648 had a related patch set uploaded (by Yuvipanda):
Use correct hooks for bastion project membership

https://gerrit.wikimedia.org/r/221648

Change 221648 merged by jenkins-bot:
Use correct hooks for bastion project membership

https://gerrit.wikimedia.org/r/221648

Wheee this is done now :D

Wait for a few days and remove the autocreate?

I assume you tested it with User:TestAccount3? If it worked, IMHO we can remove the autocreate immediately.

scfc renamed this task from Automatically grant shell user right to everyone who signs up on wikitech to Grant shell user right with project memberships and remove autocreation of shell requests.Jun 29 2015, 8:32 PM
scfc triaged this task as Medium priority.
scfc updated the task description. (Show Details)
scfc removed a project: Patch-For-Review.
scfc set Security to None.

Change 221750 had a related patch set uploaded (by Yuvipanda):
Do not make an automatic shell request for all new accounts

https://gerrit.wikimedia.org/r/221750

Change 221750 merged by jenkins-bot:
Do not make an automatic shell request for all new accounts

https://gerrit.wikimedia.org/r/221750

The Cloud-Services project tag is not intended to have any tasks. Please check the list on https://phabricator.wikimedia.org/project/profile/832/ and replace it with a more specific project tag to this task. Thanks!