Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TIMOB-25854] Android: grant permissions for Audio/Video WebRTC #9928

Closed
wants to merge 7 commits into from

Conversation

m1ga
Copy link
Contributor

@m1ga m1ga commented Mar 11, 2018

JIRA: https://jira.appcelerator.org/browse/TIMOB-25854

Create a custom conference room at: https://palava.tv/
For more details see the JIRA ticket

var win = Ti.UI.createWindow({
  title: 'Form',
  backgroundColor: 'blue'
});
 
var www = Ti.UI.createWebView({
   url: "[PALAVA URL]",
   allowWebPermission: true
});
 
win.add(www);
 
var btn  = Ti.UI.createButton({
  title:"refresh",
  bottom:10
})
btn.addEventListener("click",function(){
  www.url = "[PALAVA_URL]";
})
win.add(btn);
win.open();

tiapp.xml

<android xmlns:android="http://schemas.android.com/apk/res/android">
<manifest >
<uses-permission android:name="android.permission.RECORD_AUDIO" />
<uses-permission android:name="android.permission.MODIFY_AUDIO_SETTINGS" />
<uses-permission android:name="android.permission.CAMERA"/>
</manifest>
</android>

@build
Copy link
Contributor

build commented Mar 11, 2018

Messages
📖

🎉 Another contribution from our awesome community member, m1ga! Thanks again for helping us make Titanium SDK better. 👍

Generated by 🚫 dangerJS

@Topener Topener changed the title [AC-5655] Android: grant permissions for Audio/Video WebRTC [TIMOB-25854] Android: grant permissions for Audio/Video WebRTC Mar 12, 2018
@hansemannn
Copy link
Collaborator

@m1ga Is it possible to add a unit-test for this? Otherwise, we'll add the no tests label for now.

