permissions tweaks (#710)

[#184165426]

bundles/common/Permissions.ts

@@ -30,10 +30,8 @@ export type Permission =
   | 'tracker.add_videolink'
   | 'tracker.add_videolinktype'
   | 'tracker.add_wordfilter'
-  | 'tracker.can_change_log'
   | 'tracker.can_edit_locked_events'
-  | 'tracker.can_search'
-  | 'tracker.can_view_log'
+  | 'tracker.can_search_for_user'
   | 'tracker.can_view_tech_notes'
   | 'tracker.change_ad'
   | 'tracker.change_amountfilter'
@@ -100,7 +98,6 @@ export type Permission =
   | 'tracker.remove_prize_key_winners'
   | 'tracker.send_to_reader'
   | 'tracker.show_queries'
-  | 'tracker.show_rendertime'
   | 'tracker.top_level_bid'
   | 'tracker.view_ad'
   | 'tracker.view_ads'
@@ -117,7 +114,7 @@ export type Permission =
   | 'tracker.view_donorprizeentry'
   | 'tracker.view_emails'
   | 'tracker.view_event'
-  | 'tracker.view_full_list'
+  | 'tracker.view_full_names'
   | 'tracker.view_headset'
   | 'tracker.view_hidden_bid'
   | 'tracker.view_interstitial'
@@ -134,7 +131,6 @@ export type Permission =
   | 'tracker.view_speedrun'
   | 'tracker.view_submission'
   | 'tracker.view_test'
-  | 'tracker.view_usernames'
   | 'tracker.view_userprofile'
   | 'tracker.view_videolink'
   | 'tracker.view_videolinktype'

tests/test_auth.py

@@ -12,6 +12,7 @@
 import tracker.auth
 
 from . import util
+from .util import MigrationsTestCase
 
 AuthUser = get_user_model()
 
@@ -91,3 +92,105 @@ def test_register_active_user(self):
                 'This email is already registered. Please log in, (or reset your password if you forgot it).'
             ],
         )
+
+
+class TestPermissionsMigrationForwards(MigrationsTestCase):
+    migrate_from = [('tracker', '0041_permissions_pass')]
+    migrate_to = [('tracker', '0042_search_permissions_rename')]
+
+    def setUpBeforeMigration(self, apps):
+        User = apps.get_model('auth', 'user')
+        Group = apps.get_model('auth', 'group')
+        Permission = apps.get_model('auth', 'permission')
+
+        user = User.objects.create(username='user')
+        group = Group.objects.create(name='group')
+        old = Permission.objects.get(
+            content_type__app_label='tracker',
+            content_type__model='userprofile',
+            codename='can_search',
+        )
+        old.user_set.add(user)
+        old.group_set.add(group)
+        old = Permission.objects.get(
+            content_type__app_label='tracker',
+            content_type__model='donor',
+            codename='view_usernames',
+        )
+        old.user_set.add(user)
+        old.group_set.add(group)
+
+    def test_forwards(self):
+        from django.contrib.auth.models import Group, User
+
+        user = User.objects.get(username='user')
+        group = Group.objects.get(name='group')
+        self.permissions_helper(
+            user,
+            group,
+            {'app_label': 'tracker', 'model': 'userprofile', 'codename': 'can_search'},
+            {
+                'app_label': 'tracker',
+                'model': 'userprofile',
+                'codename': 'can_search_for_user',
+            },
+            True,
+        )
+        self.permissions_helper(
+            user,
+            group,
+            {'app_label': 'tracker', 'model': 'donor', 'codename': 'view_usernames'},
+            {'app_label': 'tracker', 'model': 'donor', 'codename': 'view_full_names'},
+            True,
+        )
+
+
+class TestPermissionsMigrationBackwards(MigrationsTestCase):
+    migrate_from = [('tracker', '0042_search_permissions_rename')]
+    migrate_to = [('tracker', '0041_permissions_pass')]
+
+    def setUpBeforeMigration(self, apps):
+        User = apps.get_model('auth', 'user')
+        Group = apps.get_model('auth', 'group')
+        Permission = apps.get_model('auth', 'permission')
+
+        user = User.objects.create(username='user')
+        group = Group.objects.create(name='group')
+        new = Permission.objects.get(
+            content_type__app_label='tracker',
+            content_type__model='userprofile',
+            codename='can_search_for_user',
+        )
+        new.user_set.add(user)
+        new.group_set.add(group)
+        new = Permission.objects.get(
+            content_type__app_label='tracker',
+            content_type__model='donor',
+            codename='view_full_names',
+        )
+        new.user_set.add(user)
+        new.group_set.add(group)
+
+    def test_backwards(self):
+        from django.contrib.auth.models import Group, User
+
+        user = User.objects.get(username='user')
+        group = Group.objects.get(name='group')
+        self.permissions_helper(
+            user,
+            group,
+            {
+                'app_label': 'tracker',
+                'model': 'userprofile',
+                'codename': 'can_search_for_user',
+            },
+            {'app_label': 'tracker', 'model': 'userprofile', 'codename': 'can_search'},
+            False,
+        )
+        self.permissions_helper(
+            user,
+            group,
+            {'app_label': 'tracker', 'model': 'donor', 'codename': 'view_full_names'},
+            {'app_label': 'tracker', 'model': 'donor', 'codename': 'view_usernames'},
+            False,
+        )

tests/test_donation.py

@@ -4,14 +4,15 @@
 from decimal import Decimal
 from unittest.mock import patch
 
+from django.contrib.admin import AdminSite
 from django.contrib.admin.helpers import ACTION_CHECKBOX_NAME
 from django.contrib.auth.models import Permission, User
-from django.test import TestCase, override_settings
+from django.test import RequestFactory, TestCase, override_settings
 from django.urls import reverse
 
-from tracker import models
+from tracker import admin, models
 
-from .util import today_noon, tomorrow_noon
+from .util import AssertionHelpers, today_noon, tomorrow_noon
 
 
 class TestDonation(TestCase):
@@ -246,8 +247,10 @@ def test_queryset(self):
         )
 
 
-class TestDonorAdmin(TestCase):
+class TestDonationAdmin(TestCase, AssertionHelpers):
     def setUp(self):
+        self.donation_admin = admin.donation.DonationAdmin(models.Donation, AdminSite())
+        self.factory = RequestFactory()
         self.super_user = User.objects.create_superuser('admin')
         self.unlocked_user = User.objects.create(username='staff', is_staff=True)
         self.unlocked_user.user_permissions.add(
@@ -312,6 +315,109 @@ def test_donation_admin(self):
                 )
                 self.assertEqual(response.status_code, 403)
 
+    def test_donation_admin_form(self):
+        # testing both get_form and get_readonly_fields, as they will not be in
+        #  base_fields if they are readonly
+
+        # url doesn't actually matter for this test, but might as well be something relatively sensible
+        request = self.factory.get(reverse('admin:tracker_donation_add'))
+
+        with self.subTest('super user'):
+            request.user = self.super_user
+
+            form = self.donation_admin.get_form(request)
+            self.assertSetDisjoint(
+                form.base_fields,
+                {'transactionstate', 'readstate', 'commentstate'},
+                msg='Fields are read only on creation',
+            )
+
+            form = self.donation_admin.get_form(request, self.donation)
+            self.assertNotIn(
+                'commentstate', form.base_fields, msg='Read only when comment is blank'
+            )
+
+            self.donation.comment = 'Not blank.'
+            self.donation.save()
+            form = self.donation_admin.get_form(request, self.donation)
+            self.assertIn(
+                'commentstate',
+                form.base_fields,
+                msg='Writeable when comment is present',
+            )
+            self.assertNotIn(
+                'ABSENT',
+                (c[0] for c in form.base_fields['commentstate'].choices),
+                msg='Absent state not selectable when comment is present',
+            )
+
+            self.assertNotIn(
+                'FLAGGED',
+                (c[0] for c in form.base_fields['readstate'].choices),
+                msg='Flagged state not selectable for one step events',
+            )
+
+        with self.subTest('normal editor'):
+            request.user = self.unlocked_user
+            form = self.donation_admin.get_form(request)
+            self.assertSetDisjoint(
+                form.base_fields,
+                {'domain', 'fee', 'testdonation'},
+                msg='Fields are read only without special permission',
+            )
+
+            form = self.donation_admin.get_form(request, self.donation)
+            self.assertNotIn(
+                'transactionstate',
+                form.base_fields,
+                msg='Field is read only without special permission',
+            )
+
+            self.donation.domain = 'PAYPAL'
+            self.donation.save()
+            form = self.donation_admin.get_form(request, self.donation)
+            self.assertSetDisjoint(
+                form.base_fields,
+                {'donor', 'event', 'timereceived', 'amount', 'currency'},
+                msg='Fields are read only for non-local donations without special permission',
+            )
+            self.assertIn(
+                'readstate', form.base_fields, msg='Field is editable when not READY'
+            )
+
+            self.event.use_one_step_screening = False
+            self.event.save()
+            self.donation.refresh_from_db()
+            self.donation.readstate = 'READY'
+            self.donation.save()
+            form = self.donation_admin.get_form(request, self.donation)
+            self.assertIn(
+                'READY',
+                (c[0] for c in form.base_fields['readstate'].choices),
+                msg='Field is already READY, so it is a valid choice for first-pass screeners (and hosts)',
+            )
+
+            self.donation.readstate = 'FLAGGED'
+            self.donation.save()
+            form = self.donation_admin.get_form(request, self.donation)
+            self.assertNotIn(
+                'READY',
+                (c[0] for c in form.base_fields['readstate'].choices),
+                msg='Cannot set to READY without permission',
+            )
+
+            request.user.user_permissions.add(
+                Permission.objects.get(codename='send_to_reader')
+            )
+            # cleanest way to reset permissions cache, refresh_from_db is not enough
+            request.user = User.objects.get(id=request.user.id)
+            form = self.donation_admin.get_form(request, self.donation)
+            self.assertIn(
+                'READY',
+                (c[0] for c in form.base_fields['readstate'].choices),
+                msg='Can set to READY with permission',
+            )
+
     @patch('tracker.tasks.post_donation_to_postbacks')
     @override_settings(TRACKER_HAS_CELERY=True)
     def test_donation_postback_with_celery(self, task):

