Commit 7ee7bac4 authored by Mike Wittman's avatar Mike Wittman Committed by Commit Bot

Revert "[MacOs Host] Provide user with a prompt to enable input injection on Mojave"

This reverts commit 19b1b85c.

Reason for revert: breaks mac-dbg build

https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/mac-dbg/1574

In file included from ../../remoting/host/remoting_me2me_host.cc:15:
In file included from ../../base/bind.h:10:
In file included from ../../base/bind_internal.h:13:
In file included from ../../base/callback_internal.h:14:
In file included from ../../base/memory/ref_counted.h:16:
../../base/logging.h:786:26: error: comparison of integers of different signs: 'const unsigned int' and 'const int' [-Werror,-Wsign-compare]
DEFINE_CHECK_OP_IMPL(NE, !=)
~~~~~~~~~~~~~~~~~~~~~~~~~^~~
../../base/logging.h:774:33: note: expanded from macro 'DEFINE_CHECK_OP_IMPL'
    if (ANALYZER_ASSUME_TRUE(v1 op v2))                                      \
                             ~~ ^  ~~
../../base/logging.h:338:36: note: expanded from macro 'ANALYZER_ASSUME_TRUE'
#define ANALYZER_ASSUME_TRUE(arg) (arg)
                                   ^~~
../../remoting/host/remoting_me2me_host.cc:1576:3: note: in instantiation of function template specialization 'logging::CheckNEImpl<unsigned int, int>' requested here
  DCHECK_NE(getuid(), 0);
  ^
../../base/logging.h:960:31: note: expanded from macro 'DCHECK_NE'
#define DCHECK_NE(val1, val2) DCHECK_OP(NE, !=, val1, val2)
                              ^
../../base/logging.h:913:18: note: expanded from macro 'DCHECK_OP'
      ::logging::Check##name##Impl((val1), (val2),                     \
                 ^
<scratch space>:21:1: note: expanded from here
CheckNEImpl
^
1 error generated.


Original change's description:
> [MacOs Host] Provide user with a prompt to enable input injection on Mojave
> 
> This change is required due to new security restrictions in Mojave.  We can no longer
> inject input w/o being added as an accessibility app in the security applet.
> 
> While this sounds like a usefulk speedbump, it causes remote access applications quite
> a bit of trouble:
> 1.) We don't use the restricted API until a user connects so they cannot approve remotely
> 2.) The dialog appears to only show up once (regardless of approve/deny status)
> 3.) Users connecting to a locked machine will never see the dialog
> 
> This is affecting quite a few CRD users, basically everyone who upgrades to Mojave
> will experience this one way or another.  This is the simplest fix (and easiest to merge)
> that I could think of to unblock users.  The prompt will only be shown on 10.14+ platforms
> and the request is only shown if the app has not been approved.  I'd like to look at the
> user feedback after releasing this change to see if more work is needed.
> 
> One problem I anticipate is that the dialog shown doesn't have a lot of context and it
> refers to the wrapper script (org.chromium.chromoting.me2me.sh) instead of Chrome Remote
> Desktop.  If this is confusing, we can wrap the prompt request in a dialog where we control
> the text.  My concern with checking in the feature first is that the new strings won't be
> available for merging.
> 
> Another behavior to call about this impl is that the prompt will be displayed in two instances:
> 1.) When the host is first started (choosing enable via app/website)
> 2.) When the user signs in and the host service is started
> 
> Scenario #2 will have less context but that is the only way to ask for permission for
> users who upgraded and had CRD installed previously.  The dialog is not displayed at the login
> screen (i.e. when no one is signed in).
> 
> One last note, there is no way that I can see to specify this permission in the manifest or
> set up via a script / at install time.  It requires a user action to complete.
> 
> Bug: 901021
> Change-Id: I9dd1b24b6d4d083e7e019af32a0da816f6060a86
> Reviewed-on: https://chromium-review.googlesource.com/c/1313170
> Commit-Queue: Joe Downing <joedow@chromium.org>
> Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#604723}

