Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delete accounts that are not staff #1615

Merged
merged 10 commits into from
Jun 27, 2018
Merged

Delete accounts that are not staff #1615

merged 10 commits into from
Jun 27, 2018

Conversation

patjouk
Copy link
Contributor

@patjouk patjouk commented Jun 22, 2018

close MozillaFoundation/mofo-devops-private#140

Add a scheduled task that will run every week to remove any accounts that are not staff or with a '@mozillafoundation.org' email address.

@alanmoo I am kicking people out that I shouldn't with this rule?

Post-merge todo:

  • Add Heroku Scheduler add-on with python network-api/manage.py delete_non_staff to prod

@patjouk patjouk requested a review from cadecairos June 22, 2018 13:40
@cadecairos cadecairos temporarily deployed to foundation-mofostaging-pr-1615 June 22, 2018 13:41 Inactive
@patjouk patjouk temporarily deployed to foundation-mofostaging-pr-1615 June 22, 2018 14:21 Inactive
@patjouk patjouk temporarily deployed to foundation-mofostaging-pr-1615 June 22, 2018 14:42 Inactive
@patjouk patjouk temporarily deployed to foundation-mofostaging-pr-1615 June 22, 2018 14:53 Inactive
@patjouk patjouk temporarily deployed to foundation-mofostaging-pr-1615 June 22, 2018 15:03 Inactive
@patjouk patjouk temporarily deployed to foundation-mofostaging-pr-1615 June 22, 2018 15:19 Inactive
@patjouk patjouk temporarily deployed to foundation-mofostaging-pr-1615 June 22, 2018 15:24 Inactive
@alanmoo
Copy link
Contributor

alanmoo commented Jun 22, 2018

As long as people who can log in and edit continue to be able to, even if they're not @mozillafoundation.org, we should be good. Test case is Luca's account.

@cadecairos cadecairos temporarily deployed to foundation-mofostaging-pr-1615 June 25, 2018 11:15 Inactive
@patjouk patjouk temporarily deployed to foundation-mofostaging-pr-1615 June 25, 2018 12:12 Inactive
@patjouk
Copy link
Contributor Author

patjouk commented Jun 25, 2018

I decided to remove apscheduler: I realized that the clock dyno would always be running, which is definitely overkill for a 10s script that would run every Sunday 😅 Instead, we should use Heroku Scheduler add-on that will spin a dyno for the duration of the script execution only.

@alanmoo the current setup would kick Luca's account. I'll check if people are part of a group or not too.

@patjouk patjouk temporarily deployed to foundation-mofostaging-pr-1615 June 25, 2018 16:53 Inactive
@patjouk patjouk temporarily deployed to foundation-mofostaging-pr-1615 June 25, 2018 16:55 Inactive
@patjouk patjouk temporarily deployed to foundation-mofostaging-pr-1615 June 25, 2018 17:04 Inactive
@patjouk patjouk temporarily deployed to foundation-mofostaging-pr-1615 June 25, 2018 17:06 Inactive
Copy link

@cadecairos cadecairos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one minor thing to update in the non-staff query.

I also think we should write and execute these kind of tasks as Django management commands - can you move it to the network-api/networkapi/management/commands folder and wrap it in the required classes for it to be callable by manage.py?

print("Deleting non staff users")
group_q = Group.objects.all()
non_staff = User.objects.exclude(
Q(email__contains='mozillafoundation.org') |

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should probably be Q(email__endswith='@mozillafoundation.org') |, which will ensure sneaky people don't get by with something like "trollolol+mozillafoundation.org@example.com" - which seems unlikely, but lets just protect against it anyways.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am totally going to start using that now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're totally right, let's fix that XD

@patjouk patjouk temporarily deployed to foundation-mofostaging-pr-1615 June 26, 2018 12:34 Inactive
@cadecairos cadecairos temporarily deployed to foundation-mofostaging-pr-1615 June 26, 2018 12:38 Inactive
@patjouk patjouk temporarily deployed to foundation-mofostaging-pr-1615 June 26, 2018 12:52 Inactive
def setUp(self):
User.objects.create(username='Alex', email='alex@mozillafoundation.org')

def test_mozilla_Foundation_users_not_deleted(self):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you fix the capitalization of this test name?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done!

@patjouk patjouk temporarily deployed to foundation-mofostaging-pr-1615 June 26, 2018 13:53 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants