diff options
7 files changed, 88 insertions, 50 deletions
diff --git a/chrome/browser/extensions/extension_chrome_auth_private_apitest.cc b/chrome/browser/extensions/extension_chrome_auth_private_apitest.cc index dc142461..b35ef72a 100644 --- a/chrome/browser/extensions/extension_chrome_auth_private_apitest.cc +++ b/chrome/browser/extensions/extension_chrome_auth_private_apitest.cc @@ -60,17 +60,9 @@ IN_PROC_BROWSER_TEST_F(ExtensionChromeAuthPrivateApiTest, #endif // !defined(OS_CHROMEOS) IN_PROC_BROWSER_TEST_F(ExtensionChromeAuthPrivateApiTest, - SetCloudPrintCredentialsFailureInstalled) { - // Run this as an installed app. Since this is not a component app, it - // should fail. - ASSERT_TRUE(RunExtensionTest("chrome_auth_private/installed_app")); -} - -IN_PROC_BROWSER_TEST_F(ExtensionChromeAuthPrivateApiTest, SetCloudPrintCredentialsFailureInstalledComponent) { // Run this as an installed component app. This should also fail because of // the explicit URL check in the API. ASSERT_TRUE(RunComponentExtensionTest( "chrome_auth_private/installed_component_app")); } - diff --git a/chrome/browser/extensions/extension_info_private_apitest_chromeos.cc b/chrome/browser/extensions/extension_info_private_apitest_chromeos.cc index 3943b5a..da9ade1 100644 --- a/chrome/browser/extensions/extension_info_private_apitest_chromeos.cc +++ b/chrome/browser/extensions/extension_info_private_apitest_chromeos.cc @@ -6,8 +6,3 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, CustomizationPrivateTest) { ASSERT_TRUE(RunComponentExtensionTest("chromeos_info_private")) << message_; } - -IN_PROC_BROWSER_TEST_F(ExtensionApiTest, CustomizationPrivateFailTest) { - // Only component extensions can use it. - ASSERT_FALSE(RunExtensionTest("chromeos_info_private")) << message_; -} diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc index 00b4d40..384d07e 100644 --- a/chrome/common/extensions/extension.cc +++ b/chrome/common/extensions/extension.cc @@ -1491,6 +1491,20 @@ bool Extension::InitFromValue(const DictionaryValue& source, int flags, base::i18n::AdjustStringForLocaleDirection(&localized_name); name_ = UTF16ToUTF8(localized_name); + // Load App settings. LoadExtent at least has to be done before + // ParsePermissions(), because the valid permissions depend on what type of + // package this is. + if (!LoadIsApp(manifest_value_.get(), error) || + !LoadExtent(manifest_value_.get(), keys::kWebURLs, + &extent_, + errors::kInvalidWebURLs, errors::kInvalidWebURL, + parse_strictness, error) || + !EnsureNotHybridApp(manifest_value_.get(), error) || + !LoadLaunchURL(manifest_value_.get(), error) || + !LoadLaunchContainer(manifest_value_.get(), error)) { + return false; + } + // Initialize the permissions (optional). ExtensionAPIPermissionSet api_permissions; URLPatternSet host_permissions; @@ -1898,17 +1912,7 @@ bool Extension::InitFromValue(const DictionaryValue& source, int flags, return false; // Failed to parse file browser actions definition. } - // Load App settings. - if (!LoadIsApp(manifest_value_.get(), error) || - !LoadExtent(manifest_value_.get(), keys::kWebURLs, - &extent_, - errors::kInvalidWebURLs, errors::kInvalidWebURL, - parse_strictness, error) || - !EnsureNotHybridApp(manifest_value_.get(), error) || - !LoadLaunchURL(manifest_value_.get(), error) || - !LoadLaunchContainer(manifest_value_.get(), error)) - return false; - + // App isolation. if (api_permissions.count(ExtensionAPIPermission::kExperimental)) { if (!LoadAppIsolation(manifest_value_.get(), error)) return false; @@ -2560,14 +2564,9 @@ bool Extension::ParsePermissions(const DictionaryValue* source, ExtensionPermissionsInfo::GetInstance()->GetByName(permission_str); if (permission != NULL) { - if (CanSpecifyAPIPermission(permission, error)) - api_permissions->insert(permission->id()); - - // Sometimes when you can't specify an API permission we ignore it - // silently. This seems like a bug. Specifically, crbug.com/100489. - if (!error->empty()) + if (!CanSpecifyAPIPermission(permission, error)) return false; - + api_permissions->insert(permission->id()); continue; } @@ -2771,8 +2770,11 @@ bool Extension::CanSpecifyAPIPermission( const ExtensionAPIPermission* permission, std::string* error) const { if (permission->is_component_only()) { - if (!CanSpecifyComponentOnlyPermission()) + if (!CanSpecifyComponentOnlyPermission()) { + *error = ExtensionErrorUtils::FormatErrorMessage( + errors::kPermissionNotAllowed, permission->name()); return false; + } } if (permission->id() == ExtensionAPIPermission::kExperimental) { @@ -2783,8 +2785,11 @@ bool Extension::CanSpecifyAPIPermission( } if (is_hosted_app()) { - if (!CanSpecifyPermissionForHostedApp(permission)) + if (!CanSpecifyPermissionForHostedApp(permission)) { + *error = ExtensionErrorUtils::FormatErrorMessage( + errors::kPermissionNotAllowed, permission->name()); return false; + } } return true; diff --git a/chrome/common/extensions/extension_constants.cc b/chrome/common/extensions/extension_constants.cc index b3b60d3..e37b6de 100644 --- a/chrome/common/extensions/extension_constants.cc +++ b/chrome/common/extensions/extension_constants.cc @@ -413,6 +413,8 @@ const char* kNoWildCardsInPaths = "Wildcards are not allowed in extent URL pattern paths."; const char* kOneUISurfaceOnly = "Only one of 'browser_action', 'page_action', and 'app' can be specified."; +const char* kPermissionNotAllowed = + "Access to permission '*' denied."; const char* kReservedMessageFound = "Reserved key * found in message catalog."; const char* kSidebarExperimental = diff --git a/chrome/common/extensions/extension_constants.h b/chrome/common/extensions/extension_constants.h index 27904c6..804fb0e 100644 --- a/chrome/common/extensions/extension_constants.h +++ b/chrome/common/extensions/extension_constants.h @@ -270,6 +270,7 @@ namespace extension_manifest_errors { extern const char* kMissingFile; extern const char* kMultipleOverrides; extern const char* kNoWildCardsInPaths; + extern const char* kPermissionNotAllowed; extern const char* kOneUISurfaceOnly; extern const char* kReservedMessageFound; extern const char* kSidebarExperimental; diff --git a/chrome/common/extensions/extension_manifests_unittest.cc b/chrome/common/extensions/extension_manifests_unittest.cc index 7c41fd5..0008ef7 100644 --- a/chrome/common/extensions/extension_manifests_unittest.cc +++ b/chrome/common/extensions/extension_manifests_unittest.cc @@ -620,39 +620,82 @@ TEST_F(ExtensionManifestTest, OptionsPageInApps) { errors::kInvalidOptionsPageExpectUrlInPackage); } -TEST_F(ExtensionManifestTest, AllowUnrecognizedPermissions) { +TEST_F(ExtensionManifestTest, HostedAppPermissions) { std::string error; scoped_ptr<DictionaryValue> manifest( - LoadManifestFile("valid_app.json", &error)); + LoadManifestFile("hosted_app_absolute_options.json", &error)); ASSERT_TRUE(manifest.get()); - CommandLine old_command_line = *CommandLine::ForCurrentProcess(); + ListValue* permissions = NULL; + ASSERT_TRUE(manifest->GetList("permissions", &permissions)); - ListValue *permissions = new ListValue(); - manifest->Set(keys::kPermissions, permissions); ExtensionPermissionsInfo* info = ExtensionPermissionsInfo::GetInstance(); ExtensionAPIPermissionSet api_perms = info->GetAll(); for (ExtensionAPIPermissionSet::iterator i = api_perms.begin(); i != api_perms.end(); ++i) { + if (*i == ExtensionAPIPermission::kExperimental) + continue; + ExtensionAPIPermission* permission = info->GetByID(*i); const char* name = permission->name(); StringValue* p = new StringValue(name); permissions->Clear(); permissions->Append(p); - std::string message_name = base::StringPrintf("permission-%s", name); + Extension::Location location = Extension::INTERNAL; - if (*i == ExtensionAPIPermission::kExperimental) { - // Experimental permission is allowed, but requires this switch. - CommandLine::ForCurrentProcess()->AppendSwitch( - switches::kEnableExperimentalExtensionApis); + // Many permissions are not available to hosted apps. + if (!permission->is_hosted_app() || permission->is_component_only()) { + LoadAndExpectError(Manifest(manifest.get(), name), + errors::kPermissionNotAllowed); + + // ... unless the hosted app is a component app. + location = Extension::COMPONENT; } - // Extensions are allowed to contain unrecognized API permissions, - // so there shouldn't be any errors. - scoped_refptr<Extension> extension; - extension = LoadAndExpectSuccess( - Manifest(manifest.get(), message_name.c_str())); + LoadAndExpectSuccess(Manifest(manifest.get(), name), location); } - *CommandLine::ForCurrentProcess() = old_command_line; +} + +TEST_F(ExtensionManifestTest, ComponentOnlyPermission) { + std::string error; + scoped_ptr<DictionaryValue> manifest( + LoadManifestFile("init_valid_minimal.json", &error)); + ASSERT_TRUE(manifest.get()); + ListValue* permissions = new ListValue(); + manifest->Set("permissions", permissions); + + ExtensionPermissionsInfo* info = ExtensionPermissionsInfo::GetInstance(); + ExtensionAPIPermissionSet api_perms = info->GetAll(); + for (ExtensionAPIPermissionSet::iterator i = api_perms.begin(); + i != api_perms.end(); ++i) { + if (*i == ExtensionAPIPermission::kExperimental) + continue; + + ExtensionAPIPermission* permission = info->GetByID(*i); + const char* name = permission->name(); + StringValue* p = new StringValue(name); + permissions->Clear(); + permissions->Append(p); + + if (!permission->is_component_only()) + continue; + + // Component-only extensions should only be enabled for component + // extensions. + LoadAndExpectError(Manifest(manifest.get(), name), + errors::kPermissionNotAllowed); + LoadAndExpectSuccess(Manifest(manifest.get(), name), + Extension::COMPONENT); + } +} + +TEST_F(ExtensionManifestTest, AllowUnrecognizedPermissions) { + std::string error; + scoped_ptr<DictionaryValue> manifest( + LoadManifestFile("valid_app.json", &error)); + ListValue* permissions = NULL; + ASSERT_TRUE(manifest->GetList("permissions", &permissions)); + permissions->Append(new StringValue("not-a-valid-permission")); + LoadAndExpectSuccess(Manifest(manifest.get(), "")); } TEST_F(ExtensionManifestTest, NormalizeIconPaths) { diff --git a/chrome/test/data/extensions/api_test/web_socket_proxy_private/manifest.json b/chrome/test/data/extensions/api_test/web_socket_proxy_private/manifest.json index adac703..02515c9 100644 --- a/chrome/test/data/extensions/api_test/web_socket_proxy_private/manifest.json +++ b/chrome/test/data/extensions/api_test/web_socket_proxy_private/manifest.json @@ -4,5 +4,5 @@ "version": "0.1", "description": "Test for webSocketProxyPrivate.getPassportForTCP", "background_page": "background.html", - "permissions": ["webSocketProxyPrivate", "chromeosInfoPrivate"] + "permissions": ["webSocketProxyPrivate"] } |