TBR=jamiewalch@chromium.org,joedow@chromium.org

Change-Id: I7c948b26c00f6c6fc7c4e0a3ec4175dcae17e459
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 901021
Reviewed-on: https://chromium-review.googlesource.com/c/1313747Reviewed-by: default avatarMike Wittman <wittman@chromium.org>
Commit-Queue: Mike Wittman <wittman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604747}
parent 0564739b
...@@ -73,11 +73,6 @@ target("mac_app_bundle", "remoting_me2me_host") { ...@@ -73,11 +73,6 @@ target("mac_app_bundle", "remoting_me2me_host") {
# defines = [ "REMOTING_ENABLE_BREAKPAD" ] # defines = [ "REMOTING_ENABLE_BREAKPAD" ]
# } # }
sources = [
"permission_utils.h",
"permission_utils.mm",
]
deps = [ deps = [
"//remoting/base:breakpad", "//remoting/base:breakpad",
"//remoting/host:main", "//remoting/host:main",
......
// Copyright 2018 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.
#ifndef REMOTING_HOST_MAC_PERMISSION_UTILS_H_
#define REMOTING_HOST_MAC_PERMISSION_UTILS_H_
namespace remoting {
namespace mac {
// Prompts the user to add the current application to the set of trusted
// Accessibility applications. This is only required for input injection on
// 10.14 and later OSes.
void PromptUserToChangeTrustStateIfNeeded();
} // namespace mac
} // namespace remoting
#endif // REMOTING_HOST_MAC_PERMISSION_UTILS_H_
// Copyright 2018 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.
#import "remoting/host/mac/permission_utils.h"
#import <Cocoa/Cocoa.h>
#include "base/logging.h"
namespace remoting {
namespace mac {
void PromptUserToChangeTrustStateIfNeeded() {
// This method will only display a permission prompt if the application is
// not trusted.
NSDictionary* options = @{(NSString*)kAXTrustedCheckOptionPrompt : @YES };
if (!AXIsProcessTrustedWithOptions((CFDictionaryRef)options)) {
LOG(WARNING) << "AXIsProcessTrustedWithOptions: App is not trusted";
}
}
} // namespace mac
} // namespace remoting
...@@ -112,9 +112,7 @@ ...@@ -112,9 +112,7 @@
#endif // defined(OS_POSIX) #endif // defined(OS_POSIX)
#if defined(OS_MACOSX) #if defined(OS_MACOSX)
#include "base/mac/mac_util.h"
#include "base/mac/scoped_cftyperef.h" #include "base/mac/scoped_cftyperef.h"
#include "remoting/host/mac/permission_utils.h"
#endif // defined(OS_MACOSX) #endif // defined(OS_MACOSX)
#if defined(OS_LINUX) #if defined(OS_LINUX)
...@@ -1571,19 +1569,6 @@ void HostProcess::StartHost() { ...@@ -1571,19 +1569,6 @@ void HostProcess::StartHost() {
HostEventLogger::Create(host_->status_monitor(), kApplicationName); HostEventLogger::Create(host_->status_monitor(), kApplicationName);
#endif // !defined(REMOTING_MULTI_PROCESS) #endif // !defined(REMOTING_MULTI_PROCESS)
#if defined(OS_MACOSX)
// Ensure we are not running as root (i.e. at the login screen).
DCHECK_NE(getuid(), 0);
// MacOs 10.14+ requires an addition, runtime permission for injecting input
// using CGEventPost (we use this in our input injector for Mac). This method
// will request that the user enable this permission for us if they are on an
// affected platform and the permission has not already been approved.
if (base::mac::IsAtLeastOS10_14()) {
mac::PromptUserToChangeTrustStateIfNeeded();
}
#endif
host_->Start(host_owner_email_); host_->Start(host_owner_email_);
CreateAuthenticatorFactory(); CreateAuthenticatorFactory();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment