fix: don't register some shortcuts without accessibility (#16262)

electron · Jan 4, 2019 · c783c5e · c783c5e
1 parent 9fa37bd
commit c783c5e
Show file tree

Hide file tree

Showing 6 changed files with 59 additions and 1 deletion.
diff --git a/atom/browser/api/atom_api_global_shortcut.cc b/atom/browser/api/atom_api_global_shortcut.cc
@@ -6,15 +6,44 @@
 
 #include <string>
 
+#include "atom/browser/api/atom_api_system_preferences.h"
 #include "atom/common/native_mate_converters/accelerator_converter.h"
 #include "atom/common/native_mate_converters/callback.h"
 #include "base/stl_util.h"
+#include "base/strings/utf_string_conversions.h"
 #include "native_mate/dictionary.h"
 
 #include "atom/common/node_includes.h"
 
+#if defined(OS_MACOSX)
+#include "base/mac/mac_util.h"
+#endif
+
 using extensions::GlobalShortcutListener;
 
+namespace {
+
+#if defined(OS_MACOSX)
+bool RegisteringMediaKeyForUntrustedClient(const ui::Accelerator& accelerator) {
+  if (base::mac::IsAtLeastOS10_14()) {
+    constexpr ui::KeyboardCode mediaKeys[] = {
+        ui::VKEY_MEDIA_PLAY_PAUSE, ui::VKEY_MEDIA_NEXT_TRACK,
+        ui::VKEY_MEDIA_PREV_TRACK, ui::VKEY_MEDIA_STOP};
+
+    if (std::find(std::begin(mediaKeys), std::end(mediaKeys),
+                  accelerator.key_code()) != std::end(mediaKeys)) {
+      bool trusted =
+          atom::api::SystemPreferences::IsTrustedAccessibilityClient(false);
+      if (!trusted)
+        return true;
+    }
+  }
+  return false;
+}
+#endif
+
+}  // namespace
+
 namespace atom {
 
 namespace api {
@@ -31,7 +60,7 @@ void GlobalShortcut::OnKeyPressed(const ui::Accelerator& accelerator) {
   if (accelerator_callback_map_.find(accelerator) ==
       accelerator_callback_map_.end()) {
     // This should never occur, because if it does, GlobalGlobalShortcutListener
-    // notifes us with wrong accelerator.
+    // notifies us with wrong accelerator.
     NOTREACHED();
     return;
   }
@@ -40,6 +69,11 @@ void GlobalShortcut::OnKeyPressed(const ui::Accelerator& accelerator) {
 
 bool GlobalShortcut::Register(const ui::Accelerator& accelerator,
                               const base::Closure& callback) {
+#if defined(OS_MACOSX)
+  if (RegisteringMediaKeyForUntrustedClient(accelerator))
+    return false;
+#endif
+
   if (!GlobalShortcutListener::GetInstance()->RegisterAccelerator(accelerator,
                                                                   this)) {
     return false;

diff --git a/atom/browser/api/atom_api_system_preferences.cc b/atom/browser/api/atom_api_system_preferences.cc
@@ -83,6 +83,8 @@ void SystemPreferences::BuildPrototype(
                  &SystemPreferences::GetAppLevelAppearance)
       .SetMethod("setAppLevelAppearance",
                  &SystemPreferences::SetAppLevelAppearance)
+      .SetMethod("isTrustedAccessibilityClient",
+                 &SystemPreferences::IsTrustedAccessibilityClient)
       .SetMethod("getMediaAccessStatus",
                  &SystemPreferences::GetMediaAccessStatus)
       .SetMethod("askForMediaAccess", &SystemPreferences::AskForMediaAccess)

diff --git a/atom/browser/api/atom_api_system_preferences.h b/atom/browser/api/atom_api_system_preferences.h
@@ -91,6 +91,8 @@ class SystemPreferences : public mate::EventEmitter<SystemPreferences>
   void RemoveUserDefault(const std::string& name);
   bool IsSwipeTrackingFromScrollEventsEnabled();
 
+  static bool IsTrustedAccessibilityClient(bool prompt);
+
   // TODO(codebytere): Write tests for these methods once we
   // are running tests on a Mojave machine
   std::string GetMediaAccessStatus(const std::string& media_type,

diff --git a/atom/browser/api/atom_api_system_preferences_mac.mm b/atom/browser/api/atom_api_system_preferences_mac.mm
@@ -386,6 +386,12 @@ AVMediaType ParseMediaType(const std::string& media_type) {
   }
 }
 
+// static
+bool SystemPreferences::IsTrustedAccessibilityClient(bool prompt) {
+  NSDictionary* options = @{(id)kAXTrustedCheckOptionPrompt : @(prompt)};
+  return AXIsProcessTrustedWithOptions((CFDictionaryRef)options);
+}
+
 std::string SystemPreferences::GetMediaAccessStatus(
     const std::string& media_type,
     mate::Arguments* args) {

diff --git a/docs/api/global-shortcut.md b/docs/api/global-shortcut.md
@@ -54,6 +54,14 @@ When the accelerator is already taken by other applications, this call will
 silently fail. This behavior is intended by operating systems, since they don't
 want applications to fight for global shortcuts.
 
+The following accelerators will not be registered successfully on macOS 10.14 Mojave unless
+the app has been authorized as a [trusted accessibility client](https://developer.apple.com/library/archive/documentation/Accessibility/Conceptual/AccessibilityMacOSX/OSXAXTestingApps.html):
+
+* "Media Play/Pause"
+* "Media Next Track"
+* "Media Previous Track"
+* "Media Stop"
+
 ### `globalShortcut.isRegistered(accelerator)`
 
 * `accelerator` [Accelerator](accelerator.md)

diff --git a/docs/api/system-preferences.md b/docs/api/system-preferences.md
@@ -317,6 +317,12 @@ You can use the `setAppLevelAppearance` API to set this value.
 Sets the appearance setting for your application, this should override the
 system default and override the value of `getEffectiveAppearance`.
 
+### `systemPreferences.isTrustedAccessibilityClient(prompt)` _macOS_
+
+* `prompt` Boolean - whether or not the user will be informed via prompt if the current process is untrusted.
+
+Returns `Boolean` - `true` if the current process is a trusted accessibility client and `false` if it is not.
+
 ### `systemPreferences.getMediaAccessStatus(mediaType)` _macOS_
 
 * `mediaType` String - `microphone` or `camera`.