Add permissions to the reviewer tools (bug 796171)

2013-02-11 19:01:57 -08:00 · 2013-02-11 19:01:57 -08:00 · f2eb621eae
--- a/media/css/mkt/reviewers.less
+++ b/media/css/mkt/reviewers.less
@ -1124,6 +1124,7 @@ span.currently_viewing_warning {
 }

 #manifest-headers,
+#manifest-permissions,
 #manifest-contents {
    .border-box;
    color: @dark-gray;
@ -1136,7 +1137,8 @@ span.currently_viewing_warning {
    width: 100%;
 }

-#manifest-headers {
+#manifest-headers,
+#manifest-permissions {
    background-color: @faint-gray;
    border: @dark-gray 2px solid;
    padding: 5px 10px;
--- a/media/js/mkt/reviewers.js
+++ b/media/js/mkt/reviewers.js
@ -1,13 +1,13 @@
 (function() {

    if (z.capabilities.mobile) {
-        $('body').addClass('mobile');
+        z.body.addClass('mobile');
    }
    if (z.capabilities.tablet) {
-        $('body').addClass('tablet');
+        z.body.addClass('tablet');
    }
    if (z.capabilities.desktop) {
-        $('body').addClass('desktop');
+        z.body.addClass('desktop');
    }

    // Touch-friendly drop-downs for auxillary nav.
@ -26,42 +26,64 @@
    }

    // Prefetch manifest.
-    var manifestContents;
-    setTimeout(function() {
-        $.getJSON($viewManifest.data('url'), function(data) {
-            manifestContents = data;
+    $.getJSON($viewManifest.data('url'), function(data) {
+        var manifestContents = data;

-            // Show manifest.
-            $viewManifest.click(_pd(function() {
-                var $this = $viewManifest,
-                    $manifest = $('#manifest-headers, #manifest-contents');
-                if ($manifest.length) {
-                    $manifest.toggle();
+        // Show manifest.
+        $viewManifest.click(_pd(function() {
+            var $this = $viewManifest,
+                $manifest = $('#manifest-headers, #manifest-contents');
+            if ($manifest.length) {
+                $manifest.toggle();
+            } else {
+                if (!manifestContents.success) {
+                    // If requests couldn't fetch the manifest, let Firefox render it.
+                    $('<iframe>', {'id': 'manifest-contents',
+                                   'src': 'view-source:' + $this.data('manifest')}).insertAfter($this);
                } else {
-                    if (!manifestContents.success) {
-                        // If requests couldn't fetch the manifest, let Firefox render it.
-                        $('<iframe>', {'id': 'manifest-contents',
-                                       'src': 'view-source:' + $this.data('manifest')}).insertAfter($this);
-                    } else {
-                        var contents = '',
-                            headers = '';
+                    var contents = '',
+                        headers = '';

-                        _.each(manifestContents.content.split('\n'), function(v, k) {
-                            if (v) {
-                                contents += format('<li>{0}</li>', v);
-                            }
-                        });
-                        $('<ol>', {'id': 'manifest-contents', 'html': contents}).insertAfter($this);
-
-                        if (manifestContents.headers) {
-                            _.each(manifestContents.headers, function(v, k) {
-                                headers += format('<li><b>{0}:</b> {1}</li>', k, v);
-                            });
-                            $('<ol>', {'id': 'manifest-headers', 'html': headers}).insertAfter($this);
+                    _.each(manifestContents.content.split('\n'), function(v, k) {
+                        if (v) {
+                            contents += format('<li>{0}</li>', v);
                        }
+                    });
+                    $('<ol>', {'id': 'manifest-contents', 'html': contents}).insertAfter($this);
+
+                    if (manifestContents.headers) {
+                        _.each(manifestContents.headers, function(v, k) {
+                            headers += format('<li><b>{0}:</b> {1}</li>', k, v);
+                        });
+                        $('<ol>', {'id': 'manifest-headers', 'html': headers}).insertAfter($this);
+                    }
+
+                    if (manifestContents.permissions) {
+                        var permissions = format('<h4>{0}</h4><dl>', gettext('Requested Permissions:'));
+                        permissions += _.map(
+                            manifestContents.permissions,
+                            function(details, permission) {
+                                var type;
+                                if (details.type) {
+                                    switch (details.type) {
+                                        case 'cert':
+                                            type = gettext('Certified');
+                                            break;
+                                        case 'priv':
+                                            type = gettext('Privileged');
+                                            break;
+                                        case 'web':
+                                            type = gettext('Unprivileged');
+                                    }
+                                }
+                                return format('<dt>{0}</dt><dd>{1}, {2}</dd>',
+                                              permission, type, details.description || gettext('No reason given'));
+                            }
+                        ).join('');
+                        $('<div>', {'id': 'manifest-permissions', 'html': permissions}).insertAfter($this);
                    }
                }
-            }));
-        });
-    }, 200);
+            }
+        }));
+    });
 })();
--- a/mkt/reviewers/tests/test_views.py
+++ b/mkt/reviewers/tests/test_views.py
@ -1290,7 +1290,8 @@ class TestReviewApp(AppReviewerTest, AccessMixin, PackagedFilesMixin):
            'content': 'the manifest contents &lt;script&gt;',
            'headers': {'content-type':
                        'application/x-web-app-manifest+json &lt;script&gt;'},
-            'success': True
+            'success': True,
+            'permissions': {}
        }

        r = self.client.get(reverse('reviewers.apps.review.manifest',
@ -1309,7 +1310,8 @@ class TestReviewApp(AppReviewerTest, AccessMixin, PackagedFilesMixin):
                                    args=[self.app.app_slug]))
        eq_(r.status_code, 200)
        eq_(json.loads(r.content), {'content': u'كك some foreign ish',
-                                    'headers': {}, 'success': True})
+                                    'headers': {}, 'success': True,
+                                    'permissions': {}})

    @mock.patch('mkt.reviewers.views.requests.get')
    def test_manifest_json_encoding(self, mock_get):
