feat: grant staff access to superusers automatically

common/djangoapps/student/migrations/0050_grant_staff_to_existing_superusers.py

@@ -0,0 +1,38 @@
+"""
+Backfill ``is_staff`` for existing superusers.
+
+A new ``pre_save`` signal keeps ``is_staff`` in sync with ``is_superuser`` for
+any user saved from now on, but that does not touch superusers that already
+exist in the database. This one-time data migration grants staff access to
+those accounts so the behaviour is consistent for old and new superusers alike.
+"""
+
+from django.conf import settings
+from django.db import migrations
+
+
+def grant_staff_to_superusers(apps, schema_editor):
+    """Mark every existing superuser as staff."""
+    User = apps.get_model(*settings.AUTH_USER_MODEL.split('.', 1))
+    User.objects.filter(is_superuser=True, is_staff=False).update(is_staff=True)
+
+
+def noop(apps, schema_editor):
+    """
+    Reverse is a no-op.
+
+    We cannot know which superusers were intentionally non-staff before this
+    migration ran, so removing ``is_staff`` on reverse would be unsafe.
+    """
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('student', '0049_manualenrollmentaudit_statetransition_typo'),
+        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
+    ]
+
+    operations = [
+        migrations.RunPython(grant_staff_to_superusers, noop),
+    ]

common/djangoapps/student/signals/receivers.py

@@ -66,6 +66,29 @@ def on_user_updated(sender, instance, **kwargs):
             )
 
 
+@receiver(pre_save, sender=get_user_model())
+def grant_staff_access_to_superusers(sender, instance, **kwargs):
+    """
+    Keep Django's ``is_staff`` flag in sync with ``is_superuser``.
+
+    A superuser bypasses Django's permission checks and can grant itself the
+    ``is_staff`` flag at any time, so the state where a user is a superuser but
+    not staff is confusing and offers no real protection: it just hides the
+    Django admin and various staff-only Studio/LMS views from that account for
+    no good reason. To avoid that surprise, ensure every superuser is also
+    marked as staff.
+
+    See:
+    https://discuss.openedx.org/t/shouldnt-superuser-automatically-inherit-staff-access-in-open-edx/18657
+    """
+    # Don't interfere with objects being loaded verbatim from a fixture.
+    if kwargs.get('raw'):
+        return
+
+    if instance.is_superuser and not instance.is_staff:
+        instance.is_staff = True
+
+
 @receiver(post_save, sender=CourseEnrollment)
 def create_course_enrollment_celebration(sender, instance, created, **kwargs):
     """

common/djangoapps/student/tests/test_receivers.py

@@ -3,6 +3,8 @@
 from unittest import skipUnless
 from unittest.mock import patch
 
+from django.contrib.auth import get_user_model
+from django.test import TestCase
 from edx_toggles.toggles.testutils import override_waffle_flag
 
 from common.djangoapps.student.models import CourseEnrollmentCelebration, PendingNameChange, UserProfile
@@ -92,3 +94,43 @@ def test_listen_for_user_email_changed(self, mock_get_email_client):
 
         assert mock_get_email_client.called
         assert request.session.get('email', None) == user.email
+
+
+class GrantStaffToSuperusersTest(TestCase):
+    """
+    Tests for the ``grant_staff_access_to_superusers`` pre_save receiver, which
+    keeps ``is_staff`` in sync with ``is_superuser``.
+    """
+
+    def test_new_superuser_is_marked_staff(self):
+        """ A user created as a superuser is automatically marked as staff. """
+        user = get_user_model().objects.create(
+            username='super', email='super@test.com', is_superuser=True,
+        )
+        user.refresh_from_db()
+        assert user.is_superuser is True
+        assert user.is_staff is True
+
+    def test_promoting_to_superuser_grants_staff(self):
+        """ Promoting an existing non-staff user to superuser grants staff access. """
+        user = UserFactory(is_superuser=False, is_staff=False)
+        assert user.is_staff is False
+
+        user.is_superuser = True
+        user.save()
+
+        user.refresh_from_db()
+        assert user.is_staff is True
+
+    def test_non_superuser_is_not_forced_to_staff(self):
+        """ A regular, non-superuser user keeps its (non-)staff status untouched. """
+        user = UserFactory(is_superuser=False, is_staff=False)
+        user.refresh_from_db()
+        assert user.is_staff is False
+
+    def test_existing_staff_superuser_unchanged(self):
+        """ A superuser that is already staff stays staff. """
+        user = UserFactory(is_superuser=True, is_staff=True)
+        user.save()
+        user.refresh_from_db()
+        assert user.is_staff is True