From 2aad606e9d34b8c7cdc6ef6366b78a0d00ad48d5 Mon Sep 17 00:00:00 2001
From: Tanner Prestegard <tanner.prestegard@ligo.org>
Date: Tue, 18 Jun 2019 14:55:26 -0500
Subject: [PATCH] Delete RobotUser model
This was an exact duplicate of the user model and basically just
provided a simple way to organize robot accounts. It's not useful
anymore since we'll just use a Group going forward to organize
these accounts.
---
docs/admin_docs/source/robot_certificate.rst | 22 +++++-----
gracedb/ligoauth/admin.py | 3 +-
gracedb/ligoauth/middleware.py | 5 ---
.../migrations/0046_delete_robotuser_model.py | 36 +++++++++++++++++
gracedb/ligoauth/models.py | 7 ----
gracedb/ligoauth/tests/test_middleware.py | 40 ++++++++++++++-----
6 files changed, 78 insertions(+), 35 deletions(-)
create mode 100644 gracedb/ligoauth/migrations/0046_delete_robotuser_model.py
diff --git a/docs/admin_docs/source/robot_certificate.rst b/docs/admin_docs/source/robot_certificate.rst
index d4cfb9558..6a17f8b8e 100644
--- a/docs/admin_docs/source/robot_certificate.rst
+++ b/docs/admin_docs/source/robot_certificate.rst
@@ -103,13 +103,14 @@ Edit the migration to do what you want it to do. You could use this as a templat
]
def create_robots(apps, schema_editor):
- RobotUser = apps.get_model('ligoauth', 'RobotUser')
+ User = apps.get_model('auth', 'User')
X509Cert = apps.get_model('ligoauth', 'X509Cert')
- Group = apps.get_model('auth', 'Group')
- lvc_group = Group.objects.get(name=settings.LVC_GROUP)
+ AuthGroup = apps.get_model('ligoauth', 'AuthGroup')
+ lvc_group = AuthGroup.objects.get(name=settings.LVC_GROUP)
+ robot_group = AuthGroup.objects.get(name='robot_accounts')
for entry in ROBOTS:
- user, created = RobotUser.objects.get_or_create(username=entry['username'])
+ user, created = User.objects.get_or_create(username=entry['username'])
if created:
user.first_name = entry['first_name']
user.last_name = entry['last_name']
@@ -121,10 +122,8 @@ Edit the migration to do what you want it to do. You could use this as a templat
# Create the cert objects and link them to our user.
for dn in entry['dns']:
- cert, created = X509Cert.objects.get_or_create(subject=dn)
- if created:
- cert.save()
- cert.users.add(user)
+ cert, created = X509Cert.objects.get_or_create(subject=dn,
+ user=user)
# Add our user to the LVC group. This permission is required to
# do most things, but may *NOT* always be appropriate. It may
@@ -132,14 +131,17 @@ Edit the migration to do what you want it to do. You could use this as a templat
# a particular pipeline.
lvc_group.user_set.add(user)
+ # Add user to robot accounts
+ robot_group.user_set.add(user)
+
def delete_robots(apps, schema_editor):
- RobotUser = apps.get_model('ligoauth', 'RobotUser')
+ User = apps.get_model('auth', 'User')
X509Cert = apps.get_model('ligoauth', 'X509Cert')
for entry in ROBOTS:
for dn in entry['dns']:
X509Cert.objects.get(subject=dn).delete()
- RobotUser.objects.get(username=entry['username']).delete()
+ User.objects.get(username=entry['username']).delete()
class Migration(migrations.Migration):
diff --git a/gracedb/ligoauth/admin.py b/gracedb/ligoauth/admin.py
index d77a96cc4..ff30c7b6f 100644
--- a/gracedb/ligoauth/admin.py
+++ b/gracedb/ligoauth/admin.py
@@ -1,6 +1,6 @@
from django.contrib import admin
-from .models import RobotUser, LigoLdapUser, X509Cert
+from .models import LigoLdapUser, X509Cert
class LigoLdapUserAdmin(admin.ModelAdmin):
list_display = ['username', 'first_name', 'last_name']
@@ -10,6 +10,5 @@ class X509CertAdmin(admin.ModelAdmin):
list_display = ['subject']
search_fields = ['subject']
-admin.site.register(RobotUser)
admin.site.register(LigoLdapUser, LigoLdapUserAdmin)
admin.site.register(X509Cert, X509CertAdmin)
diff --git a/gracedb/ligoauth/middleware.py b/gracedb/ligoauth/middleware.py
index 223a24492..e10be4daa 100644
--- a/gracedb/ligoauth/middleware.py
+++ b/gracedb/ligoauth/middleware.py
@@ -85,11 +85,6 @@ class ShibbolethWebAuthMiddleware(PersistentRemoteUserMiddleware):
the Shibboleth session. Session group data is treated as definitive.
"""
- # Don't do anything if the user is a robot account since their group
- # memberships are managed internally.
- if hasattr(user, 'robotuser'):
- return
-
# Get groups from session which are in database as a QuerySet
session_group_names = request.META.get(cls.group_header, '').split(
cls.group_delimiter)
diff --git a/gracedb/ligoauth/migrations/0046_delete_robotuser_model.py b/gracedb/ligoauth/migrations/0046_delete_robotuser_model.py
new file mode 100644
index 000000000..71243f17b
--- /dev/null
+++ b/gracedb/ligoauth/migrations/0046_delete_robotuser_model.py
@@ -0,0 +1,36 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.20 on 2019-06-18 18:03
+from __future__ import unicode_literals
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ('events', '0034_add_subgrb_search'),
+ ('superevents', '0002_fix_permission_typo'),
+ ('alerts', '0003_add_created_updated_time_fields_to_notification'),
+ ('django_twilio', '0001_initial'),
+ ('admin', '0002_logentry_remove_auto_add'),
+ ('guardian', '0005_authorize_raven_users_to_populate_pipelines'),
+ ('user_sessions', '0003_auto_20161205_1516'),
+ ('ligoauth', '0045_populate_robot_accounts_authgroup'),
+ ]
+
+ # NOTE: I (TP) commented out the RemoveField operation since it was giving
+ # an error like (1090, "You can't delete all columns with ALTER TABLE; use
+ # DROP TABLE instead"). There are a few issues about this:
+ # https://code.djangoproject.com/ticket/27746
+ # https://code.djangoproject.com/ticket/24424
+ # It looks like it may be fixed in Django 2.2.2, so we can test it out once
+ # we get to that version.
+ operations = [
+ #migrations.RemoveField(
+ # model_name='robotuser',
+ # name='user_ptr',
+ #),
+ migrations.DeleteModel(
+ name='RobotUser',
+ ),
+ ]
diff --git a/gracedb/ligoauth/models.py b/gracedb/ligoauth/models.py
index b88e64081..8dfad9e64 100644
--- a/gracedb/ligoauth/models.py
+++ b/gracedb/ligoauth/models.py
@@ -1,5 +1,3 @@
-from __future__ import unicode_literals
-
from django.db import models
from django.contrib.auth.models import User, Group
@@ -18,11 +16,6 @@ class LigoLdapUser(User):
return u"{0} {1}".format(self.first_name, self.last_name).encode('utf-8')
-# Class for robot accounts
-class RobotUser(User):
- pass
-
-
class X509Cert(models.Model):
"""Model for storing X.509 certificate subjects for API access"""
subject = models.CharField(max_length=255, unique=True, null=False)
diff --git a/gracedb/ligoauth/tests/test_middleware.py b/gracedb/ligoauth/tests/test_middleware.py
index ce419bfcf..b567b2c8d 100644
--- a/gracedb/ligoauth/tests/test_middleware.py
+++ b/gracedb/ligoauth/tests/test_middleware.py
@@ -8,7 +8,7 @@ from django.urls import reverse
from user_sessions.middleware import SessionMiddleware
-from ligoauth.models import RobotUser, AuthGroup
+from ligoauth.models import AuthGroup
from ligoauth.middleware import (
ControlRoomMiddleware, ShibbolethWebAuthMiddleware,
)
@@ -253,6 +253,14 @@ class TestShibbolethWebAuthMiddleware(GraceDbTestBase):
# Attach middleware to class
cls.mw_instance = ShibbolethWebAuthMiddleware()
+ @classmethod
+ def setUpTestData(cls):
+ super(TestShibbolethWebAuthMiddleware, cls).setUpTestData()
+
+ # Create robot group
+ cls.robot_group = AuthGroup.objects.create(name='robot_accounts',
+ ldap_name='robot_accounts_ldap_name')
+
def test_internal_user_authentication_post_login(self):
"""
Internal user can authenticate at post-login view with
@@ -486,9 +494,10 @@ class TestShibbolethWebAuthMiddleware(GraceDbTestBase):
"""
Shib group header content is not used to add groups for a robotuser
"""
- # Create a RobotUser and add to internal group
- r_user = RobotUser.objects.create(username='robot.user')
+ # Create a robot user account
+ r_user = User.objects.create(username='robot.user')
r_user.groups.add(self.internal_group)
+ r_user.groups.add(self.robot_group)
# Create new group for testing
new_group = AuthGroup.objects.create(name='new_group',
@@ -505,10 +514,12 @@ class TestShibbolethWebAuthMiddleware(GraceDbTestBase):
settings.SHIB_GROUPS_HEADER: groups_str,
})
- # Make sure user just has internal group initially
- self.assertEqual(r_user.groups.count(), 1)
+ # Make sure user just has internal and robot groups initially
+ self.assertEqual(r_user.groups.count(), 2)
self.assertTrue(r_user.groups.filter(
pk=self.internal_group.pk).exists())
+ self.assertTrue(r_user.groups.filter(
+ pk=self.robot_group.pk).exists())
# Necessary pre-processing middleware
SessionMiddleware().process_request(request)
@@ -522,9 +533,11 @@ class TestShibbolethWebAuthMiddleware(GraceDbTestBase):
self.assertTrue(request.user.is_authenticated)
self.assertEqual(request.user.backend,
'ligoauth.backends.ShibbolethRemoteUserBackend')
- self.assertEqual(r_user.groups.count(), 1)
+ self.assertEqual(r_user.groups.count(), 2)
self.assertTrue(r_user.groups.filter(
pk=self.internal_group.pk).exists())
+ self.assertTrue(r_user.groups.filter(
+ pk=self.robot_group.pk).exists())
self.assertFalse(r_user.groups.filter(
pk=new_group.pk).exists())
@@ -532,9 +545,10 @@ class TestShibbolethWebAuthMiddleware(GraceDbTestBase):
"""
Shib group header content is not used to remove groups for a robotuser
"""
- # Create a RobotUser and add to internal group
- r_user = RobotUser.objects.create(username='robot.user')
+ # Create a robot user account
+ r_user = User.objects.create(username='robot.user')
r_user.groups.add(self.internal_group)
+ r_user.groups.add(self.robot_group)
# Create new group and add robotuser
new_group = AuthGroup.objects.create(name='new_group',
ldap_name='new_ldap_group')
@@ -548,10 +562,12 @@ class TestShibbolethWebAuthMiddleware(GraceDbTestBase):
settings.SHIB_GROUPS_HEADER: self.internal_group.ldap_name,
})
- # Make sure user has both groups initially
- self.assertEqual(r_user.groups.count(), 2)
+ # Make sure user has three groups initially
+ self.assertEqual(r_user.groups.count(), 3)
self.assertTrue(r_user.groups.filter(
pk=self.internal_group.pk).exists())
+ self.assertTrue(r_user.groups.filter(
+ pk=self.robot_group.pk).exists())
self.assertTrue(r_user.groups.filter(
pk=new_group.pk).exists())
@@ -567,9 +583,11 @@ class TestShibbolethWebAuthMiddleware(GraceDbTestBase):
self.assertTrue(request.user.is_authenticated)
self.assertEqual(request.user.backend,
'ligoauth.backends.ShibbolethRemoteUserBackend')
- self.assertEqual(r_user.groups.count(), 2)
+ self.assertEqual(r_user.groups.count(), 3)
self.assertTrue(r_user.groups.filter(
pk=self.internal_group.pk).exists())
+ self.assertTrue(r_user.groups.filter(
+ pk=self.robot_group.pk).exists())
self.assertTrue(r_user.groups.filter(
pk=new_group.pk).exists())
--
GitLab