Public Sessions - prompt the user for audioCapture/videoCapture requests

chrome/browser/media/extension_media_access_handler.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/media/extension_media_access_handler.h"
 
 #include <utility>
 
 #include "chrome/browser/media/webrtc/media_capture_devices_dispatcher.h"
 #include "chrome/browser/media/webrtc/media_stream_capture_indicator.h"
 #include "chrome/browser/media/webrtc/media_stream_device_permissions.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/pref_names.h"
+#include "chromeos/login/login_state.h"
 #include "content/public/browser/web_contents.h"
 #include "extensions/common/extension.h"
+#include "extensions/common/permissions/manifest_permission_set.h"
 #include "extensions/common/permissions/permissions_data.h"
+#include "extensions/common/url_pattern_set.h"
 
 namespace {
 
 // This is a short-term solution to grant camera and/or microphone access to
 // extensions:
 // 1. Virtual keyboard extension.
 // 2. Flutter gesture recognition extension.
 // 3. TODO(smus): Airbender experiment 1.
 // 4. TODO(smus): Airbender experiment 2.
 // 5. Hotwording component extension.
 // 6. XKB input method component extension.
 // 7. M17n/T13n/CJK input method component extension.
 // Once http://crbug.com/292856 is fixed, remove this whitelist.
 bool IsMediaRequestWhitelistedForExtension(
     const extensions::Extension* extension) {
   return extension->id() == "mppnpdlheglhdfmldimlhpnegondlapf" ||
          extension->id() == "jokbpnebhdcladagohdnfgjcpejggllo" ||
          extension->id() == "clffjmdilanldobdnedchkdbofoimcgb" ||
          extension->id() == "nnckehldicaciogcbchegobnafnjkcne" ||
          extension->id() == "nbpagnldghgfoolbancepceaanlmhfmd" ||
          extension->id() == "jkghodnilhceideoidjikpgommlajknk" ||
          extension->id() == "gjaehgfemfahhmlgpdfknkhdnemmolop";
 }
 
+// Returns true if we're in a Public Session.
+bool IsPublicSession() {
+#if defined(OS_CHROMEOS)
+  if (chromeos::LoginState::IsInitialized()) {
+    return chromeos::LoginState::Get()->IsPublicSessionUser();
+  }
+#endif
+  return false;
+}
+
 }  // namespace
 
 ExtensionMediaAccessHandler::ExtensionMediaAccessHandler() {
 }
 
 ExtensionMediaAccessHandler::~ExtensionMediaAccessHandler() {
 }
 
 bool ExtensionMediaAccessHandler::SupportsStreamType(
     const content::MediaStreamType type,
     const extensions::Extension* extension) {
   return extension && (extension->is_platform_app() ||
                        IsMediaRequestWhitelistedForExtension(extension)) &&

[Sergey Ulanov, 2016/11/29 19:12:05]

This class is applicable for platform apps and whitelisted built-in extensions,
while CL description says it applies to all extensions. Regular extensions are
handled by PermissionBubbleMediaAccessHandler and I believe it shows the prompt
by default.
Also even it makes sense to show the prompt for platform apps, I'm not sure
about built-in whitelisted extensions.

[Ivan Šandrk, 2016/11/29 23:33:46]

You have a good point, the goal of this CL is to show the prompt only for
platform apps. I've updated the CL description to reflect this (added another
paragraph at the end). I've also added code that limits the dialog only to
platform apps.

          (type == content::MEDIA_DEVICE_AUDIO_CAPTURE ||
           type == content::MEDIA_DEVICE_VIDEO_CAPTURE);
 }
 
 bool ExtensionMediaAccessHandler::CheckMediaAccessPermission(
     content::WebContents* web_contents,
     const GURL& security_origin,
     content::MediaStreamType type,
     const extensions::Extension* extension) {
   return extension->permissions_data()->HasAPIPermission(
       type == content::MEDIA_DEVICE_AUDIO_CAPTURE
           ? extensions::APIPermission::kAudioCapture
           : extensions::APIPermission::kVideoCapture);
 }
 
