Support permission properties (maxSdkVersion and usesPermissionFlags) + remove WRITE_EXTERNAL_STORAGE permission, which has been previously declared by default (#2725)

* Support permission properties (maxSdkVersion and usesPermissionFlags) + remove WRITE_EXTERNAL_STORAGE permission, which has been previously required by default

* Fix test for ValueError

Author: misl6

Makefile

@@ -40,7 +40,8 @@ testapps-with-numpy: virtualenv
 	. $(ACTIVATE) && cd testapps/on_device_unit_tests/ && \
     python setup.py apk --sdk-dir $(ANDROID_SDK_HOME) --ndk-dir $(ANDROID_NDK_HOME) \
     --requirements libffi,sdl2,pyjnius,kivy,python3,openssl,requests,urllib3,chardet,idna,sqlite3,setuptools,numpy \
-    --arch=armeabi-v7a --arch=arm64-v8a --arch=x86_64 --arch=x86
+    --arch=armeabi-v7a --arch=arm64-v8a --arch=x86_64 --arch=x86 \
+	--permission "(name=android.permission.WRITE_EXTERNAL_STORAGE;maxSdkVersion=18)" --permission "(name=android.permission.INTERNET)"
 
 testapps-with-scipy: virtualenv
 	. $(ACTIVATE) && cd testapps/on_device_unit_tests/ && \
@@ -53,7 +54,8 @@ testapps-with-numpy-aab: virtualenv
 	. $(ACTIVATE) && cd testapps/on_device_unit_tests/ && \
     python setup.py aab --sdk-dir $(ANDROID_SDK_HOME) --ndk-dir $(ANDROID_NDK_HOME) \
     --requirements libffi,sdl2,pyjnius,kivy,python3,openssl,requests,urllib3,chardet,idna,sqlite3,setuptools,numpy \
-    --arch=armeabi-v7a --arch=arm64-v8a --arch=x86_64 --arch=x86 --release
+    --arch=armeabi-v7a --arch=arm64-v8a --arch=x86_64 --arch=x86 --release \
+	--permission "(name=android.permission.WRITE_EXTERNAL_STORAGE;maxSdkVersion=18)" --permission "(name=android.permission.INTERNET)"
 
 testapps-service_library-aar: virtualenv
 	. $(ACTIVATE) && cd testapps/on_device_unit_tests/ && \

doc/source/buildoptions.rst

@@ -64,9 +64,24 @@ options (this list may not be exhaustive):
   ``android:screenOrientation`` in the `Android documentation
   <https://developer.android.com/guide/topics/manifest/activity-element.html>`__.
 - ``--icon``: A path to the png file to use as the application icon.
-- ``--permission``: A permission name for the app,
-  e.g. ``--permission VIBRATE``. For multiple permissions, add
-  multiple ``--permission`` arguments.
+- ``--permission``: A permission that needs to be declared into the App ``AndroidManifest.xml``.
+  For multiple permissions, add multiple ``--permission`` arguments.
+
+  .. Note ::
+    ``--permission`` accepts the following syntaxes: 
+    ``--permission (name=android.permission.WRITE_EXTERNAL_STORAGE;maxSdkVersion=18)``
+    or ``--permission android.permission.WRITE_EXTERNAL_STORAGE``.
+
+    The first syntax is used to set additional properties to the permission 
+    (``android:maxSdkVersion`` and ``android:usesPermissionFlags`` are the only ones supported for now).
+
+    The second one can be used when there's no need to add any additional properties.
+
+  .. Warning ::
+    The syntax ``--permission VIBRATE`` (only the permission name, without the prefix),
+    is also supported for backward compatibility, but it will be removed in the future.
+
+
 - ``--meta-data``: Custom key=value pairs to add in the application metadata.
 - ``--presplash``: A path to the image file to use as a screen while
   the application is loading.

pythonforandroid/bootstraps/common/build/build.py

@@ -53,10 +53,6 @@ def get_bootstrap_name():
 
 curdir = dirname(__file__)
 
-PYTHON = get_hostpython()
-if PYTHON is not None and not exists(PYTHON):
-    PYTHON = None
-
 BLACKLIST_PATTERNS = [
     # code versionning
     '^*.hg/*',
@@ -75,9 +71,19 @@ def get_bootstrap_name():
 ]
 
 WHITELIST_PATTERNS = []
-if get_bootstrap_name() in ('sdl2', 'webview', 'service_only'):
-    WHITELIST_PATTERNS.append('pyconfig.h')
 
+if os.environ.get("P4A_BUILD_IS_RUNNING_UNITTESTS", "0") != "1":
+    PYTHON = get_hostpython()
+    _bootstrap_name = get_bootstrap_name()
+else:
+    PYTHON = "python3"
+    _bootstrap_name = "sdl2"
+
+if PYTHON is not None and not exists(PYTHON):
+    PYTHON = None
+
+if _bootstrap_name in ('sdl2', 'webview', 'service_only'):
+    WHITELIST_PATTERNS.append('pyconfig.h')
 
 environment = jinja2.Environment(loader=jinja2.FileSystemLoader(
     join(curdir, 'templates')))
@@ -646,6 +652,44 @@ def make_package(args):
                 subprocess.check_output(patch_command)
 
 
+def parse_permissions(args_permissions):
+    if args_permissions and isinstance(args_permissions[0], list):
+        args_permissions = [p for perm in args_permissions for p in perm]
+
+    def _is_advanced_permission(permission):
+        return permission.startswith("(") and permission.endswith(")")
+
+    def _decode_advanced_permission(permission):
+        SUPPORTED_PERMISSION_PROPERTIES = ["name", "maxSdkVersion", "usesPermissionFlags"]
+        _permission_args = permission[1:-1].split(";")
+        _permission_args = (arg.split("=") for arg in _permission_args)
+        advanced_permission = dict(_permission_args)
+
+        if "name" not in advanced_permission:
+            raise ValueError("Advanced permission must have a name property")
+
+        for key in advanced_permission.keys():
+            if key not in SUPPORTED_PERMISSION_PROPERTIES:
+                raise ValueError(
+                    f"Property '{key}' is not supported. "
+                    "Advanced permission only supports: "
+                    f"{', '.join(SUPPORTED_PERMISSION_PROPERTIES)} properties"
+                )
+
+        return advanced_permission
+
+    _permissions = []
+    for permission in args_permissions:
+        if _is_advanced_permission(permission):
+            _permissions.append(_decode_advanced_permission(permission))
+        else:
+            if "." in permission:
+                _permissions.append(dict(name=permission))
+            else:
+                _permissions.append(dict(name=f"android.permission.{permission}"))
+    return _permissions
+
+
 def parse_args_and_make_package(args=None):
     global BLACKLIST_PATTERNS, WHITELIST_PATTERNS, PYTHON
 
@@ -918,8 +962,7 @@ def _read_configuration():
               'deprecated and does nothing.')
         args.sdk_version = -1  # ensure it is not used
 
