Revamped permissions around editors (bug 685382)

The following permissions are being migrated: Editors.* -> Addons:Review Editors:Apps -> Apps:Review Editors:ReviewPersonas -> Personas:Review
2012-02-13 16:23:16 -08:00 · 2012-02-13 16:23:16 -08:00 · cf5c146d88
--- a/apps/access/acl.py
+++ b/apps/access/acl.py
@ -110,3 +110,9 @@ def check_addon_ownership(request, addon, viewer=False, dev=False,
        roles += (amo.AUTHOR_ROLE_SUPPORT,)
    return addon.authors.filter(user=request.amo_user,
                                addonuser__role__in=roles).exists()
+
+
+def check_reviewer(request):
+    return (action_allowed(request, 'Addons', 'Review') or
+            action_allowed(request, 'Apps', 'Review') or
+            action_allowed(request, 'Personas', 'Review'))
--- a/apps/access/tests.py
+++ b/apps/access/tests.py
@ -42,6 +42,9 @@ def test_match_rules():
    rules = ('Doctors:*',
        'Stats:View',
        'CollectionStats:View',
+        'Addons:Review',
+        'Apps:Review',
+        'Personas:Review',
        )

    for rule in rules:
--- a/apps/amo/context_processors.py
+++ b/apps/amo/context_processors.py
@ -111,7 +111,7 @@ def global_settings(request):
            tools_links.append({'text': _('Developer Hub'),
                                'href': reverse('devhub.index')})

-        if acl.action_allowed(request, 'Editors', '%'):
+        if acl.check_reviewer(request):
            tools_links.append({'text': _('Editor Tools'),
                                'href': reverse('editors.home')})
        if acl.action_allowed(request, 'Localizers', '%'):
--- a/apps/amo/fixtures/base/users.json
+++ b/apps/amo/fixtures/base/users.json
@ -251,12 +251,32 @@
        "pk": 3,
        "model": "access.group",
        "fields": {
-            "rules": "*:*",
+            "rules": "API.Users:View",
            "modified": "2012-01-27 13:32:08",
            "name": "API.Users",
            "created": "2012-01-27 12:41:00"
        }
    },
+    {
+        "pk": 50002,
+        "model": "access.group",
+        "fields": {
+            "rules": "Addons:Review",
+            "modified": "2012-01-27 13:32:08",
+            "name": "Add-on Reviewers",
+            "created": "2012-01-27 12:41:00"
+        }
+    },
+    {
+        "pk": 50003,
+        "model": "access.group",
+        "fields": {
+            "rules": "Apps:Review",
+            "modified": "2012-01-27 13:32:08",
+            "name": "App Reviewers",
+            "created": "2012-01-27 12:41:00"
+        }
+    },
    {
        "pk": 10,
        "model": "access.groupuser",
@ -274,10 +294,18 @@
        }
    },
    {
-        "pk": 258,
+        "pk": 259,
        "model": "access.groupuser",
        "fields": {
-            "group": 2,
+            "group": 50002,
+            "user": 5497308
+        }
+    },
+    {
+        "pk": 260,
+        "model": "access.groupuser",
+        "fields": {
+            "group": 50003,
            "user": 5497308
        }
    }
--- a/apps/devhub/decorators.py
+++ b/apps/devhub/decorators.py
@ -25,7 +25,7 @@ def dev_required(owner_for_post=False, allow_editors=False, webapp=False):
            fun = lambda: f(request, addon_id=addon.id, addon=addon,
                            *args, **kw)
            if allow_editors:
-                if acl.action_allowed(request, 'Editors', '%'):
+                if acl.check_reviewer(request):
                    return fun()
            # Require an owner or dev for POST requests.
            if request.method == 'POST':
--- a/apps/devhub/tests/test_views_validation.py
+++ b/apps/devhub/tests/test_views_validation.py
@ -450,6 +450,7 @@ class TestCompatibilityResults(amo.tests.TestCase):
        r = self.client.post(reverse('devhub.bulk_compat_result',
                                     args=[self.addon.slug, self.result.id]),
                             follow=True)
+        eq_(r.status_code, 200)
        doc = pq(r.content)
        assert doc('time').text()
        eq_(doc('table tr td:eq(1)').text(), 'Firefox 4.0.*')
--- a/apps/editors/tests/test_views.py
+++ b/apps/editors/tests/test_views.py
@ -1397,7 +1397,7 @@ class ReviewBase(QueueTest):

 class TestReview(ReviewBase):

-    def test_editor_required(self):
+    def test_reviewer_required(self):
        eq_(self.client.head(self.url).status_code, 200)

    def test_not_anonymous(self):
--- a/apps/editors/views.py
+++ b/apps/editors/views.py
@ -22,7 +22,6 @@ from addons.models import Addon, Version
 from amo.decorators import (login_required, json_view, post_required,
                            permission_required)
 from amo.utils import paginate
-from amo.utils import urlencode
 from amo.urlresolvers import reverse
 from devhub.models import ActivityLog
 from editors import forms
@ -40,12 +39,23 @@ from webapps.models import Webapp
 from zadmin.models import get_config, set_config