+bool ExtensionMediaAccessHandler::UserChoice::Disallowed(

[Sergey Ulanov, 2016/11/29 19:12:06]

IsAllowed()?
That would make code that uses this function cleaner.

[Ivan Šandrk, 2016/11/29 23:33:46]

I believe there's a subtle semantic difference between IsAllowed and
!Disallowed, but I may be wrong. It seems to me that the first implies that we
need explicit permission to do it, while the second implies that it just mustn't
be explicitly forbidden (the third option here is status quo outside of Public
Sessions where it's neither explicitly allowed or forbidden in which case we
allow it).

What do you think?

[Ivan Šandrk, 2016/11/30 17:00:50]

Decided to go with the IsAllowed in the end.

+    content::MediaStreamType type) const {
+  if (IsPublicSession()) {
+    CHECK(type == content::MEDIA_DEVICE_AUDIO_CAPTURE ||

[Sergey Ulanov, 2016/11/29 19:12:06]

Why is this CHECK instead of DCHECK?
https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md...
Also it would be cleaner to put a check for type ==
content::MEDIA_DEVICE_VIDEO_CAPTURE in the else case below.

[Ivan Šandrk, 2016/11/29 23:33:46]

Good point, done. And changed the other CHECK as well.

+          type == content::MEDIA_DEVICE_VIDEO_CAPTURE);
+    if (type == content::MEDIA_DEVICE_AUDIO_CAPTURE) {
+      return audio_prompted_ && !audio_allowed_;
+    } else {
+      return video_prompted_ && !video_allowed_;
+    }
+  }
+  return false;
+}
+
+bool ExtensionMediaAccessHandler::UserChoice::NeedsPrompting(
+    content::MediaStreamType type) const {
+  if (type == content::MEDIA_DEVICE_AUDIO_CAPTURE) {
+    return !audio_prompted_;
+  }
+  if (type == content::MEDIA_DEVICE_VIDEO_CAPTURE) {
+    return !video_prompted_;
+  }
+  return false;
+}
+
+void ExtensionMediaAccessHandler::UserChoice::Set(
+    content::MediaStreamType type, bool allowed) {
+  CHECK(type == content::MEDIA_DEVICE_AUDIO_CAPTURE ||
+        type == content::MEDIA_DEVICE_VIDEO_CAPTURE);
+  if (type == content::MEDIA_DEVICE_AUDIO_CAPTURE) {
+    DCHECK(!audio_prompted_);
+    audio_prompted_ = true;
+    audio_allowed_ = allowed;
+  } else {
+    DCHECK(!video_prompted_);
+    video_prompted_ = true;
+    video_allowed_ = allowed;
+  }
+}
+
+void ExtensionMediaAccessHandler::ResolvePermissionPrompt(
+    content::WebContents* web_contents,
+    const content::MediaStreamRequest& request,
+    const content::MediaResponseCallback& callback,
+    const extensions::Extension* extension,
+    ExtensionInstallPrompt::Result prompt_result) {
+  bool allowed = (prompt_result == ExtensionInstallPrompt::Result::ACCEPTED);
+  UserChoice& user_choice = user_choice_cache_[extension->id()];
+
+  if (user_choice.NeedsPrompting(request.audio_type))
+    user_choice.Set(content::MEDIA_DEVICE_AUDIO_CAPTURE, allowed);
+  if (user_choice.NeedsPrompting(request.video_type))
+    user_choice.Set(content::MEDIA_DEVICE_VIDEO_CAPTURE, allowed);
+
+  HandleRequestContinuation(web_contents, request, callback, extension);
+}
+
 void ExtensionMediaAccessHandler::HandleRequest(
     content::WebContents* web_contents,
     const content::MediaStreamRequest& request,
     const content::MediaResponseCallback& callback,
     const extensions::Extension* extension) {
   // TODO(vrk): This code is largely duplicated in

[Sergey Ulanov, 2016/11/29 19:12:06]

Move this TODO to HandleRequestContinuation()

[Ivan Šandrk, 2016/11/29 23:33:46]

Done.

   // MediaStreamDevicesController::Accept(). Move this code into a shared method
   // between the two classes.
 
+  // In Public Sessions extensions are installed by admin policy hence they can
+  // freely use the camera and microphone without the user knowing. This is not
+  // acceptable from a security/privacy standpoint so we show the user a dialog
+  // where he can choose whether to allow the extension access to camera and/or
+  // microphone.
+  if (IsPublicSession()) {

[Sergey Ulanov, 2016/11/29 19:12:05]

Did you consider adding a separate MediaAccessHandler for public sessions? It
would allow to avoid mixing all the new logic that's applicable to public
session with everything else.
Potentially it could be implemented as a wrapper for ExtensionMediaAccessHandler
or as a separate MediaAccessHandler that's used before
ExtensionMediaAccessHandler (access handlers priority is determined by their
order here:
https://codesearch.chromium.org/chromium/src/chrome/browser/media/webrtc/medi...
)

[Ivan Šandrk, 2016/11/29 23:33:46]

This is a good idea :) I will investigate it more tomorrow.

[Ivan Šandrk, 2016/11/30 17:00:50]

Implemented a separate MediaAccessHandler :)

