Commit c392aa5f authored by kinaba@chromium.org's avatar kinaba@chromium.org

Grant Files.app access for mount points added after its launch.

Previous implementation of GrantFullAccessToExtension() granted permissions
only for paths that are mounted at that moment, and required hack for
one dynamically mounted path, namely, Google Drive directory.

It's not what is meant for GrantFullAccessToExtension(). Rather, it
should just simply allow access for the extension any time.
In multi-profile settings, more directories, i.e., Downloads/Drive
directories from other profiles are going to be dynamically added.

BUG=337705

Review URL: https://codereview.chromium.org/148233008

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@247852 0039d316-1c4b-4281-b951-d872f2087c98
parent 916a66a9
...@@ -45,36 +45,6 @@ using fileapi::FileSystemURL; ...@@ -45,36 +45,6 @@ using fileapi::FileSystemURL;
namespace extensions { namespace extensions {
namespace { namespace {
// Sets permissions for the Drive mount point so Files.app can access files
// in the mount point directory. It's safe to call this function even if
// Drive is disabled by the setting (i.e. prefs::kDisableDrive is true).
void SetDriveMountPointPermissions(
Profile* profile,
const std::string& extension_id,
content::RenderViewHost* render_view_host) {
if (!render_view_host ||
!render_view_host->GetSiteInstance() || !render_view_host->GetProcess()) {
return;
}
fileapi::ExternalFileSystemBackend* backend =
file_manager::util::GetFileSystemContextForRenderViewHost(
profile, render_view_host)->external_backend();
if (!backend)
return;
const base::FilePath mount_point =
drive::util::GetDriveMountPointPath(profile);
// Grant R/W permissions to drive 'folder'. File API layer still
// expects this to be satisfied.
ChildProcessSecurityPolicy::GetInstance()->GrantCreateReadWriteFile(
render_view_host->GetProcess()->GetID(), mount_point);
base::FilePath mount_point_virtual;
if (backend->GetVirtualPath(mount_point, &mount_point_virtual))
backend->GrantFileAccessToExtension(extension_id, mount_point_virtual);
}
// Retrieves total and remaining available size on |mount_path|. // Retrieves total and remaining available size on |mount_path|.
void GetSizeStatsOnBlockingPool(const std::string& mount_path, void GetSizeStatsOnBlockingPool(const std::string& mount_path,
uint64* total_size, uint64* total_size,
...@@ -305,8 +275,9 @@ bool FileBrowserPrivateRequestFileSystemFunction::RunImpl() { ...@@ -305,8 +275,9 @@ bool FileBrowserPrivateRequestFileSystemFunction::RunImpl() {
// Note that we call this function even when Drive is disabled by the // Note that we call this function even when Drive is disabled by the
// setting. Otherwise, we need to call this when the setting is changed at // setting. Otherwise, we need to call this when the setting is changed at
// a later time, which complicates the code. // a later time, which complicates the code.
SetDriveMountPointPermissions( ChildProcessSecurityPolicy::GetInstance()->GrantCreateReadWriteFile(
GetProfile(), extension_id(), render_view_host()); render_view_host()->GetProcess()->GetID(),
drive::util::GetDriveMountPointPath(GetProfile()));
fileapi::FileSystemInfo info = fileapi::FileSystemInfo info =
fileapi::GetFileSystemInfoForChromeOS(source_url_.GetOrigin()); fileapi::GetFileSystemInfoForChromeOS(source_url_.GetOrigin());
......
...@@ -4,29 +4,32 @@ ...@@ -4,29 +4,32 @@
#include "chrome/browser/chromeos/fileapi/file_access_permissions.h" #include "chrome/browser/chromeos/fileapi/file_access_permissions.h"
#include "base/command_line.h"
#include "base/logging.h" #include "base/logging.h"
namespace chromeos { namespace chromeos {
namespace {
// Empty path is prefix of any other paths, hence it represents full permission.
base::FilePath FullPermission() { return base::FilePath(); }
} // namespace
FileAccessPermissions::FileAccessPermissions() {} FileAccessPermissions::FileAccessPermissions() {}
FileAccessPermissions::~FileAccessPermissions() {} FileAccessPermissions::~FileAccessPermissions() {}
void FileAccessPermissions::GrantFullAccessPermission(
const std::string& extension_id) {
base::AutoLock locker(lock_);
path_map_[extension_id].insert(FullPermission());
}
void FileAccessPermissions::GrantAccessPermission( void FileAccessPermissions::GrantAccessPermission(
const std::string& extension_id, const base::FilePath& path) { const std::string& extension_id, const base::FilePath& path) {
DCHECK(!path.empty());
base::AutoLock locker(lock_); base::AutoLock locker(lock_);
PathAccessMap::iterator path_map_iter = path_map_.find(extension_id); path_map_[extension_id].insert(path);
if (path_map_iter == path_map_.end()) {
PathSet path_set;
path_set.insert(path);
path_map_.insert(PathAccessMap::value_type(extension_id, path_set));
} else {
if (path_map_iter->second.find(path) != path_map_iter->second.end())
return;
path_map_iter->second.insert(path);
}
} }
bool FileAccessPermissions::HasAccessPermission( bool FileAccessPermissions::HasAccessPermission(
...@@ -35,13 +38,17 @@ bool FileAccessPermissions::HasAccessPermission( ...@@ -35,13 +38,17 @@ bool FileAccessPermissions::HasAccessPermission(
PathAccessMap::const_iterator path_map_iter = path_map_.find(extension_id); PathAccessMap::const_iterator path_map_iter = path_map_.find(extension_id);
if (path_map_iter == path_map_.end()) if (path_map_iter == path_map_.end())
return false; return false;
const PathSet& path_set = path_map_iter->second;
if (path_set.find(FullPermission()) != path_set.end())
return true;
// Check this file and walk up its directory tree to find if this extension // Check this file and walk up its directory tree to find if this extension
// has access to it. // has access to it.
base::FilePath current_path = path.StripTrailingSeparators(); base::FilePath current_path = path.StripTrailingSeparators();
base::FilePath last_path; base::FilePath last_path;
while (current_path != last_path) { while (current_path != last_path) {
if (path_map_iter->second.find(current_path) != path_map_iter->second.end()) if (path_set.find(current_path) != path_set.end())
return true; return true;
last_path = current_path; last_path = current_path;
current_path = current_path.DirName(); current_path = current_path.DirName();
......
...@@ -12,15 +12,18 @@ ...@@ -12,15 +12,18 @@
#include "base/basictypes.h" #include "base/basictypes.h"
#include "base/files/file_path.h" #include "base/files/file_path.h"
#include "base/synchronization/lock.h" #include "base/synchronization/lock.h"
#include "webkit/browser/webkit_storage_browser_export.h"
namespace chromeos { namespace chromeos {
// In a thread safe manner maintains the set of paths allowed to access for
// each extension.
class FileAccessPermissions { class FileAccessPermissions {
public: public:
FileAccessPermissions(); FileAccessPermissions();
virtual ~FileAccessPermissions(); virtual ~FileAccessPermissions();
// Grants |extension_id| full access to all paths.
void GrantFullAccessPermission(const std::string& extension_id);
// Grants |extension_id| access to |path|. // Grants |extension_id| access to |path|.
void GrantAccessPermission(const std::string& extension_id, void GrantAccessPermission(const std::string& extension_id,
const base::FilePath& path); const base::FilePath& path);
......
...@@ -47,6 +47,12 @@ TEST(FileAccessPermissionsTest, FileAccessChecks) { ...@@ -47,6 +47,12 @@ TEST(FileAccessPermissionsTest, FileAccessChecks) {
EXPECT_TRUE(permissions.HasAccessPermission(extension2, good_file)); EXPECT_TRUE(permissions.HasAccessPermission(extension2, good_file));
EXPECT_TRUE(permissions.HasAccessPermission(extension2, bad_file)); EXPECT_TRUE(permissions.HasAccessPermission(extension2, bad_file));
// After granting full access, it can access all directories and files.
permissions.GrantFullAccessPermission(extension1);
EXPECT_TRUE(permissions.HasAccessPermission(extension1, good_dir));
EXPECT_TRUE(permissions.HasAccessPermission(extension1, good_file));
EXPECT_TRUE(permissions.HasAccessPermission(extension1, bad_file));
// After revoking rights for extensions, they should not be able to access // After revoking rights for extensions, they should not be able to access
// any file system element anymore. // any file system element anymore.
permissions.RevokePermissions(extension1); permissions.RevokePermissions(extension1);
......
...@@ -147,16 +147,7 @@ void FileSystemBackend::GrantFullAccessToExtension( ...@@ -147,16 +147,7 @@ void FileSystemBackend::GrantFullAccessToExtension(
DCHECK(special_storage_policy_->IsFileHandler(extension_id)); DCHECK(special_storage_policy_->IsFileHandler(extension_id));
if (!special_storage_policy_->IsFileHandler(extension_id)) if (!special_storage_policy_->IsFileHandler(extension_id))
return; return;
file_access_permissions_->GrantFullAccessPermission(extension_id);
std::vector<fileapi::MountPoints::MountPointInfo> files;
mount_points_->AddMountPointInfosTo(&files);
system_mount_points_->AddMountPointInfosTo(&files);
for (size_t i = 0; i < files.size(); ++i) {
file_access_permissions_->GrantAccessPermission(
extension_id,
base::FilePath::FromUTF8Unsafe(files[i].name));
}
} }
void FileSystemBackend::GrantFileAccessToExtension( void FileSystemBackend::GrantFileAccessToExtension(
......
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