Respect disabling removable devices policy for enterprise users.

Respect disabling removable devices policy for enterprise users.

After recent refactoring, the volume manager is created for every profile, and listens to cros disks events on each of them. As a result, if a USB is inserted, then it is handled by all of the available profiles. If one of them has the policy disabling removable devices set to false, then all profiles (including those having the policy set to true) will get the device mounted.

To fix this, this CL prevents this situation, by not adding the VolumeInfo and not emiting an event, if the device is not permitted (but got mounted by another profile).

TEST=Tested manually on a enterprise enrolled device.
BUG=349298
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=255610

[mtomasz, 2014-03-07 07:18:54]

@kinaba: PTAL. This patch works on ToT. For M-33 the patch will not merge
easily, though.

[kinaba, 2014-03-07 07:30:02]

lgtm

[mtomasz, 2014-03-07 07:34:53]

On 2014/03/07 07:30:02, kinaba (ooo until Mar 9) wrote:
> lgtm

Thanks. For M-33, we would have to filter out in two different places. In
GetVolumeInfoList and before emitting the mount event. WDYT about such solution?

[kinaba, 2014-03-07 07:40:21]

On 2014/03/07 07:34:53, mtomasz wrote:
> On 2014/03/07 07:30:02, kinaba (ooo until Mar 9) wrote:
> > lgtm
> 
> Thanks. For M-33, we would have to filter out in two different places. In
> GetVolumeInfoList and before emitting the mount event. WDYT about such
solution?

That should work, but I'm thinking it might be safer to have what I wrote in the
bug #c24.

As you pointed out, filtering in VolumeManager doesn't stop exposing the disk in
dev console
without separated filesystem object, or direct access via file:/// URL is
another thing that
cannot be prevented.

Wrapping disk_mount_manager_->AddObserver() by if(chromeos::IsLoggedIn) should
not have such
issues in M33/34, and if I understand correctly that's exactly how it was
handled in older versions.
WDYT?

[mtomasz, 2014-03-07 07:51:06]

On 2014/03/07 07:40:21, kinaba (ooo until Mar 9) wrote:
> On 2014/03/07 07:34:53, mtomasz wrote:
> > On 2014/03/07 07:30:02, kinaba (ooo until Mar 9) wrote:
> > > lgtm
> > 
> > Thanks. For M-33, we would have to filter out in two different places. In
> > GetVolumeInfoList and before emitting the mount event. WDYT about such
> solution?
> 
> That should work, but I'm thinking it might be safer to have what I wrote in
the
> bug #c24.
> 
> As you pointed out, filtering in VolumeManager doesn't stop exposing the disk
in
> dev console
> without separated filesystem object, or direct access via file:/// URL is
> another thing that
> cannot be prevented.
> 
> Wrapping disk_mount_manager_->AddObserver() by if(chromeos::IsLoggedIn) should
> not have such
> issues in M33/34, and if I understand correctly that's exactly how it was
> handled in older versions.
> WDYT?

That would make sense. But this CL for ToT, would is also needed. So for M34/34
only IsLoggedIn() trick, and for ToT IsLoggedIn() + this CL. Am I right?

[kinaba, 2014-03-07 07:51:42]

On 2014/03/07 07:51:06, mtomasz wrote:
> On 2014/03/07 07:40:21, kinaba (ooo until Mar 9) wrote:
> > On 2014/03/07 07:34:53, mtomasz wrote:
> > > On 2014/03/07 07:30:02, kinaba (ooo until Mar 9) wrote:
> > > > lgtm
> > > 
> > > Thanks. For M-33, we would have to filter out in two different places. In
> > > GetVolumeInfoList and before emitting the mount event. WDYT about such
> > solution?
> > 
> > That should work, but I'm thinking it might be safer to have what I wrote in
> the
> > bug #c24.
> > 
> > As you pointed out, filtering in VolumeManager doesn't stop exposing the
disk
> in
> > dev console
> > without separated filesystem object, or direct access via file:/// URL is
> > another thing that
> > cannot be prevented.
> > 
> > Wrapping disk_mount_manager_->AddObserver() by if(chromeos::IsLoggedIn)
should
> > not have such
> > issues in M33/34, and if I understand correctly that's exactly how it was
> > handled in older versions.
> > WDYT?
> 
> That would make sense. But this CL for ToT, would is also needed. So for
M34/34
> only IsLoggedIn() trick, and for ToT IsLoggedIn() + this CL. Am I right?

sg.

[mtomasz, 2014-03-07 08:10:48]

The CQ bit was checked by mtomasz@chromium.org

[commit-bot: I haz the power, 2014-03-07 08:13:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mtomasz@chromium.org/190153003/1

[commit-bot: I haz the power, 2014-03-07 08:20:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-07 08:20:39]

Try jobs failed on following builders: mac_chromium_compile_dbg

[mtomasz, 2014-03-07 08:22:29]

The CQ bit was checked by mtomasz@chromium.org

[commit-bot: I haz the power, 2014-03-07 08:24:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mtomasz@chromium.org/190153003/1

[commit-bot: I haz the power, 2014-03-07 08:46:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-07 08:46:36]

Try jobs failed on following builders: linux_chromium_clang_dbg

[mtomasz, 2014-03-07 09:32:09]

The CQ bit was checked by mtomasz@chromium.org

[commit-bot: I haz the power, 2014-03-07 09:34:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mtomasz@chromium.org/190153003/1

[commit-bot: I haz the power, 2014-03-07 14:18:19]

Change committed as 255610