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 grants ``is_staff`` to any superuser 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,33 @@ def on_user_updated(sender, instance, **kwargs):
             )
 
 
+@receiver(pre_save, sender=get_user_model())
+def grant_staff_access_to_superusers(sender, instance, **kwargs):
+    """
+    Grant Django's ``is_staff`` flag to any superuser being saved.
+
+    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.
+
+    This is intentionally one-directional (grant-only): it only ever adds
+    ``is_staff`` to superusers and never removes it, so demoting a superuser
+    leaves ``is_staff`` untouched.
+
+    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,59 @@ 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
+    grants ``is_staff`` to any superuser. The behaviour is grant-only: it never
+    removes ``is_staff`` when a user stops being a 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):
+        """ Saving a regular, non-superuser user leaves its non-staff status untouched. """
+        user = UserFactory(is_superuser=False, is_staff=False)
+
+        # Explicitly re-save to exercise the pre_save receiver on an update.
+        user.save()
+
+        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
+
+    def test_demotion_does_not_remove_staff(self):
+        """ Dropping ``is_superuser`` is grant-only, so ``is_staff`` is left in place. """
+        user = UserFactory(is_superuser=True, is_staff=True)
+
+        user.is_superuser = False
+        user.save()
+
+        user.refresh_from_db()
+        assert user.is_superuser is False
+        assert user.is_staff is True