1178810 - ensure that only OpenMobile Application Compatibility Layer apps have "external-app" permission

Status: VERIFIED FIXED

(Marketplace Graveyard :: Security, defect, P2)

Description

[Bill Walker [:bwalker] [@wfwalker]]

In bug 1169472, we ensure that ACL-enabled apps must have the "external-app" permission in their app manifest. We must also ensure that only ACL-enabled apps have that permission.

Comment 1

[Andrew Williamson [:eviljeff]]

We do this for some moz- permissions by auto-escalating them to the escalation queue.  We can possibly adapt that to do this for external-app permission also. (It would ideally be moz-external-app if its non intended for 3rd party use, imo.)

Comment 2

[Mathieu Pillard [:mat]]

Auto escalating sounds like the right thing to do. It's a little too late to change the name IMHO, and it's not technically reserved to moz, so I vote for leaving it as is. https://github.com/mozilla/zamboni/pull/3202

Comment 3

[Mathieu Pillard [:mat]]

Fixed in https://github.com/mozilla/zamboni/commit/9cea88775459d9138b85818566b754c879eb6388

STR:
- Submit an app using the "external-app" permission in the manifest
- Make sure that once submitted, in reviewer tools it's in the Escalation Queue and not the regular one.

Comment 4

[Madalin Cotetiu]

Verified in FF43(Win7) in marketplace-dev and -stage
I have uploaded an app with the following manifest: 
{
    "name": "{appname}",
    "description": "This app has been automatically generated by 8637e7ac.testmanifest.com",
    "version": "1.0",
    "icons": {
        "16": "http://8637e7ac.testmanifest.com/icon-16.png",
        "48": "http://8637e7ac.testmanifest.com/icon-48.png",
        "128": "http://8637e7ac.testmanifest.com/icon-128.png"
    },
    "install_allowed_from": [
        "*"
    ],
    "developer": {
        "name": "Test Manifest User",
        "url": "http://testmanifest.com"
    },
    "permissions": {
        "contacts": {
            "description": "Required for autocompletion in the share screen",
            "access": "readcreate"
        },
        "external-app": {
            "description": "test permission"
        }
    }
}

An the app went to the apps queue not to the escalation queue.
I'm not sure I that is the correct way to add the "external-app" permission to the manifest. 
Mat, what do you think? That's the correct way to add it?

Comment 5

[Mathieu Pillard [:mat]]

Yep, something is wrong, reopening.

Comment 6

[Mathieu Pillard [:mat]]

Fixed in https://github.com/mozilla/app-validator/commit/4678dcea75e5114e6153240dbdf1c5dcb3340a35

Validator bumped in https://github.com/mozilla/zamboni/commit/dd96ac8e9af99785ac7c46f1a2e421956b51fd8b

Will land on -dev in ~30 minutes tops. (It won't land in stage/prod till next week)

QA: The same STR as before apply, *but* with the manifest you used, you should get an error forbidding you to use the "external-app" permission since your app is not privileged. Please check that, and then add "type": "privileged" to your manifest and follow the original STR, thanks.

Comment 7

[Madalin Cotetiu]

Verified as fixed in FF42(win7) in marketplace-allizom.org
After the file is submitted the app is in escalation queue.

When uploading an app with that permission but with "type": "privileged" missing the validation fail with an error: http://screencast.com/t/qtjMp2t3z
The message looks a bit unclear: "App requested unavailable permission" when in fact the validator is expecting for "type": "privileged" . Can we change the message for something clearer?

Comment 8

[Mathieu Pillard [:mat]]

Please file a separate bug to improve the message, as it's the generic message for missing permissions because of type.

Comment 9

[Madalin Cotetiu]

Closing bug.
Filled Bug 1192776 to change the message.