@Override
public void onPermissionRequest(final PermissionRequest request)
{
TiViewProxy proxy = tiWebView.getProxy();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

correct, tried a different approach before and that survived the code change. Gone now

{
TiViewProxy proxy = tiWebView.getProxy();
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) {
request.grant(request.getResources());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jquick-axway Should we always grant this? I feel this might cause side-effects for other components.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You still have to show the popup in your app to access camera (camera permission)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So what do we do if a customer wants to explicitly disallow it? :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The he clicks no when you ask for Ti.Android.requestPermissions() in your app. But you are right: you can't allow the camera in other parts of your app and disallow it for the webview.
So a property/method would be better to allow it and then the developer has to reload the page when it is set to true!

@m1ga
Copy link
Contributor Author

m1ga commented Mar 12, 2018

About the test:
It's only visible inside the webview and if you run the code in the current SDK you won't see any difference, just that website inside the webview will display a "please allow permissions" text.

Not sure if that is test-able

@m1ga
Copy link
Contributor Author

m1ga commented Mar 12, 2018

@hansemannn how about:

TiViewProxy proxy = tiWebView.getProxy();
if (proxy.getProperty(TiC.PROPERTY_ALLOW_WEB_PERMISSION)) {
	request.grant(request.getResources());
}

and a property called allowWebPermission that you can enable/disable. If the dev wants to open a WebRTC page he'll check if allowWebPermission is true otherwise display a OptionDialog and set it to true and load the page

@build build added the android label May 10, 2018
@hansemannn
Copy link
Collaborator

Requesting @jquick-axway for the review.

@sgtcoolguy sgtcoolguy added this to the 7.4.0 milestone Jun 26, 2018
@jquick-axway
Copy link
Contributor

jquick-axway commented Jun 28, 2018

I'm not sure how I feel about having a boolean that grants permission to all resources and all URLs like this. Especially since it includes resources such as audio/video recording. I don't want this code to get red-flagged as a security concern. I believe Google's main intention was for the app to display a dialog asking the end-user to grant permission as can be seen in their example app below (see its screenshots).
https://github.com/googlesamples/android-PermissionRequest

@hansemannn, do you know how Apple expects this to be handled by a WKWebView/UIWebView?

Edit: I think there is a design issue here. Audio/Video recording via a WebView won't simply work with this boolean anyways. You have to use Google's Java Context.requestPermissions() method to ask the end-user for permission (can be seen in their example code above). In Titanium, that means using our Ti.Media library's requestCameraPermission() or requestAudioRecorderPermissions() methods. So, a simple boolean isn't going to do the job. It needs to be an event so that the app developer can request permission for the appropriate resource being requested. You can only call the Java PermissionRequest.grant() method once the end-user has given permission, otherwise you have to call PermissionRequest.deny(). But let's wait for feedback on the iOS side before proceeding. If iOS automatically requests permission, then perhaps instead of making it an event, we could automatically request permission as well on Android. Thanks.

@hansemannn
Copy link
Collaborator

@jquick-axway I agree. For iOS, permission handling is necessary in two places:

  1. The user needs to have the privacy keys in the Info.plist / tiapp.xml, e.g.:
<ios>
        <plist>
            <dict>
                <!-- Permissions: Microphone -->
                <key>NSMicrophoneUsageDescription</key>
                <string>We need permission to access the microphone on your device for audio input or recording.</string>
                <!-- Permissions: Camera -->
                <key>NSCameraUsageDescription</key>
                <string>We need permission to access your device camera.</string>
            </dict>
        </plist>
    </ios>
  1. The user is recommended to check and request the permissions before attempting to load a web-view that requires the permission. But: If not set, iOS will prompt for the permissions automatically before accessing any resource.

So @m1ga, @jquick-axway: I would agree we should re-design this on Android and come back with a more generic solution.

@hansemannn hansemannn closed this Jun 28, 2018
@hansemannn hansemannn removed this from the 7.4.0 milestone Jun 28, 2018
@jquick-axway
Copy link
Contributor

Okay. Since iOS permission handling is automatic after adding the plist settings mentioned by @hansemannn, then we should make it automatic on Android as well. This means we don't need a new property or event.

What I recommend is to modify onPermissionRequest() to do the following:

  1. Check if a manifest permission is defined for the requested resource(s). If not, deny().
  2. On API Level < 23, grant() if the manifest permissions are defined. (We're done.)
  3. On API Level >= 23 (aka: Android 6.), call Context.checkSelfPermission() for the requested resource(s) to see if the end-user has granted permission yet. If already granted, thengrant().
  4. If not granted yet, then call Activity.requestPermissions() to display a dialog to the end-user.

Example on how to request permission:
https://github.com/appcelerator/titanium_mobile/blob/master/android/modules/media/src/java/ti/modules/titanium/media/MediaModule.java#L512

@m1ga, I appreciate the effort you've made here and I hope you don't feel discouraged.

@m1ga
Copy link
Contributor Author

m1ga commented Jun 28, 2018

@jquick-axway no problem at all!

I think there are two request you need to do:

  • The access camera that is already in the SDK with requestPermission()
  • e.g. PermissionRequest.RESOURCE_VIDEO_CAPTURE (with a custom AlertDialog as far as I see)

right? The first is the to access the camera at all and the second is to allow the webview to access it.

I'm checking some more examples and see how it can be done. If someone else is working on that I'm fine, too 😄 It will take a bit for me to get back to that PR

@hansemannn
Copy link
Collaborator

Thanks @m1ga, keep your great contributions! Maybe a look over the edge how other frameworks / native apps do this would help as well.

@jquick-axway
Copy link
Contributor

@m1ga, the good news here is that you don't need to display a custom alert dialog. Google's Activity.requestPermissions() method will display a standard permission request dialog for us. The only custom thing you need to do is use our TiBaseActivity.registerPermissionRequestCallback() method to set up the onActivityResult() handling. The link I posted above shows you how to do this. It's pretty easy to set up and I'm okay with you duplicating our Java "media" module's permission requesting code. It's a small amount of code and the only thing you have to do differently is map the Java PermissionRequest resource constants to the appropriate permission.

Google's source code below shows how to do this for RESOURCE_VIDEO_CAPTURE. We just got to figure out how to handle the other 3 resource constants next. Particularly RESOURCE_MIDI_SYSEX and RESOURCE_PROTECTED_MEDIA_ID because I'm not sure how those are supposed to be handled.
https://github.com/googlesamples/android-PermissionRequest/blob/master/Application/src/main/java/com/example/android/permissionrequest/PermissionRequestFragment.java

@m1ga m1ga deleted the webview branch August 7, 2020 07:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants