[Extensions Click-to-Script] Adjust permissions API for withheld permissions

Adjust the chrome.permissions API for use with withheld permissions as
a result of the RuntimeHostPermissions feature. This patch allows the
permissions API to be used to request withheld or optional host
permissions, rather than just optional host permissions.

This is important for extensions that want to be able to request
permissions at runtime using the chrome.permissions API as a workaround
to having required permisssions withheld.

Additionally, move requesting optional permissions to looking at the
runtime granted permission set, rather than the granted permission set.
This greatly simplifies the logic, as now all permission requests are
compared to the same permissions set, and we don't need to isolate host
permission requests versus API permission requests. This also ensures
we avoid granting a permission that was explicitly revoked by the user
(as could happen if we just checked granted permissions).

As of Chrome 70, all permissions granted through the permissions API
are added to the runtime granted set. However, this does mean that
optional permissions that were granted prior to M70, and then later
removed via permissions.remove(), will re-prompt the user. In practice,
this is very rare. This also means that optional permissions will not be
granted automatically for permissions that were used by previous versions
of the extension (similarly, very rare in practice).

This does *not* address the issue of scriptable hosts in requested
permissions; that is still an issue.

Bug: 889654

Change-Id: I8c20f03ec7b402cd61ae1db04b782447dc39414e
Reviewed-on: https://chromium-review.googlesource.com/c/1301851
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607474}

chrome/browser/extensions/api/permissions/permissions_api.cc

@@ -31,8 +31,8 @@ const char kBlockedByEnterprisePolicy[] =
     "Permissions are blocked by enterprise policy.";
 const char kCantRemoveRequiredPermissionsError[] =
     "You cannot remove required permissions.";
-const char kNotInOptionalPermissionsError[] =
-    "Optional permissions must be listed in extension manifest.";
+const char kNotInManifestPermissionsError[] =
+    "Only permissions specified in the manifest may be requested.";
 const char kNotWhitelistedError[] =
     "The optional permissions API does not support '*'.";
 const char kUserGestureRequiredError[] =
@@ -92,7 +92,8 @@ ExtensionFunction::ResponseAction PermissionsRemoveFunction::Run() {
   if (!permissions.get())
     return RespondNow(Error(error));
 
-  // Make sure they're only trying to remove permissions supported by this API.
+  // Make sure the extension is only trying to remove permissions supported by
+  // this API.
   APIPermissionSet apis = permissions->apis();
   for (const APIPermission* permission : apis) {
     if (!permission->info()->supports_optional())
@@ -104,6 +105,9 @@ ExtensionFunction::ResponseAction PermissionsRemoveFunction::Run() {
   // optional and required (and should assume its required), so we need both of
   // these checks.
   // TODO(devlin): *Why* do we support that? Should be a load error.
+  // NOTE(devlin): This won't support removal of required permissions that can
+  // withheld. I don't think that will be a common use case, and so is probably
+  // fine.
   const PermissionSet& optional =
       PermissionsParser::GetOptionalPermissions(extension());
   const PermissionSet& required =
@@ -133,6 +137,10 @@ void PermissionsRequestFunction::SetAutoConfirmForTests(bool should_proceed) {
   auto_confirm_for_tests = should_proceed ? PROCEED : ABORT;
 }
 
+void PermissionsRequestFunction::ResetAutoConfirmForTests() {
+  auto_confirm_for_tests = DO_NOT_SKIP;
+}
+
 // static
 void PermissionsRequestFunction::SetIgnoreUserGestureForTests(
     bool ignore) {
@@ -152,7 +160,7 @@ ExtensionFunction::ResponseAction PermissionsRequestFunction::Run() {
 
   gfx::NativeWindow native_window =
       ChromeExtensionFunctionDetails(this).GetNativeWindowForUI();
-  if (!native_window)
+  if (!native_window && auto_confirm_for_tests == DO_NOT_SKIP)
     return RespondNow(Error("Could not find an active window."));
 
   std::unique_ptr<api::permissions::Request::Params> params(
@@ -160,83 +168,107 @@ ExtensionFunction::ResponseAction PermissionsRequestFunction::Run() {
   EXTENSION_FUNCTION_VALIDATE(params);
 
   std::string error;
-  requested_permissions_ = permissions_api_helpers::UnpackPermissionSet(
-      params->permissions,
-      ExtensionPrefs::Get(browser_context())->AllowFileAccess(extension_->id()),
-      &error);
-  if (!requested_permissions_.get())
+  std::unique_ptr<const PermissionSet> requested_permissions =
+      permissions_api_helpers::UnpackPermissionSet(
+          params->permissions,
+          ExtensionPrefs::Get(browser_context())
+              ->AllowFileAccess(extension_->id()),
+          &error);
+  if (!requested_permissions.get())
     return RespondNow(Error(error));
 
   // Make sure they're only requesting permissions supported by this API.
-  APIPermissionSet apis = requested_permissions_->apis();
-  for (APIPermissionSet::const_iterator i = apis.begin();
-       i != apis.end(); ++i) {
-    if (!i->info()->supports_optional()) {
-      return RespondNow(Error(
-          ErrorUtils::FormatErrorMessage(kNotWhitelistedError, i->name())));
+  for (const auto* api_permission : requested_permissions->apis()) {
+    if (!api_permission->info()->supports_optional()) {
+      return RespondNow(Error(ErrorUtils::FormatErrorMessage(
+          kNotWhitelistedError, api_permission->name())));
     }
   }
 
-  // The requested permissions must be defined as optional in the manifest.
-  if (!PermissionsParser::GetOptionalPermissions(extension())
-           .Contains(*requested_permissions_)) {
-    return RespondNow(Error(kNotInOptionalPermissionsError));
+  const PermissionsData* permissions_data = extension()->permissions_data();
+  // Subtract out any permissions already active on the extension.
+  std::unique_ptr<const PermissionSet> new_permissions =
+      PermissionSet::CreateDifference(*requested_permissions,
+                                      permissions_data->active_permissions());
+
+  // If all permissions are already active, nothing left to do.
+  if (new_permissions->IsEmpty()) {
+    constexpr bool granted = true;
+    return RespondNow(OneArgument(std::make_unique<base::Value>(granted)));
   }
 
   // Automatically declines api permissions requests, which are blocked by
   // enterprise policy.
   if (!ExtensionManagementFactory::GetForBrowserContext(browser_context())
-           ->IsPermissionSetAllowed(extension(), *requested_permissions_)) {
+           ->IsPermissionSetAllowed(extension(), *new_permissions)) {
     return RespondNow(Error(kBlockedByEnterprisePolicy));
   }
 
-  // We don't need to prompt the user if the requested permissions are a subset
-  // of the granted permissions set.
-  std::unique_ptr<const PermissionSet> granted =
-      ExtensionPrefs::Get(browser_context())
-          ->GetGrantedPermissions(extension()->id());
-  if (granted.get() && granted->Contains(*requested_permissions_)) {
-    PermissionsUpdater perms_updater(browser_context());
-    perms_updater.GrantOptionalPermissions(*extension(),
-                                           *requested_permissions_);
-    return RespondNow(
-        ArgumentList(api::permissions::Request::Results::Create(true)));
+  // Look for any permissions that were withheld. These previously would have
+  // been auto-granted to the extension, but are now waiting for deliberate
+  // user approval.
+  requested_withheld_ = PermissionSet::CreateIntersection(
+      *new_permissions, permissions_data->withheld_permissions());
+  // Assume any permissions that weren't withheld are being requested as
+  // optional permissions.
+  requested_optional_ =
+      PermissionSet::CreateDifference(*new_permissions, *requested_withheld_);
+
+  // Check that the remaining permissions (assumed to be optional) are specified
+  // in the manifest.
+  if (!PermissionsParser::GetOptionalPermissions(extension())
+           .Contains(*requested_optional_)) {
+    return RespondNow(Error(kNotInManifestPermissionsError));
   }
 
-  // Filter out the granted permissions so we only prompt for new ones.
-  requested_permissions_ =
-      PermissionSet::CreateDifference(*requested_permissions_, *granted);
-
-  // Filter out the active permissions.
-  requested_permissions_ = PermissionSet::CreateDifference(
-      *requested_permissions_,
-      extension()->permissions_data()->active_permissions());
+  // Find the permissions to prompt the user for.
+  // We prompt for |requested_withheld_| +
+  // (|requested_optional_| - |already_granted_permissions|).
+  // We prompt for |requested_withheld_| since these were deliberately revoked
+  // by the user. We don't prompt for |already_granted_permissions|
+  // since these were either granted to an earlier extension version or removed
+  // by the extension itself (using the permissions.remove() method).
+  std::unique_ptr<const PermissionSet> granted_permissions =
+      ExtensionPrefs::Get(browser_context())
+          ->GetRuntimeGrantedPermissions(extension()->id());
+  std::unique_ptr<const PermissionSet> already_granted_permissions =
+      PermissionSet::CreateIntersection(*granted_permissions,
+                                        *requested_optional_);
+  new_permissions = PermissionSet::CreateDifference(
+      *new_permissions, *already_granted_permissions);
 
   AddRef();  // Balanced in OnInstallPromptDone().
 
   // We don't need to show the prompt if there are no new warnings, or if
-  // we're skipping the confirmation UI. All extension types but INTERNAL
-  // are allowed to silently increase their permission level.
+  // we're skipping the confirmation UI. COMPONENT extensions are allowed to
+  // silently increase their permission level.
   const PermissionMessageProvider* message_provider =
       PermissionMessageProvider::Get();
+  // TODO(devlin): We should probably use the same logic we do for permissions
+  // increases here, where we check if there are *new* warnings (e.g., so we
+  // don't warn about the tabs permission if history is already granted).
   bool has_no_warnings =
-      message_provider->GetPermissionMessages(
-                          message_provider->GetAllPermissionIDs(
-                              *requested_permissions_, extension()->GetType()))
+      message_provider
+          ->GetPermissionMessages(message_provider->GetAllPermissionIDs(
+              // |new_permissions| all permissions that need user consent.
+              *new_permissions, extension()->GetType()))
           .empty();
-  if (auto_confirm_for_tests == PROCEED || has_no_warnings ||
-      extension_->location() == Manifest::COMPONENT) {
+  if (has_no_warnings || extension_->location() == Manifest::COMPONENT) {
     OnInstallPromptDone(ExtensionInstallPrompt::Result::ACCEPTED);
     return AlreadyResponded();
   }
 
-  if (auto_confirm_for_tests == ABORT) {
-    // Pretend the user clicked cancel.
-    OnInstallPromptDone(ExtensionInstallPrompt::Result::USER_CANCELED);
+  // Otherwise, we have to prompt the user (though we might "autoconfirm" for a
+  // test.
+  if (auto_confirm_for_tests != DO_NOT_SKIP) {
+    prompted_permissions_for_testing_ = new_permissions->Clone();
+    if (auto_confirm_for_tests == PROCEED)
+      OnInstallPromptDone(ExtensionInstallPrompt::Result::ACCEPTED);
+    else if (auto_confirm_for_tests == ABORT)
+      OnInstallPromptDone(ExtensionInstallPrompt::Result::USER_CANCELED);
     return AlreadyResponded();
   }
 
-  CHECK_EQ(DO_NOT_SKIP, auto_confirm_for_tests);
   install_ui_.reset(new ExtensionInstallPrompt(
       Profile::FromBrowserContext(browser_context()), native_window));
   install_ui_->ShowDialog(
@@ -244,7 +276,9 @@ ExtensionFunction::ResponseAction PermissionsRequestFunction::Run() {
       extension(), nullptr,
       std::make_unique<ExtensionInstallPrompt::Prompt>(
           ExtensionInstallPrompt::PERMISSIONS_PROMPT),
-      requested_permissions_->Clone(),
+      // |new_permissions| includes both |requested_optional_| and
+      // |requested_withheld_|.
+      std::move(new_permissions),
       ExtensionInstallPrompt::GetDefaultShowDialogCallback());
 
   // ExtensionInstallPrompt::ShowDialog() can call the response synchronously.
@@ -255,13 +289,20 @@ void PermissionsRequestFunction::OnInstallPromptDone(
     ExtensionInstallPrompt::Result result) {
   bool granted = result == ExtensionInstallPrompt::Result::ACCEPTED;
   if (granted) {
-    PermissionsUpdater perms_updater(browser_context());
-    perms_updater.GrantOptionalPermissions(*extension(),
-                                           *requested_permissions_);
+    PermissionsUpdater permissions_updater(browser_context());
+    permissions_updater.GrantRuntimePermissions(*extension(),
+                                                *requested_withheld_);
+    permissions_updater.GrantOptionalPermissions(*extension(),
+                                                 *requested_optional_);
   }
 
   Respond(ArgumentList(api::permissions::Request::Results::Create(granted)));
   Release();  // Balanced in Run().
 }
 
+std::unique_ptr<const PermissionSet>
+PermissionsRequestFunction::TakePromptedPermissionsForTesting() {
+  return std::move(prompted_permissions_for_testing_);
+}
+
 }  // namespace extensions

chrome/browser/extensions/api/permissions/permissions_api.h

@@ -60,8 +60,12 @@ class PermissionsRequestFunction : public UIThreadExtensionFunction {
 
   // FOR TESTS ONLY to bypass the confirmation UI.
   static void SetAutoConfirmForTests(bool should_proceed);
+  static void ResetAutoConfirmForTests();
   static void SetIgnoreUserGestureForTests(bool ignore);
 
+  // Returns the set of permissions that the user was prompted for, if any.
+  std::unique_ptr<const PermissionSet> TakePromptedPermissionsForTesting();
+
  protected:
   ~PermissionsRequestFunction() override;
 
@@ -72,7 +76,16 @@ class PermissionsRequestFunction : public UIThreadExtensionFunction {
   void OnInstallPromptDone(ExtensionInstallPrompt::Result result);
 
   std::unique_ptr<ExtensionInstallPrompt> install_ui_;
-  std::unique_ptr<const PermissionSet> requested_permissions_;
+
+  // Requested permissions that are currently withheld.
+  std::unique_ptr<const PermissionSet> requested_withheld_;
+  // Requested permissions that are currently optional, and not granted.
+  std::unique_ptr<const PermissionSet> requested_optional_;
+
+  // The permissions, if any, that Chrome would prompt the user for. This will
+  // be recorded if and only if the prompt is being bypassed for a test (see
+  // also SetAutoConfirmForTests()).
+  std::unique_ptr<const PermissionSet> prompted_permissions_for_testing_;
 
   DISALLOW_COPY_AND_ASSIGN(PermissionsRequestFunction);
 };

chrome/browser/extensions/api/permissions/permissions_api_unittest.cc

@@ -24,6 +24,19 @@ namespace extensions {
 
 namespace {
 
+// Returns a list of |patterns| as strings, making it easy to compare for
+// equality with readable errors.
+// TODO(devlin): This was blatantly copy-pasted from
+// scriptable_permissions_modifier_unittest.cc. Put it somewhere common.
+std::vector<std::string> GetPatternsAsStrings(const URLPatternSet& patterns) {
+  std::vector<std::string> pattern_strings;
+  pattern_strings.reserve(patterns.size());
+  for (const auto& pattern : patterns)
+    pattern_strings.push_back(pattern.GetAsString());
+
+  return pattern_strings;
+}
+
 scoped_refptr<const Extension> CreateExtensionWithPermissions(
     std::unique_ptr<base::Value> permissions,
     const std::string& name,
@@ -45,6 +58,45 @@ scoped_refptr<const Extension> CreateExtensionWithPermissions(
       .Build();
 }
 
+// Helper function to create a base::Value from a list of strings.
+std::unique_ptr<base::Value> StringVectorToValue(
+    const std::vector<std::string>& strings) {
+  ListBuilder builder;
+  for (const auto& str : strings)
+    builder.Append(str);
+  return builder.Build();
+}
+
+// Runs permissions.request() with the provided |args|, and returns the result
+// of the API call. Expects the function to succeed.
+// Populates |did_prompt_user| with whether the user would be prompted for the
+// new permissions.
+bool RunRequestFunction(
+    const Extension& extension,
+    Browser* browser,
+    const char* args,
+    std::unique_ptr<const PermissionSet>* prompted_permissions_out) {
+  auto function = base::MakeRefCounted<PermissionsRequestFunction>();
+  function->set_user_gesture(true);
+  function->set_extension(&extension);
+  std::unique_ptr<base::Value> result(
+      extension_function_test_utils::RunFunctionAndReturnSingleResult(
+          function.get(), args, browser, api_test_utils::NONE));
+  if (!function->GetError().empty()) {
+    ADD_FAILURE() << "Unexpected function error: " << function->GetError();
+    return false;
+  }
+
+  if (!result || !result->is_bool()) {
+    ADD_FAILURE() << "Unexpected function result.";
+    return false;
+  }
+
+  *prompted_permissions_out = function->TakePromptedPermissionsForTesting();
+
+  return result->GetBool();
+}
+
 }  // namespace
 
 class PermissionsAPIUnitTest : public ExtensionServiceTestWithInstall {
@@ -73,10 +125,20 @@ class PermissionsAPIUnitTest : public ExtensionServiceTestWithInstall {
     return has_permission;
   }
 
+  // Adds the extension to the ExtensionService, and grants any inital
+  // permissions.
+  void AddExtensionAndGrantPermissions(const Extension& extension) {
+    PermissionsUpdater updater(profile());
+    updater.InitializePermissions(&extension);
+    updater.GrantActivePermissions(&extension);
+    service()->AddExtension(&extension);
+  }
+
  private:
   // ExtensionServiceTestBase:
   void SetUp() override {
     ExtensionServiceTestWithInstall::SetUp();
+    PermissionsRequestFunction::SetAutoConfirmForTests(true);
     InitializeEmptyExtensionService();
     browser_window_.reset(new TestBrowserWindow());
     Browser::CreateParams params(profile(), true);
@@ -88,6 +150,7 @@ class PermissionsAPIUnitTest : public ExtensionServiceTestWithInstall {
   void TearDown() override {
     browser_.reset();
     browser_window_.reset();
+    PermissionsRequestFunction::ResetAutoConfirmForTests();
     ExtensionServiceTestWithInstall::TearDown();
   }
 
@@ -136,6 +199,8 @@ TEST_F(PermissionsAPIUnitTest, ContainsAndGetAllWithRuntimeHostPermissions) {
       ExtensionBuilder("extension")
           .AddPermissions({"https://example.com/"})
           .Build();
+
+  AddExtensionAndGrantPermissions(*extension);
   PermissionsUpdater updater(profile());
   updater.InitializePermissions(extension.get());
   updater.GrantActivePermissions(extension.get());
@@ -204,4 +269,245 @@ TEST_F(PermissionsAPIUnitTest, ContainsAndGetAllWithRuntimeHostPermissions) {
   EXPECT_THAT(get_all(), testing::IsEmpty());
 }
 
+// Tests requesting withheld permissions with the permissions.request() API.
+TEST_F(PermissionsAPIUnitTest, RequestingWithheldPermissions) {
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitAndEnableFeature(
+      extensions_features::kRuntimeHostPermissions);
+
+  // Create an extension with required host permissions, and withhold those
+  // permissions.
+  scoped_refptr<const Extension> extension =
+      ExtensionBuilder("extension")
+          .AddPermissions({"https://example.com/*", "https://google.com/*"})
+          .Build();
+  AddExtensionAndGrantPermissions(*extension);
+  ScriptingPermissionsModifier(profile(), extension)
+      .SetWithholdHostPermissions(true);
+
+  const GURL kExampleCom("https://example.com");
+  const GURL kGoogleCom("https://google.com");
+  const PermissionsData* permissions_data = extension->permissions_data();
+  EXPECT_TRUE(
+      permissions_data->active_permissions().effective_hosts().is_empty());
+
+  // Request one of the withheld permissions.
+  std::unique_ptr<const PermissionSet> prompted_permissions;
+  EXPECT_TRUE(RunRequestFunction(*extension, browser(),
+                                 R"([{"origins": ["https://example.com/*"]}])",
+                                 &prompted_permissions));
+  ASSERT_TRUE(prompted_permissions);
+  EXPECT_THAT(GetPatternsAsStrings(prompted_permissions->effective_hosts()),
+              testing::UnorderedElementsAre("https://example.com/*"));
+
+  // The withheld permission should be granted.
+  EXPECT_TRUE(
+      permissions_data->active_permissions().effective_hosts().MatchesURL(
+          kExampleCom));
+  EXPECT_FALSE(
+      permissions_data->active_permissions().effective_hosts().MatchesURL(
+          kGoogleCom));
+}
+
+// Tests an extension re-requesting an optional host after the user removes it.
+TEST_F(PermissionsAPIUnitTest, ReRequestingWithheldOptionalPermissions) {
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitAndEnableFeature(
+      extensions_features::kRuntimeHostPermissions);
+
+  // Create an extension an optional host permissions, and withhold those
+  // permissions.
+  scoped_refptr<const Extension> extension =
+      ExtensionBuilder("extension")
+          .SetManifestKey("optional_permissions",
+                          StringVectorToValue({"https://chromium.org/*"}))
+          .Build();
+  AddExtensionAndGrantPermissions(*extension);
+
+  const GURL kChromiumOrg("https://chromium.org");
+  const PermissionsData* permissions_data = extension->permissions_data();
+  EXPECT_TRUE(
+      permissions_data->active_permissions().effective_hosts().is_empty());
+  {
+    std::unique_ptr<const PermissionSet> prompted_permissions;
+    EXPECT_TRUE(RunRequestFunction(
+        *extension, browser(),
+        R"([{"origins": ["https://chromium.org/*"]}])", &prompted_permissions));
+    ASSERT_TRUE(prompted_permissions);
+    EXPECT_THAT(GetPatternsAsStrings(prompted_permissions->effective_hosts()),
+                testing::UnorderedElementsAre("https://chromium.org/*"));
+  }
+
+  EXPECT_TRUE(
+      permissions_data->active_permissions().effective_hosts().MatchesURL(
+          kChromiumOrg));
+
+  {
+    URLPattern chromium_org_pattern(Extension::kValidHostPermissionSchemes,
+                                    "https://chromium.org/*");
+    PermissionSet permissions(APIPermissionSet(), ManifestPermissionSet(),
+                              URLPatternSet({chromium_org_pattern}),
+                              URLPatternSet());
+    PermissionsUpdater(profile()).RevokeRuntimePermissions(*extension,
+                                                           permissions);
+  }
+  EXPECT_TRUE(
+      permissions_data->active_permissions().effective_hosts().is_empty());
+
+  PermissionsRequestFunction::SetAutoConfirmForTests(false);
+  {
+    std::unique_ptr<const PermissionSet> prompted_permissions;
+    EXPECT_FALSE(RunRequestFunction(
+        *extension, browser(),
+        R"([{"origins": ["https://chromium.org/*"]}])", &prompted_permissions));
+    ASSERT_TRUE(prompted_permissions);
+    EXPECT_THAT(GetPatternsAsStrings(prompted_permissions->effective_hosts()),
+                testing::UnorderedElementsAre("https://chromium.org/*"));
+  }
+  EXPECT_TRUE(
+      permissions_data->active_permissions().effective_hosts().is_empty());
+}
+
+// Tests requesting both optional and withheld permissions in the same call to
+// permissions.request().
+TEST_F(PermissionsAPIUnitTest, RequestingWithheldAndOptionalPermissions) {
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitAndEnableFeature(
+      extensions_features::kRuntimeHostPermissions);
+
+  // Create an extension with required and optional host permissions, and
+  // withhold the required permissions.
+  scoped_refptr<const Extension> extension =
+      ExtensionBuilder("extension")
+          .AddPermissions({"https://example.com/*", "https://google.com/*"})
+          .SetManifestKey("optional_permissions",
+                          StringVectorToValue({"https://chromium.org/*"}))
+          .Build();
+  AddExtensionAndGrantPermissions(*extension);
+  ScriptingPermissionsModifier(profile(), extension)
+      .SetWithholdHostPermissions(true);
+
+  const GURL kExampleCom("https://example.com");
+  const GURL kGoogleCom("https://google.com");
+  const GURL kChromiumOrg("https://chromium.org");
+  const PermissionsData* permissions_data = extension->permissions_data();
+  EXPECT_TRUE(
+      permissions_data->active_permissions().effective_hosts().is_empty());
+
+  // Request one of the withheld host permissions and an optional host
+  // permission in the same call.
+  std::unique_ptr<const PermissionSet> prompted_permissions;
+  EXPECT_TRUE(RunRequestFunction(
+      *extension, browser(),
+      R"([{"origins": ["https://example.com/*", "https://chromium.org/*"]}])",
+      &prompted_permissions));
+  ASSERT_TRUE(prompted_permissions);
+  EXPECT_THAT(GetPatternsAsStrings(prompted_permissions->effective_hosts()),
+              testing::UnorderedElementsAre("https://chromium.org/*",
+                                            "https://example.com/*"));
+
+  // The requested permissions should be added.
+  EXPECT_TRUE(
+      permissions_data->active_permissions().effective_hosts().MatchesURL(
+          kExampleCom));
+  EXPECT_FALSE(
+      permissions_data->active_permissions().effective_hosts().MatchesURL(
+          kGoogleCom));
+  EXPECT_TRUE(
+      permissions_data->active_permissions().effective_hosts().MatchesURL(
+          kChromiumOrg));
+}
+
+// Tests requesting permissions that weren't specified in the manifest (either
+// in optional permissions or in required permissions).
+TEST_F(PermissionsAPIUnitTest, RequestingPermissionsNotSpecifiedInManifest) {
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitAndEnableFeature(
+      extensions_features::kRuntimeHostPermissions);
+
+  // Create an extension with required and optional host permissions, and
+  // withhold the required permissions.
+  scoped_refptr<const Extension> extension =
+      ExtensionBuilder("extension")
+          .AddPermissions({
+              "https://example.com/*",
+          })
+          .SetManifestKey("optional_permissions",
+                          StringVectorToValue({"https://chromium.org/*"}))
+          .Build();
+  AddExtensionAndGrantPermissions(*extension);
+  ScriptingPermissionsModifier(profile(), extension)
+      .SetWithholdHostPermissions(true);
+
+  const GURL kExampleCom("https://example.com");
+  const GURL kGoogleCom("https://google.com");
+  const GURL kChromiumOrg("https://chromium.org");
+
+  // Request permission for an optional and required permission, as well as a
+  // permission that wasn't specified in the manifest. The call should fail.
+  // Note: Not using RunRequestFunction(), since that expects function success.
+  auto function = base::MakeRefCounted<PermissionsRequestFunction>();
+  function->set_user_gesture(true);
+  function->set_extension(extension.get());
+  EXPECT_EQ("Only permissions specified in the manifest may be requested.",
+            extension_function_test_utils::RunFunctionAndReturnError(
+                function.get(),
+                R"([{
+               "origins": [
+                 "https://example.com/*",
+                 "https://chromium.org/*",
+                 "https://google.com/*"
+               ]
+             }])",
+                browser(), api_test_utils::NONE));
+}
+
+// Tests requesting withheld permissions that have already been granted.
+TEST_F(PermissionsAPIUnitTest, RequestingAlreadyGrantedWithheldPermissions) {
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitAndEnableFeature(
+      extensions_features::kRuntimeHostPermissions);
+
+  // Create an extension with required host permissions, withhold host
+  // permissions, and then grant one of the hosts.
+  scoped_refptr<const Extension> extension =
+      ExtensionBuilder("extension")
+          .AddPermissions({"https://example.com/*", "https://google.com/*"})
+          .Build();
+  AddExtensionAndGrantPermissions(*extension);
+  ScriptingPermissionsModifier modifier(profile(), extension);
+  modifier.SetWithholdHostPermissions(true);
+
+  const GURL kExampleCom("https://example.com");
+  const GURL kGoogleCom("https://google.com");
+  modifier.GrantHostPermission(kExampleCom);
+
+  const PermissionsData* permissions_data = extension->permissions_data();
+  EXPECT_TRUE(
+      permissions_data->active_permissions().effective_hosts().MatchesURL(
+          kExampleCom));
+  EXPECT_FALSE(
+      permissions_data->active_permissions().effective_hosts().MatchesURL(
+          kGoogleCom));
+
+  // Request the already-granted host permission. The function should succeed
+  // (without even prompting the user), and the permission should (still) be
+  // granted.
+  PermissionsRequestFunction::SetAutoConfirmForTests(false);
+
+  std::unique_ptr<const PermissionSet> prompted_permissions;
+  EXPECT_TRUE(RunRequestFunction(*extension, browser(),
+                                 R"([{"origins": ["https://example.com/*"]}])",
+                                 &prompted_permissions));
+  ASSERT_FALSE(prompted_permissions);
+
+  // The withheld permission should be granted.
+  EXPECT_TRUE(
+      permissions_data->active_permissions().effective_hosts().MatchesURL(
+          kExampleCom));
+  EXPECT_FALSE(
+      permissions_data->active_permissions().effective_hosts().MatchesURL(
+          kGoogleCom));
+}
+
 }  // namespace extensions

chrome/browser/extensions/api/permissions/permissions_apitest.cc

@@ -87,9 +87,10 @@ IN_PROC_BROWSER_TEST_F(PermissionsApiTest, OptionalPermissionsGranted) {
   AddPattern(&explicit_hosts, "http://*.c.com/*");
 
   ExtensionPrefs* prefs = ExtensionPrefs::Get(browser()->profile());
-  prefs->AddGrantedPermissions("kjmkgkdkpedkejedfhmfcenooemhbpbo",
-                               PermissionSet(apis, manifest_permissions,
-                                             explicit_hosts, URLPatternSet()));
+  prefs->AddRuntimeGrantedPermissions(
+      "kjmkgkdkpedkejedfhmfcenooemhbpbo",
+      PermissionSet(apis, manifest_permissions, explicit_hosts,
+                    URLPatternSet()));
 
   PermissionsRequestFunction::SetIgnoreUserGestureForTests(true);
   ASSERT_TRUE(StartEmbeddedTestServer());
@@ -114,9 +115,10 @@ IN_PROC_BROWSER_TEST_F(PermissionsApiTest, OptionalPermissionsDeny) {
   apis.insert(APIPermission::kManagement);
 
   ExtensionPrefs* prefs = ExtensionPrefs::Get(browser()->profile());
-  prefs->AddGrantedPermissions("kjmkgkdkpedkejedfhmfcenooemhbpbo",
-                               PermissionSet(apis, ManifestPermissionSet(),
-                                             URLPatternSet(), URLPatternSet()));
+  prefs->AddRuntimeGrantedPermissions(
+      "kjmkgkdkpedkejedfhmfcenooemhbpbo",
+      PermissionSet(apis, ManifestPermissionSet(), URLPatternSet(),
+                    URLPatternSet()));
 
   PermissionsRequestFunction::SetAutoConfirmForTests(false);
   PermissionsRequestFunction::SetIgnoreUserGestureForTests(true);

chrome/test/data/extensions/api_test/permissions/host_subsets/background.js

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-var ERROR = 'Optional permissions must be listed in extension manifest.';
+var ERROR = 'Only permissions specified in the manifest may be requested.';
 var test = chrome.test;
 
 // The URL patterns that we've supposedly been granted access to so far. Use

chrome/test/data/extensions/api_test/permissions/optional/background.js

@@ -11,7 +11,7 @@ var pass = chrome.test.callbackPass;
 var listenOnce = chrome.test.listenOnce;
 
 var NOT_OPTIONAL_ERROR =
-    "Optional permissions must be listed in extension manifest.";
+    "Only permissions specified in the manifest may be requested.";
 
 var REQUIRED_ERROR =
     "You cannot remove required permissions.";