-    if args.permissions and isinstance(args.permissions[0], list):
-        args.permissions = [p for perm in args.permissions for p in perm]
+    args.permissions = parse_permissions(args.permissions)
 
     if args.res_xmls and isinstance(args.res_xmls[0], list):
         args.res_xmls = [x for res in args.res_xmls for x in res]
@@ -959,4 +1002,6 @@ def _read_configuration():
 
 
 if __name__ == "__main__":
+    if get_bootstrap_name() in ('sdl2', 'webview', 'service_only'):
+        WHITELIST_PATTERNS.append('pyconfig.h')
     parse_args_and_make_package()

pythonforandroid/bootstraps/sdl2/build/templates/AndroidManifest.tmpl.xml

@@ -24,14 +24,9 @@
     <!-- OpenGL ES 2.0 -->
     <uses-feature android:glEsVersion="0x00020000" />
 
-    <!-- Allow writing to external storage -->
-    <uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE" android:maxSdkVersion="29"/>
+    <!-- Set permissions -->
     {% for perm in args.permissions %}
-    {% if '.' in perm %}
-    <uses-permission android:name="{{ perm }}" />
-    {% else %}
-    <uses-permission android:name="android.permission.{{ perm }}" />
-    {% endif %}
+        <uses-permission android:name="{{ perm.name }}"{% if perm.maxSdkVersion %} android:maxSdkVersion="{{ perm.maxSdkVersion }}"{% endif %}{% if perm.usesPermissionFlags %} android:usesPermissionFlags="{{ perm.usesPermissionFlags }}"{% endif %} />
     {% endfor %}
 
     {% if args.wakelock %}

pythonforandroid/bootstraps/service_only/build/templates/AndroidManifest.tmpl.xml

@@ -20,11 +20,7 @@
 
     <!-- Set permissions -->
     {% for perm in args.permissions %}
-    {% if '.' in perm %}
-    <uses-permission android:name="{{ perm }}" />
-    {% else %}
-    <uses-permission android:name="android.permission.{{ perm }}" />
-    {% endif %}
+        <uses-permission android:name="{{ perm.name }}"{% if perm.maxSdkVersion %} android:maxSdkVersion="{{ perm.maxSdkVersion }}"{% endif %}{% if perm.usesPermissionFlags %} android:usesPermissionFlags="{{ perm.usesPermissionFlags }}"{% endif %} />
     {% endfor %}
 
     {% if args.wakelock %}

pythonforandroid/bootstraps/webview/build/templates/AndroidManifest.tmpl.xml

@@ -21,11 +21,7 @@
     <!-- Allow writing to external storage -->
     <uses-permission android:name="android.permission.INTERNET" />
     {% for perm in args.permissions %}