@ -1336,7 +1338,7 @@ class TestReviewApp(AppReviewerTest, AccessMixin, PackagedFilesMixin):
                                    args=[self.app.app_slug]))
        eq_(r.status_code, 200)
        eq_(json.loads(r.content), {'content': u'', 'headers': {},
-                                    'success': True})
+                                    'success': True, 'permissions': {}})

    @mock.patch('mkt.reviewers.views.requests.get')
    def test_manifest_json_traceback_in_response(self, mock_get):
@ -1366,6 +1368,27 @@ class TestReviewApp(AppReviewerTest, AccessMixin, PackagedFilesMixin):
        eq_(res.status_code, 200)
        assert mock_.called

+    @mock.patch('mkt.reviewers.views.requests.get')
+    def test_manifest_json_perms(self, mock_get):
+        m = mock.Mock()
+        m.content = """
+        {"permissions":
+            {"foo": {"description": "foo"},
+             "camera": {"description": "<script>"}
+            }
+        }
+        """
+        m.headers = {'content-type':
+                     'application/x-web-app-manifest+json <script>'}
+        mock_get.return_value = m
+
+        r = self.client.get(reverse('reviewers.apps.review.manifest',
+                                    args=[self.app.app_slug]))
+        eq_(r.status_code, 200)
+        eq_(json.loads(r.content)['permissions'],
+            {'foo': {'description': 'foo', 'type': 'web'},
+             'camera': {'description': '&lt;script&gt;', 'type': 'cert'}})
+
    def test_abuse(self):
        AbuseReport.objects.create(addon=self.app, message='!@#$')
        res = self.client.get(self.url)
--- a/mkt/reviewers/views.py
+++ b/mkt/reviewers/views.py
@ -549,18 +549,56 @@ def motd(request):
    return jingo.render(request, 'reviewers/motd.html', data)


+# TODO: Move these to the validator when they live there someday.
+PRIVILEGED_PERMISSIONS = set([
+    'tcp-socket', 'contacts', 'device-storage:pictures',
+    'device-storage:videos', 'device-storage:music', 'device-storage:sdcard',
+    'browser', 'systemXHR', 'audio-channel-notification',
+    'audio-channel-alarm'])
+CERTIFIED_PERMISSIONS = set([
+    'camera', 'tcp-socket', 'network-events', 'contacts',
+    'device-storage:apps', 'device-storage:pictures',
+    'device-storage:videos', 'device-storage:music', 'device-storage:sdcard',
+    'sms', 'telephony', 'browser', 'bluetooth', 'mobileconnection', 'power',
+    'settings', 'permissions', 'attention', 'webapps-manage',
+    'backgroundservice', 'networkstats-manage', 'wifi-manage', 'systemXHR',
+    'voicemail', 'deprecated-hwvideo', 'idle', 'time', 'embed-apps',
+    'background-sensors', 'cellbroadcast', 'audio-channel-notification',
+    'audio-channel-alarm', 'audio-channel-telephony', 'audio-channel-ringer',
+    'audio-channel-publicnotification', 'open-remote-window'])
+
+def _get_permissions(manifest):
+    if 'permissions' not in manifest:
+        return {}
+
+    permissions = {}
+    for perm in manifest['permissions'].keys():
+        pval = permissions[perm] = {'type': 'web'}
+        if perm in PRIVILEGED_PERMISSIONS:
+            pval['type'] = 'priv'
+        elif perm in CERTIFIED_PERMISSIONS:
+            pval['type'] = 'cert'
+
+        pval['description'] = manifest['permissions'][perm].get('description')
+
+    return permissions
+
+
@permission_required('Apps', 'Review')
@addon_view
@json_view
 def app_view_manifest(request, addon):
+    manifest = {}
+    success = False
+    headers = ''
    if addon.is_packaged:
        version = addon.versions.latest()
-        content = json.dumps(json.loads(_mini_manifest(addon, version.id)),
-                             indent=4)
-        return escape_all({'content': content, 'headers': '', 'success': True})
+        manifest = json.loads(_mini_manifest(addon, version.id))
+        content = json.dumps(manifest, indent=4)
+        success = True

    else:  # Show the hosted manifest_url.
-        content, headers, success = u'', {}, False
+        content, headers = u'', {}
        if addon.manifest_url:
            try:
                req = requests.get(addon.manifest_url, verify=False)
@ -568,16 +606,21 @@ def app_view_manifest(request, addon):
                success = True
            except Exception:
                content = u''.join(traceback.format_exception(*sys.exc_info()))
+            else:
+                success = True

            try:
                # Reindent the JSON.
-                content = json.dumps(json.loads(content), indent=4)
+                manifest = json.loads(content)
+                content = json.dumps(manifest, indent=4)
            except:
                # If it's not valid JSON, just return the content as is.
                pass
-        return escape_all({'content': smart_decode(content),
-                           'headers': headers,
-                           'success': success})
+
+    return escape_all({'content': smart_decode(content),
+                       'headers': headers,
+                       'success': success,
+                       'permissions': _get_permissions(manifest)})


@permission_required('Apps', 'Review')