From 5461963cae636e6f8768758242728d70d73774ae Mon Sep 17 00:00:00 2001
From: Tanner Prestegard <tanner.prestegard@ligo.org>
Date: Mon, 3 Jun 2019 14:38:53 -0500
Subject: [PATCH] X509Cert: connect to user via ForeignKey rather than m2m
relation
An X509 certificate should only map to a single user account - we
can't do authentication properly if it maps to multiple, so this
was an obviously necessary change. There are several migrations for
making this conversion in steps.
---
gracedb/api/backends.py | 11 +----
gracedb/api/tests/test_authentication.py | 4 +-
gracedb/api/tests/test_backends.py | 4 +-
.../0041_x509cert_add_user_foreignkey.py | 28 ++++++++++++
.../0042_populate_x509cert_user_field.py | 44 +++++++++++++++++++
.../0043_x509cert_delete_users_m2m_field.py | 26 +++++++++++
gracedb/ligoauth/models.py | 2 +-
7 files changed, 104 insertions(+), 15 deletions(-)
create mode 100644 gracedb/ligoauth/migrations/0041_x509cert_add_user_foreignkey.py
create mode 100644 gracedb/ligoauth/migrations/0042_populate_x509cert_user_field.py
create mode 100644 gracedb/ligoauth/migrations/0043_x509cert_delete_users_m2m_field.py
diff --git a/gracedb/api/backends.py b/gracedb/api/backends.py
index 6a202e3cd..9274ae56d 100644
--- a/gracedb/api/backends.py
+++ b/gracedb/api/backends.py
@@ -157,17 +157,8 @@ class GraceDbX509Authentication(authentication.BaseAuthentication):
'subject'))
cert = certs.first()
- # Handle incorrect number of users for a certificate
- num_users = cert.users.count()
- if (num_users > 1):
- raise exceptions.AuthenticationFailed(_('Multiple users have the '
- 'same certificate subject'))
- elif (num_users == 0):
- raise exceptions.AuthenticationFailed(_('No user found for this '
- 'certificate'))
- user = cert.users.first()
-
# Check if user is active
+ user = cert.user
if not user.is_active:
raise exceptions.AuthenticationFailed(
_('User inactive or deleted'))
diff --git a/gracedb/api/tests/test_authentication.py b/gracedb/api/tests/test_authentication.py
index 0b8497e3f..d1f0c9f7e 100644
--- a/gracedb/api/tests/test_authentication.py
+++ b/gracedb/api/tests/test_authentication.py
@@ -107,8 +107,8 @@ class TestGraceDbX509Authentication(GraceDbApiTestBase):
# Set up certificate for internal user account
cls.x509_subject = '/x509_subject'
- cert = X509Cert.objects.create(subject=cls.x509_subject)
- cert.users.add(cls.internal_user)
+ cert = X509Cert.objects.create(subject=cls.x509_subject,
+ user=cls.internal_user)
def test_user_authenticate_to_api_with_x509_cert(self):
"""User can authenticate to API with valid X509 certificate"""
diff --git a/gracedb/api/tests/test_backends.py b/gracedb/api/tests/test_backends.py
index 6cbb390e3..e0caec2b4 100644
--- a/gracedb/api/tests/test_backends.py
+++ b/gracedb/api/tests/test_backends.py
@@ -135,8 +135,8 @@ class TestGraceDbX509Authentication(GraceDbApiTestBase):
# Set up certificate for internal user account
cls.x509_subject = '/x509_subject'
- cert = X509Cert.objects.create(subject=cls.x509_subject)
- cert.users.add(cls.internal_user)
+ cert = X509Cert.objects.create(subject=cls.x509_subject,
+ user=cls.internal_user)
def test_user_authenticate_to_api_with_x509_cert(self):
"""User can authenticate to API with valid X509 certificate"""
diff --git a/gracedb/ligoauth/migrations/0041_x509cert_add_user_foreignkey.py b/gracedb/ligoauth/migrations/0041_x509cert_add_user_foreignkey.py
new file mode 100644
index 000000000..4cadf8d80
--- /dev/null
+++ b/gracedb/ligoauth/migrations/0041_x509cert_add_user_foreignkey.py
@@ -0,0 +1,28 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.20 on 2019-06-03 18:08
+from __future__ import unicode_literals
+
+from django.conf import settings
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ migrations.swappable_dependency(settings.AUTH_USER_MODEL),
+ ('ligoauth', '0040_x509cert_unique_subject'),
+ ]
+
+ operations = [
+ migrations.AddField(
+ model_name='x509cert',
+ name='user',
+ field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL),
+ ),
+ migrations.AlterField(
+ model_name='x509cert',
+ name='users',
+ field=models.ManyToManyField(related_name='certs', to=settings.AUTH_USER_MODEL),
+ ),
+ ]
diff --git a/gracedb/ligoauth/migrations/0042_populate_x509cert_user_field.py b/gracedb/ligoauth/migrations/0042_populate_x509cert_user_field.py
new file mode 100644
index 000000000..d05b8d1d2
--- /dev/null
+++ b/gracedb/ligoauth/migrations/0042_populate_x509cert_user_field.py
@@ -0,0 +1,44 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.20 on 2019-06-03 18:09
+from __future__ import unicode_literals
+
+from django.db import migrations
+
+def populate_user_field(apps, schema_editor):
+ X509Cert = apps.get_model('ligoauth', 'X509Cert')
+
+ # Loop over all certificates, get first user, add as user ForeignKey
+ for cert in X509Cert.objects.iterator():
+ cert.user = cert.users.first()
+ cert.save(update_fields=['user'])
+
+ # Clear out users m2m field
+ # NOTE: this isn't exactly "safe" if there is more than one
+ # user associated with a certificate, but there *shouldn't* be
+ # (no examples found in production DB after fixing some errors),
+ # and even if there is, it should be cleared up after running
+ # the 'update_user_accounts_from_ligo_ldap' script once or twice.
+ cert.users.clear()
+
+
+def populate_users_field(apps, schema_editor):
+ X509Cert = apps.get_model('ligoauth', 'X509Cert')
+
+ # Loop over all certificates and add user to users m2m field
+ for cert in X509Cert.objects.iterator():
+ cert.users.add(cert.user)
+
+ # Clear out user ForeignKey
+ cert.user = None
+ cert.save(update_fields=['user'])
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ('ligoauth', '0041_x509cert_add_user_foreignkey'),
+ ]
+
+ operations = [
+ migrations.RunPython(populate_user_field, populate_users_field),
+ ]
diff --git a/gracedb/ligoauth/migrations/0043_x509cert_delete_users_m2m_field.py b/gracedb/ligoauth/migrations/0043_x509cert_delete_users_m2m_field.py
new file mode 100644
index 000000000..57e4825fe
--- /dev/null
+++ b/gracedb/ligoauth/migrations/0043_x509cert_delete_users_m2m_field.py
@@ -0,0 +1,26 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.20 on 2019-06-03 18:54
+from __future__ import unicode_literals
+
+from django.conf import settings
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ('ligoauth', '0042_populate_x509cert_user_field'),
+ ]
+
+ operations = [
+ migrations.RemoveField(
+ model_name='x509cert',
+ name='users',
+ ),
+ migrations.AlterField(
+ model_name='x509cert',
+ name='user',
+ field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL),
+ ),
+ ]
diff --git a/gracedb/ligoauth/models.py b/gracedb/ligoauth/models.py
index 1a5685448..b88e64081 100644
--- a/gracedb/ligoauth/models.py
+++ b/gracedb/ligoauth/models.py
@@ -26,7 +26,7 @@ class RobotUser(User):
class X509Cert(models.Model):
"""Model for storing X.509 certificate subjects for API access"""
subject = models.CharField(max_length=255, unique=True, null=False)
- users = models.ManyToManyField(User)
+ user = models.ForeignKey(User)
class AuthGroup(Group):
--
GitLab