-    {% if '.' in perm %}
-    <uses-permission android:name="{{ perm }}" />
-    {% else %}
-    <uses-permission android:name="android.permission.{{ perm }}" />
-    {% endif %}
+        <uses-permission android:name="{{ perm.name }}"{% if perm.maxSdkVersion %} android:maxSdkVersion="{{ perm.maxSdkVersion }}"{% endif %}{% if perm.usesPermissionFlags %} android:usesPermissionFlags="{{ perm.usesPermissionFlags }}"{% endif %} />
     {% endfor %}
 
     {% if args.wakelock %}

tests/test_bootstrap_build.py

@@ -0,0 +1,87 @@
+import unittest
+import pytest
+import os
+import argparse
+
+from pythonforandroid.util import load_source
+
+
+class TestParsePermissions(unittest.TestCase):
+    def test_parse_permissions_with_migrations(self):
+        # Test that permissions declared in the old format are migrated to the
+        # new format.
+        # (Users can new declare permissions in both formats, even a mix)
+        os.environ["P4A_BUILD_IS_RUNNING_UNITTESTS"] = "1"
+
+        ap = argparse.ArgumentParser()
+        ap.add_argument(
+            "--permission",
+            dest="permissions",
+            action="append",
+            default=[],
+            help="The permissions to give this app.",
+            nargs="+",
+        )
+
+        args = [
+            "--permission",
+            "INTERNET",
+            "--permission",
+            "com.android.voicemail.permission.ADD_VOICEMAIL",
+            "--permission",
+            "(name=android.permission.WRITE_EXTERNAL_STORAGE;maxSdkVersion=18)",
+            "--permission",
+            "(name=android.permission.BLUETOOTH_SCAN;usesPermissionFlags=neverForLocation)",
+        ]
+
+        args = ap.parse_args(args)
+
+        build_src = os.path.join(
+            os.path.dirname(os.path.abspath(__file__)),
+            "../pythonforandroid/bootstraps/common/build/build.py",
+        )
+
+        buildpy = load_source("buildpy", build_src)
+        parsed_permissions = buildpy.parse_permissions(args.permissions)
+
+        assert parsed_permissions == [
+            dict(name="android.permission.INTERNET"),
+            dict(name="com.android.voicemail.permission.ADD_VOICEMAIL"),
+            dict(name="android.permission.WRITE_EXTERNAL_STORAGE", maxSdkVersion="18"),
+            dict(
+                name="android.permission.BLUETOOTH_SCAN",
+                usesPermissionFlags="neverForLocation",
+            ),
+        ]
+
+    def test_parse_permissions_invalid_property(self):
+        os.environ["P4A_BUILD_IS_RUNNING_UNITTESTS"] = "1"
+
+        ap = argparse.ArgumentParser()
+        ap.add_argument(
+            "--permission",
+            dest="permissions",
+            action="append",
+            default=[],
+            help="The permissions to give this app.",
+            nargs="+",
+        )
+
+        args = [
+            "--permission",
+            "(name=android.permission.BLUETOOTH_SCAN;propertyThatFails=neverForLocation)",
+        ]
+
+        args = ap.parse_args(args)
+
+        build_src = os.path.join(
+            os.path.dirname(os.path.abspath(__file__)),
+            "../pythonforandroid/bootstraps/common/build/build.py",
+        )
+
+        buildpy = load_source("buildpy", build_src)
+
+        with pytest.raises(
+            ValueError, match="Property 'propertyThatFails' is not supported."
+        ):
+            buildpy.parse_permissions(args.permissions)

tests/test_build.py

@@ -64,7 +64,10 @@ def test_android_manifest_xml(self):
         args.min_sdk_version = 12
         args.build_mode = 'debug'
         args.native_services = ['abcd', ]
-        args.permissions = []
+        args.permissions = [
+            dict(name="android.permission.INTERNET"),
+            dict(name="android.permission.WRITE_EXTERNAL_STORAGE", maxSdkVersion=18),
+            dict(name="android.permission.BLUETOOTH_SCAN", usesPermissionFlags="neverForLocation")]
         args.add_activity = []
         args.android_used_libs = []
         args.meta_data = []
@@ -91,6 +94,9 @@ def test_android_manifest_xml(self):
         assert xml.count('targetSdkVersion="1234"') == 1
         assert xml.count('android:debuggable="true"') == 1
         assert xml.count('<service android:name="abcd" />') == 1
+        assert xml.count('<uses-permission android:name="android.permission.INTERNET" />') == 1
+        assert xml.count('<uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE" android:maxSdkVersion="18" />') == 1
+        assert xml.count('<uses-permission android:name="android.permission.BLUETOOTH_SCAN" android:usesPermissionFlags="neverForLocation" />') == 1
         # TODO: potentially some other checks to be added here to cover other "logic" (flags and loops) in the template