-def editor_required(func):
-    """Requires the user to be logged in as an editor or admin."""
+def reviewer_required(func):
+    """Requires the user to be logged in as a reviewer or admin.
+
+    Reviewer is someone who is in one of the groups with the following
+    permissions:
+
+        Addons:Review
+        Apps:Review
+        Personas:Review
+
+    For finer grained access control, use one of the above rather than this
+    decorator.
+    """
    @functools.wraps(func)
    @login_required
    def wrapper(request, *args, **kw):
-        if acl.action_allowed(request, 'Editors', '%'):
+        if acl.check_reviewer(request):
            return func(request, *args, **kw)
        else:
            return http.HttpResponseForbidden()
@ -59,7 +69,7 @@ def context(**kw):
    return ctx


-@editor_required
+@reviewer_required
 def eventlog(request):
    form = forms.EventLogForm(request.GET)
    eventlog = ActivityLog.objects.editor_events()
@ -78,14 +88,14 @@ def eventlog(request):
    return jingo.render(request, 'editors/eventlog.html', data)


-@editor_required
+@reviewer_required
 def eventlog_detail(request, id):
    log = get_object_or_404(ActivityLog.objects.editor_events(), pk=id)
    data = context(log=log)
    return jingo.render(request, 'editors/eventlog_detail.html', data)


-@editor_required
+@reviewer_required
 def home(request):
    durations = (('new', _('New Add-ons (Under 5 days)')),
                 ('med', _('Passable (5 to 10 days)')),
@ -126,7 +136,7 @@ def _editor_progress():
    return (progress, percentage)


-@editor_required
+@reviewer_required
 def performance(request, user_id=False):
    user = request.amo_user
    editors = _recent_editors()
@ -224,7 +234,7 @@ def _performance_by_month(user_id, months=12, end_month=None, end_year=None):
    return monthly_data


-@editor_required
+@reviewer_required
 def motd(request):
    form = None
    if acl.action_allowed(request, 'Admin', 'EditorsMOTD'):
@ -233,7 +243,7 @@ def motd(request):
    return jingo.render(request, 'editors/motd.html', data)


-@editor_required
+@reviewer_required
@post_required
 def save_motd(request):
    if not acl.action_allowed(request, 'Admin', 'EditorsMOTD'):
@ -320,32 +330,32 @@ def queue_counts(type=None, **kw):
    return rv


-@editor_required
+@reviewer_required
 def queue(request):
    return redirect(reverse('editors.queue_pending'))


-@editor_required
+@reviewer_required
 def queue_nominated(request):
    return _queue(request, ViewFullReviewQueueTable, 'nominated')


-@editor_required
+@reviewer_required
 def queue_pending(request):
    return _queue(request, ViewPendingQueueTable, 'pending')


-@editor_required
+@reviewer_required
 def queue_prelim(request):
    return _queue(request, ViewPreliminaryQueueTable, 'prelim')


-@editor_required
+@reviewer_required
 def queue_fast_track(request):
    return _queue(request, ViewFastTrackQueueTable, 'fast_track')


-@editor_required
+@reviewer_required
 def queue_moderated(request):
    rf = (Review.objects.filter(editorreview=1, reviewflag__isnull=False,
                                addon__isnull=False)
@ -368,13 +378,13 @@ def queue_moderated(request):
                                search_form=None))


-@permission_required('Editors', 'Apps')
+@permission_required('Apps', 'Review')
 def queue_apps(request):
    qs = Webapp.objects.pending().annotate(Count('abuse_reports'))
    return _queue(request, WebappQueueTable, 'apps', qs=qs)


-@editor_required
+@reviewer_required
@post_required
@json_view
 def application_versions_json(request):
@ -383,13 +393,13 @@ def application_versions_json(request):
    return {'choices': f.version_choices_for_app_id(app_id)}


-@editor_required
+@reviewer_required
@addon_view
 def review(request, addon):
    return _review(request, addon)


-@permission_required('Editors', 'Apps')
+@permission_required('Apps', 'Review')
@addon_view
 def app_review(request, addon):
    return _review(request, addon)
@ -477,7 +487,7 @@ def _review(request, addon):

@never_cache
@json_view
-@editor_required
+@reviewer_required
 def review_viewing(request):
    if 'addon_id' not in request.POST:
        return {}
@ -509,7 +519,7 @@ def review_viewing(request):

@never_cache
@json_view
-@editor_required
+@reviewer_required
 def queue_viewing(request):
    if 'addon_ids' not in request.POST:
        return {}
@ -530,7 +540,7 @@ def queue_viewing(request):


@json_view
-@editor_required
+@reviewer_required
 def queue_version_notes(request, addon_id):
    addon = get_object_or_404(Addon, pk=addon_id)
    version = addon.latest_version
@ -538,7 +548,7 @@ def queue_version_notes(request, addon_id):
            'approvalnotes': version.approvalnotes}


-@editor_required
+@reviewer_required
 def reviewlog(request):
    data = request.GET.copy()

@ -578,7 +588,7 @@ def reviewlog(request):
    return jingo.render(request, 'editors/reviewlog.html', data)