+    bool needs_prompt = false;
+    extensions::APIPermissionSet new_apis;
+    const UserChoice& user_choice = user_choice_cache_[extension->id()];
+
+    if (user_choice.NeedsPrompting(request.audio_type)) {
+      new_apis.insert(extensions::APIPermission::kAudioCapture);
+      needs_prompt = true;
+    }
+    if (user_choice.NeedsPrompting(request.video_type)) {
+      new_apis.insert(extensions::APIPermission::kVideoCapture);
+      needs_prompt = true;
+    }
+
+    if (needs_prompt) {
+      std::unique_ptr<const extensions::PermissionSet> permission_set(
+          new extensions::PermissionSet(
+              new_apis, extensions::ManifestPermissionSet(),
+              extensions::URLPatternSet(), extensions::URLPatternSet()));
+
+      prompt_.reset(new ExtensionInstallPrompt(web_contents));

[Sergey Ulanov, 2016/11/29 19:12:06]

There may be multiple requests that will need to be handled in parallel, so I
think you need to queue requests here instead of replacing the old |prompt_|.

[Sergey Ulanov, 2016/11/29 19:12:06]

Does ExtensionInstallPrompt() work when the web_contents is a background page or
a platform app window?

[Ivan Šandrk, 2016/11/29 23:33:46]

I believe it should always work when there's web_contents available. I've tested
this functionality with a couple of apps and it works in each case.

[Ivan Šandrk, 2016/11/29 23:33:46]

Now I've prompted to save the ExtensionInstallPrompt inside the callback itself,
I think it should be fine. Do you think there's something inherently wrong with
this approach?

+      prompt_->ShowDialog(
+          base::Bind(&ExtensionMediaAccessHandler::ResolvePermissionPrompt,
+              base::Unretained(this), web_contents, request, callback,
+              extension),
+          extension,
+          nullptr,
+          base::MakeUnique<ExtensionInstallPrompt::Prompt>(
+              ExtensionInstallPrompt::PERMISSIONS_PROMPT),
+          std::move(permission_set),
+          ExtensionInstallPrompt::GetDefaultShowDialogCallback());
+      return;
+    }
+  }
+
+  HandleRequestContinuation(web_contents, request, callback, extension);
+}
+
+void ExtensionMediaAccessHandler::HandleRequestContinuation(
+    content::WebContents* web_contents,
+    const content::MediaStreamRequest& request,
+    const content::MediaResponseCallback& callback,
+    const extensions::Extension* extension) {
   Profile* profile =
       Profile::FromBrowserContext(web_contents->GetBrowserContext());
+  const UserChoice& user_choice = user_choice_cache_[extension->id()];
 
   bool audio_allowed =
       request.audio_type == content::MEDIA_DEVICE_AUDIO_CAPTURE &&
       extension->permissions_data()->HasAPIPermission(
           extensions::APIPermission::kAudioCapture) &&
       GetDevicePolicy(profile, extension->url(), prefs::kAudioCaptureAllowed,
-                      prefs::kAudioCaptureAllowedUrls) != ALWAYS_DENY;
+                      prefs::kAudioCaptureAllowedUrls) != ALWAYS_DENY &&
+      !user_choice.Disallowed(content::MEDIA_DEVICE_AUDIO_CAPTURE);
   bool video_allowed =
       request.video_type == content::MEDIA_DEVICE_VIDEO_CAPTURE &&
       extension->permissions_data()->HasAPIPermission(
           extensions::APIPermission::kVideoCapture) &&
       GetDevicePolicy(profile, extension->url(), prefs::kVideoCaptureAllowed,
-                      prefs::kVideoCaptureAllowedUrls) != ALWAYS_DENY;
+                      prefs::kVideoCaptureAllowedUrls) != ALWAYS_DENY &&
+      !user_choice.Disallowed(content::MEDIA_DEVICE_VIDEO_CAPTURE);
 
   bool get_default_audio_device = audio_allowed;
   bool get_default_video_device = video_allowed;
 
   content::MediaStreamDevices devices;
 
   // Set an initial error result. If neither audio or video is allowed, we'll
   // never try to get any device below but will just create |ui| and return an
   // empty list with "invalid state" result. If at least one is allowed, we'll
   // try to get device(s), and if failure, we want to return "no hardware"
@@ skipping 37 matching lines @@
   std::unique_ptr<content::MediaStreamUI> ui;
   if (!devices.empty()) {
     result = content::MEDIA_DEVICE_OK;
     ui = MediaCaptureDevicesDispatcher::GetInstance()
              ->GetMediaStreamCaptureIndicator()
              ->RegisterMediaStream(web_contents, devices);
   }
 
   callback.Run(devices, result, std::move(ui));
 }