tests/test_donor.py

@@ -3,14 +3,21 @@
 from decimal import Decimal
 from unittest import skip
 
-from django.contrib.auth.models import User
-from django.test import TestCase
+from django.contrib.admin import AdminSite
+from django.contrib.auth.models import AnonymousUser, Permission, User
+from django.test import RequestFactory, TestCase
 from django.urls import reverse
 
-from tracker import models, viewutil
+from tracker import admin, models, viewutil
 
 from . import randgen
-from .util import MigrationsTestCase, long_ago_noon, today_noon, tomorrow_noon
+from .util import (
+    AssertionHelpers,
+    MigrationsTestCase,
+    long_ago_noon,
+    today_noon,
+    tomorrow_noon,
+)
 
 
 class TestDonorTotals(TestCase):
@@ -414,19 +421,20 @@ def test_donor_with_missing_alias_num(self):
         self.assertNotEqual(donor.alias_num, None, msg='Alias number was not filled in')
 
 
-class TestDonorAdmin(TestCase):
+class TestDonorAdmin(TestCase, AssertionHelpers):
     def setUp(self):
-        self.super_user = User.objects.create_superuser(
-            'admin', 'admin@example.com', 'password'
-        )
+        self.admin = admin.donation.DonorAdmin(models.Donor, AdminSite())
+        self.factory = RequestFactory()
+        self.super_user = User.objects.create_superuser('admin', 'admin@example.com')
+        self.limited_user = User.objects.create(username='staff')
         self.event = models.Event.objects.create(
             short='ev1', name='Event 1', targetamount=5, datetime=today_noon
         )
 
         self.donor = models.Donor.objects.create(firstname='John', lastname='Doe')
 
     def test_donor_admin(self):
-        self.client.login(username='admin', password='password')
+        self.client.force_login(self.super_user)
         response = self.client.get(reverse('admin:tracker_donor_changelist'))
         self.assertEqual(response.status_code, 200)
         response = self.client.get(reverse('admin:tracker_donor_add'))
@@ -435,3 +443,35 @@ def test_donor_admin(self):
             reverse('admin:tracker_donor_change', args=(self.donor.id,))
         )
         self.assertEqual(response.status_code, 200)
+
+    def test_search_fields(self):
+        request = self.factory.get('admin:tracker_donor_changelist')
+
+        with self.subTest('all permissions'):
+            request.user = self.super_user
+            search_fields = self.admin.get_search_fields(request)
+            self.assertSetEqual(
+                {'email', 'paypalemail', 'alias', 'firstname', 'lastname'},
+                set(search_fields),
+            )
+
+        with self.subTest('partial permissions'):
+            request.user = self.limited_user
+            self.limited_user.user_permissions.add(
+                Permission.objects.get(codename='view_emails')
+            )
+            search_fields = self.admin.get_search_fields(request)
+            self.assertSetEqual({'email', 'paypalemail', 'alias'}, set(search_fields))
+
+            self.limited_user.user_permissions.set(
+                [Permission.objects.get(codename='view_full_names')]
+            )
+            self.limited_user = User.objects.get(id=self.limited_user.id)
+            request.user = self.limited_user
+            search_fields = self.admin.get_search_fields(request)
+            self.assertSetEqual({'firstname', 'lastname', 'alias'}, set(search_fields))
+
+        with self.subTest('no special permissions'):
+            request.user = AnonymousUser()
+            search_fields = self.admin.get_search_fields(request)
+            self.assertSetEqual({'alias'}, set(search_fields))