-@editor_required
+@reviewer_required
@addon_view
 def abuse_reports(request, addon):
    reports = AbuseReport.objects.filter(addon=addon).order_by('-created')
--- a/apps/files/decorators.py
+++ b/apps/files/decorators.py
@ -10,7 +10,7 @@ from django.utils.http import http_date

 import amo
 from amo.utils import Token
-from access.acl import check_addon_ownership, action_allowed
+from access import acl
 from files.helpers import DiffHelper, FileViewer
 from files.models import File

@ -19,7 +19,7 @@ log = commonware.log.getLogger('z.addons')


 def allowed(request, file):
-    allowed = action_allowed(request, 'Editors', '%')
+    allowed = acl.check_reviewer(request)
    if not allowed:
        try:
            addon = file.version.addon
@ -29,8 +29,8 @@ def allowed(request, file):
        if addon.view_source and addon.status in amo.REVIEWED_STATUSES:
            allowed = True
        else:
-            allowed = check_addon_ownership(request, addon,
-                                            viewer=True, dev=True)
+            allowed = acl.check_addon_ownership(request, addon, viewer=True,
+                                                dev=True)
    if not allowed:
        return http.HttpResponseForbidden()
    return True
--- a/apps/files/views.py
+++ b/apps/files/views.py
@ -32,14 +32,14 @@ def setup_viewer(request, file_obj):
            'validate_url': ''}


-    if (acl.action_allowed(request, 'Editors', '%') or
+    if (acl.check_reviewer(request) or
        acl.check_addon_ownership(request, file_obj.version.addon,
                                  viewer=True, ignore_disabled=True)):
        data['validate_url'] = reverse('devhub.json_file_validation',
                                       args=[file_obj.version.addon.slug,
                                             file_obj.id])

-    if acl.action_allowed(request, 'Editors', '%'):
+    if acl.check_reviewer(request):
        data['file_link'] = {'text': _('Back to review'),
                             'url': reverse('editors.review',
                                            args=[data['addon'].slug])}
--- a/apps/reviews/views.py
+++ b/apps/reviews/views.py
@ -11,7 +11,6 @@ import amo
 from amo import messages
 from amo.decorators import json_view, login_required, post_required
 from amo.helpers import absolutify, shared_url
-from amo.urlresolvers import reverse
 import amo.utils
 from access import acl
 from addons.decorators import addon_view_factory, has_purchased
@ -66,7 +65,7 @@ def review_list(request, addon, review_id=None, user_id=None, template=None):
    if request.user.is_authenticated():
        ctx['review_perms'] = {
            'is_admin': acl.action_allowed(request, 'Admin', 'EditAnyAddon'),
-            'is_editor': acl.action_allowed(request, 'Editor', '%'),
+            'is_editor': acl.check_reviewer(request),
            'is_author': acl.check_addon_ownership(request, addon, dev=True),
            'can_delete': acl.action_allowed(request, 'Editors',
                                             'DeleteReview'),
--- a/apps/users/models.py
+++ b/apps/users/models.py
@ -205,9 +205,11 @@ class UserProfile(amo.models.OnChangeMixin, amo.models.ModelBase):

    @amo.cached_property
    def needs_tougher_password(user):
-        from access.acl import action_allowed_user
-        return (action_allowed_user(user, 'Editors', '%')
-                or action_allowed_user(user, 'Admin', '%'))
+        from access import acl
+        return (acl.action_allowed_user(user, 'Admin', '%') or
+                acl.action_allowed_user(user, 'Addons', 'Review') or
+                acl.action_allowed_user(user, 'Apps', 'Review') or
+                acl.action_allowed_user(user, 'Personas', 'Review'))

    @property
    def name(self):
--- a/migrations/310-perms-reviewers.sql
+++ b/migrations/310-perms-reviewers.sql
@ -0,0 +1,16 @@
+INSERT INTO groups VALUES (50002, 'Add-on Reviewers', 'Addons:Review', NOW(), NOW());
+INSERT INTO groups VALUES (50003, 'App Reviewers', 'Apps:Review', NOW(), NOW());
+INSERT INTO groups VALUES (50004, 'Persona Reviewers', 'Personas:Review', NOW(), NOW());
+-- We will append to this group as we edit more permissions.
+INSERT INTO groups VALUES (50005, 'Senior Add-on Reviewers', 'Addons:Review', NOW(), NOW());
+
+
+INSERT INTO groups_users (
+	SELECT NULL, 50002, groups_users.user_id FROM groups, groups_users
+	WHERE groups.id=groups_users.group_id AND groups.name='Editors' AND groups.id < 50000);
+INSERT INTO groups_users (
+	SELECT NULL, 50004, groups_users.user_id FROM groups, groups_users
+	WHERE groups.id=groups_users.group_id AND groups.name='Persona Reviewer' AND groups.id < 50000);
+INSERT INTO groups_users (
+	SELECT NULL, 50005, groups_users.user_id FROM groups, groups_users
+	WHERE groups.id=groups_users.group_id AND groups.name='Senior Editors' AND groups.id < 50000);