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.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 27 2015, 8:21 PM

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.

scfc added a subscriber: scfc.Apr 27 2015, 9:55 PM

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.

Restricted Application added a project: Cloud-Services. · View Herald TranscriptJun 23 2015, 4:55 PM

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

scfc added a comment.Jun 24 2015, 2:59 PM

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?

scfc added a comment.Jun 24 2015, 5:42 PM

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 :-).)

scfc added a comment.Jun 24 2015, 5:46 PM

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' );
}

Doing it manually works.

scfc added a comment.Jun 24 2015, 6:14 PM

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?

scfc added a comment.Jun 24 2015, 6:18 PM

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

That seems totally possible!

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?

scfc added a comment.Jun 29 2015, 8:30 PM

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 Normal 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

We could SWAT ^ whenever :)

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

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

Ltrlg added a subscriber: Ltrlg.Jul 5 2015, 3:43 PM
yuvipanda closed this task as Resolved.Jul 24 2015, 3:54 AM
yuvipanda claimed this task.

Wooo